Skip to content

Commit

Permalink
fix review findings
Browse files Browse the repository at this point in the history
  • Loading branch information
sbueringer committed Nov 23, 2022
1 parent 181c426 commit 10f016c
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 10 deletions.
4 changes: 2 additions & 2 deletions controlplane/kubeadm/internal/controllers/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,10 +331,10 @@ func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, cluster *

// Ensure all required labels exist on the controlled Machines.
// This logic is needed to add the `cluster.x-k8s.io/control-plane-name` label to Machines
// which were created before the `cluster.x-k8s.io/control-plane-name` label was introduced.
// which were created before the `cluster.x-k8s.io/control-plane-name` label was introduced
// or if a user manually removed the label.
// NOTE: Changes will be applied to the Machines in reconcileControlPlaneConditions.
// NOTE: cluster.x-k8s.io/control-plane is already set at this stage (it is used when reading controlPlane.Machines).
// TODO(sbueringer): Drop the following code with v1.4 after all existing Machines are guaranteed to have the new label.
for i := range controlPlane.Machines {
machine := controlPlane.Machines[i]
if value, ok := machine.Labels[clusterv1.MachineControlPlaneNameLabel]; !ok || value != kcp.Name {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,7 @@ func (r *Reconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster,
// Ensure all required labels exist on the controlled MachineSets.
// This logic is needed to add the `cluster.x-k8s.io/deployment-name` label to MachineSets
// which were created before the `cluster.x-k8s.io/deployment-name` label was added
// to all MachineSets created by a MachineDeployment.
// TODO(sbueringer): Drop the following code with v1.4 after all existing MachineSets are guaranteed to have the new label.
// to all MachineSets created by a MachineDeployment or if a user manually removed the label.
for idx := range msList {
machineSet := msList[idx]
if name, ok := machineSet.Labels[clusterv1.MachineDeploymentLabelName]; ok && name == d.Name {
Expand Down
18 changes: 12 additions & 6 deletions internal/controllers/machineset/machineset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,10 +293,16 @@ func (r *Reconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster,
// Ensure all required labels exist on the controlled Machines.
// This logic is needed to add the `cluster.x-k8s.io/set-name` label to Machines
// which were created before the `cluster.x-k8s.io/set-name` label was added to
// all Machines created by a MachineSet.
// TODO(sbueringer): Drop the following code with v1.4 after all existing Machines are guaranteed to have the new label.
// all Machines created by a MachineSet or if a user manually removed the label.
for _, machine := range filteredMachines {
if name, ok := machine.Labels[clusterv1.MachineSetLabelName]; ok && name == machineSet.Name {
mdNameOnMachineSet, mdNameSetOnMachineSet := machineSet.Labels[clusterv1.MachineDeploymentLabelName]
mdNameOnMachine := machine.Labels[clusterv1.MachineDeploymentLabelName]

if msName, ok := machine.Labels[clusterv1.MachineSetLabelName]; ok && msName == machineSet.Name &&
(!mdNameSetOnMachineSet || mdNameOnMachineSet == mdNameOnMachine) {
// Continue if the MachineSet name label is already set correctly and
// either the MachineDeployment name label is not set on the MachineSet or
// the MachineDeployment name label is set correctly on the Machine.
continue
}

Expand All @@ -305,9 +311,9 @@ func (r *Reconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster,
return ctrl.Result{}, errors.Wrapf(err, "failed to apply %s label to Machine %q", clusterv1.MachineSetLabelName, machine.Name)
}
machine.Labels[clusterv1.MachineSetLabelName] = machineSet.Name
// Propagate the MachineDeploymentLabelName from MachineSet to Machine if it exists.
if mdName, ok := machineSet.Labels[clusterv1.MachineDeploymentLabelName]; ok {
machine.Labels[clusterv1.MachineDeploymentLabelName] = mdName
// Propagate the MachineDeploymentLabelName from MachineSet to Machine if it is set on the MachineSet.
if mdNameSetOnMachineSet {
machine.Labels[clusterv1.MachineDeploymentLabelName] = mdNameOnMachineSet
}
if err := helper.Patch(ctx, machine); err != nil {
return ctrl.Result{}, errors.Wrapf(err, "failed to apply %s label to Machine %q", clusterv1.MachineSetLabelName, machine.Name)
Expand Down

0 comments on commit 10f016c

Please sign in to comment.