MachineDeployment loses track of MachineSet when labels change

[sethp-nr]

/kind bug

What steps did you take and what happened:

Given a MachineDeployment:

apiVersion: cluster.x-k8s.io/v1alpha2
kind: MachineDeployment
metadata:
  annotations:
    machinedeployment.clusters.k8s.io/revision: "1"
  name: nodes-0
spec:
  replicas: 1
  selector:
    matchLabels:
      set: node
  template:
    metadata:
      labels:
        cluster.x-k8s.io/cluster-name: cluster
        set: node
    spec:
      ...

I added (via kubectl edit) a label to both the selector and the template:

apiVersion: cluster.x-k8s.io/v1alpha2
kind: MachineDeployment
metadata:
  annotations:
    machinedeployment.clusters.k8s.io/revision: "1"
  name: nodes-0
spec:
  replicas: 1
  selector:
    matchLabels:
      set: node
      someLabel: foo
  template:
    metadata:
      labels:
        cluster.x-k8s.io/cluster-name: cluster
        set: node
        someLabel: foo
    spec:
      ...

The MachineDeployment controller created a new MachineSet matching the new template and scaled it up to the appropriate number of replicas, but never scaled down the old MachineSet.

What did you expect to happen:

waves hands the deploy to have completed successfully, respecting whatever rolling update strategy is configured (I was testing with a single replica, but I would be surprised if it followed the strategy).

I think the way the Deployment controller works is to use the owner ref to infer that the old ReplicaSet is still part of the same "deployment": https://github.com/kubernetes/kubernetes/blob/06dc8cf4cb0d3bdc7b718392a66ca82678d9982f/pkg/controller/deployment/deployment_controller.go#L498-L499

Anything else you would like to add:

I haven't done a thorough look yet, but it seems like we might be able to add a "is controlled by this machinedeployment -> append to filtered" block before we check the label selector here:

cluster-api/controllers/machinedeployment_controller.go

Line 196 in e8858ef

if !selector.Matches(labels.Set(ms.Labels)) {

?

Environment:

• Cluster-api version: recent v1alpha2 (and v1alpha1)

[detiber]

/priority important-soon