Fix v1alpha7 machine restorer and test

The v1alpha7 machine spec restorer was missing the exclusion for
ProviderID and InstanceID was missing, and the test which was supposed
to ensure it worked was broken. This fixes both.

Author: mdbooth

api/v1alpha7/conversion_test.go

@@ -166,22 +166,24 @@ func TestFuzzyConversion(t *testing.T) {
 func TestMachineConversionControllerSpecFields(t *testing.T) {
 	// This tests that we still do field restoration when the controller modifies ProviderID and InstanceID in the spec
 
-	g := gomega.NewWithT(t)
-	scheme := runtime.NewScheme()
-	g.Expect(AddToScheme(scheme)).To(gomega.Succeed())
-	g.Expect(infrav1.AddToScheme(scheme)).To(gomega.Succeed())
-
+	// Define an initial state which cannot be converted losslessly. We add
+	// an IdentityRef with a Kind, which has been removed in v1beta1.
 	testMachine := func() *OpenStackMachine {
 		return &OpenStackMachine{
-			Spec: OpenStackMachineSpec{},
+			Spec: OpenStackMachineSpec{
+				IdentityRef: &OpenStackIdentityReference{
+					Kind: "InvalidKind",
+					Name: "test-name",
+				},
+			},
 		}
 	}
 
 	tests := []struct {
-		name              string
-		modifyUp          func(*infrav1.OpenStackMachine)
-		testAfter         func(*OpenStackMachine)
-		expectNetworkDiff bool
+		name                  string
+		modifyUp              func(*infrav1.OpenStackMachine)
+		testAfter             func(gomega.Gomega, *OpenStackMachine)
+		expectIdentityRefDiff bool
 	}{
 		{
 			name: "No change",
@@ -191,47 +193,52 @@ func TestMachineConversionControllerSpecFields(t *testing.T) {
 			modifyUp: func(up *infrav1.OpenStackMachine) {
 				up.Spec.Flavor = "new-flavor"
 			},
-			testAfter: func(after *OpenStackMachine) {
+			testAfter: func(g gomega.Gomega, after *OpenStackMachine) {
 				g.Expect(after.Spec.Flavor).To(gomega.Equal("new-flavor"))
 			},
-			expectNetworkDiff: true,
+			expectIdentityRefDiff: true,
 		},
 		{
 			name: "Set ProviderID",
 			modifyUp: func(up *infrav1.OpenStackMachine) {
 				up.Spec.ProviderID = pointer.String("new-provider-id")
 			},
-			testAfter: func(after *OpenStackMachine) {
+			testAfter: func(g gomega.Gomega, after *OpenStackMachine) {
 				g.Expect(after.Spec.ProviderID).To(gomega.Equal(pointer.String("new-provider-id")))
 			},
-			expectNetworkDiff: false,
+			expectIdentityRefDiff: false,
 		},
 		{
 			name: "Set InstanceID",
 			modifyUp: func(up *infrav1.OpenStackMachine) {
 				up.Spec.InstanceID = pointer.String("new-instance-id")
 			},
-			testAfter: func(after *OpenStackMachine) {
+			testAfter: func(g gomega.Gomega, after *OpenStackMachine) {
 				g.Expect(after.Spec.InstanceID).To(gomega.Equal(pointer.String("new-instance-id")))
 			},
-			expectNetworkDiff: false,
+			expectIdentityRefDiff: false,
 		},
 		{
 			name: "Set ProviderID and non-ignored change",
 			modifyUp: func(up *infrav1.OpenStackMachine) {
 				up.Spec.ProviderID = pointer.String("new-provider-id")
 				up.Spec.Flavor = "new-flavor"
 			},
-			testAfter: func(after *OpenStackMachine) {
+			testAfter: func(g gomega.Gomega, after *OpenStackMachine) {
 				g.Expect(after.Spec.ProviderID).To(gomega.Equal(pointer.String("new-provider-id")))
 				g.Expect(after.Spec.Flavor).To(gomega.Equal("new-flavor"))
 			},
-			expectNetworkDiff: true,
+			expectIdentityRefDiff: true,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			g := gomega.NewWithT(t)
+			scheme := runtime.NewScheme()
+			g.Expect(AddToScheme(scheme)).To(gomega.Succeed())
+			g.Expect(infrav1.AddToScheme(scheme)).To(gomega.Succeed())
+
 			before := testMachine()
 
 			up := infrav1.OpenStackMachine{}
@@ -245,7 +252,14 @@ func TestMachineConversionControllerSpecFields(t *testing.T) {
 			g.Expect(after.ConvertFrom(&up)).To(gomega.Succeed())
 
 			if tt.testAfter != nil {
-				tt.testAfter(&after)
+				tt.testAfter(g, &after)
+			}
+
+			g.Expect(after.Spec.IdentityRef).ToNot(gomega.BeNil())
+			if tt.expectIdentityRefDiff {
+				g.Expect(after.Spec.IdentityRef.Kind).ToNot(gomega.Equal("InvalidKind"))
+			} else {
+				g.Expect(after.Spec.IdentityRef.Kind).To(gomega.Equal("InvalidKind"))
 			}
 		})
 	}

api/v1alpha7/openstackmachine_conversion.go

@@ -66,6 +66,19 @@ var v1alpha7OpenStackMachineRestorer = conversion.RestorerFor[*OpenStackMachine]
 			return &c.Spec
 		},
 		restorev1alpha7MachineSpec,
+		conversion.HashedFilterField[*OpenStackMachine, OpenStackMachineSpec](func(s *OpenStackMachineSpec) *OpenStackMachineSpec {
+			// Despite being spec fields, ProviderID and InstanceID
+			// are both set by the machine controller. If these are
+			// the only changes to the spec, we still want to
+			// restore the rest of the spec to its original state.
+			if s.ProviderID != nil || s.InstanceID != nil {
+				f := *s
+				f.ProviderID = nil
+				f.InstanceID = nil
+				return &f
+			}
+			return s
+		}),
 	),
 }