Write the user provided IG spec to state store instead of the full spec #14127
Conversation
k8s-ci-robot
added
cncf-cla: yes
olemarkus
force-pushed
the
write-ig-input
branch
from
f5826f8 to
081075a
Compare
|
/retest |
| if c.Image != "" { | ||
| for _, group := range instanceGroups { | ||
| group.Spec.Image = c.Image | ||
| } | ||
| } | ||
| if c.MasterImage != "" { | ||
| for _, group := range masters { | ||
| group.Spec.Image = c.MasterImage | ||
| } | ||
| } | ||
| if c.NodeImage != "" { | ||
| for _, group := range nodes { | ||
| group.Spec.Image = c.NodeImage | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
There was a problem hiding this comment.
I don't see any reference to NodeImage and I don't see Image being set first and MasterImage and NodeImage overriding it.
There was a problem hiding this comment.
It happens the other way around now. So if the image doesn't get set during IG creation, it gets set by the default logic.
But I think you are right about NodeImage not being respected. We may be missing a test case here. Let me try to add one.
olemarkus
force-pushed
the
write-ig-input
branch
from
081075a to
20ececb
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hakman The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
lgtm
This restores the behaviour before kubernetes#14127, which wasn't documented / intended.
This restores the behaviour before kubernetes#14127, which wasn't documented / intended.
This restores the behaviour before kubernetes#14127, which wasn't documented / intended.
With clusters, we write the input spec to the cluster store. The full spec is also uploaded, which is eventually used by nodeup.
With IGs, however, we write the full spec (after expanding and setting all defaults) to the cluster store. Further defaults are then done in places less appropriate such as during nodeup or during the building of nodeup config. The reason for this is probably that nodeup used the IG spec earlier, while now this isn't needed since nodeup only uses the nodeup config.
This PR writes the input spec to the cluster store, but it does validate the expanded cluster spec (before the not so appropriate expansion though!). This will allow us to move more expansions to populateInstanceGroupSpec without mutating the user input before storing it.
Some of the setup logic done in populateInstanceGroupSpec such as setting image and machine type has been moved to new_cluster.
Some of the mutation has been skipped as well (some node labels and the provider type), which is the cause of the create cluster golden output diffs. There should be no diffs to update cluster or nodeup configs though.