From a3d3dcdad5ad162285cfe298815c83292f0b35fe Mon Sep 17 00:00:00 2001 From: Shelly Kagan Date: Sun, 10 Nov 2024 17:14:32 +0200 Subject: [PATCH] Populators: Add to PVC' exclude velero backup label When backing up with Velero if a PVC is terminating then the backup is marked as PartiallyFailed, while the backup is still valid to use and its more of a warning, PVC' should be transparent to the user so have the backup partiallyFailed because of PVC' is a bad user experience. To fix that added to all our PVC' an label so it will be excluded from the Velero backup. This is a fix for previous PR https://github.com/kubevirt/containerized-data-importer/pull/3500 Which added an annotation instead of label by accident. Signed-off-by: Shelly Kagan --- pkg/controller/clone/csi-clone.go | 2 +- pkg/controller/clone/csi-clone_test.go | 2 +- pkg/controller/clone/host-clone.go | 2 +- pkg/controller/clone/host-clone_test.go | 2 +- pkg/controller/clone/snap-clone.go | 2 +- pkg/controller/clone/snap-clone_test.go | 2 +- pkg/controller/common/util.go | 6 +++--- pkg/controller/populators/import-populator_test.go | 2 +- pkg/controller/populators/populator-base.go | 2 +- pkg/controller/populators/upload-populator_test.go | 4 ++-- 10 files changed, 13 insertions(+), 13 deletions(-) diff --git a/pkg/controller/clone/csi-clone.go b/pkg/controller/clone/csi-clone.go index 6596ea29d0..68306db6bf 100644 --- a/pkg/controller/clone/csi-clone.go +++ b/pkg/controller/clone/csi-clone.go @@ -106,10 +106,10 @@ func (p *CSIClonePhase) createClaim(ctx context.Context) (*corev1.PersistentVolu desiredClaim.Spec.Resources.Requests[corev1.ResourceStorage] = sourceSize cc.AddAnnotation(desiredClaim, cc.AnnPopulatorKind, cdiv1.VolumeCloneSourceRef) - cc.AddAnnotation(desiredClaim, cc.AnnExcludeFromVeleroBackup, "true") if p.OwnershipLabel != "" { AddOwnershipLabel(p.OwnershipLabel, desiredClaim, p.Owner) } + cc.AddLabel(desiredClaim, cc.LabelExcludeFromVeleroBackup, "true") if err := p.Client.Create(ctx, desiredClaim); err != nil { checkQuotaExceeded(p.Recorder, p.Owner, err) diff --git a/pkg/controller/clone/csi-clone_test.go b/pkg/controller/clone/csi-clone_test.go index 80a06b2ac8..742baca494 100644 --- a/pkg/controller/clone/csi-clone_test.go +++ b/pkg/controller/clone/csi-clone_test.go @@ -155,10 +155,10 @@ var _ = Describe("CSIClonePhase test", func() { Expect(pvc.Spec.DataSourceRef.Namespace).To(BeNil()) Expect(pvc.Spec.DataSourceRef.Name).To(Equal(sourceClaim.Name)) Expect(pvc.Annotations[cc.AnnPopulatorKind]).To(Equal(cdiv1.VolumeCloneSourceRef)) - Expect(pvc.Annotations[cc.AnnExcludeFromVeleroBackup]).To(Equal("true")) Expect(pvc.Spec.Resources.Requests[corev1.ResourceStorage]). To(Equal(sourceClaim.Status.Capacity[corev1.ResourceStorage])) Expect(pvc.Labels[p.OwnershipLabel]).To(Equal("uid")) + Expect(pvc.Labels[cc.LabelExcludeFromVeleroBackup]).To(Equal("true")) }) Context("with desired claim created", func() { diff --git a/pkg/controller/clone/host-clone.go b/pkg/controller/clone/host-clone.go index 03a2cad98f..d48151bfe3 100644 --- a/pkg/controller/clone/host-clone.go +++ b/pkg/controller/clone/host-clone.go @@ -120,7 +120,6 @@ 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.AnnExcludeFromVeleroBackup, "true") 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 != "" { @@ -132,6 +131,7 @@ func (p *HostClonePhase) createClaim(ctx context.Context) (*corev1.PersistentVol if p.PriorityClassName != "" { cc.AddAnnotation(claim, cc.AnnPriorityClassName, p.PriorityClassName) } + cc.AddLabel(claim, cc.LabelExcludeFromVeleroBackup, "true") if err := p.Client.Create(ctx, claim); err != nil { checkQuotaExceeded(p.Recorder, p.Owner, err) diff --git a/pkg/controller/clone/host-clone_test.go b/pkg/controller/clone/host-clone_test.go index a8230ff7a1..f5b1e49ef5 100644 --- a/pkg/controller/clone/host-clone_test.go +++ b/pkg/controller/clone/host-clone_test.go @@ -105,8 +105,8 @@ var _ = Describe("HostClonePhase test", func() { Expect(pvc.Annotations[cc.AnnPodRestarts]).To(Equal("0")) Expect(pvc.Annotations[cc.AnnCloneRequest]).To(Equal("ns/source")) Expect(pvc.Annotations[cc.AnnPopulatorKind]).To(Equal(cdiv1.VolumeCloneSourceRef)) - Expect(pvc.Annotations[cc.AnnExcludeFromVeleroBackup]).To(Equal("true")) Expect(pvc.Labels[p.OwnershipLabel]).To(Equal("uid")) + Expect(pvc.Labels[cc.LabelExcludeFromVeleroBackup]).To(Equal("true")) Expect(pvc.Annotations[cc.AnnImmediateBinding]).To(Equal("")) _, ok := pvc.Annotations[cc.AnnPriorityClassName] Expect(ok).To(BeFalse()) diff --git a/pkg/controller/clone/snap-clone.go b/pkg/controller/clone/snap-clone.go index 949a84fb8d..e554905945 100644 --- a/pkg/controller/clone/snap-clone.go +++ b/pkg/controller/clone/snap-clone.go @@ -100,10 +100,10 @@ func (p *SnapshotClonePhase) createClaim(ctx context.Context, snapshot *snapshot } cc.AddAnnotation(claim, cc.AnnPopulatorKind, cdiv1.VolumeCloneSourceRef) - cc.AddAnnotation(claim, cc.AnnExcludeFromVeleroBackup, "true") if p.OwnershipLabel != "" { AddOwnershipLabel(p.OwnershipLabel, claim, p.Owner) } + cc.AddLabel(claim, cc.LabelExcludeFromVeleroBackup, "true") if err := p.Client.Create(ctx, claim); err != nil { checkQuotaExceeded(p.Recorder, p.Owner, err) diff --git a/pkg/controller/clone/snap-clone_test.go b/pkg/controller/clone/snap-clone_test.go index bcef28436c..9cac768433 100644 --- a/pkg/controller/clone/snap-clone_test.go +++ b/pkg/controller/clone/snap-clone_test.go @@ -175,9 +175,9 @@ var _ = Describe("SnapshotClonePhase test", func() { Expect(pvc.Spec.DataSourceRef.Namespace).To(BeNil()) Expect(pvc.Spec.DataSourceRef.Name).To(Equal(sourceName)) Expect(pvc.Annotations[cc.AnnPopulatorKind]).To(Equal(cdiv1.VolumeCloneSourceRef)) - Expect(pvc.Annotations[cc.AnnExcludeFromVeleroBackup]).To(Equal("true")) Expect(pvc.Spec.Resources.Requests[corev1.ResourceStorage]).To(Equal(restoreSize)) Expect(pvc.Labels[p.OwnershipLabel]).To(Equal("uid")) + Expect(pvc.Labels[cc.LabelExcludeFromVeleroBackup]).To(Equal("true")) }) Context("with desired claim created", func() { diff --git a/pkg/controller/common/util.go b/pkg/controller/common/util.go index bf90a2fe80..9e24cbb9cb 100644 --- a/pkg/controller/common/util.go +++ b/pkg/controller/common/util.go @@ -239,9 +239,6 @@ const ( // AnnImmediateBinding provides a const to indicate whether immediate binding should be performed on the PV (overrides global config) AnnImmediateBinding = AnnAPIGroup + "/storage.bind.immediate.requested" - //AnnExcludeFromVeleroBackup provides a const to indicate whether an object should be excluded from velero backup - AnnExcludeFromVeleroBackup = "velero.io/exclude-from-backup" - // AnnSelectedNode annotation is added to a PVC that has been triggered by scheduler to // be dynamically provisioned. Its value is the name of the selected node. AnnSelectedNode = "volume.kubernetes.io/selected-node" @@ -305,6 +302,9 @@ const ( // LabelDynamicCredentialSupport specifies if the OS supports updating credentials at runtime. LabelDynamicCredentialSupport = "kubevirt.io/dynamic-credentials-support" + // LabelExcludeFromVeleroBackup provides a const to indicate whether an object should be excluded from velero backup + LabelExcludeFromVeleroBackup = "velero.io/exclude-from-backup" + // ProgressDone this means we are DONE ProgressDone = "100.0%" diff --git a/pkg/controller/populators/import-populator_test.go b/pkg/controller/populators/import-populator_test.go index d178a3786c..e05834a75e 100644 --- a/pkg/controller/populators/import-populator_test.go +++ b/pkg/controller/populators/import-populator_test.go @@ -266,11 +266,11 @@ var _ = Describe("Import populator tests", func() { Expect(pvcPrime.GetAnnotations()[AnnImmediateBinding]).To(Equal("")) Expect(pvcPrime.GetAnnotations()[AnnUploadRequest]).To(Equal("")) Expect(pvcPrime.GetAnnotations()[AnnPopulatorKind]).To(Equal(cdiv1.VolumeImportSourceRef)) - Expect(pvcPrime.GetAnnotations()[AnnExcludeFromVeleroBackup]).To(Equal("true")) Expect(pvcPrime.GetAnnotations()[AnnPreallocationRequested]).To(Equal("true")) Expect(pvcPrime.GetAnnotations()[AnnEndpoint]).To(Equal("http://example.com/data")) Expect(pvcPrime.GetAnnotations()[AnnSource]).To(Equal(SourceHTTP)) Expect(pvcPrime.Annotations[key]).To(Equal(expectedValue)) + Expect(pvcPrime.GetLabels()[LabelExcludeFromVeleroBackup]).To(Equal("true")) }, table.Entry("No extra annotations", "", "", ""), table.Entry("Invalid extra annotation is not passed", "invalid", "test", ""), diff --git a/pkg/controller/populators/populator-base.go b/pkg/controller/populators/populator-base.go index 7a8efba8bd..09189df9df 100644 --- a/pkg/controller/populators/populator-base.go +++ b/pkg/controller/populators/populator-base.go @@ -171,8 +171,8 @@ func (r *ReconcilerBase) getPVCPrime(pvc *corev1.PersistentVolumeClaim) (*corev1 func (r *ReconcilerBase) createPVCPrime(pvc *corev1.PersistentVolumeClaim, source client.Object, waitForFirstConsumer bool, updatePVCForPopulation pvcModifierFunc) (*corev1.PersistentVolumeClaim, error) { labels := make(map[string]string) labels[common.CDILabelKey] = common.CDILabelValue + labels[cc.LabelExcludeFromVeleroBackup] = "true" annotations := make(map[string]string) - annotations[cc.AnnExcludeFromVeleroBackup] = "true" annotations[cc.AnnImmediateBinding] = "" if waitForFirstConsumer { annotations[cc.AnnSelectedNode] = pvc.Annotations[cc.AnnSelectedNode] diff --git a/pkg/controller/populators/upload-populator_test.go b/pkg/controller/populators/upload-populator_test.go index 72c26e3046..2537aa7512 100644 --- a/pkg/controller/populators/upload-populator_test.go +++ b/pkg/controller/populators/upload-populator_test.go @@ -79,7 +79,7 @@ var _ = Describe("Datavolume controller reconcile loop", func() { Expect(pvcPrime.GetAnnotations()[cc.AnnContentType]).To(Equal(contentType)) Expect(pvcPrime.GetAnnotations()[cc.AnnPreallocationRequested]).To(Equal(strconv.FormatBool(preallocation))) Expect(pvcPrime.GetAnnotations()[cc.AnnPopulatorKind]).To(Equal(cdiv1.VolumeUploadSourceRef)) - Expect(pvcPrime.GetAnnotations()[cc.AnnExcludeFromVeleroBackup]).To(Equal("true")) + Expect(pvcPrime.GetLabels()[cc.LabelExcludeFromVeleroBackup]).To(Equal("true")) _, err = r.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-pvc", Namespace: metav1.NamespaceDefault}}) Expect(err).ToNot(HaveOccurred()) @@ -326,8 +326,8 @@ var _ = Describe("Datavolume controller reconcile loop", func() { Expect(pvcPrime.GetAnnotations()[cc.AnnImmediateBinding]).To(Equal("")) Expect(pvcPrime.GetAnnotations()[cc.AnnUploadRequest]).To(Equal("")) Expect(pvcPrime.GetAnnotations()[cc.AnnPopulatorKind]).To(Equal(cdiv1.VolumeUploadSourceRef)) - Expect(pvcPrime.GetAnnotations()[cc.AnnExcludeFromVeleroBackup]).To(Equal("true")) Expect(pvcPrime.Annotations[key]).To(Equal(expectedValue)) + Expect(pvcPrime.GetLabels()[cc.LabelExcludeFromVeleroBackup]).To(Equal("true")) }, Entry("No extra annotations", "", "", ""), Entry("Invalid extra annotation is not passed", "invalid", "test", ""),