-
Notifications
You must be signed in to change notification settings - Fork 4.1k
fix: AWSCloudProvider should ignore unrecognized provider IDs #8047
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
base: master
Are you sure you want to change the base?
Conversation
Welcome @raykrueger! |
Hi @raykrueger. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: raykrueger The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
I had two tabs open, one filled out, one that was not, I just clicked submit on the wrong one. Sigh. Please hold! |
a72fdcd
to
e8a86c2
Compare
Ok, all better. Apologies for the spam. |
/lgtm |
/ok-to-test |
The AWSCloudProvider only supports aws://zone/name ProviderIDs. It should ignore ProviderIDs it does not recognize. Prior to this fix, an unrecognized ProviderID, such as eks-hybrid://zone/cluster/my-node which is used by EKS Hybrid Nodes, will break the Autoscaler loop. This fix returns logs a warning, and returns nil, nil instead of returning the error.
e8a86c2
to
3a19738
Compare
/lgtm |
Fixes #8045
The AWSCloudProvider only supports aws://zone/name ProviderIDs. It should ignore ProviderIDs it does not recognize. Prior to this fix, an unrecognized ProviderID, such as eks-hybrid://zone/cluster/my-node which is used by EKS Hybrid Nodes, will break the Autoscaler loop.
This fix returns logs a warning, and returns nil, nil instead of returning the error.
What type of PR is this?
/kind bug
What this PR does / why we need it:
The launch of Amazon EKS Hybrid Nodes introduced a new ProviderID of
eks-hybrid://zone/cluster/name
. This new ProviderID causes the AWSCloudProvider to break out of it's reconciliation loop because it returns an error when encountering this new ID.The AWSCloudProvider only supports aws://zone/name ProviderIDs, it should ignore any alternatives (just like it does for a blank iD). This prevents another fix if AWS introduces any alternate IDs in the future.
Which issue(s) this PR fixes:
Fixes #8045
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: