Add error message on virtctl image-upload to WaitForFirstConsumer Dat…

…aVolume When the DV is in phase WaitForFirstConsumer the PVC cannot be bound until first consumer pod shows up, so the upload is not possible. Previously the virtclt timed out, and now it shows an error informing what happened. The check only works for upload to DataVolume. Implementing this check for upload to PVC would mean re-implementing CDI logic that updates DataVolume state. It is assumed that users wanting to utilize all CDI functionality use DataVolumes, and users that use PVC have a good reason not to use DV. Signed-off-by: Bartosz Rybacki <[email protected]>
bk201 · Jan 15, 2021 · 7001bcd · 7001bcd
1 parent 246eb6f
commit 7001bcd
Show file tree

Hide file tree

Showing 2 changed files with 99 additions and 6 deletions.
diff --git a/pkg/virtctl/imageupload/imageupload.go b/pkg/virtctl/imageupload/imageupload.go
@@ -262,11 +262,17 @@ func (c *command) run(cmd *cobra.Command, args []string) error {
 		fmt.Printf("Using existing PVC %s/%s\n", namespace, pvc.Name)
 	}
 
-	err = waitUploadServerReady(virtClient, namespace, name, uploadReadyWaitInterval, time.Duration(uploadPodWaitSecs)*time.Second)
-	if err != nil {
-		return err
+	if createPVC {
+		err = waitUploadServerReady(virtClient, namespace, name, uploadReadyWaitInterval, time.Duration(uploadPodWaitSecs)*time.Second)
+		if err != nil {
+			return err
+		}
+	} else {
+		err = waitDvUploadScheduled(virtClient, namespace, name, uploadReadyWaitInterval, time.Duration(uploadPodWaitSecs)*time.Second)
+		if err != nil {
+			return err
+		}
 	}
-
 	if uploadProxyURL == "" {
 		uploadProxyURL, err = getUploadProxyURL(virtClient.CdiClient())
 		if err != nil {
@@ -423,6 +429,40 @@ func getUploadToken(client cdiClientset.Interface, namespace, name string) (stri
 	return response.Status.Token, nil
 }
 
+func waitDvUploadScheduled(client kubecli.KubevirtClient, namespace, name string, interval, timeout time.Duration) error {
+	loggedStatus := false
+	//
+	err := wait.PollImmediate(interval, timeout, func() (bool, error) {
+		dv, err := client.CdiClient().CdiV1alpha1().DataVolumes(namespace).Get(name, metav1.GetOptions{})
+
+		if err != nil {
+			// DataVolume controller may not have created the DV yet ? TODO:
+			if k8serrors.IsNotFound(err) {
+				return false, nil
+			}
+
+			return false, err
+		}
+
+		if dv.Status.Phase == cdiv1.WaitForFirstConsumer {
+			return false, fmt.Errorf("cannot upload to DataVolume in WaitForFirstConsumer state, make sure the PVC is Bound")
+		}
+		done := dv.Status.Phase == cdiv1.UploadReady
+		if !done && !loggedStatus {
+			fmt.Printf("Waiting for PVC %s upload pod to be ready...\n", name)
+			loggedStatus = true
+		}
+
+		if done && loggedStatus {
+			fmt.Printf("Pod now ready\n")
+		}
+
+		return done, nil
+	})
+
+	return err
+}
+
 func waitUploadServerReady(client kubernetes.Interface, namespace, name string, interval, timeout time.Duration) error {
 	loggedStatus := false
 

diff --git a/pkg/virtctl/imageupload/imageupload_test.go b/pkg/virtctl/imageupload/imageupload_test.go
@@ -138,6 +138,25 @@ var _ = Describe("ImageUpload", func() {
 		return spec
 	}
 
+	dvSpecWithWaitForFirstConsumer := func() *cdiv1.DataVolume {
+		dv := &cdiv1.DataVolume{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      targetName,
+				Namespace: "default",
+			},
+			TypeMeta: metav1.TypeMeta{
+				APIVersion: cdiv1.SchemeGroupVersion.String(),
+				Kind:       "DataVolume",
+			},
+			Status: cdiv1.DataVolumeStatus{Phase: cdiv1.WaitForFirstConsumer},
+			Spec: cdiv1.DataVolumeSpec{
+				Source: cdiv1.DataVolumeSource{Upload: &cdiv1.DataVolumeSourceUpload{}},
+				PVC:    &v1.PersistentVolumeClaimSpec{},
+			},
+		}
+		return dv
+	}
+
 	addPodPhaseAnnotation := func() {
 		defer GinkgoRecover()
 		time.Sleep(10 * time.Millisecond)
@@ -152,6 +171,19 @@ var _ = Describe("ImageUpload", func() {
 		Expect(err).To(BeNil())
 	}
 
+	addDvPhase := func() {
+		defer GinkgoRecover()
+		time.Sleep(10 * time.Millisecond)
+		dv, err := cdiClient.CdiV1alpha1().DataVolumes(targetNamespace).Get(targetName, metav1.GetOptions{})
+		Expect(err).To(BeNil())
+		dv.Status.Phase = cdiv1.UploadReady
+		dv, err = cdiClient.CdiV1alpha1().DataVolumes(targetNamespace).Update(dv)
+		if err != nil {
+			fmt.Fprintf(GinkgoWriter, "Error: %v\n", err)
+		}
+		Expect(err).To(BeNil())
+	}
+
 	createPVC := func(dv *cdiv1.DataVolume) {
 		defer GinkgoRecover()
 		time.Sleep(10 * time.Millisecond)
@@ -176,6 +208,7 @@ var _ = Describe("ImageUpload", func() {
 			dvCreateCalled = true
 
 			go createPVC(dv)
+			go addDvPhase()
 
 			return false, nil, nil
 		})
@@ -296,15 +329,16 @@ var _ = Describe("ImageUpload", func() {
 		return nil
 	}
 
-	testInitAsync := func(statusCode int, async bool, kubeobjects ...runtime.Object) {
+	testInitAsyncWithCdiObjects := func(statusCode int, async bool, kubeobjects []runtime.Object, cdiobjects []runtime.Object) {
 		dvCreateCalled = false
 		pvcCreateCalled = false
 		updateCalled = false
 
 		config := createCDIConfig()
+		cdiobjects = append(cdiobjects, config)
 
 		kubeClient = fakek8sclient.NewSimpleClientset(kubeobjects...)
-		cdiClient = fakecdiclient.NewSimpleClientset(config)
+		cdiClient = fakecdiclient.NewSimpleClientset(cdiobjects...)
 
 		kubecli.MockKubevirtClientInstance.EXPECT().CoreV1().Return(kubeClient.CoreV1()).AnyTimes()
 		kubecli.MockKubevirtClientInstance.EXPECT().CdiClient().Return(cdiClient).AnyTimes()
@@ -331,6 +365,10 @@ var _ = Describe("ImageUpload", func() {
 		})
 	}
 
+	testInitAsync := func(statusCode int, async bool, kubeobjects ...runtime.Object) {
+		testInitAsyncWithCdiObjects(statusCode, async, kubeobjects, nil)
+	}
+
 	testInit := func(statusCode int, kubeobjects ...runtime.Object) {
 		testInitAsync(statusCode, true, kubeobjects...)
 	}
@@ -341,6 +379,7 @@ var _ = Describe("ImageUpload", func() {
 	}
 
 	Context("Successful upload to PVC", func() {
+
 		It("PVC does not exist deprecated args", func() {
 			testInit(http.StatusOK)
 			cmd := tests.NewRepeatableVirtctlCommand(commandName, "--pvc-name", targetName, "--pvc-size", pvcSize,
@@ -470,6 +509,20 @@ var _ = Describe("ImageUpload", func() {
 			Expect(cmd()).NotTo(BeNil())
 		})
 
+		It("DV in phase WaitForFirstConsumer", func() {
+			testInitAsyncWithCdiObjects(
+				http.StatusOK,
+				true,
+				[]runtime.Object{pvcSpecWithUploadAnnotation()},
+				[]runtime.Object{dvSpecWithWaitForFirstConsumer()},
+			)
+			cmd := tests.NewRepeatableVirtctlCommand(commandName, "dv", targetName, "--size", pvcSize,
+				"--uploadproxy-url", server.URL, "--insecure", "--image-path", imagePath)
+			err := cmd()
+			Expect(err).NotTo(BeNil())
+			Expect(err.Error()).Should(ContainSubstring("cannot upload to DataVolume in WaitForFirstConsumer state"))
+		})
+
 		It("uploadProxyURL not configured", func() {
 			testInit(http.StatusOK)
 			cmd := tests.NewRepeatableVirtctlCommand(commandName, "dv", targetName, "--size", pvcSize,