diff --git a/pkg/controller/machine/controller.go b/pkg/controller/machine/controller.go index 8b84b7e0ab..a68208aa96 100644 --- a/pkg/controller/machine/controller.go +++ b/pkg/controller/machine/controller.go @@ -27,6 +27,7 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" @@ -434,7 +435,16 @@ func (r *ReconcileMachine) setPhase(machine *machinev1.Machine, phase string, er } // Since we may have mutated the local copy of the machine above, we need to calculate baseToPatch here. + // Any updates to the status must be done after this point. baseToPatch := client.MergeFrom(machine.DeepCopy()) + + if phase == phaseFailed { + if err := r.overrideFailedMachineProviderStatusState(machine); err != nil { + klog.Errorf("Failed to update machine provider status %q: %v", machine.GetName(), err) + return err + } + } + machine.Status.Phase = &phase machine.Status.ErrorMessage = nil now := metav1.Now() @@ -462,6 +472,45 @@ func (r *ReconcileMachine) patchFailedMachineInstanceAnnotation(machine *machine return nil } +// overrideFailedMachineProviderStatusState patches the state of the VM in the provider status if it is set. +// Not all providers set a state, but AWS, Azure, GCP and vSphere do. +// If the machine has gone into the Failed phase, and the providerStatus has already been set, +// the VM is in an unknown state. This function overrides the state. +func (r *ReconcileMachine) overrideFailedMachineProviderStatusState(machine *machinev1.Machine) error { + if machine.Status.ProviderStatus == nil { + return nil + } + + // instanceState is used by AWS, GCP and vSphere; vmState is used by Azure. + const instanceStateField = "instanceState" + const vmStateField = "vmState" + + providerStatus, err := runtime.DefaultUnstructuredConverter.ToUnstructured(machine.Status.ProviderStatus) + if err != nil { + return fmt.Errorf("could not covert provider status to unstructured: %v", err) + } + + // if the instanceState is set already, update it to unknown + if _, found, err := unstructured.NestedString(providerStatus, instanceStateField); err == nil && found { + if err := unstructured.SetNestedField(providerStatus, unknownInstanceState, instanceStateField); err != nil { + return fmt.Errorf("could not set %s: %v", instanceStateField, err) + } + } + + // if the vmState is set already, update it to unknown + if _, found, err := unstructured.NestedString(providerStatus, vmStateField); err == nil && found { + if err := unstructured.SetNestedField(providerStatus, unknownInstanceState, vmStateField); err != nil { + return fmt.Errorf("could not set %s: %v", instanceStateField, err) + } + } + + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(providerStatus, machine.Status.ProviderStatus); err != nil { + return fmt.Errorf("could not convert provider status from unstructured: %v", err) + } + + return nil +} + func machineIsProvisioned(machine *machinev1.Machine) bool { return len(machine.Status.Addresses) > 0 || stringPointerDeref(machine.Spec.ProviderID) != "" } diff --git a/pkg/controller/machine/controller_test.go b/pkg/controller/machine/controller_test.go index 3cf43202ad..0a3d34b6f4 100644 --- a/pkg/controller/machine/controller_test.go +++ b/pkg/controller/machine/controller_test.go @@ -326,10 +326,12 @@ func TestReconcileRequest(t *testing.T) { func TestSetPhase(t *testing.T) { testCases := []struct { - name string - phase string - errorMessage string - annotations map[string]string + name string + phase string + errorMessage string + annotations map[string]string + existingProviderStatus string + expectedProviderStatus string }{ { name: "when updating the phase to Running", @@ -345,6 +347,36 @@ func TestSetPhase(t *testing.T) { MachineInstanceStateAnnotationName: unknownInstanceState, }, }, + { + name: "when updating the phase to Failed with instanceState Set", + phase: phaseFailed, + errorMessage: "test", + annotations: map[string]string{ + MachineInstanceStateAnnotationName: unknownInstanceState, + }, + existingProviderStatus: `{"instanceState":"Running"}`, + expectedProviderStatus: `{"instanceState":"Unknown"}`, + }, + { + name: "when updating the phase to Failed with vmState Set", + phase: phaseFailed, + errorMessage: "test", + annotations: map[string]string{ + MachineInstanceStateAnnotationName: unknownInstanceState, + }, + existingProviderStatus: `{"vmState":"Running"}`, + expectedProviderStatus: `{"vmState":"Unknown"}`, + }, + { + name: "when updating the phase to Failed with state Set", + phase: phaseFailed, + errorMessage: "test", + annotations: map[string]string{ + MachineInstanceStateAnnotationName: unknownInstanceState, + }, + existingProviderStatus: `{"state":"Running"}`, + expectedProviderStatus: `{"state":"Running"}`, + }, } for _, tc := range testCases { @@ -374,10 +406,18 @@ func TestSetPhase(t *testing.T) { GenerateName: name, Namespace: namespace.Name, }, - Status: machinev1.MachineStatus{}, } + g.Expect(k8sClient.Create(ctx, machine)).To(Succeed()) + if tc.existingProviderStatus != "" { + machine.Status.ProviderStatus = &runtime.RawExtension{ + Raw: []byte(tc.existingProviderStatus), + } + } + + g.Expect(k8sClient.Status().Update(ctx, machine)).To(Succeed()) + namespacedName := types.NamespacedName{ Namespace: machine.Namespace, Name: machine.Name, @@ -422,6 +462,11 @@ func TestSetPhase(t *testing.T) { g.Expect(got.GetAnnotations()).To(Equal(tc.annotations)) g.Expect(machine.GetAnnotations()).To(Equal(tc.annotations)) + + if tc.existingProviderStatus != "" { + g.Expect(got.Status.ProviderStatus).ToNot(BeNil()) + g.Expect(got.Status.ProviderStatus.Raw).To(BeEquivalentTo(tc.expectedProviderStatus)) + } }) } }