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

Fix for disabled cloud provider rate limiting #24413

Merged
merged 1 commit into from Mar 17, 2023

Conversation

hemanthmalla
Copy link
Member

Earlier versions of these flags had a prefix of ENI and were later renamed with a prefix of IPAM to be consistent across cloud providers. While removing the deprecated old flags, #12676 also removed lines needed for setting OperatorConfig struct fields from the values read in. This resulted in fields having golang defaults, which caused the rate limiter to be completely bypassed.

Earlier versions of these flags had a prefix of ENI and were later
renamed with a prefix of IPAM to be consistent across cloud providers.
While removing the deprecated old flags, cilium#12676 also removed lines
needed for setting OperatorConfig struct fields from the values read in.
This resulted in fields having golang defaults, which caused the rate
limiter to be completely bypassed.

Signed-off-by: Hemanth Malla <hemanth.malla@datadoghq.com>
@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 Mar 16, 2023
@hemanthmalla hemanthmalla marked this pull request as ready for review March 16, 2023 13:14
@hemanthmalla hemanthmalla requested a review from a team as a code owner March 16, 2023 13:14
@hemanthmalla hemanthmalla added affects/v1.11 This issue affects v1.11 branch affects/v1.12 This issue affects v1.12 branch affects/v1.10 This issue affects v1.10 branch affects/v1.13 This issue affects v1.13 branch labels Mar 16, 2023
@gandro gandro added needs-backport/1.11 needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch release-note/bug This PR fixes an issue in a previous release of Cilium. labels Mar 16, 2023
@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 Mar 16, 2023
@hemanthmalla
Copy link
Member Author

Will make the backports once the PR is merged.
Note: This also impacts v1.9

@gandro
Copy link
Member

gandro commented Mar 16, 2023

Will make the backports once the PR is merged. Note: This also impacts v1.9

If you intend to make the backports yourself, we should add the backport/author label. Otherwise, the person assigned to be the weekly tophat/backporter will do it for you.

Note that we're not maintaining the v1.9 and v1.10 branches anymore, as they are EOL.

@christarazi
Copy link
Member

/test

Copy link
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

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

Nice find!

@christarazi christarazi added area/operator Impacts the cilium-operator component integration/cloud Related to integration with cloud environments such as AKS, EKS, GKE, etc. sig/ipam IP address management, including cloud IPAM kind/bug This is a bug in the Cilium logic. kind/regression This functionality worked fine before, but was broken in a newer release of Cilium. labels Mar 16, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 17, 2023
@borkmann borkmann merged commit 0557a2f into cilium:master Mar 17, 2023
42 checks passed
@NikAleksandrov NikAleksandrov mentioned this pull request Mar 20, 2023
5 tasks
@NikAleksandrov NikAleksandrov mentioned this pull request Mar 20, 2023
9 tasks
@NikAleksandrov NikAleksandrov mentioned this pull request Mar 23, 2023
29 tasks
@NikAleksandrov NikAleksandrov added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Mar 23, 2023
@youngnick youngnick added backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. and removed backport-pending/1.11 labels Mar 23, 2023
@tklauser tklauser removed affects/v1.11 This issue affects v1.11 branch affects/v1.12 This issue affects v1.12 branch affects/v1.10 This issue affects v1.10 branch affects/v1.13 This issue affects v1.13 branch labels Mar 28, 2023
@jibi jibi added backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. and removed backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. labels Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/operator Impacts the cilium-operator component backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. integration/cloud Related to integration with cloud environments such as AKS, EKS, GKE, etc. kind/bug This is a bug in the Cilium logic. kind/regression This functionality worked fine before, but was broken in a newer release of Cilium. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium. sig/ipam IP address management, including cloud IPAM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants