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

CI: fix kubectl.CiliumPolicyAction() by casting "cilium policy wait" duration to integer #11358

Merged
merged 1 commit into from May 6, 2020

Conversation

qmonnet
Copy link
Member

@qmonnet qmonnet commented May 6, 2020

In the e2e framework, helper kubectl.CiliumPolicyAction() runs a call to the CLI command:

cilium policy wait <desiredRevision> <duration>

For the duration, ShortCommandTimeout.Seconds() is used. But this resolves to the string representation of a float ("10.000000") that fails to be interpreted by the CLI parser!

Let's cast the float to an integer instead, to make sure that kubectl.CiliumPolicyAction() correctly waits until the desired policy has been deployed. We also need to reverse the condition on the check for the cilium policy wait success: The wait failed if the return code is non-zero.

This should (hopefully) fix the flake reported in #11325.

Helper kubectl.CiliumPolicyAction() runs a call to the CLI command:

    cilium policy wait <desiredRevision> <duration>

For the duration, ShortCommandTimeout.Seconds() is used. But this
resolves to a float (10.000000) that fails to be interpreted by the CLI
parser!

Let's cast it to an integer instead, to make sure that
kubectl.CiliumPolicyAction() correctly waits until the desired policy
has been deployed. We also need to reverse the condition on the check
for the "cilium policy wait" success: The wait failed if the return code
is non-zero.

Fixes: 2a583b1 ("CI: Wait for cilium to regenerate when updating k8s network policies")
Fixes: #11325 (hopefully)
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@qmonnet qmonnet added pending-review 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. labels May 6, 2020
@qmonnet qmonnet requested a review from a team May 6, 2020 10:39
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 May 6, 2020
@qmonnet
Copy link
Member Author

qmonnet commented May 6, 2020

test-me-please

@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 44.447% when pulling 2d3c371 on pr/qmonnet/seconds_to_int into 5aeea78 on master.

Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

Nice catch!

@tklauser tklauser added the ci/flake This is a known failure that occurs in the tree. Please investigate me! label May 6, 2020
@nebril
Copy link
Member

nebril commented May 6, 2020

retest-ginkgo

@qmonnet
Copy link
Member Author

qmonnet commented May 6, 2020

restart-ginkgo

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 6, 2020
@joestringer joestringer merged commit 1e225a2 into master May 6, 2020
1.8.0 automation moved this from In progress to Merged May 6, 2020
@joestringer joestringer deleted the pr/qmonnet/seconds_to_int branch May 6, 2020 17:46
@b3a-dev b3a-dev added this to Fixed / Done in CI Force May 8, 2020
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. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
No open projects
1.8 CI
Awaiting triage
1.8.0
  
Merged
CI Force
  
Fixed / Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants