Merge pull request #7032 from deefdragon/main

Add check for owner references in backup sync, removing if missing

changelogs/unreleased/7032-deefdragon

@@ -0,0 +1 @@
+Fix #6857. Added check for matching Owner References when synchronizing backups, removing references that are not found/have mismatched uid.

pkg/controller/backup_sync_controller.go

@@ -29,6 +29,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 
@@ -181,6 +182,9 @@ func (b *backupSyncReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		}
 		backup.Labels[velerov1api.StorageLocationLabel] = label.GetValidName(backup.Spec.StorageLocation)
 
+		//check for the ownership references. If they do not exist, remove them.
+		backup.ObjectMeta.OwnerReferences = b.filterBackupOwnerReferences(ctx, backup, log)
+
 		// attempt to create backup custom resource via API
 		err = b.client.Create(ctx, backup, &client.CreateOptions{})
 		switch {
@@ -296,6 +300,35 @@ func (b *backupSyncReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	return ctrl.Result{}, nil
 }
 
+func (b *backupSyncReconciler) filterBackupOwnerReferences(ctx context.Context, backup *velerov1api.Backup, log logrus.FieldLogger) []metav1.OwnerReference {
+	listedReferences := backup.ObjectMeta.OwnerReferences
+	foundReferences := make([]metav1.OwnerReference, 0)
+	for _, v := range listedReferences {
+		switch v.Kind {
+		case "Schedule":
+			schedule := new(velerov1api.Schedule)
+			err := b.client.Get(ctx, types.NamespacedName{
+				Name:      v.Name,
+				Namespace: backup.Namespace,
+			}, schedule)
+			switch {
+			case err != nil && apierrors.IsNotFound(err):
+				log.Warnf("Removing missing schedule ownership reference %s/%s from backup", backup.Namespace, v.Name)
+				continue
+			case schedule.UID != v.UID:
+				log.Warnf("Removing schedule ownership reference with mismatched UIDs. Expected %s, got %s", v.UID, schedule.UID)
+				continue
+			case err != nil && !apierrors.IsNotFound(err):
+				log.WithError(errors.WithStack(err)).Error("Error finding schedule ownership reference, keeping schedule on backup")
+			}
+		default:
+			log.Warnf("Unable to check ownership reference for unknown kind, %s", v.Kind)
+		}
+		foundReferences = append(foundReferences, v)
+	}
+	return foundReferences
+}
+
 // SetupWithManager is used to setup controller and its watching sources.
 func (b *backupSyncReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	backupSyncSource := kube.NewPeriodicalEnqueueSource(

pkg/controller/backup_sync_controller_test.go

@@ -725,4 +725,174 @@ var _ = Describe("Backup Sync Reconciler", func() {
 		Expect(testObjList).To(BeEquivalentTo(locationList))
 
 	})
+
+	When("testing validateOwnerReferences", func() {
+
+		testCases := []struct {
+			name               string
+			backup             *velerov1api.Backup
+			toCreate           []ctrlClient.Object
+			expectedReferences []metav1.OwnerReference
+		}{
+			{
+				name: "handles empty owner references",
+				backup: &velerov1api.Backup{
+					ObjectMeta: metav1.ObjectMeta{
+						OwnerReferences: []metav1.OwnerReference{},
+					},
+				},
+				expectedReferences: []metav1.OwnerReference{},
+			},
+			{
+				name: "handles missing schedule",
+				backup: &velerov1api.Backup{
+					ObjectMeta: metav1.ObjectMeta{
+						OwnerReferences: []metav1.OwnerReference{
+							{
+								Kind: "Schedule",
+								Name: "some name",
+							},
+						},
+					},
+				},
+				expectedReferences: []metav1.OwnerReference{},
+			},
+			{
+				name: "handles existing reference",
+				backup: &velerov1api.Backup{
+					ObjectMeta: metav1.ObjectMeta{
+						OwnerReferences: []metav1.OwnerReference{
+							{
+								Kind: "Schedule",
+								Name: "existing-schedule",
+							},
+						},
+						Namespace: "test-namespace",
+					},
+				},
+				toCreate: []ctrlClient.Object{
+					&velerov1api.Schedule{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "existing-schedule",
+							Namespace: "test-namespace",
+						},
+					},
+				},
+				expectedReferences: []metav1.OwnerReference{
+					{
+						Kind: "Schedule",
+						Name: "existing-schedule",
+					},
+				},
+			},
+			{
+				name: "handles existing mismatched UID",
+				backup: &velerov1api.Backup{
+					ObjectMeta: metav1.ObjectMeta{
+						OwnerReferences: []metav1.OwnerReference{
+							{
+								Kind: "Schedule",
+								Name: "existing-schedule",
+								UID:  "backup-UID",
+							},
+						},
+						Namespace: "test-namespace",
+					},
+				},
+				toCreate: []ctrlClient.Object{
+					&velerov1api.Schedule{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "existing-schedule",
+							Namespace: "test-namespace",
+							UID:       "schedule-UID",
+						},
+					},
+				},
+				expectedReferences: []metav1.OwnerReference{},
+			},
+			{
+				name: "handles multiple references",
+				backup: &velerov1api.Backup{
+					ObjectMeta: metav1.ObjectMeta{
+						OwnerReferences: []metav1.OwnerReference{
+							{
+								Kind: "Schedule",
+								Name: "existing-schedule",
+								UID:  "1",
+							},
+							{
+								Kind: "Schedule",
+								Name: "missing-schedule",
+								UID:  "2",
+							},
+							{
+								Kind: "Schedule",
+								Name: "mismatched-uid-schedule",
+								UID:  "3",
+							},
+							{
+								Kind: "Schedule",
+								Name: "another-existing-schedule",
+								UID:  "4",
+							},
+						},
+						Namespace: "test-namespace",
+					},
+				},
+				toCreate: []ctrlClient.Object{
+					&velerov1api.Schedule{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "existing-schedule",
+							Namespace: "test-namespace",
+							UID:       "1",
+						},
+					},
+					&velerov1api.Schedule{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "mismatched-uid-schedule",
+							Namespace: "test-namespace",
+							UID:       "not-3",
+						},
+					},
+					&velerov1api.Schedule{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "another-existing-schedule",
+							Namespace: "test-namespace",
+							UID:       "4",
+						},
+					},
+				},
+				expectedReferences: []metav1.OwnerReference{
+					{
+						Kind: "Schedule",
+						Name: "existing-schedule",
+						UID:  "1",
+					},
+					{
+						Kind: "Schedule",
+						Name: "another-existing-schedule",
+						UID:  "4",
+					},
+				},
+			},
+		}
+		for _, test := range testCases {
+			test := test
+			It(test.name, func() {
+				logger := velerotest.NewLogger()
+				b := backupSyncReconciler{
+					client: ctrlfake.NewClientBuilder().Build(),
+				}
+
+				//create all required schedules as needed.
+				for _, creatable := range test.toCreate {
+					err := b.client.Create(context.Background(), creatable)
+					Expect(err).ShouldNot(HaveOccurred())
+				}
+
+				references := b.filterBackupOwnerReferences(context.Background(), test.backup, logger)
+				Expect(references).To(BeEquivalentTo(test.expectedReferences))
+			})
+		}
+	})
 })