neonvm-controller: Remove support for DIMM slots (#1070)

This is part 1 of 2 for removing DIMM slots support and completing our
transition to using virtio-mem instead (ref #1060).

With this change, neonvm-controller will always assume that VMs are
using virtio-mem. The only checks exist at the top of the reconcile
functions, making sure that this is indeed the case.

The fields still exist in the CRD (although the're marked as
deprecated), so that rollback of this change is still sound.

neonvm-runner also retains support for both memory providers, so that
custom neonvm-runner images continue to work for at least one version
across the change.

Author: sharnoff

neonvm-controller/cmd/main.go

@@ -97,7 +97,6 @@ func main() {
 	var disableRunnerCgroup bool
 	var defaultCpuScalingMode vmv1.CpuScalingMode
 	var qemuDiskCacheSettings string
-	var defaultMemoryProvider vmv1.MemoryProvider
 	var memhpAutoMovableRatio string
 	var failurePendingPeriod time.Duration
 	var failingRefreshInterval time.Duration
@@ -137,7 +136,6 @@ func main() {
 	flag.Func("default-cpu-scaling-mode", "Set default cpu scaling mode to use for new VMs", defaultCpuScalingMode.FlagFunc)
 	flag.BoolVar(&disableRunnerCgroup, "disable-runner-cgroup", false, "Disable creation of a cgroup in neonvm-runner for fractional CPU limiting")
 	flag.StringVar(&qemuDiskCacheSettings, "qemu-disk-cache-settings", "cache=none", "Set neonvm-runner's QEMU disk cache settings")
-	flag.Func("default-memory-provider", "Set default memory provider to use for new VMs", defaultMemoryProvider.FlagFunc)
 	flag.StringVar(&memhpAutoMovableRatio, "memhp-auto-movable-ratio", "301", "For virtio-mem, set VM kernel's memory_hotplug.auto_movable_ratio")
 	flag.DurationVar(&failurePendingPeriod, "failure-pending-period", 1*time.Minute,
 		"the period for the propagation of reconciliation failures to the observability instruments")
@@ -148,11 +146,6 @@ func main() {
 			"Otherwise, the outdated pod might be left to terminate, while the new one is already running.")
 	flag.Parse()
 
-	if defaultMemoryProvider == "" {
-		fmt.Fprintln(os.Stderr, "missing required flag '-default-memory-provider'")
-		os.Exit(1)
-	}
-
 	logConfig := zap.NewProductionConfig()
 	logConfig.Sampling = nil // Disabling sampling; it's enabled by default for zap's production configs.
 	logConfig.Level.SetLevel(zap.InfoLevel)
@@ -197,7 +190,6 @@ func main() {
 		MaxConcurrentReconciles: concurrencyLimit,
 		SkipUpdateValidationFor: skipUpdateValidationFor,
 		QEMUDiskCacheSettings:   qemuDiskCacheSettings,
-		DefaultMemoryProvider:   defaultMemoryProvider,
 		MemhpAutoMovableRatio:   memhpAutoMovableRatio,
 		FailurePendingPeriod:    failurePendingPeriod,
 		FailingRefreshInterval:  failingRefreshInterval,

neonvm-controller/deployment.yaml

@@ -60,7 +60,6 @@ spec:
         # * cache.direct=on    - use O_DIRECT (don't abuse host's page cache!)
         # * cache.no-flush=on  - ignores disk flush operations (not needed; our disks are ephemeral)
         - "--qemu-disk-cache-settings=cache.writeback=on,cache.direct=on,cache.no-flush=on"
-        - "--default-memory-provider=VirtioMem"
         - "--memhp-auto-movable-ratio=801" # for virtio-mem, set memory_hotplug.auto_movable_ratio=801
         - "--failure-pending-period=1m"
         - "--failing-refresh-interval=15s"

neonvm/apis/neonvm/v1/virtualmachine_types.go

@@ -221,6 +221,7 @@ type Guest struct {
 	MemorySlotSize resource.Quantity `json:"memorySlotSize"`
 	// +optional
 	MemorySlots MemorySlots `json:"memorySlots"`
+	// Deprecated: MemoryProvider is ignored and always interpreted as equal to VirtioMem.
 	// +optional
 	MemoryProvider *MemoryProvider `json:"memoryProvider,omitempty"`
 	// +optional
@@ -248,18 +249,11 @@ type Guest struct {
 
 const virtioMemBlockSizeBytes = 8 * 1024 * 1024 // 8 MiB
 
-// ValidateForMemoryProvider returns an error iff the guest memory settings are invalid for the
-// MemoryProvider.
-//
-// This is used in two places. First, to validate VirtualMachine object creation. Second, to handle
-// the defaulting behavior for VirtualMachines that would be switching from DIMMSlots to VirtioMem
-// on restart. We place more restrictions on VirtioMem because we use 8MiB block sizes, so changing
-// to a new default can only happen if the memory slot size is a multiple of 8MiB.
-func (g Guest) ValidateForMemoryProvider(p MemoryProvider) error {
-	if p == MemoryProviderVirtioMem {
-		if g.MemorySlotSize.Value()%virtioMemBlockSizeBytes != 0 {
-			return fmt.Errorf("memorySlotSize invalid for memoryProvider VirtioMem: must be a multiple of 8Mi")
-		}
+// ValidateMemorySize returns an error iff the memory settings are invalid for use with virtio-mem
+// (the backing memory provider that we use)
+func (g Guest) ValidateMemorySize() error {
+	if g.MemorySlotSize.Value()%virtioMemBlockSizeBytes != 0 {
+		return fmt.Errorf("memorySlotSize invalid for use with virtio-mem: must be a multiple of 8Mi")
 	}
 	return nil
 }
@@ -584,6 +578,7 @@ type VirtualMachineStatus struct {
 	CPUs *MilliCPU `json:"cpus,omitempty"`
 	// +optional
 	MemorySize *resource.Quantity `json:"memorySize,omitempty"`
+	// Deprecated: MemoryProvider is ignored and always interpreted as equal to VirtioMem.
 	// +optional
 	MemoryProvider *MemoryProvider `json:"memoryProvider,omitempty"`
 	// +optional

neonvm/apis/neonvm/v1/virtualmachine_webhook.go

@@ -59,11 +59,13 @@ func (r *VirtualMachine) ValidateCreate() (admission.Warnings, error) {
 			r.Spec.Guest.CPUs.Max)
 	}
 
-	// validate .spec.guest.memorySlotSize w.r.t. .spec.guest.memoryProvider
-	if r.Spec.Guest.MemoryProvider != nil {
-		if err := r.Spec.Guest.ValidateForMemoryProvider(*r.Spec.Guest.MemoryProvider); err != nil {
-			return nil, fmt.Errorf(".spec.guest: %w", err)
-		}
+	// DIMMSlots memory provider is deprecated, and assumed to never be used.
+	if r.Spec.Guest.MemoryProvider != nil && *r.Spec.Guest.MemoryProvider == MemoryProviderDIMMSlots {
+		return nil, errors.New("DIMMSlots memory provider is deprecated and disabled")
+	}
+
+	if err := r.Spec.Guest.ValidateMemorySize(); err != nil {
+		return nil, fmt.Errorf(".spec.guest: %w", err)
 	}
 
 	// validate .spec.guest.memorySlots.use and .spec.guest.memorySlots.max

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

@@ -2585,6 +2585,8 @@ spec:
                   kernelImage:
                     type: string
                   memoryProvider:
+                    description: 'Deprecated: MemoryProvider is ignored and always
+                      interpreted as equal to VirtioMem.'
                     enum:
                     - DIMMSlots
                     - VirtioMem
@@ -3000,6 +3002,8 @@ spec:
               extraNetMask:
                 type: string
               memoryProvider:
+                description: 'Deprecated: MemoryProvider is ignored and always interpreted
+                  as equal to VirtioMem.'
                 enum:
                 - DIMMSlots
                 - VirtioMem

neonvm/samples/vm-example.dimmslots.yaml

@@ -31,10 +31,6 @@ type ReconcilerConfig struct {
 	// used in setting up the VM disks via QEMU's `-drive` flag.
 	QEMUDiskCacheSettings string
 
-	// DefaultMemoryProvider is the memory provider (dimm slots or virtio-mem) that will be used for
-	// new VMs (or, when old ones restart) if nothing is explicitly set.
-	DefaultMemoryProvider vmv1.MemoryProvider
-
 	// MemhpAutoMovableRatio specifies the value that new neonvm-runners will set as the
 	// kernel's 'memory_hotplug.auto_movable_ratio', iff the memory provider is virtio-mem.
 	//

pkg/neonvm/controllers/config.go

@@ -110,7 +110,6 @@ var _ = Describe("VirtualMachine controller", func() {
 					MaxConcurrentReconciles: 1,
 					SkipUpdateValidationFor: nil,
 					QEMUDiskCacheSettings:   "cache=none",
-					DefaultMemoryProvider:   vmv1.MemoryProviderDIMMSlots,
 					MemhpAutoMovableRatio:   "301",
 					FailurePendingPeriod:    1 * time.Minute,
 					FailingRefreshInterval:  1 * time.Minute,