fix: keep bootstrapRef across the VM creation request-id patch

controllers/nutanixmachine_controller.go

@@ -1885,23 +1885,26 @@ func (r *NutanixMachineReconciler) getOrMintVMCreationRequestID(rctx *nctx.Machi
 		return requestID, nil
 	}
 
-	// Snapshot the object *before* mutating it: patchMachine builds its diff baseline from
-	// rctx.NutanixMachine at the time it's called, so if we mutated it first, the baseline
-	// would already contain the new annotation and the resulting patch would be a no-op -
-	// silently defeating the "persist before anything else" guarantee this function exists
-	// to provide.
-	before := rctx.NutanixMachine.DeepCopy()
-
+	// Patch a copy, not rctx.NutanixMachine. The client overwrites the object it is given
+	// with the API server's response, which would discard every change made in memory
+	// earlier in this reconcile that is not persisted yet (the finalizer, spec.bootstrapRef).
+	// Those stay on rctx.NutanixMachine and are persisted by the deferred patch in Reconcile.
 	requestID := uuid.NewString()
-	if rctx.NutanixMachine.Annotations == nil {
-		rctx.NutanixMachine.Annotations = map[string]string{}
+	patched := rctx.NutanixMachine.DeepCopy()
+	if patched.Annotations == nil {
+		patched.Annotations = map[string]string{}
 	}
-	rctx.NutanixMachine.Annotations[VMCreationRequestIDAnnotation] = requestID
+	patched.Annotations[VMCreationRequestIDAnnotation] = requestID
 
-	if err := r.Patch(rctx.Context, rctx.NutanixMachine, client.MergeFrom(before)); err != nil {
+	if err := r.Patch(rctx.Context, patched, client.MergeFrom(rctx.NutanixMachine)); err != nil {
 		return "", fmt.Errorf("failed to persist vm creation request id: %w", err)
 	}
 
+	if rctx.NutanixMachine.Annotations == nil {
+		rctx.NutanixMachine.Annotations = map[string]string{}
+	}
+	rctx.NutanixMachine.Annotations[VMCreationRequestIDAnnotation] = requestID
+
 	return requestID, nil
 }
 
@@ -2446,6 +2449,9 @@ func (r *NutanixMachineReconciler) logProfileNicMapping(
 func (r *NutanixMachineReconciler) addGuestCustomizationToDeployParams(rctx *nctx.MachineContext, params *vmmconfig.DeployVmFromVmProfileParams) error {
 	// Get the bootstrapData
 	bootstrapRef := rctx.NutanixMachine.Spec.BootstrapRef
+	if bootstrapRef == nil {
+		return errors.New("NutanixMachine spec.BootstrapRef is nil.")
+	}
 	if bootstrapRef.Kind == infrav1.NutanixMachineBootstrapRefKindSecret {
 		bootstrapData, err := r.getBootstrapData(rctx)
 		if err != nil {
@@ -2536,6 +2542,9 @@ func (r *NutanixMachineReconciler) powerOnVM(rctx *nctx.MachineContext, vmUUID,
 func (r *NutanixMachineReconciler) addGuestCustomizationToVM(rctx *nctx.MachineContext, vm *vmmconfig.Vm) error {
 	// Get the bootstrapData
 	bootstrapRef := rctx.NutanixMachine.Spec.BootstrapRef
+	if bootstrapRef == nil {
+		return errors.New("NutanixMachine spec.BootstrapRef is nil.")
+	}
 	if bootstrapRef.Kind == infrav1.NutanixMachineBootstrapRefKindSecret {
 		bootstrapData, err := r.getBootstrapData(rctx)
 		if err != nil {

controllers/nutanixmachine_controller_test.go

@@ -3115,7 +3115,7 @@ func TestNutanixMachineReconciler_getOrMintVMCreationRequestID(t *testing.T) {
 		mockK8sClient := mockctlclient.NewMockClient(ctrl)
 
 		var appliedPatch []byte
-		mockK8sClient.EXPECT().Patch(ctx, ntnxMachine, gomock.Any()).DoAndReturn(
+		mockK8sClient.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&infrav1.NutanixMachine{}), gomock.Any()).DoAndReturn(
 			func(_ context.Context, obj client.Object, patch client.Patch, _ ...client.PatchOption) error {
 				data, err := patch.Data(obj)
 				require.NoError(t, err)
@@ -3169,6 +3169,59 @@ func TestNutanixMachineReconciler_getOrMintVMCreationRequestID(t *testing.T) {
 		require.NoError(t, err)
 		assert.Equal(t, existingRequestID, requestID)
 	})
+
+	t.Run("keeps unpersisted in-memory changes across the request-id patch", func(t *testing.T) {
+		ctx := context.Background()
+		// The API object has neither the finalizer nor bootstrapRef yet. reconcileNormal
+		// and ensureBootstrapRef set them in memory before this patch runs.
+		stored := &infrav1.NutanixMachine{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test-machine",
+				Namespace: "default",
+			},
+		}
+		scheme := runtime.NewScheme()
+		require.NoError(t, infrav1.AddToScheme(scheme))
+		fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(stored).Build()
+
+		live := stored.DeepCopy()
+		live.Finalizers = []string{infrav1.NutanixMachineFinalizer}
+		live.Spec.BootstrapRef = &corev1.ObjectReference{
+			APIVersion: "v1",
+			Kind:       infrav1.NutanixMachineBootstrapRefKindSecret,
+			Name:       "bootstrap-secret",
+			Namespace:  "default",
+		}
+
+		reconciler := &NutanixMachineReconciler{Client: fakeClient}
+		rctx := &nctx.MachineContext{Context: ctx, NutanixMachine: live}
+
+		requestID, err := reconciler.getOrMintVMCreationRequestID(rctx)
+		require.NoError(t, err)
+		require.NotNil(t, live.Spec.BootstrapRef, "in-memory bootstrapRef must survive the patch response")
+		assert.Equal(t, "bootstrap-secret", live.Spec.BootstrapRef.Name)
+		assert.Equal(t, []string{infrav1.NutanixMachineFinalizer}, live.Finalizers, "in-memory finalizer must survive the patch response")
+		assert.Equal(t, requestID, live.Annotations[VMCreationRequestIDAnnotation])
+
+		persisted := &infrav1.NutanixMachine{}
+		require.NoError(t, fakeClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: "test-machine"}, persisted))
+		assert.Equal(t, requestID, persisted.Annotations[VMCreationRequestIDAnnotation])
+	})
+}
+
+func TestAddGuestCustomizationNilBootstrapRef(t *testing.T) {
+	reconciler := &NutanixMachineReconciler{}
+	rctx := &nctx.MachineContext{
+		Context:        context.Background(),
+		NutanixMachine: &infrav1.NutanixMachine{},
+		Machine:        &capiv1beta2.Machine{ObjectMeta: metav1.ObjectMeta{Name: "test-machine"}},
+	}
+
+	err := reconciler.addGuestCustomizationToVM(rctx, vmmModels.NewVm())
+	require.Error(t, err)
+
+	err = reconciler.addGuestCustomizationToDeployParams(rctx, vmmModels.NewDeployVmFromVmProfileParams())
+	require.Error(t, err)
 }
 
 func TestNutanixMachineReconciler_getOrCreateVM(t *testing.T) {