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

.github: Disable flow validation in flaky tests #16388

Merged
merged 1 commit into from Jun 2, 2021

Conversation

pchaigno
Copy link
Member

@pchaigno pchaigno commented Jun 1, 2021

The flow validation of cilium connectivity test has been reported to be flaky in several of the ci-xxx tests [1, 2, 3, 4]. To reduce CI noise, we can disable flow validation until the flakes are fixed.

1 - #16292.
2 - #16291.
3 - #16096.
4 - #16392.

@pchaigno pchaigno added 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 Jun 1, 2021
@pchaigno pchaigno requested review from a team as code owners June 1, 2021 21:44
@pchaigno pchaigno requested review from nebril and aanm June 1, 2021 21:44
The flow validation of cilium connectivity test has been reported to be
flaky in several of the ci-xxx tests [1, 2, 3]. To reduce CI noise, we
can disable flow validation until the flakes are fixed.

1 - cilium#16292.
2 - cilium#16291.
3 - cilium#16096.
Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno
Copy link
Member Author

pchaigno commented Jun 2, 2021

I updated the branch to also disable in the Kind1.19 test given it's also failing there (cf. #16392).

@pchaigno pchaigno requested a review from a team as a code owner June 2, 2021 09:09
@pchaigno
Copy link
Member Author

pchaigno commented Jun 2, 2021

So cilium connectivity test --flow-validation=disabled doesn't actually disable flow validation: https://github.com/cilium/cilium/runs/2726941249?check_suite_focus=true...

EDIT: That's just because these tests execute the master versions on PR 🤦

@pchaigno pchaigno marked this pull request as draft June 2, 2021 12:18
@pchaigno pchaigno marked this pull request as ready for review June 2, 2021 12:24
@pchaigno
Copy link
Member Author

pchaigno commented Jun 2, 2021

Al team reviews are covered. The Kind1.19 test is passing. Other triggered tests are running the master version, not this PR version. Marking as ready to merge.

@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 2, 2021
Copy link
Member

@nathanjsweet nathanjsweet left a comment

Choose a reason for hiding this comment

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

@pchaigno can you validate that the client-to-client-client-ingress-from-client test in the aks conformance tests has nothing to do with this PR? Otherwise LGTM.

NVM, didn't see the previous comment

@nathanjsweet nathanjsweet merged commit 49ce8e9 into cilium:master Jun 2, 2021
@pchaigno pchaigno deleted the disable-flow-validation branch June 2, 2021 19:20
nebril pushed a commit to nbusseneau/cilium that referenced this pull request Jun 23, 2021
Seems like we missed that in cilium#16388.

Fixes 49ce8e9

Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
nbusseneau added a commit to nbusseneau/cilium that referenced this pull request Jun 29, 2021
Seems like we missed that in cilium#16388.

Fixes 49ce8e9

Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
nbusseneau added a commit that referenced this pull request Jul 5, 2021
In #16388, we disabled flow validation as it was unstable in the CLI.
In #16617, we partially moved to `warning` for testing purposes.

This reverts back to `disabled` for all connectivity tests in order to
be consistent. Also fix missing disabled flow validation in EKS, as it
seems like we missed that one in #16388.

We can always enable flow validation again later once we are confident
flow validation works better in the CLI.

Fixes 49ce8e9
Fixes 21c9b41

Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
nbusseneau added a commit that referenced this pull request Jul 7, 2021
In #16388, we disabled flow validation as it was unstable in the CLI.
In #16617, we partially moved to `warning` for testing purposes.

This reverts back to `disabled` for all connectivity tests in order to
be consistent. Also fix missing disabled flow validation in EKS, as it
seems like we missed that one in #16388.

We can always enable flow validation again later once we are confident
flow validation works better in the CLI.

Fixes 49ce8e9
Fixes 21c9b41

Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
kkourt pushed a commit that referenced this pull request Jul 8, 2021
In #16388, we disabled flow validation as it was unstable in the CLI.
In #16617, we partially moved to `warning` for testing purposes.

This reverts back to `disabled` for all connectivity tests in order to
be consistent. Also fix missing disabled flow validation in EKS, as it
seems like we missed that one in #16388.

We can always enable flow validation again later once we are confident
flow validation works better in the CLI.

Fixes 49ce8e9
Fixes 21c9b41

Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
aanm pushed a commit that referenced this pull request Jul 15, 2021
[ upstream commit 36aa4f7 ]

In #16388, we disabled flow validation as it was unstable in the CLI.
In #16617, we partially moved to `warning` for testing purposes.

This reverts back to `disabled` for all connectivity tests in order to
be consistent. Also fix missing disabled flow validation in EKS, as it
seems like we missed that one in #16388.

We can always enable flow validation again later once we are confident
flow validation works better in the CLI.

Fixes 49ce8e9
Fixes 21c9b41

Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
Signed-off-by: André Martins <andre@cilium.io>
aanm pushed a commit that referenced this pull request Jul 15, 2021
[ upstream commit 36aa4f7 ]

In #16388, we disabled flow validation as it was unstable in the CLI.
In #16617, we partially moved to `warning` for testing purposes.

This reverts back to `disabled` for all connectivity tests in order to
be consistent. Also fix missing disabled flow validation in EKS, as it
seems like we missed that one in #16388.

We can always enable flow validation again later once we are confident
flow validation works better in the CLI.

Fixes 49ce8e9
Fixes 21c9b41

Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
Signed-off-by: André Martins <andre@cilium.io>
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! 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
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants