Skip to content

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

Merged
yuxiang-zhang merged 1 commit intoeksctl-io:mainfrom
Wyldern:public-endpoint-cidrs-unordered
Jan 19, 2024
Merged

Handle unordered public endpoint CIDRs from EKS in endpoint updates#7483
yuxiang-zhang merged 1 commit intoeksctl-io:mainfrom
Wyldern:public-endpoint-cidrs-unordered

Conversation

@Wyldern
Copy link
Copy Markdown
Contributor

@Wyldern Wyldern commented Jan 16, 2024

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
Copy Markdown
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


"github.com/aws/aws-sdk-go-v2/aws"
"github.com/kris-nova/logger"
"k8s.io/apimachinery/pkg/util/sets"
Copy link
Copy Markdown
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
@Wyldern Wyldern 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.

3 participants