Add support for MachineDeployment to detect infrastructure spec changes

[vincepri]

Signed-off-by: Vince Prignano [email protected]

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Related to #1038

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vincepri

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:

• OWNERS [vincepri]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[vincepri]

/assign @ncdc @detiber

[vincepri]

/test pull-cluster-api-test

[detiber]

In the past we had discussed making the InfrastructureSpec (as well as the BootstrapperSpec) be immutable, which would avoid the need to compute the hashes of those as part of the MachineDeployment, and instead would force a user to update the references to different objects.

[ncdc]

@detiber I don't remember having that discussion, but I definitely like the idea!

[detiber]

@ncdc the potential problem there is that we use the same object type for the template and linking to a Machine (where updates may actually be needed). I'm wondering if this is a good reason to introduce a provider-specified Template type that embeds the provider-specified type for this purpose. With the expectation that the provider-specified Template types have an associated webhook to disallow update/patch.

[vincepri]

Wouldn't the change proposed make the Machine template immutable in the same way when working in AWS with AutoScalingGroups? The InfrastructureRef is used as a template, but when modified a new version is rolled out as you'd expect.

The Machine template copies will be untouched. Infrastructure providers can disallow updating those objects without the need of another layer of indirection.

[detiber]

@vincepri I'm thinking that the Infrastrcture CRDs may have use cases where they need to be updated in place, similar to what we do today with the aws providerspecs.

[vincepri]

@detiber That could be allowed by providers on individual Machine objects (by using the referenced Infrastructure object). When we talked about MachineDeployment/MachineSet, I recall we wanted these to act like template spec is always immutable.

[detiber]

@vincepri that makes admission control more difficult, because now you have to query the api server or check owner references instead of just allowing/disallowing update/patch by type.

[vincepri]

Let's chat more on Friday 😄, I might be missing some details

[vincepri]

/close

[k8s-ci-robot]

@vincepri: Closed this PR.

In response to this:

/close

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/test-infra repository.