From 3b9a410b693e4908d258104bb9eaac01a71aa10f Mon Sep 17 00:00:00 2001 From: Vladik Romanovsky Date: Tue, 30 Apr 2019 16:17:29 -0400 Subject: [PATCH] fix migration cancelation that was posted before migration start Signed-off-by: Vladik Romanovsky --- pkg/virt-controller/watch/migration.go | 18 +++--- pkg/virt-controller/watch/migration_test.go | 3 +- tests/migration_test.go | 63 ++++++++++++++++++++- 3 files changed, 70 insertions(+), 14 deletions(-) diff --git a/pkg/virt-controller/watch/migration.go b/pkg/virt-controller/watch/migration.go index 76ef31b89d5a..f2ce7a87ac28 100644 --- a/pkg/virt-controller/watch/migration.go +++ b/pkg/virt-controller/watch/migration.go @@ -293,15 +293,14 @@ func (c *MigrationController) updateStatus(migration *virtv1.VirtualMachineInsta migrationCopy.Status.Phase = virtv1.MigrationFailed c.recorder.Eventf(migration, k8sv1.EventTypeWarning, FailedMigrationReason, "Source node reported migration failed") log.Log.Object(migration).Error("VMI reported migration failed.") - } else if migration.DeletionTimestamp != nil && !migration.IsFinal() { - if !conditionManager.HasCondition(migration, virtv1.VirtualMachineInstanceMigrationAbortRequested) { - condition := virtv1.VirtualMachineInstanceMigrationCondition{ - Type: virtv1.VirtualMachineInstanceMigrationAbortRequested, - Status: k8sv1.ConditionTrue, - LastProbeTime: v1.Now(), - } - migrationCopy.Status.Conditions = append(migrationCopy.Status.Conditions, condition) + } else if migration.DeletionTimestamp != nil && !migration.IsFinal() && + !conditionManager.HasCondition(migration, virtv1.VirtualMachineInstanceMigrationAbortRequested) { + condition := virtv1.VirtualMachineInstanceMigrationCondition{ + Type: virtv1.VirtualMachineInstanceMigrationAbortRequested, + Status: k8sv1.ConditionTrue, + LastProbeTime: v1.Now(), } + migrationCopy.Status.Conditions = append(migrationCopy.Status.Conditions, condition) } else { switch migration.Status.Phase { @@ -415,7 +414,8 @@ func (c *MigrationController) sync(key string, migration *virtv1.VirtualMachineI pod = pods[0] } - if vmi != nil && migration.DeletionTimestamp != nil && !migration.IsFinal() { + if vmi != nil && migration.DeletionTimestamp != nil && + migration.Status.Phase == virtv1.MigrationRunning { vmiCopy := vmi.DeepCopy() if vmiCopy.Status.MigrationState != nil { vmiCopy.Status.MigrationState.AbortRequested = true diff --git a/pkg/virt-controller/watch/migration_test.go b/pkg/virt-controller/watch/migration_test.go index b0fb4fc6509d..e8879921a451 100644 --- a/pkg/virt-controller/watch/migration_test.go +++ b/pkg/virt-controller/watch/migration_test.go @@ -671,7 +671,7 @@ var _ = Describe("Migration watcher", func() { It("should abort the migration", func() { vmi := newVirtualMachine("testvmi", v1.Running) vmi.Status.NodeName = "node02" - migration := newMigration("testmigration", vmi.Name, v1.MigrationTargetReady) + migration := newMigration("testmigration", vmi.Name, v1.MigrationRunning) condition := v1.VirtualMachineInstanceMigrationCondition{ Type: v1.VirtualMachineInstanceMigrationAbortRequested, Status: k8sv1.ConditionTrue, @@ -693,7 +693,6 @@ var _ = Describe("Migration watcher", func() { podFeeder.Add(pod) vmiInterface.EXPECT().Patch(vmi.Name, types.JSONPatchType, gomock.Any()).Return(vmi, nil) - controller.Execute() testutils.ExpectEvent(recorder, SuccessfulAbortMigrationReason) }) diff --git a/tests/migration_test.go b/tests/migration_test.go index a4753175714e..c1d6d201c245 100644 --- a/tests/migration_test.go +++ b/tests/migration_test.go @@ -123,9 +123,7 @@ var _ = Describe("Migrations", func() { vmi, err = virtClient.VirtualMachineInstance(vmi.Namespace).Get(vmi.Name, &metav1.GetOptions{}) Expect(err).To(BeNil()) - Expect(vmi.Status.MigrationState).ToNot(BeNil()) - - if vmi.Status.MigrationState.Completed && + if vmi.Status.MigrationState != nil && vmi.Status.MigrationState.Completed && vmi.Status.MigrationState.AbortStatus == v1.MigrationAbortSucceeded { return true } @@ -208,6 +206,32 @@ var _ = Describe("Migrations", func() { return uid } + runAndImmediatelyCancelMigration := func(migration *v1.VirtualMachineInstanceMigration, vmi *v1.VirtualMachineInstance, timeout int) string { + By("Starting a Migration") + Eventually(func() error { + _, err := virtClient.VirtualMachineInstanceMigration(migration.Namespace).Create(migration) + return err + }, timeout, 1*time.Second).ShouldNot(HaveOccurred()) + + By("Waiting until the Migration is Running") + + uid := "" + Eventually(func() bool { + migration, err := virtClient.VirtualMachineInstanceMigration(migration.Namespace).Get(migration.Name, &metav1.GetOptions{}) + Expect(err).To(BeNil()) + uid = string(migration.UID) + if uid != "" { + return true + } + return false + + }, timeout, 1*time.Second).Should(Equal(true)) + + By("Cancelling a Migration") + Expect(virtClient.VirtualMachineInstanceMigration(migration.Namespace).Delete(migration.Name, &metav1.DeleteOptions{})).To(Succeed(), "Migration should be deleted successfully") + + return uid + } runMigrationAndExpectFailure := func(migration *v1.VirtualMachineInstanceMigration, timeout int) string { By("Starting a Migration") @@ -700,6 +724,39 @@ var _ = Describe("Migrations", func() { tests.WaitForVirtualMachineToDisappearWithTimeout(vmi, 240) }) + It("should be able successfully cancel a migration right after posting it", func() { + vmi := tests.NewRandomFedoraVMIWitGuestAgent() + vmi.Spec.Domain.Resources.Requests[k8sv1.ResourceMemory] = resource.MustParse("1Gi") + + By("Starting the VirtualMachineInstance") + vmi = runVMIAndExpectLaunch(vmi, 240) + + By("Checking that the VirtualMachineInstance console has expected output") + expecter, expecterErr := tests.LoggedInFedoraExpecter(vmi) + Expect(expecterErr).To(BeNil()) + defer expecter.Close() + + // execute a migration, wait for finalized state + By("Starting the Migration") + migration := tests.NewRandomMigration(vmi.Name, vmi.Namespace) + + migrationUID := runAndImmediatelyCancelMigration(migration, vmi, 180) + + // check VMI, confirm migration state + confirmVMIPostMigrationAborted(vmi, migrationUID, 180) + + By("Waiting for the migration object to disappear") + tests.WaitForMigrationToDisappearWithTimeout(migration, 240) + + // delete VMI + By("Deleting the VMI") + err = virtClient.VirtualMachineInstance(vmi.Namespace).Delete(vmi.Name, &metav1.DeleteOptions{}) + Expect(err).To(BeNil()) + + By("Waiting for VMI to disappear") + tests.WaitForVirtualMachineToDisappearWithTimeout(vmi, 240) + + }) }) })