k8s-ci-robot
commented
1 month ago Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.
Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
linux-foundation-easycla[bot]
dims
sanjams2
kmala
== Motivation ==
Allow further variant specific customization
== Details ==
This change adds the ability for variants to implement their own logic to parse a k8s providerID into an identifier specific to that variant. This is done by adding a new method 'NodeId' on each variant that takes the providerID (in pre-parsed url format) and returns a respective NodeId. That said, the behavior of the existing Fargate variant and EC2 node are left unchanged.
NodeId is a refactor of the previous InstanceId which was inadequately named for variants other than EC2 instances (e.g. fargate). I have also taken this opportunity to add a bit more strong typing to the variant methods to make these methods a bit easier to grok and safer to implement. I have also renamed the KubernetesInstanceId to KubernetesProviderId which felt a more adequate name as well. Finally, in order to prevent circular depedencies and to maintain (what I felt) was a more logical concept of a NodeId, we have created a new submodule in the v1 package named 'awsnode' which contains the NodeId type. This allows both the base v1 module and the variant submodule to use this NodeId type withou circular dependencies.
I have opted not to change variables named
instanceIdor similar for the time being given the bloat that would cause on this CR size.Im pushing this change now to get feedback quickly. Can work on adding the necessary testing steps/release notes as required. Based on https://github.com/kubernetes/cloud-provider-aws/pull/917, it seems this may not be required for this change.