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.

Sign up for GitHub

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

Jump to bottom

ci: Bump cilium-cli version #16617

Merged
merged 2 commits into from Jun 22, 2021
Merged

ci: Bump cilium-cli version #16617

merged 2 commits into from Jun 22, 2021

Conversation

nebril
Copy link
Member

@nebril nebril commented Jun 22, 2021

No description provided.

@nebril nebril requested review from a team as code owners June 22, 2021 09:31
@nebril nebril requested a review from christarazi June 22, 2021 09:31
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 22, 2021
@nebril nebril requested a review from pchaigno June 22, 2021 09:31
@nebril nebril added the dont-merge/preview-only Only for preview or testing, don't merge it. label Jun 22, 2021
@nebril
Copy link
Member Author

nebril commented Jun 22, 2021

tested in #16618

@nebril nebril added ci-run/aks release-note/ci This PR makes changes to the CI. and removed ci-run/aks dont-merge/preview-only Only for preview or testing, don't merge it. labels Jun 22, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 22, 2021
@nebril
ci: Enable flow validation
21c9b41 
Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
@nebril
ci: Bump cilium-cli version
db752d6 
Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
@christarazi christarazi merged commit d9eff9a into master Jun 22, 2021
@christarazi christarazi deleted the pr/bump-cli-version branch June 22, 2021 18:59
@nebril nebril mentioned this pull request Jun 23, 2021
Closed
pchaigno
pchaigno reviewed Jun 24, 2021
View reviewed changes
.github/workflows/conformance-aks.yaml
@@ -191,7 +191,7 @@ jobs:

- name: Run connectivity test
run: |
cilium connectivity test --flow-validation=disabled
cilium connectivity test --flow-validation=warning
Copy link
Member

Choose a reason for hiding this comment

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

@nebril Was this merged by mistake? I had understood it was added to debug a flake in this PR, but wasn't meant to be merged. It's effectively reverting #16388 which was aimed to reduce flakes until flow validation works better.

Copy link
Member

Choose a reason for hiding this comment

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

(I have the same question)

Copy link
Member

Choose a reason for hiding this comment

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

Especially since the commit does not change flow-validation in all instances, some of them are still disabled 🤔

Copy link
Member

@nbusseneau nbusseneau Jul 5, 2021 •
edited

Choose a reason for hiding this comment

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

Discussed offline: am reverting to a consistent state with flow validation disabled in #16787 as per #16388, we can always switch back to any other level once we are confident the CLI has improved.

nbusseneau added a commit that referenced this pull request Jul 5, 2021
@nbusseneau
workflows: consistently disabling flow validation
5df9959 
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
@nbusseneau
workflows: consistently disabling flow validation
f98a99d 
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
@nbusseneau @kkourt
workflows: consistently disabling flow validation
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>
@pchaigno pchaigno mentioned this pull request Jul 13, 2021
Merged
aanm pushed a commit that referenced this pull request Jul 15, 2021
@nbusseneau @aanm
workflows: consistently disabling flow validation
2cefd5d 
[ 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
@nbusseneau @aanm
workflows: consistently disabling flow validation
c30d727 
[ 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
Reviewers

@pchaigno pchaigno pchaigno approved these changes

@nbusseneau nbusseneau nbusseneau left review comments

@christarazi christarazi christarazi approved these changes

Assignees
No one assigned
Labels
release-note/ci This PR makes changes to the CI.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@nebril @pchaigno @christarazi @nbusseneau