diff --git a/pkg/virt-api/webhooks/validating-webhook/validating-webhook.go b/pkg/virt-api/webhooks/validating-webhook/validating-webhook.go index 79fef3a2db78..eb4c489faf2a 100644 --- a/pkg/virt-api/webhooks/validating-webhook/validating-webhook.go +++ b/pkg/virt-api/webhooks/validating-webhook/validating-webhook.go @@ -1729,11 +1729,13 @@ func admitMigrationCreate(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionRespons } // Reject migration jobs for non-migratable VMIs - cond := getVMIMigrationCondition(vmi) - if cond != nil && cond.Status == k8sv1.ConditionFalse { - errMsg := fmt.Errorf("Cannot migrate VMI, Reason: %s, Message: %s", - cond.Reason, cond.Message) - return webhooks.ToAdmissionResponseError(errMsg) + for _, c := range vmi.Status.Conditions { + if c.Type == v1.VirtualMachineInstanceIsMigratable && + c.Status == k8sv1.ConditionFalse { + errMsg := fmt.Errorf("Cannot migrate VMI, Reason: %s, Message: %s", + c.Reason, c.Message) + return webhooks.ToAdmissionResponseError(errMsg) + } } // Don't allow new migration jobs to be introduced when previous migration jobs @@ -1751,15 +1753,6 @@ func admitMigrationCreate(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionRespons return &reviewResponse } -func getVMIMigrationCondition(vmi *v1.VirtualMachineInstance) (cond *v1.VirtualMachineInstanceCondition) { - for _, c := range vmi.Status.Conditions { - if c.Type == v1.VirtualMachineInstanceIsMigratable { - cond = &c - } - } - return cond -} - func ServeMigrationCreate(resp http.ResponseWriter, req *http.Request) { serve(resp, req, admitMigrationCreate) } diff --git a/pkg/virt-api/webhooks/validating-webhook/validating-webhook_test.go b/pkg/virt-api/webhooks/validating-webhook/validating-webhook_test.go index d92eb9eb1147..1d8d5372929b 100644 --- a/pkg/virt-api/webhooks/validating-webhook/validating-webhook_test.go +++ b/pkg/virt-api/webhooks/validating-webhook/validating-webhook_test.go @@ -864,7 +864,7 @@ var _ = Describe("Validating Webhook", func() { It("should reject Migration spec for non-migratable VMIs", func() { vmi := v1.NewMinimalVMI("testmigratevmi3") - vmi.Status.Phase = v1.Succeeded + vmi.Status.Phase = v1.Running vmi.Status.Conditions = []v1.VirtualMachineInstanceCondition{ { Type: v1.VirtualMachineInstanceIsMigratable, @@ -872,6 +872,10 @@ var _ = Describe("Validating Webhook", func() { Reason: v1.VirtualMachineInstanceReasonDisksNotMigratable, Message: "cannot migrate VMI with mixes shared and non-shared volumes", }, + { + Type: v1.VirtualMachineInstanceReady, + Status: k8sv1.ConditionTrue, + }, } informers := webhooks.GetInformers() @@ -900,6 +904,7 @@ var _ = Describe("Validating Webhook", func() { resp := admitMigrationCreate(ar) Expect(resp.Allowed).To(Equal(false)) + Expect(resp.Result.Message).To(ContainSubstring("DisksNotLiveMigratable")) }) It("should reject Migration on update if spec changes", func() { diff --git a/tests/migration_test.go b/tests/migration_test.go index cbe1f1996c3a..cc6a9a918e88 100644 --- a/tests/migration_test.go +++ b/tests/migration_test.go @@ -373,7 +373,57 @@ var _ = Describe("Migrations", func() { By("Waiting for VMI to disappear") tests.WaitForVirtualMachineToDisappearWithTimeout(vmi, 240) + }) + }) + Context("with an Cirros non-shared ISCSI PVC", func() { + var pvName string + BeforeEach(func() { + pvName = "test-iscsi-lun" + rand.String(48) + // Start a ISCSI POD and service + By("Starting an iSCSI POD") + iscsiIP := tests.CreateISCSITargetPOD(tests.ContainerDiskCirros) + // create a new PV and PVC (PVs can't be reused) + By("create a new iSCSI PV and PVC") + tests.NewISCSIPvAndPvc(pvName, "1Gi", iscsiIP, k8sv1.ReadWriteOnce) + }, 60) + + AfterEach(func() { + // create a new PV and PVC (PVs can't be reused) + tests.DeletePvAndPvc(pvName) + }, 60) + It("should reject migrations for a non-migratable vmi", func() { + // Start the VirtualMachineInstance with the PVC attached + vmi := tests.NewRandomVMIWithPVC(pvName) + tests.AddUserData(vmi, "cloud-init", "#!/bin/bash\necho 'hello'\n") + vmi.Spec.Hostname = fmt.Sprintf("%s", tests.ContainerDiskCirros) + vmi = runVMIAndExpectLaunch(vmi, 180) + By("Checking that the VirtualMachineInstance console has expected output") + expecter, err := tests.LoggedInCirrosExpecter(vmi) + Expect(err).To(BeNil()) + expecter.Close() + + for _, c := range vmi.Status.Conditions { + if c.Type == v1.VirtualMachineInstanceIsMigratable { + Expect(c.Status).To(Equal(k8sv1.ConditionFalse)) + } + } + + // execute a migration, wait for finalized state + migration := tests.NewRandomMigration(vmi.Name, vmi.Namespace) + + By("Starting a Migration") + _, err = virtClient.VirtualMachineInstanceMigration(migration.Namespace).Create(migration) + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("DisksNotLiveMigratable")) + + // 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, 120) }) }) }) diff --git a/tests/utils.go b/tests/utils.go index 85653be8a4c2..08fbff4f5f21 100644 --- a/tests/utils.go +++ b/tests/utils.go @@ -2686,21 +2686,25 @@ func CreateISCSITargetPOD(containerDiskName ContainerDisk) (iscsiTargetIP string } func CreateISCSIPvAndPvc(name string, size string, iscsiTargetIP string) { + accessMode := k8sv1.ReadWriteMany + NewISCSIPvAndPvc(name, size, iscsiTargetIP, accessMode) +} +func NewISCSIPvAndPvc(name string, size string, iscsiTargetIP string, accessMode k8sv1.PersistentVolumeAccessMode) { virtCli, err := kubecli.GetKubevirtClient() PanicOnError(err) - _, err = virtCli.CoreV1().PersistentVolumes().Create(newISCSIPV(name, size, iscsiTargetIP)) + _, err = virtCli.CoreV1().PersistentVolumes().Create(newISCSIPV(name, size, iscsiTargetIP, accessMode)) if !errors.IsAlreadyExists(err) { PanicOnError(err) } - _, err = virtCli.CoreV1().PersistentVolumeClaims(NamespaceTestDefault).Create(newISCSIPVC(name, size)) + _, err = virtCli.CoreV1().PersistentVolumeClaims(NamespaceTestDefault).Create(newISCSIPVC(name, size, accessMode)) if !errors.IsAlreadyExists(err) { PanicOnError(err) } } -func newISCSIPV(name string, size string, iscsiTargetIP string) *k8sv1.PersistentVolume { +func newISCSIPV(name string, size string, iscsiTargetIP string, accessMode k8sv1.PersistentVolumeAccessMode) *k8sv1.PersistentVolume { quantity, err := resource.ParseQuantity(size) PanicOnError(err) @@ -2712,7 +2716,7 @@ func newISCSIPV(name string, size string, iscsiTargetIP string) *k8sv1.Persisten Name: name, }, Spec: k8sv1.PersistentVolumeSpec{ - AccessModes: []k8sv1.PersistentVolumeAccessMode{k8sv1.ReadWriteMany}, + AccessModes: []k8sv1.PersistentVolumeAccessMode{accessMode}, Capacity: k8sv1.ResourceList{ "storage": quantity, }, @@ -2734,7 +2738,7 @@ func newISCSIPV(name string, size string, iscsiTargetIP string) *k8sv1.Persisten } } -func newISCSIPVC(name string, size string) *k8sv1.PersistentVolumeClaim { +func newISCSIPVC(name string, size string, accessMode k8sv1.PersistentVolumeAccessMode) *k8sv1.PersistentVolumeClaim { quantity, err := resource.ParseQuantity(size) PanicOnError(err) @@ -2744,7 +2748,7 @@ func newISCSIPVC(name string, size string) *k8sv1.PersistentVolumeClaim { return &k8sv1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{Name: name}, Spec: k8sv1.PersistentVolumeClaimSpec{ - AccessModes: []k8sv1.PersistentVolumeAccessMode{k8sv1.ReadWriteMany}, + AccessModes: []k8sv1.PersistentVolumeAccessMode{accessMode}, Resources: k8sv1.ResourceRequirements{ Requests: k8sv1.ResourceList{ "storage": quantity,