diff --git a/controllers/nutanixmachine_controller.go b/controllers/nutanixmachine_controller.go index 25aa33b8f3..c6b777d222 100644 --- a/controllers/nutanixmachine_controller.go +++ b/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 { diff --git a/controllers/nutanixmachine_controller_test.go b/controllers/nutanixmachine_controller_test.go index e48e57c836..29800b332d 100644 --- a/controllers/nutanixmachine_controller_test.go +++ b/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) {