add PVC claimName to datavolume status (#2060)

* Make it possible to find the underlying PVC name using the DV

Right now a lot of things assume that the underlying PVC has the
same name/namespace, let's make it possible to reach over and not
need to have this implicit knowledge in a lot of places.

Signed-off-by: Maya Rashish <mrashish@redhat.com>

* Install some artifacts on the old version of CDI during upgrade tests

And use this to test that DataVolume.Status.ClaimName is set after
upgrades.

Signed-off-by: Maya Rashish <mrashish@redhat.com>

* Bump CDI pod update timeout

Signed-off-by: Maya Rashish <mrashish@redhat.com>

* Only check if non-testing CDI pods have updated.

We don't update the testing environment, so it looks like some of the
update fails.

Signed-off-by: Maya Rashish <mrashish@redhat.com>

* Restore lower timeouts

Signed-off-by: Maya Rashish <mrashish@redhat.com>

* As elsewhere, don't use local registry artifacts with external provider

Signed-off-by: Maya Rashish <mrashish@redhat.com>

api/openapi-spec/swagger.json

@@ -4114,6 +4114,10 @@
     "description": "DataVolumeStatus contains the current status of the DataVolume",
     "type": "object",
     "properties": {
+     "claimName": {
+      "description": "ClaimName is the name of the underlying PVC used by the DataVolume.",
+      "type": "string"
+     },
      "conditions": {
       "type": "array",
       "items": {

cluster-sync/sync.sh

@@ -106,7 +106,7 @@ function wait_cdi_pods_updated {
   ret=0
   while [[ $ret -eq 0 ]] && [[ $wait_time -lt ${CDI_PODS_UPDATE_TIMEOUT} ]]; do
     wait_time=$((wait_time + 5))
-    _kubectl get pods -n $CDI_NAMESPACE -o=jsonpath='{range .items[*]}{.metadata.name}{"\n"}{.metadata.uid}{"\n"}{.spec.containers[*].image}{"\n"}{end}' > $NEW_CDI_VER_PODS
+    _kubectl get pods -n $CDI_NAMESPACE -l '!cdi.kubevirt.io/testing' -o=jsonpath='{range .items[*]}{.metadata.name}{"\n"}{.metadata.uid}{"\n"}{.spec.containers[*].image}{"\n"}{end}' > $NEW_CDI_VER_PODS
     if [ -f $OLD_CDI_VER_PODS ] ; then
       grep -qFxf $OLD_CDI_VER_PODS $NEW_CDI_VER_PODS || ret=$?
       if [ $ret -eq 0 ] ; then
@@ -123,6 +123,29 @@ function wait_cdi_pods_updated {
   fi
 }
 
+# Setup some datavolumes in older version for testing upgrades
+# Done unconditionally to make it easier to write tests.
+function setup_for_upgrade_testing {
+  if _kubectl get namespace cdi-testing-old-version-artifacts ; then
+    echo "Old version testing environment already setup"
+    return
+  fi
+  if [ "${KUBEVIRT_PROVIDER}" == "external" ]; then
+    echo "External provider, not setting up upgrade testing"
+    return
+  fi
+  echo "Missing old version environment setup, creating"
+  _kubectl apply -f "./_out/manifests/cdi-testing-sa.yaml"
+  _kubectl apply -f "./_out/manifests/file-host.yaml"
+  _kubectl apply -f "./_out/manifests/registry-host.yaml"
+  echo "Waiting for testing tools to be ready"
+  _kubectl wait pod -n ${CDI_NAMESPACE} --for=condition=Ready --all --timeout=${CDI_AVAILABLE_TIMEOUT}s
+  _kubectl create namespace cdi-testing-old-version-artifacts
+  _kubectl apply --namespace cdi-testing-old-version-artifacts -f "./_out/manifests/upgrade-testing-artifacts.yaml"
+  echo "Waiting for old version artifacts to come up"
+  _kubectl wait dv --namespace cdi-testing-old-version-artifacts --for=condition=Ready --all --timeout=${CDI_AVAILABLE_TIMEOUT}s
+}
+
 # Start functional test HTTP server.
 # We skip the functional test additions for external provider for now, as they're specific
 if [ "${KUBEVIRT_PROVIDER}" != "external" ] && [ "${CDI_SYNC}" == "test-infra" ]; then
@@ -190,6 +213,7 @@ if [[ ! -z "$UPGRADE_FROM" ]]; then
     fi
     echo "Currently at version: $VERSION"
     wait_cdi_pods_updated
+    setup_for_upgrade_testing
   done
   echo "Upgrading to latest"
   retry_counter=0

manifests/templates/upgrade-testing-artifacts.yaml.in

@@ -0,0 +1,30 @@
+# This example assumes you are using a default storage class
+apiVersion: cdi.kubevirt.io/v1alpha1
+kind: DataVolume
+metadata:
+  name: olddv-v1alpha1
+spec:
+  source:
+      http:
+         url: "http://cdi-file-host.{{ .Namespace }}/tinyCore.iso"
+  pvc:
+    accessModes:
+      - ReadWriteOnce
+    resources:
+      requests:
+        storage: 500Mi
+---
+apiVersion: cdi.kubevirt.io/v1beta1
+kind: DataVolume
+metadata:
+  name: olddv-v1beta1
+spec:
+  source:
+      http:
+         url: "http://cdi-file-host.{{ .Namespace }}/tinyCore.iso"
+  pvc:
+    accessModes:
+      - ReadWriteOnce
+    resources:
+      requests:
+        storage: 500Mi

pkg/controller/datavolume-controller.go

@@ -1955,6 +1955,7 @@ func (r *DatavolumeReconciler) reconcileDataVolumeStatus(dataVolume *cdiv1.DataV
 		if err != nil {
 			return reconcile.Result{}, err
 		}
+		dataVolumeCopy.Status.ClaimName = pvc.Name
 
 		// the following check is for a case where the request is to create a blank disk for a block device.
 		// in that case, we do not create a pod as there is no need to create a blank image.

pkg/operator/resources/crds_generated.go

@@ -5289,6 +5289,10 @@ spec:
                     description: DataVolumeStatus contains the current status of the
                       DataVolume
                     properties:
+                      claimName:
+                        description: ClaimName is the name of the underlying PVC used
+                          by the DataVolume.
+                        type: string
                       conditions:
                         items:
                           description: DataVolumeCondition represents the state of
@@ -6049,6 +6053,10 @@ spec:
           status:
             description: DataVolumeStatus contains the current status of the DataVolume
             properties:
+              claimName:
+                description: ClaimName is the name of the underlying PVC used by the
+                  DataVolume.
+                type: string
               conditions:
                 items:
                   description: DataVolumeCondition represents the state of a data
@@ -6623,6 +6631,10 @@ spec:
           status:
             description: DataVolumeStatus contains the current status of the DataVolume
             properties:
+              claimName:
+                description: ClaimName is the name of the underlying PVC used by the
+                  DataVolume.
+                type: string
               conditions:
                 items:
                   description: DataVolumeCondition represents the state of a data

staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1alpha1/types.go

@@ -201,6 +201,8 @@ type DataVolumeSourceVDDK struct {
 
 // DataVolumeStatus contains the current status of the DataVolume
 type DataVolumeStatus struct {
+	// ClaimName is the name of the underlying PVC used by the DataVolume.
+	ClaimName string `json:"claimName,omitempty"`
 	//Phase is the current phase of the data volume
 	Phase    DataVolumePhase    `json:"phase,omitempty"`
 	Progress DataVolumeProgress `json:"progress,omitempty"`

staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/types.go

@@ -248,6 +248,8 @@ const (
 
 // DataVolumeStatus contains the current status of the DataVolume
 type DataVolumeStatus struct {
+	// ClaimName is the name of the underlying PVC used by the DataVolume.
+	ClaimName string `json:"claimName,omitempty"`
 	//Phase is the current phase of the data volume
 	Phase    DataVolumePhase    `json:"phase,omitempty"`
 	Progress DataVolumeProgress `json:"progress,omitempty"`

tests/BUILD.bazel

@@ -49,6 +49,7 @@ go_test(
         "tests_suite_test.go",
         "transfer_test.go",
         "transport_test.go",
+        "upgrade_test.go",
         "upload_test.go",
         "webhook_test.go",
     ],

tests/upgrade_test.go

@@ -0,0 +1,42 @@
+package tests_test
+
+import (
+	"context"
+	"fmt"
+
+	. "github.com/onsi/ginkgo"
+	"github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/gomega"
+
+	apierrs "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"kubevirt.io/containerized-data-importer/tests/framework"
+)
+
+/*
+ * Tests that use artifacts created on older version.
+ * Artifacts are created by manifests/templates/upgrade-testing-artifacts.yaml.in
+ */
+
+const oldVersionArtifactsNamespace = "cdi-testing-old-version-artifacts"
+
+var _ = Describe("[Upgrade]", func() {
+	f := framework.NewFramework("upgrade-test")
+
+	BeforeEach(func() {
+		_, err := f.K8sClient.CoreV1().Namespaces().Get(context.TODO(), oldVersionArtifactsNamespace, metav1.GetOptions{})
+		if apierrs.IsNotFound(err) {
+			Skip(fmt.Sprintf("Not setup to perform upgrade testing; missing namespace %s", oldVersionArtifactsNamespace))
+		}
+	})
+
+	table.DescribeTable("[rfe_id:5493]DV status.name is populated after upgrade", func(dvName string) {
+		dv, err := f.CdiClient.CdiV1beta1().DataVolumes(oldVersionArtifactsNamespace).Get(context.TODO(), dvName, metav1.GetOptions{})
+		Expect(err).ToNot(HaveOccurred())
+		Expect(dv.Status.ClaimName).To(Equal(dvName))
+	},
+		table.Entry("[test_id:7714]with v1alpha1 datavolume", "olddv-v1alpha1"),
+		table.Entry("[test_id:7715]with v1beta1 datavolume", "olddv-v1beta1"),
+	)
+})