From 1df31f9c8401d3809c99afed07928d9c0f39b1c1 Mon Sep 17 00:00:00 2001 From: Michael Henriksen Date: Tue, 15 Oct 2019 21:48:21 -0400 Subject: [PATCH] virtctl imageupload should create datavolumes instead of PVCs Signed-off-by: Michael Henriksen --- .gitignore | 1 + BUILD.bazel | 4 + go.mod | 2 +- ...di-v1.10.1.yaml.in => cdi-v1.10.9.yaml.in} | 0 pkg/virtctl/imageupload/BUILD.bazel | 1 + pkg/virtctl/imageupload/imageupload.go | 101 +++++------ pkg/virtctl/imageupload/imageupload_test.go | 170 ++++++++++-------- tests/imageupload_test.go | 4 + 8 files changed, 150 insertions(+), 133 deletions(-) rename manifests/testing/{cdi-v1.10.1.yaml.in => cdi-v1.10.9.yaml.in} (100%) diff --git a/.gitignore b/.gitignore index 673f3961a0ec..c0fcc15bfaeb 100644 --- a/.gitignore +++ b/.gitignore @@ -37,3 +37,4 @@ manifests/**/*.tmp /bazel-* /.bazelrc _ci-configs/ +.history diff --git a/BUILD.bazel b/BUILD.bazel index 3202cc60705a..cbaef934d163 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -33,6 +33,10 @@ load("@com_github_atlassian_bazel_tools//goimports:def.bzl", "goimports") goimports( name = "goimports", display_diffs = True, + exclude_paths = [ + "./vendor/*", + "./.history/*", + ], local = ["kubevirt.io"], prefix = "kubevirt.io/kubevirt", write = True, diff --git a/go.mod b/go.mod index 1e0f61e527dd..0632e43415b0 100644 --- a/go.mod +++ b/go.mod @@ -74,7 +74,7 @@ require ( k8s.io/kube-aggregator v0.0.0-20190228175259-3e0149950b0e k8s.io/utils v0.0.0-20190607212802-c55fbcfc754a kubevirt.io/client-go v0.0.0-00010101000000-000000000000 - kubevirt.io/containerized-data-importer v1.10.6 + kubevirt.io/containerized-data-importer v1.10.9 kubevirt.io/qe-tools v0.1.3-0.20190512140058-934db0579e0c sigs.k8s.io/controller-runtime v0.1.9 // indirect ) diff --git a/manifests/testing/cdi-v1.10.1.yaml.in b/manifests/testing/cdi-v1.10.9.yaml.in similarity index 100% rename from manifests/testing/cdi-v1.10.1.yaml.in rename to manifests/testing/cdi-v1.10.9.yaml.in diff --git a/pkg/virtctl/imageupload/BUILD.bazel b/pkg/virtctl/imageupload/BUILD.bazel index 76c2a728921b..a479c3ffc412 100644 --- a/pkg/virtctl/imageupload/BUILD.bazel +++ b/pkg/virtctl/imageupload/BUILD.bazel @@ -17,6 +17,7 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//vendor/k8s.io/client-go/kubernetes:go_default_library", "//vendor/k8s.io/client-go/tools/clientcmd:go_default_library", + "//vendor/kubevirt.io/containerized-data-importer/pkg/apis/core/v1alpha1:go_default_library", "//vendor/kubevirt.io/containerized-data-importer/pkg/apis/upload/v1alpha1:go_default_library", "//vendor/kubevirt.io/containerized-data-importer/pkg/client/clientset/versioned:go_default_library", ], diff --git a/pkg/virtctl/imageupload/imageupload.go b/pkg/virtctl/imageupload/imageupload.go index 8c4f60b060b8..7f62bfba82db 100644 --- a/pkg/virtctl/imageupload/imageupload.go +++ b/pkg/virtctl/imageupload/imageupload.go @@ -26,6 +26,7 @@ import ( "net/url" "os" "path" + "strconv" "strings" "time" @@ -40,6 +41,7 @@ import ( "k8s.io/client-go/tools/clientcmd" "kubevirt.io/client-go/kubecli" + cdiv1 "kubevirt.io/containerized-data-importer/pkg/apis/core/v1alpha1" uploadcdiv1 "kubevirt.io/containerized-data-importer/pkg/apis/upload/v1alpha1" cdiClientset "kubevirt.io/containerized-data-importer/pkg/client/clientset/versioned" "kubevirt.io/kubevirt/pkg/virtctl/templates" @@ -49,9 +51,12 @@ const ( // PodPhaseAnnotation is the annotation on a PVC containing the upload pod phase PodPhaseAnnotation = "cdi.kubevirt.io/storage.pod.phase" + // PodReadyAnnotation tells whether the uploadserver pod is ready + PodReadyAnnotation = "cdi.kubevirt.io/storage.pod.ready" + uploadRequestAnnotation = "cdi.kubevirt.io/storage.upload.target" - uploadPodWaitInterval = 2 * time.Second + uploadReadyWaitInterval = 2 * time.Second //UploadProxyURI is a URI of the upoad proxy UploadProxyURI = "/v1alpha1/upload" @@ -107,7 +112,7 @@ func NewImageUploadCommand(clientConfig clientcmd.ClientConfig) *cobra.Command { } cmd.Flags().BoolVar(&insecure, "insecure", insecure, "Allow insecure server connections when using HTTPS.") cmd.Flags().StringVar(&uploadProxyURL, "uploadproxy-url", "", "The URL of the cdi-upload proxy service.") - cmd.Flags().StringVar(&pvcName, "pvc-name", "", "The destination PVC.") + cmd.Flags().StringVar(&pvcName, "pvc-name", "", "The destination DataVolume/PVC name.") cmd.MarkFlagRequired("pvc-name") cmd.Flags().StringVar(&pvcSize, "pvc-size", "", "The size of the PVC to create (ex. 10Gi, 500Mi).") cmd.Flags().StringVar(&storageClass, "storage-class", "", "The storage class for the PVC.") @@ -115,7 +120,7 @@ func NewImageUploadCommand(clientConfig clientcmd.ClientConfig) *cobra.Command { cmd.Flags().BoolVar(&blockVolume, "block-volume", blockVolume, "Create a PVC with VolumeMode=Block (default Filesystem).") cmd.Flags().StringVar(&imagePath, "image-path", "", "Path to the local VM image.") cmd.MarkFlagRequired("image-path") - cmd.Flags().BoolVar(&noCreate, "no-create", noCreate, "Don't attempt to create a new PVC.") + cmd.Flags().BoolVar(&noCreate, "no-create", noCreate, "Don't attempt to create a new DataVolume/PVC.") cmd.Flags().UintVar(&uploadPodWaitSecs, "wait-secs", uploadPodWaitSecs, "Seconds to wait for upload pod to start.") cmd.SetUsageTemplate(templates.UsageTemplate()) return cmd @@ -153,16 +158,17 @@ func (c *command) run(cmd *cobra.Command, args []string) error { if !(k8serrors.IsNotFound(err) && !noCreate) { return err } + if !noCreate && len(pvcSize) == 0 { return fmt.Errorf("When creating PVC, the size must be specified") } - pvc, err = createUploadPVC(virtClient, namespace, pvcName, pvcSize, storageClass, accessMode, blockVolume) + dv, err := createUploadDataVolume(virtClient, namespace, pvcName, pvcSize, storageClass, accessMode, blockVolume) if err != nil { return err } - fmt.Printf("PVC %s/%s created\n", namespace, pvc.Name) + fmt.Printf("DataVolume %s/%s created\n", dv.Namespace, dv.Name) } else { pvc, err = ensurePVCSupportsUpload(virtClient, pvc) if err != nil { @@ -172,15 +178,11 @@ func (c *command) run(cmd *cobra.Command, args []string) error { fmt.Printf("Using existing PVC %s/%s\n", namespace, pvc.Name) } - err = waitUploadPodRunning(virtClient, namespace, pvcName, uploadPodWaitInterval, time.Duration(uploadPodWaitSecs)*time.Second) + err = waitUploadServerReady(virtClient, namespace, pvcName, uploadReadyWaitInterval, time.Duration(uploadPodWaitSecs)*time.Second) if err != nil { return err } - token, err := getUploadToken(virtClient.CdiClient(), namespace, pvcName) - if err != nil { - return err - } if uploadProxyURL == "" { uploadProxyURL, err = getUploadProxyURL(virtClient.CdiClient()) if err != nil { @@ -190,14 +192,23 @@ func (c *command) run(cmd *cobra.Command, args []string) error { return fmt.Errorf("Upload Proxy URL not found") } } + u, err := url.Parse(uploadProxyURL) if err != nil { return err - } else if u.Scheme == "" { + } + + if u.Scheme == "" { uploadProxyURL = fmt.Sprintf("https://%s", uploadProxyURL) } + fmt.Printf("Uploading data to %s\n", uploadProxyURL) + token, err := getUploadToken(virtClient.CdiClient(), namespace, pvcName) + if err != nil { + return err + } + err = uploadData(uploadProxyURL, token, file, insecure) if err != nil { return err @@ -236,7 +247,6 @@ func ConstructUploadProxyPath(uploadProxyURL string) (string, error) { } func uploadData(uploadProxyURL, token string, file *os.File, insecure bool) error { - url, err := ConstructUploadProxyPath(uploadProxyURL) if err != nil { return err @@ -294,52 +304,31 @@ func getUploadToken(client cdiClientset.Interface, namespace, name string) (stri return response.Status.Token, nil } -func waitUploadPodRunning(client kubernetes.Interface, namespace, name string, interval, timeout time.Duration) error { - serviceName := "cdi-upload-" + name +func waitUploadServerReady(client kubernetes.Interface, namespace, name string, interval, timeout time.Duration) error { loggedStatus := false err := wait.PollImmediate(interval, timeout, func() (bool, error) { pvc, err := client.CoreV1().PersistentVolumeClaims(namespace).Get(name, metav1.GetOptions{}) if err != nil { - return false, err - } - - endpoints, err := client.CoreV1().Endpoints(namespace).Get(serviceName, metav1.GetOptions{}) - if err != nil { + // DataVolume controller may not have created the PVC yet if k8serrors.IsNotFound(err) { return false, nil } - return false, err - } - podPhase, _ := pvc.Annotations[PodPhaseAnnotation] - - done := false - availableEndpoint := false - for _, subset := range endpoints.Subsets { - if len(subset.Addresses) > 0 { - // we're looking to make sure the service endpoint has - // the upload pod marked as being available, which means - // that it is ready to accept connections - availableEndpoint = true - break - } + return false, err } - running := (podPhase == string(v1.PodRunning)) - if running && availableEndpoint { - done = true - } + // upload controler sets this to true when uploadserver pod is ready to receive data + podReady, _ := pvc.Annotations[PodReadyAnnotation] + done, _ := strconv.ParseBool(podReady) if !done && !loggedStatus { - fmt.Printf("Waiting for PVC %s upload pod to be running...\n", name) + fmt.Printf("Waiting for PVC %s upload pod to be ready...\n", name) loggedStatus = true } if done && loggedStatus { - // be really sure - time.Sleep(interval) - fmt.Printf("Pod now running\n") + fmt.Printf("Pod now ready\n") } return done, nil @@ -348,48 +337,50 @@ func waitUploadPodRunning(client kubernetes.Interface, namespace, name string, i return err } -func createUploadPVC(client kubernetes.Interface, namespace, name, size, storageClass, accessMode string, blockVolume bool) (*v1.PersistentVolumeClaim, error) { +func createUploadDataVolume(client kubecli.KubevirtClient, namespace, name, size, storageClass, accessMode string, blockVolume bool) (*cdiv1.DataVolume, error) { quantity, err := resource.ParseQuantity(size) if err != nil { return nil, fmt.Errorf("Validation failed for size=%s: %s", size, err) } - pvc := &v1.PersistentVolumeClaim{ + dv := &cdiv1.DataVolume{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, - Annotations: map[string]string{ - uploadRequestAnnotation: "", - }, }, - Spec: v1.PersistentVolumeClaimSpec{ - Resources: v1.ResourceRequirements{ - Requests: v1.ResourceList{ - v1.ResourceStorage: quantity, + Spec: cdiv1.DataVolumeSpec{ + Source: cdiv1.DataVolumeSource{ + Upload: &cdiv1.DataVolumeSourceUpload{}, + }, + PVC: &v1.PersistentVolumeClaimSpec{ + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceStorage: quantity, + }, }, }, }, } if storageClass != "" { - pvc.Spec.StorageClassName = &storageClass + dv.Spec.PVC.StorageClassName = &storageClass } if accessMode != "" { - pvc.Spec.AccessModes = []v1.PersistentVolumeAccessMode{v1.PersistentVolumeAccessMode(accessMode)} + dv.Spec.PVC.AccessModes = []v1.PersistentVolumeAccessMode{v1.PersistentVolumeAccessMode(accessMode)} } if blockVolume { volMode := v1.PersistentVolumeBlock - pvc.Spec.VolumeMode = &volMode + dv.Spec.PVC.VolumeMode = &volMode } - pvc, err = client.CoreV1().PersistentVolumeClaims(namespace).Create(pvc) + dv, err = client.CdiClient().CdiV1alpha1().DataVolumes(namespace).Create(dv) if err != nil { return nil, err } - return pvc, nil + return dv, nil } func ensurePVCSupportsUpload(client kubernetes.Interface, pvc *v1.PersistentVolumeClaim) (*v1.PersistentVolumeClaim, error) { diff --git a/pkg/virtctl/imageupload/imageupload_test.go b/pkg/virtctl/imageupload/imageupload_test.go index da46bf2027ab..2c1e89906b36 100644 --- a/pkg/virtctl/imageupload/imageupload_test.go +++ b/pkg/virtctl/imageupload/imageupload_test.go @@ -31,6 +31,7 @@ const ( commandName = "image-upload" uploadRequestAnnotation = "cdi.kubevirt.io/storage.upload.target" podPhaseAnnotation = "cdi.kubevirt.io/storage.pod.phase" + podReadyAnnotation = "cdi.kubevirt.io/storage.pod.ready" ) const ( @@ -70,12 +71,54 @@ var _ = Describe("ImageUpload", func() { os.Remove(imagePath) }) + pvcSpec := func() *v1.PersistentVolumeClaim { + quantity, _ := resource.ParseQuantity(pvcSize) + + pvc := &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvcName, + Namespace: "default", + Annotations: map[string]string{}, + }, + Spec: v1.PersistentVolumeClaimSpec{ + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceStorage: quantity, + }, + }, + }, + } + + return pvc + } + + pvcSpecWithUploadAnnotation := func() *v1.PersistentVolumeClaim { + spec := pvcSpec() + spec.Annotations = map[string]string{ + uploadRequestAnnotation: "", + podPhaseAnnotation: "Running", + podReadyAnnotation: "true", + } + return spec + } + + pvcSpecWithUploadSucceeded := func() *v1.PersistentVolumeClaim { + spec := pvcSpec() + spec.Annotations = map[string]string{ + uploadRequestAnnotation: "", + podPhaseAnnotation: "Succeeded", + podReadyAnnotation: "false", + } + return spec + } + addPodPhaseAnnotation := func() { defer GinkgoRecover() time.Sleep(10 * time.Millisecond) pvc, err := kubeClient.CoreV1().PersistentVolumeClaims(pvcNamespace).Get(pvcName, metav1.GetOptions{}) Expect(err).To(BeNil()) pvc.Annotations[podPhaseAnnotation] = "Running" + pvc.Annotations[podReadyAnnotation] = "true" pvc, err = kubeClient.CoreV1().PersistentVolumeClaims(pvcNamespace).Update(pvc) if err != nil { fmt.Fprintf(GinkgoWriter, "Error: %v\n", err) @@ -83,19 +126,30 @@ var _ = Describe("ImageUpload", func() { Expect(err).To(BeNil()) } + createPVC := func(dv *cdiv1.DataVolume) { + defer GinkgoRecover() + time.Sleep(10 * time.Millisecond) + pvc := pvcSpecWithUploadAnnotation() + pvc.Spec.VolumeMode = dv.Spec.PVC.VolumeMode + pvc.Spec.AccessModes = append([]v1.PersistentVolumeAccessMode(nil), dv.Spec.PVC.AccessModes...) + pvc.Spec.StorageClassName = dv.Spec.PVC.StorageClassName + pvc, err := kubeClient.CoreV1().PersistentVolumeClaims(pvcNamespace).Create(pvc) + Expect(err).To(BeNil()) + } + addReactors := func() { - kubeClient.Fake.PrependReactor("create", "persistentvolumeclaims", func(action testing.Action) (bool, runtime.Object, error) { + cdiClient.Fake.PrependReactor("create", "datavolumes", func(action testing.Action) (bool, runtime.Object, error) { create, ok := action.(testing.CreateAction) Expect(ok).To(BeTrue()) - pvc, ok := create.GetObject().(*v1.PersistentVolumeClaim) + dv, ok := create.GetObject().(*cdiv1.DataVolume) Expect(ok).To(BeTrue()) - Expect(pvc.Name).To(Equal(pvcName)) + Expect(dv.Name).To(Equal(pvcName)) Expect(createCalled).To(BeFalse()) createCalled = true - go addPodPhaseAnnotation() + go createPVC(dv) return false, nil, nil }) @@ -118,51 +172,50 @@ var _ = Describe("ImageUpload", func() { }) } - validateModePVC := func(blockMode bool) { - pvc, err := kubeClient.CoreV1().PersistentVolumeClaims(pvcNamespace).Get(pvcName, metav1.GetOptions{}) - Expect(err).To(BeNil()) - - resource, ok := pvc.Spec.Resources.Requests[v1.ResourceStorage] + validatePVCSpec := func(spec *v1.PersistentVolumeClaimSpec, mode v1.PersistentVolumeMode) { + resource, ok := spec.Resources.Requests[v1.ResourceStorage] Expect(ok).To(BeTrue()) Expect(resource.String()).To(Equal(pvcSize)) - _, ok = pvc.Annotations[uploadRequestAnnotation] + volumeMode := spec.VolumeMode + if volumeMode == nil { + vm := v1.PersistentVolumeFilesystem + volumeMode = &vm + } + Expect(mode).To(Equal(*volumeMode)) + } + + validatePVCArgs := func(mode v1.PersistentVolumeMode) { + pvc, err := kubeClient.CoreV1().PersistentVolumeClaims(pvcNamespace).Get(pvcName, metav1.GetOptions{}) + Expect(err).To(BeNil()) + + _, ok := pvc.Annotations[uploadRequestAnnotation] Expect(ok).To(BeTrue()) - volumeMode := v1.PersistentVolumeFilesystem - if blockMode { - volumeMode = v1.PersistentVolumeBlock - } - // pvc.Spec.VolumeMode is not always set, ignore when Filesystem is expected - if pvc.Spec.VolumeMode != nil || blockMode { - Expect(pvc.Spec.VolumeMode).To(Equal(&volumeMode)) - } + validatePVCSpec(&pvc.Spec, mode) } validatePVC := func() { - validateModePVC(false) + validatePVCArgs(v1.PersistentVolumeFilesystem) } validateBlockPVC := func() { - validateModePVC(true) + validatePVCArgs(v1.PersistentVolumeBlock) } - createEndpoints := func() *v1.Endpoints { - return &v1.Endpoints{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cdi-upload-" + pvcName, - Namespace: pvcNamespace, - }, - Subsets: []v1.EndpointSubset{ - { - Addresses: []v1.EndpointAddress{ - { - IP: "10.10.10.10", - }, - }, - }, - }, - } + validateDataVolumeArgs := func(mode v1.PersistentVolumeMode) { + dv, err := cdiClient.CdiV1alpha1().DataVolumes(pvcNamespace).Get(pvcName, metav1.GetOptions{}) + Expect(err).To(BeNil()) + + validatePVCSpec(dv.Spec.PVC, mode) + } + + validateDataVolume := func() { + validateDataVolumeArgs(v1.PersistentVolumeFilesystem) + } + + validateBlockDataVolume := func() { + validateDataVolumeArgs(v1.PersistentVolumeBlock) } createCDIConfig := func() *cdiv1.CDIConfig { @@ -191,10 +244,9 @@ var _ = Describe("ImageUpload", func() { createCalled = false updateCalled = false - objs := append([]runtime.Object{createEndpoints()}, kubeobjects...) config := createCDIConfig() - kubeClient = fakek8sclient.NewSimpleClientset(objs...) + kubeClient = fakek8sclient.NewSimpleClientset(kubeobjects...) cdiClient = fakecdiclient.NewSimpleClientset(config) kubecli.MockKubevirtClientInstance.EXPECT().CoreV1().Return(kubeClient.CoreV1()).AnyTimes() @@ -218,45 +270,6 @@ var _ = Describe("ImageUpload", func() { server.Close() } - pvcSpec := func() *v1.PersistentVolumeClaim { - quantity, _ := resource.ParseQuantity(pvcSize) - - pvc := &v1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: pvcName, - Namespace: "default", - Annotations: map[string]string{}, - }, - Spec: v1.PersistentVolumeClaimSpec{ - Resources: v1.ResourceRequirements{ - Requests: v1.ResourceList{ - v1.ResourceStorage: quantity, - }, - }, - }, - } - - return pvc - } - - pvcSpecWithUploadAnnotation := func() *v1.PersistentVolumeClaim { - spec := pvcSpec() - spec.Annotations = map[string]string{ - uploadRequestAnnotation: "", - podPhaseAnnotation: "Running", - } - return spec - } - - pvcSpecWithUploadSucceeded := func() *v1.PersistentVolumeClaim { - spec := pvcSpec() - spec.Annotations = map[string]string{ - uploadRequestAnnotation: "", - podPhaseAnnotation: "Succeeded", - } - return spec - } - Context("Successful upload to PVC", func() { It("PVC does not exist", func() { testInit(http.StatusOK) @@ -265,6 +278,7 @@ var _ = Describe("ImageUpload", func() { Expect(cmd()).To(BeNil()) Expect(createCalled).To(BeTrue()) validatePVC() + validateDataVolume() }) It("Use CDI Config UploadProxyURL", func() { @@ -274,6 +288,7 @@ var _ = Describe("ImageUpload", func() { Expect(cmd()).To(BeNil()) Expect(createCalled).To(BeTrue()) validatePVC() + validateDataVolume() }) It("Create a VolumeMode=Block PVC", func() { @@ -283,6 +298,7 @@ var _ = Describe("ImageUpload", func() { Expect(cmd()).To(BeNil()) Expect(createCalled).To(BeTrue()) validateBlockPVC() + validateBlockDataVolume() }) DescribeTable("PVC does exist", func(pvc *v1.PersistentVolumeClaim) { diff --git a/tests/imageupload_test.go b/tests/imageupload_test.go index 72ab5c75674c..c52bbf833f8d 100644 --- a/tests/imageupload_test.go +++ b/tests/imageupload_test.go @@ -92,6 +92,10 @@ var _ = Describe("ImageUpload", func() { Expect(err).ToNot(HaveOccurred()) } + By("Get DataVolume") + _, err = virtClient.CdiClient().CdiV1alpha1().DataVolumes(namespace).Get(pvcName, metav1.GetOptions{}) + Expect(err).ToNot(HaveOccurred()) + By("Start VM") vmi := tests.NewRandomVMIWithPVC(pvcName) vmi, err = virtClient.VirtualMachineInstance(namespace).Create(vmi)