neonvm: add architecture field to vm.spec

neonvm/apis/neonvm/v1/virtualmachine_types.go

@@ -107,6 +107,10 @@ type VirtualMachineSpec struct {
 
 	ImagePullSecrets []corev1.LocalObjectReference `json:"imagePullSecrets,omitempty"`
 
+	// +kubebuilder:default:=amd64
+	// +optional
+	TargetArchitecture *CPUArchitecture `json:"targetArchitecture,omitempty"`
+
 	Guest Guest `json:"guest"`
 
 	// Running init containers is costly, so InitScript field should be preferred over ExtraInitContainers
@@ -170,6 +174,14 @@ func (spec *VirtualMachineSpec) Resources() VirtualMachineResources {
 	}
 }
 
+// +kubebuilder:validation:Enum=amd64;arm64
+type CPUArchitecture string
+
+const (
+	CPUArchitectureAMD64 CPUArchitecture = "amd64"
+	CPUArchitectureARM64 CPUArchitecture = "arm64"
+)
+
 // +kubebuilder:validation:Enum=QmpScaling;SysfsScaling
 type CpuScalingMode string
 
@@ -639,6 +651,7 @@ func (p VmPhase) IsAlive() bool {
 // +kubebuilder:printcolumn:name="Node",type=string,priority=1,JSONPath=`.status.node`
 // +kubebuilder:printcolumn:name="Image",type=string,priority=1,JSONPath=`.spec.guest.rootDisk.image`
 // +kubebuilder:printcolumn:name="CPUScalingMode",type=string,priority=1,JSONPath=`.spec.cpuScalingMode`
+// +kubebuilder:printcolumn:name="TargetArchitecture",type=string,priority=1,JSONPath=`.spec.targetArchitecture`
 type VirtualMachine struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`

neonvm/apis/neonvm/v1/virtualmachine_webhook.go

@@ -148,9 +148,20 @@ func (r *VirtualMachine) ValidateUpdate(old runtime.Object) (admission.Warnings,
 		}
 	}
 
-	// allow to change CPU scaling mode only if it's not set
-	if before.Spec.CpuScalingMode != nil && (r.Spec.CpuScalingMode == nil || *r.Spec.CpuScalingMode != *before.Spec.CpuScalingMode) {
-		return nil, fmt.Errorf(".spec.cpuScalingMode is not allowed to be changed once it's set")
+	fieldsAllowedToChangeFromNilOnly := []struct {
+		fieldName string
+		getter    func(*VirtualMachine) any
+	}{
+		{".spec.cpuScalingMode", func(v *VirtualMachine) any { return v.Spec.CpuScalingMode }},
+		{".spec.targetArchitecture", func(v *VirtualMachine) any { return v.Spec.TargetArchitecture }},
+	}
+
+	for _, info := range fieldsAllowedToChangeFromNilOnly {
+		beforeValue := info.getter(before)
+		newValue := info.getter(r)
+		if !reflect.ValueOf(beforeValue).IsNil() && (reflect.ValueOf(newValue).IsNil() || !reflect.DeepEqual(newValue, beforeValue)) {
+			return nil, fmt.Errorf("%s is not allowed to be changed once it's set", info.fieldName)
+		}
 	}
 
 	// validate .spec.guest.cpu.use

neonvm/apis/neonvm/v1/virtualmachine_webhook_test.go

@@ -0,0 +1,74 @@
+package v1
+
+import (
+	"testing"
+
+	"github.com/samber/lo"
+	"github.com/tychoish/fun/assert"
+)
+
+func TestFieldsAllowedToChangeFromNilOnly(t *testing.T) {
+	t.Run("should allow change from nil values", func(t *testing.T) {
+		defaultVm := &VirtualMachine{}
+		// defaultVm.Default() returns an object with nil value for fields we are interested in
+		defaultVm.Default()
+
+		fromNilFields := []struct {
+			setter func(*VirtualMachine)
+			field  string
+		}{
+			{
+				setter: func(vm *VirtualMachine) {
+					vm.Spec.CpuScalingMode = lo.ToPtr(CpuScalingModeQMP)
+				},
+				field: ".spec.cpuScalingMode",
+			},
+			{
+				setter: func(vm *VirtualMachine) {
+					vm.Spec.TargetArchitecture = lo.ToPtr(CPUArchitectureAMD64)
+				},
+				field: ".spec.targetArchitecture",
+			},
+		}
+
+		for _, field := range fromNilFields {
+			vm2 := defaultVm.DeepCopy()
+			field.setter(vm2)
+			_, err := vm2.ValidateUpdate(defaultVm)
+			assert.NotError(t, err)
+		}
+	})
+
+	t.Run("should not allow change from non-nil values", func(t *testing.T) {
+		defaultVm := &VirtualMachine{}
+		defaultVm.Default()
+		// override nil values with non-nil values
+		defaultVm.Spec.CpuScalingMode = lo.ToPtr(CpuScalingModeQMP)
+		defaultVm.Spec.TargetArchitecture = lo.ToPtr(CPUArchitectureAMD64)
+
+		fromNilFields := []struct {
+			setter func(*VirtualMachine)
+			field  string
+		}{
+			{
+				setter: func(vm *VirtualMachine) {
+					vm.Spec.CpuScalingMode = lo.ToPtr(CpuScalingModeSysfs)
+				},
+				field: ".spec.cpuScalingMode",
+			},
+			{
+				setter: func(vm *VirtualMachine) {
+					vm.Spec.TargetArchitecture = lo.ToPtr(CPUArchitectureARM64)
+				},
+				field: ".spec.targetArchitecture",
+			},
+		}
+
+		for _, field := range fromNilFields {
+			vm2 := defaultVm.DeepCopy()
+			field.setter(vm2)
+			_, err := vm2.ValidateUpdate(defaultVm)
+			assert.Error(t, err)
+		}
+	})
+}

neonvm/config/crd/bases/vm.neon.tech_virtualmachines.yaml

@@ -48,6 +48,10 @@ spec:
       name: CPUScalingMode
       priority: 1
       type: string
+    - jsonPath: .spec.targetArchitecture
+      name: TargetArchitecture
+      priority: 1
+      type: string
     name: v1
     schema:
       openAPIV3Schema:
@@ -2942,6 +2946,12 @@ spec:
                 type: boolean
               serviceAccountName:
                 type: string
+              targetArchitecture:
+                default: amd64
+                enum:
+                - amd64
+                - arm64
+                type: string
               targetRevision:
                 description: |-
                   TargetRevision is the identifier set by external party to track when changes to the spec

pkg/neonvm/controllers/vm_controller.go

@@ -27,7 +27,6 @@ import (
 	"fmt"
 	"os"
 	"reflect"
-	sysruntime "runtime"
 	"strconv"
 	"time"
 
@@ -164,15 +163,32 @@ func (r *VMReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Re
 		return ctrl.Result{}, nil
 	}
 
-	// examine cpuScalingMode and set it to the default value if it is not set
-	if vm.Spec.CpuScalingMode == nil {
-		log.Info("Setting default CPU scaling mode", "default", r.Config.DefaultCPUScalingMode)
-		vm.Spec.CpuScalingMode = lo.ToPtr(r.Config.DefaultCPUScalingMode)
-		if err := r.tryUpdateVM(ctx, &vm); err != nil {
-			log.Error(err, "Failed to set default CPU scaling mode")
-			return ctrl.Result{}, err
+	// examine for nil values that should be defaulted
+	// this part is done for values that we want eventually explicitly override in the kube-api storage
+	// to a default value.
+	{
+		changed := false
+		// examine targetArchitecture and set it to the default value if it is not set
+		if vm.Spec.TargetArchitecture == nil {
+			log.Info("Setting default target architecture", "default", vmv1.CPUArchitectureAMD64)
+			vm.Spec.TargetArchitecture = lo.ToPtr(vmv1.CPUArchitectureAMD64)
+			changed = true
+		}
+
+		// examine cpuScalingMode and set it to the default value if it is not set
+		if vm.Spec.CpuScalingMode == nil {
+			log.Info("Setting default CPU scaling mode", "default", r.Config.DefaultCPUScalingMode)
+			vm.Spec.CpuScalingMode = lo.ToPtr(r.Config.DefaultCPUScalingMode)
+			changed = true
+		}
+
+		if changed {
+			if err := r.tryUpdateVM(ctx, &vm); err != nil {
+				log.Error(err, "Failed to set default values for VirtualMachine")
+				return ctrl.Result{}, err
+			}
+			return ctrl.Result{Requeue: true}, nil
 		}
-		return ctrl.Result{Requeue: true}, nil
 	}
 
 	statusBefore := vm.Status.DeepCopy()
@@ -1145,25 +1161,28 @@ func affinityForVirtualMachine(vm *vmv1.VirtualMachine) *corev1.Affinity {
 	if a.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution == nil {
 		a.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution = &corev1.NodeSelector{}
 	}
-	// if NodeSelectorTerms list is empty - add default values (arch==amd64 or os==linux)
-	if len(a.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms) == 0 {
-		a.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms = append(
-			a.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms,
-			corev1.NodeSelectorTerm{
-				MatchExpressions: []corev1.NodeSelectorRequirement{
-					{
-						Key:      "kubernetes.io/arch",
-						Operator: "In",
-						Values:   []string{sysruntime.GOARCH},
-					},
-					{
-						Key:      "kubernetes.io/os",
-						Operator: "In",
-						Values:   []string{"linux"},
-					},
+	nodeSelector := a.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution
+
+	// always add default values (arch==vm.Spec.Affinity or os==linux) even if there are already some values
+	nodeSelector.NodeSelectorTerms = append(
+		nodeSelector.NodeSelectorTerms,
+		corev1.NodeSelectorTerm{
+			MatchExpressions: []corev1.NodeSelectorRequirement{
+				{
+					Key:      "kubernetes.io/os",
+					Operator: "In",
+					Values:   []string{"linux"},
 				},
-			})
-	}
+				{
+					Key:      "kubernetes.io/arch",
+					Operator: "In",
+					// vm.Spec.TargetArchitecture is guaranteed to be set by reconciler loop
+					Values: []string{string(*vm.Spec.TargetArchitecture)},
+				},
+			},
+		},
+	)
+
 	return a
 }
 

pkg/neonvm/controllers/vm_controller_unit_test.go

@@ -190,11 +190,12 @@ func TestReconcile(t *testing.T) {
 	// We now have a pod
 	vm := params.getVM()
 	assert.NotEmpty(t, vm.Status.PodName)
-	// Spec is unchanged except cpuScalingMode
-	var origVMWithCPUScalingMode vmv1.VirtualMachine
-	origVM.DeepCopy().DeepCopyInto(&origVMWithCPUScalingMode)
-	origVMWithCPUScalingMode.Spec.CpuScalingMode = lo.ToPtr(vmv1.CpuScalingModeQMP)
-	assert.Equal(t, vm.Spec, origVMWithCPUScalingMode.Spec)
+	// Spec is unchanged except cpuScalingMode and targetArchitecture
+	var origWithModifiedFields vmv1.VirtualMachine
+	origVM.DeepCopy().DeepCopyInto(&origWithModifiedFields)
+	origWithModifiedFields.Spec.CpuScalingMode = lo.ToPtr(vmv1.CpuScalingModeQMP)
+	origWithModifiedFields.Spec.TargetArchitecture = lo.ToPtr(vmv1.CPUArchitectureAMD64)
+	assert.Equal(t, vm.Spec, origWithModifiedFields.Spec)
 
 	// Round 4
 	res, err = params.r.Reconcile(params.ctx, req)
@@ -270,3 +271,39 @@ func TestRunningPod(t *testing.T) {
 	assert.Len(t, vm.Status.Conditions, 1)
 	assert.Equal(t, vm.Status.Conditions[0].Type, typeAvailableVirtualMachine)
 }
+
+func TestNodeAffinity(t *testing.T) {
+	t.Run("no affinity", func(t *testing.T) {
+		origVM := defaultVm()
+		origVM.Spec.TargetArchitecture = lo.ToPtr(vmv1.CPUArchitectureAMD64)
+		affinity := affinityForVirtualMachine(origVM)
+		prettyPrint(t, affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms)
+		assert.Equal(t, 1, len(affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms))
+		assert.Equal(t, "linux", affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions[0].Values[0])
+		assert.Equal(t, "amd64", affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions[1].Values[0])
+	})
+
+	t.Run("affinity given without architecture", func(t *testing.T) {
+		origVM := defaultVm()
+		origVM.Spec.TargetArchitecture = lo.ToPtr(vmv1.CPUArchitectureAMD64)
+		origVM.Spec.Affinity = &corev1.Affinity{
+			NodeAffinity: &corev1.NodeAffinity{
+				RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{
+					NodeSelectorTerms: []corev1.NodeSelectorTerm{
+						{
+							MatchExpressions: []corev1.NodeSelectorRequirement{
+								{Key: "topology.kubernetes.io/zone", Operator: "In", Values: []string{"zoneid"}},
+							},
+						},
+					},
+				},
+			},
+		}
+		affinity := affinityForVirtualMachine(origVM)
+		prettyPrint(t, affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms)
+		assert.Equal(t, 2, len(affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms))
+		assert.Equal(t, "zoneid", affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions[0].Values[0])
+		assert.Equal(t, "linux", affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[1].MatchExpressions[0].Values[0])
+		assert.Equal(t, "amd64", affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[1].MatchExpressions[1].Values[0])
+	})
+}

tests/e2e/default-values/00-assert.yaml

@@ -0,0 +1,12 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestAssert
+timeout: 10
+---
+apiVersion: vm.neon.tech/v1
+kind: VirtualMachine
+metadata:
+  name: example
+# default values for cpuScalingMode and cpuArchitecture
+spec:
+  cpuScalingMode: QmpScaling
+  targetArchitecture: amd64