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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle unordered public endpoint CIDRs from EKS in endpoint updates #7483

Merged

Conversation

Emberwalker
Copy link
Contributor

Description

For some clusters, EKS can return the list of public endpoint CIDRs out of order, and won't allow updates where the incoming and current sets have set equality (i.e. regardless of order of CIDR entries). This change restores the set equality check that was removed in commit 72605fb and adds an additional test case to cover this case.

In our clusters, EKS always appears to return the CIDRs in a specific order, but not sorted - possibly due to the accumulation and removal of entries over time. Without this change, eksctl 0.167.0 will always attempt an update and get a 400 InvalidParameterException ("Cluster is already at the desired configuration with [...]") - with this applied, it correctly identifies no update is needed.

Checklist

  • Added tests that cover your change (if possible)
  • Added/modified documentation as required (such as the README.md, or the userdocs directory)
  • Manually tested
  • Made sure the title of the PR is a good description that can go into the release notes
  • (Core team) Added labels for change area (e.g. area/nodegroup) and kind (e.g. kind/improvement)

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 馃く

  • Backfilled missing tests for code in same general area 馃帀
  • Refactored something and made the world a better place 馃専

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hello Emberwalker 馃憢 Thank you for opening a Pull Request in eksctl project. The team will review the Pull Request and aim to respond within 1-10 business days. Meanwhile, please read about the Contribution and Code of Conduct guidelines here. You can find out more information about eksctl on our website

@@ -6,6 +6,7 @@ import (

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/kris-nova/logger"
"k8s.io/apimachinery/pkg/util/sets"
Copy link
Member

Choose a reason for hiding this comment

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

Nice, TIL about apimachinery string sets!

For some clusters, EKS can return the list of public endpoint CIDRs out of
order, and won't allow updates where the incoming and current sets have set
equality (i.e. regardless of order of CIDR entries). This change restores the
set equality check that was removed in commit
72605fb and adds an additional test case to
cover this case.
@yuxiang-zhang yuxiang-zhang merged commit 054a558 into eksctl-io:main Jan 19, 2024
9 checks passed
@Emberwalker Emberwalker deleted the public-endpoint-cidrs-unordered branch January 22, 2024 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants