Merge pull request kubevirt#5917 from davidvossel/pdb-segfault-v41

[release-0.41] fix pdb controller segfault
liuminjian · Jun 25, 2021 · 7221a56 · 7221a56
2 parents 5b11da6 + b5d456e
commit 7221a56
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 9 deletions.
diff --git a/pkg/virt-controller/watch/drain/disruptionbudget/disruptionbudget.go b/pkg/virt-controller/watch/drain/disruptionbudget/disruptionbudget.go
@@ -342,16 +342,22 @@ func (c *DisruptionBudgetController) execute(key string) error {
 	}
 
 	// Fetch the latest Vm state from cache
-	obj, exists, err := c.vmiInformer.GetStore().GetByKey(key)
+	obj, vmiExists, err := c.vmiInformer.GetStore().GetByKey(key)
 
 	if err != nil {
 		return err
 	}
 
 	var vmi *virtv1.VirtualMachineInstance
 	// Once all finalizers are removed the vmi gets deleted and we can clean all expectations
-	if exists {
+	if vmiExists {
 		vmi = obj.(*virtv1.VirtualMachineInstance)
+	} else {
+		namespace, name, err := cache.SplitMetaNamespaceKey(key)
+		if err != nil {
+			return err
+		}
+		vmi = virtv1.NewVMIReferenceFromNameWithNS(namespace, name)
 	}
 
 	namespace, name, err := cache.SplitMetaNamespaceKey(key)
@@ -370,11 +376,11 @@ func (c *DisruptionBudgetController) execute(key string) error {
 	}
 
 	if len(pdbs) == 0 {
-		return c.sync(key, vmi, nil)
+		return c.sync(key, vmiExists, vmi, nil)
 	}
 
 	for i := range pdbs {
-		if syncErr := c.sync(key, vmi, pdbs[i]); syncErr != nil {
+		if syncErr := c.sync(key, vmiExists, vmi, pdbs[i]); syncErr != nil {
 			err = syncErr
 		}
 	}
@@ -473,12 +479,12 @@ func isPDBFromOldVMI(vmi *virtv1.VirtualMachineInstance, pdb *v1beta1.PodDisrupt
 	return ownerRef != nil && ownerRef.UID != vmi.UID
 }
 
-func (c *DisruptionBudgetController) sync(key string, vmi *virtv1.VirtualMachineInstance, pdb *v1beta1.PodDisruptionBudget) error {
-	migratableOnDrain := vmiMigratableOnDrain(vmi)
+func (c *DisruptionBudgetController) sync(key string, vmiExists bool, vmi *virtv1.VirtualMachineInstance, pdb *v1beta1.PodDisruptionBudget) error {
+	migratableOnDrain := vmiMigratableOnDrain(vmiExists, vmi)
 
 	// check for deletions if pod exists
 	if pdb != nil {
-		if vmi == nil || vmi.DeletionTimestamp != nil {
+		if !vmiExists || vmi.DeletionTimestamp != nil {
 			// being deleted
 			log.Log.Infof("deleting pdb %s/%s due to VMI deletion", pdb.Namespace, pdb.Name)
 			return c.deletePDB(key, pdb, vmi)
@@ -527,8 +533,8 @@ func (c *DisruptionBudgetController) pdbsForVMI(namespace, name string) ([]*v1be
 	return pdbs, nil
 }
 
-func vmiMigratableOnDrain(vmi *virtv1.VirtualMachineInstance) bool {
-	if vmi == nil || vmi.DeletionTimestamp != nil || vmi.Spec.EvictionStrategy == nil {
+func vmiMigratableOnDrain(vmiExists bool, vmi *virtv1.VirtualMachineInstance) bool {
+	if !vmiExists || vmi.DeletionTimestamp != nil || vmi.Spec.EvictionStrategy == nil {
 		return false
 	}
 	return *vmi.Spec.EvictionStrategy == virtv1.EvictionStrategyLiveMigrate

diff --git a/pkg/virt-controller/watch/drain/disruptionbudget/disruptionbudget_test.go b/pkg/virt-controller/watch/drain/disruptionbudget/disruptionbudget_test.go
@@ -96,6 +96,7 @@ var _ = Describe("Disruptionbudget", func() {
 		vmimInformer, _ = testutils.NewFakeInformerFor(&v1.VirtualMachineInstanceMigration{})
 		podInformer, _ = testutils.NewFakeInformerFor(&v12.Pod{})
 		recorder = record.NewFakeRecorder(100)
+		recorder.IncludeObject = true
 
 		controller = disruptionbudget.NewDisruptionBudgetController(vmiInformer, pdbInformer, podInformer, vmimInformer, recorder, virtClient)
 		mockQueue = testutils.NewMockWorkQueue(controller.Queue)
@@ -164,6 +165,17 @@ var _ = Describe("Disruptionbudget", func() {
 			testutils.ExpectEvent(recorder, disruptionbudget.SuccessfulDeletePodDisruptionBudgetReason)
 		})
 
+		It("should remove the pdb if VMI doesn't exist", func() {
+			vmi := newVirtualMachine()
+			vmi.Spec.EvictionStrategy = newEvictionStrategy()
+			pdb := newPodDisruptionBudget(vmi)
+			pdbFeeder.Add(pdb)
+
+			shouldExpectPDBDeletion(pdb)
+			controller.Execute()
+			testutils.ExpectEvent(recorder, disruptionbudget.SuccessfulDeletePodDisruptionBudgetReason)
+		})
+
 		It("should recreate the PDB if the VMI is recreated", func() {
 			vmi := newVirtualMachine()
 			vmi.Spec.EvictionStrategy = newEvictionStrategy()