Skip to content

Add a flag to disable the ipcache check#503

Merged
michi-covalent merged 1 commit intomasterfrom
pr/michi/ipcache
Aug 31, 2021
Merged

Add a flag to disable the ipcache check#503
michi-covalent merged 1 commit intomasterfrom
pr/michi/ipcache

Conversation

@michi-covalent
Copy link
Copy Markdown
Contributor

@michi-covalent michi-covalent commented Aug 30, 2021 •
edited
Loading

Add --skip-ip-cache-check flag with the default set to true. This is
meant to be a temporary flag while we investigate what's causing the flake.

Ref: #361

Signed-off-by: Michi Mutsuzaki [email protected]

@michi-covalent michi-covalent requested a review from a team as a code owner August 30, 2021 23:12
@michi-covalent michi-covalent requested a review from twpayne August 30, 2021 23:12
@michi-covalent michi-covalent temporarily deployed to ci August 30, 2021 23:12 Inactive
@michi-covalent
Add a flag to disable the ipcache check
6ae2ab5 
Add `--skip-ip-cache-check` flag with the default set to true. This is
meant to be a temporary flag while we investigate what's causing the flake.

Ref: #361

Signed-off-by: Michi Mutsuzaki <[email protected]>
@michi-covalent michi-covalent temporarily deployed to ci August 30, 2021 23:25 Inactive
@michi-covalent michi-covalent changed the title Remove the ipcache check Add a flag to disable the ipcache check Aug 30, 2021
@michi-covalent
Copy link
Copy Markdown
Contributor Author

michi-covalent commented Aug 31, 2021

aks cluster creation failed with this error. re-triggering.

https://github.com/cilium/cilium-cli/actions/runs/1184098825

ERROR: (ReconcileStandardLoadBalancerError) AKS encountered an internal error while attempting the requested Creating operation. AKS will continuously retry the requested operation until successful or a retry timeout is hit. Check back to see if the operation requires resubmission. Correlation ID: cc4b16c3-e207-4ec0-935b-172e2abdbb8b, Operation ID: 597493aa-980d-41f3-8c4c-5a418e13f412, Timestamp: 2021-08-30T23:27:08Z.

@michi-covalent
Copy link
Copy Markdown
Contributor Author

michi-covalent commented Aug 31, 2021

hitting the same issue again. not sure how to proceed 😞

tklauser
tklauser approved these changes Aug 31, 2021
View reviewed changes
internal/cli/cmd/connectivity.go
cmd.Flags().BoolVarP(&params.Verbose, "verbose", "v", false, "Show informational messages and don't buffer any lines")
cmd.Flags().BoolVarP(&params.Debug, "debug", "d", false, "Show debug messages")
cmd.Flags().BoolVarP(&params.PauseOnFail, "pause-on-fail", "p", false, "Pause execution on test failure")
cmd.Flags().BoolVar(&params.SkipIPCacheCheck, "skip-ip-cache-check", true, "Skip IPCache check")
Copy link
Copy Markdown
Member

@tklauser tklauser Aug 31, 2021

Choose a reason for hiding this comment

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

Should we mark this flag as hidden so we can easily remove it again? Given that we intend to fix the ipcache flake #361 eventually,

Copy link
Copy Markdown
Contributor

@bmcustodio bmcustodio Aug 31, 2021

Choose a reason for hiding this comment

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

Hmmm this makes sense, but at the same time I think that if users face the issue and the flag is hidden they won't know they can use it to proceed.

Copy link
Copy Markdown
Member

@tklauser tklauser Aug 31, 2021

Choose a reason for hiding this comment

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

Given that we default the flag to true (i.e. skip the ipcache check by default), I think users should not face the ipcache issue anymore. They would need to figure out to run with --skip-ip-cache-check=false at which point they already figured out the flag 😉

Copy link
Copy Markdown
Contributor

@bmcustodio bmcustodio Aug 31, 2021

Choose a reason for hiding this comment

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

Oh right, makes total sense 💯 — I hadn't looked at the default.

Copy link
Copy Markdown
Member

