Correct code flow of node pools #703
Conversation
Signed-off-by: Vatsal Parekh <[email protected]>
| clusterSpecCopy.NodePools = make([]aksv1.AKSNodePool, 0, len(config.Spec.NodePools)) | ||
| for _, n := range config.Spec.NodePools { | ||
| if _, ok := upstreamNodePools[*n.Name]; ok { | ||
| clusterSpecCopy.NodePools = append(clusterSpecCopy.NodePools, n) |
There was a problem hiding this comment.
Why are you changing logic for provisioned clusters by Rancher? In results of this change non-imported clusters won't be updated.
There was a problem hiding this comment.
The loop iterating over config.NodePools checks if that NodePool is present in upstreamNodePools, which means that no new pool can be added or deleted. This logic seems confusing. Changing this only for Imported cluster seems logical
There was a problem hiding this comment.
Updates for NodePools need to work for both rancher managed clusters and imported one. Your change will break this and only imported cluster will be updated.
| clusterSpecCopy.NodePools = make([]aksv1.AKSNodePool, 0, len(config.Spec.NodePools)) | ||
| for _, n := range config.Spec.NodePools { | ||
| if _, ok := upstreamNodePools[*n.Name]; ok { | ||
| clusterSpecCopy.NodePools = append(clusterSpecCopy.NodePools, n) |
There was a problem hiding this comment.
Updates for NodePools need to work for both rancher managed clusters and imported one. Your change will break this and only imported cluster will be updated.
|
After testing this a couple of times, the behaviour described in #667 is still present:
|
What this PR does / why we need it:
Which issue(s) this PR fixes
Issue #667
Special notes for your reviewer:
Checklist: