Skip to content

Commit

Permalink
move filtering code to seperate method, add tests
Browse files Browse the repository at this point in the history 
Signed-off-by: Jeffrey Koehler <[email protected]>
  • Loading branch information
Jeffrey Koehler committed Nov 16, 2023
1 parent 8eec686 commit 292aa34
Show file tree
Hide file tree
Showing 2 changed files with 200 additions and 28 deletions.
58 changes: 30 additions & 28 deletions pkg/controller/backup_sync_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
Expand Down Expand Up @@ -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":

Check warning on line 298 in pkg/controller/backup_sync_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/backup_sync_controller.go#L296-L298 

Added lines #L296 - L298 were not covered by tests
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
}

Check warning on line 321 in pkg/controller/backup_sync_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/backup_sync_controller.go#L321 

Added line #L321 was not covered by tests
// SetupWithManager is used to setup controller and its watching sources.
func (b *backupSyncReconciler) SetupWithManager(mgr ctrl.Manager) error {
backupSyncSource := kube.NewPeriodicalEnqueueSource(
Expand Down
170 changes: 170 additions & 0 deletions pkg/controller/backup_sync_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
}
})
})

0 comments on commit 292aa34

Please sign in to comment.