-
Notifications
You must be signed in to change notification settings - Fork 119
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
Improve Docker Compose container execution #687
Changes from 2 commits
8cd9f83
0cb6892
36c410a
960acdd
24ac388
18fccc7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,8 @@ import ( | |
"github.com/elastic/elastic-package/internal/signal" | ||
) | ||
|
||
const waitForHealthyTimeout = 10 * time.Minute | ||
|
||
// Project represents a Docker Compose project. | ||
type Project struct { | ||
name string | ||
|
@@ -275,20 +277,23 @@ func (p *Project) WaitForHealthy(opts CommandOptions) error { | |
return err | ||
} | ||
|
||
timeout := time.Duration(waitForHealthyTimeout) | ||
startTime := time.Now() | ||
timeoutTime := startTime.Add(timeout) | ||
interval := time.Duration(1 * time.Second) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I think you can push this one to consts too. |
||
healthy := true | ||
|
||
containerIDs := strings.Split(strings.TrimSpace(b.String()), "\n") | ||
for { | ||
for time.Now().Before(timeoutTime) { | ||
if signal.SIGINT() { | ||
return errors.New("SIGINT: cancel waiting for policy assigned") | ||
} | ||
|
||
healthy := true | ||
|
||
logger.Debugf("Wait for healthy containers: %s", strings.Join(containerIDs, ",")) | ||
descriptions, err := docker.InspectContainers(containerIDs...) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: let's leave this empty line |
||
for _, containerDescription := range descriptions { | ||
logger.Debugf("Container status: %s", containerDescription.String()) | ||
|
||
|
@@ -316,12 +321,19 @@ func (p *Project) WaitForHealthy(opts CommandOptions) error { | |
healthy = false | ||
mtojek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
// end loop before timeout if healthy | ||
if healthy { | ||
break | ||
} | ||
|
||
time.Sleep(time.Second) | ||
mtojek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// NOTE: using sleep does not guarantee interval but it's ok for this use case | ||
time.Sleep(interval) | ||
} | ||
|
||
if !healthy { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you sure about shuffling around with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the bug is obvious, I misinterpreted the logic. |
||
return errors.New("timeout waiting for healthy container") | ||
} | ||
|
||
return nil | ||
} | ||
|
||
|
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.
nit: you can push this one as
var
to the top of the file and name accordingly.