Skip to content

Commit

Permalink
šŸ› MachineSet should allow scale down operations to proceed when templā€¦
Browse files Browse the repository at this point in the history 
ā€¦ates don't exist

Signed-off-by: Vince Prignano <[email protected]>
  • Loading branch information
@vincepri
vincepri committed Jul 31, 2024
1 parent 3119f1c commit 9e00a00
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 37 deletions.
27 changes: 21 additions & 6 deletions internal/controllers/machineset/machineset_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,12 +240,12 @@ func (r *Reconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster,
}

// Make sure to reconcile the external infrastructure reference.
if err := reconcileExternalTemplateReference(ctx, r.UnstructuredCachingClient, cluster, &machineSet.Spec.Template.Spec.InfrastructureRef); err != nil {
if err := r.reconcileExternalTemplateReference(ctx, cluster, machineSet, &machineSet.Spec.Template.Spec.InfrastructureRef); err != nil {
return ctrl.Result{}, err
}
// Make sure to reconcile the external bootstrap reference, if any.
if machineSet.Spec.Template.Spec.Bootstrap.ConfigRef != nil {
if err := reconcileExternalTemplateReference(ctx, r.UnstructuredCachingClient, cluster, machineSet.Spec.Template.Spec.Bootstrap.ConfigRef); err != nil {
if err := r.reconcileExternalTemplateReference(ctx, cluster, machineSet, machineSet.Spec.Template.Spec.Bootstrap.ConfigRef); err != nil {
return ctrl.Result{}, err
}
}
Expand Down Expand Up @@ -1115,21 +1115,36 @@ func (r *Reconciler) reconcileUnhealthyMachines(ctx context.Context, cluster *cl
return ctrl.Result{}, nil
}

func reconcileExternalTemplateReference(ctx context.Context, c client.Client, cluster *clusterv1.Cluster, ref *corev1.ObjectReference) error {
func (r *Reconciler) reconcileExternalTemplateReference(ctx context.Context, cluster *clusterv1.Cluster, ms *clusterv1.MachineSet, ref *corev1.ObjectReference) error {
if !strings.HasSuffix(ref.Kind, clusterv1.TemplateSuffix) {
return nil
}

if err := utilconversion.UpdateReferenceAPIContract(ctx, c, ref); err != nil {
if err := utilconversion.UpdateReferenceAPIContract(ctx, r.Client, ref); err != nil {
return err
}

obj, err := external.Get(ctx, c, ref, cluster.Namespace)
obj, err := external.Get(ctx, r.Client, ref, cluster.Namespace)
if err != nil {
if apierrors.IsNotFound(err) {
if _, ok := ms.Labels[clusterv1.MachineDeploymentNameLabel]; !ok {
// If the MachineSet is not in a MachineDeployment, return the error immediately.
return err
}
// When the MachineSet is part of a MachineDeployment but isn't the current revision, we should
// ignore the not found references and allow the controller to proceed.
owner, err := r.getOwnerMachineDeployment(ctx, ms)
if err != nil {
return err
}
if owner.Annotations[clusterv1.RevisionAnnotation] != ms.Annotations[clusterv1.RevisionAnnotation] {
return nil
}
}
return err
}

patchHelper, err := patch.NewHelper(obj, c)
patchHelper, err := patch.NewHelper(obj, r.Client)
if err != nil {
return err
}
Expand Down
119 changes: 88 additions & 31 deletions internal/controllers/machineset/machineset_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,71 @@ func TestMachineSetReconciler(t *testing.T) {
duration5m := &metav1.Duration{Duration: 5 * time.Minute}
replicas := int32(2)
version := "v1.14.2"
machineTemplateSpec := clusterv1.MachineTemplateSpec{
ObjectMeta: clusterv1.ObjectMeta{
Labels: map[string]string{
"label-1": "true",
},
Annotations: map[string]string{
"annotation-1": "true",
"precedence": "MachineSet",
},
},
Spec: clusterv1.MachineSpec{
ClusterName: testCluster.Name,
Version: &version,
Bootstrap: clusterv1.Bootstrap{
ConfigRef: &corev1.ObjectReference{
APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
Kind: "GenericBootstrapConfigTemplate",
Name: "ms-template",
},
},
InfrastructureRef: corev1.ObjectReference{
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
Kind: "GenericInfrastructureMachineTemplate",
Name: "ms-template",
},
NodeDrainTimeout: duration10m,
NodeDeletionTimeout: duration10m,
NodeVolumeDetachTimeout: duration10m,
},
}

machineDeployment := &clusterv1.MachineDeployment{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "md-",
Namespace: namespace.Name,
Annotations: map[string]string{
clusterv1.RevisionAnnotation: "10",
},
},
Spec: clusterv1.MachineDeploymentSpec{
ClusterName: testCluster.Name,
Replicas: &replicas,
Template: machineTemplateSpec,
},
}
g.Expect(env.Create(ctx, machineDeployment)).To(Succeed())

instance := &clusterv1.MachineSet{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "ms-",
Namespace: namespace.Name,
Labels: map[string]string{
"label-1": "true",
"label-1": "true",
clusterv1.MachineDeploymentNameLabel: machineDeployment.Name,
},
Annotations: map[string]string{
clusterv1.RevisionAnnotation: "10",
},
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: clusterv1.GroupVersion.String(),
Kind: "MachineDeployment",
Name: machineDeployment.Name,
UID: machineDeployment.UID,
},
},
},
Spec: clusterv1.MachineSetSpec{
Expand All @@ -100,36 +159,7 @@ func TestMachineSetReconciler(t *testing.T) {
"label-1": "true",
},
},
Template: clusterv1.MachineTemplateSpec{
ObjectMeta: clusterv1.ObjectMeta{
Labels: map[string]string{
"label-1": "true",
},
Annotations: map[string]string{
"annotation-1": "true",
"precedence": "MachineSet",
},
},
Spec: clusterv1.MachineSpec{
ClusterName: testCluster.Name,
Version: &version,
Bootstrap: clusterv1.Bootstrap{
ConfigRef: &corev1.ObjectReference{
APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
Kind: "GenericBootstrapConfigTemplate",
Name: "ms-template",
},
},
InfrastructureRef: corev1.ObjectReference{
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
Kind: "GenericInfrastructureMachineTemplate",
Name: "ms-template",
},
NodeDrainTimeout: duration10m,
NodeDeletionTimeout: duration10m,
NodeVolumeDetachTimeout: duration10m,
},
},
Template: machineTemplateSpec,
},
}

Expand Down Expand Up @@ -405,6 +435,33 @@ func TestMachineSetReconciler(t *testing.T) {

// Validate that the controller set the cluster name label in selector.
g.Expect(instance.Status.Selector).To(ContainSubstring(testCluster.Name))

t.Log("Verifying MachineSet can be scaled down when templates don't exist, and MachineSet is not current")
g.Expect(env.CleanupAndWait(ctx, bootstrapTmpl)).To(Succeed())
g.Expect(env.CleanupAndWait(ctx, infraTmpl)).To(Succeed())

t.Log("Updating Replicas on MachineSet")
patchHelper, err = patch.NewHelper(instance, env)
g.Expect(err).ToNot(HaveOccurred())
instance.SetAnnotations(map[string]string{
clusterv1.RevisionAnnotation: "9",
})
instance.Spec.Replicas = ptr.To(int32(1))
g.Expect(patchHelper.Patch(ctx, instance)).Should(Succeed())

// Verify that we have 1 replicas.
g.Eventually(func() (ready int) {
if err := env.List(ctx, machines, client.InNamespace(namespace.Name)); err != nil {
return -1
}
for _, m := range machines.Items {
if !m.DeletionTimestamp.IsZero() {
continue
}
ready++
}
return
}, timeout*3).Should(BeEquivalentTo(1))
})
}

Expand Down

0 comments on commit 9e00a00

Please sign in to comment.