update kube-proxy to auto choose version #750
Conversation
zjaco13
requested review from
aws-ia-ci,
a team,
parkand1,
shapirov103,
youngjeong46 and
elamaran11
as code owners
There was a problem hiding this comment.
@zjaco13 Have a minor comment. Also the doc is not updated.
@shapirov103 Is this approach good. CA uses helm approach and coreaddon props is readonly.
There was a problem hiding this comment.
@zjaco13 It is a great start, I like the map. Please see my comment on the design. Happy to discuss if needed.
lib/addons/kube-proxy/index.ts
Outdated
| this.version = version ?? "auto"; | ||
| } | ||
|
|
||
| deploy(clusterInfo: ClusterInfo): Promise<Construct> { |
There was a problem hiding this comment.
Hey Zach, while this implementation works, it almost nullifies the inheritance - there is very little reason to derive this from a core addon.
I think we what we should do in this case is to enable this type of version selection for any core addon.
That means that the core addon should be able to accept optionally something like a versionProviderFunction that takes cluster info and spits out the version.
The kubeproxy will supply its function that will use the map. However, the default function will to just return whatever customers passed.
In this way, other core addons, the number of which will grow, will be able to address this aspect without sacrificing the central implementation component of such addons.
There was a problem hiding this comment.
Yep, that approach makes a ton of sense. We should discuss implementation.
lib/addons/core-addon/index.ts
Outdated
| provideVersion(clusterInfo: ClusterInfo, versionMap: Map<KubernetesVersion, string>) : string { | ||
| let version: string; | ||
| if(!versionMap.get(clusterInfo.version)) { |
There was a problem hiding this comment.
nitpick: no need to get from the map twice on line 144 and 147.
let version = versionMap.get();
|
@elamaran11 please review the latest |
|
/do-e2e-tests |
We can merge after e2e. |
Issue #, if available: #747
Description of changes:
Added version map for kube-proxy addon
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.