v1alpha2: remove ProxmoxMachine.Spec.Target (mostly redundant with AllowedNodes of 1)

Author: 65278

api/v1alpha1/conversion.go

@@ -499,6 +499,12 @@ func Convert_v1beta1_ObjectMeta_To_v1beta2_ObjectMeta(in *clusterv1beta1.ObjectM
 	return nil
 }
 
+func Convert_v1alpha1_VirtualMachineCloneSpec_To_v1alpha2_VirtualMachineCloneSpec(in *VirtualMachineCloneSpec, out *v1alpha2.VirtualMachineCloneSpec, s conversion.Scope) error {
+	err := autoConvert_v1alpha1_VirtualMachineCloneSpec_To_v1alpha2_VirtualMachineCloneSpec(in, out, s)
+	// Target is honored as a single item slice in ConvertTo.
+	return err
+}
+
 // //
 // helpers
 // //

api/v1alpha1/conversion_test.go

@@ -282,6 +282,13 @@ func spokeProxmoxMachineSpec(in *ProxmoxMachineSpec, c randfill.Continue) {
 		in.Network = &NetworkSpec{}
 	}
 
+	// Target is converted to AllowedNodes[0] in hub and not restored on down-conversion.
+	// Normalize by moving Target to AllowedNodes (Target dominates AllowedNodes in v1alpha1).
+	if in.Target != nil {
+		in.AllowedNodes = []string{*in.Target}
+		in.Target = nil
+	}
+
 	// Handle Default Network Device
 	if in.Network.Default == nil {
 		in.Network.Default = &NetworkDevice{

api/v1alpha1/proxmoxmachine_conversion.go

@@ -87,6 +87,12 @@ func restoreProxmoxMachineSpec(src *ProxmoxMachineSpec, dst *v1alpha2.ProxmoxMac
 	clusterv1.Convert_int32_To_Pointer_int32(src.NumSockets, ok, restored.NumSockets, &dst.NumSockets)
 	clusterv1.Convert_int32_To_Pointer_int32(src.MemoryMiB, ok, restored.MemoryMiB, &dst.MemoryMiB)
 
+	// Turn ProxmoxMachineSpec.Target into allowedNodes. in v1alpha1, target will
+	// ignore AllowedNodes, so we can literally overwrite these.
+	if src.Target != nil {
+		dst.AllowedNodes = []string{*src.Target}
+	}
+
 	// restore fields that don't exist in v1alpha1
 	if dst.Network != nil && restored.Network != nil {
 		dst.Network.Zone = restored.Network.Zone

api/v1alpha1/zz_generated.conversion.go

@@ -249,10 +249,6 @@ type VirtualMachineCloneSpec struct {
 	// storage for full clone.
 	// +optional
 	Storage *string `json:"storage,omitempty"`
-
-	// target node. Only allowed if the original VM is on shared storage.
-	// +optional
-	Target *string `json:"target,omitempty"`
 }
 
 // TemplateMatchPolicy defines how MatchTags are evaluated against template tags.
@@ -702,8 +698,8 @@ func (r *ProxmoxMachine) GetTemplateMatchPolicy() TemplateMatchPolicy {
 	return TemplateMatchPolicyExact
 }
 
-// GetNode get the Proxmox node used to provision this machine.
-func (r *ProxmoxMachine) GetNode() string {
+// GetSourceNode gets the Proxmox node used to clone this machine from.
+func (r *ProxmoxMachine) GetSourceNode() string {
 	return ptr.Deref(r.Spec.SourceNode, "")
 }
 

api/v1alpha2/proxmoxmachine_types.go

@@ -1345,10 +1345,6 @@ spec:
                 minItems: 1
                 type: array
                 x-kubernetes-list-type: set
-              target:
-                description: target node. Only allowed if the original VM is on shared
-                  storage.
-                type: string
               templateID:
                 description: templateID the vm_template vmid used for cloning a new
                   VM.

api/v1alpha2/zz_generated.deepcopy.go

@@ -1221,10 +1221,6 @@ spec:
                         minItems: 1
                         type: array
                         x-kubernetes-list-type: set
-                      target:
-                        description: target node. Only allowed if the original VM
-                          is on shared storage.
-                        type: string
                       templateID:
                         description: templateID the vm_template vmid used for cloning
                           a new VM.

config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachines.yaml

@@ -434,7 +434,7 @@ func createVM(ctx context.Context, scope *scope.MachineScope) (proxmox.VMCloneRe
 	}
 
 	options := proxmox.VMCloneRequest{
-		Node:  scope.ProxmoxMachine.GetNode(),
+		Node:  scope.ProxmoxMachine.GetSourceNode(),
 		NewID: int(vmid),
 		Name:  scope.ProxmoxMachine.GetName(),
 	}
@@ -459,19 +459,12 @@ func createVM(ctx context.Context, scope *scope.MachineScope) (proxmox.VMCloneRe
 	if scope.ProxmoxMachine.Spec.Storage != nil {
 		options.Storage = *scope.ProxmoxMachine.Spec.Storage
 	}
-	if scope.ProxmoxMachine.Spec.Target != nil {
-		options.Target = *scope.ProxmoxMachine.Spec.Target
-	}
 
 	if scope.InfraCluster.ProxmoxCluster.Status.NodeLocations == nil {
 		scope.InfraCluster.ProxmoxCluster.Status.NodeLocations = new(infrav1.NodeLocations)
 	}
 
-	// if no target was specified but we have a set of nodes defined in the spec, we want to evenly distribute
-	// the nodes across the cluster.
-	if scope.ProxmoxMachine.Spec.Target == nil &&
-		(len(scope.InfraCluster.ProxmoxCluster.Spec.AllowedNodes) > 0 || len(scope.ProxmoxMachine.Spec.AllowedNodes) > 0) {
-		// select next node as a target
+	if len(scope.InfraCluster.ProxmoxCluster.Spec.AllowedNodes) > 0 || len(scope.ProxmoxMachine.Spec.AllowedNodes) > 0 {
 		var err error
 		options.Target, err = selectNextNode(ctx, scope)
 		if err != nil {

config/crd/bases/infrastructure.cluster.x-k8s.io_proxmoxmachinetemplates.yaml

@@ -119,7 +119,7 @@ func TestEnsureVirtualMachine_CreateVM_FullOptions(t *testing.T) {
 	machineScope.ProxmoxMachine.Spec.Pool = ptr.To("pool")
 	machineScope.ProxmoxMachine.Spec.SnapName = ptr.To("snap")
 	machineScope.ProxmoxMachine.Spec.Storage = ptr.To("storage")
-	machineScope.ProxmoxMachine.Spec.Target = ptr.To("node2")
+	machineScope.ProxmoxMachine.Spec.AllowedNodes = []string{"node2"}
 	expectedOptions := proxmox.VMCloneRequest{
 		Node:        "node1",
 		Name:        "test",
@@ -133,6 +133,7 @@ func TestEnsureVirtualMachine_CreateVM_FullOptions(t *testing.T) {
 	}
 	response := proxmox.VMCloneResponse{NewID: 123, Task: newTask()}
 	proxmoxClient.EXPECT().CloneVM(context.Background(), 123, expectedOptions).Return(response, nil).Once()
+	proxmoxClient.EXPECT().GetReservableMemoryBytes(context.Background(), "node2", int64(100)).Return(^uint64(0), nil).Once()
 
 	requeue, err := ensureVirtualMachine(context.Background(), machineScope)
 	require.NoError(t, err)
@@ -154,13 +155,14 @@ func TestEnsureVirtualMachine_CreateVM_FullOptions_TemplateSelector(t *testing.T
 			},
 		},
 	}
+
 	machineScope.ProxmoxMachine.Spec.Description = ptr.To("test vm")
 	machineScope.ProxmoxMachine.Spec.Format = ptr.To(infrav1.TargetStorageFormatRaw)
 	machineScope.ProxmoxMachine.Spec.Full = ptr.To(true)
 	machineScope.ProxmoxMachine.Spec.Pool = ptr.To("pool")
 	machineScope.ProxmoxMachine.Spec.SnapName = ptr.To("snap")
 	machineScope.ProxmoxMachine.Spec.Storage = ptr.To("storage")
-	machineScope.ProxmoxMachine.Spec.Target = ptr.To("node2")
+	machineScope.ProxmoxMachine.Spec.AllowedNodes = []string{"node1", "node2"}
 	expectedOptions := proxmox.VMCloneRequest{
 		Node:        "node1",
 		Name:        "test",
@@ -175,10 +177,15 @@ func TestEnsureVirtualMachine_CreateVM_FullOptions_TemplateSelector(t *testing.T
 
 	// ResolutionPolicy is not set on the TemplateSelector in this test, so the default
 	// policy is "exact". The vmservice must therefore pass "exact" to FindVMTemplateByTags.
-	proxmoxClient.EXPECT().FindVMTemplateByTags(context.Background(), vmTemplateTags, "exact").Return("node1", 123, nil).Once()
+	proxmoxClient.EXPECT().
+		FindVMTemplateByTags(context.Background(), vmTemplateTags, string(infrav1.TemplateMatchPolicyExact)).
+		Return("node1", 123, nil).
+		Once()
 
 	response := proxmox.VMCloneResponse{NewID: 123, Task: newTask()}
 	proxmoxClient.EXPECT().CloneVM(context.Background(), 123, expectedOptions).Return(response, nil).Once()
+	proxmoxClient.EXPECT().GetReservableMemoryBytes(context.Background(), "node1", int64(100)).Return(0, nil).Once()
+	proxmoxClient.EXPECT().GetReservableMemoryBytes(context.Background(), "node2", int64(100)).Return(^uint64(0), nil).Once()
 
 	requeue, err := ensureVirtualMachine(context.Background(), machineScope)
 	require.NoError(t, err)
@@ -207,11 +214,10 @@ func TestEnsureVirtualMachine_CreateVM_FullOptions_TemplateSelector_VMTemplateNo
 	machineScope.ProxmoxMachine.Spec.Pool = ptr.To("pool")
 	machineScope.ProxmoxMachine.Spec.SnapName = ptr.To("snap")
 	machineScope.ProxmoxMachine.Spec.Storage = ptr.To("storage")
-	machineScope.ProxmoxMachine.Spec.Target = ptr.To("node2")
+	machineScope.ProxmoxMachine.Spec.AllowedNodes = []string{"node2"}
 
-	// As above, no ResolutionPolicy is set on the TemplateSelector, so "exact" is the default
-	// and must be passed to FindVMTemplateByTags. In this scenario the template lookup fails.
 	proxmoxClient.EXPECT().FindVMTemplateByTags(context.Background(), vmTemplateTags, "exact").Return("", -1, goproxmox.ErrTemplateNotFound).Once()
+	proxmoxClient.EXPECT().GetReservableMemoryBytes(context.Background(), "node2", int64(100)).Return(^uint64(0), nil).Once()
 
 	_, err := createVM(ctx, machineScope)