Fix the VM name lazy intialize (#61)

* Fix the VM name lazy intialize

* Fix tests

* Address reviews

Author: mcbenjemaa

internal/service/vmservice/find.go

@@ -35,6 +35,9 @@ var (
 
 	// ErrVMNotFound VM is not Found in Proxmox.
 	ErrVMNotFound = errors.New("vm not found")
+
+	// ErrVMNotInitialized VM is not Initialized in Proxmox.
+	ErrVMNotInitialized = errors.New("vm not initialized")
 )
 
 // FindVM returns the Proxmox VM if the vmID is set, otherwise
@@ -50,6 +53,10 @@ func FindVM(ctx context.Context, scope *scope.MachineScope) (*proxmox.VirtualMac
 			scope.Error(err, "unable to find vm")
 			return nil, ErrVMNotFound
 		}
+		if vm.Name != scope.ProxmoxMachine.GetName() {
+			scope.Error(err, "vm is not initialized yet")
+			return nil, ErrVMNotInitialized
+		}
 		return vm, nil
 	}
 
@@ -72,11 +79,17 @@ func updateVMLocation(ctx context.Context, s *scope.MachineScope) error {
 
 	// We are looking for a machine with the ID and check if the name matches.
 	// Then we have to update the node in the machine and cluster status.
-	vm, err := s.InfraCluster.ProxmoxClient.FindVMResource(ctx, uint64(vmID))
+	rsc, err := s.InfraCluster.ProxmoxClient.FindVMResource(ctx, uint64(vmID))
 	if err != nil {
 		return err
 	}
 
+	// find the VM, to make sure the vm config is up-to-date.
+	vm, err := s.InfraCluster.ProxmoxClient.GetVM(ctx, rsc.Node, vmID)
+	if err != nil {
+		return errors.Wrapf(err, "unable to find vm with id %d", rsc.VMID)
+	}
+
 	// Requeue if machine doesn't have a name yet.
 	// It seems that the Proxmox source API does not always provide
 	// the latest information about the resources in the cluster.

internal/service/vmservice/find_test.go

@@ -76,15 +76,31 @@ func TestFindVM_NotCreated(t *testing.T) {
 	require.ErrorIs(t, err, ErrVMNotCreated)
 }
 
+func TestFindVM_NotInitialized(t *testing.T) {
+	ctx := context.TODO()
+	machineScope, proxmoxClient, _ := setupReconcilerTest(t)
+	vm := newRunningVM()
+	vm.Name = "bar"
+	machineScope.ProxmoxMachine.Spec.VirtualMachineID = ptr.To(int64(vm.VMID))
+	machineScope.ProxmoxMachine.Status.ProxmoxNode = ptr.To("node2")
+
+	proxmoxClient.EXPECT().GetVM(ctx, "node2", int64(123)).Return(vm, nil).Once()
+
+	_, err := FindVM(ctx, machineScope)
+	require.ErrorIs(t, err, ErrVMNotInitialized)
+}
+
 func TestUpdateVMLocation_MissingName(t *testing.T) {
 	ctx := context.TODO()
 	machineScope, proxmoxClient, _ := setupReconcilerTest(t)
 	vm := newRunningVM()
 	vmr := newVMResource()
 	vmr.Name = ""
+	vm.Name = ""
 	machineScope.ProxmoxMachine.Spec.VirtualMachineID = ptr.To(int64(vm.VMID))
 
 	proxmoxClient.EXPECT().FindVMResource(ctx, uint64(123)).Return(vmr, nil).Once()
+	proxmoxClient.EXPECT().GetVM(ctx, "node1", int64(123)).Return(vm, nil).Once()
 
 	require.Error(t, updateVMLocation(ctx, machineScope))
 }
@@ -94,10 +110,13 @@ func TestUpdateVMLocation_NameMismatch(t *testing.T) {
 	machineScope, proxmoxClient, _ := setupReconcilerTest(t)
 	vm := newRunningVM()
 	vmr := newVMResource()
-	vmr.Name = "foo"
+	name := "foo"
+	vmr.Name = name
+	vm.Name = name
 	machineScope.ProxmoxMachine.Spec.VirtualMachineID = ptr.To(int64(vm.VMID))
 
 	proxmoxClient.EXPECT().FindVMResource(ctx, uint64(123)).Return(vmr, nil).Once()
+	proxmoxClient.EXPECT().GetVM(ctx, "node1", int64(123)).Return(vm, nil).Once()
 
 	require.Error(t, updateVMLocation(ctx, machineScope))
 	require.True(t, machineScope.HasFailed(), "expected failureReason and failureMessage to be set")
@@ -116,6 +135,7 @@ func TestUpdateVMLocation_UpdateNode(t *testing.T) {
 	}, false)
 
 	proxmoxClient.EXPECT().FindVMResource(ctx, uint64(123)).Return(vmr, nil).Once()
+	proxmoxClient.EXPECT().GetVM(ctx, "node1", int64(123)).Return(vm, nil).Once()
 
 	require.NoError(t, updateVMLocation(ctx, machineScope))
 	require.Equal(t, vmr.Node, *machineScope.ProxmoxMachine.Status.ProxmoxNode)
@@ -137,11 +157,14 @@ func TestUpdateVMLocation_WithoutTaskNameMismatch(t *testing.T) {
 	machineScope, proxmoxClient, _ := setupReconcilerTest(t)
 	vm := newRunningVM()
 	vmr := newVMResource()
-	vmr.Name = "foo"
+	name := "foo"
+	vmr.Name = name
+	vm.Name = name
 	machineScope.ProxmoxMachine.Spec.VirtualMachineID = ptr.To(int64(vm.VMID))
 	machineScope.ProxmoxMachine.Status.TaskRef = nil
 
 	proxmoxClient.EXPECT().FindVMResource(ctx, uint64(123)).Return(vmr, nil).Once()
+	proxmoxClient.EXPECT().GetVM(ctx, "node1", int64(123)).Return(vm, nil).Once()
 
 	require.Error(t, updateVMLocation(ctx, machineScope))
 	require.True(t, machineScope.HasFailed(), "expected failureReason and failureMessage to be set")

internal/service/vmservice/helpers_test.go

@@ -236,7 +236,7 @@ func newVMResource() *proxmox.ClusterResource {
 func newRunningVM() *proxmox.VirtualMachine {
 	return &proxmox.VirtualMachine{
 		VirtualMachineConfig: &proxmox.VirtualMachineConfig{},
-		Name:                 "running",
+		Name:                 "test",
 		Node:                 "node1",
 		Status:               proxmox.StatusVirtualMachineRunning,
 		VMID:                 123,
@@ -258,7 +258,7 @@ func newPausedVM() *proxmox.VirtualMachine {
 func newStoppedVM() *proxmox.VirtualMachine {
 	return &proxmox.VirtualMachine{
 		VirtualMachineConfig: &proxmox.VirtualMachineConfig{},
-		Name:                 "stopped",
+		Name:                 "test",
 		Node:                 "node1",
 		Status:               proxmox.StatusVirtualMachineStopped,
 		QMPStatus:            proxmox.StatusVirtualMachineStopped,

internal/service/vmservice/vm.go

@@ -107,6 +107,8 @@ func ensureVirtualMachine(ctx context.Context, machineScope *scope.MachineScope)
 
 			// we always want to trigger reconciliation at this point.
 			return false, err
+		case errors.Is(err, ErrVMNotInitialized):
+			return true, err
 		case !errors.Is(err, ErrVMNotCreated):
 			return false, err
 		}