Merge pull request #5694 from davidvossel/migration-segfault

Fixes migration controller null pointer dereference
kubevirt · May 24, 2021 · 3abefaa · 3abefaa
2 parents a52e9b5 + 5167f64
commit 3abefaa
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 11 deletions.
diff --git a/pkg/virt-controller/watch/migration.go b/pkg/virt-controller/watch/migration.go
@@ -687,6 +687,7 @@ func (c *MigrationController) sync(key string, migration *virtv1.VirtualMachineI
 		}
 	case virtv1.MigrationPreparingTarget, virtv1.MigrationTargetReady, virtv1.MigrationFailed:
 		if (!podExists || podIsDown(pod)) &&
+			vmi.Status.MigrationState != nil &&
 			len(vmi.Status.MigrationState.TargetDirectMigrationNodePorts) == 0 &&
 			vmi.Status.MigrationState.StartTimestamp == nil &&
 			!vmi.Status.MigrationState.Failed &&

diff --git a/pkg/virt-controller/watch/migration_test.go b/pkg/virt-controller/watch/migration_test.go
@@ -773,15 +773,18 @@ var _ = Describe("Migration watcher", func() {
 			controller.Execute()
 			testutils.ExpectEvent(recorder, SuccessfulAbortMigrationReason)
 		})
-		table.DescribeTable("should finalize migration on VMI if target pod fails before migration starts", func(phase v1.VirtualMachineInstanceMigrationPhase, hasPod bool, podPhase k8sv1.PodPhase) {
+		table.DescribeTable("should finalize migration on VMI if target pod fails before migration starts", func(phase v1.VirtualMachineInstanceMigrationPhase, hasPod bool, podPhase k8sv1.PodPhase, initializeMigrationState bool) {
 			vmi := newVirtualMachine("testvmi", v1.Running)
 			vmi.Status.NodeName = "node02"
 			migration := newMigration("testmigration", vmi.Name, phase)
 
-			vmi.Status.MigrationState = &v1.VirtualMachineInstanceMigrationState{
-				MigrationUID: migration.UID,
-				TargetNode:   "node01",
-				SourceNode:   "node02",
+			vmi.Status.MigrationState = nil
+			if initializeMigrationState {
+				vmi.Status.MigrationState = &v1.VirtualMachineInstanceMigrationState{
+					MigrationUID: migration.UID,
+					TargetNode:   "node01",
+					SourceNode:   "node02",
+				}
 			}
 			addMigration(migration)
 			addVirtualMachineInstance(vmi)
@@ -797,19 +800,25 @@ var _ = Describe("Migration watcher", func() {
 				shouldExpectMigrationFailedState(migration)
 			}
 
-			vmiInterface.EXPECT().Patch(vmi.Name, types.JSONPatchType, gomock.Any()).Return(vmi, nil)
+			if initializeMigrationState {
+				patch := `[{ "op": "test", "path": "/status/migrationState", "value": {"targetNode":"node01","sourceNode":"node02","migrationUid":"testmigration"} }, { "op": "replace", "path": "/status/migrationState", "value": {"startTimestamp":"2021-05-24T14:51:47Z","endTimestamp":"2021-05-24T14:51:47Z","targetNode":"node01","sourceNode":"node02","completed":true,"failed":true,"migrationUid":"testmigration"} }]`
+				shouldExpectVirtualMachineInstancePatch(vmi, patch)
+			}
 			controller.Execute()
 
 			// in this case, we have two failed events. one for the VMI and one on the Migration object.
-			testutils.ExpectEvent(recorder, FailedMigrationReason)
+			if initializeMigrationState {
+				testutils.ExpectEvent(recorder, FailedMigrationReason)
+			}
 			if phase != v1.MigrationFailed {
 				testutils.ExpectEvent(recorder, FailedMigrationReason)
 			}
 		},
-			table.Entry("in preparing target state", v1.MigrationPreparingTarget, true, k8sv1.PodFailed),
-			table.Entry("in target ready state", v1.MigrationTargetReady, true, k8sv1.PodFailed),
-			table.Entry("in failed state", v1.MigrationFailed, true, k8sv1.PodFailed),
-			table.Entry("in failed state and pod does not exist", v1.MigrationFailed, false, k8sv1.PodFailed),
+			table.Entry("in preparing target state", v1.MigrationPreparingTarget, true, k8sv1.PodFailed, true),
+			table.Entry("in target ready state", v1.MigrationTargetReady, true, k8sv1.PodFailed, true),
+			table.Entry("in failed state", v1.MigrationFailed, true, k8sv1.PodFailed, true),
+			table.Entry("in failed state before pod is created", v1.MigrationFailed, false, k8sv1.PodFailed, false),
+			table.Entry("in failed state and pod does not exist", v1.MigrationFailed, false, k8sv1.PodFailed, false),
 		)
 	})
 })