From 12ff489908a3e1396c917a9796439d5e0f85cd3c Mon Sep 17 00:00:00 2001 From: Michael Weibel Date: Wed, 2 Apr 2025 17:16:05 +0200 Subject: [PATCH] avoid setting FailureReason/FailureMessage field will get deprecated in v1beta2, setting this currently avoids the Machine from being reconciled at all which does not make sense in this case because a Failed provisioningState is not necessarily terminal --- .../azuremachinepoolmachine_controller.go | 2 -- ...azuremachinepoolmachine_controller_test.go | 23 ++++++++++++++++++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/exp/controllers/azuremachinepoolmachine_controller.go b/exp/controllers/azuremachinepoolmachine_controller.go index 96aab4a5a82..5198fd27702 100644 --- a/exp/controllers/azuremachinepoolmachine_controller.go +++ b/exp/controllers/azuremachinepoolmachine_controller.go @@ -294,8 +294,6 @@ func (ampmr *AzureMachinePoolMachineController) reconcileNormal(ctx context.Cont switch state { case infrav1.Failed: ampmr.Recorder.Eventf(machineScope.AzureMachinePoolMachine, corev1.EventTypeWarning, "FailedVMState", "Azure scale set VM is in failed state") - machineScope.SetFailureReason(azure.UpdateError) - machineScope.SetFailureMessage(errors.Errorf("Azure VM state is %s", state)) case infrav1.Deleting: log.V(4).Info("deleting machine because state is Deleting", "machine", machineScope.Name()) if err := ampmr.Client.Delete(ctx, machineScope.Machine); err != nil { diff --git a/exp/controllers/azuremachinepoolmachine_controller_test.go b/exp/controllers/azuremachinepoolmachine_controller_test.go index c508d43123d..c72b3a02e2a 100644 --- a/exp/controllers/azuremachinepoolmachine_controller_test.go +++ b/exp/controllers/azuremachinepoolmachine_controller_test.go @@ -27,6 +27,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" "k8s.io/utils/ptr" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1" @@ -60,6 +61,26 @@ func TestAzureMachinePoolMachineReconciler_Reconcile(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) }, }, + { + Name: "should not set failure properties if VMSS VM has state Failed", + Setup: func(cb *fake.ClientBuilder, reconciler *mock_azure.MockReconcilerMockRecorder) { + objects := getReadyMachinePoolMachineClusterObjects(false, ptr.To(infrav1.Failed)) + reconciler.Reconcile(gomock2.AContext()).Return(nil) + cb.WithObjects(objects...) + }, + Verify: func(g *WithT, c client.Client, result ctrl.Result, err error) { + g.Expect(err).NotTo(HaveOccurred()) + + ampm := &infrav1exp.AzureMachinePoolMachine{} + err = c.Get(context.Background(), types.NamespacedName{ + Name: "ampm1", + Namespace: "default", + }, ampm) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(ampm.Status.FailureReason).To(BeNil()) + g.Expect(ampm.Status.FailureMessage).To(BeNil()) + }, + }, { Name: "should successfully delete", Setup: func(cb *fake.ClientBuilder, reconciler *mock_azure.MockReconcilerMockRecorder) { @@ -136,7 +157,7 @@ func TestAzureMachinePoolMachineReconciler_Reconcile(t *testing.T) { c.Setup(cb, reconciler.EXPECT()) cl := cb.Build() - controller := NewAzureMachinePoolMachineController(cl, nil, reconcilerutils.Timeouts{}, "foo", azure.NewCredentialCache()) + controller := NewAzureMachinePoolMachineController(cl, record.NewFakeRecorder(1), reconcilerutils.Timeouts{}, "foo", azure.NewCredentialCache()) controller.reconcilerFactory = func(_ *scope.MachinePoolMachineScope) (azure.Reconciler, error) { return reconciler, nil }