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

workflows: add test exceptions to EKS tunnel workflow #450

Merged
merged 1 commit into from
Jul 22, 2021

Conversation

nbusseneau
Copy link
Member

@nbusseneau nbusseneau commented Jul 21, 2021

Following merge of #251, testing on EKS in tunnel mode is now reliably broken due to an issue in upstream: cilium/cilium#16975

This type of failure was already happening before #251, but the PR made it very evident as before we were only running checks from a few random pods (which would sometimes work) and are now running checks from all pods, drastically increasing the chance of hitting at least one failure.

Since the test setup is not to blame and this is an actual issue with Cilium in tunnel mode, we disable the failing tests until upstream issue is fixed.

@nbusseneau nbusseneau added the dont-merge/preview-only Only for preview or testing, don't merge it. label Jul 21, 2021
@nbusseneau nbusseneau temporarily deployed to ci July 21, 2021 17:13 Inactive
@nbusseneau nbusseneau temporarily deployed to ci July 21, 2021 17:18 Inactive
@nbusseneau nbusseneau temporarily deployed to ci July 21, 2021 18:21 Inactive
@nbusseneau nbusseneau temporarily deployed to ci July 22, 2021 08:38 Inactive
@nbusseneau nbusseneau temporarily deployed to ci July 22, 2021 10:37 Inactive
@nbusseneau nbusseneau temporarily deployed to ci July 22, 2021 11:37 Inactive
@nbusseneau nbusseneau temporarily deployed to ci July 22, 2021 14:49 Inactive
Following merge of #251, testing on EKS in tunnel mode is now reliably
broken due to an issue in upstream: cilium/cilium#16975

This type of failure was already happening before #251, but the PR made
it very evident as before we were only running checks from a few random
pods (which would sometimes work) and are now running checks from all
pods, drastically increasing the chance of hitting at least one failure.

Since the test setup is not to blame and this is an actual issue with
Cilium in tunnel momde, we disable the failing tests until upstream
issue is fixed.

Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@nbusseneau nbusseneau temporarily deployed to ci July 22, 2021 15:27 Inactive
@nbusseneau nbusseneau changed the title DO NOT MERGE Test PR for fixing EKS tunnel workflow workflows: add test exceptions to EKS tunnel workflow Jul 22, 2021
@nbusseneau nbusseneau requested a review from ti-mo July 22, 2021 15:28
@nbusseneau nbusseneau added area/CI Continuous Integration testing issue or flake and removed dont-merge/preview-only Only for preview or testing, don't merge it. labels Jul 22, 2021
@nbusseneau nbusseneau marked this pull request as ready for review July 22, 2021 15:28
@nbusseneau nbusseneau requested a review from a team as a code owner July 22, 2021 15:28
@nbusseneau
Copy link
Member Author

Link to working test run of the changes in the EKS tunnel workflow: https://github.com/cilium/cilium-cli/actions/runs/1056560809

Copy link
Contributor

@ti-mo ti-mo left a comment

Choose a reason for hiding this comment

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

Thanks, should provide some breathing room in the mean time. 👍

@michi-covalent
Copy link
Contributor

ok let's ship it

@michi-covalent michi-covalent merged commit 1bf9ca7 into master Jul 22, 2021
@michi-covalent michi-covalent deleted the pr/workflows-fix-eks-tunnel branch July 22, 2021 17:28
@pchaigno
Copy link
Member

It seems this is just happening because of leftover iptables rules from aws-node. I confirmed the full connectivity tests pass locally on EKS+overlay once the rules are removed. See cilium/cilium#26897 (comment) for details.

We can probably revert this and should even support removal of those rules in the CLI IMO.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants