diff --git a/pkg/controller/machine/controller.go b/pkg/controller/machine/controller.go index ab2cd5c63..ed8a0379a 100644 --- a/pkg/controller/machine/controller.go +++ b/pkg/controller/machine/controller.go @@ -346,7 +346,7 @@ func (r *ReconcileMachine) Reconcile(ctx context.Context, request reconcile.Requ "Failed to check if machine exists: %v", err, )) - if patchErr := r.updateStatus(ctx, m, ptr.Deref(m.Status.Phase, ""), nil, originalConditions); patchErr != nil { + if patchErr := r.updateStatus(ctx, m, ptr.Deref(m.Status.Phase, ""), err, originalConditions); patchErr != nil { klog.Errorf("%v: error patching status: %v", machineName, patchErr) } @@ -358,7 +358,7 @@ func (r *ReconcileMachine) Reconcile(ctx context.Context, request reconcile.Requ if err := r.actuator.Update(ctx, m); err != nil { klog.Errorf("%v: error updating machine: %v, retrying in %v seconds", machineName, err, requeueAfter) - if patchErr := r.updateStatus(ctx, m, ptr.Deref(m.Status.Phase, ""), nil, originalConditions); patchErr != nil { + if patchErr := r.updateStatus(ctx, m, ptr.Deref(m.Status.Phase, ""), err, originalConditions); patchErr != nil { klog.Errorf("%v: error patching status: %v", machineName, patchErr) } diff --git a/pkg/controller/machine/controller_test.go b/pkg/controller/machine/controller_test.go index 0b29b69b0..b6ad6993d 100644 --- a/pkg/controller/machine/controller_test.go +++ b/pkg/controller/machine/controller_test.go @@ -370,11 +370,16 @@ func TestReconcileRequest(t *testing.T) { result reconcile.Result error bool phase string + errorMessage *string } testCases := []struct { - request reconcile.Request - existsValue bool - expected expected + request reconcile.Request + existsValue bool + existsErr error + updateErr error + existsMachineHook func(*machinev1.Machine) + updateMachineHook func(*machinev1.Machine) + expected expected }{ { request: reconcile.Request{NamespacedName: types.NamespacedName{Name: machineNoPhase.Name, Namespace: machineNoPhase.Namespace}}, @@ -532,6 +537,42 @@ func TestReconcileRequest(t *testing.T) { phase: machinev1.PhaseRunning, }, }, + { + request: reconcile.Request{NamespacedName: types.NamespacedName{Name: machineProvisioning.Name, Namespace: machineProvisioning.Namespace}}, + existsValue: false, + existsErr: errors.New("actuator exists error"), + existsMachineHook: func(m *machinev1.Machine) { + m.Status.Phase = ptr.To(machinev1.PhaseFailed) + }, + expected: expected{ + createCallCount: 0, + existCallCount: 1, + updateCallCount: 0, + deleteCallCount: 0, + result: reconcile.Result{}, + error: true, + phase: machinev1.PhaseFailed, + errorMessage: ptr.To("actuator exists error"), + }, + }, + { + request: reconcile.Request{NamespacedName: types.NamespacedName{Name: machineProvisioned.Name, Namespace: machineProvisioned.Namespace}}, + existsValue: true, + updateErr: errors.New("actuator update error"), + updateMachineHook: func(m *machinev1.Machine) { + m.Status.Phase = ptr.To(machinev1.PhaseFailed) + }, + expected: expected{ + createCallCount: 0, + existCallCount: 1, + updateCallCount: 1, + deleteCallCount: 0, + result: reconcile.Result{RequeueAfter: requeueAfter}, + error: false, + phase: machinev1.PhaseFailed, + errorMessage: ptr.To("actuator update error"), + }, + }, } for _, tc := range testCases { @@ -542,6 +583,10 @@ func TestReconcileRequest(t *testing.T) { } act := newTestActuator() act.ExistsValue = tc.existsValue + act.ExistsErr = tc.existsErr + act.UpdateErr = tc.updateErr + act.ExistsMachineHook = tc.existsMachineHook + act.UpdateMachineHook = tc.updateMachineHook r := &ReconcileMachine{ Client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects( &machineNoPhase, @@ -598,6 +643,14 @@ func TestReconcileRequest(t *testing.T) { if tc.expected.phase != ptr.Deref(machine.Status.Phase, "") { t.Errorf("Case %s. Got: %v, expected: %v", tc.request.Name, ptr.Deref(machine.Status.Phase, ""), tc.expected.phase) } + + if tc.expected.errorMessage != nil { + if machine.Status.ErrorMessage == nil { + t.Errorf("Case %s. Expected errorMessage %q, got nil", tc.request.Name, *tc.expected.errorMessage) + } else if *machine.Status.ErrorMessage != *tc.expected.errorMessage { + t.Errorf("Case %s. Got errorMessage: %q, expected: %q", tc.request.Name, *machine.Status.ErrorMessage, *tc.expected.errorMessage) + } + } }) } } diff --git a/pkg/controller/machine/testactuator.go b/pkg/controller/machine/testactuator.go index 94917efda..b358416a9 100644 --- a/pkg/controller/machine/testactuator.go +++ b/pkg/controller/machine/testactuator.go @@ -26,17 +26,21 @@ import ( var _ Actuator = &TestActuator{} type TestActuator struct { - unblock chan string - BlockOnCreate bool - BlockOnDelete bool - BlockOnUpdate bool - BlockOnExists bool - CreateCallCount int64 - DeleteCallCount int64 - UpdateCallCount int64 - ExistsCallCount int64 - ExistsValue bool - Lock sync.Mutex + unblock chan string + BlockOnCreate bool + BlockOnDelete bool + BlockOnUpdate bool + BlockOnExists bool + CreateCallCount int64 + DeleteCallCount int64 + UpdateCallCount int64 + ExistsCallCount int64 + ExistsValue bool + ExistsErr error + UpdateErr error + ExistsMachineHook func(*machinev1.Machine) + UpdateMachineHook func(*machinev1.Machine) + Lock sync.Mutex } func (a *TestActuator) Create(context.Context, *machinev1.Machine) error { @@ -74,10 +78,13 @@ func (a *TestActuator) Update(ctx context.Context, machine *machinev1.Machine) e a.Lock.Lock() defer a.Lock.Unlock() a.UpdateCallCount++ - return nil + if a.UpdateMachineHook != nil { + a.UpdateMachineHook(machine) + } + return a.UpdateErr } -func (a *TestActuator) Exists(context.Context, *machinev1.Machine) (bool, error) { +func (a *TestActuator) Exists(_ context.Context, machine *machinev1.Machine) (bool, error) { defer func() { if a.BlockOnExists { <-a.unblock @@ -87,7 +94,10 @@ func (a *TestActuator) Exists(context.Context, *machinev1.Machine) (bool, error) a.Lock.Lock() defer a.Lock.Unlock() a.ExistsCallCount++ - return a.ExistsValue, nil + if a.ExistsMachineHook != nil { + a.ExistsMachineHook(machine) + } + return a.ExistsValue, a.ExistsErr } func newTestActuator() *TestActuator {