fix: guard GetHealth against nil docker client in dry-run mode#6008
Open
veeceey wants to merge 1 commit intonektos:masterfrom
Open
fix: guard GetHealth against nil docker client in dry-run mode#6008veeceey wants to merge 1 commit intonektos:masterfrom
veeceey wants to merge 1 commit intonektos:masterfrom
Conversation
In dry-run mode, the docker client is never initialized because connect() is skipped via .IfNot(common.Dryrun). However, GetHealth() was called unconditionally by waitForServiceContainer, causing a nil pointer dereference (SIGSEGV) when service containers are defined. Return HealthHealthy early when cr.cli is nil, matching the pattern used by other methods like Start() and Remove() that skip execution during dry-run. Fixes nektos#5934
Author
|
Good suggestion about documenting the dry-run health check behavior. Happy to add a note to the README if the maintainers would like that as part of this PR. |
Author
|
gentle bump on this when you get a chance |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When running
act -n(dry-run mode) with workflows that define service containers, the process panics with a SIGSEGV becauseGetHealth()tries to callContainerInspecton a nil docker client.In dry-run mode,
connect()is skipped via.IfNot(common.Dryrun), socr.cliis never set. ButwaitForServiceContainerstill callsGetHealth()unconditionally, which dereferences the nil client.The fix adds a nil check on
cr.cliat the top ofGetHealth()— if there's no client, we just returnHealthHealthyand move on. This matches the pattern already used byStart()andRemove()which skip execution during dry-run.Tested by verifying the build compiles cleanly. The existing container tests require a live Docker daemon so they can't run in CI-less environments, but the logic is straightforward — just a nil guard.
Fixes #5934