Added VolumesCreated condition to indicate status of created volumes

from VMSnapshots. Also added skipped phase in case no volume snapshots exist as part of the VMsnapshot. Signed-off-by: Alexander Wels <[email protected]>
zyjj1 · Aug 4, 2022 · 8ee68c3 · 8ee68c3
1 parent d6e09b8
commit 8ee68c3
Show file tree

Hide file tree

Showing 5 changed files with 238 additions and 67 deletions.
diff --git a/pkg/virt-controller/watch/export/export.go b/pkg/virt-controller/watch/export/export.go
@@ -74,15 +74,19 @@ const (
 	failedKeyFromObjectFmt = "failed to get key from object: %v, %v"
 	enqueuedForSyncFmt     = "enqueued %q for sync"
 
-	pvcNotFoundReason  = "pvcNotFound"
-	pvcBoundReason     = "pvcBound"
-	pvcPendingReason   = "pvcPending"
-	pvcInUseReason     = "pvcInUse"
-	unknownReason      = "unknown"
-	initializingReason = "initializing"
-	podPendingReason   = "podPending"
-	podReadyReason     = "podReady"
-	podCompletedReason = "podCompleted"
+	pvcNotFoundReason      = "pvcNotFound"
+	pvcBoundReason         = "pvcBound"
+	pvcPendingReason       = "pvcPending"
+	pvcInUseReason         = "pvcInUse"
+	unknownReason          = "unknown"
+	initializingReason     = "initializing"
+	podPendingReason       = "podPending"
+	podReadyReason         = "podReady"
+	podCompletedReason     = "podCompleted"
+	noVolumeSnapshotReason = "vmSnapshotNoVolumes"
+	notAllPVCsCreated      = "notAllPVCsCreated"
+	allPVCsReady           = "AllPVCsReady"
+	notAllPVCsReady        = "NotAllPVCsReady"
 
 	exportServiceLabel = "kubevirt.io.virt-export-service"
 
@@ -952,11 +956,17 @@ func (ctrl *VMExportController) updateVMExportPvcStatus(vmExport *exportv1.Virtu
 		}
 	}
 