@tklauser tklauser Sep 1, 2021

Choose a reason for hiding this comment

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

Looks like this hasn't been addressed before merging. I've opened another PR: #508

Copy link
Copy Markdown
Contributor Author

@michi-covalent michi-covalent Sep 1, 2021

Choose a reason for hiding this comment

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

thanks @tklauser , i just saw the green merge button and pushed it 😅

@nbusseneau
Copy link
Copy Markdown
Member

nbusseneau commented Aug 31, 2021

hitting the same issue again. not sure how to proceed disappointed

This is an infrastructure issue, waiting for AKS support atm: cilium/cilium#17260

@michi-covalent
Copy link
Copy Markdown
Contributor Author

michi-covalent commented Aug 31, 2021

re-triggered to see if aks happy now 👀

@michi-covalent michi-covalent merged commit af81105 into master Aug 31, 2021
@michi-covalent michi-covalent deleted the pr/michi/ipcache branch August 31, 2021 20:52
tklauser added a commit that referenced this pull request Sep 1, 2021
@tklauser
internal/cli/cmd: mark --skip-ip-cache-check as hidden
98dc142 
Follow-up for #503 to address
#503 (comment)

Also add a comment so we don't forget to re-enable the check again once
issue #361 is resolved.

Signed-off-by: Tobias Klauser <[email protected]>
@tklauser tklauser mentioned this pull request Sep 1, 2021
Merged
tklauser added a commit that referenced this pull request Sep 1, 2021
@tklauser
internal/cli/cmd: mark --skip-ip-cache-check as hidden
89639cb 
Follow-up for #503 to address
#503 (comment)

Also add a comment so we don't forget to re-enable the check again once
issue #361 is resolved.

Signed-off-by: Tobias Klauser <[email protected]>
aditighag pushed a commit to aditighag/cilium-cli that referenced this pull request Apr 21, 2023
@tklauser @aditighag
internal/cli/cmd: mark --skip-ip-cache-check as hidden
da93337 
Follow-up for cilium#503 to address
cilium#503 (comment)

Also add a comment so we don't forget to re-enable the check again once
issue cilium#361 is resolved.

Signed-off-by: Tobias Klauser <[email protected]>
michi-covalent pushed a commit to michi-covalent/cilium that referenced this pull request May 30, 2023
@tklauser @michi-covalent
internal/cli/cmd: mark --skip-ip-cache-check as hidden
319eafc 
Follow-up for cilium#503 to address
cilium/cilium-cli#503 (comment)

Also add a comment so we don't forget to re-enable the check again once
issue cilium#361 is resolved.

Signed-off-by: Tobias Klauser <[email protected]>
tklauser added a commit to cilium/cilium that referenced this pull request Oct 17, 2025
@tklauser
cilium-cli: drop disabled IP cache check from connectivity tests
383bb78 
The IP cache check has been disabled and hidden by default since
cilium-cli v0.9.0 in 2021[^0][^1]. The IP cache check has been known to be a
source of test flakiness [1] and none of Cilium's CI workflows enable
the flag. this all indicates that the check isn't really of much use
anymore and can be dropped.

[^0]: cilium/cilium-cli#503
[^1]: cilium/cilium-cli#361

Signed-off-by: Tobias Klauser <[email protected]>
tklauser added a commit to cilium/cilium that referenced this pull request Oct 17, 2025
@tklauser
cilium-cli: drop disabled IP cache check from connectivity tests
a970448 
The IP cache check has been disabled and hidden by default since
cilium-cli v0.9.0 in 2021[^0][^1]. The IP cache check has been known to be a
source of test flakiness [1] and none of Cilium's CI workflows enable
the flag. this all indicates that the check isn't really of much use
anymore and can be dropped.

[^0]: cilium/cilium-cli#503
[^1]: cilium/cilium-cli#361

