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 POD policy revision increment in all cases. #11995

Merged
merged 1 commit into from Jun 10, 2020

Conversation

jrajahalme
Copy link
Member

@jrajahalme jrajahalme commented Jun 9, 2020

Wait for each POD incrementing their policy revision also after
adding a cluster-wide policy or deleting a policy, as otherwise there is a race between policy
add or deletion and the following test traffic that expects policy add or deletion
to have taken effect already.

This fixes CI flakes following policy deletes like:

08:35:50 STEP: Deleting Egress deny all clusterwide policy
08:35:51 STEP: Testing ingress connectivity from "app2" to "app1" in "2020060908341k8spolicytestclusterwidepoliciestestclusterwidecon" namespace
FAIL: Ingress connectivity should be allowed for service in 2020060908341k8spolicytestclusterwidepoliciestestclusterwidecon namespace
Expected command: kubectl exec -n 2020060908341k8spolicytestclusterwidepoliciestestclusterwidecon app2-88b7f8c4b-cvg64 -- curl --path-as-is -s -D /dev/stderr --fail --connect-timeout 5 --max-time 20 http://10.110.187.138/public -w "time-> DNS: '%{time_namelookup}(%{remote_ip})', Connect: '%{time_connect}',Transfer '%{time_starttransfer}', total '%{time_total}'"
To succeed, but it failed:
Exitcode: 28
Stdout:
time-> DNS: '0.000013()', Connect: '0.000000',Transfer '0.000000', total '5.000688'
Stderr:
command terminated with exit code 28

Note to backporters: v1.6 does not have CiliumClusterwidePolicyAction() so changes to it can safely be dropped.

Wait for each POD incrementing their policy revision also after
deleting a policy, as otherwise there is a race between policy
deletion and the following test traffic that expects policy deletion
to have taken effect already.

This fixes CI flakes following policy deletes like:

08:35:50 STEP: Deleting Egress deny all clusterwide policy
08:35:51 STEP: Testing ingress connectivity from "app2" to "app1" in "2020060908341k8spolicytestclusterwidepoliciestestclusterwidecon" namespace
FAIL: Ingress connectivity should be allowed for service in 2020060908341k8spolicytestclusterwidepoliciestestclusterwidecon namespace
Expected command: kubectl exec -n 2020060908341k8spolicytestclusterwidepoliciestestclusterwidecon app2-88b7f8c4b-cvg64 -- curl --path-as-is -s -D /dev/stderr --fail --connect-timeout 5 --max-time 20 http://10.110.187.138/public -w "time-> DNS: '%{time_namelookup}(%{remote_ip})', Connect: '%{time_connect}',Transfer '%{time_starttransfer}', total '%{time_total}'" 
To succeed, but it failed:
Exitcode: 28 
Stdout:
 	 time-> DNS: '0.000013()', Connect: '0.000000',Transfer '0.000000', total '5.000688'
Stderr:
 	 command terminated with exit code 28

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
@jrajahalme jrajahalme added kind/bug/CI This is a bug in the testing code. needs-backport/1.6 release-note/ci This PR makes changes to the CI. labels Jun 9, 2020
@jrajahalme jrajahalme requested a review from a team as a code owner June 9, 2020 17:56
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 Jun 9, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.7.5 Jun 9, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.6.10 Jun 9, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.8.0 Jun 9, 2020
@jrajahalme
Copy link
Member Author

test-me-please

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 37.017% when pulling 913c84b on pr/jrajahalme/test-wait-for-policy-after-delete into b4627b7 on master.

@jrajahalme jrajahalme changed the title test: Wait for POD policy revision increment also after policy delete test: Wait for POD policy revision increment in all cases. Jun 9, 2020
@jrajahalme
Copy link
Member Author

The following tests may have been flaking because of this:
K8sPolicyTest:

  • Tests proxy visibility interactions with policy lifecycle operations
  • Kubernetes Network Policy by namespace selector
  • Test clusterwide connectivity with policies

@jrajahalme jrajahalme added the ci/flake This is a known failure that occurs in the tree. Please investigate me! label Jun 10, 2020
@jrajahalme jrajahalme added this to In Progress (Cilium) in CI Force via automation Jun 10, 2020
@aanm aanm merged commit c16e111 into master Jun 10, 2020
1.8.0 automation moved this from In progress to Merged Jun 10, 2020
CI Force automation moved this from In Progress (Cilium) to Fixed / Done Jun 10, 2020
@aanm aanm deleted the pr/jrajahalme/test-wait-for-policy-after-delete branch June 10, 2020 08:18
@aanm aanm mentioned this pull request Jun 11, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.8 in 1.8.0 Jun 11, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.8 to Backport done to v1.8 in 1.8.0 Jun 12, 2020
@aanm aanm removed this from Needs backport from master in 1.7.5 Jun 12, 2020
@aanm aanm added this to Needs backport from master in 1.7.6 Jun 12, 2020
This was referenced Jun 12, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Backport pending to v1.7 in 1.7.5 Jun 12, 2020
@jrajahalme
Copy link
Member Author

v1.6 backport seems complicated, will not do it.

@joestringer joestringer removed this from Needs backport from master in 1.6.10 Jul 1, 2020
@aanm aanm removed this from Needs backport from master in 1.7.6 Jul 3, 2020
@aanm aanm removed this from Backport pending to v1.7 in 1.7.5 Jul 3, 2020
@aanm aanm added this to Needs backport from master in 1.7.7 Jul 3, 2020
@joestringer joestringer moved this from Needs backport from master to Backport pending to v1.7 in 1.7.7 Jul 9, 2020
@qmonnet
Copy link
Member

qmonnet commented Aug 3, 2020

We also decided not to backport to v1.7, see #12060 (comment).

@joestringer joestringer removed this from Backport pending to v1.7 in 1.7.7 Aug 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
1.8.0
  
Merged
1.8.0
Backport done to v1.8
CI Force
  
Fixed / Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants