move filtering code to seperate method, add tests

Signed-off-by: Jeffrey Koehler <koehler@streem.tech>
This commit is contained in:
Jeffrey Koehler
2023-11-16 03:57:36 -06:00
parent 8eec6865d1
commit 292aa34a48
2 changed files with 200 additions and 28 deletions

View File

@@ -173,34 +173,7 @@ 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.
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.Warn("Removing missing schedule ownership reference from backup")
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)
}
backup.ObjectMeta.OwnerReferences = foundReferences
backup.ObjectMeta.OwnerReferences = b.filterBackupOwnerReferences(ctx, backup, log)
// attempt to create backup custom resource via API
err = b.client.Create(ctx, backup, &client.CreateOptions{})
@@ -317,6 +290,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(

View File

@@ -724,4 +724,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))
})
}
})
})