Signed-off-by: Tobias Klauser <[email protected]>
github-merge-queue bot pushed a commit to cilium/cilium that referenced this pull request Oct 20, 2025
@tklauser
cilium-cli: drop disabled IP cache check from connectivity tests
e965964 
The IP cache check has been disabled and hidden by default since
cilium-cli v0.9.0 in 2021[^0][^1]. The IP cache check has been known to be a
source of test flakiness [1] and none of Cilium's CI workflows enable
the flag. this all indicates that the check isn't really of much use
anymore and can be dropped.

[^0]: cilium/cilium-cli#503
[^1]: cilium/cilium-cli#361

Signed-off-by: Tobias Klauser <[email protected]>
github-merge-queue bot pushed a commit to cilium/cilium that referenced this pull request Oct 20, 2025
@tklauser @julianwiedmann
cilium-cli: drop disabled IP cache check from connectivity tests
f92a761 
The IP cache check has been disabled and hidden by default since
cilium-cli v0.9.0 in 2021[^0][^1]. The IP cache check has been known to be a
source of test flakiness [1] and none of Cilium's CI workflows enable
the flag. this all indicates that the check isn't really of much use
anymore and can be dropped.

[^0]: cilium/cilium-cli#503
[^1]: cilium/cilium-cli#361

Signed-off-by: Tobias Klauser <[email protected]>
github-merge-queue bot pushed a commit to cilium/cilium that referenced this pull request Oct 20, 2025
@tklauser @julianwiedmann
cilium-cli: drop disabled IP cache check from connectivity tests
5762fec 
The IP cache check has been disabled and hidden by default since
cilium-cli v0.9.0 in 2021[^0][^1]. The IP cache check has been known to be a
source of test flakiness [1] and none of Cilium's CI workflows enable
the flag. this all indicates that the check isn't really of much use
anymore and can be dropped.

[^0]: cilium/cilium-cli#503
[^1]: cilium/cilium-cli#361

Signed-off-by: Tobias Klauser <[email protected]>
github-merge-queue bot pushed a commit to cilium/cilium that referenced this pull request Oct 20, 2025
@tklauser
cilium-cli: drop disabled IP cache check from connectivity tests
176c650 
The IP cache check has been disabled and hidden by default since
cilium-cli v0.9.0 in 2021[^0][^1]. The IP cache check has been known to be a
source of test flakiness [1] and none of Cilium's CI workflows enable
the flag. this all indicates that the check isn't really of much use
anymore and can be dropped.

[^0]: cilium/cilium-cli#503
[^1]: cilium/cilium-cli#361

Signed-off-by: Tobias Klauser <[email protected]>
github-merge-queue bot pushed a commit to cilium/cilium that referenced this pull request Oct 20, 2025
@tklauser
cilium-cli: drop disabled IP cache check from connectivity tests
42f3db8 
The IP cache check has been disabled and hidden by default since
cilium-cli v0.9.0 in 2021[^0][^1]. The IP cache check has been known to be a
source of test flakiness [1] and none of Cilium's CI workflows enable
the flag. this all indicates that the check isn't really of much use
anymore and can be dropped.

[^0]: cilium/cilium-cli#503
[^1]: cilium/cilium-cli#361

Signed-off-by: Tobias Klauser <[email protected]>
github-merge-queue bot pushed a commit to cilium/cilium that referenced this pull request Oct 21, 2025
@tklauser @julianwiedmann
cilium-cli: drop disabled IP cache check from connectivity tests
24fa235 
The IP cache check has been disabled and hidden by default since
cilium-cli v0.9.0 in 2021[^0][^1]. The IP cache check has been known to be a
source of test flakiness [1] and none of Cilium's CI workflows enable
the flag. this all indicates that the check isn't really of much use
anymore and can be dropped.

[^0]: cilium/cilium-cli#503
[^1]: cilium/cilium-cli#361

Signed-off-by: Tobias Klauser <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tklauser tklauser tklauser approved these changes

@jrajahalme jrajahalme Awaiting requested review from jrajahalme

+2 more reviewers

@bmcustodio bmcustodio bmcustodio approved these changes

@twpayne twpayne twpayne approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jrajahalme jrajahalme

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@michi-covalent @nbusseneau @twpayne @tklauser @bmcustodio @jrajahalme