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

test: Wait for cilium monitor to match expected output #15848

Merged
merged 1 commit into from
Apr 27, 2021

Conversation

pchaigno
Copy link
Member

@pchaigno pchaigno commented Apr 23, 2021

In some tests we check the output of cilium monitor and compare it against expected substrings (e.g., allowed verdict). When doing so, we however need to repeat with a timeout until it matches as the cilium monitor output may be buffered and may not match the expected output as soon as we check. This pull request fixes a couple cases where we forgot to wait for the output to match.

It is expected to fix flake #14676. See details at #14676 (comment)

In some tests we check the output of cilium monitor and compare it
against expected substrings (e.g., allowed verdict). When doing so, we
however need to repeat with a timeout until it matches as the cilium
monitor output may be buffered and may not match the expected output as
soon as we check. This commit fixes a couple cases where we forgot to
wait for the output to match.

It is expected to fix flake cilium#14676.

Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno pchaigno added kind/bug/CI This is a bug in the testing code. area/CI Continuous Integration testing issue or flake release-note/ci This PR makes changes to the CI. ci/flake This is a known failure that occurs in the tree. Please investigate me! labels Apr 23, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Apr 23, 2021
@pchaigno
Copy link
Member Author

pchaigno commented Apr 23, 2021

Only Runtime is required since only Runtime tests are touched.

test-runtime

@pchaigno pchaigno marked this pull request as ready for review April 23, 2021 17:28
@pchaigno pchaigno requested a review from a team as a code owner April 23, 2021 17:28
@pchaigno pchaigno requested a review from a team April 23, 2021 17:28
@pchaigno pchaigno requested a review from a team as a code owner April 23, 2021 17:28
@pchaigno pchaigno requested a review from nebril April 23, 2021 17:28
@aanm aanm merged commit ef9d75f into cilium:master Apr 27, 2021
1.10.0 automation moved this from In progress to Done Apr 27, 2021
@pchaigno pchaigno deleted the fix-cilium-monitor-matches branch April 27, 2021 17:16
@joestringer
Copy link
Member

joestringer commented Jul 16, 2021

@pchaigno
Copy link
Member Author

I believe I have just observed the issue in v1.9 backports PR #16910:

It's hard to tell for sure because we're missing the artifacts. I've marked #16759, #16540, and #16489 for backports to ensure we have artifacts in the future.

Do we need to backport this to v1.9?

I'm not against backporting this PR as well 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI Continuous Integration testing issue or flake ci/flake This is a known failure that occurs in the tree. Please investigate me! kind/bug/CI This is a bug in the testing code. release-note/ci This PR makes changes to the CI.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants