Skip to content
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

tests/runk: fix the "run ps command" flaky test #9009

Merged
merged 4 commits into from Feb 28, 2024

Conversation

wainersm
Copy link
Contributor

@wainersm wainersm commented Feb 2, 2024

The "run ps command" test of the runk suite has failed intermittently. So here I'm fixing that but before I converted it to bats entirely; it should help on future maintenance of the test suite besides of improving the reporting text.

Fixes #8975

@wainersm wainersm added area/ci Issues affecting the continuous integration area/testing Issues related to testing labels Feb 2, 2024
@katacontainersbot katacontainersbot added the size/large Task of significant size label Feb 2, 2024
@wainersm wainersm changed the title tests/runk: disable the "run ps command" flaky test tests/runk: fix the "run ps command" flaky test Feb 5, 2024
@wainersm
Copy link
Contributor Author

wainersm commented Feb 5, 2024

hi @ManaSugi , mind to review it? Also on commit 69ea7f1 I added you as a co-author, so could you please check the signed-off string is accurate?

Copy link
Member

@ManaSugi ManaSugi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wainersm Sorry for the late review. my sign-off is good, thanks!

[ $status -eq 1 ]
# Check kill --all should not fail
sudo ctr t kill --signal SIGKILL --all "${CONTAINER_ID}"
}
Copy link
Member

@ManaSugi ManaSugi Feb 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: insert an empty line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.


setup_file() {
export RUNK_BIN_PATH="/usr/local/bin/runk"
export TEST_IMAGE="docker.io/library/busybox:latest"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is quay.io/prometheus/busybox preferrable, to work around image repository limits?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @danmihai1 ! you are right, fixed!

[ "${STATUS}" == "RUNNING" ]
}

teardown() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @GabyCT , you mean use check_processes to ensure that shimv2 process really got killed?

Migrated runk tests from pure shell script to bats to be consistent with
other test suites.

The install_dependencies() will install the bats tool locally.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Likely copied from the tracing workflow by mistake.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
The "run ps command" test has failed once in a while because it doesn't
wait the sh command to start within the container, consequently `ps`
won't report the amount of lines expected.

Fixes kata-containers#8975
Signed-off-by: Manabu Sugimoto <Manabu.Sugimoto@sony.com>
Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
It's recommended to avoid images from docker.io to avoid errors related
with hitting the pull limits that happens mostly on bare-metal machines.

So this replaced the docker.io's busybox with
quay.io/prometheus/busybox.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
@wainersm
Copy link
Contributor Author

Updates:

  • rebased code to main
  • replaced busybox image as suggested by @danmihai1
  • added extra empty line as suggested by @ManaSugi

@ManaSugi
Copy link
Member

lgtm, thanks @wainersm !

@wainersm
Copy link
Contributor Author

/test

1 similar comment
@wainersm
Copy link
Contributor Author

/test

@wainersm wainersm merged commit c4b8270 into kata-containers:main Feb 28, 2024
294 of 301 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci Issues affecting the continuous integration area/testing Issues related to testing ok-to-test size/large Task of significant size
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: run-runk failing intermittently on the test ps command checker
5 participants