Skip to content

Commit

Permalink
Improve handling of patching and updates in MachineSet and MachineDep…
Browse files Browse the repository at this point in the history
…loyment controllers
  • Loading branch information
detiber committed Sep 9, 2019
1 parent 32d7146 commit 7139c0d
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 6 deletions.
8 changes: 7 additions & 1 deletion controllers/machinedeployment_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ func (r *MachineDeploymentReconciler) reconcile(ctx context.Context, d *clusterv
if d.Status.ObservedGeneration < d.Generation {
patch := client.MergeFrom(d.DeepCopy())
d.Status.ObservedGeneration = d.Generation
if err := r.Client.Status().Patch(ctx, d, patch); err != nil {
// Patch using a deep copy to avoid overwriting any unexpected Spec/Metadata changes from the returned result
if err := r.Client.Status().Patch(ctx, d.DeepCopy(), patch); err != nil {
klog.Warningf("Failed to patch status for MachineDeployment %q: %v", d.Name, err)
return ctrl.Result{}, err
}
Expand Down Expand Up @@ -144,12 +145,17 @@ func (r *MachineDeploymentReconciler) reconcile(ctx context.Context, d *clusterv
}

if cluster != nil && r.shouldAdopt(d) {
patch := client.MergeFrom(d.DeepCopy())
d.OwnerReferences = util.EnsureOwnerRef(d.OwnerReferences, metav1.OwnerReference{
APIVersion: cluster.APIVersion,
Kind: cluster.Kind,
Name: cluster.Name,
UID: cluster.UID,
})
// Patch using a deep copy to avoid overwriting any unexpected Status changes from the returned result
if err := r.Client.Patch(ctx, d.DeepCopy(), patch); err != nil {
return ctrl.Result{}, errors.Wrapf(err, "Failed to add OwnerReference to MachineDeployment %s/%s", d.Namespace, d.Name)
}
}

msList, err := r.getMachineSetsForDeployment(d)
Expand Down
10 changes: 6 additions & 4 deletions controllers/machinedeployment_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,14 +103,15 @@ func (r *MachineDeploymentReconciler) getNewMachineSet(d *clusterv1.MachineDeplo
// latest revision.
if existingNewMS != nil {
msCopy := existingNewMS.DeepCopy()
patch := client.MergeFrom(msCopy)

// Set existing new machine set's annotation
annotationsUpdated := mdutil.SetNewMachineSetAnnotations(d, msCopy, newRevision, true)

minReadySecondsNeedsUpdate := msCopy.Spec.MinReadySeconds != *d.Spec.MinReadySeconds
if annotationsUpdated || minReadySecondsNeedsUpdate {
msCopy.Spec.MinReadySeconds = *d.Spec.MinReadySeconds
return nil, r.Client.Update(context.Background(), msCopy)
return nil, r.Client.Patch(context.Background(), msCopy, patch)
}

// Apply revision annotation from existingNewMS if it is missing from the deployment.
Expand Down Expand Up @@ -341,7 +342,8 @@ func (r *MachineDeploymentReconciler) syncDeploymentStatus(allMSs []*clusterv1.M

patch := client.MergeFrom(d.DeepCopy())
d.Status = newStatus
return r.Client.Status().Patch(context.Background(), d, patch)
// Patch using a deep copy to avoid overwriting any unexpected Spec/Metadata changes from the returned result
return r.Client.Status().Patch(context.Background(), d.DeepCopy(), patch)
}

// calculateStatus calculates the latest status for the provided deployment by looking into the provided machine sets.
Expand Down Expand Up @@ -520,7 +522,7 @@ func updateMachineDeployment(c client.Client, d *clusterv1.MachineDeployment, mo
clusterv1.PopulateDefaultsMachineDeployment(d)
// Apply modifications.
modify(d)
// Patch the MachineDeployment.
return c.Patch(context.Background(), d, patch)
// Patch using a deep copy to avoid overwriting any unexpected Spec/Metadata changes from the returned result
return c.Patch(context.Background(), d.DeepCopy(), patch)
})
}
3 changes: 2 additions & 1 deletion controllers/machineset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,9 +386,10 @@ func shouldExcludeMachine(machineSet *clusterv1.MachineSet, machine *clusterv1.M

// adoptOrphan sets the MachineSet as a controller OwnerReference to the Machine.
func (r *MachineSetReconciler) adoptOrphan(machineSet *clusterv1.MachineSet, machine *clusterv1.Machine) error {
patch := client.MergeFrom(machine)
newRef := *metav1.NewControllerRef(machineSet, machineSetKind)
machine.OwnerReferences = append(machine.OwnerReferences, newRef)
return r.Client.Update(context.Background(), machine)
return r.Client.Patch(context.Background(), machine, patch)
}

func (r *MachineSetReconciler) waitForMachineCreation(machineList []*clusterv1.Machine) error {
Expand Down

0 comments on commit 7139c0d

Please sign in to comment.