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

Add a flag to disable the ipcache check #503

Merged
merged 1 commit into from
Aug 31, 2021
Merged

Conversation

michi-covalent
Copy link
Contributor

@michi-covalent michi-covalent commented Aug 30, 2021

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]

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
Contributor Author

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
Contributor Author

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

@@ -119,6 +119,7 @@ func newCmdConnectivityTest() *cobra.Command {
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
Member

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
Contributor

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
Member

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
Contributor

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
Member

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
Contributor Author

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
Member

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
Contributor Author

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
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 added a commit that referenced this pull request Sep 1, 2021
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
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
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants