pool-manager: Prevent panic if VM template is nil (#431)

The template field is the only pointer in the relevant spec. Checking it is not nil will prevent kubemacpool from panicking Signed-off-by: Ram Lavi <[email protected]>
k8snetworkplumbingwg · Jul 3, 2024 · 1478ec9 · 1478ec9
1 parent b46ad98
commit 1478ec9
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 0 deletions.
diff --git a/pkg/pool-manager/pool_test.go b/pkg/pool-manager/pool_test.go
@@ -357,6 +357,16 @@ var _ = Describe("Pool", func() {
 		updateTransactionTimestamp := func(secondsPassed time.Duration) time.Time {
 			return time.Now().Add(secondsPassed * time.Second)
 		}
+		It("should not allocate if VM template is nil", func() {
+			poolManager := createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:02")
+			newInvalidVM := multipleInterfacesVM.DeepCopy()
+			newInvalidVM.Name = "newVM"
+			newInvalidVM.Spec.Template = nil
+
+			transactionTimestamp := updateTransactionTimestamp(0)
+			err := poolManager.AllocateVirtualMachineMac(newInvalidVM, &transactionTimestamp, true, logger)
+			Expect(err).ToNot(HaveOccurred())
+		})
 		It("should reject allocation if there are interfaces with the same name", func() {
 			poolManager := createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:02")
 			newVM := duplicateInterfacesVM.DeepCopy()
@@ -419,6 +429,26 @@ var _ = Describe("Pool", func() {
 			})
 		})
 		Describe("Update vm object", func() {
+			It("should not allocate if VM template is nil", func() {
+				poolManager := createPoolManager("02:00:00:00:00:00", "02:00:00:00:00:02")
+				newVM := multipleInterfacesVM.DeepCopy()
+				newVM.Name = "newVM"
+
+				transactionTimestamp := updateTransactionTimestamp(0)
+				err := poolManager.AllocateVirtualMachineMac(newVM, &transactionTimestamp, true, logger)
+				Expect(err).ToNot(HaveOccurred())
+
+				updateVm := multipleInterfacesVM.DeepCopy()
+				newInvalidVM := newVM.DeepCopy()
+				newInvalidVM.Spec.Template = nil
+				err = poolManager.UpdateMacAddressesForVirtualMachine(newInvalidVM, updateVm, &transactionTimestamp, true, logger)
+				Expect(err).ToNot(HaveOccurred())
+
+				updateInvalidVm := multipleInterfacesVM.DeepCopy()
+				updateInvalidVm.Spec.Template = nil
+				err = poolManager.UpdateMacAddressesForVirtualMachine(newVM, updateInvalidVm, &transactionTimestamp, true, logger)
+				Expect(err).ToNot(HaveOccurred())
+			})
 			It("should preserve disk.io configuration on update", func() {
 				addDiskIO := func(vm *kubevirt.VirtualMachine, ioName kubevirt.DriverIO) {
 					vm.Spec.Template.Spec.Domain.Devices.Disks = make([]kubevirt.Disk, 1)

diff --git a/pkg/pool-manager/virtualmachine_pool.go b/pkg/pool-manager/virtualmachine_pool.go
@@ -42,6 +42,10 @@ func (p *PoolManager) AllocateVirtualMachineMac(virtualMachine *kubevirt.Virtual
 	defer p.poolMutex.Unlock()
 	logger := parentLogger.WithName("AllocateVirtualMachineMac")
 
+	if virtualMachine.Spec.Template == nil {
+		logger.Info("virtual machine template is nil, skipping mac allocation", "virtualMachine", virtualMachine)
+		return nil
+	}
 	if len(virtualMachine.Spec.Template.Spec.Domain.Devices.Interfaces) == 0 {
 		logger.Info("no interfaces found for virtual machine, skipping mac allocation", "virtualMachine", virtualMachine)
 		return nil
@@ -133,6 +137,15 @@ func (p *PoolManager) UpdateMacAddressesForVirtualMachine(previousVirtualMachine
 	}
 	defer p.poolMutex.Unlock()
 
+	if previousVirtualMachine.Spec.Template == nil {
+		logger.Info("virtual machine template is nil, skipping mac allocation", "virtualMachine", virtualMachine)
+		return nil
+	}
+	if virtualMachine.Spec.Template == nil {
+		logger.Info("virtual machine template is nil, skipping mac allocation", "virtualMachine", virtualMachine)
+		return nil
+	}
+
 	// We can't allow for duplicate interfaces names, as interface.Name is macMap's key.
 	if isNotDryRun {
 		if err := checkVmForInterfaceDuplication(virtualMachine); err != nil {