-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
fix: add missing service container health check #2354
Conversation
@ChristopherHX this pull request has failed checks 🛠 |
🦙 MegaLinter status: ✅ SUCCESS
See detailed report in MegaLinter reports |
MYSQL_RANDOM_ROOT_PASSWORD: yes | ||
options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3 | ||
steps: | ||
- run: mysql -u dbuser -D dbname -pdbpass -h maindb -e "create table T(id INT NOT NULL AUTO_INCREMENT, val VARCHAR(255), PRIMARY KEY (id))" |
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.
currently this workflow fails to run, because maindb is still starting.
The actions runner wait for it to be ready
@@ -169,6 +169,30 @@ func (cr *containerReference) Remove() common.Executor { | |||
).IfNot(common.Dryrun) | |||
} | |||
|
|||
func (cr *containerReference) GetHealth(ctx context.Context) ContainerHealth { |
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.
Uses docker inspect to poll container health like actions/runner
@@ -452,6 +452,10 @@ func (e *HostEnvironment) GetRunnerContext(_ context.Context) map[string]interfa | |||
} | |||
} | |||
|
|||
func (e *HostEnvironment) GetHealth(ctx context.Context) ContainerHealth { |
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.
Service container are not implemented for HostEnvironment, just a stub that is probably never called
if delay > 10*time.Second { | ||
delay = 10 * time.Second | ||
} |
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.
Never poll less than every 10s, not looked up anywhere.
"Fake exponential backoff"
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2354 +/- ##
===========================================
+ Coverage 61.56% 76.34% +14.78%
===========================================
Files 53 61 +8
Lines 9002 7859 -1143
===========================================
+ Hits 5542 6000 +458
+ Misses 3020 1303 -1717
- Partials 440 556 +116 ☔ View full report in Codecov by Sentry. |
Some service container like mysql are slow to start, configuring the optional health check per GitHub Actions Docs can enshure they are ready
GitHub Docs Example based on postgres