-	if len(pvcs) == 0 {
-		log.Log.V(3).Info("PVC(s) not found, updating status to not found")
-		updateCondition(vmExportCopy.Status.Conditions, newPvcCondition(corev1.ConditionFalse, pvcNotFoundReason), true)
-	} else {
-		updateCondition(vmExportCopy.Status.Conditions, ctrl.pvcConditionFromPVC(pvcs), true)
+	if ctrl.isSourcePvc(&vmExport.Spec) {
+		if len(pvcs) == 0 {
+			log.Log.V(3).Info("PVC(s) not found, updating status to not found")
+			updateCondition(vmExportCopy.Status.Conditions, newPvcCondition(corev1.ConditionFalse, pvcNotFoundReason), true)
+		} else {
+			updateCondition(vmExportCopy.Status.Conditions, ctrl.pvcConditionFromPVC(pvcs), true)
+		}
+	} else if ctrl.isSourceVMSnapshot(&vmExport.Spec) {
+		if err := ctrl.updateVMSnapshotExportStatus(vmExportCopy, pvcs); err != nil {
+			return 0, err
+		}
 	}
 
 	vmExportCopy.Status.ServiceName = service.Name
@@ -978,6 +988,40 @@ func (ctrl *VMExportController) updateVMExportPvcStatus(vmExport *exportv1.Virtu
 	return requeue, nil
 }
 
+func (ctrl *VMExportController) updateVMSnapshotExportStatus(vmExportCopy *exportv1.VirtualMachineExport, pvcs []*corev1.PersistentVolumeClaim) error {
+	vmSnapshot, exists, err := ctrl.getVmSnapshot(vmExportCopy.Namespace, vmExportCopy.Spec.Source.Name)
+	if err != nil {
+		return err
+	} else if exists {
+		if vmSnapshot.Status.VirtualMachineSnapshotContentName != nil && *vmSnapshot.Status.VirtualMachineSnapshotContentName != "" {
+			content, exists, err := ctrl.getVmSnapshotContent(vmSnapshot.Namespace, *vmSnapshot.Status.VirtualMachineSnapshotContentName)
+			if err != nil {
+				return err
+			} else if exists {
+				if len(content.Status.VolumeSnapshotStatus) == 0 {
+					vmExportCopy.Status.Conditions = updateCondition(vmExportCopy.Status.Conditions, newVolumesCreatedCondition(corev1.ConditionFalse, noVolumeSnapshotReason), true)
+					vmExportCopy.Status.Phase = exportv1.Skipped
+				} else if len(content.Status.VolumeSnapshotStatus) != len(pvcs) {
+					vmExportCopy.Status.Conditions = updateCondition(vmExportCopy.Status.Conditions, newVolumesCreatedCondition(corev1.ConditionFalse, notAllPVCsCreated), true)
+				} else {
+					readyCount := 0
+					for _, pvc := range pvcs {
+						if pvc.Status.Phase == corev1.ClaimBound {
+							readyCount++
+						}
+					}
+					if readyCount == len(pvcs) {
+						vmExportCopy.Status.Conditions = updateCondition(vmExportCopy.Status.Conditions, newVolumesCreatedCondition(corev1.ConditionTrue, allPVCsReady), true)
+					} else {
+						vmExportCopy.Status.Conditions = updateCondition(vmExportCopy.Status.Conditions, newVolumesCreatedCondition(corev1.ConditionFalse, notAllPVCsReady), true)
+					}
+				}
+			}
+		}
+	}
+	return nil
+}
+
 func (ctrl *VMExportController) getInteralLinks(pvcs []*corev1.PersistentVolumeClaim, exporterPod *corev1.Pod, service *corev1.Service) (*exportv1.VirtualMachineExportLink, error) {
 	internalCert, err := ctrl.internalExportCa()
 	if err != nil {
@@ -1105,6 +1149,15 @@ func newPvcCondition(status corev1.ConditionStatus, reason string) exportv1.Cond
 	}
 }
 
+func newVolumesCreatedCondition(status corev1.ConditionStatus, reason string) exportv1.Condition {
+	return exportv1.Condition{
+		Type:               exportv1.ConditionVolumesCreated,
+		Status:             status,
+		Reason:             reason,
+		LastTransitionTime: *currentTime(),
+	}
+}
+
 func updateCondition(conditions []exportv1.Condition, c exportv1.Condition, includeReason bool) []exportv1.Condition {
 	found := false
 	for i := range conditions {

diff --git a/pkg/virt-controller/watch/export/export_test.go b/pkg/virt-controller/watch/export/export_test.go
@@ -946,6 +946,39 @@ var _ = Describe("Export controlleer", func() {
 			}
 		}
 
+		createTestVMSnapshotContentNoVolumes := func(name string) *snapshotv1.VirtualMachineSnapshotContent {
+			return &snapshotv1.VirtualMachineSnapshotContent{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      name,
+					Namespace: testNamespace,
+				},
+				Spec: snapshotv1.VirtualMachineSnapshotContentSpec{
+					VolumeBackups: []snapshotv1.VolumeBackup{},
+					Source: snapshotv1.SourceSpec{
+						VirtualMachine: &snapshotv1.VirtualMachine{
+							Spec: virtv1.VirtualMachineSpec{
+								Template: &virtv1.VirtualMachineInstanceTemplateSpec{
+									Spec: virtv1.VirtualMachineInstanceSpec{
+										Volumes: []virtv1.Volume{
+											{
+												Name: "test-volume",
+												VolumeSource: virtv1.VolumeSource{
+													DataVolume: &virtv1.DataVolumeSource{},
+												},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+				Status: &snapshotv1.VirtualMachineSnapshotContentStatus{
+					VolumeSnapshotStatus: []snapshotv1.VolumeSnapshotStatus{},
+				},
+			}
+		}
+
 		createTestVolumeSnapshot := func(name string) *vsv1.VolumeSnapshot {
 			size := resource.MustParse("1Gi")
 			return &vsv1.VolumeSnapshot{
@@ -978,6 +1011,9 @@ var _ = Describe("Export controlleer", func() {
 						Name:     testVolumesnapshotName,
 					},
 				},
+				Status: k8sv1.PersistentVolumeClaimStatus{
+					Phase: k8sv1.ClaimBound,
+				},
 			}
 		}
 
@@ -1006,6 +1042,40 @@ var _ = Describe("Export controlleer", func() {
 			Expect(service.Name).To(Equal(fmt.Sprintf("%s-%s", exportPrefix, testVMExport.Name)))
 		})
 
+		It("Should properly update VMExport status with a valid token with VMSnapshot without volumes", func() {
+			testVMExport := createSnapshotVMExport()
+			vmExportClient.Fake.PrependReactor("update", "virtualmachineexports", func(action testing.Action) (handled bool, obj runtime.Object, err error) {
+				update, ok := action.(testing.UpdateAction)
+				Expect(ok).To(BeTrue())
+				vmExport, ok := update.GetObject().(*exportv1.VirtualMachineExport)
+				Expect(ok).To(BeTrue())
+				verifyLinksEmpty(vmExport)
+				volumeCreateConditionSet := false
+				for _, condition := range vmExport.Status.Conditions {
+					if condition.Type == exportv1.ConditionReady {
+						Expect(condition.Status).To(Equal(k8sv1.ConditionFalse))
+						Expect(condition.Reason).To(Equal(initializingReason))
+					}
+					if condition.Type == exportv1.ConditionVolumesCreated {
+						volumeCreateConditionSet = true
+						Expect(condition.Status).To(Equal(k8sv1.ConditionFalse))
+						Expect(condition.Reason).To(Equal(noVolumeSnapshotReason))
+					}
+				}
+				Expect(volumeCreateConditionSet).To(BeTrue())
+				Expect(vmExport.Status.Phase).To(Equal(exportv1.Skipped))
+				return true, vmExport, nil
+			})
+			vmSnapshotInformer.GetStore().Add(createTestVMSnapshot(true))
+			vmSnapshotContentInformer.GetStore().Add(createTestVMSnapshotContentNoVolumes("snapshot-content"))
+			retry, err := controller.updateVMExport(testVMExport)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(retry).To(BeEquivalentTo(0))
+			service, err := k8sClient.CoreV1().Services(testNamespace).Get(context.Background(), fmt.Sprintf("%s-%s", exportPrefix, testVMExport.Name), metav1.GetOptions{})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(service.Name).To(Equal(fmt.Sprintf("%s-%s", exportPrefix, testVMExport.Name)))
+		})
+
 		It("Should create restored PVCs from VMSnapshot", func() {
 			testVMExport := createSnapshotVMExport()
 			vmExportClient.Fake.PrependReactor("update", "virtualmachineexports", func(action testing.Action) (handled bool, obj runtime.Object, err error) {
@@ -1014,12 +1084,20 @@ var _ = Describe("Export controlleer", func() {
 				vmExport, ok := update.GetObject().(*exportv1.VirtualMachineExport)
 				Expect(ok).To(BeTrue())
 				verifyLinksEmpty(vmExport)
+				volumeCreateConditionSet := false
 				for _, condition := range vmExport.Status.Conditions {
 					if condition.Type == exportv1.ConditionReady {
 						Expect(condition.Status).To(Equal(k8sv1.ConditionFalse))
 						Expect(condition.Reason).To(Equal(podPendingReason))
 					}
+					if condition.Type == exportv1.ConditionVolumesCreated {
+						volumeCreateConditionSet = true
+						Expect(condition.Status).To(Equal(k8sv1.ConditionFalse))
+						Expect(condition.Reason).To(Equal(notAllPVCsReady))
+					}
 				}
+				Expect(volumeCreateConditionSet).To(BeTrue())
+				Expect(vmExport.Status.Phase).To(Equal(exportv1.Pending))
 				return true, vmExport, nil
 			})
 
@@ -1067,12 +1145,20 @@ var _ = Describe("Export controlleer", func() {
 				vmExport, ok := update.GetObject().(*exportv1.VirtualMachineExport)
 				Expect(ok).To(BeTrue())
 				verifyLinksEmpty(vmExport)
+				volumeCreateConditionSet := false
 				for _, condition := range vmExport.Status.Conditions {
 					if condition.Type == exportv1.ConditionReady {
 						Expect(condition.Status).To(Equal(k8sv1.ConditionFalse))
 						Expect(condition.Reason).To(Equal(podPendingReason))
 					}
+					if condition.Type == exportv1.ConditionVolumesCreated {
+						volumeCreateConditionSet = true
+						Expect(condition.Status).To(Equal(k8sv1.ConditionTrue))
+						Expect(condition.Reason).To(Equal(allPVCsReady))
+					}
 				}
+				Expect(volumeCreateConditionSet).To(BeTrue())
+				Expect(vmExport.Status.Phase).To(Equal(exportv1.Pending))
 				return true, vmExport, nil
 			})
 
@@ -1204,6 +1290,20 @@ var _ = Describe("Export controlleer", func() {
 				vmExport, ok := update.GetObject().(*exportv1.VirtualMachineExport)
 				Expect(ok).To(BeTrue())
 				verifyLinksEmpty(vmExport)
+				volumeCreateConditionSet := false
+				for _, condition := range vmExport.Status.Conditions {
+					if condition.Type == exportv1.ConditionReady {
+						Expect(condition.Status).To(Equal(k8sv1.ConditionFalse))
+						Expect(condition.Reason).To(Equal(initializingReason))
+					}
+					if condition.Type == exportv1.ConditionVolumesCreated {
+						volumeCreateConditionSet = true
+						Expect(condition.Status).To(Equal(k8sv1.ConditionFalse))
+						Expect(condition.Reason).To(Equal(notAllPVCsCreated))
+					}
+				}
+				Expect(volumeCreateConditionSet).To(BeTrue())
+				Expect(vmExport.Status.Phase).To(Equal(exportv1.Pending))
 				return true, vmExport, nil
 			})
 

diff --git a/staging/src/kubevirt.io/api/export/v1alpha1/types.go b/staging/src/kubevirt.io/api/export/v1alpha1/types.go
@@ -68,6 +68,8 @@ const (
 	Ready VirtualMachineExportPhase = "Ready"
 	// Terminated means the Virtual Machine export is terminated and no longer available
 	Terminated VirtualMachineExportPhase = "Terminated"
+	// Skipped means the export is invalid in a way so the exporter pod cannot start, and we are skipping creating the exporter server pod.
+	Skipped VirtualMachineExportPhase = "Skipped"
 )
 
 // VirtualMachineExportStatus is the status for a VirtualMachineExport resource
@@ -147,6 +149,8 @@ const (
 	ConditionReady ConditionType = "Ready"
 	// ConditionPVC is the condition of the PVC we are exporting
 	ConditionPVC ConditionType = "PVCReady"
+	// ConditionVolumesCreated is the condition to see if volumes are created from volume snapshots
+	ConditionVolumesCreated ConditionType = "VolumesCreated"
 )
 
 // Condition defines conditions

diff --git a/tests/storage/BUILD.bazel b/tests/storage/BUILD.bazel
@@ -55,6 +55,7 @@ go_library(
         "//vendor/github.com/onsi/ginkgo/v2:go_default_library",
         "//vendor/github.com/onsi/gomega:go_default_library",
         "//vendor/github.com/onsi/gomega/format:go_default_library",
+        "//vendor/github.com/onsi/gomega/types:go_default_library",
         "//vendor/github.com/openshift/api/route/v1:go_default_library",
         "//vendor/github.com/pborman/uuid:go_default_library",
         "//vendor/k8s.io/api/admissionregistration/v1:go_default_library",