-
-
Notifications
You must be signed in to change notification settings - Fork 512
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added occurrence for the WaitLogStrategy #100
Conversation
/cc @giorgioazzinnaro @mdelapenya can you have a look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It LGTM, although I added a few cosmetic, minor comments
wait/log.go
Outdated
@@ -18,6 +18,7 @@ type LogStrategy struct { | |||
// additional properties | |||
Log string | |||
PollInterval time.Duration | |||
Occurence int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Occurence int
+ Occurrence int
wait/log.go
Outdated
@@ -26,6 +27,7 @@ func NewLogStrategy(log string) *LogStrategy { | |||
startupTimeout: defaultStartupTimeout(), | |||
Log: log, | |||
PollInterval: 100 * time.Millisecond, | |||
Occurence: 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
wait/log.go
Outdated
@@ -46,6 +48,15 @@ func (ws *LogStrategy) WithPollInterval(pollInterval time.Duration) *LogStrategy | |||
return ws | |||
} | |||
|
|||
func (ws *LogStrategy) WithOccurence(o int) *LogStrategy { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
@@ -61,6 +72,7 @@ func (ws *LogStrategy) WaitUntilReady(ctx context.Context, target StrategyTarget | |||
// limit context to startupTimeout | |||
ctx, cancelContext := context.WithTimeout(ctx, ws.startupTimeout) | |||
defer cancelContext() | |||
currentOccurence := 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- currentOccurence := 0
+ currentOccurrence := 0
@@ -77,7 +89,10 @@ LOOP: | |||
b, err := ioutil.ReadAll(reader) | |||
logs := string(b) | |||
if strings.Contains(logs, ws.Log) { | |||
break LOOP | |||
currentOccurence++ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
wait/log_test.go
Outdated
} | ||
wg := NewLogStrategy("dude"). | ||
WithStartupTimeout(100 * time.Microsecond). | ||
WithOccurence(2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
wait/log_test.go
Outdated
} | ||
} | ||
|
||
func TestWaitWithMaxOccurenceButItWillNeverHappen(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
wait/log_test.go
Outdated
} | ||
wg := NewLogStrategy("blaaa"). | ||
WithStartupTimeout(100 * time.Microsecond). | ||
WithOccurence(2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add an example with a high number of occurrences (around a hundred)? This way we will exercise the infinite loop better than with just two loops
456f324
to
f04588c
Compare
This is related to the work @giorgioazzinnaro is doing here #98 . Now you can declare how many times a log line should appear before having the container declared as ready. Signed-off-by: Gianluca Arbezzano <[email protected]>
f04588c
to
e64d86b
Compare
This is related to the work @giorgioazzinnaro is doing here
#98 .
Now you can declare how many times a log line should appear before
having the container declared as ready.
Signed-off-by: Gianluca Arbezzano [email protected]