From b80ff58f9feb3c2490781803435e74b85960f5ad Mon Sep 17 00:00:00 2001 From: kubevirt-bot Date: Fri, 30 Jun 2023 20:16:35 +0200 Subject: [PATCH] [release-v1.57] DataVolume Controller uses VolumeCloneSource Populator (#2783) * remove CSI clone bye bye Signed-off-by: Michael Henriksen * no more smart clone Signed-off-by: Michael Henriksen * PVC clone same namespace Signed-off-by: Michael Henriksen * cross namespace pvc clone Signed-off-by: Michael Henriksen * various fixes to get some functional tests to work Signed-off-by: Michael Henriksen * delete smart clone controller again somehow reappeared after rebase Signed-off-by: Michael Henriksen * mostly pvc clone functional test fixes make sure size detect pod only runs on kubevirt content type clone populator was skipping last round op applying pvc' annotations various func test fixes review comments Signed-off-by: Michael Henriksen * more various test fixes host clone phase should (implicitly) wait for clone source pod to exit Signed-off-by: Michael Henriksen * remove "smart" clone from snapshot Signed-off-by: Michael Henriksen * DataVolume clone from snapshot uses populator Signed-off-by: Michael Henriksen * improve clone populator/datavolume coordination on "running" condition For host clone, not much changes, values still comming from annotations on host clone PVC For smart/csi clone the DataVolume will be "running" if not in pending or error phase Will have the same values for terminal "completed" state regardless of clone type Signed-off-by: Michael Henriksen * unit tests for pvc/snapshot clone controllers Signed-off-by: Michael Henriksen * remove skipped test added in https://github.com/kubevirt/containerized-data-importer/pull/2759 Signed-off-by: Michael Henriksen * attempt address AfterSuite and generate-verify failures Signed-off-by: Michael Henriksen * handle snapshot clone with no target size specified also add more validation to some snapshot clone tests Signed-off-by: Michael Henriksen * remove Patch calls Using the controller runtime Patch API with controller runtime cached client seems to be a pretty bad fit At least given the way the CR API is designed where an old object is compared to new. I like patch in theory though and will revisit Signed-off-by: Michael Henriksen * Clone populator should plan and execute even if PVC is bound It was possible to miss "preallocation applied" annotation otherwise Signed-off-by: Michael Henriksen * add long term token to datavolume Signed-off-by: Michael Henriksen * Rename ProgressReporter to StatusReporter Should have been done back when annotations were addded to "progress" Also, if pvc is bound do not call phase Reconcile functions only Status Signed-off-by: Michael Henriksen --------- Signed-off-by: Michael Henriksen Co-authored-by: Michael Henriksen --- cmd/cdi-controller/controller.go | 2 +- pkg/controller/BUILD.bazel | 1 - pkg/controller/clone-controller.go | 24 +- pkg/controller/clone-controller_test.go | 86 +- pkg/controller/clone/common.go | 36 +- pkg/controller/clone/csi-clone.go | 16 +- pkg/controller/clone/host-clone.go | 43 +- pkg/controller/clone/host-clone_test.go | 25 + pkg/controller/clone/planner.go | 38 +- pkg/controller/clone/planner_test.go | 5 +- pkg/controller/clone/prep-claim.go | 5 +- pkg/controller/clone/rebind.go | 5 +- pkg/controller/clone/snap-clone.go | 5 +- pkg/controller/clone/snapshot.go | 18 +- pkg/controller/common/BUILD.bazel | 1 + pkg/controller/common/util.go | 122 ++- pkg/controller/datavolume/BUILD.bazel | 7 +- .../datavolume/clone-controller-base.go | 638 +++++-------- pkg/controller/datavolume/controller-base.go | 10 +- .../datavolume/import-controller_test.go | 9 +- .../datavolume/pvc-clone-controller.go | 841 +++--------------- .../datavolume/pvc-clone-controller_test.go | 778 ++++++---------- .../datavolume/smart-clone-controller.go | 427 --------- .../datavolume/smart-clone-controller_test.go | 341 ------- .../datavolume/snapshot-clone-controller.go | 382 ++++---- .../snapshot-clone-controller_test.go | 310 ++++++- pkg/controller/populators/BUILD.bazel | 1 + pkg/controller/populators/clone-populator.go | 230 +++-- .../populators/clone-populator_test.go | 99 ++- pkg/controller/upload-controller.go | 7 +- pkg/operator/resources/cluster/controller.go | 1 + .../pkg/apis/core/v1beta1/types.go | 5 + tests/basic_sanity_test.go | 2 +- tests/clone-populator_test.go | 8 +- tests/cloner_test.go | 252 ++---- tests/csiclone_test.go | 12 +- tests/datavolume_test.go | 9 +- tests/framework/pvc.go | 4 +- tests/smartclone_test.go | 13 +- 39 files changed, 1803 insertions(+), 3015 deletions(-) delete mode 100644 pkg/controller/datavolume/smart-clone-controller.go delete mode 100644 pkg/controller/datavolume/smart-clone-controller_test.go diff --git a/cmd/cdi-controller/controller.go b/cmd/cdi-controller/controller.go index c0500d1b75..2275ae1636 100644 --- a/cmd/cdi-controller/controller.go +++ b/cmd/cdi-controller/controller.go @@ -280,7 +280,7 @@ func start() { klog.Errorf("Unable to setup upload populator: %v", err) os.Exit(1) } - if _, err := populators.NewClonePopulator(ctx, mgr, log, clonerImage, pullPolicy, installerLabels); err != nil { + if _, err := populators.NewClonePopulator(ctx, mgr, log, clonerImage, pullPolicy, installerLabels, getTokenPublicKey()); err != nil { klog.Errorf("Unable to setup clone populator: %v", err) os.Exit(1) } diff --git a/pkg/controller/BUILD.bazel b/pkg/controller/BUILD.bazel index 721ebc4ef5..b2a9f1da7d 100644 --- a/pkg/controller/BUILD.bazel +++ b/pkg/controller/BUILD.bazel @@ -23,7 +23,6 @@ go_library( "//pkg/monitoring:go_default_library", "//pkg/operator:go_default_library", "//pkg/storagecapabilities:go_default_library", - "//pkg/token:go_default_library", "//pkg/util:go_default_library", "//pkg/util/cert:go_default_library", "//pkg/util/cert/fetcher:go_default_library", diff --git a/pkg/controller/clone-controller.go b/pkg/controller/clone-controller.go index c7834b7c45..176fb2b882 100644 --- a/pkg/controller/clone-controller.go +++ b/pkg/controller/clone-controller.go @@ -33,7 +33,6 @@ import ( cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" "kubevirt.io/containerized-data-importer/pkg/common" cc "kubevirt.io/containerized-data-importer/pkg/controller/common" - "kubevirt.io/containerized-data-importer/pkg/token" "kubevirt.io/containerized-data-importer/pkg/util" "kubevirt.io/containerized-data-importer/pkg/util/cert/fetcher" "kubevirt.io/containerized-data-importer/pkg/util/cert/generator" @@ -67,8 +66,7 @@ type CloneReconciler struct { clientCertGenerator generator.CertGenerator serverCAFetcher fetcher.CertBundleFetcher log logr.Logger - longTokenValidator token.Validator - shortTokenValidator token.Validator + multiTokenValidator *cc.MultiTokenValidator image string verbose string pullPolicy string @@ -88,8 +86,7 @@ func NewCloneController(mgr manager.Manager, client: mgr.GetClient(), scheme: mgr.GetScheme(), log: log.WithName("clone-controller"), - shortTokenValidator: cc.NewCloneTokenValidator(common.CloneTokenIssuer, apiServerKey), - longTokenValidator: cc.NewCloneTokenValidator(common.ExtendedCloneTokenIssuer, apiServerKey), + multiTokenValidator: cc.NewMultiTokenValidator(apiServerKey), image: image, verbose: verbose, pullPolicy: pullPolicy, @@ -249,10 +246,14 @@ func (r *CloneReconciler) reconcileSourcePod(ctx context.Context, sourcePod *cor } if len(pods) > 0 { + es, err := cc.GetAnnotatedEventSource(ctx, r.client, targetPvc) + if err != nil { + return 0, err + } for _, pod := range pods { r.log.V(1).Info("can't create clone source pod, pvc in use by other pod", "namespace", sourcePvc.Namespace, "name", sourcePvc.Name, "pod", pod.Name) - r.recorder.Eventf(targetPvc, corev1.EventTypeWarning, cc.CloneSourceInUse, + r.recorder.Eventf(es, corev1.EventTypeWarning, cc.CloneSourceInUse, "pod %s/%s using PersistentVolumeClaim %s", pod.Namespace, pod.Name, sourcePvc.Name) } return 2 * time.Second, nil @@ -406,16 +407,7 @@ func (r *CloneReconciler) findCloneSourcePod(pvc *corev1.PersistentVolumeClaim) } func (r *CloneReconciler) validateSourceAndTarget(ctx context.Context, sourcePvc, targetPvc *corev1.PersistentVolumeClaim) error { - // first check for extended token - v := r.longTokenValidator - tok, ok := targetPvc.Annotations[cc.AnnExtendedCloneToken] - if !ok { - // if token doesn't exist, no prob for same namespace - tok = targetPvc.Annotations[cc.AnnCloneToken] - v = r.shortTokenValidator - } - - if err := cc.ValidateCloneTokenPVC(tok, v, sourcePvc, targetPvc); err != nil { + if err := r.multiTokenValidator.ValidatePVC(sourcePvc, targetPvc); err != nil { return err } contentType, err := ValidateCanCloneSourceAndTargetContentType(sourcePvc, targetPvc) diff --git a/pkg/controller/clone-controller_test.go b/pkg/controller/clone-controller_test.go index e0b696ad06..0bf749e7ad 100644 --- a/pkg/controller/clone-controller_test.go +++ b/pkg/controller/clone-controller_test.go @@ -115,11 +115,11 @@ var _ = Describe("Clone controller reconcile loop", func() { AnnUploadClientName: "uploadclient"}, nil) reconciler = createCloneReconciler(testPvc, cc.CreatePvc("source", "default", map[string]string{}, nil)) By("Setting up the match token") - reconciler.shortTokenValidator.(*cc.FakeValidator).Match = "foobaz" - reconciler.shortTokenValidator.(*cc.FakeValidator).Name = "source" - reconciler.shortTokenValidator.(*cc.FakeValidator).Namespace = "default" - reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default" - reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Match = "foobaz" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Name = "source" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Namespace = "default" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1" By("Verifying no source pod exists") sourcePod, err := reconciler.findCloneSourcePod(testPvc) Expect(sourcePod).To(BeNil()) @@ -147,11 +147,11 @@ var _ = Describe("Clone controller reconcile loop", func() { sourcePvc := cc.CreatePvc("source", "default", map[string]string{}, nil) reconciler = createCloneReconciler(testPvc, sourcePvc, podFunc(sourcePvc)) By("Setting up the match token") - reconciler.shortTokenValidator.(*cc.FakeValidator).Match = "foobaz" - reconciler.shortTokenValidator.(*cc.FakeValidator).Name = "source" - reconciler.shortTokenValidator.(*cc.FakeValidator).Namespace = "default" - reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default" - reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Match = "foobaz" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Name = "source" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Namespace = "default" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1" By("Verifying no source pod exists") sourcePod, err := reconciler.findCloneSourcePod(testPvc) Expect(err).ToNot(HaveOccurred()) @@ -197,11 +197,11 @@ var _ = Describe("Clone controller reconcile loop", func() { } reconciler = createCloneReconciler(objs...) By("Setting up the match token") - reconciler.shortTokenValidator.(*cc.FakeValidator).Match = "foobaz" - reconciler.shortTokenValidator.(*cc.FakeValidator).Name = "source" - reconciler.shortTokenValidator.(*cc.FakeValidator).Namespace = "default" - reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default" - reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Match = "foobaz" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Name = "source" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Namespace = "default" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1" By("Verifying no source pod exists") sourcePod, err := reconciler.findCloneSourcePod(testPvc) Expect(err).ToNot(HaveOccurred()) @@ -269,11 +269,11 @@ var _ = Describe("Clone controller reconcile loop", func() { sourcePod.Namespace = "default" reconciler = createCloneReconciler(testPvc, cc.CreatePvc("source", "default", map[string]string{}, nil), sourcePod) By("Setting up the match token") - reconciler.shortTokenValidator.(*cc.FakeValidator).Match = "foobaz" - reconciler.shortTokenValidator.(*cc.FakeValidator).Name = "source" - reconciler.shortTokenValidator.(*cc.FakeValidator).Namespace = "default" - reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default" - reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Match = "foobaz" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Name = "source" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Namespace = "default" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1" _, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "testPvc1", Namespace: "default"}}) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("missing required " + AnnUploadClientName + " annotation")) @@ -286,11 +286,11 @@ var _ = Describe("Clone controller reconcile loop", func() { sourcePod.Namespace = "default" reconciler = createCloneReconciler(testPvc, cc.CreatePvc("source", "default", map[string]string{}, nil), sourcePod) By("Setting up the match token") - reconciler.shortTokenValidator.(*cc.FakeValidator).Match = "foobaz" - reconciler.shortTokenValidator.(*cc.FakeValidator).Name = "source" - reconciler.shortTokenValidator.(*cc.FakeValidator).Namespace = "default" - reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default" - reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Match = "foobaz" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Name = "source" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Namespace = "default" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1" _, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "testPvc1", Namespace: "default"}}) Expect(err).ToNot(HaveOccurred()) secret := &corev1.Secret{} @@ -303,11 +303,11 @@ var _ = Describe("Clone controller reconcile loop", func() { cc.AnnCloneRequest: "default/source", cc.AnnPodReady: "true", cc.AnnCloneToken: "foobaz", AnnUploadClientName: "uploadclient", cc.AnnCloneSourcePod: "default-testPvc1-source-pod"}, nil) reconciler = createCloneReconciler(testPvc, cc.CreatePvc("source", "default", map[string]string{}, nil)) By("Setting up the match token") - reconciler.shortTokenValidator.(*cc.FakeValidator).Match = "foobaz" - reconciler.shortTokenValidator.(*cc.FakeValidator).Name = "source" - reconciler.shortTokenValidator.(*cc.FakeValidator).Namespace = "default" - reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default" - reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Match = "foobaz" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Name = "source" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Namespace = "default" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1" By("Verifying no source pod exists") sourcePod, err := reconciler.findCloneSourcePod(testPvc) Expect(err).ToNot(HaveOccurred()) @@ -324,11 +324,11 @@ var _ = Describe("Clone controller reconcile loop", func() { testPvc := createTargetPvcFunc() reconciler = createCloneReconciler(testPvc, createSourcePvcFunc()) By("Setting up the match token") - reconciler.shortTokenValidator.(*cc.FakeValidator).Match = "foobaz" - reconciler.shortTokenValidator.(*cc.FakeValidator).Name = "source" - reconciler.shortTokenValidator.(*cc.FakeValidator).Namespace = "default" - reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default" - reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Match = "foobaz" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Name = "source" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Namespace = "default" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1" By("Verifying no source pod exists") sourcePod, err := reconciler.findCloneSourcePod(testPvc) Expect(err).ToNot(HaveOccurred()) @@ -431,11 +431,11 @@ var _ = Describe("Clone controller reconcile loop", func() { testPvc := createTargetPvcFunc() reconciler = createCloneReconciler(testPvc, createSourcePvcFunc()) By("Setting up the match token") - reconciler.shortTokenValidator.(*cc.FakeValidator).Match = "foobaz" - reconciler.shortTokenValidator.(*cc.FakeValidator).Name = "source" - reconciler.shortTokenValidator.(*cc.FakeValidator).Namespace = "default" - reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default" - reconciler.shortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Match = "foobaz" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Name = "source" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Namespace = "default" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetNamespace"] = "default" + reconciler.multiTokenValidator.ShortTokenValidator.(*cc.FakeValidator).Params["targetName"] = "testPvc1" By("Verifying no source pod exists") sourcePod, err := reconciler.findCloneSourcePod(testPvc) Expect(err).ToNot(HaveOccurred()) @@ -721,8 +721,10 @@ func createCloneReconciler(objects ...runtime.Object) *CloneReconciler { scheme: s, log: cloneLog, recorder: rec, - shortTokenValidator: &cc.FakeValidator{ - Params: make(map[string]string, 0), + multiTokenValidator: &cc.MultiTokenValidator{ + ShortTokenValidator: &cc.FakeValidator{ + Params: make(map[string]string, 0), + }, }, image: testImage, clientCertGenerator: &fakeCertGenerator{}, diff --git a/pkg/controller/clone/common.go b/pkg/controller/clone/common.go index 5f48b4bc04..91c462b902 100644 --- a/pkg/controller/clone/common.go +++ b/pkg/controller/clone/common.go @@ -5,6 +5,7 @@ import ( "fmt" "sort" + "github.com/go-logr/logr" snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" corev1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" @@ -21,6 +22,17 @@ import ( "kubevirt.io/containerized-data-importer/pkg/util" ) +const ( + // PendingPhaseName is the phase when the clone is pending + PendingPhaseName = "Pending" + + // SucceededPhaseName is the phase when the clone is succeeded + SucceededPhaseName = "Succeeded" + + // ErrorPhaseName is the phase when the clone is in error + ErrorPhaseName = "Error" +) + // IsDataSourcePVC checks for PersistentVolumeClaim source kind func IsDataSourcePVC(kind string) bool { return kind == "PersistentVolumeClaim" @@ -55,10 +67,20 @@ func AddOwnershipLabel(label string, obj, owner metav1.Object) { obj.GetLabels()[label] = string(owner.GetUID()) } +// IsSourceClaimReadyArgs are arguments for IsSourceClaimReady +type IsSourceClaimReadyArgs struct { + Target client.Object + SourceNamespace string + SourceName string + Client client.Client + Log logr.Logger + Recorder record.EventRecorder +} + // IsSourceClaimReady checks that PVC exists, is bound, and is not being used -func IsSourceClaimReady(ctx context.Context, c client.Client, namespace, name string) (bool, error) { +func IsSourceClaimReady(ctx context.Context, args *IsSourceClaimReadyArgs) (bool, error) { claim := &corev1.PersistentVolumeClaim{} - exists, err := getResource(ctx, c, namespace, name, claim) + exists, err := getResource(ctx, args.Client, args.SourceNamespace, args.SourceName, claim) if err != nil { return false, err } @@ -71,16 +93,22 @@ func IsSourceClaimReady(ctx context.Context, c client.Client, namespace, name st return false, nil } - pods, err := cc.GetPodsUsingPVCs(ctx, c, namespace, sets.New(name), true) + pods, err := cc.GetPodsUsingPVCs(ctx, args.Client, args.SourceNamespace, sets.New(args.SourceName), true) if err != nil { return false, err } + for _, pod := range pods { + args.Log.V(1).Info("Source PVC is being used by pod", "namespace", args.SourceNamespace, "name", args.SourceName, "pod", pod.Name) + args.Recorder.Eventf(args.Target, corev1.EventTypeWarning, cc.CloneSourceInUse, + "pod %s/%s using PersistentVolumeClaim %s", pod.Namespace, pod.Name, args.SourceName) + } + if len(pods) > 0 { return false, nil } - return cdiv1.IsPopulated(claim, dataVolumeGetter(ctx, c)) + return cdiv1.IsPopulated(claim, dataVolumeGetter(ctx, args.Client)) } // GetGlobalCloneStrategyOverride returns the global clone strategy override diff --git a/pkg/controller/clone/csi-clone.go b/pkg/controller/clone/csi-clone.go index 826849d88a..4efbb0d723 100644 --- a/pkg/controller/clone/csi-clone.go +++ b/pkg/controller/clone/csi-clone.go @@ -15,6 +15,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" ) +// CSIClonePhaseName is the name of the csi clone phase +const CSIClonePhaseName = "CSIClone" + // CSIClonePhase is responsible for csi cloning a pvc type CSIClonePhase struct { Owner client.Object @@ -31,7 +34,7 @@ var _ Phase = &CSIClonePhase{} // Name returns the name of the phase func (p *CSIClonePhase) Name() string { - return "CSIClone" + return CSIClonePhaseName } // Reconcile ensures a csi cloned pvc is created correctly @@ -43,7 +46,16 @@ func (p *CSIClonePhase) Reconcile(ctx context.Context) (*reconcile.Result, error } if !exists { - ready, err := IsSourceClaimReady(ctx, p.Client, p.Namespace, p.SourceName) + args := &IsSourceClaimReadyArgs{ + Target: p.Owner, + SourceNamespace: p.Namespace, + SourceName: p.SourceName, + Client: p.Client, + Log: p.Log, + Recorder: p.Recorder, + } + + ready, err := IsSourceClaimReady(ctx, args) if err != nil { return nil, err } diff --git a/pkg/controller/clone/host-clone.go b/pkg/controller/clone/host-clone.go index 24272c6434..f7f6d2506e 100644 --- a/pkg/controller/clone/host-clone.go +++ b/pkg/controller/clone/host-clone.go @@ -16,6 +16,9 @@ import ( cc "kubevirt.io/containerized-data-importer/pkg/controller/common" ) +// HostClonePhaseName is the name of the host clone phase +const HostClonePhaseName = "HostClone" + // HostClonePhase creates and monitors a dumb clone operation type HostClonePhase struct { Owner client.Object @@ -33,7 +36,7 @@ type HostClonePhase struct { var _ Phase = &HostClonePhase{} -var _ ProgressReporter = &HostClonePhase{} +var _ StatusReporter = &HostClonePhase{} var httpClient *http.Client @@ -43,20 +46,27 @@ func init() { // Name returns the name of the phase func (p *HostClonePhase) Name() string { - return "HostClone" + return HostClonePhaseName } -// Progress returns the progress of the operation as a percentage -func (p *HostClonePhase) Progress(ctx context.Context) (string, error) { +// Status returns the phase status +func (p *HostClonePhase) Status(ctx context.Context) (*PhaseStatus, error) { + result := &PhaseStatus{} pvc := &corev1.PersistentVolumeClaim{} exists, err := getResource(ctx, p.Client, p.Namespace, p.DesiredClaim.Name, pvc) if err != nil { - return "", err + return nil, err + } + + if !exists { + return result, nil } + result.Annotations = pvc.Annotations + podName := pvc.Annotations[cc.AnnCloneSourcePod] - if !exists || podName == "" { - return "", nil + if podName == "" { + return result, nil } args := &cc.ProgressFromClaimArgs{ @@ -70,10 +80,12 @@ func (p *HostClonePhase) Progress(ctx context.Context) (string, error) { progress, err := cc.ProgressFromClaim(ctx, args) if err != nil { - return "", err + return nil, err } - return progress, nil + result.Progress = progress + + return result, nil } // Reconcile creates the desired pvc and waits for the operation to complete @@ -91,7 +103,7 @@ func (p *HostClonePhase) Reconcile(ctx context.Context) (*reconcile.Result, erro } } - if !hostCloneComplete(actualClaim) { + if !p.hostCloneComplete(actualClaim) { // requeue to update status return &reconcile.Result{RequeueAfter: 3 * time.Second}, nil } @@ -108,6 +120,8 @@ func (p *HostClonePhase) createClaim(ctx context.Context) (*corev1.PersistentVol cc.AddAnnotation(claim, cc.AnnPodRestarts, "0") cc.AddAnnotation(claim, cc.AnnCloneRequest, fmt.Sprintf("%s/%s", p.Namespace, p.SourceName)) cc.AddAnnotation(claim, cc.AnnPopulatorKind, cdiv1.VolumeCloneSourceRef) + cc.AddAnnotation(claim, cc.AnnEventSourceKind, p.Owner.GetObjectKind().GroupVersionKind().Kind) + cc.AddAnnotation(claim, cc.AnnEventSource, fmt.Sprintf("%s/%s", p.Owner.GetNamespace(), p.Owner.GetName())) if p.OwnershipLabel != "" { AddOwnershipLabel(p.OwnershipLabel, claim, p.Owner) } @@ -126,6 +140,13 @@ func (p *HostClonePhase) createClaim(ctx context.Context) (*corev1.PersistentVol return claim, nil } -func hostCloneComplete(pvc *corev1.PersistentVolumeClaim) bool { +func (p *HostClonePhase) hostCloneComplete(pvc *corev1.PersistentVolumeClaim) bool { + // this is awfully lame + // both the upload controller and clone controller update the PVC status to succeeded + // but only the clone controller will set the preallocation annotation + // so we have to wait for that + if p.Preallocation && pvc.Annotations[cc.AnnPreallocationApplied] != "true" { + return false + } return pvc.Annotations[cc.AnnPodPhase] == string(cdiv1.Succeeded) } diff --git a/pkg/controller/clone/host-clone_test.go b/pkg/controller/clone/host-clone_test.go index bb040d4520..965dbb9180 100644 --- a/pkg/controller/clone/host-clone_test.go +++ b/pkg/controller/clone/host-clone_test.go @@ -148,6 +148,19 @@ var _ = Describe("HostClonePhase test", func() { Expect(result.RequeueAfter).ToNot(BeZero()) }) + It("should wait for clone to succeed with preallocation", func() { + desired := getCliam() + desired.Annotations[cc.AnnPodPhase] = "Succeeded" + p := creatHostClonePhase(desired) + p.Preallocation = true + + result, err := p.Reconcile(context.Background()) + Expect(err).ToNot(HaveOccurred()) + Expect(result).ToNot(BeNil()) + Expect(result.Requeue).To(BeFalse()) + Expect(result.RequeueAfter).ToNot(BeZero()) + }) + It("should succeed", func() { desired := getCliam() desired.Annotations[cc.AnnPodPhase] = "Succeeded" @@ -157,5 +170,17 @@ var _ = Describe("HostClonePhase test", func() { Expect(err).ToNot(HaveOccurred()) Expect(result).To(BeNil()) }) + + It("should succeed with preallocation", func() { + desired := getCliam() + desired.Annotations[cc.AnnPodPhase] = "Succeeded" + desired.Annotations[cc.AnnPreallocationApplied] = "true" + p := creatHostClonePhase(desired) + p.Preallocation = true + + result, err := p.Reconcile(context.Background()) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(BeNil()) + }) }) }) diff --git a/pkg/controller/clone/planner.go b/pkg/controller/clone/planner.go index 16da4a53b4..9a4d23792b 100644 --- a/pkg/controller/clone/planner.go +++ b/pkg/controller/clone/planner.go @@ -24,7 +24,6 @@ import ( cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" cc "kubevirt.io/containerized-data-importer/pkg/controller/common" - "kubevirt.io/containerized-data-importer/pkg/util" ) const ( @@ -82,9 +81,15 @@ type Phase interface { Reconcile(context.Context) (*reconcile.Result, error) } -// ProgressReporter allows a phase to report progress -type ProgressReporter interface { - Progress(context.Context) (string, error) +// PhaseStatus contains phase status data +type PhaseStatus struct { + Progress string + Annotations map[string]string +} + +// StatusReporter allows a phase to report status +type StatusReporter interface { + Status(context.Context) (*PhaseStatus, error) } // list of all possible (core) types created @@ -416,7 +421,14 @@ func (p *Planner) validateTargetStorageClassAssignment(ctx context.Context, args } func (p *Planner) validateSourcePVC(args *ChooseStrategyArgs, sourceClaim *corev1.PersistentVolumeClaim) error { + _, permissive := args.TargetClaim.Annotations[cc.AnnPermissiveClone] + if permissive { + args.Log.V(3).Info("permissive clone annotation found, skipping size validation") + return nil + } + if err := cc.ValidateRequestedCloneSize(sourceClaim.Spec.Resources, args.TargetClaim.Spec.Resources); err != nil { + p.Recorder.Eventf(args.TargetClaim, corev1.EventTypeWarning, cc.ErrIncompatiblePVC, err.Error()) return err } @@ -459,15 +471,6 @@ func (p *Planner) validateAdvancedClonePVC(ctx context.Context, args *ChooseStra func (p *Planner) planHostAssistedFromPVC(ctx context.Context, args *PlanArgs) ([]Phase, error) { desiredClaim := createDesiredClaim(args.DataSource.Namespace, args.TargetClaim) - if util.ResolveVolumeMode(desiredClaim.Spec.VolumeMode) == corev1.PersistentVolumeFilesystem { - ds := desiredClaim.Spec.Resources.Requests[corev1.ResourceStorage] - is, err := cc.InflateSizeWithOverhead(ctx, p.Client, ds.Value(), &args.TargetClaim.Spec) - if err != nil { - return nil, err - } - desiredClaim.Spec.Resources.Requests[corev1.ResourceStorage] = is - } - hcp := &HostClonePhase{ Owner: args.TargetClaim, Namespace: args.DataSource.Namespace, @@ -536,14 +539,6 @@ func (p *Planner) planHostAssistedFromSnapshot(ctx context.Context, args *PlanAr } desiredClaim := createDesiredClaim(args.DataSource.Namespace, args.TargetClaim) - if util.ResolveVolumeMode(desiredClaim.Spec.VolumeMode) == corev1.PersistentVolumeFilesystem { - ds := desiredClaim.Spec.Resources.Requests[corev1.ResourceStorage] - is, err := cc.InflateSizeWithOverhead(ctx, p.Client, ds.Value(), &args.TargetClaim.Spec) - if err != nil { - return nil, err - } - desiredClaim.Spec.Resources.Requests[corev1.ResourceStorage] = is - } hcp := &HostClonePhase{ Owner: args.TargetClaim, @@ -650,6 +645,7 @@ func (p *Planner) planSnapshotFromPVC(ctx context.Context, args *PlanArgs) ([]Ph OwnershipLabel: p.OwnershipLabel, Client: p.Client, Log: args.Log, + Recorder: p.Recorder, } desiredClaim := createDesiredClaim(args.DataSource.Namespace, args.TargetClaim) diff --git a/pkg/controller/clone/planner_test.go b/pkg/controller/clone/planner_test.go index 0949cc977c..8ea4515aab 100644 --- a/pkg/controller/clone/planner_test.go +++ b/pkg/controller/clone/planner_test.go @@ -325,7 +325,7 @@ var _ = Describe("Planner test", func() { planner := createPlanner(createStorageClass(), source) strategy, err := planner.ChooseStrategy(context.Background(), args) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(Equal("target resources requests storage size is smaller than the source")) + Expect(err.Error()).To(HavePrefix("target resources requests storage size is smaller than the source")) Expect(strategy).To(BeNil()) expectEvent(planner, CloneValidationFailed) }) @@ -635,9 +635,6 @@ var _ = Describe("Planner test", func() { } Expect(hc.ImmediateBind).To(BeTrue()) Expect(hc.OwnershipLabel).To(Equal(planner.OwnershipLabel)) - desiredSize := hc.DesiredClaim.Spec.Resources.Requests[corev1.ResourceStorage] - requestedSize := args.TargetClaim.Spec.Resources.Requests[corev1.ResourceStorage] - Expect(desiredSize.Cmp(requestedSize)).To(Equal(1)) } validateRebindPhase := func(planner *Planner, args *PlanArgs, p Phase) { diff --git a/pkg/controller/clone/prep-claim.go b/pkg/controller/clone/prep-claim.go index cf4a940fef..9fb0cbd44a 100644 --- a/pkg/controller/clone/prep-claim.go +++ b/pkg/controller/clone/prep-claim.go @@ -16,6 +16,9 @@ import ( cc "kubevirt.io/containerized-data-importer/pkg/controller/common" ) +// PrepClaimPhaseName is the name of the prep claim phase +const PrepClaimPhaseName = "PrepClaim" + // PrepClaimPhase is responsible for prepping a PVC for rebind type PrepClaimPhase struct { Owner client.Object @@ -33,7 +36,7 @@ var _ Phase = &PrepClaimPhase{} // Name returns the name of the phase func (p *PrepClaimPhase) Name() string { - return "PrepClaim" + return PrepClaimPhaseName } // Reconcile ensures that a pvc is bound and resized if necessary diff --git a/pkg/controller/clone/rebind.go b/pkg/controller/clone/rebind.go index 8e8588dad2..4df4174ea6 100644 --- a/pkg/controller/clone/rebind.go +++ b/pkg/controller/clone/rebind.go @@ -13,6 +13,9 @@ import ( cc "kubevirt.io/containerized-data-importer/pkg/controller/common" ) +// RebindPhaseName is the name of the rebind phase +const RebindPhaseName = "Rebind" + // RebindPhase binds a PV from one PVC to another type RebindPhase struct { SourceNamespace string @@ -28,7 +31,7 @@ var _ Phase = &RebindPhase{} // Name returns the name of the phase func (p *RebindPhase) Name() string { - return "Rebind" + return RebindPhaseName } // Reconcile rebinds a PV diff --git a/pkg/controller/clone/snap-clone.go b/pkg/controller/clone/snap-clone.go index 4e6bc4ae02..d34f9398b1 100644 --- a/pkg/controller/clone/snap-clone.go +++ b/pkg/controller/clone/snap-clone.go @@ -16,6 +16,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" ) +// SnapshotClonePhaseName is the name of the snapshot clone phase +const SnapshotClonePhaseName = "SnapshotClone" + // SnapshotClonePhase waits for a snapshot to be ready and creates a PVC from it type SnapshotClonePhase struct { Owner client.Object @@ -32,7 +35,7 @@ var _ Phase = &SnapshotClonePhase{} // Name returns the name of the phase func (p *SnapshotClonePhase) Name() string { - return "SnapshotClone" + return SnapshotClonePhaseName } // Reconcile ensures a snapshot is created correctly diff --git a/pkg/controller/clone/snapshot.go b/pkg/controller/clone/snapshot.go index d1e94ccc49..92285611bd 100644 --- a/pkg/controller/clone/snapshot.go +++ b/pkg/controller/clone/snapshot.go @@ -7,10 +7,14 @@ import ( "github.com/go-logr/logr" snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) +// SnapshotPhaseName is the name of the snapshot phase +const SnapshotPhaseName = "Snapshot" + // SnapshotPhase snapshots a PVC type SnapshotPhase struct { Owner client.Object @@ -21,13 +25,14 @@ type SnapshotPhase struct { OwnershipLabel string Client client.Client Log logr.Logger + Recorder record.EventRecorder } var _ Phase = &SnapshotPhase{} // Name returns the name of the phase func (p *SnapshotPhase) Name() string { - return "Snapshot" + return SnapshotPhaseName } // Reconcile ensures a snapshot is created correctly @@ -39,7 +44,16 @@ func (p *SnapshotPhase) Reconcile(ctx context.Context) (*reconcile.Result, error } if !exists { - ready, err := IsSourceClaimReady(ctx, p.Client, p.SourceNamespace, p.SourceName) + args := &IsSourceClaimReadyArgs{ + Target: p.Owner, + SourceNamespace: p.SourceNamespace, + SourceName: p.SourceName, + Client: p.Client, + Log: p.Log, + Recorder: p.Recorder, + } + + ready, err := IsSourceClaimReady(ctx, args) if err != nil { return nil, err } diff --git a/pkg/controller/common/BUILD.bazel b/pkg/controller/common/BUILD.bazel index 4e59a426b3..abb4dd39cd 100644 --- a/pkg/controller/common/BUILD.bazel +++ b/pkg/controller/common/BUILD.bazel @@ -30,6 +30,7 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", + "//vendor/k8s.io/client-go/tools/cache:go_default_library", "//vendor/k8s.io/client-go/tools/record:go_default_library", "//vendor/k8s.io/klog/v2:go_default_library", "//vendor/k8s.io/utils/pointer:go_default_library", diff --git a/pkg/controller/common/util.go b/pkg/controller/common/util.go index ca9e034a71..7f12768ebb 100644 --- a/pkg/controller/common/util.go +++ b/pkg/controller/common/util.go @@ -46,6 +46,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" "k8s.io/klog/v2" "k8s.io/utils/pointer" @@ -58,7 +59,7 @@ import ( "kubevirt.io/containerized-data-importer/pkg/token" "kubevirt.io/containerized-data-importer/pkg/util" sdkapi "kubevirt.io/controller-lifecycle-operator-sdk/api" - "sigs.k8s.io/controller-runtime/pkg/cache" + runtimecache "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -292,6 +293,11 @@ const ( // ProgressDone this means we are DONE ProgressDone = "100.0%" + + // AnnEventSourceKind is the source kind that should be related to events + AnnEventSourceKind = "cdi.kubevirt.io/events.source.kind" + // AnnEventSource is the source that should be related to events (namespace/name) + AnnEventSource = "cdi.kubevirt.io/events.source" ) // Size-detection pod error codes @@ -341,6 +347,62 @@ func (v *FakeValidator) Validate(value string) (*token.Payload, error) { }, nil } +// MultiTokenValidator is a token validator that can validate both short and long tokens +type MultiTokenValidator struct { + ShortTokenValidator token.Validator + LongTokenValidator token.Validator +} + +// ValidatePVC validates a PVC +func (mtv *MultiTokenValidator) ValidatePVC(source, target *corev1.PersistentVolumeClaim) error { + tok, v := mtv.getTokenAndValidator(target) + return ValidateCloneTokenPVC(tok, v, source, target) +} + +// ValidatePopulator valades a token for a populator +func (mtv *MultiTokenValidator) ValidatePopulator(vcs *cdiv1.VolumeCloneSource, pvc *corev1.PersistentVolumeClaim) error { + if vcs.Namespace == pvc.Namespace { + return nil + } + + tok, v := mtv.getTokenAndValidator(pvc) + + tokenData, err := v.Validate(tok) + if err != nil { + return errors.Wrap(err, "error verifying token") + } + + var tokenResourceName string + switch vcs.Spec.Source.Kind { + case "PersistentVolumeClaim": + tokenResourceName = "persistentvolumeclaims" + case "VolumeSnapshot": + tokenResourceName = "volumesnapshots" + } + srcName := vcs.Spec.Source.Name + + return validateTokenData(tokenData, vcs.Namespace, srcName, pvc.Namespace, pvc.Name, string(pvc.UID), tokenResourceName) +} + +func (mtv *MultiTokenValidator) getTokenAndValidator(pvc *corev1.PersistentVolumeClaim) (string, token.Validator) { + v := mtv.LongTokenValidator + tok, ok := pvc.Annotations[AnnExtendedCloneToken] + if !ok { + // if token doesn't exist, no prob for same namespace + tok = pvc.Annotations[AnnCloneToken] + v = mtv.ShortTokenValidator + } + return tok, v +} + +// NewMultiTokenValidator returns a new multi token validator +func NewMultiTokenValidator(key *rsa.PublicKey) *MultiTokenValidator { + return &MultiTokenValidator{ + ShortTokenValidator: NewCloneTokenValidator(common.CloneTokenIssuer, key), + LongTokenValidator: NewCloneTokenValidator(common.ExtendedCloneTokenIssuer, key), + } +} + // NewCloneTokenValidator returns a new token validator func NewCloneTokenValidator(issuer string, key *rsa.PublicKey) token.Validator { return token.NewValidator(issuer, key, cloneTokenLeeway) @@ -648,7 +710,7 @@ func ValidateCloneTokenPVC(t string, v token.Validator, source, target *corev1.P // ValidateCloneTokenDV validates clone token for DV func ValidateCloneTokenDV(validator token.Validator, dv *cdiv1.DataVolume) error { - sourceName, sourceNamespace := GetCloneSourceNameAndNamespace(dv) + _, sourceName, sourceNamespace := GetCloneSourceInfo(dv) if sourceNamespace == "" || sourceNamespace == dv.Namespace { return nil } @@ -900,7 +962,7 @@ func ValidateRequestedCloneSize(sourceResources corev1.ResourceRequirements, tar // Verify that the target PVC size is equal or larger than the source. if sourceRequest.Value() > targetRequest.Value() { - return errors.New("target resources requests storage size is smaller than the source") + return errors.Errorf("target resources requests storage size is smaller than the source %d < %d", targetRequest.Value(), sourceRequest.Value()) } return nil } @@ -1206,7 +1268,7 @@ func IsErrCacheNotStarted(err error) bool { if err == nil { return false } - _, ok := err.(*cache.ErrCacheNotStarted) + _, ok := err.(*runtimecache.ErrCacheNotStarted) return ok } @@ -1243,19 +1305,20 @@ func NewImportDataVolume(name string) *cdiv1.DataVolume { } } -// GetCloneSourceNameAndNamespace returns the name and namespace of the cloning source -func GetCloneSourceNameAndNamespace(dv *cdiv1.DataVolume) (name, namespace string) { - var sourceName, sourceNamespace string +// GetCloneSourceInfo returns the type, name and namespace of the cloning source +func GetCloneSourceInfo(dv *cdiv1.DataVolume) (sourceType, sourceName, sourceNamespace string) { // Cloning sources are mutually exclusive if dv.Spec.Source.PVC != nil { + sourceType = "pvc" sourceName = dv.Spec.Source.PVC.Name sourceNamespace = dv.Spec.Source.PVC.Namespace } else if dv.Spec.Source.Snapshot != nil { + sourceType = "snapshot" sourceName = dv.Spec.Source.Snapshot.Name sourceNamespace = dv.Spec.Source.Snapshot.Namespace } - return sourceName, sourceNamespace + return } // IsWaitForFirstConsumerEnabled tells us if we should respect "real" WFFC behavior or just let our worker pods randomly spawn @@ -1758,3 +1821,46 @@ func GetResource(ctx context.Context, c client.Client, namespace, name string, o return true, nil } + +// PatchArgs are the args for Patch +type PatchArgs struct { + Client client.Client + Log logr.Logger + Obj client.Object + OldObj client.Object +} + +// GetAnnotatedEventSource returns resource referenced by AnnEventSource annotations +func GetAnnotatedEventSource(ctx context.Context, c client.Client, obj client.Object) (client.Object, error) { + esk, ok := obj.GetAnnotations()[AnnEventSourceKind] + if !ok { + return obj, nil + } + if esk != "PersistentVolumeClaim" { + return obj, nil + } + es, ok := obj.GetAnnotations()[AnnEventSource] + if !ok { + return obj, nil + } + namespace, name, err := cache.SplitMetaNamespaceKey(es) + if err != nil { + return nil, err + } + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: name, + }, + } + if err := c.Get(ctx, client.ObjectKeyFromObject(pvc), pvc); err != nil { + return nil, err + } + return pvc, nil +} + +// OwnedByDataVolume returns true if the object is owned by a DataVolume +func OwnedByDataVolume(obj metav1.Object) bool { + owner := metav1.GetControllerOf(obj) + return owner != nil && owner.Kind == "DataVolume" +} diff --git a/pkg/controller/datavolume/BUILD.bazel b/pkg/controller/datavolume/BUILD.bazel index 4d62031395..b56868a8fc 100644 --- a/pkg/controller/datavolume/BUILD.bazel +++ b/pkg/controller/datavolume/BUILD.bazel @@ -10,7 +10,6 @@ go_library( "garbagecollect.go", "import-controller.go", "pvc-clone-controller.go", - "smart-clone-controller.go", "snapshot-clone-controller.go", "upload-controller.go", "util.go", @@ -19,6 +18,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/common:go_default_library", + "//pkg/controller/clone:go_default_library", "//pkg/controller/common:go_default_library", "//pkg/controller/populators:go_default_library", "//pkg/feature-gates:go_default_library", @@ -43,6 +43,7 @@ go_library( "//vendor/k8s.io/client-go/tools/cache:go_default_library", "//vendor/k8s.io/client-go/tools/record:go_default_library", "//vendor/k8s.io/component-helpers/storage/volume:go_default_library", + "//vendor/k8s.io/utils/pointer:go_default_library", "//vendor/sigs.k8s.io/controller-runtime/pkg/client:go_default_library", "//vendor/sigs.k8s.io/controller-runtime/pkg/controller:go_default_library", "//vendor/sigs.k8s.io/controller-runtime/pkg/controller/controllerutil:go_default_library", @@ -63,7 +64,6 @@ go_test( "external-population-controller_test.go", "import-controller_test.go", "pvc-clone-controller_test.go", - "smart-clone-controller_test.go", "snapshot-clone-controller_test.go", "static-volume_test.go", "upload-controller_test.go", @@ -72,7 +72,9 @@ go_test( embed = [":go_default_library"], deps = [ "//pkg/common:go_default_library", + "//pkg/controller/clone:go_default_library", "//pkg/controller/common:go_default_library", + "//pkg/controller/populators:go_default_library", "//pkg/feature-gates:go_default_library", "//pkg/token:go_default_library", "//staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1:go_default_library", @@ -92,6 +94,7 @@ go_test( "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", "//vendor/k8s.io/client-go/kubernetes/scheme:go_default_library", "//vendor/k8s.io/client-go/tools/record:go_default_library", + "//vendor/k8s.io/utils/pointer:go_default_library", "//vendor/sigs.k8s.io/controller-runtime/pkg/client:go_default_library", "//vendor/sigs.k8s.io/controller-runtime/pkg/client/fake:go_default_library", "//vendor/sigs.k8s.io/controller-runtime/pkg/log:go_default_library", diff --git a/pkg/controller/datavolume/clone-controller-base.go b/pkg/controller/datavolume/clone-controller-base.go index e610ef0b85..8c424e9ba8 100644 --- a/pkg/controller/datavolume/clone-controller-base.go +++ b/pkg/controller/datavolume/clone-controller-base.go @@ -20,28 +20,26 @@ import ( "context" "fmt" - "github.com/go-logr/logr" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - - cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" - "kubevirt.io/containerized-data-importer/pkg/common" - - cc "kubevirt.io/containerized-data-importer/pkg/controller/common" - "kubevirt.io/containerized-data-importer/pkg/token" - "kubevirt.io/containerized-data-importer/pkg/util" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" + + cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" + "kubevirt.io/containerized-data-importer/pkg/controller/clone" + cc "kubevirt.io/containerized-data-importer/pkg/controller/common" + "kubevirt.io/containerized-data-importer/pkg/controller/populators" ) const ( @@ -58,16 +56,8 @@ const ( CloneFromSnapshotSourceInProgress = "CloneFromSnapshotSourceInProgress" // SnapshotForSmartCloneCreated provides a const to indicate snapshot creation for smart-clone has been completed SnapshotForSmartCloneCreated = "SnapshotForSmartCloneCreated" - // SmartClonePVCInProgress provides a const to indicate snapshot creation for smart-clone is in progress - SmartClonePVCInProgress = "SmartClonePVCInProgress" - // SmartCloneSourceInUse provides a const to indicate a smart clone is being delayed because the source is in use - SmartCloneSourceInUse = "SmartCloneSourceInUse" // CSICloneInProgress provides a const to indicate csi volume clone is in progress CSICloneInProgress = "CSICloneInProgress" - // CSICloneSourceInUse provides a const to indicate a csi volume clone is being delayed because the source is in use - CSICloneSourceInUse = "CSICloneSourceInUse" - // HostAssistedCloneSourceInUse provides a const to indicate a host-assisted clone is being delayed because the source is in use - HostAssistedCloneSourceInUse = "HostAssistedCloneSourceInUse" // CloneFailed provides a const to indicate clone has failed CloneFailed = "CloneFailed" // CloneSucceeded provides a const to indicate clone has succeeded @@ -84,9 +74,7 @@ const ( // MessageSmartCloneInProgress provides a const to form snapshot for smart-clone is in progress message MessageSmartCloneInProgress = "Creating snapshot for smart-clone is in progress (for pvc %s/%s)" // MessageCloneFromSnapshotSourceInProgress provides a const to form clone from snapshot source is in progress message - MessageCloneFromSnapshotSourceInProgress = "Creating PVC from snapshot source is in progress (for snapshot %s/%s)" - // MessageSmartClonePVCInProgress provides a const to form snapshot for smart-clone is in progress message - MessageSmartClonePVCInProgress = "Creating PVC for smart-clone is in progress (for pvc %s/%s)" + MessageCloneFromSnapshotSourceInProgress = "Creating PVC from snapshot source is in progress (for %s %s/%s)" // MessageCsiCloneInProgress provides a const to form a CSI Volume Clone in progress message MessageCsiCloneInProgress = "CSI Volume clone in progress (for pvc %s/%s)" @@ -118,6 +106,14 @@ const ( CloneWithoutSource = "CloneWithoutSource" // MessageCloneWithoutSource reports that the source of a clone doesn't exists (message) MessageCloneWithoutSource = "The source %s %s doesn't exist" + // PrepClaimInProgress is const representing target PVC prep + PrepClaimInProgress = "PrepClaimInProgress" + // MessagePrepClaimInProgress is a const for reporting target prep + MessagePrepClaimInProgress = "Prepping PersistentVolumeClaim for DataVolume %s/%s" + // RebindInProgress is const representing target PVC rebind + RebindInProgress = "RebindInProgress" + // MessageRebindInProgress is a const for reporting target rebind + MessageRebindInProgress = "Rebinding PersistentVolumeClaim for DataVolume %s/%s" // AnnCSICloneRequest annotation associates object with CSI Clone Request AnnCSICloneRequest = "cdi.kubevirt.io/CSICloneRequest" @@ -129,381 +125,235 @@ const ( AnnSourceCapacity = "cdi.Kubervirt.io/sourceCapacity" crossNamespaceFinalizer = "cdi.kubevirt.io/dataVolumeFinalizer" - - annReadyForTransfer = "cdi.kubevirt.io/readyForTransfer" ) // CloneReconcilerBase members type CloneReconcilerBase struct { ReconcilerBase - clonerImage string - importerImage string - pullPolicy string - tokenValidator token.Validator - tokenGenerator token.Generator + clonerImage string + importerImage string + pullPolicy string + cloneSourceAPIGroup *string + cloneSourceKind string + shortTokenValidator token.Validator + longTokenValidator token.Validator + tokenGenerator token.Generator } -func (r *CloneReconcilerBase) ensureExtendedToken(pvc *corev1.PersistentVolumeClaim) error { - _, ok := pvc.Annotations[cc.AnnExtendedCloneToken] - if ok { - return nil - } +func (r *CloneReconcilerBase) addVolumeCloneSourceWatch(datavolumeController controller.Controller) error { + return datavolumeController.Watch(&source.Kind{Type: &cdiv1.VolumeCloneSource{}}, handler.EnqueueRequestsFromMapFunc( + func(obj client.Object) []reconcile.Request { + var err error + var hasDataVolumeOwner bool + var ownerNamespace, ownerName string + ownerRef := metav1.GetControllerOf(obj) + if ownerRef != nil && ownerRef.Kind == "DataVolume" { + hasDataVolumeOwner = true + ownerNamespace = obj.GetNamespace() + ownerName = ownerRef.Name + } else if hasAnnOwnedByDataVolume(obj) { + hasDataVolumeOwner = true + ownerNamespace, ownerName, err = getAnnOwnedByDataVolume(obj) + if err != nil { + return nil + } + } + if !hasDataVolumeOwner { + return nil + } + dv := &cdiv1.DataVolume{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ownerNamespace, + Name: ownerName, + }, + } + if err = r.client.Get(context.TODO(), client.ObjectKeyFromObject(dv), dv); err != nil { + r.log.Info("Failed to get DataVolume", "error", err) + return nil + } + if err := r.populateSourceIfSourceRef(dv); err != nil { + r.log.Info("Failed to check DataSource", "error", err) + return nil + } + if (r.cloneSourceKind == "PersistentVolumeClaim" && dv.Spec.Source.PVC != nil) || + (r.cloneSourceKind == "VolumeSnapshot" && dv.Spec.Source.Snapshot != nil) { + return []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: ownerNamespace, Name: ownerName}}} + } + return nil + }), + ) +} - token, ok := pvc.Annotations[cc.AnnCloneToken] - if !ok { - return fmt.Errorf("token missing") +func (r *CloneReconcilerBase) updatePVCForPopulation(dataVolume *cdiv1.DataVolume, pvc *corev1.PersistentVolumeClaim) error { + if dataVolume.Spec.Source.PVC == nil && dataVolume.Spec.Source.Snapshot == nil { + return errors.Errorf("no source set for clone datavolume") } - - payload, err := r.tokenValidator.Validate(token) - if err != nil { + if err := addCloneToken(dataVolume, pvc); err != nil { return err } - - if payload.Params == nil { - payload.Params = make(map[string]string) - } - payload.Params["uid"] = string(pvc.UID) - - newToken, err := r.tokenGenerator.Generate(payload) - if err != nil { - return err + if isCrossNamespaceClone(dataVolume) { + _, _, sourcNamespace := cc.GetCloneSourceInfo(dataVolume) + cc.AddAnnotation(pvc, populators.AnnDataSourceNamespace, sourcNamespace) } - - pvc.Annotations[cc.AnnExtendedCloneToken] = newToken - - if err := r.updatePVC(pvc); err != nil { - return err + apiGroup := cc.AnnAPIGroup + pvc.Spec.DataSourceRef = &corev1.TypedObjectReference{ + APIGroup: &apiGroup, + Kind: cdiv1.VolumeCloneSourceRef, + Name: volumeCloneSourceName(dataVolume), } - return nil } -// When the clone is finished some additional actions may be applied -// like namespaceTransfer Cleanup or size expansion -func (r *CloneReconcilerBase) finishClone(log logr.Logger, syncState *dvSyncState, transferName string) (reconcile.Result, error) { - //DO Nothing, not yet ready - if syncState.pvc.Annotations[cc.AnnCloneOf] != "true" { - return reconcile.Result{}, nil +func (r *CloneReconcilerBase) ensureExtendedTokenDV(dv *cdiv1.DataVolume) (bool, error) { + if !isCrossNamespaceClone(dv) { + return false, nil } - // expand for non-namespace case - return r.expandPvcAfterClone(log, syncState, syncState.pvc, cdiv1.Succeeded, false) -} - -func (r *CloneReconcilerBase) setCloneOfOnPvc(pvc *corev1.PersistentVolumeClaim) error { - if v, ok := pvc.Annotations[cc.AnnCloneOf]; !ok || v != "true" { - if pvc.Annotations == nil { - pvc.Annotations = make(map[string]string) - } - pvc.Annotations[cc.AnnCloneOf] = "true" - - return r.updatePVC(pvc) + _, ok := dv.Annotations[cc.AnnExtendedCloneToken] + if ok { + return false, nil } - return nil -} - -// temporary pvc is used when the clone src and tgt are in two distinct namespaces -func (r *CloneReconcilerBase) expandPvcAfterClone( - log logr.Logger, - syncState *dvSyncState, - pvc *corev1.PersistentVolumeClaim, - nextPhase cdiv1.DataVolumePhase, - isTemp bool) (reconcile.Result, error) { - - done, err := r.expand(log, syncState, pvc) - if err != nil { - return reconcile.Result{}, err + token, ok := dv.Annotations[cc.AnnCloneToken] + if !ok { + return false, fmt.Errorf("token missing") } - if !done { - return reconcile.Result{Requeue: true}, - r.syncCloneStatusPhase(syncState, cdiv1.ExpansionInProgress, pvc) + payload, err := r.shortTokenValidator.Validate(token) + if err != nil { + return false, err } - if isTemp { - // trigger transfer and next reconcile should have pvcExists == true - pvc.Annotations[annReadyForTransfer] = "true" - pvc.Annotations[cc.AnnPopulatedFor] = syncState.dvMutated.Name - if err := r.updatePVC(pvc); err != nil { - return reconcile.Result{}, err - } + if payload.Params == nil { + payload.Params = make(map[string]string) } + payload.Params["uid"] = string(dv.UID) - return reconcile.Result{}, r.syncCloneStatusPhase(syncState, nextPhase, pvc) -} - -func (r *CloneReconcilerBase) doCrossNamespaceClone(log logr.Logger, - syncState *dvSyncState, - pvcName, sourceNamespace string, - returnWhenCloneInProgress bool, - selectedCloneStrategy cloneStrategy) (*reconcile.Result, error) { - - initialized, err := r.initTransfer(log, syncState, pvcName, sourceNamespace) + newToken, err := r.tokenGenerator.Generate(payload) if err != nil { - return &reconcile.Result{}, err + return false, err } - // get reconciled again v soon - if !initialized { - return &reconcile.Result{}, r.syncCloneStatusPhase(syncState, cdiv1.CloneScheduled, nil) - } + dv.Annotations[cc.AnnExtendedCloneToken] = newToken - tmpPVC := &corev1.PersistentVolumeClaim{} - nn := types.NamespacedName{Namespace: sourceNamespace, Name: pvcName} - if err := r.client.Get(context.TODO(), nn, tmpPVC); err != nil { - if !k8serrors.IsNotFound(err) { - return &reconcile.Result{}, err - } - } else if tmpPVC.Annotations[cc.AnnCloneOf] == "true" { - result, err := r.expandPvcAfterClone(log, syncState, tmpPVC, cdiv1.NamespaceTransferInProgress, true) - return &result, err - } else { - // AnnCloneOf != true, so cloneInProgress - if returnWhenCloneInProgress { - return &reconcile.Result{}, nil - } - } - - return nil, nil + return true, nil } -func (r *CloneReconcilerBase) initTransfer(log logr.Logger, syncState *dvSyncState, name, namespace string) (bool, error) { - initialized := true - dv := syncState.dvMutated - - log.Info("Initializing transfer") - - if !cc.HasFinalizer(dv, crossNamespaceFinalizer) { - cc.AddFinalizer(dv, crossNamespaceFinalizer) - initialized = false +func (r *CloneReconcilerBase) ensureExtendedTokenPVC(dv *cdiv1.DataVolume, pvc *corev1.PersistentVolumeClaim) error { + if !isCrossNamespaceClone(dv) { + return nil } - ot := &cdiv1.ObjectTransfer{} - if err := r.client.Get(context.TODO(), types.NamespacedName{Name: name}, ot); err != nil { - if !k8serrors.IsNotFound(err) { - return false, err - } - - if err := cc.ValidateCloneTokenDV(r.tokenValidator, dv); err != nil { - return false, err - } - - ot = &cdiv1.ObjectTransfer{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Labels: map[string]string{ - common.CDILabelKey: common.CDILabelValue, - common.CDIComponentLabel: "", - }, - }, - Spec: cdiv1.ObjectTransferSpec{ - Source: cdiv1.TransferSource{ - Kind: "PersistentVolumeClaim", - Namespace: namespace, - Name: name, - RequiredAnnotations: map[string]string{ - annReadyForTransfer: "true", - }, - }, - Target: cdiv1.TransferTarget{ - Namespace: &dv.Namespace, - Name: &dv.Name, - }, - }, - } - util.SetRecommendedLabels(ot, r.installerLabels, "cdi-controller") - - if err := setAnnOwnedByDataVolume(ot, dv); err != nil { - return false, err - } - - if err := r.client.Create(context.TODO(), ot); err != nil { - return false, err - } - - initialized = false + _, ok := pvc.Annotations[cc.AnnExtendedCloneToken] + if ok { + return nil } - return initialized, nil -} - -func expansionPodName(pvc *corev1.PersistentVolumeClaim) string { - return "cdi-expand-" + string(pvc.UID) -} - -func (r *CloneReconcilerBase) expand(log logr.Logger, syncState *dvSyncState, pvc *corev1.PersistentVolumeClaim) (bool, error) { - if pvc.Status.Phase != corev1.ClaimBound { - return false, fmt.Errorf("cannot expand volume in %q phase", pvc.Status.Phase) + token, ok := dv.Annotations[cc.AnnExtendedCloneToken] + if !ok { + return fmt.Errorf("token missing") } - requestedSize, hasRequested := syncState.pvcSpec.Resources.Requests[corev1.ResourceStorage] - currentSize, hasCurrent := pvc.Spec.Resources.Requests[corev1.ResourceStorage] - actualSize, hasActual := pvc.Status.Capacity[corev1.ResourceStorage] - if !hasRequested || !hasCurrent || !hasActual { - return false, fmt.Errorf("PVC sizes missing") + payload, err := r.longTokenValidator.Validate(token) + if err != nil { + return err } - expansionRequired := actualSize.Cmp(requestedSize) < 0 - updateRequestSizeRequired := currentSize.Cmp(requestedSize) < 0 - - log.V(3).Info("Expand sizes", "req", requestedSize, "cur", currentSize, "act", actualSize, "exp", expansionRequired) - - if updateRequestSizeRequired { - pvc.Spec.Resources.Requests[corev1.ResourceStorage] = requestedSize - if err := r.updatePVC(pvc); err != nil { - return false, err - } - - return false, nil + if payload.Params["uid"] != string(dv.UID) { + return fmt.Errorf("token uid mismatch") } - podName := expansionPodName(pvc) - podExists := true - pod := &corev1.Pod{} - nn := types.NamespacedName{Namespace: pvc.Namespace, Name: podName} - if err := r.client.Get(context.TODO(), nn, pod); err != nil { - if !k8serrors.IsNotFound(err) { - return false, err - } + // now use pvc uid + payload.Params["uid"] = string(pvc.UID) - podExists = false + newToken, err := r.tokenGenerator.Generate(payload) + if err != nil { + return err } - if !expansionRequired && !podExists { - // finally done - return true, nil - } + pvc.Annotations[cc.AnnExtendedCloneToken] = newToken - hasPendingResizeCondition := false - for _, cond := range pvc.Status.Conditions { - if cond.Type == corev1.PersistentVolumeClaimFileSystemResizePending { - hasPendingResizeCondition = true - break - } + if err := r.updatePVC(pvc); err != nil { + return err } - if !podExists && !hasPendingResizeCondition { - // wait for resize condition - return false, nil - } + return nil +} - if !podExists { - var err error - pod, err = r.createExpansionPod(pvc, syncState.dvMutated, podName) - // Check if pod has failed and, in that case, record an event with the error - if podErr := cc.HandleFailedPod(err, podName, pvc, r.recorder, r.client); podErr != nil { - return false, podErr - } +func (r *CloneReconcilerBase) reconcileVolumeCloneSourceCR(syncState *dvSyncState) error { + dv := syncState.dvMutated + volumeCloneSource := &cdiv1.VolumeCloneSource{} + volumeCloneSourceName := volumeCloneSourceName(dv) + _, sourceName, sourceNamespace := cc.GetCloneSourceInfo(dv) + deletedOrSucceeded := dv.DeletionTimestamp != nil || dv.Status.Phase == cdiv1.Succeeded + exists, err := cc.GetResource(context.TODO(), r.client, sourceNamespace, volumeCloneSourceName, volumeCloneSource) + if err != nil { + return err } - if pod.Status.Phase == corev1.PodSucceeded { - if err := r.client.Delete(context.TODO(), pod); err != nil { - if k8serrors.IsNotFound(err) { - return true, err + if deletedOrSucceeded || exists { + if deletedOrSucceeded && exists { + if err := r.client.Delete(context.TODO(), volumeCloneSource); err != nil { + if !k8serrors.IsNotFound(err) { + return err + } } - - return false, err } - return false, nil - } - - return false, nil -} - -func (r *CloneReconcilerBase) createExpansionPod(pvc *corev1.PersistentVolumeClaim, dv *cdiv1.DataVolume, podName string) (*corev1.Pod, error) { - resourceRequirements, err := cc.GetDefaultPodResourceRequirements(r.client) - if err != nil { - return nil, err - } - - imagePullSecrets, err := cc.GetImagePullSecrets(r.client) - if err != nil { - return nil, err - } + if deletedOrSucceeded { + cc.RemoveFinalizer(dv, crossNamespaceFinalizer) + } - workloadNodePlacement, err := cc.GetWorkloadNodePlacement(context.TODO(), r.client) - if err != nil { - return nil, err + return nil } - pod := &corev1.Pod{ + volumeCloneSource = &cdiv1.VolumeCloneSource{ ObjectMeta: metav1.ObjectMeta{ - Name: podName, - Namespace: pvc.Namespace, - Annotations: map[string]string{ - cc.AnnCreatedBy: "yes", - }, - Labels: map[string]string{ - common.CDILabelKey: common.CDILabelValue, - common.CDIComponentLabel: "cdi-expander", - }, + Name: volumeCloneSourceName, + Namespace: sourceNamespace, }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "dummy", - Image: r.clonerImage, - ImagePullPolicy: corev1.PullPolicy(r.pullPolicy), - Command: []string{"/bin/bash"}, - Args: []string{"-c", "echo", "'hello cdi'"}, - }, + Spec: cdiv1.VolumeCloneSourceSpec{ + Source: corev1.TypedLocalObjectReference{ + APIGroup: r.cloneSourceAPIGroup, + Kind: r.cloneSourceKind, + Name: sourceName, }, - ImagePullSecrets: imagePullSecrets, - RestartPolicy: corev1.RestartPolicyOnFailure, - Volumes: []corev1.Volume{ - { - Name: cc.DataVolName, - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: pvc.Name, - }, - }, - }, - }, - NodeSelector: workloadNodePlacement.NodeSelector, - Tolerations: workloadNodePlacement.Tolerations, - Affinity: workloadNodePlacement.Affinity, + Preallocation: dv.Spec.Preallocation, }, } - util.SetRecommendedLabels(pod, r.installerLabels, "cdi-controller") - - if pvc.Spec.VolumeMode != nil && *pvc.Spec.VolumeMode == corev1.PersistentVolumeBlock { - pod.Spec.Containers[0].VolumeDevices = cc.AddVolumeDevices() - } else { - pod.Spec.Containers[0].VolumeMounts = []corev1.VolumeMount{ - { - Name: cc.DataVolName, - MountPath: common.ClonerMountPath, - }, - } - } - if resourceRequirements != nil { - pod.Spec.Containers[0].Resources = *resourceRequirements + if dv.Spec.PriorityClassName != "" { + volumeCloneSource.Spec.PriorityClassName = &dv.Spec.PriorityClassName } - if err := setAnnOwnedByDataVolume(pod, dv); err != nil { - return nil, err + if sourceNamespace == dv.Namespace { + if err := controllerutil.SetControllerReference(dv, volumeCloneSource, r.scheme); err != nil { + return err + } + } else { + if err := setAnnOwnedByDataVolume(volumeCloneSource, dv); err != nil { + return err + } } - cc.SetRestrictedSecurityContext(&pod.Spec) - - if err := r.client.Create(context.TODO(), pod); err != nil { + if err := r.client.Create(context.TODO(), volumeCloneSource); err != nil { if !k8serrors.IsAlreadyExists(err) { - return nil, err + return err } } - return pod, nil + return nil } func (r *CloneReconcilerBase) syncCloneStatusPhase(syncState *dvSyncState, phase cdiv1.DataVolumePhase, pvc *corev1.PersistentVolumeClaim) error { var event Event dataVolume := syncState.dvMutated - sourceName, sourceNamespace := cc.GetCloneSourceNameAndNamespace(dataVolume) + r.setEventForPhase(dataVolume, phase, &event) + return r.syncDataVolumeStatusPhaseWithEvent(syncState, phase, pvc, event) +} +func (r *CloneReconcilerBase) setEventForPhase(dataVolume *cdiv1.DataVolume, phase cdiv1.DataVolumePhase, event *Event) { + sourceType, sourceName, sourceNamespace := cc.GetCloneSourceInfo(dataVolume) switch phase { case cdiv1.CloneScheduled: event.eventType = corev1.EventTypeNormal @@ -516,29 +366,72 @@ func (r *CloneReconcilerBase) syncCloneStatusPhase(syncState *dvSyncState, phase case cdiv1.CloneFromSnapshotSourceInProgress: event.eventType = corev1.EventTypeNormal event.reason = CloneFromSnapshotSourceInProgress - event.message = fmt.Sprintf(MessageCloneFromSnapshotSourceInProgress, sourceNamespace, sourceName) + event.message = fmt.Sprintf(MessageCloneFromSnapshotSourceInProgress, sourceType, sourceNamespace, sourceName) case cdiv1.CSICloneInProgress: event.eventType = corev1.EventTypeNormal - event.reason = string(cdiv1.CSICloneInProgress) + event.reason = CSICloneInProgress event.message = fmt.Sprintf(MessageCsiCloneInProgress, sourceNamespace, sourceName) - case cdiv1.ExpansionInProgress: - event.eventType = corev1.EventTypeNormal - event.reason = ExpansionInProgress - event.message = fmt.Sprintf(MessageExpansionInProgress, dataVolume.Namespace, dataVolume.Name) - case cdiv1.NamespaceTransferInProgress: - event.eventType = corev1.EventTypeNormal - event.reason = NamespaceTransferInProgress - event.message = fmt.Sprintf(MessageNamespaceTransferInProgress, dataVolume.Namespace, dataVolume.Name) case cdiv1.Succeeded: event.eventType = corev1.EventTypeNormal event.reason = CloneSucceeded event.message = fmt.Sprintf(MessageCloneSucceeded, sourceNamespace, sourceName, dataVolume.Namespace, dataVolume.Name) + case cdiv1.CloneInProgress: + event.eventType = corev1.EventTypeNormal + event.reason = CloneInProgress + event.message = fmt.Sprintf(MessageCloneInProgress, sourceNamespace, sourceName, dataVolume.Namespace, dataVolume.Name) + case cdiv1.PrepClaimInProgress: + event.eventType = corev1.EventTypeNormal + event.reason = PrepClaimInProgress + event.message = fmt.Sprintf(MessagePrepClaimInProgress, dataVolume.Namespace, dataVolume.Name) + case cdiv1.RebindInProgress: + event.eventType = corev1.EventTypeNormal + event.reason = RebindInProgress + event.message = fmt.Sprintf(MessageRebindInProgress, dataVolume.Namespace, dataVolume.Name) + default: + r.log.V(3).Info("No event set for phase", "phase", phase) } +} - return r.syncDataVolumeStatusPhaseWithEvent(syncState, phase, pvc, event) +var populatorPhaseMap = map[string]cdiv1.DataVolumePhase{ + "": cdiv1.CloneScheduled, + clone.PendingPhaseName: cdiv1.CloneScheduled, + clone.SucceededPhaseName: cdiv1.Succeeded, + clone.CSIClonePhaseName: cdiv1.CSICloneInProgress, + clone.HostClonePhaseName: cdiv1.CloneInProgress, + clone.PrepClaimPhaseName: cdiv1.PrepClaimInProgress, + clone.RebindPhaseName: cdiv1.RebindInProgress, + clone.SnapshotClonePhaseName: cdiv1.CloneFromSnapshotSourceInProgress, + clone.SnapshotPhaseName: cdiv1.SnapshotForSmartCloneInProgress, + //clone.ErrorPhaseName: cdiv1.Error, // Want to hold off on this for now +} + +func (r *CloneReconcilerBase) updateStatusPhaseForPopulator(pvc *corev1.PersistentVolumeClaim, dataVolumeCopy *cdiv1.DataVolume, event *Event) error { + popPhase := pvc.Annotations[populators.AnnClonePhase] + dvPhase, ok := populatorPhaseMap[popPhase] + if !ok { + r.log.V(1).Info("Unknown populator phase", "phase", popPhase) + //dataVolumeCopy.Status.Phase = cdiv1.Unknown // hold off on this for now + return nil + } + dataVolumeCopy.Status.Phase = dvPhase + r.setEventForPhase(dataVolumeCopy, dvPhase, event) + return nil } func (r *CloneReconcilerBase) updateStatusPhase(pvc *corev1.PersistentVolumeClaim, dataVolumeCopy *cdiv1.DataVolume, event *Event) error { + if err := r.populateSourceIfSourceRef(dataVolumeCopy); err != nil { + return err + } + _, sourceName, sourceNamespace := cc.GetCloneSourceInfo(dataVolumeCopy) + + usePopulator, err := CheckPVCUsingPopulators(pvc) + if err != nil { + return err + } + if usePopulator { + return r.updateStatusPhaseForPopulator(pvc, dataVolumeCopy, event) + } + phase, ok := pvc.Annotations[cc.AnnPodPhase] if phase != string(corev1.PodSucceeded) { _, ok = pvc.Annotations[cc.AnnCloneRequest] @@ -551,11 +444,6 @@ func (r *CloneReconcilerBase) updateStatusPhase(pvc *corev1.PersistentVolumeClai return nil } - if err := r.populateSourceIfSourceRef(dataVolumeCopy); err != nil { - return err - } - sourceName, sourceNamespace := cc.GetCloneSourceNameAndNamespace(dataVolumeCopy) - switch phase { case string(corev1.PodPending): dataVolumeCopy.Status.Phase = cdiv1.CloneScheduled @@ -605,92 +493,12 @@ func (r *CloneReconcilerBase) populateSourceIfSourceRef(dv *cdiv1.DataVolume) er return nil } -func (r *CloneReconcilerBase) cleanupTransfer(dv *cdiv1.DataVolume) error { - transferName := getTransferName(dv) - if !cc.HasFinalizer(dv, crossNamespaceFinalizer) { - return nil - } - - r.log.V(1).Info("Doing cleanup of transfer") - - if dv.DeletionTimestamp != nil && dv.Status.Phase != cdiv1.Succeeded { - // delete all potential PVCs that may not have owner refs - namespaces := []string{dv.Namespace} - names := []string{dv.Name} - namespaces, names = appendTmpPvcIfNeeded(dv, namespaces, names, transferName) - - for i := range namespaces { - pvc := &corev1.PersistentVolumeClaim{} - nn := types.NamespacedName{Namespace: namespaces[i], Name: names[i]} - if err := r.client.Get(context.TODO(), nn, pvc); err != nil { - if !k8serrors.IsNotFound(err) { - return err - } - } else { - pod := &corev1.Pod{} - nn := types.NamespacedName{Namespace: namespaces[i], Name: expansionPodName(pvc)} - if err := r.client.Get(context.TODO(), nn, pod); err != nil { - if !k8serrors.IsNotFound(err) { - return err - } - } else { - if err := r.client.Delete(context.TODO(), pod); err != nil { - if !k8serrors.IsNotFound(err) { - return err - } - } - } - - if err := r.client.Delete(context.TODO(), pvc); err != nil { - if !k8serrors.IsNotFound(err) { - return err - } - } - } - } - } - - ot := &cdiv1.ObjectTransfer{} - if err := r.client.Get(context.TODO(), types.NamespacedName{Name: transferName}, ot); err != nil { - if !k8serrors.IsNotFound(err) { - return err - } - } else { - if ot.DeletionTimestamp == nil { - if err := r.client.Delete(context.TODO(), ot); err != nil { - if !k8serrors.IsNotFound(err) { - return err - } - } - } - return fmt.Errorf("waiting for ObjectTransfer %s to delete", transferName) - } - - cc.RemoveFinalizer(dv, crossNamespaceFinalizer) - return nil -} - -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 { - _, sourceNamespace := cc.GetCloneSourceNameAndNamespace(dv) + _, _, sourceNamespace := cc.GetCloneSourceInfo(dv) return sourceNamespace != "" && sourceNamespace != dv.Namespace } -func getTransferName(dv *cdiv1.DataVolume) string { - return fmt.Sprintf("cdi-tmp-%s", dv.UID) -} - // addCloneWithoutSourceWatch reconciles clones created without source once the matching PVC is created func addCloneWithoutSourceWatch(mgr manager.Manager, datavolumeController controller.Controller, typeToWatch client.Object, indexingKey string) error { getKey := func(namespace, name string) string { @@ -700,7 +508,7 @@ func addCloneWithoutSourceWatch(mgr manager.Manager, datavolumeController contro if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &cdiv1.DataVolume{}, indexingKey, func(obj client.Object) []string { dv := obj.(*cdiv1.DataVolume) if source := dv.Spec.Source; source != nil { - sourceName, sourceNamespace := cc.GetCloneSourceNameAndNamespace(dv) + _, sourceName, sourceNamespace := cc.GetCloneSourceInfo(dv) if sourceName != "" { ns := cc.GetNamespace(sourceNamespace, obj.GetNamespace()) return []string{getKey(ns, sourceName)} diff --git a/pkg/controller/datavolume/controller-base.go b/pkg/controller/datavolume/controller-base.go index 370ebf5679..04788ebb87 100644 --- a/pkg/controller/datavolume/controller-base.go +++ b/pkg/controller/datavolume/controller-base.go @@ -779,7 +779,7 @@ func (r *ReconcilerBase) updateDataVolumeStatusPhaseWithEvent( return r.emitEvent(dataVolume, dataVolumeCopy, curPhase, dataVolume.Status.Conditions, &event) } -func (r ReconcilerBase) updateStatus(req reconcile.Request, phaseSync *statusPhaseSync, dvc dvController) (reconcile.Result, error) { +func (r *ReconcilerBase) updateStatus(req reconcile.Request, phaseSync *statusPhaseSync, dvc dvController) (reconcile.Result, error) { result := reconcile.Result{} dv, err := r.getDataVolume(req.NamespacedName) if dv == nil || err != nil { @@ -944,7 +944,7 @@ func (r *ReconcilerBase) emitFailureConditionEvent(dataVolume *cdiv1.DataVolume, if curReady.Status == corev1.ConditionFalse && curRunning.Status == corev1.ConditionFalse && dvBoundOrPopulationInProgress(dataVolume, curBound) { //Bound or in progress, not ready, and not running - if curRunning.Message != "" && orgRunning.Message != curRunning.Message { + if curRunning.Message != "" && (orgRunning == nil || orgRunning.Message != curRunning.Message) { r.recorder.Event(dataVolume, corev1.EventTypeWarning, curRunning.Reason, curRunning.Message) } } @@ -1207,9 +1207,9 @@ func (r *ReconcilerBase) shouldUseCDIPopulator(syncState *dvSyncState) (bool, er } // currently we don't support populator with import source of VDDK or Imageio // or clone either from PVC nor snapshot - if dv.Spec.Source.Imageio != nil || dv.Spec.Source.VDDK != nil || - dv.Spec.Source.PVC != nil || dv.Spec.Source.Snapshot != nil { - log.Info("Not using CDI populators, currently we don't support populators with Imageio/VDDk/Clone source") + if dv.Spec.Source.Imageio != nil || + dv.Spec.Source.VDDK != nil { + log.Info("Not using CDI populators, currently we don't support populators with Imageio/VDDk source") return false, nil } diff --git a/pkg/controller/datavolume/import-controller_test.go b/pkg/controller/datavolume/import-controller_test.go index e3101d1e70..27501458ca 100644 --- a/pkg/controller/datavolume/import-controller_test.go +++ b/pkg/controller/datavolume/import-controller_test.go @@ -34,7 +34,6 @@ import ( storagev1 "k8s.io/api/storage/v1" extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/api/errors" - k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -82,7 +81,7 @@ var _ = Describe("All DataVolume Tests", func() { pvc := &corev1.PersistentVolumeClaim{} err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, pvc) Expect(err).To(HaveOccurred()) - if !k8serrors.IsNotFound(err) { + if !errors.IsNotFound(err) { Fail("Error getting pvc") } }) @@ -1585,12 +1584,6 @@ var _ = Describe("All DataVolume Tests", func() { Entry("VDDK", &cdiv1.DataVolumeSource{ VDDK: &cdiv1.DataVolumeSourceVDDK{}, }), - Entry("PVC", &cdiv1.DataVolumeSource{ - PVC: &cdiv1.DataVolumeSourcePVC{}, - }), - Entry("Snapshot", &cdiv1.DataVolumeSource{ - Snapshot: &cdiv1.DataVolumeSourceSnapshot{}, - }), ) It("Should return false if storage class has wffc bindingMode and honorWaitForFirstConsumer feature gate is disabled", func() { diff --git a/pkg/controller/datavolume/pvc-clone-controller.go b/pkg/controller/datavolume/pvc-clone-controller.go index aa01eb5c58..91fd75d89b 100644 --- a/pkg/controller/datavolume/pvc-clone-controller.go +++ b/pkg/controller/datavolume/pvc-clone-controller.go @@ -20,50 +20,38 @@ import ( "context" "crypto/rsa" "fmt" - "reflect" "strconv" "time" "github.com/go-logr/logr" - snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" - storagev1 "k8s.io/api/storage/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" - - cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" - "kubevirt.io/containerized-data-importer/pkg/common" - - cc "kubevirt.io/containerized-data-importer/pkg/controller/common" - featuregates "kubevirt.io/containerized-data-importer/pkg/feature-gates" - - "kubevirt.io/containerized-data-importer/pkg/util" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" -) -type cloneStrategy int + cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" + "kubevirt.io/containerized-data-importer/pkg/common" + cc "kubevirt.io/containerized-data-importer/pkg/controller/common" + featuregates "kubevirt.io/containerized-data-importer/pkg/feature-gates" +) -// Possible clone strategies, including default special value NoClone const ( - NoClone cloneStrategy = iota - HostAssistedClone - SmartClone - CsiClone -) + sourceInUseRequeueDuration = time.Duration(5 * time.Second) -const sourceInUseRequeueDuration = time.Duration(5 * time.Second) + pvcCloneControllerName = "datavolume-pvc-clone-controller" -const pvcCloneControllerName = "datavolume-pvc-clone-controller" + volumeCloneSourcePrefix = "volume-clone-source" +) // ErrInvalidTermMsg reports that the termination message from the size-detection pod doesn't exists or is not a valid quantity var ErrInvalidTermMsg = fmt.Errorf("the termination message from the size-detection pod is not-valid") @@ -71,7 +59,6 @@ var ErrInvalidTermMsg = fmt.Errorf("the termination message from the size-detect // PvcCloneReconciler members type PvcCloneReconciler struct { CloneReconcilerBase - sccs controllerStarter } // NewPvcCloneController creates a new instance of the datavolume clone controller @@ -87,12 +74,6 @@ func NewPvcCloneController( installerLabels map[string]string, ) (controller.Controller, error) { client := mgr.GetClient() - sccs := &smartCloneControllerStarter{ - log: log, - installerLabels: installerLabels, - startSmartCloneController: make(chan struct{}, 1), - mgr: mgr, - } reconciler := &PvcCloneReconciler{ CloneReconcilerBase: CloneReconcilerBase{ ReconcilerBase: ReconcilerBase{ @@ -104,14 +85,15 @@ func NewPvcCloneController( installerLabels: installerLabels, shouldUpdateProgress: true, }, - clonerImage: clonerImage, - importerImage: importerImage, - pullPolicy: pullPolicy, - tokenValidator: cc.NewCloneTokenValidator(common.CloneTokenIssuer, tokenPublicKey), + clonerImage: clonerImage, + importerImage: importerImage, + pullPolicy: pullPolicy, + cloneSourceKind: "PersistentVolumeClaim", + shortTokenValidator: cc.NewCloneTokenValidator(common.CloneTokenIssuer, tokenPublicKey), + longTokenValidator: cc.NewCloneTokenValidator(common.ExtendedCloneTokenIssuer, tokenPublicKey), // for long term tokens to handle cross namespace dumb clones tokenGenerator: newLongTermCloneTokenGenerator(tokenPrivateKey), }, - sccs: sccs, } dataVolumeCloneController, err := controller.New(pvcCloneControllerName, mgr, controller.Options{ @@ -121,51 +103,14 @@ func NewPvcCloneController( return nil, err } - if err = addDataVolumeCloneControllerWatches(mgr, dataVolumeCloneController); err != nil { + if err = reconciler.addDataVolumeCloneControllerWatches(mgr, dataVolumeCloneController); err != nil { return nil, err } - if err = mgr.Add(sccs); err != nil { - return nil, err - } return dataVolumeCloneController, nil } -type controllerStarter interface { - Start(ctx context.Context) error - StartController() -} -type smartCloneControllerStarter struct { - log logr.Logger - installerLabels map[string]string - startSmartCloneController chan struct{} - mgr manager.Manager -} - -func (sccs *smartCloneControllerStarter) Start(ctx context.Context) error { - started := false - for { - select { - case <-sccs.startSmartCloneController: - if !started { - sccs.log.Info("Starting smart clone controller as CSI snapshot CRDs are detected") - if _, err := NewSmartCloneController(sccs.mgr, sccs.log, sccs.installerLabels); err != nil { - sccs.log.Error(err, "Unable to setup smart clone controller: %v") - } else { - started = true - } - } - case <-ctx.Done(): - return nil - } - } -} - -func (sccs *smartCloneControllerStarter) StartController() { - sccs.startSmartCloneController <- struct{}{} -} - -func addDataVolumeCloneControllerWatches(mgr manager.Manager, datavolumeController controller.Controller) error { +func (r *PvcCloneReconciler) addDataVolumeCloneControllerWatches(mgr manager.Manager, datavolumeController controller.Controller) error { if err := addDataVolumeControllerCommonWatches(mgr, datavolumeController, dataVolumePvcClone); err != nil { return err } @@ -179,6 +124,10 @@ func addDataVolumeCloneControllerWatches(mgr manager.Manager, datavolumeControll return err } + if err := r.addVolumeCloneSourceWatch(datavolumeController); err != nil { + return err + } + return nil } @@ -237,19 +186,50 @@ func (r *PvcCloneReconciler) prepare(syncState *dvSyncState) error { return nil } +func (r *PvcCloneReconciler) cleanup(syncState *dvSyncState) error { + dv := syncState.dvMutated + if err := r.populateSourceIfSourceRef(dv); err != nil { + return err + } + + if dv.DeletionTimestamp == nil && dv.Status.Phase != cdiv1.Succeeded { + return nil + } + + return r.reconcileVolumeCloneSourceCR(syncState) +} + +func addCloneToken(dv *cdiv1.DataVolume, pvc *corev1.PersistentVolumeClaim) error { + // first clear out tokens that may have already been added + delete(pvc.Annotations, cc.AnnCloneToken) + delete(pvc.Annotations, cc.AnnExtendedCloneToken) + if isCrossNamespaceClone(dv) { + // only want this initially + // extended token is added later + token, ok := dv.Annotations[cc.AnnCloneToken] + if !ok { + return errors.Errorf("no clone token") + } + cc.AddAnnotation(pvc, cc.AnnCloneToken, token) + } + return nil +} + +func volumeCloneSourceName(dv *cdiv1.DataVolume) string { + return fmt.Sprintf("%s-%s", volumeCloneSourcePrefix, dv.UID) +} + func (r *PvcCloneReconciler) updateAnnotations(dataVolume *cdiv1.DataVolume, pvc *corev1.PersistentVolumeClaim) error { if dataVolume.Spec.Source.PVC == nil { return errors.Errorf("no source set for clone datavolume") } + if err := addCloneToken(dataVolume, pvc); err != nil { + return err + } sourceNamespace := dataVolume.Spec.Source.PVC.Namespace if sourceNamespace == "" { sourceNamespace = dataVolume.Namespace } - token, ok := dataVolume.Annotations[cc.AnnCloneToken] - if !ok { - return errors.Errorf("no clone token") - } - pvc.Annotations[cc.AnnCloneToken] = token pvc.Annotations[cc.AnnCloneRequest] = sourceNamespace + "/" + dataVolume.Spec.Source.PVC.Name return nil } @@ -271,8 +251,6 @@ func (r *PvcCloneReconciler) syncClone(log logr.Logger, req reconcile.Request) ( pvc := syncRes.pvc pvcSpec := syncRes.pvcSpec datavolume := syncRes.dvMutated - transferName := getTransferName(datavolume) - pvcPopulated := pvcIsPopulated(pvc, datavolume) staticProvisionPending := checkStaticProvisionPending(pvc, datavolume) prePopulated := dvIsPrePopulated(datavolume) @@ -281,88 +259,54 @@ func (r *PvcCloneReconciler) syncClone(log logr.Logger, req reconcile.Request) ( return syncRes, nil } - // Check if source PVC exists and do proper validation before attempting to clone - if done, err := r.validateCloneAndSourcePVC(&syncRes, log); err != nil { + if addedToken, err := r.ensureExtendedTokenDV(datavolume); err != nil { return syncRes, err - } else if !done { + } else if addedToken { + // make sure token gets persisted before doing anything else return syncRes, nil } - // Get the most appropiate clone strategy - selectedCloneStrategy, err := r.selectCloneStrategy(datavolume, pvcSpec) - if err != nil { - return syncRes, err - } - if selectedCloneStrategy != NoClone { - cc.AddAnnotation(datavolume, cc.AnnCloneType, cloneStrategyToCloneType(selectedCloneStrategy)) - } - - if selectedCloneStrategy == SmartClone { - r.sccs.StartController() - } - - // If the target's size is not specified, we can extract that value from the source PVC - targetRequest, hasTargetRequest := pvcSpec.Resources.Requests[corev1.ResourceStorage] - if !hasTargetRequest || targetRequest.IsZero() { - done, err := r.detectCloneSize(&syncRes, selectedCloneStrategy) - if err != nil { + if pvc == nil { + // Check if source PVC exists and do proper validation before attempting to clone + if done, err := r.validateCloneAndSourcePVC(&syncRes, log); err != nil { return syncRes, err } else if !done { - // Check if the source PVC is ready to be cloned - if readyToClone, err := r.isSourceReadyToClone(datavolume, selectedCloneStrategy); err != nil { - return syncRes, err - } else if !readyToClone { - if syncRes.result == nil { - syncRes.result = &reconcile.Result{} - } - syncRes.result.RequeueAfter = sourceInUseRequeueDuration - return syncRes, - r.syncCloneStatusPhase(&syncRes, cdiv1.CloneScheduled, nil) - } return syncRes, nil } - } - if pvc == nil { - if selectedCloneStrategy == SmartClone { - snapshotClassName, err := cc.GetSnapshotClassForSmartClone(datavolume.Name, pvcSpec.StorageClassName, r.log, r.client) + // Always call detect size, it will handle the case where size is specified + // and detection pod not necessary + if datavolume.Spec.Storage != nil { + done, err := r.detectCloneSize(&syncRes) if err != nil { return syncRes, err - } - res, err := r.reconcileSmartClonePvc(log, &syncRes, transferName, snapshotClassName) - syncRes.result = &res - return syncRes, err - } - if selectedCloneStrategy == CsiClone { - csiDriverAvailable, err := storageClassCSIDriverExists(r.client, r.log, pvcSpec.StorageClassName) - if err != nil && !k8serrors.IsNotFound(err) { - return syncRes, err - } - if !csiDriverAvailable { - // err csi clone not possible - storageClass, err := cc.GetStorageClassByName(context.TODO(), r.client, pvcSpec.StorageClassName) - if err != nil { + } else if !done { + // Check if the source PVC is ready to be cloned + if readyToClone, err := r.isSourceReadyToClone(datavolume); err != nil { return syncRes, err + } else if !readyToClone { + if syncRes.result == nil { + syncRes.result = &reconcile.Result{} + } + syncRes.result.RequeueAfter = sourceInUseRequeueDuration + return syncRes, r.syncCloneStatusPhase(&syncRes, cdiv1.CloneScheduled, nil) } - noCsiDriverMsg := "CSI Clone configured, failed to look for CSIDriver - target storage class could not be found" - if storageClass != nil { - noCsiDriverMsg = fmt.Sprintf("CSI Clone configured, but no CSIDriver available for %s", storageClass.Name) - } - return syncRes, - r.syncDataVolumeStatusPhaseWithEvent(&syncRes, cdiv1.CloneScheduled, pvc, - Event{ - eventType: corev1.EventTypeWarning, - reason: ErrUnableToClone, - message: noCsiDriverMsg, - }) + return syncRes, nil } + } - res, err := r.reconcileCsiClonePvc(log, &syncRes, transferName) - syncRes.result = &res - return syncRes, err + pvcModifier := r.updateAnnotations + if syncRes.usePopulator { + if isCrossNamespaceClone(datavolume) { + if !cc.HasFinalizer(datavolume, crossNamespaceFinalizer) { + cc.AddFinalizer(datavolume, crossNamespaceFinalizer) + return syncRes, r.syncCloneStatusPhase(&syncRes, cdiv1.CloneScheduled, nil) + } + } + pvcModifier = r.updatePVCForPopulation } - newPvc, err := r.createPvcForDatavolume(datavolume, pvcSpec, r.updateAnnotations) + newPvc, err := r.createPvcForDatavolume(datavolume, pvcSpec, pvcModifier) if err != nil { if cc.ErrQuotaExceeded(err) { syncErr = r.syncDataVolumeStatusPhaseWithEvent(&syncRes, cdiv1.Pending, nil, @@ -380,305 +324,24 @@ func (r *PvcCloneReconciler) syncClone(log logr.Logger, req reconcile.Request) ( pvc = newPvc } - shouldBeMarkedWaitForFirstConsumer, err := r.shouldBeMarkedWaitForFirstConsumer(pvc) - if err != nil { - return syncRes, err - } - - switch selectedCloneStrategy { - case HostAssistedClone: - if err := r.ensureExtendedToken(pvc); err != nil { - return syncRes, err - } - case CsiClone: - switch pvc.Status.Phase { - case corev1.ClaimBound: - if err := r.setCloneOfOnPvc(pvc); err != nil { - return syncRes, err - } - case corev1.ClaimPending: - r.log.V(3).Info("ClaimPending CSIClone") - if !shouldBeMarkedWaitForFirstConsumer { - return syncRes, r.syncCloneStatusPhase(&syncRes, cdiv1.CSICloneInProgress, pvc) - } - case corev1.ClaimLost: - return syncRes, - r.syncDataVolumeStatusPhaseWithEvent(&syncRes, cdiv1.Failed, pvc, - Event{ - eventType: corev1.EventTypeWarning, - reason: ErrClaimLost, - message: fmt.Sprintf(MessageErrClaimLost, pvc.Name), - }) - } - fallthrough - case SmartClone: - if !shouldBeMarkedWaitForFirstConsumer { - res, err := r.finishClone(log, &syncRes, transferName) - syncRes.result = &res + if syncRes.usePopulator { + if err := r.reconcileVolumeCloneSourceCR(&syncRes); err != nil { return syncRes, err - } - } - - return syncRes, syncErr -} -func (r *PvcCloneReconciler) selectCloneStrategy(datavolume *cdiv1.DataVolume, pvcSpec *corev1.PersistentVolumeClaimSpec) (cloneStrategy, error) { - preferredCloneStrategy, err := r.getCloneStrategy(datavolume) - if err != nil { - if k8serrors.IsNotFound(err) { - return NoClone, nil - } - return NoClone, err - } - - bindingMode, err := r.getStorageClassBindingMode(pvcSpec.StorageClassName) - if err != nil { - return NoClone, err - } - if bindingMode != nil && *bindingMode == storagev1.VolumeBindingWaitForFirstConsumer { - waitForFirstConsumerEnabled, err := cc.IsWaitForFirstConsumerEnabled(datavolume, r.featureGates) - if err != nil { - return NoClone, err - } - if !waitForFirstConsumerEnabled { - return HostAssistedClone, nil - } - } - - if preferredCloneStrategy != nil && *preferredCloneStrategy == cdiv1.CloneStrategyCsiClone { - csiClonePossible, err := r.advancedClonePossible(datavolume, pvcSpec) - if err != nil { - return NoClone, err - } - - if csiClonePossible && - (!isCrossNamespaceClone(datavolume) || *bindingMode == storagev1.VolumeBindingImmediate) { - return CsiClone, nil - } - } else if preferredCloneStrategy != nil && *preferredCloneStrategy == cdiv1.CloneStrategySnapshot { - snapshotClassName, err := cc.GetSnapshotClassForSmartClone(datavolume.Name, pvcSpec.StorageClassName, r.log, r.client) - if err != nil { - return NoClone, err - } - snapshotClassAvailable := snapshotClassName != "" - - snapshotPossible, err := r.advancedClonePossible(datavolume, pvcSpec) - if err != nil { - return NoClone, err - } - - if snapshotClassAvailable && snapshotPossible && - (!isCrossNamespaceClone(datavolume) || *bindingMode == storagev1.VolumeBindingImmediate) { - return SmartClone, nil - } - } - - return HostAssistedClone, nil -} - -func (r *PvcCloneReconciler) reconcileCsiClonePvc(log logr.Logger, - syncRes *dvSyncState, - transferName string) (reconcile.Result, error) { - - log = log.WithName("reconcileCsiClonePvc") - datavolume := syncRes.dvMutated - pvcSpec := syncRes.pvcSpec - pvcName := datavolume.Name - - if isCrossNamespaceClone(datavolume) { - pvcName = transferName - - result, err := r.doCrossNamespaceClone(log, syncRes, pvcName, datavolume.Spec.Source.PVC.Namespace, false, CsiClone) - if result != nil { - return *result, err - } - } - - if datavolume.Status.Phase == cdiv1.NamespaceTransferInProgress { - return reconcile.Result{}, nil - } - - sourcePvcNs := datavolume.Spec.Source.PVC.Namespace - if sourcePvcNs == "" { - sourcePvcNs = datavolume.Namespace - } - log.V(3).Info("CSI-Clone is available") - - // Get source pvc - sourcePvc := &corev1.PersistentVolumeClaim{} - if err := r.client.Get(context.TODO(), types.NamespacedName{Namespace: sourcePvcNs, Name: datavolume.Spec.Source.PVC.Name}, sourcePvc); err != nil { - if k8serrors.IsNotFound(err) { - log.V(3).Info("Source PVC no longer exists") - return reconcile.Result{}, err - } - return reconcile.Result{}, err - } - - // Check if the source PVC is ready to be cloned - if readyToClone, err := r.isSourceReadyToClone(datavolume, CsiClone); err != nil { - return reconcile.Result{}, err - } else if !readyToClone { - return reconcile.Result{RequeueAfter: sourceInUseRequeueDuration}, - r.syncCloneStatusPhase(syncRes, cdiv1.CloneScheduled, nil) - } - - log.Info("Creating PVC for datavolume") - cloneTargetPvc, err := r.newVolumeClonePVC(datavolume, sourcePvc, pvcSpec, pvcName) - if err != nil { - return reconcile.Result{}, err - } - pvc := &corev1.PersistentVolumeClaim{} - if err := r.client.Get(context.TODO(), types.NamespacedName{Namespace: cloneTargetPvc.Namespace, Name: cloneTargetPvc.Name}, pvc); err != nil { - if !k8serrors.IsNotFound(err) { - return reconcile.Result{}, err - } - if err := r.client.Create(context.TODO(), cloneTargetPvc); err != nil && !k8serrors.IsAlreadyExists(err) { - if cc.ErrQuotaExceeded(err) { - syncErr := r.syncDataVolumeStatusPhaseWithEvent(syncRes, cdiv1.Pending, nil, - Event{ - eventType: corev1.EventTypeWarning, - reason: cc.ErrExceededQuota, - message: err.Error(), - }) - if syncErr != nil { - log.Error(syncErr, "failed to sync DataVolume status with event") - } - } - return reconcile.Result{}, err + ct, ok := pvc.Annotations[cc.AnnCloneType] + if ok { + cc.AddAnnotation(datavolume, cc.AnnCloneType, ct) } } else { - // PVC already exists, check for name clash - pvcControllerRef := metav1.GetControllerOf(cloneTargetPvc) - pvcClashControllerRef := metav1.GetControllerOf(pvc) - - if pvc.Name == cloneTargetPvc.Name && - pvc.Namespace == cloneTargetPvc.Namespace && - !reflect.DeepEqual(pvcControllerRef, pvcClashControllerRef) { - return reconcile.Result{}, errors.Errorf("Target Pvc Name in use") - } - - if pvc.Status.Phase == corev1.ClaimBound { - if err := r.setCloneOfOnPvc(pvc); err != nil { - return reconcile.Result{}, err - } - } - } - - return reconcile.Result{}, r.syncCloneStatusPhase(syncRes, cdiv1.CSICloneInProgress, nil) -} - -func cloneStrategyToCloneType(selectedCloneStrategy cloneStrategy) string { - switch selectedCloneStrategy { - case SmartClone: - return "snapshot" - case CsiClone: - return "csi-clone" - case HostAssistedClone: - return "copy" - } - return "" -} - -func (r *PvcCloneReconciler) reconcileSmartClonePvc(log logr.Logger, - syncState *dvSyncState, - transferName string, - snapshotClassName string) (reconcile.Result, error) { - - datavolume := syncState.dvMutated - pvcName := datavolume.Name - - if isCrossNamespaceClone(datavolume) { - pvcName = transferName - result, err := r.doCrossNamespaceClone(log, syncState, pvcName, datavolume.Spec.Source.PVC.Namespace, true, SmartClone) - if result != nil { - return *result, err - } - } - - if datavolume.Status.Phase == cdiv1.NamespaceTransferInProgress { - return reconcile.Result{}, nil + cc.AddAnnotation(datavolume, cc.AnnCloneType, string(cdiv1.CloneStrategyHostAssisted)) } - r.log.V(3).Info("Smart-Clone via Snapshot is available with Volume Snapshot Class", - "snapshotClassName", snapshotClassName) - - newSnapshot := newSnapshot(datavolume, pvcName, snapshotClassName) - util.SetRecommendedLabels(newSnapshot, r.installerLabels, "cdi-controller") - - if err := setAnnOwnedByDataVolume(newSnapshot, datavolume); err != nil { - return reconcile.Result{}, err - } - - nn := client.ObjectKeyFromObject(newSnapshot) - if err := r.client.Get(context.TODO(), nn, newSnapshot.DeepCopy()); err != nil { - if !k8serrors.IsNotFound(err) { - return reconcile.Result{}, err - } - - // Check if the source PVC is ready to be cloned - if readyToClone, err := r.isSourceReadyToClone(datavolume, SmartClone); err != nil { - return reconcile.Result{}, err - } else if !readyToClone { - return reconcile.Result{RequeueAfter: sourceInUseRequeueDuration}, - r.syncCloneStatusPhase(syncState, cdiv1.CloneScheduled, nil) - } - - targetPvc := &corev1.PersistentVolumeClaim{} - if err := r.client.Get(context.TODO(), nn, targetPvc); err != nil { - if !k8serrors.IsNotFound(err) { - return reconcile.Result{}, err - } - - if err := r.client.Create(context.TODO(), newSnapshot); err != nil { - if !k8serrors.IsAlreadyExists(err) { - return reconcile.Result{}, err - } - } else { - r.log.V(1).Info("snapshot created successfully", "snapshot.Namespace", newSnapshot.Namespace, "snapshot.Name", newSnapshot.Name) - } - } + if err := r.ensureExtendedTokenPVC(datavolume, pvc); err != nil { + return syncRes, err } - return reconcile.Result{}, r.syncCloneStatusPhase(syncState, cdiv1.SnapshotForSmartCloneInProgress, nil) -} - -func newSnapshot(dataVolume *cdiv1.DataVolume, snapshotName, snapshotClassName string) *snapshotv1.VolumeSnapshot { - annotations := make(map[string]string) - annotations[AnnSmartCloneRequest] = "true" - className := snapshotClassName - labels := map[string]string{ - common.CDILabelKey: common.CDILabelValue, - common.CDIComponentLabel: common.SmartClonerCDILabel, - } - snapshotNamespace := dataVolume.Namespace - if dataVolume.Spec.Source.PVC.Namespace != "" { - snapshotNamespace = dataVolume.Spec.Source.PVC.Namespace - } - snapshot := &snapshotv1.VolumeSnapshot{ - ObjectMeta: metav1.ObjectMeta{ - Name: snapshotName, - Namespace: snapshotNamespace, - Labels: labels, - Annotations: annotations, - }, - Spec: snapshotv1.VolumeSnapshotSpec{ - Source: snapshotv1.VolumeSnapshotSource{ - PersistentVolumeClaimName: &dataVolume.Spec.Source.PVC.Name, - }, - VolumeSnapshotClassName: &className, - }, - } - if dataVolume.Namespace == snapshotNamespace { - snapshot.OwnerReferences = []metav1.OwnerReference{ - *metav1.NewControllerRef(dataVolume, schema.GroupVersionKind{ - Group: cdiv1.SchemeGroupVersion.Group, - Version: cdiv1.SchemeGroupVersion.Version, - Kind: "DataVolume", - }), - } - } - return snapshot + return syncRes, syncErr } // Verify that the source PVC has been completely populated. @@ -706,180 +369,6 @@ func (r *PvcCloneReconciler) sourceInUse(dv *cdiv1.DataVolume, eventReason strin return len(pods) > 0, nil } -func (r *PvcCloneReconciler) cleanup(syncState *dvSyncState) error { - dv := syncState.dvMutated - - // This cleanup should be done if dv is marked for deletion or in case it succeeded - if dv.DeletionTimestamp == nil && dv.Status.Phase != cdiv1.Succeeded { - return nil - } - - 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 - } - } - - return nil -} - -// Returns true if methods different from HostAssisted are possible, -// both snapshot and csi volume clone share the same basic requirements -func (r *PvcCloneReconciler) advancedClonePossible(dataVolume *cdiv1.DataVolume, targetStorageSpec *corev1.PersistentVolumeClaimSpec) (bool, error) { - log := r.log.WithName("ClonePossible").V(3) - - sourcePvc, err := r.findSourcePvc(dataVolume) - if err != nil { - if k8serrors.IsNotFound(err) { - return false, errors.New("source PVC not found") - } - return false, err - } - - targetStorageClass, err := cc.GetStorageClassByName(context.TODO(), r.client, targetStorageSpec.StorageClassName) - if err != nil { - return false, err - } - if targetStorageClass == nil { - log.Info("Target PVC's Storage Class not found") - return false, nil - } - - if ok := r.validateSameStorageClass(sourcePvc, targetStorageClass); !ok { - return false, nil - } - - if ok, err := r.validateSameVolumeMode(dataVolume, sourcePvc, targetStorageClass); !ok || err != nil { - return false, err - } - - return r.validateAdvancedCloneSizeCompatible(sourcePvc, targetStorageSpec) -} - -func (r *PvcCloneReconciler) validateSameStorageClass( - sourcePvc *corev1.PersistentVolumeClaim, - targetStorageClass *storagev1.StorageClass) bool { - - targetPvcStorageClassName := &targetStorageClass.Name - sourcePvcStorageClassName := sourcePvc.Spec.StorageClassName - - // Compare source and target storage classess - if *sourcePvcStorageClassName != *targetPvcStorageClassName { - r.log.V(3).Info("Source PVC and target PVC belong to different storage classes", - "source storage class", *sourcePvcStorageClassName, - "target storage class", *targetPvcStorageClassName) - return false - } - - return true -} - -func (r *PvcCloneReconciler) validateSameVolumeMode( - dataVolume *cdiv1.DataVolume, - sourcePvc *corev1.PersistentVolumeClaim, - targetStorageClass *storagev1.StorageClass) (bool, error) { - - sourceVolumeMode := util.ResolveVolumeMode(sourcePvc.Spec.VolumeMode) - targetSpecVolumeMode, err := getStorageVolumeMode(r.client, dataVolume, targetStorageClass) - if err != nil { - return false, err - } - targetVolumeMode := util.ResolveVolumeMode(targetSpecVolumeMode) - - if sourceVolumeMode != targetVolumeMode { - r.log.V(3).Info("Source PVC and target PVC have different volume modes, falling back to host assisted clone", - "source volume mode", sourceVolumeMode, "target volume mode", targetVolumeMode) - return false, nil - } - - return true, nil -} - -func getStorageVolumeMode(c client.Client, dataVolume *cdiv1.DataVolume, storageClass *storagev1.StorageClass) (*corev1.PersistentVolumeMode, error) { - if dataVolume.Spec.PVC != nil { - return dataVolume.Spec.PVC.VolumeMode, nil - } else if dataVolume.Spec.Storage != nil { - if dataVolume.Spec.Storage.VolumeMode != nil { - return dataVolume.Spec.Storage.VolumeMode, nil - } - volumeMode, err := getDefaultVolumeMode(c, storageClass, dataVolume.Spec.Storage.AccessModes) - if err != nil { - return nil, err - } - return volumeMode, nil - } - - return nil, errors.Errorf("no target storage defined") -} - -func (r *PvcCloneReconciler) validateAdvancedCloneSizeCompatible( - sourcePvc *corev1.PersistentVolumeClaim, - targetStorageSpec *corev1.PersistentVolumeClaimSpec) (bool, error) { - srcStorageClass := &storagev1.StorageClass{} - if sourcePvc.Spec.StorageClassName == nil { - return false, fmt.Errorf("source PVC Storage Class name wasn't populated yet by PVC controller") - } - if err := r.client.Get(context.TODO(), types.NamespacedName{Name: *sourcePvc.Spec.StorageClassName}, srcStorageClass); cc.IgnoreNotFound(err) != nil { - return false, err - } - - srcRequest, hasSrcRequest := sourcePvc.Spec.Resources.Requests[corev1.ResourceStorage] - srcCapacity, hasSrcCapacity := sourcePvc.Status.Capacity[corev1.ResourceStorage] - targetRequest, hasTargetRequest := targetStorageSpec.Resources.Requests[corev1.ResourceStorage] - allowExpansion := srcStorageClass.AllowVolumeExpansion != nil && *srcStorageClass.AllowVolumeExpansion - if !hasSrcRequest || !hasSrcCapacity || !hasTargetRequest { - // return error so we retry the reconcile - return false, errors.New("source/target size info missing") - } - - if srcCapacity.Cmp(targetRequest) < 0 && !allowExpansion { - return false, nil - } - - if srcRequest.Cmp(targetRequest) > 0 && !targetRequest.IsZero() { - return false, nil - } - - return true, nil -} - -func (r *PvcCloneReconciler) getCloneStrategy(dataVolume *cdiv1.DataVolume) (*cdiv1.CDICloneStrategy, error) { - defaultCloneStrategy := cdiv1.CloneStrategySnapshot - sourcePvc, err := r.findSourcePvc(dataVolume) - if err != nil { - return nil, err - } - storageClass, err := cc.GetStorageClassByName(context.TODO(), r.client, sourcePvc.Spec.StorageClassName) - if err != nil { - return nil, err - } - - strategyOverride, err := r.getGlobalCloneStrategyOverride() - if err != nil { - return nil, err - } - if strategyOverride != nil { - return strategyOverride, nil - } - - // do check storageProfile and apply the preferences - strategy, err := r.getPreferredCloneStrategyForStorageClass(storageClass) - if err != nil { - return nil, err - } - if strategy != nil { - return strategy, err - } - - return &defaultCloneStrategy, nil -} - func (r *PvcCloneReconciler) findSourcePvc(dataVolume *cdiv1.DataVolume) (*corev1.PersistentVolumeClaim, error) { sourcePvcSpec := dataVolume.Spec.Source.PVC if sourcePvcSpec == nil { @@ -902,75 +391,6 @@ func (r *PvcCloneReconciler) findSourcePvc(dataVolume *cdiv1.DataVolume) (*corev return pvc, nil } -func (r *PvcCloneReconciler) getGlobalCloneStrategyOverride() (*cdiv1.CDICloneStrategy, error) { - cr, err := cc.GetActiveCDI(context.TODO(), r.client) - if err != nil { - return nil, err - } - - if cr == nil { - return nil, fmt.Errorf("no active CDI") - } - - if cr.Spec.CloneStrategyOverride == nil { - return nil, nil - } - - r.log.V(3).Info(fmt.Sprintf("Overriding default clone strategy with %s", *cr.Spec.CloneStrategyOverride)) - return cr.Spec.CloneStrategyOverride, nil -} - -// NewVolumeClonePVC creates a PVC object to be used during CSI volume cloning. -func (r *PvcCloneReconciler) newVolumeClonePVC(dv *cdiv1.DataVolume, - sourcePvc *corev1.PersistentVolumeClaim, - targetPvcSpec *corev1.PersistentVolumeClaimSpec, - pvcName string) (*corev1.PersistentVolumeClaim, error) { - - // Override name - might be temporary pod when in transfer - pvcNamespace := dv.Namespace - if dv.Spec.Source.PVC.Namespace != "" { - pvcNamespace = dv.Spec.Source.PVC.Namespace - } - - pvc, err := r.newPersistentVolumeClaim(dv, targetPvcSpec, pvcNamespace, pvcName, r.updateAnnotations) - if err != nil { - return nil, err - } - - // be sure correct clone method is selected - delete(pvc.Annotations, cc.AnnCloneRequest) - pvc.Annotations[AnnCSICloneRequest] = "true" - - // take source size while cloning - if pvc.Spec.Resources.Requests == nil { - pvc.Spec.Resources.Requests = corev1.ResourceList{} - } - sourceSize := sourcePvc.Status.Capacity.Storage() - pvc.Spec.Resources.Requests[corev1.ResourceStorage] = *sourceSize - - pvc.Spec.DataSource = &corev1.TypedLocalObjectReference{ - Name: dv.Spec.Source.PVC.Name, - Kind: "PersistentVolumeClaim", - } - - return pvc, nil -} - -func (r *PvcCloneReconciler) getPreferredCloneStrategyForStorageClass(storageClass *storagev1.StorageClass) (*cdiv1.CDICloneStrategy, error) { - if storageClass == nil { - // fallback to defaults - return nil, nil - } - - storageProfile := &cdiv1.StorageProfile{} - err := r.client.Get(context.TODO(), types.NamespacedName{Name: storageClass.Name}, storageProfile) - if err != nil { - return nil, errors.Wrap(err, "cannot get StorageProfile") - } - - return storageProfile.Status.CloneStrategy, nil -} - // validateCloneAndSourcePVC checks if the source PVC of a clone exists and does proper validation func (r *PvcCloneReconciler) validateCloneAndSourcePVC(syncState *dvSyncState, log logr.Logger) (bool, error) { datavolume := syncState.dvMutated @@ -1002,20 +422,10 @@ func (r *PvcCloneReconciler) validateCloneAndSourcePVC(syncState *dvSyncState, l } // isSourceReadyToClone handles the reconciling process of a clone when the source PVC is not ready -func (r *PvcCloneReconciler) isSourceReadyToClone( - datavolume *cdiv1.DataVolume, - selectedCloneStrategy cloneStrategy) (bool, error) { - - var eventReason string +func (r *PvcCloneReconciler) isSourceReadyToClone(datavolume *cdiv1.DataVolume) (bool, error) { + // TODO preper const + eventReason := "CloneSourceInUse" - switch selectedCloneStrategy { - case SmartClone: - eventReason = SmartCloneSourceInUse - case CsiClone: - eventReason = CSICloneSourceInUse - case HostAssistedClone: - eventReason = HostAssistedCloneSourceInUse - } // Check if any pods are using the source PVC inUse, err := r.sourceInUse(datavolume, eventReason) if err != nil { @@ -1035,22 +445,34 @@ func (r *PvcCloneReconciler) isSourceReadyToClone( } // detectCloneSize obtains and assigns the original PVC's size when cloning using an empty storage value -func (r *PvcCloneReconciler) detectCloneSize(syncState *dvSyncState, cloneType cloneStrategy) (bool, error) { - var targetSize int64 +func (r *PvcCloneReconciler) detectCloneSize(syncState *dvSyncState) (bool, error) { sourcePvc, err := r.findSourcePvc(syncState.dvMutated) if err != nil { return false, err } + + // because of filesystem overhead calculations when cloning + // even if storage size is requested we have to calculate source size + // when source is filesystem and target is block + requestedSize, hasSize := syncState.pvcSpec.Resources.Requests[corev1.ResourceStorage] + sizeRequired := !hasSize || requestedSize.IsZero() + targetIsBlock := syncState.pvcSpec.VolumeMode != nil && *syncState.pvcSpec.VolumeMode == corev1.PersistentVolumeBlock + sourceIsFilesystem := cc.GetVolumeMode(sourcePvc) == corev1.PersistentVolumeFilesystem + // have to be explicit here or detection pod will crash + sourceIsKubevirt := sourcePvc.Annotations[cc.AnnContentType] == string(cdiv1.DataVolumeKubeVirt) + if !sizeRequired && (!targetIsBlock || !sourceIsFilesystem || !sourceIsKubevirt) { + return true, nil + } + + var targetSize int64 sourceCapacity := sourcePvc.Status.Capacity.Storage() // Due to possible filesystem overhead complications when cloning // using host-assisted strategy, we create a pod that automatically // collects the size of the original virtual image with 'qemu-img'. - // If another strategy is used or the original PVC's volume mode - // is "block", we simply extract the value from the original PVC's spec. - if cloneType == HostAssistedClone && - cc.GetVolumeMode(sourcePvc) == corev1.PersistentVolumeFilesystem && - cc.GetPVCContentType(sourcePvc) == string(cdiv1.DataVolumeKubeVirt) { + // If the original PVC's volume mode is "block", + // we simply extract the value from the original PVC's spec. + if sourceIsFilesystem && sourceIsKubevirt { var available bool // If available, we first try to get the virtual size from previous iterations targetSize, available = getSizeFromAnnotations(sourcePvc) @@ -1067,13 +489,24 @@ func (r *PvcCloneReconciler) detectCloneSize(syncState *dvSyncState, cloneType c targetSize, _ = sourceCapacity.AsInt64() } - // Allow the clone-controller to skip the size comparison requirement - // if the source's size ends up being larger due to overhead differences - // TODO: Fix this in next PR that uses actual size also in validation - if sourceCapacity.CmpInt64(targetSize) == 1 { + var isPermissiveClone bool + if sizeRequired { + // Allow the clone-controller to skip the size comparison requirement + // if the source's size ends up being larger due to overhead differences + // TODO: Fix this in next PR that uses actual size also in validation + isPermissiveClone = sourceCapacity.CmpInt64(targetSize) == 1 + } else { + isPermissiveClone = requestedSize.CmpInt64(targetSize) >= 0 + } + + if isPermissiveClone { syncState.dvMutated.Annotations[cc.AnnPermissiveClone] = "true" } + if !sizeRequired { + return true, nil + } + // Parse size into a 'Quantity' struct and, if needed, inflate it with filesystem overhead targetCapacity, err := cc.InflateSizeWithOverhead(context.TODO(), r.client, targetSize, syncState.pvcSpec) if err != nil { @@ -1104,7 +537,11 @@ func getSizeFromAnnotations(sourcePvc *corev1.PersistentVolumeClaim) (int64, boo // getSizeFromPod attempts to get the image size from a pod that directly obtains said value from the source PVC func (r *PvcCloneReconciler) getSizeFromPod(targetPvc, sourcePvc *corev1.PersistentVolumeClaim, dv *cdiv1.DataVolume) (int64, error) { // The pod should not be created until the source PVC has finished the import process - if !cc.IsPVCComplete(sourcePvc) { + populated, err := cc.IsPopulated(sourcePvc, r.client) + if err != nil { + return 0, err + } + if !populated { r.recorder.Event(dv, corev1.EventTypeNormal, ImportPVCNotReady, MessageImportPVCNotReady) return 0, nil } diff --git a/pkg/controller/datavolume/pvc-clone-controller_test.go b/pkg/controller/datavolume/pvc-clone-controller_test.go index 995d51c94f..c0451659a3 100644 --- a/pkg/controller/datavolume/pvc-clone-controller_test.go +++ b/pkg/controller/datavolume/pvc-clone-controller_test.go @@ -18,7 +18,6 @@ package datavolume import ( "context" - "fmt" "reflect" "strings" @@ -28,6 +27,7 @@ import ( snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" corev1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" @@ -36,13 +36,17 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/record" + "k8s.io/utils/pointer" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" "kubevirt.io/containerized-data-importer/pkg/common" + "kubevirt.io/containerized-data-importer/pkg/controller/clone" . "kubevirt.io/containerized-data-importer/pkg/controller/common" + "kubevirt.io/containerized-data-importer/pkg/controller/populators" featuregates "kubevirt.io/containerized-data-importer/pkg/feature-gates" "kubevirt.io/containerized-data-importer/pkg/token" ) @@ -61,160 +65,240 @@ var _ = Describe("All DataVolume Tests", func() { } }) - var _ = Describe("Datavolume controller reconcile loop", func() { - AfterEach(func() { - if reconciler != nil && reconciler.recorder != nil { - close(reconciler.recorder.(*record.FakeRecorder).Events) - } - }) - - It("Should create a snapshot if cloning and the PVC doesn't exist, and the snapshot class can be found", func() { - dv := newCloneDataVolume("test-dv") - scName := "testsc" - sc := CreateStorageClassWithProvisioner(scName, map[string]string{ - AnnDefaultStorageClass: "true", - }, map[string]string{}, "csi-plugin") - sp := createStorageProfile(scName, []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}, BlockMode) - - dv.Spec.PVC.StorageClassName = &scName - pvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound) - expectedSnapshotClass := "snap-class" - snapClass := createSnapshotClass(expectedSnapshotClass, nil, "csi-plugin") - reconciler = createCloneReconciler(sc, sp, dv, pvc, snapClass, createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd()) - _, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) - Expect(err).ToNot(HaveOccurred()) - By("Verifying that snapshot now exists and phase is snapshot in progress") - snap := &snapshotv1.VolumeSnapshot{} - err = reconciler.client.Get(context.TODO(), types.NamespacedName{Namespace: dv.Namespace, Name: dv.Name}, snap) - Expect(err).ToNot(HaveOccurred()) - Expect(snap.Labels[common.AppKubernetesPartOfLabel]).To(Equal("testing")) - dv = &cdiv1.DataVolume{} - err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv) - Expect(err).ToNot(HaveOccurred()) - Expect(dv.Status.Phase).To(Equal(cdiv1.SnapshotForSmartCloneInProgress)) - }) + var _ = Describe("PVC clone controller populator integration", func() { + Context("with CSI provisioner", func() { + const ( + pluginName = "csi-plugin" + ) + + var ( + scName = "testSC" + storageClass *storagev1.StorageClass + csiDriver = &storagev1.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: pluginName, + }, + } + ) - It("Should not recreate snpashot that was cleaned-up", func() { - dv := newCloneDataVolume("test-dv") - scName := "testsc" - sc := CreateStorageClassWithProvisioner(scName, map[string]string{ - AnnDefaultStorageClass: "true", - }, map[string]string{}, "csi-plugin") - sp := createStorageProfile(scName, []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}, BlockMode) + BeforeEach(func() { + storageClass = CreateStorageClassWithProvisioner(scName, map[string]string{AnnDefaultStorageClass: "true"}, map[string]string{}, pluginName) + }) - dv.Spec.PVC.StorageClassName = &scName - pvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound) - expectedSnapshotClass := "snap-class" - snapClass := createSnapshotClass(expectedSnapshotClass, nil, "csi-plugin") - reconciler = createCloneReconciler(sc, sp, dv, pvc, snapClass, createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd()) - _, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) - Expect(err).ToNot(HaveOccurred()) - By("Verifying that snapshot now exists and phase is snapshot in progress") - snap := &snapshotv1.VolumeSnapshot{} - err = reconciler.client.Get(context.TODO(), types.NamespacedName{Namespace: dv.Namespace, Name: dv.Name}, snap) - Expect(err).ToNot(HaveOccurred()) - Expect(snap.Labels[common.AppKubernetesPartOfLabel]).To(Equal("testing")) - dv = &cdiv1.DataVolume{} - err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv) - Expect(err).ToNot(HaveOccurred()) - Expect(dv.Status.Phase).To(Equal(cdiv1.SnapshotForSmartCloneInProgress)) + It("should add extended token", func() { + dv := newCloneDataVolumeWithPVCNS("test-dv", "source-ns") + srcPvc := CreatePvcInStorageClass("test", "source-ns", &scName, nil, nil, corev1.ClaimBound) + reconciler = createCloneReconciler(storageClass, csiDriver, dv, srcPvc) + result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Requeue).To(BeFalse()) + Expect(result.RequeueAfter).To(BeZero()) + dv = &cdiv1.DataVolume{} + err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv) + Expect(err).ToNot(HaveOccurred()) + Expect(dv.Annotations).To(HaveKey(AnnExtendedCloneToken)) + }) - err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, pvc) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("persistentvolumeclaims \"test-dv\" not found")) - // Create smart clone PVC ourselves and delete snapshot (do smart clone controller's job) - // Shouldn't see a recreated snapshot as it was legitimately cleaned up - targetPvc := CreatePvcInStorageClass("test-dv", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound) - controller := true - targetPvc.OwnerReferences = append(targetPvc.OwnerReferences, metav1.OwnerReference{ - Kind: "DataVolume", - Controller: &controller, - Name: "test-dv", - UID: dv.UID, + It("should add finalizer for cross namespace clone", func() { + dv := newCloneDataVolumeWithPVCNS("test-dv", "source-ns") + dv.Annotations[AnnExtendedCloneToken] = "test-token" + srcPvc := CreatePvcInStorageClass("test", "source-ns", &scName, nil, nil, corev1.ClaimBound) + reconciler = createCloneReconciler(storageClass, csiDriver, dv, srcPvc) + result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Requeue).To(BeFalse()) + Expect(result.RequeueAfter).To(BeZero()) + dv = &cdiv1.DataVolume{} + err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv) + Expect(err).ToNot(HaveOccurred()) + Expect(dv.Finalizers).To(ContainElement(crossNamespaceFinalizer)) + Expect(dv.Status.Phase).To(Equal(cdiv1.CloneScheduled)) }) - err = reconciler.client.Create(context.TODO(), targetPvc) - Expect(err).ToNot(HaveOccurred()) - err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, targetPvc) - Expect(err).ToNot(HaveOccurred()) - // Smart clone target PVC is done (bound), cleaning up snapshot - err = reconciler.client.Delete(context.TODO(), snap) - Expect(err).ToNot(HaveOccurred()) - err = reconciler.client.Get(context.TODO(), types.NamespacedName{Namespace: dv.Namespace, Name: dv.Name}, snap) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("volumesnapshots.snapshot.storage.k8s.io \"test-dv\" not found")) - // Reconcile and check it wasn't recreated - _, err = reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) - Expect(err).ToNot(HaveOccurred()) - err = reconciler.client.Get(context.TODO(), types.NamespacedName{Namespace: dv.Namespace, Name: dv.Name}, snap) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("volumesnapshots.snapshot.storage.k8s.io \"test-dv\" not found")) - }) - It("Should do nothing when smart clone with namespace transfer and not target found", func() { - dv := newCloneDataVolume("test-dv") - scName := "testsc" - sc := CreateStorageClassWithProvisioner(scName, map[string]string{ - AnnDefaultStorageClass: "true", - }, map[string]string{}, "csi-plugin") - sp := createStorageProfile(scName, []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}, BlockMode) - - dv.Spec.PVC.StorageClassName = &scName - pvc := CreatePvcInStorageClass("test", "test", &scName, nil, nil, corev1.ClaimBound) - dv.Finalizers = append(dv.Finalizers, "cdi.kubevirt.io/dataVolumeFinalizer") - dv.Spec.Source.PVC.Namespace = pvc.Namespace - dv.Spec.Source.PVC.Name = pvc.Name - dv.Status.Phase = cdiv1.NamespaceTransferInProgress - ot := &cdiv1.ObjectTransfer{ - ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("cdi-tmp-%s", dv.UID), - }, - } - expectedSnapshotClass := "snap-class" - snapClass := createSnapshotClass(expectedSnapshotClass, nil, "csi-plugin") - reconciler = createCloneReconciler(sc, sp, dv, pvc, snapClass, ot, createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd()) - _, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) - Expect(err).ToNot(HaveOccurred()) - By("Verifying that phase is still NamespaceTransferInProgress") - dv = &cdiv1.DataVolume{} - err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv) - Expect(err).ToNot(HaveOccurred()) - Expect(dv.Status.Phase).To(Equal(cdiv1.NamespaceTransferInProgress)) - }) + DescribeTable("should create PVC and VolumeCloneSource CR", func(sourceNamespace string) { + dv := newCloneDataVolume("test-dv") + dv.Annotations[AnnExtendedCloneToken] = "foobar" + dv.Spec.Source.PVC.Namespace = sourceNamespace + if sourceNamespace != dv.Namespace { + dv.Finalizers = append(dv.Finalizers, crossNamespaceFinalizer) + } + srcPvc := CreatePvcInStorageClass("test", sourceNamespace, &scName, nil, nil, corev1.ClaimBound) + reconciler = createCloneReconciler(storageClass, csiDriver, dv, srcPvc) + result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Requeue).To(BeFalse()) + Expect(result.RequeueAfter).To(BeZero()) + dv = &cdiv1.DataVolume{} + err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv) + Expect(err).ToNot(HaveOccurred()) + pvc := &corev1.PersistentVolumeClaim{} + err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, pvc) + Expect(err).ToNot(HaveOccurred()) + Expect(pvc.Labels[common.AppKubernetesPartOfLabel]).To(Equal("testing")) + Expect(pvc.Labels[common.KubePersistentVolumeFillingUpSuppressLabelKey]).To(Equal(common.KubePersistentVolumeFillingUpSuppressLabelValue)) + Expect(pvc.Spec.DataSourceRef).ToNot(BeNil()) + if sourceNamespace != dv.Namespace { + Expect(pvc.Annotations[populators.AnnDataSourceNamespace]).To(Equal(sourceNamespace)) + } else { + Expect(pvc.Annotations).ToNot(HaveKey(populators.AnnDataSourceNamespace)) + } + cloneSourceName := volumeCloneSourceName(dv) + Expect(pvc.Spec.DataSourceRef.Name).To(Equal(cloneSourceName)) + Expect(pvc.Spec.DataSourceRef.Kind).To(Equal(cdiv1.VolumeCloneSourceRef)) + Expect(pvc.GetAnnotations()[AnnUsePopulator]).To(Equal("true")) + vcs := &cdiv1.VolumeCloneSource{} + err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: cloneSourceName, Namespace: sourceNamespace}, vcs) + Expect(err).ToNot(HaveOccurred()) + Expect(vcs.Spec.Source.Kind).To(Equal("PersistentVolumeClaim")) + Expect(vcs.Spec.Source.Name).To(Equal(srcPvc.Name)) + }, + Entry("with same namespace", metav1.NamespaceDefault), + Entry("with different namespace", "source-ns"), + ) - DescribeTable("Should NOT create a snapshot if source PVC mounted", func(podFunc func(*cdiv1.DataVolume) *corev1.Pod) { - dv := newCloneDataVolume("test-dv") - scName := "testsc" - sc := CreateStorageClassWithProvisioner(scName, map[string]string{ - AnnDefaultStorageClass: "true", - }, map[string]string{}, "csi-plugin") - sp := createStorageProfile(scName, []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}, BlockMode) + It("should add cloneType annotation", func() { + dv := newCloneDataVolume("test-dv") + anno := map[string]string{ + AnnExtendedCloneToken: "test-token", + AnnCloneType: string(cdiv1.CloneStrategySnapshot), + AnnUsePopulator: "true", + } + pvc := CreatePvcInStorageClass("test-dv", metav1.NamespaceDefault, &scName, anno, nil, corev1.ClaimPending) + pvc.Spec.DataSourceRef = &corev1.TypedObjectReference{ + Kind: cdiv1.VolumeCloneSourceRef, + Name: volumeCloneSourceName(dv), + } + pvc.OwnerReferences = append(pvc.OwnerReferences, metav1.OwnerReference{ + Kind: "DataVolume", + Controller: pointer.Bool(true), + Name: "test-dv", + UID: dv.UID, + }) + vcs := &cdiv1.VolumeCloneSource{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metav1.NamespaceDefault, + Name: volumeCloneSourceName(dv), + }, + Spec: cdiv1.VolumeCloneSourceSpec{ + Source: corev1.TypedLocalObjectReference{ + Kind: "PersistentVolumeClaim", + Name: dv.Spec.Source.PVC.Name, + }, + }, + } + reconciler = createCloneReconciler(storageClass, csiDriver, dv, pvc, vcs) + result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Requeue).To(BeFalse()) + Expect(result.RequeueAfter).To(BeZero()) + dv = &cdiv1.DataVolume{} + err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv) + Expect(err).ToNot(HaveOccurred()) + Expect(dv.Annotations[AnnCloneType]).To(Equal(string(cdiv1.CloneStrategySnapshot))) + }) - dv.Spec.PVC.StorageClassName = &scName - pvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound) - expectedSnapshotClass := "snap-class" - snapClass := createSnapshotClass(expectedSnapshotClass, nil, "csi-plugin") - reconciler = createCloneReconciler(sc, sp, dv, pvc, snapClass, podFunc(dv), createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd()) - result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) - Expect(err).ToNot(HaveOccurred()) - Expect(result.RequeueAfter).To(Equal(sourceInUseRequeueDuration)) - By("Checking events recorded") - close(reconciler.recorder.(*record.FakeRecorder).Events) - found := false - for event := range reconciler.recorder.(*record.FakeRecorder).Events { - if strings.Contains(event, "SmartCloneSourceInUse") { - found = true + DescribeTable("should map phase correctly", func(phaseName string, dvPhase cdiv1.DataVolumePhase, eventReason string) { + dv := newCloneDataVolume("test-dv") + anno := map[string]string{ + AnnExtendedCloneToken: "test-token", + AnnCloneType: string(cdiv1.CloneStrategySnapshot), + populators.AnnClonePhase: phaseName, + AnnUsePopulator: "true", } - } - reconciler.recorder = nil - Expect(found).To(BeTrue()) - }, - Entry("read/write", func(dv *cdiv1.DataVolume) *corev1.Pod { - return podUsingCloneSource(dv, false) - }), - Entry("read only", func(dv *cdiv1.DataVolume) *corev1.Pod { - return podUsingCloneSource(dv, true) - }), - ) + pvc := CreatePvcInStorageClass("test-dv", metav1.NamespaceDefault, &scName, anno, nil, corev1.ClaimPending) + pvc.Spec.DataSourceRef = &corev1.TypedObjectReference{ + Kind: cdiv1.VolumeCloneSourceRef, + Name: volumeCloneSourceName(dv), + } + pvc.OwnerReferences = append(pvc.OwnerReferences, metav1.OwnerReference{ + Kind: "DataVolume", + Controller: pointer.Bool(true), + Name: "test-dv", + UID: dv.UID, + }) + vcs := &cdiv1.VolumeCloneSource{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metav1.NamespaceDefault, + Name: volumeCloneSourceName(dv), + }, + Spec: cdiv1.VolumeCloneSourceSpec{ + Source: corev1.TypedLocalObjectReference{ + Kind: "PersistentVolumeClaim", + Name: dv.Spec.Source.PVC.Name, + }, + }, + } + reconciler = createCloneReconciler(storageClass, csiDriver, dv, pvc, vcs) + result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Requeue).To(BeFalse()) + Expect(result.RequeueAfter).To(BeZero()) + dv = &cdiv1.DataVolume{} + err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv) + Expect(err).ToNot(HaveOccurred()) + Expect(dv.Status.Phase).To(Equal(dvPhase)) + found := false + for event := range reconciler.recorder.(*record.FakeRecorder).Events { + if strings.Contains(event, eventReason) { + found = true + break + } + } + Expect(found).To(BeTrue()) + }, + Entry("empty phase", "", cdiv1.CloneScheduled, CloneScheduled), + Entry("pending phase", clone.PendingPhaseName, cdiv1.CloneScheduled, CloneScheduled), + Entry("succeeded phase", clone.SucceededPhaseName, cdiv1.Succeeded, CloneSucceeded), + Entry("csi clone phase", clone.CSIClonePhaseName, cdiv1.CSICloneInProgress, CSICloneInProgress), + Entry("host clone phase", clone.HostClonePhaseName, cdiv1.CloneInProgress, CloneInProgress), + Entry("prep claim phase", clone.PrepClaimPhaseName, cdiv1.PrepClaimInProgress, PrepClaimInProgress), + Entry("rebind phase", clone.RebindPhaseName, cdiv1.RebindInProgress, RebindInProgress), + Entry("pvc from snapshot phase", clone.SnapshotClonePhaseName, cdiv1.CloneFromSnapshotSourceInProgress, CloneFromSnapshotSourceInProgress), + Entry("create snapshot phase", clone.SnapshotPhaseName, cdiv1.SnapshotForSmartCloneInProgress, SnapshotForSmartCloneInProgress), + ) + + It("should delete VolumeCloneSource on success", func() { + dv := newCloneDataVolume("test-dv") + dv.Status.Phase = cdiv1.Succeeded + anno := map[string]string{ + AnnExtendedCloneToken: "test-token", + AnnCloneType: string(cdiv1.CloneStrategySnapshot), + populators.AnnClonePhase: clone.SucceededPhaseName, + AnnUsePopulator: "true", + } + pvc := CreatePvcInStorageClass("test-dv", metav1.NamespaceDefault, &scName, anno, nil, corev1.ClaimPending) + pvc.Spec.DataSourceRef = &corev1.TypedObjectReference{ + Kind: cdiv1.VolumeCloneSourceRef, + Name: volumeCloneSourceName(dv), + } + pvc.OwnerReferences = append(pvc.OwnerReferences, metav1.OwnerReference{ + Kind: "DataVolume", + Controller: pointer.Bool(true), + Name: "test-dv", + UID: dv.UID, + }) + vcs := &cdiv1.VolumeCloneSource{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metav1.NamespaceDefault, + Name: volumeCloneSourceName(dv), + }, + Spec: cdiv1.VolumeCloneSourceSpec{ + Source: corev1.TypedLocalObjectReference{ + Kind: "PersistentVolumeClaim", + Name: dv.Spec.Source.PVC.Name, + }, + }, + } + reconciler = createCloneReconciler(storageClass, csiDriver, dv, pvc, vcs) + result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Requeue).To(BeFalse()) + Expect(result.RequeueAfter).To(BeZero()) + err = reconciler.client.Get(context.TODO(), client.ObjectKeyFromObject(vcs), vcs) + Expect(err).To(HaveOccurred()) + Expect(k8serrors.IsNotFound(err)).To(BeTrue()) + }) + }) }) var _ = Describe("Reconcile Datavolume status", func() { @@ -293,267 +377,6 @@ var _ = Describe("All DataVolume Tests", func() { }) }) - var _ = Describe("Smart clone", func() { - It("Should err, if no source pvc provided", func() { - dv := NewImportDataVolume("test-dv") - reconciler = createCloneReconciler(dv) - possible, err := reconciler.advancedClonePossible(dv, dv.Spec.PVC) - Expect(err).To(HaveOccurred()) - Expect(possible).To(BeFalse()) - }) - - It("Should not return storage class, if no CSI CRDs exist", func() { - dv := newCloneDataVolume("test-dv") - scName := "test" - sc := CreateStorageClass(scName, map[string]string{ - AnnDefaultStorageClass: "true", - }) - reconciler = createCloneReconciler(dv, sc) - snapclass, err := GetSnapshotClassForSmartClone(dv.Name, dv.Spec.PVC.StorageClassName, reconciler.log, reconciler.client) - Expect(err).ToNot(HaveOccurred()) - Expect(snapclass).To(BeEmpty()) - }) - - It("Should not return snapshot class, if source PVC doesn't exist", func() { - dv := newCloneDataVolumeWithPVCNS("test-dv", "ns2") - scName := "test" - sc := CreateStorageClass(scName, map[string]string{ - AnnDefaultStorageClass: "true", - }) - reconciler = createCloneReconciler(dv, sc, createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd()) - snapshotClass, err := GetSnapshotClassForSmartClone(dv.Name, dv.Spec.PVC.StorageClassName, reconciler.log, reconciler.client) - Expect(err).ToNot(HaveOccurred()) - Expect(snapshotClass).To(BeEmpty()) - }) - - It("Should err, if source PVC doesn't exist", func() { - dv := newCloneDataVolumeWithPVCNS("test-dv", "ns2") - scName := "test" - sc := CreateStorageClass(scName, map[string]string{ - AnnDefaultStorageClass: "true", - }) - reconciler = createCloneReconciler(dv, sc, createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd()) - possible, err := reconciler.advancedClonePossible(dv, dv.Spec.PVC) - Expect(err).To(HaveOccurred()) - Expect(possible).To(BeFalse()) - }) - - It("Should not allow smart clone, if source PVC exist, but no storage class exists, and no storage class in PVC def", func() { - dv := newCloneDataVolume("test-dv") - pvc := CreatePvc("test", metav1.NamespaceDefault, nil, nil) - reconciler = createCloneReconciler(dv, pvc) - possible, err := reconciler.advancedClonePossible(dv, dv.Spec.PVC) - Expect(err).ToNot(HaveOccurred()) - Expect(possible).To(BeFalse()) - }) - - It("Should not allow smart clone, if source SC and target SC do not match", func() { - dv := newCloneDataVolume("test-dv") - targetSc := "testsc" - tsc := CreateStorageClass(targetSc, map[string]string{ - AnnDefaultStorageClass: "true", - }) - dv.Spec.PVC.StorageClassName = &targetSc - sourceSc := "testsc2" - ssc := CreateStorageClass(sourceSc, map[string]string{ - AnnDefaultStorageClass: "true", - }) - pvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &sourceSc, nil, nil, corev1.ClaimBound) - reconciler = createCloneReconciler(ssc, tsc, dv, pvc) - possible, err := reconciler.advancedClonePossible(dv, dv.Spec.PVC) - Expect(err).ToNot(HaveOccurred()) - Expect(possible).To(BeFalse()) - }) - - It("Should not return snapshot class, if storage class does not exist", func() { - dv := newCloneDataVolume("test-dv") - scName := "testsc" - dv.Spec.PVC.StorageClassName = &scName - pvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound) - reconciler = createCloneReconciler(dv, pvc, createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd()) - snapclass, err := GetSnapshotClassForSmartClone(dv.Name, dv.Spec.PVC.StorageClassName, reconciler.log, reconciler.client) - Expect(err).ToNot(HaveOccurred()) - Expect(snapclass).To(BeEmpty()) - }) - - It("Should not return snapshot class, if storage class exists but snapshot class does not exist", func() { - dv := newCloneDataVolume("test-dv") - scName := "testsc" - sc := CreateStorageClass(scName, map[string]string{ - AnnDefaultStorageClass: "true", - }) - dv.Spec.PVC.StorageClassName = &scName - pvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound) - reconciler = createCloneReconciler(sc, dv, pvc) - snapclass, err := GetSnapshotClassForSmartClone(dv.Name, dv.Spec.PVC.StorageClassName, reconciler.log, reconciler.client) - Expect(err).ToNot(HaveOccurred()) - Expect(snapclass).To(BeEmpty()) - }) - - It("Should return snapshot class, everything is available", func() { - dv := newCloneDataVolume("test-dv") - scName := "testsc" - sc := CreateStorageClassWithProvisioner(scName, map[string]string{ - AnnDefaultStorageClass: "true", - }, map[string]string{}, "csi-plugin") - dv.Spec.PVC.StorageClassName = &scName - pvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound) - expectedSnapshotClass := "snap-class" - snapClass := createSnapshotClass(expectedSnapshotClass, nil, "csi-plugin") - reconciler = createCloneReconciler(sc, dv, pvc, snapClass, createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd()) - snapclass, err := GetSnapshotClassForSmartClone(dv.Name, dv.Spec.PVC.StorageClassName, reconciler.log, reconciler.client) - Expect(err).ToNot(HaveOccurred()) - Expect(snapclass).To(Equal(expectedSnapshotClass)) - }) - - DescribeTable("Setting clone strategy affects the output of getGlobalCloneStrategyOverride", func(expectedCloneStrategy cdiv1.CDICloneStrategy) { - dv := newCloneDataVolume("test-dv") - reconciler = createCloneReconciler(dv) - - cr := &cdiv1.CDI{} - err := reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "cdi"}, cr) - Expect(err).ToNot(HaveOccurred()) - - cr.Spec.CloneStrategyOverride = &expectedCloneStrategy - err = reconciler.client.Update(context.TODO(), cr) - Expect(err).ToNot(HaveOccurred()) - - cloneStrategy, err := reconciler.getGlobalCloneStrategyOverride() - Expect(err).ToNot(HaveOccurred()) - Expect(*cloneStrategy).To(Equal(expectedCloneStrategy)) - }, - Entry("copy", cdiv1.CloneStrategyHostAssisted), - Entry("snapshot", cdiv1.CloneStrategySnapshot), - ) - - DescribeTable("After smart clone", func(actualSize resource.Quantity, currentSize resource.Quantity, expectedDvPhase cdiv1.DataVolumePhase) { - strategy := cdiv1.CloneStrategySnapshot - controller := true - - dv := newCloneDataVolume("test-dv") - scName := "testsc" - sc := CreateStorageClassWithProvisioner(scName, map[string]string{ - AnnDefaultStorageClass: "true", - }, map[string]string{}, "csi-plugin") - accessMode := []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany} - storageProfile := createStorageProfileWithCloneStrategy(scName, - []cdiv1.ClaimPropertySet{{AccessModes: accessMode, VolumeMode: &BlockMode}}, - &strategy) - snapshotClassName := "snap-class" - snapClass := createSnapshotClass(snapshotClassName, nil, "csi-plugin") - - srcPvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound) - targetPvc := CreatePvcInStorageClass("test-dv", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound) - targetPvc.OwnerReferences = append(targetPvc.OwnerReferences, metav1.OwnerReference{ - Kind: "DataVolume", - Controller: &controller, - Name: "test-dv", - UID: dv.UID, - }) - targetPvc.Spec.Resources.Requests[corev1.ResourceStorage] = currentSize - targetPvc.Status.Capacity[corev1.ResourceStorage] = actualSize - targetPvc.SetAnnotations(make(map[string]string)) - targetPvc.GetAnnotations()[AnnCloneOf] = "true" - - reconciler = createCloneReconciler(dv, srcPvc, targetPvc, storageProfile, sc, snapClass, createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd()) - - By("Reconcile") - result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) - Expect(err).To(Not(HaveOccurred())) - Expect(result).To(Not(BeNil())) - - By(fmt.Sprintf("Verifying that dv phase is now in %s", expectedDvPhase)) - dv = &cdiv1.DataVolume{} - err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv) - Expect(err).ToNot(HaveOccurred()) - Expect(dv.Status.Phase).To(Equal(expectedDvPhase)) - - By("Verifying that pvc request size as expected") - pvc := &corev1.PersistentVolumeClaim{} - err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, pvc) - Expect(err).ToNot(HaveOccurred()) - Expect(pvc.Spec.Resources.Requests[corev1.ResourceStorage]).To(Equal(resource.MustParse("1G"))) - - }, - Entry("Should expand pvc when actual and current differ then the requested size", resource.MustParse("500M"), resource.MustParse("500M"), cdiv1.ExpansionInProgress), - Entry("Should update request size when current size differ then the requested size and actual size is bigger then both", resource.MustParse("2G"), resource.MustParse("500M"), cdiv1.ExpansionInProgress), - Entry("Should update request size when current size differ from requested size", resource.MustParse("1G"), resource.MustParse("500M"), cdiv1.ExpansionInProgress), - Entry("Should complete clone in case all sizes match", resource.MustParse("1G"), resource.MustParse("1G"), cdiv1.Succeeded), - ) - }) - - var _ = Describe("CSI clone", func() { - DescribeTable("Starting from Failed DV", - func(targetPvcPhase corev1.PersistentVolumeClaimPhase, expectedDvPhase cdiv1.DataVolumePhase) { - strategy := cdiv1.CloneStrategyCsiClone - controller := true - - dv := newCloneDataVolume("test-dv") - dv.Status.Phase = cdiv1.Failed - - scName := "testsc" - srcPvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound) - targetPvc := CreatePvcInStorageClass("test-dv", metav1.NamespaceDefault, &scName, nil, nil, targetPvcPhase) - targetPvc.OwnerReferences = append(targetPvc.OwnerReferences, metav1.OwnerReference{ - Kind: "DataVolume", - Controller: &controller, - Name: "test-dv", - UID: dv.UID, - }) - sc := CreateStorageClassWithProvisioner(scName, map[string]string{ - AnnDefaultStorageClass: "true", - }, map[string]string{}, "csi-plugin") - - accessMode := []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany} - storageProfile := createStorageProfileWithCloneStrategy(scName, - []cdiv1.ClaimPropertySet{{AccessModes: accessMode, VolumeMode: &BlockMode}}, - &strategy) - - reconciler = createCloneReconciler(dv, srcPvc, targetPvc, storageProfile, sc) - - By("Reconcile") - result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) - Expect(err).To(Not(HaveOccurred())) - Expect(result).To(Not(BeNil())) - - By(fmt.Sprintf("Verifying that phase is now in %s", expectedDvPhase)) - dv = &cdiv1.DataVolume{} - err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv) - Expect(err).ToNot(HaveOccurred()) - Expect(dv.Status.Phase).To(Equal(expectedDvPhase)) - - }, - Entry("Should be in progress, if source pvc is ClaimPending", corev1.ClaimPending, cdiv1.CSICloneInProgress), - Entry("Should be failed, if source pvc is ClaimLost", corev1.ClaimLost, cdiv1.Failed), - Entry("Should be Succeeded, if source pvc is ClaimBound", corev1.ClaimBound, cdiv1.Succeeded), - ) - - It("Should not panic if CSI Driver not available and no storage class on PVC spec", func() { - strategy := cdiv1.CDICloneStrategy(cdiv1.CloneStrategyCsiClone) - - dv := newCloneDataVolume("test-dv") - - scName := "testsc" - srcPvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound) - sc := CreateStorageClassWithProvisioner(scName, map[string]string{ - AnnDefaultStorageClass: "true", - }, map[string]string{}, "csi-plugin") - - accessMode := []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany} - storageProfile := createStorageProfileWithCloneStrategy(scName, - []cdiv1.ClaimPropertySet{{AccessModes: accessMode, VolumeMode: &BlockMode}}, - &strategy) - - reconciler := createCloneReconciler(dv, srcPvc, storageProfile, sc, createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd()) - - By("Reconcile") - result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: dv.Name, Namespace: dv.Namespace}}) - Expect(err).ToNot(HaveOccurred()) - Expect(result).ToNot(BeNil()) - }) - - }) - var _ = Describe("Clone without source", func() { scName := "testsc" sc := CreateStorageClassWithProvisioner(scName, map[string]string{ @@ -657,58 +480,6 @@ var _ = Describe("All DataVolume Tests", func() { ) }) - var _ = Describe("Clone strategy", func() { - var ( - hostAssisted = cdiv1.CloneStrategyHostAssisted - snapshot = cdiv1.CloneStrategySnapshot - csiClone = cdiv1.CloneStrategyCsiClone - ) - - DescribeTable("Setting clone strategy affects the output of getCloneStrategy", - func(override, preferredCloneStrategy *cdiv1.CDICloneStrategy, expectedCloneStrategy cdiv1.CDICloneStrategy) { - dv := newCloneDataVolume("test-dv") - scName := "testsc" - pvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound) - sc := CreateStorageClassWithProvisioner(scName, map[string]string{ - AnnDefaultStorageClass: "true", - }, map[string]string{}, "csi-plugin") - - accessMode := []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany} - storageProfile := createStorageProfileWithCloneStrategy(scName, - []cdiv1.ClaimPropertySet{{AccessModes: accessMode, VolumeMode: &BlockMode}}, - preferredCloneStrategy) - - reconciler = createCloneReconciler(dv, pvc, storageProfile, sc) - - cr := &cdiv1.CDI{} - err := reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "cdi"}, cr) - Expect(err).ToNot(HaveOccurred()) - - cr.Spec.CloneStrategyOverride = override - err = reconciler.client.Update(context.TODO(), cr) - Expect(err).ToNot(HaveOccurred()) - - cloneStrategy, err := reconciler.getCloneStrategy(dv) - Expect(err).ToNot(HaveOccurred()) - Expect(*cloneStrategy).To(Equal(expectedCloneStrategy)) - }, - Entry("override hostAssisted /host", &hostAssisted, &hostAssisted, cdiv1.CloneStrategyHostAssisted), - Entry("override hostAssisted /snapshot", &hostAssisted, &snapshot, cdiv1.CloneStrategyHostAssisted), - Entry("override hostAssisted /csiClone", &hostAssisted, &csiClone, cdiv1.CloneStrategyHostAssisted), - Entry("override hostAssisted /nil", &hostAssisted, nil, cdiv1.CloneStrategyHostAssisted), - - Entry("override snapshot /host", &snapshot, &hostAssisted, cdiv1.CloneStrategySnapshot), - Entry("override snapshot /snapshot", &snapshot, &snapshot, cdiv1.CloneStrategySnapshot), - Entry("override snapshot /csiClone", &snapshot, &csiClone, cdiv1.CloneStrategySnapshot), - Entry("override snapshot /nil", &snapshot, nil, cdiv1.CloneStrategySnapshot), - - Entry("preferred snapshot", nil, &snapshot, cdiv1.CloneStrategySnapshot), - Entry("preferred hostassisted", nil, &hostAssisted, cdiv1.CloneStrategyHostAssisted), - Entry("preferred csiClone", nil, &csiClone, cdiv1.CloneStrategyCsiClone), - Entry("should default to snapshot", nil, nil, cdiv1.CloneStrategySnapshot), - ) - }) - var _ = Describe("Clone with empty storage size", func() { scName := "testsc" accessMode := []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany} @@ -731,7 +502,7 @@ var _ = Describe("All DataVolume Tests", func() { reconciler := createCloneReconciler(dv, storageProfile, sc) pvcSpec, err := renderPvcSpec(reconciler.client, reconciler.recorder, reconciler.log, dv, nil) Expect(err).ToNot(HaveOccurred()) - done, err := reconciler.detectCloneSize(syncState(dv, targetPvc, pvcSpec), HostAssistedClone) + done, err := reconciler.detectCloneSize(syncState(dv, targetPvc, pvcSpec)) Expect(err).To(HaveOccurred()) Expect(done).To(BeFalse()) Expect(k8serrors.IsNotFound(err)).To(BeTrue()) @@ -743,12 +514,29 @@ var _ = Describe("All DataVolume Tests", func() { storageProfile := createStorageProfileWithCloneStrategy(scName, []cdiv1.ClaimPropertySet{ {AccessModes: accessMode, VolumeMode: &BlockMode}}, &cloneStrategy) + sourceDV := &cdiv1.DataVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "source-dv", + Namespace: metav1.NamespaceDefault, + }, + Status: cdiv1.DataVolumeStatus{ + Phase: cdiv1.ImportInProgress, + }, + } pvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound) - reconciler := createCloneReconciler(dv, pvc, storageProfile, sc) + pvc.OwnerReferences = []metav1.OwnerReference{ + { + Kind: "DataVolume", + Name: sourceDV.Name, + Controller: pointer.Bool(true), + }, + } + AddAnnotation(pvc, AnnContentType, "kubevirt") + reconciler := createCloneReconciler(dv, sourceDV, pvc, storageProfile, sc) pvcSpec, err := renderPvcSpec(reconciler.client, reconciler.recorder, reconciler.log, dv, pvc) Expect(err).ToNot(HaveOccurred()) - done, err := reconciler.detectCloneSize(syncState(dv, pvc, pvcSpec), HostAssistedClone) + done, err := reconciler.detectCloneSize(syncState(dv, pvc, pvcSpec)) Expect(err).ToNot(HaveOccurred()) Expect(done).To(BeFalse()) By("Checking events recorded") @@ -771,12 +559,12 @@ var _ = Describe("All DataVolume Tests", func() { pvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound) pvc.SetAnnotations(make(map[string]string)) - pvc.GetAnnotations()[AnnPodPhase] = string(corev1.PodSucceeded) + pvc.Annotations[AnnContentType] = "kubevirt" reconciler := createCloneReconciler(dv, pvc, storageProfile, sc) pvcSpec, err := renderPvcSpec(reconciler.client, reconciler.recorder, reconciler.log, dv, pvc) Expect(err).ToNot(HaveOccurred()) - done, err := reconciler.detectCloneSize(syncState(dv, pvc, pvcSpec), HostAssistedClone) + done, err := reconciler.detectCloneSize(syncState(dv, pvc, pvcSpec)) Expect(err).ToNot(HaveOccurred()) Expect(done).To(BeFalse()) By("Checking events recorded") @@ -800,7 +588,7 @@ var _ = Describe("All DataVolume Tests", func() { pvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound) pvc.SetAnnotations(make(map[string]string)) - pvc.GetAnnotations()[AnnPodPhase] = string(corev1.PodSucceeded) + pvc.Annotations[AnnContentType] = "kubevirt" reconciler := createCloneReconciler(dv, pvc, storageProfile, sc) // Prepare the size-detection Pod with the required information @@ -812,7 +600,7 @@ var _ = Describe("All DataVolume Tests", func() { // Checks pvcSpec, err := renderPvcSpec(reconciler.client, reconciler.recorder, reconciler.log, dv, pvc) Expect(err).ToNot(HaveOccurred()) - done, err := reconciler.detectCloneSize(syncState(dv, pvc, pvcSpec), HostAssistedClone) + done, err := reconciler.detectCloneSize(syncState(dv, pvc, pvcSpec)) Expect(err).To(HaveOccurred()) Expect(err).To(Equal(ErrInvalidTermMsg)) Expect(done).To(BeFalse()) @@ -831,7 +619,7 @@ var _ = Describe("All DataVolume Tests", func() { pvc := CreatePvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound) pvc.SetAnnotations(make(map[string]string)) - pvc.GetAnnotations()[AnnPodPhase] = string(corev1.PodSucceeded) + pvc.Annotations[AnnContentType] = "kubevirt" reconciler := createCloneReconciler(dv, pvc, storageProfile, sc) // Prepare the size-detection Pod with the required information @@ -859,7 +647,7 @@ var _ = Describe("All DataVolume Tests", func() { // Checks syncState := syncState(dv, pvc, pvcSpec) - done, err := reconciler.detectCloneSize(syncState, HostAssistedClone) + done, err := reconciler.detectCloneSize(syncState) Expect(err).ToNot(HaveOccurred()) Expect(done).To(BeTrue()) Expect(syncState.dvMutated.Annotations[AnnPermissiveClone]).To(Equal("true")) @@ -879,6 +667,7 @@ var _ = Describe("All DataVolume Tests", func() { pvc.SetAnnotations(make(map[string]string)) pvc.GetAnnotations()[AnnVirtualImageSize] = "100" // Mock value pvc.GetAnnotations()[AnnSourceCapacity] = string(pvc.Status.Capacity.Storage().String()) + pvc.GetAnnotations()[AnnContentType] = "kubevirt" reconciler := createCloneReconciler(dv, pvc, storageProfile, sc) // Get the expected value @@ -890,7 +679,7 @@ var _ = Describe("All DataVolume Tests", func() { // Checks syncState := syncState(dv, pvc, pvcSpec) - done, err := reconciler.detectCloneSize(syncState, HostAssistedClone) + done, err := reconciler.detectCloneSize(syncState) Expect(err).ToNot(HaveOccurred()) Expect(done).To(BeTrue()) Expect(syncState.dvMutated.Annotations[AnnPermissiveClone]).To(Equal("true")) @@ -900,7 +689,7 @@ var _ = Describe("All DataVolume Tests", func() { }) DescribeTable("Should automatically collect the clone size from the source PVC's spec", - func(cloneStrategy cdiv1.CDICloneStrategy, selectedCloneStrategy cloneStrategy, volumeMode corev1.PersistentVolumeMode) { + func(cloneStrategy cdiv1.CDICloneStrategy, volumeMode corev1.PersistentVolumeMode) { dv := newCloneDataVolumeWithEmptyStorage("test-dv", "default") storageProfile := createStorageProfileWithCloneStrategy(scName, []cdiv1.ClaimPropertySet{ {AccessModes: accessMode, VolumeMode: &volumeMode}}, &cloneStrategy) @@ -912,41 +701,16 @@ var _ = Describe("All DataVolume Tests", func() { pvcSpec, err := renderPvcSpec(reconciler.client, reconciler.recorder, reconciler.log, dv, pvc) Expect(err).ToNot(HaveOccurred()) expectedSize := *pvc.Status.Capacity.Storage() - done, err := reconciler.detectCloneSize(syncState(dv, pvc, pvcSpec), selectedCloneStrategy) + done, err := reconciler.detectCloneSize(syncState(dv, pvc, pvcSpec)) Expect(err).ToNot(HaveOccurred()) Expect(done).To(BeTrue()) Expect(pvc.Spec.Resources.Requests.Storage().Cmp(expectedSize)).To(Equal(0)) }, - Entry("snapshot with empty size and 'Block' volume mode", cdiv1.CloneStrategySnapshot, SmartClone, BlockMode), - Entry("csiClone with empty size and 'Block' volume mode", cdiv1.CloneStrategyCsiClone, CsiClone, BlockMode), - Entry("hostAssited with empty size and 'Block' volume mode", cdiv1.CloneStrategyHostAssisted, HostAssistedClone, BlockMode), - Entry("snapshot with empty size and 'Filesystem' volume mode", cdiv1.CloneStrategySnapshot, SmartClone, FilesystemMode), - Entry("csiClone with empty size and 'Filesystem' volume mode", cdiv1.CloneStrategyCsiClone, CsiClone, FilesystemMode), + Entry("hostAssited with empty size and 'Block' volume mode", cdiv1.CloneStrategyHostAssisted, BlockMode), ) }) }) -func podUsingCloneSource(dv *cdiv1.DataVolume, readOnly bool) *corev1.Pod { - return &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: dv.Spec.Source.PVC.Namespace, - Name: dv.Spec.Source.PVC.Name + "-pod", - }, - Spec: corev1.PodSpec{ - Volumes: []corev1.Volume{ - { - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: dv.Spec.Source.PVC.Name, - ReadOnly: readOnly, - }, - }, - }, - }, - }, - } -} - func createCloneReconciler(objects ...runtime.Object) *PvcCloneReconciler { cdiConfig := MakeEmptyCDIConfigSpec(common.ConfigName) cdiConfig.Status = cdiv1.CDIConfigStatus{ @@ -985,8 +749,6 @@ func createCloneReconcilerWithoutConfig(objects ...runtime.Object) *PvcCloneReco rec := record.NewFakeRecorder(10) - sccs := &fakeControllerStarter{} - // Create a ReconcileMemcached object with the scheme and fake client. r := &PvcCloneReconciler{ CloneReconcilerBase: CloneReconcilerBase{ @@ -1002,10 +764,11 @@ func createCloneReconcilerWithoutConfig(objects ...runtime.Object) *PvcCloneReco }, shouldUpdateProgress: true, }, - tokenValidator: &FakeValidator{Match: "foobar"}, - tokenGenerator: &FakeGenerator{token: "foobar"}, + shortTokenValidator: &FakeValidator{Match: "foobar"}, + longTokenValidator: &FakeValidator{Match: "foobar", Params: map[string]string{"uid": "uid"}}, + tokenGenerator: &FakeGenerator{token: "foobar"}, + cloneSourceKind: "PersistentVolumeClaim", }, - sccs: sccs, } return r } @@ -1069,15 +832,6 @@ func newCloneDataVolumeWithEmptyStorage(name string, pvcNamespace string) *cdiv1 } } -type fakeControllerStarter struct{} - -func (f *fakeControllerStarter) Start(ctx context.Context) error { - return nil -} - -func (f *fakeControllerStarter) StartController() { -} - func createSnapshotClass(name string, annotations map[string]string, snapshotter string) *snapshotv1.VolumeSnapshotClass { return &snapshotv1.VolumeSnapshotClass{ TypeMeta: metav1.TypeMeta{ diff --git a/pkg/controller/datavolume/smart-clone-controller.go b/pkg/controller/datavolume/smart-clone-controller.go deleted file mode 100644 index ca7317d06c..0000000000 --- a/pkg/controller/datavolume/smart-clone-controller.go +++ /dev/null @@ -1,427 +0,0 @@ -/* -Copyright 2022 The CDI Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -limitations under the License. -See the License for the specific language governing permissions and -*/ - -package datavolume - -import ( - "context" - "fmt" - "strings" - - "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" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/tools/cache" - "k8s.io/client-go/tools/record" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller" - "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/manager" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - "sigs.k8s.io/controller-runtime/pkg/source" - - cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" - "kubevirt.io/containerized-data-importer/pkg/common" - cc "kubevirt.io/containerized-data-importer/pkg/controller/common" - "kubevirt.io/containerized-data-importer/pkg/util" -) - -const ( - //AnnSmartCloneRequest sets our expected annotation for a CloneRequest - AnnSmartCloneRequest = "k8s.io/SmartCloneRequest" - - annSmartCloneSnapshot = "cdi.kubevirt.io/smartCloneSnapshot" -) - -// SmartCloneReconciler members -type SmartCloneReconciler struct { - client client.Client - recorder record.EventRecorder - scheme *runtime.Scheme - log logr.Logger - installerLabels map[string]string -} - -// NewSmartCloneController creates a new instance of the Smart clone controller. -func NewSmartCloneController(mgr manager.Manager, log logr.Logger, installerLabels map[string]string) (controller.Controller, error) { - reconciler := &SmartCloneReconciler{ - client: mgr.GetClient(), - scheme: mgr.GetScheme(), - log: log.WithName("smartclone-controller"), - recorder: mgr.GetEventRecorderFor("smartclone-controller"), - installerLabels: installerLabels, - } - smartCloneController, err := controller.New("smartclone-controller", mgr, controller.Options{ - Reconciler: reconciler, - }) - if err != nil { - return nil, err - } - if err := addSmartCloneControllerWatches(mgr, smartCloneController); err != nil { - return nil, err - } - return smartCloneController, nil -} - -func addSmartCloneControllerWatches(mgr manager.Manager, smartCloneController controller.Controller) error { - if err := smartCloneController.Watch(&source.Kind{Type: &corev1.PersistentVolumeClaim{}}, handler.EnqueueRequestsFromMapFunc( - func(obj client.Object) []reconcile.Request { - pvc := obj.(*corev1.PersistentVolumeClaim) - if hasAnnOwnedByDataVolume(pvc) && shouldReconcilePvc(pvc) { - return []reconcile.Request{ - { - NamespacedName: types.NamespacedName{ - Namespace: pvc.Namespace, - Name: pvc.Name, - }, - }, - } - } - return nil - }, - )); err != nil { - return err - } - - // check if volume snapshots exist - err := mgr.GetClient().List(context.TODO(), &snapshotv1.VolumeSnapshotList{}, &client.ListOptions{Limit: 1}) - if meta.IsNoMatchError(err) { - return nil - } - - if err != nil && !cc.IsErrCacheNotStarted(err) { - return err - } - - if err := smartCloneController.Watch(&source.Kind{Type: &snapshotv1.VolumeSnapshot{}}, handler.EnqueueRequestsFromMapFunc( - func(obj client.Object) []reconcile.Request { - snapshot := obj.(*snapshotv1.VolumeSnapshot) - if hasAnnOwnedByDataVolume(snapshot) && shouldReconcileSnapshot(snapshot) { - return []reconcile.Request{ - { - NamespacedName: types.NamespacedName{ - Namespace: snapshot.Namespace, - Name: snapshot.Name, - }, - }, - } - } - return nil - }, - )); err != nil { - return err - } - - return nil -} - -func shouldReconcileSnapshot(snapshot *snapshotv1.VolumeSnapshot) bool { - _, ok := snapshot.GetAnnotations()[AnnSmartCloneRequest] - return ok -} - -func shouldReconcilePvc(pvc *corev1.PersistentVolumeClaim) bool { - val, ok := pvc.GetAnnotations()[AnnSmartCloneRequest] - return ok && val == "true" -} - -// Reconcile the reconcile loop for smart cloning. -func (r *SmartCloneReconciler) Reconcile(_ context.Context, req reconcile.Request) (reconcile.Result, error) { - log := r.log.WithValues("VolumeSnapshot/PersistentVolumeClaim", req.NamespacedName) - log.Info("reconciling smart clone") - pvc := &corev1.PersistentVolumeClaim{} - if err := r.client.Get(context.TODO(), req.NamespacedName, pvc); err != nil { - if k8serrors.IsNotFound(err) { - // PVC not found, look up smart clone. - snapshot := &snapshotv1.VolumeSnapshot{} - if err := r.client.Get(context.TODO(), req.NamespacedName, snapshot); err != nil { - if k8serrors.IsNotFound(err) { - return reconcile.Result{}, nil - } - return reconcile.Result{}, err - } - return r.reconcileSnapshot(log, snapshot) - } - return reconcile.Result{}, err - } - return r.reconcilePvc(log, pvc) -} - -func (r *SmartCloneReconciler) reconcilePvc(log logr.Logger, pvc *corev1.PersistentVolumeClaim) (reconcile.Result, error) { - log.WithValues("pvc.Name", pvc.Name).WithValues("pvc.Namespace", pvc.Namespace).Info("Reconciling PVC") - - snapshotName, hasSnapshot := pvc.Annotations[annSmartCloneSnapshot] - - // Don't delete snapshot unless the PVC is bound. - if hasSnapshot && pvc.Status.Phase == corev1.ClaimBound { - namespace, name, err := cache.SplitMetaNamespaceKey(snapshotName) - if err != nil { - return reconcile.Result{}, err - } - - if err := r.deleteSnapshot(log, namespace, name); err != nil { - return reconcile.Result{}, err - } - - if v, ok := pvc.Annotations[cc.AnnCloneOf]; !ok || v != "true" { - if pvc.Annotations == nil { - pvc.Annotations = make(map[string]string) - } - pvc.Annotations[cc.AnnCloneOf] = "true" - - if err := r.client.Update(context.TODO(), pvc); err != nil { - return reconcile.Result{}, err - } - } - } - - return reconcile.Result{}, nil -} - -func (r *SmartCloneReconciler) reconcileSnapshot(log logr.Logger, snapshot *snapshotv1.VolumeSnapshot) (reconcile.Result, error) { - log.WithValues("snapshot.Name", snapshot.Name). - WithValues("snapshot.Namespace", snapshot.Namespace). - Info("Reconciling snapshot") - - if snapshot.DeletionTimestamp != nil { - return reconcile.Result{}, nil - } - - dataVolume, err := r.getDataVolume(snapshot) - if err != nil { - return reconcile.Result{}, err - } - - if dataVolume == nil || dataVolume.DeletionTimestamp != nil { - if err := r.deleteSnapshot(log, snapshot.Namespace, snapshot.Name); err != nil { - return reconcile.Result{}, err - } - - return reconcile.Result{}, nil - } - - // pvc may have been transferred - targetPVC, err := r.getTargetPVC(dataVolume) - if err != nil { - return reconcile.Result{}, err - } - - if targetPVC != nil { - return reconcile.Result{}, nil - } - - if !cc.IsSnapshotReady(snapshot) { - // wait for ready to use - return reconcile.Result{}, nil - } - - targetPvcSpec, err := renderPvcSpec(r.client, r.recorder, r.log, dataVolume, nil) - if err != nil { - return reconcile.Result{}, err - } - - newPvc, err := newPvcFromSnapshot(dataVolume, snapshot.Name, snapshot, targetPvcSpec) - if err != nil { - return reconcile.Result{}, err - } - util.SetRecommendedLabels(newPvc, r.installerLabels, "cdi-controller") - - if err := setAnnOwnedByDataVolume(newPvc, dataVolume); err != nil { - return reconcile.Result{}, err - } - //passing annotations from the target DV to the matching target PVC - if len(dataVolume.GetAnnotations()) > 0 { - for k, v := range dataVolume.GetAnnotations() { - if !strings.Contains(k, common.CDIAnnKey) { - newPvc.Annotations[k] = v - } - } - } - if snapshot.Spec.Source.PersistentVolumeClaimName != nil { - event := &Event{ - eventType: corev1.EventTypeNormal, - reason: SmartClonePVCInProgress, - message: fmt.Sprintf(MessageSmartClonePVCInProgress, snapshot.Namespace, *snapshot.Spec.Source.PersistentVolumeClaimName), - } - - r.emitEvent(snapshot, event) - } - - log.V(3).Info("Creating PVC from snapshot", "pvc.Namespace", newPvc.Namespace, "pvc.Name", newPvc.Name) - if err := r.client.Create(context.TODO(), newPvc); err != nil { - if cc.ErrQuotaExceeded(err) { - event := &Event{ - eventType: corev1.EventTypeWarning, - reason: cc.ErrExceededQuota, - message: err.Error(), - } - - r.emitEvent(snapshot, event) - } - log.Error(err, "error creating pvc from snapshot") - return reconcile.Result{}, err - } - - return reconcile.Result{}, nil -} - -func (r *SmartCloneReconciler) deleteSnapshot(log logr.Logger, namespace, name string) error { - snapshotToDelete := &snapshotv1.VolumeSnapshot{} - if err := r.client.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, snapshotToDelete); err != nil { - if k8serrors.IsNotFound(err) { - return nil - } - return err - } - - if snapshotToDelete.DeletionTimestamp != nil { - return nil - } - - if _, ok := snapshotToDelete.Labels[common.CDIComponentLabel]; !ok { - // Not a CDI snapshot, don't delete - return nil - } - - if err := r.client.Delete(context.TODO(), snapshotToDelete); err != nil { - if !k8serrors.IsNotFound(err) { - log.Error(err, "error deleting snapshot for smart-clone") - return err - } - } - - log.V(3).Info("Snapshot deleted") - return nil -} - -func (r *SmartCloneReconciler) emitEvent(snapshot *snapshotv1.VolumeSnapshot, event *Event) { - if event.eventType != "" { - r.recorder.Event(snapshot, event.eventType, event.reason, event.message) - } -} - -func (r *SmartCloneReconciler) getDataVolume(snapshot *snapshotv1.VolumeSnapshot) (*cdiv1.DataVolume, error) { - namespace, name, err := getAnnOwnedByDataVolume(snapshot) - if err != nil { - return nil, err - } - - dataVolume := &cdiv1.DataVolume{} - nn := types.NamespacedName{Name: name, Namespace: namespace} - if err := r.client.Get(context.TODO(), nn, dataVolume); err != nil { - if k8serrors.IsNotFound(err) { - return nil, nil - } - - return nil, err - } - - return dataVolume, nil -} - -func (r *SmartCloneReconciler) getTargetPVC(dataVolume *cdiv1.DataVolume) (*corev1.PersistentVolumeClaim, error) { - // TODO update when PVC name may differ from DataVolume - pvc := &corev1.PersistentVolumeClaim{} - nn := types.NamespacedName{Name: dataVolume.Name, Namespace: dataVolume.Namespace} - if err := r.client.Get(context.TODO(), nn, pvc); err != nil { - if k8serrors.IsNotFound(err) { - return nil, nil - } - - return nil, err - } - - return pvc, nil -} - -func newPvcFromSnapshot(obj metav1.Object, name string, snapshot *snapshotv1.VolumeSnapshot, targetPvcSpec *corev1.PersistentVolumeClaimSpec) (*corev1.PersistentVolumeClaim, error) { - targetPvcSpecCopy := targetPvcSpec.DeepCopy() - restoreSize := snapshot.Status.RestoreSize - if restoreSize == nil || restoreSize.Sign() == -1 { - return nil, fmt.Errorf("snapshot has no RestoreSize") - } - if restoreSize.IsZero() { - reqSize := targetPvcSpec.Resources.Requests[corev1.ResourceStorage] - restoreSize = &reqSize - } - - key, err := cache.MetaNamespaceKeyFunc(snapshot) - if err != nil { - return nil, err - } - - labels := map[string]string{ - "cdi-controller": snapshot.Name, - common.CDILabelKey: common.CDILabelValue, - common.CDIComponentLabel: common.SmartClonerCDILabel, - } - for k, v := range obj.GetLabels() { - labels[k] = v - } - - annotations := map[string]string{ - AnnSmartCloneRequest: "true", - cc.AnnRunningCondition: string(corev1.ConditionFalse), - cc.AnnRunningConditionMessage: cc.CloneComplete, - cc.AnnRunningConditionReason: "Completed", - annSmartCloneSnapshot: key, - } - for k, v := range obj.GetAnnotations() { - annotations[k] = v - } - - if util.ResolveVolumeMode(targetPvcSpecCopy.VolumeMode) == corev1.PersistentVolumeFilesystem { - labels[common.KubePersistentVolumeFillingUpSuppressLabelKey] = common.KubePersistentVolumeFillingUpSuppressLabelValue - } - - target := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: snapshot.Namespace, - Labels: labels, - Annotations: annotations, - }, - Spec: corev1.PersistentVolumeClaimSpec{ - DataSource: &corev1.TypedLocalObjectReference{ - Name: snapshot.Name, - Kind: "VolumeSnapshot", - APIGroup: &snapshotv1.SchemeGroupVersion.Group, - }, - VolumeMode: targetPvcSpecCopy.VolumeMode, - AccessModes: targetPvcSpecCopy.AccessModes, - StorageClassName: targetPvcSpecCopy.StorageClassName, - Resources: targetPvcSpecCopy.Resources, - }, - } - - if target.Spec.Resources.Requests == nil { - target.Spec.Resources.Requests = corev1.ResourceList{} - } - - target.Spec.Resources.Requests[corev1.ResourceStorage] = *restoreSize - - ownerRef := metav1.GetControllerOf(snapshot) - if ownerRef != nil { - target.OwnerReferences = append(target.OwnerReferences, *ownerRef) - } - - return target, nil -} diff --git a/pkg/controller/datavolume/smart-clone-controller_test.go b/pkg/controller/datavolume/smart-clone-controller_test.go deleted file mode 100644 index 8d88bf284c..0000000000 --- a/pkg/controller/datavolume/smart-clone-controller_test.go +++ /dev/null @@ -1,341 +0,0 @@ -/* -Copyright 2020 The CDI Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package datavolume - -import ( - "context" - "fmt" - - "k8s.io/apimachinery/pkg/api/resource" - - snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" - . "github.com/onsi/ginkgo" - "github.com/onsi/ginkgo/extensions/table" - . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" - k8serrors "k8s.io/apimachinery/pkg/api/errors" - 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/client-go/tools/record" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - - cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" - "kubevirt.io/containerized-data-importer/pkg/common" - . "kubevirt.io/containerized-data-importer/pkg/controller/common" -) - -var ( - scLog = logf.Log.WithName("smart-clone-controller-test") -) - -var _ = Describe("All smart clone tests", func() { - var _ = Describe("Smart-clone reconcile functions", func() { - table.DescribeTable("snapshot", func(annotation string, expectSuccess bool) { - annotations := make(map[string]string) - if annotation != "" { - annotations[annotation] = "" - } - val := &snapshotv1.VolumeSnapshot{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: annotations, - }, - } - Expect(shouldReconcileSnapshot(val)).To(Equal(expectSuccess)) - }, - table.Entry("should reconcile if annotation exists", AnnSmartCloneRequest, true), - table.Entry("should not reconcile if annotation does not exist", "", false), - ) - - table.DescribeTable("pvc", func(key, value string, expectSuccess bool) { - annotations := make(map[string]string) - if key != "" { - annotations[key] = value - } - val := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: annotations, - }, - } - Expect(shouldReconcilePvc(val)).To(Equal(expectSuccess)) - }, - table.Entry("should reconcile if annotation exists, and is true", AnnSmartCloneRequest, "true", true), - table.Entry("should not reconcile if annotation exists, and is false", AnnSmartCloneRequest, "false", false), - table.Entry("should not reconcile if annotation doesn't exist", "", "true", false), - ) - }) - - var _ = Describe("Smart-clone controller reconcile loop", func() { - var ( - reconciler *SmartCloneReconciler - ) - AfterEach(func() { - if reconciler != nil { - close(reconciler.recorder.(*record.FakeRecorder).Events) - reconciler = nil - } - }) - - It("should return nil if no pvc or snapshot can be found", func() { - reconciler := createSmartCloneReconciler() - _, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) - Expect(err).ToNot(HaveOccurred()) - }) - }) - - var _ = Describe("Smart-clone controller reconcilePVC loop", func() { - var ( - reconciler *SmartCloneReconciler - ) - AfterEach(func() { - if reconciler != nil { - close(reconciler.recorder.(*record.FakeRecorder).Events) - reconciler = nil - } - }) - - It("Should return nil if PVC not bound", func() { - reconciler := createSmartCloneReconciler() - pvc := createPVCWithSnapshotSource("test-dv", "invalid") - pvc.Status.Phase = corev1.ClaimPending - _, err := reconciler.reconcilePvc(reconciler.log, pvc) - Expect(err).ToNot(HaveOccurred()) - }) - - It("Should error with malformed annotation", func() { - reconciler := createSmartCloneReconciler() - pvc := createPVCWithSnapshotSource("test-dv", "invalid") - pvc.Annotations["cdi.kubevirt.io/smartCloneSnapshot"] = "foo/bar/baz" - _, err := reconciler.reconcilePvc(reconciler.log, pvc) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("unexpected key format")) - }) - - It("Should add cloneOf annotation and delete snapshot", func() { - pvc := createPVCWithSnapshotSource("test-dv", "invalid") - snapshot := createSnapshotVolume("invalid", pvc.Namespace, nil) - reconciler := createSmartCloneReconciler(pvc, snapshot) - - _, err := reconciler.reconcilePvc(reconciler.log, pvc) - Expect(err).ToNot(HaveOccurred()) - - pvc2 := &corev1.PersistentVolumeClaim{} - nn := types.NamespacedName{Namespace: pvc.Namespace, Name: pvc.Name} - err = reconciler.client.Get(context.TODO(), nn, pvc2) - Expect(err).ToNot(HaveOccurred()) - Expect(pvc2.Annotations["k8s.io/CloneOf"]).To(Equal("true")) - - nn = types.NamespacedName{Namespace: snapshot.Namespace, Name: snapshot.Name} - err = reconciler.client.Get(context.TODO(), nn, &snapshotv1.VolumeSnapshot{}) - Expect(err).To(HaveOccurred()) - Expect(k8serrors.IsNotFound(err)).To(BeTrue()) - }) - }) - - var _ = Describe("Smart-clone controller reconcileSnapshot loop", func() { - var ( - reconciler *SmartCloneReconciler - ) - AfterEach(func() { - if reconciler != nil { - close(reconciler.recorder.(*record.FakeRecorder).Events) - reconciler = nil - } - }) - - It("Okay if no matching DV can be found", func() { - reconciler := createSmartCloneReconciler() - _, err := reconciler.reconcileSnapshot(reconciler.log, createSnapshotVolume("test-dv", metav1.NamespaceDefault, nil)) - Expect(err).ToNot(HaveOccurred()) - }) - - It("Should do nothing if snapshot deleted", func() { - reconciler := createSmartCloneReconciler() - snapshot := createSnapshotVolume("test-dv", metav1.NamespaceDefault, nil) - ts := metav1.Now() - snapshot.DeletionTimestamp = &ts - _, err := reconciler.reconcileSnapshot(reconciler.log, snapshot) - Expect(err).ToNot(HaveOccurred()) - - nn := types.NamespacedName{Namespace: snapshot.Namespace, Name: snapshot.Name} - err = reconciler.client.Get(context.TODO(), nn, &corev1.PersistentVolumeClaim{}) - Expect(err).To(HaveOccurred()) - Expect(k8serrors.IsNotFound(err)).To(BeTrue()) - }) - - It("Should delete snapshot if DataVolume deleted", func() { - dv := newCloneDataVolume("test-dv") - ts := metav1.Now() - dv.DeletionTimestamp = &ts - snapshot := createSnapshotVolume("invalid", dv.Namespace, nil) - Expect(setAnnOwnedByDataVolume(snapshot, dv)).To(Succeed()) - - reconciler := createSmartCloneReconciler(dv, snapshot) - _, err := reconciler.reconcileSnapshot(reconciler.log, snapshot) - Expect(err).ToNot(HaveOccurred()) - - nn := types.NamespacedName{Namespace: snapshot.Namespace, Name: snapshot.Name} - err = reconciler.client.Get(context.TODO(), nn, &snapshotv1.VolumeSnapshot{}) - Expect(err).To(HaveOccurred()) - Expect(k8serrors.IsNotFound(err)).To(BeTrue()) - }) - - It("Should return nil if snapshot not ready", func() { - dv := newCloneDataVolume("test-dv") - snapshot := createSnapshotVolume("invalid", dv.Namespace, nil) - snapshot.Status = &snapshotv1.VolumeSnapshotStatus{ - ReadyToUse: &[]bool{false}[0], - } - Expect(setAnnOwnedByDataVolume(snapshot, dv)).To(Succeed()) - - reconciler := createSmartCloneReconciler(dv, snapshot) - _, err := reconciler.reconcileSnapshot(reconciler.log, snapshot) - Expect(err).ToNot(HaveOccurred()) - }) - - It("Should create PVC if snapshot ready", func() { - dv := newCloneDataVolume("test-dv") - q, _ := resource.ParseQuantity("500Mi") - // Set annotation and label on DV which we can verify on PVC later - dv.Annotations["test"] = "test-value" - dv.Labels = map[string]string{"test": "test-label"} - - snapshot := createSnapshotVolume(dv.Name, dv.Namespace, nil) - snapshot.Spec.Source = snapshotv1.VolumeSnapshotSource{ - PersistentVolumeClaimName: &[]string{"source"}[0], - } - snapshot.Status = &snapshotv1.VolumeSnapshotStatus{ - ReadyToUse: &[]bool{true}[0], - RestoreSize: &q, - } - Expect(setAnnOwnedByDataVolume(snapshot, dv)).To(Succeed()) - - reconciler := createSmartCloneReconciler(dv, snapshot) - _, err := reconciler.reconcileSnapshot(reconciler.log, snapshot) - Expect(err).ToNot(HaveOccurred()) - - pvc := &corev1.PersistentVolumeClaim{} - nn := types.NamespacedName{Namespace: dv.Namespace, Name: dv.Name} - Expect(reconciler.client.Get(context.TODO(), nn, pvc)).To(Succeed()) - Expect(pvc.Labels[common.AppKubernetesVersionLabel]).To(Equal("v0.0.0-tests")) - Expect(pvc.Labels[common.KubePersistentVolumeFillingUpSuppressLabelKey]).To(Equal(common.KubePersistentVolumeFillingUpSuppressLabelValue)) - Expect(pvc.Labels["test"]).To(Equal("test-label")) - // Verify PVC's annotation - Expect(pvc.Annotations["test"]).To(Equal("test-value")) - event := <-reconciler.recorder.(*record.FakeRecorder).Events - Expect(event).To(ContainSubstring("Creating PVC for smart-clone is in progress")) - }) - }) - - createSnapshotWithRestoreSize := func(size int64) *snapshotv1.VolumeSnapshot { - snapshot := createSnapshotVolume("snapshot", "default", nil) - snapshot.Status.RestoreSize = resource.NewQuantity(size, resource.BinarySI) - return snapshot - } - - table.DescribeTable("newPvcFromSnapshot should return proper size", func(snapshot *snapshotv1.VolumeSnapshot, targetSize, expectedSize int64, expectedError error) { - sizeQuantity := resource.NewQuantity(targetSize, resource.BinarySI) - targetPvcSpec := &corev1.PersistentVolumeClaimSpec{ - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceStorage: *sizeQuantity, - }, - }, - } - pvc, err := newPvcFromSnapshot(&cdiv1.DataVolume{}, "targetPvc", snapshot, targetPvcSpec) - if expectedError == nil { - Expect(err).ToNot(HaveOccurred()) - Expect(pvc).ToNot(BeNil()) - Expect(pvc.Spec.Resources.Requests.Storage().Value()).To(Equal(expectedSize)) - } else { - Expect(err).To(Equal(expectedError)) - } - }, - table.Entry("with nil restoreSize", createSnapshotVolume("snapshot", "default", nil), int64(0), int64(0), fmt.Errorf("snapshot has no RestoreSize")), - table.Entry("with negative restoreSize", createSnapshotWithRestoreSize(int64(-1024)), int64(0), int64(0), fmt.Errorf("snapshot has no RestoreSize")), - table.Entry("with 0 restoreSize, and target size", createSnapshotWithRestoreSize(int64(0)), int64(1024), int64(1024), nil), - table.Entry("with smaller restoreSize than target size", createSnapshotWithRestoreSize(int64(1024)), int64(2048), int64(1024), nil), - ) -}) - -func createSmartCloneReconciler(objects ...runtime.Object) *SmartCloneReconciler { - objs := []runtime.Object{} - objs = append(objs, objects...) - - // Register operator types with the runtime scheme. - s := scheme.Scheme - _ = cdiv1.AddToScheme(s) - _ = snapshotv1.AddToScheme(s) - - cdiConfig := MakeEmptyCDIConfigSpec(common.ConfigName) - cdiConfig.Status = cdiv1.CDIConfigStatus{ - ScratchSpaceStorageClass: testStorageClass, - } - - // Create a fake client to mock API calls. - cl := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(objs...).Build() - - rec := record.NewFakeRecorder(1) - // Create a ReconcileMemcached object with the scheme and fake client. - r := &SmartCloneReconciler{ - client: cl, - scheme: s, - log: scLog, - recorder: rec, - installerLabels: map[string]string{ - common.AppKubernetesPartOfLabel: "testing", - common.AppKubernetesVersionLabel: "v0.0.0-tests", - }, - } - return r -} - -func createPVCWithSnapshotSource(name, snapshotName string) *corev1.PersistentVolumeClaim { - pvc := CreatePvc(name, metav1.NamespaceDefault, map[string]string{}, nil) - pvc.Annotations = map[string]string{ - "cdi.kubevirt.io/smartCloneSnapshot": metav1.NamespaceDefault + "/" + snapshotName, - } - pvc.Spec.DataSource = &corev1.TypedLocalObjectReference{ - Name: snapshotName, - Kind: "VolumeSnapshot", - APIGroup: &snapshotv1.SchemeGroupVersion.Group, - } - pvc.Status.Phase = corev1.ClaimBound - return pvc -} - -func createSnapshotVolume(name, namespace string, owner *metav1.OwnerReference) *snapshotv1.VolumeSnapshot { - var ownerRefs []metav1.OwnerReference - if owner != nil { - ownerRefs = append(ownerRefs, *owner) - } - return &snapshotv1.VolumeSnapshot{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - OwnerReferences: ownerRefs, - Labels: map[string]string{ - common.CDILabelKey: common.CDILabelValue, - common.CDIComponentLabel: common.SmartClonerCDILabel, - }, - }, - Status: &snapshotv1.VolumeSnapshotStatus{}, - } -} diff --git a/pkg/controller/datavolume/snapshot-clone-controller.go b/pkg/controller/datavolume/snapshot-clone-controller.go index 2197872336..b016912690 100644 --- a/pkg/controller/datavolume/snapshot-clone-controller.go +++ b/pkg/controller/datavolume/snapshot-clone-controller.go @@ -31,21 +31,28 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/cache" + "k8s.io/utils/pointer" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/reconcile" cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" "kubevirt.io/containerized-data-importer/pkg/common" - "kubevirt.io/containerized-data-importer/pkg/util" - cc "kubevirt.io/containerized-data-importer/pkg/controller/common" featuregates "kubevirt.io/containerized-data-importer/pkg/feature-gates" - - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller" - "sigs.k8s.io/controller-runtime/pkg/manager" - "sigs.k8s.io/controller-runtime/pkg/reconcile" + "kubevirt.io/containerized-data-importer/pkg/util" ) -const snapshotCloneControllerName = "datavolume-snapshot-clone-controller" +const ( + //AnnSmartCloneRequest sets our expected annotation for a CloneRequest + AnnSmartCloneRequest = "k8s.io/SmartCloneRequest" + + annSmartCloneSnapshot = "cdi.kubevirt.io/smartCloneSnapshot" + + snapshotCloneControllerName = "datavolume-snapshot-clone-controller" +) // SnapshotCloneReconciler members type SnapshotCloneReconciler struct { @@ -76,10 +83,13 @@ func NewSnapshotCloneController( installerLabels: installerLabels, shouldUpdateProgress: true, }, - clonerImage: clonerImage, - importerImage: importerImage, - pullPolicy: pullPolicy, - tokenValidator: cc.NewCloneTokenValidator(common.CloneTokenIssuer, tokenPublicKey), + clonerImage: clonerImage, + importerImage: importerImage, + pullPolicy: pullPolicy, + cloneSourceAPIGroup: pointer.String(snapshotv1.GroupName), + cloneSourceKind: "VolumeSnapshot", + shortTokenValidator: cc.NewCloneTokenValidator(common.CloneTokenIssuer, tokenPublicKey), + longTokenValidator: cc.NewCloneTokenValidator(common.ExtendedCloneTokenIssuer, tokenPublicKey), // for long term tokens to handle cross namespace dumb clones tokenGenerator: newLongTermCloneTokenGenerator(tokenPrivateKey), }, @@ -92,14 +102,14 @@ func NewSnapshotCloneController( return nil, err } - if err := addDataVolumeSnapshotCloneControllerWatches(mgr, dataVolumeCloneController); err != nil { + if err := reconciler.addDataVolumeSnapshotCloneControllerWatches(mgr, dataVolumeCloneController); err != nil { return nil, err } return dataVolumeCloneController, nil } -func addDataVolumeSnapshotCloneControllerWatches(mgr manager.Manager, datavolumeController controller.Controller) error { +func (r *SnapshotCloneReconciler) addDataVolumeSnapshotCloneControllerWatches(mgr manager.Manager, datavolumeController controller.Controller) error { if err := addDataVolumeControllerCommonWatches(mgr, datavolumeController, dataVolumeSnapshotClone); err != nil { return err } @@ -114,10 +124,15 @@ func addDataVolumeSnapshotCloneControllerWatches(mgr manager.Manager, datavolume return err } } + if err := addCloneWithoutSourceWatch(mgr, datavolumeController, &snapshotv1.VolumeSnapshot{}, "spec.source.snapshot"); err != nil { return err } + if err := r.addVolumeCloneSourceWatch(datavolumeController); err != nil { + return err + } + return nil } @@ -139,15 +154,13 @@ func (r *SnapshotCloneReconciler) updateAnnotations(dataVolume *cdiv1.DataVolume if dataVolume.Spec.Source.Snapshot == nil { return errors.Errorf("no source set for clone datavolume") } + if err := addCloneToken(dataVolume, pvc); err != nil { + return err + } sourceNamespace := dataVolume.Spec.Source.Snapshot.Namespace if sourceNamespace == "" { sourceNamespace = dataVolume.Namespace } - token, ok := dataVolume.Annotations[cc.AnnCloneToken] - if !ok { - return errors.Errorf("no clone token") - } - pvc.Annotations[cc.AnnCloneToken] = token tempPvcName := getTempHostAssistedSourcePvcName(dataVolume) pvc.Annotations[cc.AnnCloneRequest] = sourceNamespace + "/" + tempPvcName return nil @@ -170,7 +183,6 @@ func (r *SnapshotCloneReconciler) syncSnapshotClone(log logr.Logger, req reconci pvc := syncRes.pvc pvcSpec := syncRes.pvcSpec datavolume := syncRes.dvMutated - transferName := getTransferName(datavolume) pvcPopulated := pvcIsPopulated(pvc, datavolume) staticProvisionPending := checkStaticProvisionPending(pvc, datavolume) @@ -180,83 +192,142 @@ func (r *SnapshotCloneReconciler) syncSnapshotClone(log logr.Logger, req reconci return syncRes, nil } - // Check if source snapshot exists and do proper validation before attempting to clone - if done, err := r.validateCloneAndSourceSnapshot(&syncRes); err != nil { + if addedToken, err := r.ensureExtendedTokenDV(datavolume); err != nil { return syncRes, err - } else if !done { + } else if addedToken { + // make sure token gets persisted before doing anything else return syncRes, nil } - nn := types.NamespacedName{Namespace: datavolume.Spec.Source.Snapshot.Namespace, Name: datavolume.Spec.Source.Snapshot.Name} - snapshot := &snapshotv1.VolumeSnapshot{} - if err := r.client.Get(context.TODO(), nn, snapshot); err != nil { - return syncRes, err - } - - valid, err := r.isSnapshotValidForClone(snapshot) - if err != nil || !valid { - return syncRes, err - } - - fallBackToHostAssisted, err := r.evaluateFallBackToHostAssistedNeeded(datavolume, pvcSpec, snapshot) - if err != nil { - return syncRes, err - } - if pvc == nil { - if !fallBackToHostAssisted { - res, err := r.reconcileRestoreSnapshot(log, datavolume, snapshot, pvcSpec, transferName, &syncRes) - syncRes.result = &res - return syncRes, err + if datavolume.Spec.Storage != nil { + done, err := r.detectCloneSize(log, &syncRes) + if err != nil { + return syncRes, err + } + if !done { + if syncRes.result == nil { + syncRes.result = &reconcile.Result{} + } + syncRes.result.RequeueAfter = sourceInUseRequeueDuration + // I think pending is more accurate but doing scheduled to be consistent with PVC controller + return syncRes, r.syncCloneStatusPhase(&syncRes, cdiv1.CloneScheduled, nil) + } } - if err := r.createTempHostAssistedSourcePvc(datavolume, snapshot, pvcSpec, &syncRes); err != nil { - return syncRes, err + pvcModifier := r.updateAnnotations + if syncRes.usePopulator { + if isCrossNamespaceClone(datavolume) { + if !cc.HasFinalizer(datavolume, crossNamespaceFinalizer) { + cc.AddFinalizer(datavolume, crossNamespaceFinalizer) + return syncRes, r.syncCloneStatusPhase(&syncRes, cdiv1.CloneScheduled, nil) + } + } + pvcModifier = r.updatePVCForPopulation + } else { + if done, err := r.validateAndInitLegacyClone(&syncRes); err != nil { + return syncRes, err + } else if !done { + return syncRes, nil + } } - targetHostAssistedPvc, err := r.createPvcForDatavolume(datavolume, pvcSpec, r.updateAnnotations) + + targetPvc, err := r.createPvcForDatavolume(datavolume, pvcSpec, pvcModifier) if err != nil { if cc.ErrQuotaExceeded(err) { - syncEventErr := r.syncDataVolumeStatusPhaseWithEvent(&syncRes, cdiv1.Pending, nil, + syncErr = r.syncDataVolumeStatusPhaseWithEvent(&syncRes, cdiv1.Pending, nil, Event{ eventType: corev1.EventTypeWarning, reason: cc.ErrExceededQuota, message: err.Error(), }) - if syncEventErr != nil { - r.log.Error(syncEventErr, "failed sync status phase") + if syncErr != nil { + log.Error(syncErr, "failed to sync DataVolume status with event") } } return syncRes, err } - pvc = targetHostAssistedPvc + pvc = targetPvc } - if fallBackToHostAssisted { - if err := r.ensureExtendedToken(pvc); err != nil { + if syncRes.usePopulator { + if err := r.reconcileVolumeCloneSourceCR(&syncRes); err != nil { return syncRes, err } - return syncRes, syncErr - } - switch pvc.Status.Phase { - case corev1.ClaimBound: - if err := r.setCloneOfOnPvc(pvc); err != nil { - return syncRes, err + ct, ok := pvc.Annotations[cc.AnnCloneType] + if ok { + cc.AddAnnotation(datavolume, cc.AnnCloneType, ct) } + } else { + cc.AddAnnotation(datavolume, cc.AnnCloneType, string(cdiv1.CloneStrategyHostAssisted)) } - shouldBeMarkedWaitForFirstConsumer, err := r.shouldBeMarkedWaitForFirstConsumer(pvc) - if err != nil { + if err := r.ensureExtendedTokenPVC(datavolume, pvc); err != nil { return syncRes, err } - if !shouldBeMarkedWaitForFirstConsumer { - res, err := r.finishClone(log, &syncRes, transferName) - syncRes.result = &res - return syncRes, err + return syncRes, syncErr +} + +func (r *SnapshotCloneReconciler) detectCloneSize(log logr.Logger, syncState *dvSyncState) (bool, error) { + pvcSpec := syncState.pvcSpec + requestedSize := pvcSpec.Resources.Requests[corev1.ResourceStorage] + if !requestedSize.IsZero() { + log.V(3).Info("requested size is set, skipping size detection", "size", requestedSize) + return true, nil } - return syncRes, syncErr + datavolume := syncState.dvMutated + nn := types.NamespacedName{Namespace: datavolume.Spec.Source.Snapshot.Namespace, Name: datavolume.Spec.Source.Snapshot.Name} + snapshot := &snapshotv1.VolumeSnapshot{} + if err := r.client.Get(context.TODO(), nn, snapshot); err != nil { + if k8serrors.IsNotFound(err) { + log.V(3).Info("snapshot source does not exist", "namespace", nn.Namespace, "name", nn.Name) + return false, nil + } + + return false, err + } + + if snapshot.Status == nil || snapshot.Status.RestoreSize == nil { + log.V(3).Info("snapshot source does not have restoreSize", "namespace", nn.Namespace, "name", nn.Name) + return false, nil + } + + pvcSpec.Resources.Requests[corev1.ResourceStorage] = *snapshot.Status.RestoreSize + + log.V(3).Info("set pvc request size", "size", pvcSpec.Resources.Requests[corev1.ResourceStorage]) + + return true, nil +} + +func (r *SnapshotCloneReconciler) validateAndInitLegacyClone(syncState *dvSyncState) (bool, error) { + // Check if source snapshot exists and do proper validation before attempting to clone + if done, err := r.validateCloneAndSourceSnapshot(syncState); err != nil { + return false, err + } else if !done { + return false, nil + } + + datavolume := syncState.dvMutated + pvcSpec := syncState.pvcSpec + nn := types.NamespacedName{Namespace: datavolume.Spec.Source.Snapshot.Namespace, Name: datavolume.Spec.Source.Snapshot.Name} + snapshot := &snapshotv1.VolumeSnapshot{} + if err := r.client.Get(context.TODO(), nn, snapshot); err != nil { + return false, err + } + + valid, err := r.isSnapshotValidForClone(snapshot) + if err != nil || !valid { + return false, err + } + + if err := r.createTempHostAssistedSourcePvc(datavolume, snapshot, pvcSpec, syncState); err != nil { + return false, err + } + + return true, nil } // validateCloneAndSourceSnapshot checks if the source snapshot of a clone exists and does proper validation @@ -291,102 +362,6 @@ func (r *SnapshotCloneReconciler) validateCloneAndSourceSnapshot(syncState *dvSy return true, nil } -func (r *SnapshotCloneReconciler) evaluateFallBackToHostAssistedNeeded(datavolume *cdiv1.DataVolume, pvcSpec *corev1.PersistentVolumeClaimSpec, snapshot *snapshotv1.VolumeSnapshot) (bool, error) { - bindingMode, err := r.getStorageClassBindingMode(pvcSpec.StorageClassName) - if err != nil { - return true, err - } - if bindingMode != nil && *bindingMode == storagev1.VolumeBindingWaitForFirstConsumer { - waitForFirstConsumerEnabled, err := cc.IsWaitForFirstConsumerEnabled(datavolume, r.featureGates) - if err != nil { - return true, err - } - if !waitForFirstConsumerEnabled { - return true, nil - } - } - // Storage and snapshot class validation - targetStorageClass, err := cc.GetStorageClassByName(context.TODO(), r.client, pvcSpec.StorageClassName) - if err != nil { - return true, err - } - valid, err := cc.ValidateSnapshotCloneProvisioners(context.TODO(), r.client, snapshot, targetStorageClass) - if err != nil { - return true, err - } - if !valid { - r.log.V(3).Info("Provisioner differs, need to fall back to host assisted") - return true, nil - } - // Size validation - valid, err = cc.ValidateSnapshotCloneSize(snapshot, pvcSpec, targetStorageClass, r.log) - if err != nil || !valid { - return true, err - } - - if !isCrossNamespaceClone(datavolume) || *bindingMode == storagev1.VolumeBindingImmediate { - return false, nil - } - - return true, nil -} - -func (r *SnapshotCloneReconciler) reconcileRestoreSnapshot(log logr.Logger, - datavolume *cdiv1.DataVolume, - snapshot *snapshotv1.VolumeSnapshot, - pvcSpec *corev1.PersistentVolumeClaimSpec, - transferName string, - syncRes *dvSyncState) (reconcile.Result, error) { - - pvcName := datavolume.Name - - if isCrossNamespaceClone(datavolume) { - pvcName = transferName - result, err := r.doCrossNamespaceClone(log, syncRes, pvcName, datavolume.Spec.Source.Snapshot.Namespace, false, SmartClone) - if result != nil { - return *result, err - } - } - - if datavolume.Status.Phase == cdiv1.NamespaceTransferInProgress { - return reconcile.Result{}, nil - } - - newPvc, err := r.makePvcFromSnapshot(pvcName, datavolume, snapshot, pvcSpec) - if err != nil { - return reconcile.Result{}, err - } - - currentRestoreFromSnapshotPvc := &corev1.PersistentVolumeClaim{} - if err := r.client.Get(context.TODO(), client.ObjectKeyFromObject(newPvc), currentRestoreFromSnapshotPvc); err != nil { - if !k8serrors.IsNotFound(err) { - return reconcile.Result{}, err - } - if err := r.client.Create(context.TODO(), newPvc); err != nil { - if cc.ErrQuotaExceeded(err) { - syncEventErr := r.syncDataVolumeStatusPhaseWithEvent(syncRes, cdiv1.Pending, nil, - Event{ - eventType: corev1.EventTypeWarning, - reason: cc.ErrExceededQuota, - message: err.Error(), - }) - if syncEventErr != nil { - r.log.Error(syncEventErr, "failed sync status phase") - } - } - return reconcile.Result{}, err - } - } else { - if currentRestoreFromSnapshotPvc.Status.Phase == corev1.ClaimBound { - if err := r.setCloneOfOnPvc(currentRestoreFromSnapshotPvc); err != nil { - return reconcile.Result{}, err - } - } - } - - return reconcile.Result{}, r.syncCloneStatusPhase(syncRes, cdiv1.CloneFromSnapshotSourceInProgress, nil) -} - func (r *SnapshotCloneReconciler) createTempHostAssistedSourcePvc(dv *cdiv1.DataVolume, snapshot *snapshotv1.VolumeSnapshot, targetPvcSpec *corev1.PersistentVolumeClaimSpec, syncState *dvSyncState) error { tempPvcName := getTempHostAssistedSourcePvcName(dv) tempHostAssistedSourcePvc, err := r.makePvcFromSnapshot(tempPvcName, dv, snapshot, targetPvcSpec) @@ -500,6 +475,9 @@ func getTempHostAssistedSourcePvcName(dv *cdiv1.DataVolume) string { func (r *SnapshotCloneReconciler) cleanup(syncState *dvSyncState) error { dv := syncState.dvMutated + if err := r.populateSourceIfSourceRef(dv); err != nil { + return err + } // This cleanup should be done if dv is marked for deletion or in case it succeeded if dv.DeletionTimestamp == nil && dv.Status.Phase != cdiv1.Succeeded { @@ -508,16 +486,10 @@ func (r *SnapshotCloneReconciler) cleanup(syncState *dvSyncState) error { r.log.V(3).Info("Cleanup initiated in dv snapshot clone controller") - if err := r.populateSourceIfSourceRef(dv); err != nil { + if err := r.reconcileVolumeCloneSourceCR(syncState); err != nil { return err } - if isCrossNamespaceClone(dv) { - if err := r.cleanupTransfer(dv); err != nil { - return err - } - } - if err := r.cleanupHostAssistedSnapshotClone(dv); err != nil { return err } @@ -582,3 +554,77 @@ func (r *SnapshotCloneReconciler) isSnapshotValidForClone(snapshot *snapshotv1.V } return true, nil } + +func newPvcFromSnapshot(obj metav1.Object, name string, snapshot *snapshotv1.VolumeSnapshot, targetPvcSpec *corev1.PersistentVolumeClaimSpec) (*corev1.PersistentVolumeClaim, error) { + targetPvcSpecCopy := targetPvcSpec.DeepCopy() + restoreSize := snapshot.Status.RestoreSize + if restoreSize == nil || restoreSize.Sign() == -1 { + return nil, fmt.Errorf("snapshot has no RestoreSize") + } + if restoreSize.IsZero() { + reqSize := targetPvcSpec.Resources.Requests[corev1.ResourceStorage] + restoreSize = &reqSize + } + + key, err := cache.MetaNamespaceKeyFunc(snapshot) + if err != nil { + return nil, err + } + + labels := map[string]string{ + "cdi-controller": snapshot.Name, + common.CDILabelKey: common.CDILabelValue, + common.CDIComponentLabel: common.SmartClonerCDILabel, + } + for k, v := range obj.GetLabels() { + labels[k] = v + } + + annotations := map[string]string{ + AnnSmartCloneRequest: "true", + cc.AnnRunningCondition: string(corev1.ConditionFalse), + cc.AnnRunningConditionMessage: cc.CloneComplete, + cc.AnnRunningConditionReason: "Completed", + annSmartCloneSnapshot: key, + } + for k, v := range obj.GetAnnotations() { + annotations[k] = v + } + + if util.ResolveVolumeMode(targetPvcSpecCopy.VolumeMode) == corev1.PersistentVolumeFilesystem { + labels[common.KubePersistentVolumeFillingUpSuppressLabelKey] = common.KubePersistentVolumeFillingUpSuppressLabelValue + } + + target := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: snapshot.Namespace, + Labels: labels, + Annotations: annotations, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + DataSource: &corev1.TypedLocalObjectReference{ + Name: snapshot.Name, + Kind: "VolumeSnapshot", + APIGroup: &snapshotv1.SchemeGroupVersion.Group, + }, + VolumeMode: targetPvcSpecCopy.VolumeMode, + AccessModes: targetPvcSpecCopy.AccessModes, + StorageClassName: targetPvcSpecCopy.StorageClassName, + Resources: targetPvcSpecCopy.Resources, + }, + } + + if target.Spec.Resources.Requests == nil { + target.Spec.Resources.Requests = corev1.ResourceList{} + } + + target.Spec.Resources.Requests[corev1.ResourceStorage] = *restoreSize + + ownerRef := metav1.GetControllerOf(snapshot) + if ownerRef != nil { + target.OwnerReferences = append(target.OwnerReferences, *ownerRef) + } + + return target, nil +} diff --git a/pkg/controller/datavolume/snapshot-clone-controller_test.go b/pkg/controller/datavolume/snapshot-clone-controller_test.go index 5ad27d5e92..d43e7c8bb9 100644 --- a/pkg/controller/datavolume/snapshot-clone-controller_test.go +++ b/pkg/controller/datavolume/snapshot-clone-controller_test.go @@ -19,12 +19,15 @@ package datavolume import ( "context" "fmt" + "strings" . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" corev1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" @@ -33,13 +36,17 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/record" + "k8s.io/utils/pointer" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" "kubevirt.io/containerized-data-importer/pkg/common" + "kubevirt.io/containerized-data-importer/pkg/controller/clone" . "kubevirt.io/containerized-data-importer/pkg/controller/common" + "kubevirt.io/containerized-data-importer/pkg/controller/populators" featuregates "kubevirt.io/containerized-data-importer/pkg/feature-gates" ) @@ -93,40 +100,6 @@ var _ = Describe("All DataVolume Tests", func() { } } - It("Should create a restore PVC if snapclass exists and no reason to fall back to host assisted", func() { - dv := newCloneFromSnapshotDataVolume("test-dv") - scName := "testsc" - expectedSnapshotClass := "snap-class" - sc := CreateStorageClassWithProvisioner(scName, map[string]string{ - AnnDefaultStorageClass: "true", - }, map[string]string{}, "csi-plugin") - sp := createStorageProfile(scName, []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}, BlockMode) - - dv.Spec.PVC.StorageClassName = &scName - snapshot := createSnapshotInVolumeSnapshotClass("test-snap", metav1.NamespaceDefault, &expectedSnapshotClass, nil, nil, true) - snapClass := createSnapshotClass(expectedSnapshotClass, nil, "csi-plugin") - reconciler = createSnapshotCloneReconciler(sc, sp, dv, snapshot, snapClass, createDefaultVolumeSnapshotContent(), createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd()) - _, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) - Expect(err).ToNot(HaveOccurred()) - - By("Verifying that target PVC now exists") - pvc := &corev1.PersistentVolumeClaim{} - err = reconciler.client.Get(context.TODO(), types.NamespacedName{Namespace: dv.Namespace, Name: dv.Name}, pvc) - Expect(err).ToNot(HaveOccurred()) - expectedDataSource := &corev1.TypedLocalObjectReference{ - Name: snapshot.Name, - Kind: "VolumeSnapshot", - APIGroup: &snapshotv1.SchemeGroupVersion.Group, - } - Expect(pvc.Spec.DataSource).To(Equal(expectedDataSource)) - Expect(pvc.Labels[common.AppKubernetesPartOfLabel]).To(Equal("testing")) - - dv = &cdiv1.DataVolume{} - err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv) - Expect(err).ToNot(HaveOccurred()) - Expect(dv.Status.Phase).To(Equal(cdiv1.CloneFromSnapshotSourceInProgress)) - }) - It("Should fall back to host assisted when target DV storage class has different provisioner", func() { dv := newCloneFromSnapshotDataVolume("test-dv") scName := "testsc" @@ -236,6 +209,268 @@ var _ = Describe("All DataVolume Tests", func() { err = reconciler.client.Get(context.TODO(), types.NamespacedName{Namespace: tempHostAssistedPvc.Namespace, Name: tempHostAssistedPvc.Name}, tempHostAssistedPvc) Expect(k8serrors.IsNotFound(err)).To(BeTrue()) }) + + var _ = Describe("Snapshot clone controller populator integration", func() { + Context("with CSI provisioner", func() { + const ( + pluginName = "csi-plugin" + ) + + var ( + scName = "testSC" + storageClass *storagev1.StorageClass + csiDriver = &storagev1.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: pluginName, + }, + } + expectedSnapshotClass = "snap-class" + ) + + BeforeEach(func() { + storageClass = CreateStorageClassWithProvisioner(scName, map[string]string{AnnDefaultStorageClass: "true"}, map[string]string{}, pluginName) + }) + + It("should add extended token", func() { + dv := newCloneFromSnapshotDataVolumeWithPVCNS("test-dv", "source-ns") + snapshot := createSnapshotInVolumeSnapshotClass("test-snap", "source-ns", &expectedSnapshotClass, nil, nil, true) + reconciler = createSnapshotCloneReconciler(storageClass, csiDriver, dv, snapshot) + result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Requeue).To(BeFalse()) + Expect(result.RequeueAfter).To(BeZero()) + dv = &cdiv1.DataVolume{} + err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv) + Expect(err).ToNot(HaveOccurred()) + Expect(dv.Annotations).To(HaveKey(AnnExtendedCloneToken)) + }) + + It("should add finalizer for cross namespace clone", func() { + dv := newCloneFromSnapshotDataVolumeWithPVCNS("test-dv", "source-ns") + dv.Annotations[AnnExtendedCloneToken] = "test-token" + snapshot := createSnapshotInVolumeSnapshotClass("test-snap", "source-ns", &expectedSnapshotClass, nil, nil, true) + reconciler = createSnapshotCloneReconciler(storageClass, csiDriver, dv, snapshot) + result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Requeue).To(BeFalse()) + Expect(result.RequeueAfter).To(BeZero()) + dv = &cdiv1.DataVolume{} + err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv) + Expect(err).ToNot(HaveOccurred()) + Expect(dv.Finalizers).To(ContainElement(crossNamespaceFinalizer)) + Expect(dv.Status.Phase).To(Equal(cdiv1.CloneScheduled)) + }) + + DescribeTable("should create PVC and VolumeCloneSource CR", func(sourceNamespace string) { + dv := newCloneFromSnapshotDataVolumeWithPVCNS("test-dv", sourceNamespace) + dv.Annotations[AnnExtendedCloneToken] = "foobar" + if sourceNamespace != dv.Namespace { + dv.Finalizers = append(dv.Finalizers, crossNamespaceFinalizer) + } + snapshot := createSnapshotInVolumeSnapshotClass("test-snap", sourceNamespace, &expectedSnapshotClass, nil, nil, true) + reconciler = createSnapshotCloneReconciler(storageClass, csiDriver, dv, snapshot) + result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Requeue).To(BeFalse()) + Expect(result.RequeueAfter).To(BeZero()) + dv = &cdiv1.DataVolume{} + err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv) + Expect(err).ToNot(HaveOccurred()) + pvc := &corev1.PersistentVolumeClaim{} + err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, pvc) + Expect(err).ToNot(HaveOccurred()) + Expect(pvc.Labels[common.AppKubernetesPartOfLabel]).To(Equal("testing")) + Expect(pvc.Labels[common.KubePersistentVolumeFillingUpSuppressLabelKey]).To(Equal(common.KubePersistentVolumeFillingUpSuppressLabelValue)) + Expect(pvc.Spec.DataSourceRef).ToNot(BeNil()) + if sourceNamespace != dv.Namespace { + Expect(pvc.Annotations[populators.AnnDataSourceNamespace]).To(Equal(sourceNamespace)) + } else { + Expect(pvc.Annotations).ToNot(HaveKey(populators.AnnDataSourceNamespace)) + } + cloneSourceName := volumeCloneSourceName(dv) + Expect(pvc.Spec.DataSourceRef.Name).To(Equal(cloneSourceName)) + Expect(pvc.Spec.DataSourceRef.Kind).To(Equal(cdiv1.VolumeCloneSourceRef)) + Expect(pvc.GetAnnotations()[AnnUsePopulator]).To(Equal("true")) + vcs := &cdiv1.VolumeCloneSource{} + err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: cloneSourceName, Namespace: sourceNamespace}, vcs) + Expect(err).ToNot(HaveOccurred()) + Expect(vcs.Spec.Source.APIGroup).ToNot(BeNil()) + Expect(*vcs.Spec.Source.APIGroup).To(Equal("snapshot.storage.k8s.io")) + Expect(vcs.Spec.Source.Kind).To(Equal("VolumeSnapshot")) + Expect(vcs.Spec.Source.Name).To(Equal(snapshot.Name)) + }, + Entry("with same namespace", metav1.NamespaceDefault), + Entry("with different namespace", "source-ns"), + ) + + It("should handle size omitted", func() { + dv := newCloneFromSnapshotDataVolume("test-dv") + vm := corev1.PersistentVolumeFilesystem + dv.Spec.Storage = &cdiv1.StorageSpec{ + AccessModes: dv.Spec.PVC.AccessModes, + VolumeMode: &vm, + } + dv.Spec.PVC = nil + snapshot := createSnapshotInVolumeSnapshotClass("test-snap", dv.Namespace, &expectedSnapshotClass, nil, nil, true) + reconciler = createSnapshotCloneReconciler(storageClass, csiDriver, dv, snapshot) + result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Requeue).To(BeFalse()) + Expect(result.RequeueAfter).To(BeZero()) + dv = &cdiv1.DataVolume{} + err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv) + Expect(err).ToNot(HaveOccurred()) + pvc := &corev1.PersistentVolumeClaim{} + err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, pvc) + Expect(err).ToNot(HaveOccurred()) + Expect(pvc.Spec.Resources.Requests[corev1.ResourceStorage]).To(Equal(*snapshot.Status.RestoreSize)) + }) + + It("should add cloneType annotation", func() { + dv := newCloneFromSnapshotDataVolume("test-dv") + anno := map[string]string{ + AnnExtendedCloneToken: "test-token", + AnnCloneType: string(cdiv1.CloneStrategySnapshot), + AnnUsePopulator: "true", + } + pvc := CreatePvcInStorageClass("test-dv", metav1.NamespaceDefault, &scName, anno, nil, corev1.ClaimPending) + pvc.Spec.DataSourceRef = &corev1.TypedObjectReference{ + Kind: cdiv1.VolumeCloneSourceRef, + Name: volumeCloneSourceName(dv), + } + pvc.OwnerReferences = append(pvc.OwnerReferences, metav1.OwnerReference{ + Kind: "DataVolume", + Controller: pointer.Bool(true), + Name: "test-dv", + UID: dv.UID, + }) + vcs := &cdiv1.VolumeCloneSource{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metav1.NamespaceDefault, + Name: volumeCloneSourceName(dv), + }, + Spec: cdiv1.VolumeCloneSourceSpec{ + Source: corev1.TypedLocalObjectReference{ + APIGroup: pointer.String("snapshot.storage.k8s.io"), + Kind: "VolumeSnapshot", + Name: dv.Spec.Source.Snapshot.Name, + }, + }, + } + reconciler = createSnapshotCloneReconciler(storageClass, csiDriver, dv, pvc, vcs) + result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Requeue).To(BeFalse()) + Expect(result.RequeueAfter).To(BeZero()) + dv = &cdiv1.DataVolume{} + err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv) + Expect(err).ToNot(HaveOccurred()) + Expect(dv.Annotations[AnnCloneType]).To(Equal(string(cdiv1.CloneStrategySnapshot))) + }) + + DescribeTable("should map phase correctly", func(phaseName string, dvPhase cdiv1.DataVolumePhase, eventReason string) { + dv := newCloneFromSnapshotDataVolume("test-dv") + anno := map[string]string{ + AnnExtendedCloneToken: "test-token", + AnnCloneType: string(cdiv1.CloneStrategySnapshot), + populators.AnnClonePhase: phaseName, + AnnUsePopulator: "true", + } + pvc := CreatePvcInStorageClass("test-dv", metav1.NamespaceDefault, &scName, anno, nil, corev1.ClaimPending) + pvc.Spec.DataSourceRef = &corev1.TypedObjectReference{ + Kind: cdiv1.VolumeCloneSourceRef, + Name: volumeCloneSourceName(dv), + } + pvc.OwnerReferences = append(pvc.OwnerReferences, metav1.OwnerReference{ + Kind: "DataVolume", + Controller: pointer.Bool(true), + Name: "test-dv", + UID: dv.UID, + }) + vcs := &cdiv1.VolumeCloneSource{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metav1.NamespaceDefault, + Name: volumeCloneSourceName(dv), + }, + Spec: cdiv1.VolumeCloneSourceSpec{ + Source: corev1.TypedLocalObjectReference{ + APIGroup: pointer.String("snapshot.storage.k8s.io"), + Kind: "VolumeSnapshot", + Name: dv.Spec.Source.Snapshot.Name, + }, + }, + } + reconciler = createSnapshotCloneReconciler(storageClass, csiDriver, dv, pvc, vcs) + result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Requeue).To(BeFalse()) + Expect(result.RequeueAfter).To(BeZero()) + dv = &cdiv1.DataVolume{} + err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv) + Expect(err).ToNot(HaveOccurred()) + Expect(dv.Status.Phase).To(Equal(dvPhase)) + found := false + for event := range reconciler.recorder.(*record.FakeRecorder).Events { + if strings.Contains(event, eventReason) { + found = true + break + } + } + Expect(found).To(BeTrue()) + }, + Entry("empty phase", "", cdiv1.CloneScheduled, CloneScheduled), + Entry("pending phase", clone.PendingPhaseName, cdiv1.CloneScheduled, CloneScheduled), + Entry("succeeded phase", clone.SucceededPhaseName, cdiv1.Succeeded, CloneSucceeded), + Entry("host clone phase", clone.HostClonePhaseName, cdiv1.CloneInProgress, CloneInProgress), + Entry("prep claim phase", clone.PrepClaimPhaseName, cdiv1.PrepClaimInProgress, PrepClaimInProgress), + Entry("rebind phase", clone.RebindPhaseName, cdiv1.RebindInProgress, RebindInProgress), + Entry("pvc from snapshot phase", clone.SnapshotClonePhaseName, cdiv1.CloneFromSnapshotSourceInProgress, CloneFromSnapshotSourceInProgress), + ) + + It("should delete VolumeCloneSource on success", func() { + dv := newCloneFromSnapshotDataVolume("test-dv") + dv.Status.Phase = cdiv1.Succeeded + anno := map[string]string{ + AnnExtendedCloneToken: "test-token", + AnnCloneType: string(cdiv1.CloneStrategySnapshot), + populators.AnnClonePhase: clone.SucceededPhaseName, + AnnUsePopulator: "true", + } + pvc := CreatePvcInStorageClass("test-dv", metav1.NamespaceDefault, &scName, anno, nil, corev1.ClaimPending) + pvc.Spec.DataSourceRef = &corev1.TypedObjectReference{ + Kind: cdiv1.VolumeCloneSourceRef, + Name: volumeCloneSourceName(dv), + } + pvc.OwnerReferences = append(pvc.OwnerReferences, metav1.OwnerReference{ + Kind: "DataVolume", + Controller: pointer.Bool(true), + Name: "test-dv", + UID: dv.UID, + }) + vcs := &cdiv1.VolumeCloneSource{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metav1.NamespaceDefault, + Name: volumeCloneSourceName(dv), + }, + Spec: cdiv1.VolumeCloneSourceSpec{ + Source: corev1.TypedLocalObjectReference{ + APIGroup: pointer.String("snapshot.storage.k8s.io"), + Kind: "VolumeSnapshot", + Name: dv.Spec.Source.Snapshot.Name, + }, + }, + } + reconciler = createSnapshotCloneReconciler(storageClass, csiDriver, dv, pvc, vcs) + result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Requeue).To(BeFalse()) + Expect(result.RequeueAfter).To(BeZero()) + err = reconciler.client.Get(context.TODO(), client.ObjectKeyFromObject(vcs), vcs) + Expect(err).To(HaveOccurred()) + Expect(k8serrors.IsNotFound(err)).To(BeTrue()) + }) + }) + }) }) }) @@ -292,8 +527,11 @@ func createSnapshotCloneReconcilerWithoutConfig(objects ...runtime.Object) *Snap }, shouldUpdateProgress: true, }, - tokenValidator: &FakeValidator{Match: "foobar"}, - tokenGenerator: &FakeGenerator{token: "foobar"}, + shortTokenValidator: &FakeValidator{Match: "foobar"}, + longTokenValidator: &FakeValidator{Match: "foobar", Params: map[string]string{"uid": "uid"}}, + tokenGenerator: &FakeGenerator{token: "foobar"}, + cloneSourceAPIGroup: pointer.String("snapshot.storage.k8s.io"), + cloneSourceKind: "VolumeSnapshot", }, } return r diff --git a/pkg/controller/populators/BUILD.bazel b/pkg/controller/populators/BUILD.bazel index 1665bf95d3..880193be2d 100644 --- a/pkg/controller/populators/BUILD.bazel +++ b/pkg/controller/populators/BUILD.bazel @@ -52,6 +52,7 @@ go_test( "//pkg/controller/clone:go_default_library", "//pkg/controller/common:go_default_library", "//pkg/feature-gates:go_default_library", + "//pkg/token:go_default_library", "//staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1:go_default_library", "//tests/reporters:go_default_library", "//vendor/github.com/go-logr/logr:go_default_library", diff --git a/pkg/controller/populators/clone-populator.go b/pkg/controller/populators/clone-populator.go index fc51a8b41c..efd6531395 100644 --- a/pkg/controller/populators/clone-populator.go +++ b/pkg/controller/populators/clone-populator.go @@ -18,6 +18,7 @@ package populators import ( "context" + "crypto/rsa" "fmt" "time" @@ -47,17 +48,20 @@ const ( // AnnCloneError has the error string for error phase AnnCloneError = "cdi.kubevirt.io/cloneError" + // AnnDataSourceNamespace has the namespace of the DataSource + // this will be deprecated when cross namespace datasource goes beta + AnnDataSourceNamespace = "cdi.kubevirt.io/dataSourceNamespace" + clonePopulatorName = "clone-populator" cloneFinalizer = "cdi.kubevirt.io/clonePopulator" - - pendingPhase = "Pending" - - succeededPhase = "Succeeded" - - errorPhase = "Error" ) +var desiredCloneAnnotations = map[string]struct{}{ + cc.AnnPreallocationApplied: {}, + cc.AnnCloneOf: {}, +} + // Planner is an interface to mock out planner implementation for testing type Planner interface { ChooseStrategy(context.Context, *clone.ChooseStrategyArgs) (*cdiv1.CDICloneStrategy, error) @@ -68,7 +72,8 @@ type Planner interface { // ClonePopulatorReconciler reconciles PVCs with VolumeCloneSources type ClonePopulatorReconciler struct { ReconcilerBase - planner Planner + planner Planner + multiTokenValidator *cc.MultiTokenValidator } // NewClonePopulator creates a new instance of the clone-populator controller @@ -79,6 +84,7 @@ func NewClonePopulator( clonerImage string, pullPolicy string, installerLabels map[string]string, + publicKey *rsa.PublicKey, ) (controller.Controller, error) { client := mgr.GetClient() reconciler := &ClonePopulatorReconciler{ @@ -91,6 +97,7 @@ func NewClonePopulator( sourceKind: cdiv1.VolumeCloneSourceRef, installerLabels: installerLabels, }, + multiTokenValidator: cc.NewMultiTokenValidator(publicKey), } clonePopulator, err := controller.New(clonePopulatorName, mgr, controller.Options{ @@ -138,70 +145,69 @@ func (r *ClonePopulatorReconciler) Reconcile(ctx context.Context, req reconcile. return reconcile.Result{}, err } - // replace this eventually - if pvc.Spec.DataSourceRef != nil && - pvc.Spec.DataSourceRef.Namespace != nil && - *pvc.Spec.DataSourceRef.Namespace != pvc.Namespace { - return reconcile.Result{}, r.updateClonePhaseError(ctx, pvc, fmt.Errorf("cross namespace datasource not supported yet")) + // don't think this should happen but better safe than sorry + if !IsPVCDataSourceRefKind(pvc, cdiv1.VolumeCloneSourceRef) { + return reconcile.Result{}, nil } hasFinalizer := cc.HasFinalizer(pvc, cloneFinalizer) isBound := cc.IsBound(pvc) isDeleted := !pvc.DeletionTimestamp.IsZero() + isSucceeded := isClonePhaseSucceeded(pvc) - log.V(3).Info("pvc state", "hasFinalizer", hasFinalizer, "isBound", isBound, "isDeleted", isDeleted) + log.V(3).Info("pvc state", "hasFinalizer", hasFinalizer, + "isBound", isBound, "isDeleted", isDeleted, "isSucceeded", isSucceeded) - if !isDeleted && !isBound { - return r.reconcilePending(ctx, log, pvc) + if !isDeleted && !isSucceeded { + return r.reconcilePending(ctx, log, pvc, isBound) } if hasFinalizer { - if isBound && !isDeleted && !isClonePhaseSucceeded(pvc) { - log.V(1).Info("setting phase to Succeeded") - return reconcile.Result{}, r.updateClonePhaseSucceeded(ctx, pvc) - } - return r.reconcileDone(ctx, log, pvc) } return reconcile.Result{}, nil } -func (r *ClonePopulatorReconciler) reconcilePending(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim) (reconcile.Result, error) { +func (r *ClonePopulatorReconciler) reconcilePending(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim, statusOnly bool) (reconcile.Result, error) { ready, _, err := claimReadyForPopulation(ctx, r.client, pvc) if err != nil { - return reconcile.Result{}, r.updateClonePhaseError(ctx, pvc, err) + return reconcile.Result{}, r.updateClonePhaseError(ctx, log, pvc, err) } if !ready { log.V(3).Info("claim not ready for population, exiting") - return reconcile.Result{}, r.updateClonePhasePending(ctx, pvc) + return reconcile.Result{}, r.updateClonePhasePending(ctx, log, pvc) } - vcs, err := getVolumeCloneSource(ctx, r.client, pvc) + vcs, err := r.getVolumeCloneSource(ctx, log, pvc) if err != nil { - return reconcile.Result{}, r.updateClonePhaseError(ctx, pvc, err) + return reconcile.Result{}, r.updateClonePhaseError(ctx, log, pvc, err) } if vcs == nil { log.V(3).Info("dataSourceRef does not exist, exiting") - return reconcile.Result{}, r.updateClonePhasePending(ctx, pvc) + return reconcile.Result{}, r.updateClonePhasePending(ctx, log, pvc) + } + + if err = r.validateCrossNamespace(pvc, vcs); err != nil { + return reconcile.Result{}, r.updateClonePhaseError(ctx, log, pvc, err) } cs, err := r.getCloneStrategy(ctx, log, pvc, vcs) if err != nil { - return reconcile.Result{}, r.updateClonePhaseError(ctx, pvc, err) + return reconcile.Result{}, r.updateClonePhaseError(ctx, log, pvc, err) } if cs == nil { log.V(3).Info("unable to choose clone strategy now") // TODO maybe create index/watch to deal with this - return reconcile.Result{RequeueAfter: 5 * time.Second}, r.updateClonePhasePending(ctx, pvc) + return reconcile.Result{RequeueAfter: 5 * time.Second}, r.updateClonePhasePending(ctx, log, pvc) } - updated, err := r.initTargetClaim(ctx, pvc, vcs, *cs) + updated, err := r.initTargetClaim(ctx, log, pvc, vcs, *cs) if err != nil { - return reconcile.Result{}, r.updateClonePhaseError(ctx, pvc, err) + return reconcile.Result{}, r.updateClonePhaseError(ctx, log, pvc, err) } if updated { @@ -217,7 +223,7 @@ func (r *ClonePopulatorReconciler) reconcilePending(ctx context.Context, log log Strategy: *cs, } - return r.planAndExecute(ctx, log, pvc, args) + return r.planAndExecute(ctx, log, pvc, statusOnly, args) } func (r *ClonePopulatorReconciler) getCloneStrategy(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim, vcs *cdiv1.VolumeCloneSource) (*cdiv1.CDICloneStrategy, error) { @@ -240,37 +246,61 @@ func (r *ClonePopulatorReconciler) getCloneStrategy(ctx context.Context, log log return cs, nil } -func (r *ClonePopulatorReconciler) planAndExecute(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim, args *clone.PlanArgs) (reconcile.Result, error) { +func (r *ClonePopulatorReconciler) planAndExecute(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim, statusOnly bool, args *clone.PlanArgs) (reconcile.Result, error) { phases, err := r.planner.Plan(ctx, args) if err != nil { - return reconcile.Result{}, r.updateClonePhaseError(ctx, pvc, err) + return reconcile.Result{}, r.updateClonePhaseError(ctx, log, pvc, err) } log.V(3).Info("created phases", "num", len(phases)) + var statusResults []*clone.PhaseStatus for _, p := range phases { + var result *reconcile.Result + var err error var progress string - result, err := p.Reconcile(ctx) - if err != nil { - return reconcile.Result{}, r.updateClonePhaseError(ctx, pvc, err) + if !statusOnly { + result, err = p.Reconcile(ctx) + if err != nil { + return reconcile.Result{}, r.updateClonePhaseError(ctx, log, pvc, err) + } } - if pr, ok := p.(clone.ProgressReporter); ok { - progress, err = pr.Progress(ctx) + if sr, ok := p.(clone.StatusReporter); ok { + ps, err := sr.Status(ctx) if err != nil { - return reconcile.Result{}, r.updateClonePhaseError(ctx, pvc, err) + return reconcile.Result{}, r.updateClonePhaseError(ctx, log, pvc, err) } + progress = ps.Progress + statusResults = append(statusResults, ps) } if result != nil { log.V(1).Info("currently in phase, returning", "name", p.Name(), "progress", progress) - return *result, r.updateClonePhase(ctx, pvc, p.Name(), progress) + return *result, r.updateClonePhase(ctx, log, pvc, p.Name(), statusResults) } } log.V(3).Info("executed all phases, setting phase to Succeeded") - return reconcile.Result{}, r.updateClonePhaseSucceeded(ctx, pvc) + return reconcile.Result{}, r.updateClonePhaseSucceeded(ctx, log, pvc, statusResults) +} + +func (r *ClonePopulatorReconciler) validateCrossNamespace(pvc *corev1.PersistentVolumeClaim, vcs *cdiv1.VolumeCloneSource) error { + if pvc.Namespace == vcs.Namespace { + return nil + } + + anno, ok := pvc.Annotations[AnnDataSourceNamespace] + if ok && anno == vcs.Namespace { + if err := r.multiTokenValidator.ValidatePopulator(vcs, pvc); err != nil { + return err + } + + return nil + } + + return fmt.Errorf("cross-namespace with resource grants is not supported yet") } func (r *ClonePopulatorReconciler) reconcileDone(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim) (reconcile.Result, error) { @@ -280,16 +310,17 @@ func (r *ClonePopulatorReconciler) reconcileDone(ctx context.Context, log logr.L } log.V(1).Info("removing finalizer") - cc.RemoveFinalizer(pvc, cloneFinalizer) - return reconcile.Result{}, r.client.Update(ctx, pvc) + claimCpy := pvc.DeepCopy() + cc.RemoveFinalizer(claimCpy, cloneFinalizer) + return reconcile.Result{}, r.client.Update(ctx, claimCpy) } -func (r *ClonePopulatorReconciler) initTargetClaim(ctx context.Context, pvc *corev1.PersistentVolumeClaim, vcs *cdiv1.VolumeCloneSource, cs cdiv1.CDICloneStrategy) (bool, error) { +func (r *ClonePopulatorReconciler) initTargetClaim(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim, vcs *cdiv1.VolumeCloneSource, cs cdiv1.CDICloneStrategy) (bool, error) { claimCpy := pvc.DeepCopy() clone.AddCommonClaimLabels(claimCpy) setSavedCloneStrategy(claimCpy, cs) if claimCpy.Annotations[AnnClonePhase] == "" { - cc.AddAnnotation(claimCpy, AnnClonePhase, pendingPhase) + cc.AddAnnotation(claimCpy, AnnClonePhase, clone.PendingPhaseName) } cc.AddFinalizer(claimCpy, cloneFinalizer) @@ -304,22 +335,38 @@ func (r *ClonePopulatorReconciler) initTargetClaim(ctx context.Context, pvc *cor return false, nil } -func (r *ClonePopulatorReconciler) updateClonePhasePending(ctx context.Context, pvc *corev1.PersistentVolumeClaim) error { - return r.updateClonePhase(ctx, pvc, pendingPhase, "") +func (r *ClonePopulatorReconciler) updateClonePhasePending(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim) error { + return r.updateClonePhase(ctx, log, pvc, clone.PendingPhaseName, nil) } -func (r *ClonePopulatorReconciler) updateClonePhaseSucceeded(ctx context.Context, pvc *corev1.PersistentVolumeClaim) error { - return r.updateClonePhase(ctx, pvc, succeededPhase, cc.ProgressDone) +func (r *ClonePopulatorReconciler) updateClonePhaseSucceeded(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim, status []*clone.PhaseStatus) error { + if status == nil { + status = []*clone.PhaseStatus{{}} + } + status[len(status)-1].Progress = cc.ProgressDone + return r.updateClonePhase(ctx, log, pvc, clone.SucceededPhaseName, status) } -func (r *ClonePopulatorReconciler) updateClonePhase(ctx context.Context, pvc *corev1.PersistentVolumeClaim, phase, progress string) error { +func (r *ClonePopulatorReconciler) updateClonePhase(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim, phase string, status []*clone.PhaseStatus) error { claimCpy := pvc.DeepCopy() delete(claimCpy.Annotations, AnnCloneError) cc.AddAnnotation(claimCpy, AnnClonePhase, phase) - if progress != "" { - cc.AddAnnotation(claimCpy, cc.AnnPopulatorProgress, progress) + + var mergedAnnotations = make(map[string]string) + for _, ps := range status { + if ps.Progress != "" { + cc.AddAnnotation(claimCpy, cc.AnnPopulatorProgress, ps.Progress) + } + for k, v := range ps.Annotations { + mergedAnnotations[k] = v + if _, ok := desiredCloneAnnotations[k]; ok { + cc.AddAnnotation(claimCpy, k, v) + } + } } + r.addRunningAnnotations(claimCpy, phase, mergedAnnotations) + if !apiequality.Semantic.DeepEqual(pvc, claimCpy) { return r.client.Update(ctx, claimCpy) } @@ -327,43 +374,68 @@ func (r *ClonePopulatorReconciler) updateClonePhase(ctx context.Context, pvc *co return nil } -func (r *ClonePopulatorReconciler) updateClonePhaseError(ctx context.Context, pvc *corev1.PersistentVolumeClaim, lastError error) error { +func (r *ClonePopulatorReconciler) updateClonePhaseError(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim, lastError error) error { claimCpy := pvc.DeepCopy() - cc.AddAnnotation(claimCpy, AnnClonePhase, errorPhase) + cc.AddAnnotation(claimCpy, AnnClonePhase, clone.ErrorPhaseName) cc.AddAnnotation(claimCpy, AnnCloneError, lastError.Error()) + r.addRunningAnnotations(claimCpy, clone.ErrorPhaseName, nil) + if !apiequality.Semantic.DeepEqual(pvc, claimCpy) { if err := r.client.Update(ctx, claimCpy); err != nil { - r.log.V(1).Info("error setting error annotations") + log.V(1).Info("error setting error annotations") } } return lastError } -func isClonePhaseSucceeded(obj client.Object) bool { - return obj.GetAnnotations()[AnnClonePhase] == succeededPhase -} - -func getSavedCloneStrategy(obj client.Object) *cdiv1.CDICloneStrategy { - if val, ok := obj.GetAnnotations()[cc.AnnCloneType]; ok { - cs := cdiv1.CDICloneStrategy(val) - return &cs +func (r *ClonePopulatorReconciler) addRunningAnnotations(pvc *corev1.PersistentVolumeClaim, phase string, annotations map[string]string) { + if !cc.OwnedByDataVolume(pvc) { + return + } + + var running, message, reason string + if phase == clone.SucceededPhaseName { + running = "false" + message = "Clone Complete" + reason = "Completed" + } else if phase == clone.PendingPhaseName { + running = "false" + message = "Clone Pending" + reason = "Pending" + } else if phase == clone.ErrorPhaseName { + running = "false" + message = pvc.Annotations[AnnCloneError] + reason = "Error" + } else if _, ok := annotations[cc.AnnRunningCondition]; ok { + running = annotations[cc.AnnRunningCondition] + message = annotations[cc.AnnRunningConditionMessage] + reason = annotations[cc.AnnRunningConditionReason] + if restarts, ok := annotations[cc.AnnPodRestarts]; ok { + cc.AddAnnotation(pvc, cc.AnnPodRestarts, restarts) + } + } else { + running = "true" + reason = "Populator is running" } - return nil -} -func setSavedCloneStrategy(obj client.Object, strategy cdiv1.CDICloneStrategy) { - cc.AddAnnotation(obj, cc.AnnCloneType, string(strategy)) + cc.AddAnnotation(pvc, cc.AnnRunningCondition, running) + cc.AddAnnotation(pvc, cc.AnnRunningConditionMessage, message) + cc.AddAnnotation(pvc, cc.AnnRunningConditionReason, reason) } -func getVolumeCloneSource(ctx context.Context, c client.Client, pvc *corev1.PersistentVolumeClaim) (*cdiv1.VolumeCloneSource, error) { +func (r *ClonePopulatorReconciler) getVolumeCloneSource(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim) (*cdiv1.VolumeCloneSource, error) { if !IsPVCDataSourceRefKind(pvc, cdiv1.VolumeCloneSourceRef) { - return nil, nil + return nil, fmt.Errorf("pvc %s/%s does not refer to a %s", pvc.Namespace, pvc.Name, cdiv1.VolumeCloneSourceRef) } ns := pvc.Namespace - if pvc.Spec.DataSourceRef.Namespace != nil { + anno, ok := pvc.Annotations[AnnDataSourceNamespace] + if ok { + log.V(3).Info("found datasource namespace annotation", "namespace", ns) + ns = anno + } else if pvc.Spec.DataSourceRef.Namespace != nil { ns = *pvc.Spec.DataSourceRef.Namespace } @@ -374,7 +446,7 @@ func getVolumeCloneSource(ctx context.Context, c client.Client, pvc *corev1.Pers }, } - if err := c.Get(ctx, client.ObjectKeyFromObject(obj), obj); err != nil { + if err := r.client.Get(ctx, client.ObjectKeyFromObject(obj), obj); err != nil { if k8serrors.IsNotFound(err) { return nil, nil } @@ -384,3 +456,19 @@ func getVolumeCloneSource(ctx context.Context, c client.Client, pvc *corev1.Pers return obj, nil } + +func isClonePhaseSucceeded(obj client.Object) bool { + return obj.GetAnnotations()[AnnClonePhase] == clone.SucceededPhaseName +} + +func getSavedCloneStrategy(obj client.Object) *cdiv1.CDICloneStrategy { + if val, ok := obj.GetAnnotations()[cc.AnnCloneType]; ok { + cs := cdiv1.CDICloneStrategy(val) + return &cs + } + return nil +} + +func setSavedCloneStrategy(obj client.Object, strategy cdiv1.CDICloneStrategy) { + cc.AddAnnotation(obj, cc.AnnCloneType, string(strategy)) +} diff --git a/pkg/controller/populators/clone-populator_test.go b/pkg/controller/populators/clone-populator_test.go index 5973e693f2..eb80a36b21 100644 --- a/pkg/controller/populators/clone-populator_test.go +++ b/pkg/controller/populators/clone-populator_test.go @@ -22,6 +22,7 @@ import ( "github.com/go-logr/logr" . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" @@ -45,6 +46,7 @@ import ( "kubevirt.io/containerized-data-importer/pkg/controller/clone" cc "kubevirt.io/containerized-data-importer/pkg/controller/common" featuregates "kubevirt.io/containerized-data-importer/pkg/feature-gates" + "kubevirt.io/containerized-data-importer/pkg/token" ) const ( @@ -116,7 +118,7 @@ var _ = Describe("Clone populator tests", func() { initinializedTargetAndDataSource := func() (*corev1.PersistentVolumeClaim, *cdiv1.VolumeCloneSource) { target, source := targetAndDataSource() target.Annotations = map[string]string{ - AnnClonePhase: pendingPhase, + AnnClonePhase: clone.PendingPhaseName, cc.AnnCloneType: "snapshot", } clone.AddCommonClaimLabels(target) @@ -126,7 +128,7 @@ var _ = Describe("Clone populator tests", func() { succeededTarget := func() *corev1.PersistentVolumeClaim { target, _ := initinializedTargetAndDataSource() - target.Annotations[AnnClonePhase] = string(succeededPhase) + target.Annotations[AnnClonePhase] = string(clone.SucceededPhaseName) target.Spec.VolumeName = "volume" return target } @@ -149,7 +151,7 @@ var _ = Describe("Clone populator tests", func() { verifyPending := func(c client.Client) { target := getTarget(c) - Expect(target.Annotations[AnnClonePhase]).To(Equal(string(pendingPhase))) + Expect(target.Annotations[AnnClonePhase]).To(Equal(string(clone.PendingPhaseName))) } It("should do nothing if PVC is not found", func() { @@ -158,13 +160,14 @@ var _ = Describe("Clone populator tests", func() { isDefaultResult(result, err) }) - It("should error if cross namespace datasource is used", func() { + It("should do nothing if unexpected PVC", func() { target, _ := targetAndDataSource() - target.Spec.DataSourceRef.Namespace = pointer.String("other") + target.Spec.DataSourceRef.Kind = "Unexpected" reconciler := createClonePopulatorReconciler(target) - _, err := reconciler.Reconcile(context.Background(), nn) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(Equal("cross namespace datasource not supported yet")) + result, err := reconciler.Reconcile(context.Background(), nn) + isDefaultResult(result, err) + target = getTarget(reconciler.client) + Expect(target.Annotations).ToNot(HaveKey(AnnClonePhase)) }) It("should be pending storageclass is nil", func() { @@ -212,6 +215,33 @@ var _ = Describe("Clone populator tests", func() { verifyPending(reconciler.client) }) + It("should be pending if choosestrategy returns nil (cross namespace validation)", func() { + target, source := targetAndDataSource() + source.Namespace = "other" + cc.AddAnnotation(target, AnnDataSourceNamespace, source.Namespace) + cc.AddAnnotation(target, cc.AnnCloneToken, "foo") + reconciler := createClonePopulatorReconciler(target, storageClass(), source) + reconciler.planner = &fakePlanner{} + reconciler.multiTokenValidator = &cc.MultiTokenValidator{ + ShortTokenValidator: &cc.FakeValidator{ + Match: "foo", + Operation: token.OperationClone, + Name: source.Spec.Source.Name, + Namespace: "other", + Resource: metav1.GroupVersionResource{ + Resource: "persistentvolumeclaims", + }, + Params: map[string]string{ + "targetName": target.Name, + "targetNamespace": target.Namespace, + }, + }, + } + result, err := reconciler.Reconcile(context.Background(), nn) + isRequeueResult(result, err) + verifyPending(reconciler.client) + }) + It("should be pending and initialize target if choosestrategy returns something", func() { target, source := targetAndDataSource() reconciler := createClonePopulatorReconciler(target, storageClass(), source) @@ -222,7 +252,7 @@ var _ = Describe("Clone populator tests", func() { result, err := reconciler.Reconcile(context.Background(), nn) isDefaultResult(result, err) pvc := getTarget(reconciler.client) - Expect(pvc.Annotations[AnnClonePhase]).To(Equal(string(pendingPhase))) + Expect(pvc.Annotations[AnnClonePhase]).To(Equal(string(clone.PendingPhaseName))) Expect(pvc.Annotations[cc.AnnCloneType]).To(Equal(string(csr))) Expect(pvc.Finalizers).To(ContainElement(cloneFinalizer)) }) @@ -258,20 +288,36 @@ var _ = Describe("Clone populator tests", func() { Expect(pvc.Annotations[AnnCloneError]).To(Equal("phase error")) }) - It("should report phase name and progress", func() { + DescribeTable("should report phase name and progress", func(ownedByDataVolume bool) { target, source := initinializedTargetAndDataSource() + if ownedByDataVolume { + target.OwnerReferences = []metav1.OwnerReference{ + { + Kind: "DataVolume", + Controller: pointer.Bool(true), + }, + } + } reconciler := createClonePopulatorReconciler(target, storageClass(), source) reconciler.planner = &fakePlanner{ planResult: []clone.Phase{ &fakePhase{ name: "phase1", }, - &fakePhaseWithProgress{ + &fakePhaseWithStatus{ fakePhase: fakePhase{ name: "phase2", result: &reconcile.Result{}, }, - progress: "50.0%", + status: &clone.PhaseStatus{ + Progress: "50.0%", + Annotations: map[string]string{ + "foo": "bar", + cc.AnnRunningCondition: "true", + cc.AnnRunningConditionMessage: "message", + cc.AnnRunningConditionReason: "reason", + }, + }, }, }, } @@ -280,19 +326,32 @@ var _ = Describe("Clone populator tests", func() { pvc := getTarget(reconciler.client) Expect(pvc.Annotations[AnnClonePhase]).To(Equal("phase2")) Expect(pvc.Annotations[cc.AnnPopulatorProgress]).To(Equal("50.0%")) - }) + Expect(pvc.Annotations).ToNot(HaveKey("foo")) + if ownedByDataVolume { + Expect(pvc.Annotations).To(HaveKey(cc.AnnRunningCondition)) + Expect(pvc.Annotations).To(HaveKey(cc.AnnRunningConditionMessage)) + Expect(pvc.Annotations).To(HaveKey(cc.AnnRunningConditionReason)) + } else { + Expect(pvc.Annotations).ToNot(HaveKey(cc.AnnRunningCondition)) + Expect(pvc.Annotations).ToNot(HaveKey(cc.AnnRunningConditionMessage)) + Expect(pvc.Annotations).ToNot(HaveKey(cc.AnnRunningConditionReason)) + } + }, + Entry("NOT owned by data volume", false), + Entry("owned by data volume", true), + ) It("should be in error phase if progress returns an error", func() { target, source := initinializedTargetAndDataSource() reconciler := createClonePopulatorReconciler(target, storageClass(), source) reconciler.planner = &fakePlanner{ planResult: []clone.Phase{ - &fakePhaseWithProgress{ + &fakePhaseWithStatus{ fakePhase: fakePhase{ name: "phase1", result: &reconcile.Result{}, }, - proogressErr: fmt.Errorf("progress error"), + statusErr: fmt.Errorf("progress error"), }, }, } @@ -371,14 +430,14 @@ func (p *fakePhase) Reconcile(ctx context.Context) (*reconcile.Result, error) { return p.result, p.err } -type fakePhaseWithProgress struct { +type fakePhaseWithStatus struct { fakePhase - progress string - proogressErr error + status *clone.PhaseStatus + statusErr error } -func (p *fakePhaseWithProgress) Progress(ctx context.Context) (string, error) { - return p.progress, p.proogressErr +func (p *fakePhaseWithStatus) Status(ctx context.Context) (*clone.PhaseStatus, error) { + return p.status, p.statusErr } func createClonePopulatorReconciler(objects ...runtime.Object) *ClonePopulatorReconciler { diff --git a/pkg/controller/upload-controller.go b/pkg/controller/upload-controller.go index 19f1ac4940..b71f796a63 100644 --- a/pkg/controller/upload-controller.go +++ b/pkg/controller/upload-controller.go @@ -203,10 +203,15 @@ func (r *UploadReconciler) reconcilePVC(log logr.Logger, pvc *corev1.PersistentV } if len(podsUsingPVC) > 0 { + es, err := cc.GetAnnotatedEventSource(context.TODO(), r.client, pvc) + if err != nil { + return reconcile.Result{}, err + } + for _, pod := range podsUsingPVC { r.log.V(1).Info("can't create upload pod, pvc in use by other pod", "namespace", pvc.Namespace, "name", pvc.Name, "pod", pod.Name) - r.recorder.Eventf(pvc, corev1.EventTypeWarning, UploadTargetInUse, + r.recorder.Eventf(es, corev1.EventTypeWarning, UploadTargetInUse, "pod %s/%s using PersistentVolumeClaim %s", pod.Namespace, pod.Name, pvc.Name) } diff --git a/pkg/operator/resources/cluster/controller.go b/pkg/operator/resources/cluster/controller.go index cf3d7fc37f..cc3aa03618 100644 --- a/pkg/operator/resources/cluster/controller.go +++ b/pkg/operator/resources/cluster/controller.go @@ -69,6 +69,7 @@ func getControllerClusterPolicyRules() []rbacv1.PolicyRule { "update", "delete", "deletecollection", + "patch", }, }, { diff --git a/staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/types.go b/staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/types.go index 1ceb1b5f10..c033649761 100644 --- a/staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/types.go +++ b/staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/types.go @@ -379,6 +379,11 @@ const ( // Paused represents a DataVolumePhase of Paused Paused DataVolumePhase = "Paused" + // PrepClaimInProgress represents a data volume with a current phase of PrepClaimInProgress + PrepClaimInProgress DataVolumePhase = "PrepClaimInProgress" + // RebindInProgress represents a data volume with a current phase of RebindInProgress + RebindInProgress DataVolumePhase = "RebindInProgress" + // DataVolumeReady is the condition that indicates if the data volume is ready to be consumed. DataVolumeReady DataVolumeConditionType = "Ready" // DataVolumeBound is the condition that indicates if the underlying PVC is bound or not. diff --git a/tests/basic_sanity_test.go b/tests/basic_sanity_test.go index ab5f009781..0557cc2996 100644 --- a/tests/basic_sanity_test.go +++ b/tests/basic_sanity_test.go @@ -80,7 +80,7 @@ var _ = Describe("[rfe_id:1347][crit:high][vendor:cnv-qe@redhat.com][level:compo pvcExpectedResult["delete"] = "yes" pvcExpectedResult["create"] = "yes" pvcExpectedResult["update"] = "yes" - pvcExpectedResult["patch"] = "no" + pvcExpectedResult["patch"] = "yes" pvcExpectedResult["deletecollection"] = "yes" ValidateRBACForResource(f, pvcExpectedResult, "persistentvolumeclaims", sa) ValidateRBACForResource(f, pvcExpectedResult, "persistentvolumeclaims/finalizers", sa) diff --git a/tests/clone-populator_test.go b/tests/clone-populator_test.go index 822df06ce0..f88cfe08c0 100644 --- a/tests/clone-populator_test.go +++ b/tests/clone-populator_test.go @@ -240,12 +240,10 @@ var _ = Describe("Clone Populator tests", func() { } source := createSource(defaultSize, corev1.PersistentVolumeBlock) createDataSource() - target := createTarget(defaultSize, corev1.PersistentVolumeFilesystem) + target := createTarget(biggerSize, corev1.PersistentVolumeFilesystem) target = waitSucceeded(target) - targetSize := target.Status.Capacity[corev1.ResourceStorage] - Expect(targetSize.Cmp(defaultSize)).To(BeNumerically(">", 0)) - sourceHash := getHash(source, 0) - targetHash := getHash(target, 0) + sourceHash := getHash(source, 100000) + targetHash := getHash(target, 100000) Expect(targetHash).To(Equal(sourceHash)) }) diff --git a/tests/cloner_test.go b/tests/cloner_test.go index 96c011e7a5..81a0f68019 100644 --- a/tests/cloner_test.go +++ b/tests/cloner_test.go @@ -311,56 +311,6 @@ var _ = Describe("all clone tests", func() { doInUseCloneTest(f, pvcDef, targetNs, "target-dv") }) - It("[posneg:negative][test_id:6612]Clone with CSI as PVC source with target name that already exists", func() { - if utils.DefaultStorageClassCsiDriver == nil { - Skip("No CSI driver found") - } - if cloneType == "copy" { - Skip("Cannot simulate target pvc name conflict for host-assisted clone ") - } - pvcDef := utils.NewPVCDefinition(sourcePVCName, "1Gi", nil, nil) - sourcePvc = f.CreateAndPopulateSourcePVC(pvcDef, sourcePodFillerName, fillCommand+testFile+"; chmod 660 "+testBaseDir+testFile) - targetNamespaceName := f.Namespace.Name - - // 1. use the srcPvc so the clone cannot be started - pod, err := f.CreateExecutorPodWithPVC("temp-pod", f.Namespace.Name, sourcePvc, false) - Expect(err).ToNot(HaveOccurred()) - - Eventually(func() bool { - pod, err = f.K8sClient.CoreV1().Pods(f.Namespace.Name).Get(context.TODO(), pod.Name, metav1.GetOptions{}) - Expect(err).ToNot(HaveOccurred()) - return pod.Status.Phase == v1.PodRunning - }, 90*time.Second, 2*time.Second).Should(BeTrue()) - - // 2. Create a clone DataVolume - targetDV := utils.NewCloningDataVolume("target-pvc", "1Gi", sourcePvc) - dataVolume, err := utils.CreateDataVolumeFromDefinition(f.CdiClient, targetNamespaceName, targetDV) - Expect(err).ToNot(HaveOccurred()) - - actualCloneType := utils.GetCloneType(f.CdiClient, dataVolume) - if actualCloneType == "snapshot" { - f.ExpectEvent(targetNamespaceName).Should(ContainSubstring(dvc.SmartCloneSourceInUse)) - } else if actualCloneType == "csi-clone" { - f.ExpectEvent(targetNamespaceName).Should(ContainSubstring(dvc.CSICloneSourceInUse)) - } else { - Fail(fmt.Sprintf("Unknown clonetype %s", actualCloneType)) - } - - // 3. Knowing that clone cannot yet advance, Create targetPvc with a "conflicting name" - By(fmt.Sprintf("Creating target pvc: %s/target-pvc", targetNamespaceName)) - annotations := map[string]string{"cdi.kubevirt.io/conflicting-pvc": dataVolumeName} - - targetPvc, err := utils.CreatePVCFromDefinition(f.K8sClient, targetNamespaceName, - utils.NewPVCDefinition("target-pvc", "1Gi", annotations, nil)) - Expect(err).ToNot(HaveOccurred()) - f.ForceBindIfWaitForFirstConsumer(targetPvc) - - err = f.K8sClient.CoreV1().Pods(f.Namespace.Name).Delete(context.TODO(), pod.Name, metav1.DeleteOptions{}) - Expect(err).ToNot(HaveOccurred()) - //verify event - f.ExpectEvent(targetNamespaceName).Should(ContainSubstring(dvc.ErrResourceExists)) - }) - It("[test_id:1356]Should not clone anything when CloneOf annotation exists", func() { pvcDef := utils.NewPVCDefinition(sourcePVCName, "1Gi", nil, nil) sourcePvc = f.CreateAndPopulateSourcePVC(pvcDef, sourcePodFillerName, fillCommand+testFile+"; chmod 660 "+testBaseDir+testFile) @@ -582,6 +532,7 @@ var _ = Describe("all clone tests", func() { Skip("csi-clone only works for the same volumeMode") } dataVolume := utils.NewDataVolumeWithHTTPImportAndStorageSpec(dataVolumeName, "2Gi", fmt.Sprintf(utils.LargeVirtualDiskQcow, f.CdiInstallNs)) + controller.AddAnnotation(dataVolume, controller.AnnDeleteAfterCompletion, "false") filesystem := v1.PersistentVolumeFilesystem dataVolume.Spec.Storage.VolumeMode = &filesystem @@ -772,11 +723,6 @@ var _ = Describe("all clone tests", func() { targetDiskImagePath = testBaseDir } - if cloneType == "snapshot" && sourceRef { - // TODO: remove this when we no longer have smart clone controller - Skip("Smart clone controller doesn't play nice with sourceRef and is being removed soon") - } - // Create the source DV dataVolume := utils.NewDataVolumeWithHTTPImportAndStorageSpec(dataVolumeName, "1Gi", fmt.Sprintf(utils.TinyCoreIsoURL, f.CdiInstallNs)) dataVolume.Spec.Storage.VolumeMode = &sourceVolumeMode @@ -903,7 +849,7 @@ var _ = Describe("all clone tests", func() { targetPvc, err := utils.WaitForPVC(f.K8sClient, targetDataVolume.Namespace, targetDataVolume.Name) Expect(err).ToNot(HaveOccurred()) By("Ensure WFFC is reported to reflect the situation correctly") - err = utils.WaitForDataVolumePhase(f, targetDataVolume.Namespace, cdiv1.WaitForFirstConsumer, targetDataVolume.Name) + err = utils.WaitForDataVolumePhase(f, targetDataVolume.Namespace, cdiv1.PendingPopulation, targetDataVolume.Name) Expect(err).ToNot(HaveOccurred()) // Force bind to ensure integrity after first consumer @@ -2111,18 +2057,10 @@ var _ = Describe("all clone tests", func() { Expect(err).ToNot(HaveOccurred()) f.ForceBindPvcIfDvIsWaitForFirstConsumer(dataVolume) - By("Verify Quota was exceeded in logs") - matchString := "\\\"cdi-upload-target-dv\\\" is forbidden: exceeded quota: test-quota, requested" - Eventually(func() string { - log, err := f.RunKubectlCommand("logs", f.ControllerPod.Name, "-n", f.CdiInstallNs) - Expect(err).NotTo(HaveOccurred()) - return log - }, controllerSkipPVCCompleteTimeout, assertionPollInterval).Should(ContainSubstring(matchString)) - expectedCondition := &cdiv1.DataVolumeCondition{ Type: cdiv1.DataVolumeRunning, Status: v1.ConditionFalse, - Message: fmt.Sprintf(controller.MessageErrStartingPod, "cdi-upload-target-dv"), + Message: "Error starting pod", Reason: controller.ErrExceededQuota, } @@ -2130,8 +2068,7 @@ var _ = Describe("all clone tests", func() { utils.WaitForConditions(f, targetDV.Name, f.Namespace.Name, timeout, pollingInterval, expectedCondition) By("Check the expected event") - msg := fmt.Sprintf(controller.MessageErrStartingPod, "cdi-upload-target-dv") - f.ExpectEvent(f.Namespace.Name).Should(ContainSubstring(msg)) + f.ExpectEvent(f.Namespace.Name).Should(ContainSubstring("Error starting pod")) f.ExpectEvent(f.Namespace.Name).Should(ContainSubstring(controller.ErrExceededQuota)) }) @@ -2160,18 +2097,10 @@ var _ = Describe("all clone tests", func() { Expect(err).ToNot(HaveOccurred()) f.ForceBindPvcIfDvIsWaitForFirstConsumer(dataVolume) - By("Verify Quota was exceeded in logs") - matchString := "\\\"cdi-upload-target-dv\\\" is forbidden: exceeded quota: test-quota, requested" - Eventually(func() string { - log, err := f.RunKubectlCommand("logs", f.ControllerPod.Name, "-n", f.CdiInstallNs) - Expect(err).NotTo(HaveOccurred()) - return log - }, controllerSkipPVCCompleteTimeout, assertionPollInterval).Should(ContainSubstring(matchString)) - expectedCondition := &cdiv1.DataVolumeCondition{ Type: cdiv1.DataVolumeRunning, Status: v1.ConditionFalse, - Message: fmt.Sprintf(controller.MessageErrStartingPod, "cdi-upload-target-dv"), + Message: "Error starting pod", Reason: controller.ErrExceededQuota, } @@ -2179,8 +2108,7 @@ var _ = Describe("all clone tests", func() { utils.WaitForConditions(f, targetDV.Name, f.Namespace.Name, timeout, pollingInterval, expectedCondition) By("Check the expected event") - msg := fmt.Sprintf(controller.MessageErrStartingPod, "cdi-upload-target-dv") - f.ExpectEvent(f.Namespace.Name).Should(ContainSubstring(msg)) + f.ExpectEvent(f.Namespace.Name).Should(ContainSubstring("Error starting pod")) f.ExpectEvent(f.Namespace.Name).Should(ContainSubstring(controller.ErrExceededQuota)) Expect(f.UpdateQuotaInNs(int64(1), int64(512*1024*1024), int64(4), int64(512*1024*1024))).To(Succeed()) @@ -2230,28 +2158,17 @@ var _ = Describe("all clone tests", func() { dataVolume, err := utils.CreateDataVolumeFromDefinition(f.CdiClient, targetNs.Name, targetDV) Expect(err).ToNot(HaveOccurred()) + f.ForceBindPvcIfDvIsWaitForFirstConsumer(dataVolume) + cloneType := utils.GetCloneType(f.CdiClient, dataVolume) if cloneType != "copy" { Skip("only valid for copy clone") } - f.ForceBindPvcIfDvIsWaitForFirstConsumer(dataVolume) - - By("Verify Quota was exceeded in logs") - targetPvc, err := utils.WaitForPVC(f.K8sClient, dataVolume.Namespace, dataVolume.Name) - Expect(err).ToNot(HaveOccurred()) - matchString := fmt.Sprintf("\\\"%s-source-pod\\\" is forbidden: exceeded quota: test-quota, requested", targetPvc.GetUID()) - Eventually(func() string { - log, err := f.RunKubectlCommand("logs", f.ControllerPod.Name, "-n", f.CdiInstallNs) - Expect(err).NotTo(HaveOccurred()) - return log - }, controllerSkipPVCCompleteTimeout, assertionPollInterval).Should(ContainSubstring(matchString)) - - podName := fmt.Sprintf("%s-source-pod", targetPvc.GetUID()) expectedCondition := &cdiv1.DataVolumeCondition{ Type: cdiv1.DataVolumeRunning, Status: v1.ConditionFalse, - Message: fmt.Sprintf(controller.MessageErrStartingPod, podName), + Message: "Error starting pod", Reason: controller.ErrExceededQuota, } @@ -2259,8 +2176,7 @@ var _ = Describe("all clone tests", func() { utils.WaitForConditions(f, targetDV.Name, targetNs.Name, timeout, pollingInterval, expectedCondition) By("Check the expected event") - msg := fmt.Sprintf(controller.MessageErrStartingPod, podName) - f.ExpectEvent(targetNs.Name).Should(ContainSubstring(msg)) + f.ExpectEvent(targetNs.Name).Should(ContainSubstring("Error starting pod")) f.ExpectEvent(targetNs.Name).Should(ContainSubstring(controller.ErrExceededQuota)) }) @@ -2282,17 +2198,24 @@ var _ = Describe("all clone tests", func() { dataVolume, err := utils.CreateDataVolumeFromDefinition(f.CdiClient, targetNs.Name, targetDV) Expect(err).ToNot(HaveOccurred()) + targetPvc, err := utils.WaitForPVC(f.K8sClient, dataVolume.Namespace, dataVolume.Name) + Expect(err).ToNot(HaveOccurred()) + + if targetPvc.Spec.DataSourceRef != nil { + Skip("only valid for non csi clone") + } + + f.ForceBindPvcIfDvIsWaitForFirstConsumer(dataVolume) + cloneType := utils.GetCloneType(f.CdiClient, dataVolume) if cloneType != "copy" { Skip("only valid for copy clone") } - f.ForceBindPvcIfDvIsWaitForFirstConsumer(dataVolume) - expectedCondition := &cdiv1.DataVolumeCondition{ Type: cdiv1.DataVolumeRunning, Status: v1.ConditionFalse, - Message: fmt.Sprintf(controller.MessageErrStartingPod, "cdi-upload-target-dv"), + Message: "Error starting pod", Reason: controller.ErrExceededQuota, } @@ -2300,8 +2223,7 @@ var _ = Describe("all clone tests", func() { utils.WaitForConditions(f, targetDV.Name, targetNs.Name, timeout, pollingInterval, expectedCondition) By("Check the expected event") - msg := fmt.Sprintf(controller.MessageErrStartingPod, "cdi-upload-target-dv") - f.ExpectEvent(targetNs.Name).Should(ContainSubstring(msg)) + f.ExpectEvent(targetNs.Name).Should(ContainSubstring("Error starting pod")) f.ExpectEvent(targetNs.Name).Should(ContainSubstring(controller.ErrExceededQuota)) }) }) @@ -2378,15 +2300,22 @@ var _ = Describe("all clone tests", func() { dataVolume, err := utils.CreateDataVolumeFromDefinition(f.CdiClient, targetNs.Name, targetDV) Expect(err).ToNot(HaveOccurred()) + targetPvc, err := utils.WaitForPVC(f.K8sClient, dataVolume.Namespace, dataVolume.Name) + Expect(err).ToNot(HaveOccurred()) + + if targetPvc.Spec.DataSourceRef != nil { + // Skipping with csi because force bind early causes to succeed very quickly + // cannot catch pod + Skip("only for non csi-clone") + } + + f.ForceBindIfWaitForFirstConsumer(targetPvc) + cloneType := utils.GetCloneType(f.CdiClient, dataVolume) if cloneType != "copy" { Skip("only valid for copy clone") } - targetPvc, err := utils.WaitForPVC(f.K8sClient, dataVolume.Namespace, dataVolume.Name) - Expect(err).ToNot(HaveOccurred()) - f.ForceBindIfWaitForFirstConsumer(targetPvc) - fmt.Fprintf(GinkgoWriter, "INFO: wait for PVC claim phase: %s\n", targetPvc.Name) Expect(utils.WaitForPersistentVolumeClaimPhase(f.K8sClient, targetNs.Name, v1.ClaimBound, targetPvc.Name)).To(Succeed()) @@ -2716,8 +2645,8 @@ var _ = Describe("all clone tests", func() { Expect(err).ToNot(HaveOccurred()) _, ok := pvc.Annotations[controller.AnnCloneRequest] Expect(ok).To(BeFalse()) - Expect(pvc.Spec.DataSource.Kind).To(Equal("VolumeSnapshot")) - Expect(pvc.Spec.DataSourceRef.Kind).To(Equal("VolumeSnapshot")) + Expect(pvc.Spec.DataSource.Kind).To(Equal("VolumeCloneSource")) + Expect(pvc.Spec.DataSourceRef.Kind).To(Equal("VolumeCloneSource")) // All labels and annotations passed Expect(pvc.Labels["test-label-1"]).To(Equal("test-label-value-1")) Expect(pvc.Annotations["test-annotation-1"]).To(Equal("test-annotation-value-1")) @@ -2786,10 +2715,17 @@ var _ = Describe("all clone tests", func() { By("Check host assisted clone is taking place") pvc, err := f.K8sClient.CoreV1().PersistentVolumeClaims(targetNs.Name).Get(context.TODO(), dvName, metav1.GetOptions{}) Expect(err).ToNot(HaveOccurred()) - suffix := "-host-assisted-source-pvc" - Expect(pvc.Annotations[controller.AnnCloneRequest]).To(HaveSuffix(suffix)) - Expect(pvc.Spec.DataSource).To(BeNil()) - Expect(pvc.Spec.DataSourceRef).To(BeNil()) + // non csi + if pvc.Spec.DataSourceRef == nil { + suffix := "-host-assisted-source-pvc" + Expect(pvc.Annotations[controller.AnnCloneRequest]).To(HaveSuffix(suffix)) + Expect(pvc.Spec.DataSource).To(BeNil()) + } else { + dv, err := f.CdiClient.CdiV1beta1().DataVolumes(targetNs.Name).Get(context.TODO(), dvName, metav1.GetOptions{}) + Expect(err).ToNot(HaveOccurred()) + cloneType := utils.GetCloneType(f.CdiClient, dv) + Expect(cloneType).To(Equal(string(cdiv1.CloneStrategyHostAssisted))) + } } By("Verify MD5 on one of the DVs") @@ -2998,6 +2934,12 @@ func doFileBasedCloneTest(f *framework.Framework, srcPVCDef *v1.PersistentVolume // All labels and annotations passed Expect(targetPvc.Labels["test-label-1"]).To(Equal("test-label-key-1")) Expect(targetPvc.Annotations["test-annotation-1"]).To(Equal("test-annotation-key-1")) + + if targetNs.Name != f.Namespace.Name { + dataVolume, err = f.CdiClient.CdiV1beta1().DataVolumes(dataVolume.Namespace).Get(context.TODO(), dataVolume.Name, metav1.GetOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(dataVolume.Annotations).To(HaveKey(controller.AnnExtendedCloneToken)) + } } func doInUseCloneTest(f *framework.Framework, srcPVCDef *v1.PersistentVolumeClaim, targetNs *v1.Namespace, targetDv string) { @@ -3016,33 +2958,13 @@ func doInUseCloneTest(f *framework.Framework, srcPVCDef *v1.PersistentVolumeClai Expect(err).ToNot(HaveOccurred()) var targetPvc *v1.PersistentVolumeClaim - cloneType := utils.GetCloneType(f.CdiClient, dataVolume) - - if cloneType == "copy" { - targetPvc, err = utils.WaitForPVC(f.K8sClient, dataVolume.Namespace, dataVolume.Name) - Expect(err).ToNot(HaveOccurred()) - f.ForceBindPvcIfDvIsWaitForFirstConsumer(dataVolume) - - f.ExpectEvent(targetNs.Name).Should(ContainSubstring(controller.CloneSourceInUse)) - err = f.K8sClient.CoreV1().Pods(f.Namespace.Name).Delete(context.TODO(), pod.Name, metav1.DeleteOptions{}) - Expect(err).ToNot(HaveOccurred()) - } else if cloneType == "snapshot" { - f.ExpectEvent(targetNs.Name).Should(ContainSubstring(dvc.SmartCloneSourceInUse)) - err = f.K8sClient.CoreV1().Pods(f.Namespace.Name).Delete(context.TODO(), pod.Name, metav1.DeleteOptions{}) - Expect(err).ToNot(HaveOccurred()) - - targetPvc, err = utils.WaitForPVC(f.K8sClient, dataVolume.Namespace, dataVolume.Name) - Expect(err).ToNot(HaveOccurred()) - f.ForceBindPvcIfDvIsWaitForFirstConsumer(dataVolume) - } else { - f.ExpectEvent(targetNs.Name).Should(ContainSubstring(dvc.CSICloneSourceInUse)) - err = f.K8sClient.CoreV1().Pods(f.Namespace.Name).Delete(context.TODO(), pod.Name, metav1.DeleteOptions{}) - Expect(err).ToNot(HaveOccurred()) + targetPvc, err = utils.WaitForPVC(f.K8sClient, dataVolume.Namespace, dataVolume.Name) + Expect(err).ToNot(HaveOccurred()) + f.ForceBindPvcIfDvIsWaitForFirstConsumer(dataVolume) - targetPvc, err = utils.WaitForPVC(f.K8sClient, dataVolume.Namespace, dataVolume.Name) - Expect(err).ToNot(HaveOccurred()) - f.ForceBindPvcIfDvIsWaitForFirstConsumer(dataVolume) - } + f.ExpectEvent(targetNs.Name).Should(ContainSubstring(controller.CloneSourceInUse)) + err = f.K8sClient.CoreV1().Pods(f.Namespace.Name).Delete(context.TODO(), pod.Name, metav1.DeleteOptions{}) + Expect(err).ToNot(HaveOccurred()) _, _ = fmt.Fprintf(GinkgoWriter, "INFO: wait for PVC claim phase: %s\n", targetPvc.Name) Expect(utils.WaitForPersistentVolumeClaimPhase(f.K8sClient, targetNs.Name, v1.ClaimBound, targetPvc.Name)).Should(Succeed()) @@ -3054,16 +2976,18 @@ func doInUseCloneTest(f *framework.Framework, srcPVCDef *v1.PersistentVolumeClai } func completeClone(f *framework.Framework, targetNs *v1.Namespace, targetPvc *v1.PersistentVolumeClaim, filePath, expectedMD5, sourcePvcDiskGroup string) { - By("Verify the clone annotation is on the target PVC") - _, cloneAnnotationFound, err := utils.WaitForPVCAnnotation(f.K8sClient, targetNs.Name, targetPvc, controller.AnnCloneOf) - if err != nil { - f.PrintControllerLog() + if _, ok := targetPvc.Annotations[controller.AnnCloneRequest]; ok { + By("Verify the clone annotation is on the target PVC") + _, cloneAnnotationFound, err := utils.WaitForPVCAnnotation(f.K8sClient, targetNs.Name, targetPvc, controller.AnnCloneOf) + if err != nil { + f.PrintControllerLog() + } + Expect(err).ToNot(HaveOccurred()) + Expect(cloneAnnotationFound).To(BeTrue()) } - Expect(err).ToNot(HaveOccurred()) - Expect(cloneAnnotationFound).To(BeTrue()) By("Verify the clone status is success on the target datavolume") - err = utils.WaitForDataVolumePhase(f, targetNs.Name, cdiv1.Succeeded, targetPvc.Name) + err := utils.WaitForDataVolumePhase(f, targetNs.Name, cdiv1.Succeeded, targetPvc.Name) Expect(err).ToNot(HaveOccurred()) By("Verify the content") @@ -3127,27 +3051,29 @@ func completeClone(f *framework.Framework, targetNs *v1.Namespace, targetPvc *v1 }, 90*time.Second, 2*time.Second).Should(BeTrue()) } case "copy": - s, err := f.K8sClient.CoreV1().Secrets(f.CdiInstallNs).Get(context.TODO(), "cdi-api-signing-key", metav1.GetOptions{}) - Expect(err).ToNot(HaveOccurred()) - bytes, ok := s.Data["id_rsa.pub"] - Expect(ok).To(BeTrue()) - objs, err := cert.ParsePublicKeysPEM(bytes) - Expect(err).ToNot(HaveOccurred()) - Expect(objs).To(HaveLen(1)) - v := token.NewValidator("cdi-deployment", objs[0].(*rsa.PublicKey), time.Minute) - - By("checking long token added") - Eventually(func(g Gomega) bool { - pvc, err := f.K8sClient.CoreV1().PersistentVolumeClaims(targetNs.Name).Get(context.TODO(), targetPvc.Name, metav1.GetOptions{}) - g.Expect(err).ToNot(HaveOccurred()) - t, ok := pvc.Annotations[controller.AnnExtendedCloneToken] - if !ok { - return false - } - _, err = v.Validate(t) - g.Expect(err).ToNot(HaveOccurred()) - return true - }, 10*time.Second, assertionPollInterval).Should(BeTrue()) + if sns != dv.Namespace { + s, err := f.K8sClient.CoreV1().Secrets(f.CdiInstallNs).Get(context.TODO(), "cdi-api-signing-key", metav1.GetOptions{}) + Expect(err).ToNot(HaveOccurred()) + bytes, ok := s.Data["id_rsa.pub"] + Expect(ok).To(BeTrue()) + objs, err := cert.ParsePublicKeysPEM(bytes) + Expect(err).ToNot(HaveOccurred()) + Expect(objs).To(HaveLen(1)) + v := token.NewValidator("cdi-deployment", objs[0].(*rsa.PublicKey), time.Minute) + + By("checking long token added") + Eventually(func(g Gomega) bool { + pvc, err := f.K8sClient.CoreV1().PersistentVolumeClaims(targetNs.Name).Get(context.TODO(), targetPvc.Name, metav1.GetOptions{}) + g.Expect(err).ToNot(HaveOccurred()) + t, ok := pvc.Annotations[controller.AnnExtendedCloneToken] + if !ok { + return false + } + _, err = v.Validate(t) + g.Expect(err).ToNot(HaveOccurred()) + return true + }, 10*time.Second, assertionPollInterval).Should(BeTrue()) + } } } diff --git a/tests/csiclone_test.go b/tests/csiclone_test.go index 27963a080b..c9a7e6c305 100644 --- a/tests/csiclone_test.go +++ b/tests/csiclone_test.go @@ -83,7 +83,7 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component][crit:high][rfe_id: verifyCSIClone(dataVolume, f) }) - It("[posneg:negative][test_id:6655] Support for CSI Clone strategy in storage profile with SC HPP - negative", func() { + It("StorageProfile setting ignored with non-csi clone", func() { if f.IsCSIVolumeCloneStorageClassAvailable() { Skip("Test should only run on non-csi storage") } @@ -97,8 +97,8 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component][crit:high][rfe_id: ).To(Succeed()) dataVolume, _ := createDataVolumeDontWait("dv-csi-clone-test-1", utils.DefaultImagePath, v1.PersistentVolumeFilesystem, cloneStorageClassName, f) - waitForDvPhase(cdiv1.CloneScheduled, dataVolume, f) - f.ExpectEvent(dataVolume.Namespace).Should(ContainSubstring(controller.ErrUnableToClone)) + f.ForceBindPvcIfDvIsWaitForFirstConsumer(dataVolume) + waitForDvPhase(cdiv1.Succeeded, dataVolume, f) }) It("[test_id:7736] Should fail to create pvc in namespace with storage quota, then succeed once the quota is large enough", func() { @@ -134,7 +134,7 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component][crit:high][rfe_id: utils.WaitForConditions(f, dataVolume.Name, f.Namespace.Name, timeout, pollingInterval, boundCondition, readyCondition) By("Increase quota") - Expect(f.UpdateStorageQuota(int64(2), int64(2*1024*1024*1024))).To(Succeed()) + Expect(f.UpdateStorageQuota(int64(3), int64(4*1024*1024*1024))).To(Succeed()) By("Verify clone completed after quota increase") // Wait for operation Succeeded @@ -185,6 +185,6 @@ func createDataVolumeDontWait(dataVolumeName, testPath string, volumeMode v1.Per func verifyCSIClone(dataVolume *cdiv1.DataVolume, f *framework.Framework) { targetPvc, err := f.K8sClient.CoreV1().PersistentVolumeClaims(dataVolume.Namespace).Get(context.TODO(), dataVolume.Name, metav1.GetOptions{}) Expect(err).ToNot(HaveOccurred()) - Expect(targetPvc.Spec.DataSource.Kind).To(Equal("PersistentVolumeClaim")) - Expect(targetPvc.Spec.DataSourceRef.Kind).To(Equal("PersistentVolumeClaim")) + Expect(targetPvc.Spec.DataSource.Kind).To(Equal("VolumeCloneSource")) + Expect(targetPvc.Spec.DataSourceRef.Kind).To(Equal("VolumeCloneSource")) } diff --git a/tests/datavolume_test.go b/tests/datavolume_test.go index b794b4427e..812c2a42d7 100644 --- a/tests/datavolume_test.go +++ b/tests/datavolume_test.go @@ -399,14 +399,7 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests", Message: "exceeded quota", Reason: controller.ErrExceededQuota, } - // for smart clone dv the dv can't be updated, only events will show the quota reason - if args.name == "dv-clone-test" && f.IsSnapshotStorageClassAvailable() { - expectedPhase = cdiv1.SnapshotForSmartCloneInProgress - boundCondition.Message = "in progress" - boundCondition.Reason = dvc.SnapshotForSmartCloneInProgress - readyCondition.Message = "in progress" - readyCondition.Reason = dvc.SnapshotForSmartCloneInProgress - } + waitForDvPhase(expectedPhase, dataVolume, f) f.ExpectEvent(dataVolume.Namespace).Should(ContainSubstring(controller.ErrExceededQuota)) utils.WaitForConditions(f, dataVolume.Name, f.Namespace.Name, timeout, pollingInterval, boundCondition, readyCondition) diff --git a/tests/framework/pvc.go b/tests/framework/pvc.go index 4f477a9dbf..d8c68f4321 100644 --- a/tests/framework/pvc.go +++ b/tests/framework/pvc.go @@ -78,9 +78,7 @@ func (f *Framework) ForceBindPvcIfDvIsWaitForFirstConsumer(dv *cdiv1.DataVolume) gomega.Expect(err).ToNot(gomega.HaveOccurred(), "PVC should exist") if f.IsBindingModeWaitForFirstConsumer(pvc.Spec.StorageClassName) { // check if pvc is a population pvc but not from pvc or snapshot - if pvc.Spec.DataSourceRef != nil && - (dv.Spec.Source == nil || dv.Spec.Source.PVC == nil) && - (dv.Spec.Source == nil || dv.Spec.Source.Snapshot == nil) { + if pvc.Spec.DataSourceRef != nil { err = utils.WaitForDataVolumePhase(f, dv.Namespace, cdiv1.PendingPopulation, dv.Name) gomega.Expect(err).ToNot(gomega.HaveOccurred()) createConsumerPodForPopulationPVC(pvc, f) diff --git a/tests/smartclone_test.go b/tests/smartclone_test.go index 94fca1b894..c641c3e359 100644 --- a/tests/smartclone_test.go +++ b/tests/smartclone_test.go @@ -12,6 +12,7 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + cc "kubevirt.io/containerized-data-importer/pkg/controller/common" controller "kubevirt.io/containerized-data-importer/pkg/controller/datavolume" "kubevirt.io/containerized-data-importer/tests/framework" "kubevirt.io/containerized-data-importer/tests/utils" @@ -69,8 +70,8 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]SmartClone tests th if strings.Contains(events, controller.SnapshotForSmartCloneInProgress) { Fail(fmt.Sprintf("seen event SmartClonePVCInProgress. Events: %s", events)) } - if strings.Contains(events, controller.SmartClonePVCInProgress) { - Fail(fmt.Sprintf("seen event SmartClonePVCInProgress. Events: %s", events)) + if strings.Contains(events, controller.CloneFromSnapshotSourceInProgress) { + Fail(fmt.Sprintf("seen event CloneFromSnapshotSourceInProgress. Events: %s", events)) } }) }) @@ -84,7 +85,7 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]SmartClone tests", } dataVolume, expectedMd5 := createDataVolume("dv-smart-clone-test-1", utils.DefaultImagePath, v1.PersistentVolumeFilesystem, f.SnapshotSCName, f) f.ExpectEvent(dataVolume.Namespace).Should(ContainSubstring(controller.SnapshotForSmartCloneInProgress)) - f.ExpectEvent(dataVolume.Namespace).Should(ContainSubstring(controller.SmartClonePVCInProgress)) + f.ExpectEvent(dataVolume.Namespace).Should(ContainSubstring(controller.CloneFromSnapshotSourceInProgress)) // Wait for operation Succeeded waitForDvPhase(cdiv1.Succeeded, dataVolume, f) f.ExpectEvent(dataVolume.Namespace).Should(ContainSubstring(controller.CloneSucceeded)) @@ -98,7 +99,7 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]SmartClone tests", } dataVolume, expectedMd5 := createDataVolume("dv-smart-clone-test-1", utils.DefaultPvcMountPath, v1.PersistentVolumeBlock, f.SnapshotSCName, f) f.ExpectEvent(dataVolume.Namespace).Should(ContainSubstring(controller.SnapshotForSmartCloneInProgress)) - f.ExpectEvent(dataVolume.Namespace).Should(ContainSubstring(controller.SmartClonePVCInProgress)) + f.ExpectEvent(dataVolume.Namespace).Should(ContainSubstring(controller.CloneFromSnapshotSourceInProgress)) // Wait for operation Succeeded waitForDvPhase(cdiv1.Succeeded, dataVolume, f) f.ExpectEvent(dataVolume.Namespace).Should(ContainSubstring(controller.CloneSucceeded)) @@ -130,12 +131,12 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]SmartClone tests", dataVolume, err = utils.CreateDataVolumeFromDefinition(f.CdiClient, f.Namespace.Name, dataVolume) Expect(err).ToNot(HaveOccurred()) - f.ExpectEvent(dataVolume.Namespace).Should(ContainSubstring(controller.SmartCloneSourceInUse)) + f.ExpectEvent(dataVolume.Namespace).Should(ContainSubstring(cc.CloneSourceInUse)) err = f.K8sClient.CoreV1().Pods(f.Namespace.Name).Delete(context.TODO(), pod.Name, metav1.DeleteOptions{}) Expect(err).ToNot(HaveOccurred()) f.ExpectEvent(dataVolume.Namespace).Should(ContainSubstring(controller.SnapshotForSmartCloneInProgress)) - f.ExpectEvent(dataVolume.Namespace).Should(ContainSubstring(controller.SmartClonePVCInProgress)) + f.ExpectEvent(dataVolume.Namespace).Should(ContainSubstring(controller.CloneFromSnapshotSourceInProgress)) // Wait for operation Succeeded f.ForceBindPvcIfDvIsWaitForFirstConsumer(dataVolume) waitForDvPhase(cdiv1.Succeeded, dataVolume, f)