diff --git a/pkg/apiserver/webhooks/datavolume-validate.go b/pkg/apiserver/webhooks/datavolume-validate.go index 3e4d2ecf5d..6a1f1afd32 100644 --- a/pkg/apiserver/webhooks/datavolume-validate.go +++ b/pkg/apiserver/webhooks/datavolume-validate.go @@ -425,21 +425,17 @@ func (wh *dataVolumeValidatingWebhook) validateSourceRef(request *admissionv1.Ad Field: field.Child("sourceRef").String(), } } - dataSourceSnapshot := dataSource.Spec.Source.Snapshot - if dataSourceSnapshot != nil { - return &metav1.StatusCause{ - Message: "Snapshot sourceRef not allowed at this point", - Field: field.Child("sourceRef").String(), - } + switch { + case dataSource.Spec.Source.PVC != nil: + return wh.validateDataVolumeSourcePVC(dataSource.Spec.Source.PVC, field.Child("sourceRef"), spec) + case dataSource.Spec.Source.Snapshot != nil: + return wh.validateDataVolumeSourceSnapshot(dataSource.Spec.Source.Snapshot, field.Child("sourceRef"), spec) } - dataSourcePVC := dataSource.Spec.Source.PVC - if dataSourcePVC == nil { - return &metav1.StatusCause{ - Message: fmt.Sprintf("Empty PVC field in '%s'. DataSource may not be ready yet", dataSource.Name), - Field: field.Child("sourceRef").String(), - } + + return &metav1.StatusCause{ + Message: fmt.Sprintf("Empty source field in '%s'. DataSource may not be ready yet", dataSource.Name), + Field: field.Child("sourceRef").String(), } - return wh.validateDataVolumeSourcePVC(dataSourcePVC, field.Child("sourceRef"), spec) } func (wh *dataVolumeValidatingWebhook) validateDataVolumeSourcePVC(PVC *cdiv1.DataVolumeSourcePVC, field *k8sfield.Path, spec *cdiv1.DataVolumeSpec) *metav1.StatusCause { diff --git a/pkg/apiserver/webhooks/datavolume-validate_test.go b/pkg/apiserver/webhooks/datavolume-validate_test.go index 43e46bbb55..bb7e9e56cb 100644 --- a/pkg/apiserver/webhooks/datavolume-validate_test.go +++ b/pkg/apiserver/webhooks/datavolume-validate_test.go @@ -730,6 +730,26 @@ var _ = Describe("Validating Webhook", func() { Expect(resp.Allowed).To(Equal(true)) }) + It("should accept DataVolume with SourceRef on create if DataSource exists but snapshot does not exist", func() { + dataVolume := newDataSourceDataVolume("testDV", &testNamespace, "test") + dataSource := &cdiv1.DataSource{ + ObjectMeta: metav1.ObjectMeta{ + Name: dataVolume.Spec.SourceRef.Name, + Namespace: testNamespace, + }, + Spec: cdiv1.DataSourceSpec{ + Source: cdiv1.DataSourceSource{ + Snapshot: &cdiv1.DataVolumeSourceSnapshot{ + Name: "testNonExistentSnap", + Namespace: testNamespace, + }, + }, + }, + } + resp := validateDataVolumeCreateEx(dataVolume, nil, []runtime.Object{dataSource}, nil) + Expect(resp.Allowed).To(Equal(true)) + }) + It("should reject DataVolume with empty SourceRef name on create", func() { dataVolume := newDataSourceDataVolume("testDV", &testNamespace, "") resp := validateDataVolumeCreate(dataVolume) diff --git a/pkg/controller/BUILD.bazel b/pkg/controller/BUILD.bazel index 95d5bcaa44..9e70d8bbff 100644 --- a/pkg/controller/BUILD.bazel +++ b/pkg/controller/BUILD.bazel @@ -34,6 +34,7 @@ go_library( "//vendor/github.com/containers/image/v5/docker/reference:go_default_library", "//vendor/github.com/go-logr/logr:go_default_library", "//vendor/github.com/gorhill/cronexpr:go_default_library", + "//vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1:go_default_library", "//vendor/github.com/openshift/api/config/v1:go_default_library", "//vendor/github.com/openshift/api/image/v1:go_default_library", "//vendor/github.com/openshift/api/route/v1:go_default_library", @@ -99,6 +100,7 @@ go_test( "//staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1:go_default_library", "//tests/reporters:go_default_library", "//vendor/github.com/google/uuid:go_default_library", + "//vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1:go_default_library", "//vendor/github.com/onsi/ginkgo:go_default_library", "//vendor/github.com/onsi/ginkgo/extensions/table:go_default_library", "//vendor/github.com/onsi/gomega:go_default_library", diff --git a/pkg/controller/dataimportcron-controller_test.go b/pkg/controller/dataimportcron-controller_test.go index 6a3c657f6d..5db78177e0 100644 --- a/pkg/controller/dataimportcron-controller_test.go +++ b/pkg/controller/dataimportcron-controller_test.go @@ -326,7 +326,7 @@ var _ = Describe("All DataImportCron Tests", func() { err := reconciler.client.Update(context.TODO(), cron) Expect(err).ToNot(HaveOccurred()) dataSource = &cdiv1.DataSource{} - verifyConditions("After DesiredDigest is set", false, false, false, noImport, outdated, noPvc) + verifyConditions("After DesiredDigest is set", false, false, false, noImport, outdated, noSource) imports := cron.Status.CurrentImports Expect(imports).ToNot(BeNil()) @@ -344,12 +344,12 @@ var _ = Describe("All DataImportCron Tests", func() { dv.Status.Phase = cdiv1.ImportScheduled err = reconciler.client.Update(context.TODO(), dv) Expect(err).ToNot(HaveOccurred()) - verifyConditions("Import scheduled", false, false, false, scheduled, inProgress, noPvc) + verifyConditions("Import scheduled", false, false, false, scheduled, inProgress, noSource) dv.Status.Phase = cdiv1.ImportInProgress err = reconciler.client.Update(context.TODO(), dv) Expect(err).ToNot(HaveOccurred()) - verifyConditions("Import in progress", true, false, false, inProgress, inProgress, noPvc) + verifyConditions("Import in progress", true, false, false, inProgress, inProgress, noSource) dv.Status.Phase = cdiv1.Succeeded err = reconciler.client.Update(context.TODO(), dv) diff --git a/pkg/controller/datasource-controller.go b/pkg/controller/datasource-controller.go index dab635aaf5..61dba1c6f6 100644 --- a/pkg/controller/datasource-controller.go +++ b/pkg/controller/datasource-controller.go @@ -22,8 +22,10 @@ import ( "reflect" "github.com/go-logr/logr" + snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" @@ -50,8 +52,8 @@ type DataSourceReconciler struct { } const ( - ready = "Ready" - noPvc = "NoPvc" + ready = "Ready" + noSource = "NoSource" ) // Reconcile loop for DataSourceReconciler @@ -75,42 +77,71 @@ func (r *DataSourceReconciler) update(ctx context.Context, dataSource *cdiv1.Dat dataSource.Status.Conditions = nil } dataSourceCopy := dataSource.DeepCopy() - sourcePVC := dataSource.Spec.Source.PVC - if sourcePVC != nil { - dv := &cdiv1.DataVolume{} - ns := cc.GetNamespace(sourcePVC.Namespace, dataSource.Namespace) - isReady := false - if err := r.client.Get(ctx, types.NamespacedName{Namespace: ns, Name: sourcePVC.Name}, dv); err != nil { + if sourcePVC := dataSource.Spec.Source.PVC; sourcePVC != nil { + if err := r.handlePvcSource(ctx, sourcePVC, dataSource); err != nil { + return err + } + } else if sourceSnapshot := dataSource.Spec.Source.Snapshot; sourceSnapshot != nil { + if err := r.handleSnapshotSource(ctx, sourceSnapshot, dataSource); err != nil { + return err + } + } else { + updateDataSourceCondition(dataSource, cdiv1.DataSourceReady, corev1.ConditionFalse, "No source PVC set", noSource) + } + + if !reflect.DeepEqual(dataSource, dataSourceCopy) { + if err := r.client.Update(ctx, dataSource); err != nil { + return err + } + } + return nil +} + +func (r *DataSourceReconciler) handlePvcSource(ctx context.Context, sourcePVC *cdiv1.DataVolumeSourcePVC, dataSource *cdiv1.DataSource) error { + dv := &cdiv1.DataVolume{} + ns := cc.GetNamespace(sourcePVC.Namespace, dataSource.Namespace) + isReady := false + if err := r.client.Get(ctx, types.NamespacedName{Namespace: ns, Name: sourcePVC.Name}, dv); err != nil { + if !k8serrors.IsNotFound(err) { + return err + } + pvc := &corev1.PersistentVolumeClaim{} + if err := r.client.Get(ctx, types.NamespacedName{Namespace: ns, Name: sourcePVC.Name}, pvc); err != nil { if !k8serrors.IsNotFound(err) { return err } - pvc := &corev1.PersistentVolumeClaim{} - if err := r.client.Get(ctx, types.NamespacedName{Namespace: ns, Name: sourcePVC.Name}, pvc); err != nil { - if !k8serrors.IsNotFound(err) { - return err - } - r.log.Info("PVC not found", "name", sourcePVC.Name) - updateDataSourceCondition(dataSource, cdiv1.DataSourceReady, corev1.ConditionFalse, "PVC not found", cc.NotFound) - } else { - isReady = true - } - } else if dv.Status.Phase == cdiv1.Succeeded { - isReady = true + r.log.Info("PVC not found", "name", sourcePVC.Name) + updateDataSourceCondition(dataSource, cdiv1.DataSourceReady, corev1.ConditionFalse, "PVC not found", cc.NotFound) } else { - updateDataSourceCondition(dataSource, cdiv1.DataSourceReady, corev1.ConditionFalse, fmt.Sprintf("Import DataVolume phase %s", dv.Status.Phase), string(dv.Status.Phase)) - } - if isReady { - updateDataSourceCondition(dataSource, cdiv1.DataSourceReady, corev1.ConditionTrue, "DataSource is ready to be consumed", ready) + isReady = true } + } else if dv.Status.Phase == cdiv1.Succeeded { + isReady = true } else { - updateDataSourceCondition(dataSource, cdiv1.DataSourceReady, corev1.ConditionFalse, "No source PVC set", noPvc) + updateDataSourceCondition(dataSource, cdiv1.DataSourceReady, corev1.ConditionFalse, fmt.Sprintf("Import DataVolume phase %s", dv.Status.Phase), string(dv.Status.Phase)) + } + if isReady { + updateDataSourceCondition(dataSource, cdiv1.DataSourceReady, corev1.ConditionTrue, "DataSource is ready to be consumed", ready) } - if !reflect.DeepEqual(dataSource, dataSourceCopy) { - if err := r.client.Update(ctx, dataSource); err != nil { + return nil +} + +func (r *DataSourceReconciler) handleSnapshotSource(ctx context.Context, sourceSnapshot *cdiv1.DataVolumeSourceSnapshot, dataSource *cdiv1.DataSource) error { + snapshot := &snapshotv1.VolumeSnapshot{} + ns := cc.GetNamespace(sourceSnapshot.Namespace, dataSource.Namespace) + if err := r.client.Get(ctx, types.NamespacedName{Namespace: ns, Name: sourceSnapshot.Name}, snapshot); err != nil { + if !k8serrors.IsNotFound(err) { return err } + r.log.Info("Snapshot not found", "name", sourceSnapshot.Name) + updateDataSourceCondition(dataSource, cdiv1.DataSourceReady, corev1.ConditionFalse, "Snapshot not found", cc.NotFound) + } else if snapshot.Status != nil && snapshot.Status.ReadyToUse != nil && *snapshot.Status.ReadyToUse { + updateDataSourceCondition(dataSource, cdiv1.DataSourceReady, corev1.ConditionTrue, "DataSource is ready to be consumed", ready) + } else { + updateDataSourceCondition(dataSource, cdiv1.DataSourceReady, corev1.ConditionFalse, "Snapshot phase is not ready", "SnapshotNotReady") } + return nil } @@ -159,18 +190,32 @@ func addDataSourceControllerWatches(mgr manager.Manager, c controller.Controller predicate.Funcs{ CreateFunc: func(e event.CreateEvent) bool { return true }, DeleteFunc: func(e event.DeleteEvent) bool { return true }, - UpdateFunc: func(e event.UpdateEvent) bool { return !sameDataSourcePvc(e.ObjectOld, e.ObjectNew) }, + UpdateFunc: func(e event.UpdateEvent) bool { return !sameSourceSpec(e.ObjectOld, e.ObjectNew) }, }, ); err != nil { return err } const dataSourcePvcField = "spec.source.pvc" + const dataSourceSnapshotField = "spec.source.snapshot" getKey := func(namespace, name string) string { return namespace + "/" + name } + appendMatchingDataSourceRequests := func(indexingKey string, obj client.Object, reqs []reconcile.Request) []reconcile.Request { + var dataSources cdiv1.DataSourceList + matchingFields := client.MatchingFields{indexingKey: getKey(obj.GetNamespace(), obj.GetName())} + if err := mgr.GetClient().List(context.TODO(), &dataSources, matchingFields); err != nil { + log.Error(err, "Unable to list DataSources", "matchingFields", matchingFields) + return reqs + } + for _, ds := range dataSources.Items { + reqs = append(reqs, reconcile.Request{NamespacedName: types.NamespacedName{Namespace: ds.Namespace, Name: ds.Name}}) + } + return reqs + } + if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &cdiv1.DataSource{}, dataSourcePvcField, func(obj client.Object) []string { if pvc := obj.(*cdiv1.DataSource).Spec.Source.PVC; pvc != nil { ns := cc.GetNamespace(pvc.Namespace, obj.GetNamespace()) @@ -180,17 +225,19 @@ func addDataSourceControllerWatches(mgr manager.Manager, c controller.Controller }); err != nil { return err } + if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &cdiv1.DataSource{}, dataSourceSnapshotField, func(obj client.Object) []string { + if snapshot := obj.(*cdiv1.DataSource).Spec.Source.Snapshot; snapshot != nil { + ns := cc.GetNamespace(snapshot.Namespace, obj.GetNamespace()) + return []string{getKey(ns, snapshot.Name)} + } + return nil + }); err != nil { + return err + } mapToDataSource := func(obj client.Object) (reqs []reconcile.Request) { - var dataSources cdiv1.DataSourceList - matchingFields := client.MatchingFields{dataSourcePvcField: getKey(obj.GetNamespace(), obj.GetName())} - if err := mgr.GetClient().List(context.TODO(), &dataSources, matchingFields); err != nil { - log.Error(err, "Unable to list DataSources", "matchingFields", matchingFields) - return - } - for _, ds := range dataSources.Items { - reqs = append(reqs, reconcile.Request{NamespacedName: types.NamespacedName{Namespace: ds.Namespace, Name: ds.Name}}) - } + reqs = appendMatchingDataSourceRequests(dataSourcePvcField, obj, reqs) + reqs = appendMatchingDataSourceRequests(dataSourceSnapshotField, obj, reqs) return } @@ -215,7 +262,35 @@ func addDataSourceControllerWatches(mgr manager.Manager, c controller.Controller predicate.Funcs{ CreateFunc: func(e event.CreateEvent) bool { return true }, DeleteFunc: func(e event.DeleteEvent) bool { return true }, - UpdateFunc: func(e event.UpdateEvent) bool { return false }, + UpdateFunc: func(e event.UpdateEvent) bool { + pvcOld, okOld := e.ObjectOld.(*corev1.PersistentVolumeClaim) + pvcNew, okNew := e.ObjectNew.(*corev1.PersistentVolumeClaim) + return okOld && okNew && pvcOld.Status.Phase != pvcNew.Status.Phase + }, + }, + ); err != nil { + return err + } + + if err := mgr.GetClient().List(context.TODO(), &snapshotv1.VolumeSnapshotList{}); err != nil { + if meta.IsNoMatchError(err) { + // Back out if there's no point to attempt watch + return nil + } + if !cc.IsErrCacheNotStarted(err) { + return err + } + } + if err := c.Watch(&source.Kind{Type: &snapshotv1.VolumeSnapshot{}}, + handler.EnqueueRequestsFromMapFunc(mapToDataSource), + predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { return true }, + DeleteFunc: func(e event.DeleteEvent) bool { return true }, + UpdateFunc: func(e event.UpdateEvent) bool { + snapOld, okOld := e.ObjectOld.(*snapshotv1.VolumeSnapshot) + snapNew, okNew := e.ObjectNew.(*snapshotv1.VolumeSnapshot) + return okOld && okNew && !reflect.DeepEqual(snapOld.Status, snapNew.Status) + }, }, ); err != nil { return err @@ -224,8 +299,19 @@ func addDataSourceControllerWatches(mgr manager.Manager, c controller.Controller return nil } -func sameDataSourcePvc(objOld, objNew client.Object) bool { +func sameSourceSpec(objOld, objNew client.Object) bool { dsOld, okOld := objOld.(*cdiv1.DataSource) dsNew, okNew := objNew.(*cdiv1.DataSource) - return okOld && okNew && reflect.DeepEqual(dsOld.Spec.Source.PVC, dsNew.Spec.Source.PVC) + + if !okOld || !okNew { + return false + } + if dsOld.Spec.Source.PVC != nil { + return reflect.DeepEqual(dsOld.Spec.Source.PVC, dsNew.Spec.Source.PVC) + } + if dsOld.Spec.Source.Snapshot != nil { + return reflect.DeepEqual(dsOld.Spec.Source.Snapshot, dsNew.Spec.Source.Snapshot) + } + + return false } diff --git a/pkg/controller/datasource-controller_test.go b/pkg/controller/datasource-controller_test.go index 1056a3a76b..d6d23b4752 100644 --- a/pkg/controller/datasource-controller_test.go +++ b/pkg/controller/datasource-controller_test.go @@ -22,10 +22,12 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/pointer" cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" . "kubevirt.io/containerized-data-importer/pkg/controller/common" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -33,8 +35,9 @@ import ( ) const ( - dsName = "test-datasource" - pvcName = "test-pvc" + dsName = "test-datasource" + pvcName = "test-pvc" + snapshotName = "test-snapshot" ) var _ = Describe("All DataSource Tests", func() { @@ -64,10 +67,10 @@ var _ = Describe("All DataSource Tests", func() { Expect(err).ToNot(HaveOccurred()) }) - It("Should update Ready condition when DataSource has no source pvc", func() { + It("Should update Ready condition when DataSource has no source", func() { ds = createDataSource() reconciler = createDataSourceReconciler(ds) - verifyConditions("No source pvc", false, noPvc) + verifyConditions("No source", false, noSource) }) It("Should update Ready condition when DataSource has source pvc", func() { @@ -103,12 +106,43 @@ var _ = Describe("All DataSource Tests", func() { Expect(err).ToNot(HaveOccurred()) verifyConditions("Source PVC Deleted", false, NotFound) }) + + It("Should update Ready condition when DataSource has source snapshot", func() { + ds = createDataSource() + ds.Spec.Source.Snapshot = &cdiv1.DataVolumeSourceSnapshot{Namespace: metav1.NamespaceDefault, Name: snapshotName} + reconciler = createDataSourceReconciler(ds) + verifyConditions("Source snapshot does not exist", false, NotFound) + + snap := &snapshotv1.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: snapshotName, + Namespace: metav1.NamespaceDefault, + }, + Spec: snapshotv1.VolumeSnapshotSpec{}, + Status: &snapshotv1.VolumeSnapshotStatus{ + ReadyToUse: pointer.Bool(false), + }, + } + err := reconciler.client.Create(context.TODO(), snap) + Expect(err).ToNot(HaveOccurred()) + verifyConditions("Source snapshot not ready", false, "SnapshotNotReady") + + snap.Status.ReadyToUse = pointer.Bool(true) + err = reconciler.client.Update(context.TODO(), snap) + Expect(err).ToNot(HaveOccurred()) + verifyConditions("Source snapshot ready", true, ready) + + err = reconciler.client.Delete(context.TODO(), snap) + Expect(err).ToNot(HaveOccurred()) + verifyConditions("Source snapshot Deleted", false, NotFound) + }) }) }) func createDataSourceReconciler(objects ...runtime.Object) *DataSourceReconciler { s := scheme.Scheme cdiv1.AddToScheme(s) + snapshotv1.AddToScheme(s) cl := fake.NewFakeClientWithScheme(s, objects...) r := &DataSourceReconciler{ client: cl, diff --git a/pkg/controller/datavolume/clone-controller-base.go b/pkg/controller/datavolume/clone-controller-base.go index c4d9e4ca08..b564c7d349 100644 --- a/pkg/controller/datavolume/clone-controller-base.go +++ b/pkg/controller/datavolume/clone-controller-base.go @@ -595,7 +595,8 @@ func (r *CloneReconcilerBase) populateSourceIfSourceRef(dv *cdiv1.DataVolume) er return err } dv.Spec.Source = &cdiv1.DataVolumeSource{ - PVC: dataSource.Spec.Source.PVC, + PVC: dataSource.Spec.Source.PVC, + Snapshot: dataSource.Spec.Source.Snapshot, } return nil } @@ -612,7 +613,7 @@ func (r *CloneReconcilerBase) cleanupTransfer(dv *cdiv1.DataVolume) error { // delete all potential PVCs that may not have owner refs namespaces := []string{dv.Namespace} names := []string{dv.Name} - appendTmpPvcIfNeeded(dv, namespaces, names, transferName) + namespaces, names = appendTmpPvcIfNeeded(dv, namespaces, names, transferName) for i := range namespaces { pvc := &corev1.PersistentVolumeClaim{} @@ -665,13 +666,15 @@ func (r *CloneReconcilerBase) cleanupTransfer(dv *cdiv1.DataVolume) error { return nil } -func appendTmpPvcIfNeeded(dv *cdiv1.DataVolume, names, namespaces []string, pvcName string) { +func appendTmpPvcIfNeeded(dv *cdiv1.DataVolume, namespaces, names []string, pvcName string) ([]string, []string) { _, sourceNamespace := cc.GetCloneSourceNameAndNamespace(dv) if sourceNamespace != "" && sourceNamespace != dv.Namespace { namespaces = append(namespaces, sourceNamespace) names = append(names, pvcName) } + + return namespaces, names } func isCrossNamespaceClone(dv *cdiv1.DataVolume) bool { @@ -713,7 +716,7 @@ func addCloneWithoutSourceWatch(mgr manager.Manager, datavolumeController contro return } for _, dv := range dvList.Items { - op := getDataVolumeOp(&dv) + op := getDataVolumeOp(mgr.GetLogger(), &dv, mgr.GetClient()) if op == dataVolumePvcClone || op == dataVolumeSnapshotClone { reqs = append(reqs, reconcile.Request{NamespacedName: types.NamespacedName{Namespace: dv.Namespace, Name: dv.Name}}) } diff --git a/pkg/controller/datavolume/controller-base.go b/pkg/controller/datavolume/controller-base.go index c6063ce033..245e48895b 100644 --- a/pkg/controller/datavolume/controller-base.go +++ b/pkg/controller/datavolume/controller-base.go @@ -198,7 +198,7 @@ func addDataVolumeControllerCommonWatches(mgr manager.Manager, dataVolumeControl } return reqs } - if getDataVolumeOp(dv) == op { + if getDataVolumeOp(mgr.GetLogger(), dv, mgr.GetClient()) == op { reqs = append(reqs, reconcile.Request{NamespacedName: dvKey}) } return reqs @@ -208,7 +208,7 @@ func addDataVolumeControllerCommonWatches(mgr manager.Manager, dataVolumeControl if err := dataVolumeController.Watch(&source.Kind{Type: &cdiv1.DataVolume{}}, handler.EnqueueRequestsFromMapFunc( func(obj client.Object) []reconcile.Request { dv := obj.(*cdiv1.DataVolume) - if getDataVolumeOp(dv) != op { + if getDataVolumeOp(mgr.GetLogger(), dv, mgr.GetClient()) != op { return nil } return []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: dv.Namespace, Name: dv.Name}}} @@ -269,7 +269,7 @@ func addDataVolumeControllerCommonWatches(mgr manager.Manager, dataVolumeControl return } for _, dv := range dvList.Items { - if getDataVolumeOp(&dv) == op { + if getDataVolumeOp(mgr.GetLogger(), &dv, mgr.GetClient()) == op { reqs = append(reqs, reconcile.Request{NamespacedName: types.NamespacedName{Name: dv.Name, Namespace: dv.Namespace}}) } } @@ -283,14 +283,16 @@ func addDataVolumeControllerCommonWatches(mgr manager.Manager, dataVolumeControl return nil } -func getDataVolumeOp(dv *cdiv1.DataVolume) dataVolumeOp { +func getDataVolumeOp(log logr.Logger, dv *cdiv1.DataVolume, client client.Client) dataVolumeOp { src := dv.Spec.Source - if (src != nil && src.PVC != nil) || dv.Spec.SourceRef != nil { + if dv.Spec.SourceRef != nil { + return getSourceRefOp(log, dv, client) + } + if src != nil && src.PVC != nil { return dataVolumePvcClone } - // FIXME: order dependent dv.Spec.SourceRef, should lookup DataSource to determine op - if (src != nil && src.Snapshot != nil) || dv.Spec.SourceRef != nil { + if src != nil && src.Snapshot != nil { return dataVolumeSnapshotClone } if src == nil { @@ -309,6 +311,28 @@ func getDataVolumeOp(dv *cdiv1.DataVolume) dataVolumeOp { return dataVolumeNop } +func getSourceRefOp(log logr.Logger, dv *cdiv1.DataVolume, client client.Client) dataVolumeOp { + dataSource := &cdiv1.DataSource{} + ns := dv.Namespace + if dv.Spec.SourceRef.Namespace != nil && *dv.Spec.SourceRef.Namespace != "" { + ns = *dv.Spec.SourceRef.Namespace + } + nn := types.NamespacedName{Namespace: ns, Name: dv.Spec.SourceRef.Name} + if err := client.Get(context.TODO(), nn, dataSource); err != nil { + log.Error(err, "Unable to get DataSource", "namespacedName", nn) + return dataVolumeNop + } + + switch { + case dataSource.Spec.Source.PVC != nil: + return dataVolumePvcClone + case dataSource.Spec.Source.Snapshot != nil: + return dataVolumeSnapshotClone + default: + return dataVolumeNop + } +} + type dvController interface { sync(log logr.Logger, req reconcile.Request) (dvSyncResult, error) updateStatusPhase(pvc *corev1.PersistentVolumeClaim, dataVolumeCopy *cdiv1.DataVolume, event *Event) error diff --git a/pkg/controller/datavolume/pvc-clone-controller.go b/pkg/controller/datavolume/pvc-clone-controller.go index 76444ffa80..5ff540e909 100644 --- a/pkg/controller/datavolume/pvc-clone-controller.go +++ b/pkg/controller/datavolume/pvc-clone-controller.go @@ -706,6 +706,10 @@ func (r *PvcCloneReconciler) cleanup(syncState *dvSyncState) error { dv := syncState.dvMutated r.log.V(3).Info("Cleanup initiated in dv PVC clone controller") + if err := r.populateSourceIfSourceRef(dv); err != nil { + return err + } + if isCrossNamespaceClone(dv) { if err := r.cleanupTransfer(dv); err != nil { return err diff --git a/pkg/controller/datavolume/snapshot-clone-controller.go b/pkg/controller/datavolume/snapshot-clone-controller.go index 4e7a32ed61..d6ce8f6363 100644 --- a/pkg/controller/datavolume/snapshot-clone-controller.go +++ b/pkg/controller/datavolume/snapshot-clone-controller.go @@ -525,6 +525,10 @@ func (r *SnapshotCloneReconciler) cleanup(syncState *dvSyncState) error { dv := syncState.dvMutated r.log.V(3).Info("Cleanup initiated in dv snapshot clone controller") + if err := r.populateSourceIfSourceRef(dv); err != nil { + return err + } + if isCrossNamespaceClone(dv) { if err := r.cleanupTransfer(dv); err != nil { return err diff --git a/tests/cloner_test.go b/tests/cloner_test.go index c3303d2fb6..b4b38f5289 100644 --- a/tests/cloner_test.go +++ b/tests/cloner_test.go @@ -263,7 +263,7 @@ var _ = Describe("all clone tests", func() { pvc, err := f.K8sClient.CoreV1().PersistentVolumeClaims(dataVolume.Namespace).Get(context.TODO(), dataVolume.Name, metav1.GetOptions{}) Expect(err).ToNot(HaveOccurred()) - targetDS := utils.NewDataSource("test-datasource", pvc.Namespace, pvc.Name, pvc.Namespace) + targetDS := utils.NewPvcDataSource("test-datasource", pvc.Namespace, pvc.Name, pvc.Namespace) By(fmt.Sprintf("Create new datasource %s", targetDS.Name)) targetDataSource, err := f.CdiClient.CdiV1beta1().DataSources(pvc.Namespace).Create(context.TODO(), targetDS, metav1.CreateOptions{}) Expect(err).ToNot(HaveOccurred()) @@ -2859,6 +2859,34 @@ var _ = Describe("all clone tests", func() { Expect(err.Error()).To(ContainSubstring("target resources requests storage size is smaller than the source")) }) }) + + Context("sourceRef support", func() { + It("Should clone data from SourceRef snapshot DataSource", func() { + size := "1Gi" + volumeMode := v1.PersistentVolumeMode(v1.PersistentVolumeFilesystem) + createSnapshot(size, nil, volumeMode) + + targetDS := utils.NewSnapshotDataSource("test-datasource", snapshot.Namespace, snapshot.Name, snapshot.Namespace) + By(fmt.Sprintf("Create new datasource %s", targetDS.Name)) + targetDataSource, err := f.CdiClient.CdiV1beta1().DataSources(snapshot.Namespace).Create(context.TODO(), targetDS, metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + targetDV := utils.NewDataVolumeWithSourceRef("target-dv", size, targetDataSource.Namespace, targetDataSource.Name) + By(fmt.Sprintf("Create new target datavolume %s", targetDV.Name)) + targetDataVolume, err := utils.CreateDataVolumeFromDefinition(f.CdiClient, f.Namespace.Name, targetDV) + Expect(err).ToNot(HaveOccurred()) + f.ForceBindPvcIfDvIsWaitForFirstConsumer(targetDataVolume) + + By("Wait for clone DV Succeeded phase") + err = utils.WaitForDataVolumePhaseWithTimeout(f, f.Namespace.Name, cdiv1.Succeeded, targetDV.Name, cloneCompleteTimeout) + Expect(err).ToNot(HaveOccurred()) + By("Verify MD5") + path := utils.DefaultImagePath + same, err := f.VerifyTargetPVCContentMD5(f.Namespace, utils.PersistentVolumeClaimFromDataVolume(targetDV), path, utils.UploadFileMD5, utils.UploadFileSize) + Expect(err).ToNot(HaveOccurred()) + Expect(same).To(BeTrue()) + }) + }) }) }) diff --git a/tests/datasource_test.go b/tests/datasource_test.go index 12a017fbb9..a40233c524 100644 --- a/tests/datasource_test.go +++ b/tests/datasource_test.go @@ -5,6 +5,7 @@ import ( "fmt" "time" + snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -21,10 +22,12 @@ import ( var _ = Describe("DataSource", func() { const ( - ds1Name = "ds1" - ds2Name = "ds2" - pvc1Name = "pvc1" - pvc2Name = "pvc2" + ds1Name = "ds1" + ds2Name = "ds2" + pvc1Name = "pvc1" + pvc2Name = "pvc2" + snap1Name = "snap1" + snap2Name = "snap2" ) f := framework.NewFramework("datasource-func-test") @@ -54,7 +57,7 @@ var _ = Describe("DataSource", func() { return ds } - testUrl := func() string { return fmt.Sprintf(utils.TinyCoreQcow2URL, f.CdiInstallNs) } + testURL := func() string { return fmt.Sprintf(utils.TinyCoreQcow2URL, f.CdiInstallNs) } createDv := func(pvcName, url string) { By(fmt.Sprintf("creating DataVolume %s %s", pvcName, url)) dv := utils.NewDataVolumeWithHTTPImport(pvcName, "1Gi", url) @@ -71,7 +74,7 @@ var _ = Describe("DataSource", func() { ds := newDataSource(ds1Name) ds, err := f.CdiClient.CdiV1beta1().DataSources(ds.Namespace).Create(context.TODO(), ds, metav1.CreateOptions{}) Expect(err).ToNot(HaveOccurred()) - ds = waitForReadyCondition(ds, corev1.ConditionFalse, "NoPvc") + ds = waitForReadyCondition(ds, corev1.ConditionFalse, "NoSource") By("Update DataSource source PVC to nonexisting one") ds.Spec.Source.PVC = &cdiv1.DataVolumeSourcePVC{Namespace: f.Namespace.Name, Name: pvc1Name} @@ -94,7 +97,7 @@ var _ = Describe("DataSource", func() { f.ExpectEvent(dv.Namespace).Should(ContainSubstring(dvc.CloneWithoutSource)) By("Create import DV so the missing DataSource source PVC will be ready") - createDv(pvc1Name, testUrl()) + createDv(pvc1Name, testURL()) ds = waitForReadyCondition(ds, corev1.ConditionTrue, "Ready") By("Wait for the clone DV success") @@ -107,7 +110,7 @@ var _ = Describe("DataSource", func() { ds.Spec.Source.PVC = nil ds, err = f.CdiClient.CdiV1beta1().DataSources(ds.Namespace).Update(context.TODO(), ds, metav1.UpdateOptions{}) Expect(err).ToNot(HaveOccurred()) - ds = waitForReadyCondition(ds, corev1.ConditionFalse, "NoPvc") + _ = waitForReadyCondition(ds, corev1.ConditionFalse, "NoSource") }) createDs := func(dsName, pvcName string) *cdiv1.DataSource { @@ -122,12 +125,12 @@ var _ = Describe("DataSource", func() { updateDsPvc := func(ds *cdiv1.DataSource, pvcName string) { By(fmt.Sprintf("updating DataSource %s -> %s", ds.Name, pvcName)) ds.Spec.Source.PVC = &cdiv1.DataVolumeSourcePVC{Namespace: "", Name: pvcName} - ds, err := f.CdiClient.CdiV1beta1().DataSources(ds.Namespace).Update(context.TODO(), ds, metav1.UpdateOptions{}) + _, err := f.CdiClient.CdiV1beta1().DataSources(ds.Namespace).Update(context.TODO(), ds, metav1.UpdateOptions{}) Expect(err).ToNot(HaveOccurred()) } It("[test_id:8067]status conditions should be updated when several DataSources refer the same pvc", func() { - createDv(pvc1Name, testUrl()) + createDv(pvc1Name, testURL()) ds1 := createDs(ds1Name, pvc1Name) ds2 := createDs(ds2Name, pvc1Name) @@ -138,26 +141,26 @@ var _ = Describe("DataSource", func() { ds1 = waitForReadyCondition(ds1, corev1.ConditionFalse, "NotFound") ds2 = waitForReadyCondition(ds2, corev1.ConditionFalse, "NotFound") - createDv(pvc2Name, testUrl()+"bad") + createDv(pvc2Name, testURL()+"bad") updateDsPvc(ds1, pvc2Name) updateDsPvc(ds2, pvc2Name) ds1 = waitForReadyCondition(ds1, corev1.ConditionFalse, "ImportInProgress") ds2 = waitForReadyCondition(ds2, corev1.ConditionFalse, "ImportInProgress") deleteDvPvc(f, pvc2Name) - ds1 = waitForReadyCondition(ds1, corev1.ConditionFalse, "NotFound") - ds2 = waitForReadyCondition(ds2, corev1.ConditionFalse, "NotFound") + _ = waitForReadyCondition(ds1, corev1.ConditionFalse, "NotFound") + _ = waitForReadyCondition(ds2, corev1.ConditionFalse, "NotFound") }) It("status conditions timestamp should be updated when DataSource referred pvc is updated, although condition status does not change", func() { - createDv(pvc1Name, testUrl()) + createDv(pvc1Name, testURL()) ds := createDs(ds1Name, pvc1Name) ds = waitForReadyCondition(ds, corev1.ConditionTrue, "Ready") cond := controller.FindDataSourceConditionByType(ds, cdiv1.DataSourceReady) Expect(cond).ToNot(BeNil()) ts := cond.LastTransitionTime - createDv(pvc2Name, testUrl()) + createDv(pvc2Name, testURL()) err := utils.WaitForDataVolumePhase(f, f.Namespace.Name, cdiv1.Succeeded, pvc2Name) Expect(err).ToNot(HaveOccurred()) updateDsPvc(ds, pvc2Name) @@ -171,6 +174,108 @@ var _ = Describe("DataSource", func() { return cond.LastTransitionTime }, 60*time.Second, pollingInterval).ShouldNot(Equal(ts)) }) + + Context("snapshot source", func() { + createSnap := func(name string) *snapshotv1.VolumeSnapshot { + pvcDef := utils.NewPVCDefinition("snap-source-pvc", "1Gi", nil, nil) + pvcDef.Namespace = f.Namespace.Name + pvc, err := f.K8sClient.CoreV1().PersistentVolumeClaims(f.Namespace.Name).Create(context.TODO(), pvcDef, metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + f.ForceBindIfWaitForFirstConsumer(pvc) + + snapClass := f.GetSnapshotClass() + snapshot := &snapshotv1.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: pvc.Namespace, + }, + Spec: snapshotv1.VolumeSnapshotSpec{ + Source: snapshotv1.VolumeSnapshotSource{ + PersistentVolumeClaimName: &pvc.Name, + }, + VolumeSnapshotClassName: &snapClass.Name, + }, + } + err = f.CrClient.Create(context.TODO(), snapshot) + Expect(err).ToNot(HaveOccurred()) + + return snapshot + } + + createSnapDs := func(dsName, snapName string) *cdiv1.DataSource { + By(fmt.Sprintf("creating DataSource %s -> %s", dsName, snapName)) + ds := newDataSource(dsName) + ds.Spec.Source.Snapshot = &cdiv1.DataVolumeSourceSnapshot{Namespace: f.Namespace.Name, Name: snapName} + ds, err := f.CdiClient.CdiV1beta1().DataSources(ds.Namespace).Create(context.TODO(), ds, metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + return waitForReadyCondition(ds, corev1.ConditionTrue, "Ready") + } + + BeforeEach(func() { + if !f.IsSnapshotStorageClassAvailable() { + Skip("Clone from volumesnapshot does not work without snapshot capable storage") + } + }) + + It("status conditions should be updated on snapshot create/update/delete", func() { + By("Create DataSource with no source") + ds := newDataSource(ds1Name) + ds, err := f.CdiClient.CdiV1beta1().DataSources(ds.Namespace).Create(context.TODO(), ds, metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + ds = waitForReadyCondition(ds, corev1.ConditionFalse, "NoSource") + + By("Update DataSource source snapshot to nonexisting one") + ds.Spec.Source.Snapshot = &cdiv1.DataVolumeSourceSnapshot{Namespace: f.Namespace.Name, Name: snap1Name} + ds, err = f.CdiClient.CdiV1beta1().DataSources(ds.Namespace).Update(context.TODO(), ds, metav1.UpdateOptions{}) + Expect(err).ToNot(HaveOccurred()) + ds = waitForReadyCondition(ds, corev1.ConditionFalse, "NotFound") + + By("Create clone DV with SourceRef pointing the DataSource") + dv := utils.NewDataVolumeWithSourceRef("clone-dv", "1Gi", ds.Namespace, ds.Name) + dv.Annotations[cc.AnnImmediateBinding] = "true" + Expect(dv).ToNot(BeNil()) + dv, err = utils.CreateDataVolumeFromDefinition(f.CdiClient, f.Namespace.Name, dv) + Expect(err).ToNot(HaveOccurred()) + + By("Verify DV conditions") + utils.WaitForConditions(f, dv.Name, dv.Namespace, time.Minute, pollingInterval, + &cdiv1.DataVolumeCondition{Type: cdiv1.DataVolumeBound, Status: corev1.ConditionUnknown, Message: "No PVC found", Reason: dvc.CloneWithoutSource}, + &cdiv1.DataVolumeCondition{Type: cdiv1.DataVolumeReady, Status: corev1.ConditionFalse, Reason: dvc.CloneWithoutSource}, + &cdiv1.DataVolumeCondition{Type: cdiv1.DataVolumeRunning, Status: corev1.ConditionFalse}) + f.ExpectEvent(dv.Namespace).Should(ContainSubstring(dvc.CloneWithoutSource)) + + By("Create snapshot so the DataSource will be ready") + snapshot := createSnap(snap1Name) + ds = waitForReadyCondition(ds, corev1.ConditionTrue, "Ready") + + By("Wait for the clone DV success") + err = utils.WaitForDataVolumePhase(f, dv.Namespace, cdiv1.Succeeded, dv.Name) + Expect(err).ToNot(HaveOccurred()) + + err = f.CrClient.Delete(context.TODO(), snapshot) + Expect(err).ToNot(HaveOccurred()) + ds = waitForReadyCondition(ds, corev1.ConditionFalse, "NotFound") + + ds.Spec.Source.Snapshot = nil + ds, err = f.CdiClient.CdiV1beta1().DataSources(ds.Namespace).Update(context.TODO(), ds, metav1.UpdateOptions{}) + Expect(err).ToNot(HaveOccurred()) + _ = waitForReadyCondition(ds, corev1.ConditionFalse, "NoSource") + }) + + It("status conditions should be updated when several DataSources refer the same snapshot", func() { + snapshot := createSnap(snap1Name) + ds1 := createSnapDs(ds1Name, snap1Name) + ds2 := createSnapDs(ds2Name, snap1Name) + + ds1 = waitForReadyCondition(ds1, corev1.ConditionTrue, "Ready") + ds2 = waitForReadyCondition(ds2, corev1.ConditionTrue, "Ready") + + err := f.CrClient.Delete(context.TODO(), snapshot) + Expect(err).ToNot(HaveOccurred()) + _ = waitForReadyCondition(ds1, corev1.ConditionFalse, "NotFound") + _ = waitForReadyCondition(ds2, corev1.ConditionFalse, "NotFound") + }) + }) }) func deleteDvPvc(f *framework.Framework, pvcName string) { diff --git a/tests/utils/datavolume.go b/tests/utils/datavolume.go index 32aed19d0c..10487ffae3 100644 --- a/tests/utils/datavolume.go +++ b/tests/utils/datavolume.go @@ -175,8 +175,8 @@ func NewDataVolumeWithSourceRef(dataVolumeName string, size, sourceRefNamespace, } } -// NewDataSource initializes a DataSource struct with PVC source -func NewDataSource(dataSourceName, dataSourceNamespace, pvcName, pvcNamespace string) *cdiv1.DataSource { +// NewPvcDataSource initializes a DataSource struct with PVC source +func NewPvcDataSource(dataSourceName, dataSourceNamespace, pvcName, pvcNamespace string) *cdiv1.DataSource { return &cdiv1.DataSource{ ObjectMeta: metav1.ObjectMeta{ Name: dataSourceName, @@ -193,6 +193,24 @@ func NewDataSource(dataSourceName, dataSourceNamespace, pvcName, pvcNamespace st } } +// NewSnapshotDataSource initializes a DataSource struct with volumesnapshot source +func NewSnapshotDataSource(dataSourceName, dataSourceNamespace, snapshotName, snapshotNamespace string) *cdiv1.DataSource { + return &cdiv1.DataSource{ + ObjectMeta: metav1.ObjectMeta{ + Name: dataSourceName, + Namespace: dataSourceNamespace, + }, + Spec: cdiv1.DataSourceSpec{ + Source: cdiv1.DataSourceSource{ + Snapshot: &cdiv1.DataVolumeSourceSnapshot{ + Name: snapshotName, + Namespace: snapshotNamespace, + }, + }, + }, + } +} + // NewDataVolumeWithHTTPImport initializes a DataVolume struct with HTTP annotations func NewDataVolumeWithHTTPImport(dataVolumeName string, size string, httpURL string) *cdiv1.DataVolume { claimSpec := &k8sv1.PersistentVolumeClaimSpec{