Add a timeout when initializing the Podman client (broken Podman should not affect odo dev on cluster)
#6808
Conversation
This file contains 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
openshift-ci
bot
added
kind/bug
✅ Deploy Preview for odo-docusaurus-preview canceled.
|
rm3l
requested review from
feloy and
valaparthvi
and removed request for
rnapoles-rh
rm3l
changed the title
odo dev on cluster)odo dev on cluster)
This command is called at dependency injection time to initialize a (nil-able) Podman client, even if users won't use Podman at all. As discussed, this command is supposed to be quite fast to return, hence this timeout of 1 second. Initially, we were using cmd.Output to get the command output, but as reported in [1], cmd.Output does not respect the context timeout. This explains the workaround of reading from both stdout and stderr pipes, *and* relying on cmd.Wait() to close those pipes properly when the program exits (either as expected or when the timeout is reached). [1] golang/go#57129 Co-authored-by: Philippe Martin <phmartin@redhat.com>
…l Kubernetes/Podman clients could not be initialized This helps debug such potential issues instead of swallowing the errors.
This will allow setting a different value for environments like in GitHub where the Podman client would take slightly more time to return (I guess because of we are running a lot of Podman commands in parallel?).
rm3l
force-pushed
the
6575-broken-podman-affects-odo-dev-run-against-cluster
branch
from
1e4370e to
c3a5b23
Compare
Some tests did not pass because the Podman client did not initialize in 1s; I guess because we are running a lot of Podman commands in parallel? This should hopefully improve this situation.
rm3l
force-pushed
the
6575-broken-podman-affects-odo-dev-run-against-cluster
branch
from
c3a5b23 to
f985ae7
Compare
feloy
reviewed
May 10, 2023
feloy
approved these changes
May 11, 2023
feloy
approved these changes
May 11, 2023
openshift-ci
bot
added
the
lgtm
|
Kudos, SonarCloud Quality Gate passed!
|
Network errors. /override windows-integration-test/Windows-test |
|
@rm3l: Overrode contexts on behalf of rm3l: windows-integration-test/Windows-test In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Previous run passed. /override OpenShift-Integration-tests/OpenShift-Integration-tests |
|
@rm3l: Overrode contexts on behalf of rm3l: OpenShift-Integration-tests/OpenShift-Integration-tests In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/override kubernetes-infra-stage-test Not related. |
|
@rm3l: Overrode contexts on behalf of rm3l: kubernetes-infra-stage-test In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What type of PR is this:
/kind bug
/area dev
What does this PR do / why we need it:
This introduces a timeout for the Podman client initialization (e.g., a timeout for the
podman versioncommand to finish). The default value is1s.For some reason, this made the Podman tests flaky on GitHub (sometimes,
podman versionwould take more than 1s to return). The timeout has therefore been made configurable (via thePODMAN_CMD_INIT_TIMEOUTenvironment variable, defaulting to1s), and set to a slightly higher value in the tests.Which issue(s) this PR fixes:
Fixes #6575
PR acceptance criteria:
Unit test
Integration test
Documentation
How to test changes / Special notes to the reviewer:
This can be tested by adding a script/program to your
PATHthat would delay all calls to Podman, e.g.:At this point,
odo devon cluster should no longer hang.odo dev --platform=podmanwould error out, but this is expected.