Repository: vshn/k8up
Pull request #463 (2021-07-23 08:52:44)

Consistent job naming
## Summary

This PR:
- Prefixes the job with the backup type 
- Sets the type as the container name

Part of #427.


<!--
Thank you for your pull request. Please provide a description above and
review the checklist below.
-->

## Checklist
<!--
Remove items that do not apply. For completed items, change [ ] to [x].
-->

- [x] Keep pull requests small so they can be easily reviewed.
- [x] Categorize the PR by setting a good title and adding one of the labels:
      `bug`, `enhancement`, `documentation`, `change`, `breaking`,
      as they show up in the changelog
- [x] Update the documentation. (Looked through the docs, the generated batch/v1.Job name is referenced nowhere)
- [x] Update tests.
- [x] Link this PR to related issues.

<!--
NOTE: these things are not required to open a PR and can be done afterwards,
while the PR is open.
-->



Patch:
diff --git a/api/v1alpha1/archive_types.go b/api/v1alpha1/archive_types.go
index 806b195e..6ec0573b 100644
--- a/api/v1alpha1/archive_types.go
+++ b/api/v1alpha1/archive_types.go
@@ -57,6 +57,11 @@ func (a *Archive) GetMetaObject() metav1.Object {
 	return a
 }
 
+// GetJobName returns the name of the underlying batch/v1 job.
+func (a *Archive) GetJobName() string {
+	return a.GetType().String() + "-" + a.Name
+}
+
 func (*Archive) GetType() JobType {
 	return ArchiveType
 }
diff --git a/api/v1alpha1/backup_types.go b/api/v1alpha1/backup_types.go
index 307976c0..8e6378a7 100644
--- a/api/v1alpha1/backup_types.go
+++ b/api/v1alpha1/backup_types.go
@@ -91,6 +91,11 @@ func (*Backup) GetType() JobType {
 	return BackupType
 }
 
+// GetJobName returns the name of the underlying batch/v1 job.
+func (b *Backup) GetJobName() string {
+	return b.GetType().String() + "-" + b.Name
+}
+
 // GetStatus retrieves the Status property
 func (b *Backup) GetStatus() Status {
 	return b.Status
diff --git a/api/v1alpha1/check_types.go b/api/v1alpha1/check_types.go
index e32b9dbf..9f4e863a 100644
--- a/api/v1alpha1/check_types.go
+++ b/api/v1alpha1/check_types.go
@@ -66,6 +66,11 @@ func (c *Check) GetMetaObject() metav1.Object {
 	return c
 }
 
+// GetJobName returns the name of the underlying batch/v1 job.
+func (c *Check) GetJobName() string {
+	return c.GetType().String() + "-" + c.Name
+}
+
 func (c *Check) GetType() JobType {
 	return CheckType
 }
diff --git a/api/v1alpha1/job_object.go b/api/v1alpha1/job_object.go
index 8fd544cd..079ae598 100644
--- a/api/v1alpha1/job_object.go
+++ b/api/v1alpha1/job_object.go
@@ -15,6 +15,8 @@ type JobObject interface {
 	GetStatus() Status
 	SetStatus(s Status)
 	GetType() JobType
+	// GetJobName returns the name of the underlying batch/v1 job.
+	GetJobName() string
 	GetResources() corev1.ResourceRequirements
 }
 
diff --git a/api/v1alpha1/prune_types.go b/api/v1alpha1/prune_types.go
index d37bc7e6..144aeb12 100644
--- a/api/v1alpha1/prune_types.go
+++ b/api/v1alpha1/prune_types.go
@@ -85,6 +85,11 @@ func (p *Prune) GetMetaObject() metav1.Object {
 	return p
 }
 
+// GetJobName returns the name of the underlying batch/v1 job.
+func (p *Prune) GetJobName() string {
+	return p.GetType().String() + "-" + p.Name
+}
+
 func (p *Prune) GetType() JobType {
 	return PruneType
 }
diff --git a/api/v1alpha1/restore_types.go b/api/v1alpha1/restore_types.go
index 60e6cfa6..e3bd640c 100644
--- a/api/v1alpha1/restore_types.go
+++ b/api/v1alpha1/restore_types.go
@@ -75,6 +75,11 @@ func (r *Restore) GetMetaObject() metav1.Object {
 	return r
 }
 
+// GetJobName returns the name of the underlying batch/v1 job.
+func (r *Restore) GetJobName() string {
+	return r.GetType().String() + "-" + r.Name
+}
+
 func (r *Restore) GetType() JobType {
 	return RestoreType
 }
diff --git a/api/v1alpha1/schedule_types.go b/api/v1alpha1/schedule_types.go
index bf67e23a..1424b673 100644
--- a/api/v1alpha1/schedule_types.go
+++ b/api/v1alpha1/schedule_types.go
@@ -119,6 +119,11 @@ func (s *Schedule) GetMetaObject() metav1.Object {
 	return s
 }
 
+// GetJobName implements the JobObject interface.
+func (s *Schedule) GetJobName() string {
+	return s.GetType().String() + "-" + s.Name
+}
+
 func (*Schedule) GetType() JobType {
 	return ScheduleType
 }
diff --git a/executor/generic.go b/executor/generic.go
index 5ca22ca9..bf21f27a 100644
--- a/executor/generic.go
+++ b/executor/generic.go
@@ -104,7 +104,7 @@ func (g *generic) GetJobNamespace() string {
 }
 
 func (g *generic) GetJobNamespacedName() types.NamespacedName {
-	return types.NamespacedName{Namespace: g.Obj.GetMetaObject().GetNamespace(), Name: g.Obj.GetMetaObject().GetName()}
+	return types.NamespacedName{Namespace: g.Obj.GetMetaObject().GetNamespace(), Name: g.Obj.GetJobName()}
 }
 
 func (g *generic) GetJobType() k8upv1alpha1.JobType {
diff --git a/job/job.go b/job/job.go
index dfa4d8d8..24a11649 100644
--- a/job/job.go
+++ b/job/job.go
@@ -52,7 +52,7 @@ func NewConfig(ctx context.Context, client client.Client, log logr.Logger, obj k
 func GenerateGenericJob(obj k8upv1alpha1.JobObject, config Config) (*batchv1.Job, error) {
 	job := &batchv1.Job{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      obj.GetMetaObject().GetName(),
+			Name:      obj.GetJobName(),
 			Namespace: obj.GetMetaObject().GetNamespace(),
 			Labels: map[string]string{
 				K8uplabel:                  "true",
@@ -65,7 +65,7 @@ func GenerateGenericJob(obj k8upv1alpha1.JobObject, config Config) (*batchv1.Job
 					RestartPolicy: corev1.RestartPolicyOnFailure,
 					Containers: []corev1.Container{
 						{
-							Name:      obj.GetMetaObject().GetName(),
+							Name:      obj.GetType().String(),
 							Image:     cfg.Config.BackupImage,
 							Resources: config.Obj.GetResources(),
 						},


Test patch:
diff --git a/api/v1alpha1/job_name_test.go b/api/v1alpha1/job_name_test.go
new file mode 100644
index 00000000..1ea42517
--- /dev/null
+++ b/api/v1alpha1/job_name_test.go
@@ -0,0 +1,26 @@
+package v1alpha1_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/vshn/k8up/api/v1alpha1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestJobName(t *testing.T) {
+	subjects := map[string]v1alpha1.JobObject{
+		"archive-job-name":  &v1alpha1.Archive{ObjectMeta: metav1.ObjectMeta{Name: "job-name"}},
+		"backup-job-name":   &v1alpha1.Backup{ObjectMeta: metav1.ObjectMeta{Name: "job-name"}},
+		"check-job-name":    &v1alpha1.Check{ObjectMeta: metav1.ObjectMeta{Name: "job-name"}},
+		"prune-job-name":    &v1alpha1.Prune{ObjectMeta: metav1.ObjectMeta{Name: "job-name"}},
+		"restore-job-name":  &v1alpha1.Restore{ObjectMeta: metav1.ObjectMeta{Name: "job-name"}},
+		"schedule-job-name": &v1alpha1.Schedule{ObjectMeta: metav1.ObjectMeta{Name: "job-name"}},
+	}
+
+	for expectedName, subject := range subjects {
+		t.Run(expectedName, func(t *testing.T) {
+			assert.Equal(t, expectedName, subject.GetJobName())
+		})
+	}
+}
diff --git a/controllers/backup_it_utils_test.go b/controllers/backup_it_utils_test.go
index f6853fb6..fd4188b4 100644
--- a/controllers/backup_it_utils_test.go
+++ b/controllers/backup_it_utils_test.go
@@ -262,7 +262,7 @@ func (ts *BackupTestSuite) notifyObserverOfBackupJobStatusChange(status k8upObse
 	event := observer.GetJobByName(ts.BackupResource.Namespace + "/" + ts.BackupResource.Name)
 	event.Job = &batchv1.Job{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      ts.BackupResource.Name,
+			Name:      ts.BackupResource.GetJobName(),
 			Namespace: ts.BackupResource.Namespace,
 		},
 	}
diff --git a/controllers/check_integration_test.go b/controllers/check_integration_test.go
index ca5a20a8..ec368e53 100644
--- a/controllers/check_integration_test.go
+++ b/controllers/check_integration_test.go
@@ -76,10 +76,10 @@ func (ts *CheckTestSuite) TestJobCleanup() {
 	successfulJobs := 2
 	failedJobs := 3
 	for i := 0; i < successfulJobs; i++ {
-		ts.whenJobCallbackIsInvoked(ts.CheckNames[i], observer.Succeeded)
+		ts.whenJobCallbackIsInvoked(ts.GivenChecks[i], observer.Succeeded)
 	}
 	for i := successfulJobs; i < successfulJobs+failedJobs; i++ {
-		ts.whenJobCallbackIsInvoked(ts.CheckNames[i], observer.Failed)
+		ts.whenJobCallbackIsInvoked(ts.GivenChecks[i], observer.Failed)
 	}
 
 	ts.expectCheckCleanupEventually((successfulJobs - ts.KeepSuccessful) + (failedJobs - ts.KeepFailed))
@@ -110,16 +110,16 @@ func (ts *CheckTestSuite) expectCheckCleanupEventually(expectedDeletes int) {
 	})
 }
 
-func (ts *CheckTestSuite) whenJobCallbackIsInvoked(checkName string, evtType observer.EventType) {
-	checkNSName := types.NamespacedName{Name: checkName, Namespace: ts.NS}
+func (ts *CheckTestSuite) whenJobCallbackIsInvoked(check k8upv1a1.JobObject, evtType observer.EventType) {
+	checkNSName := types.NamespacedName{Name: check.GetJobName(), Namespace: ts.NS}
 
-	check := &batchv1.Job{}
-	ts.FetchResource(checkNSName, check)
+	childJob := &batchv1.Job{}
+	ts.FetchResource(checkNSName, childJob)
 
 	o := observer.GetObserver()
 	observableJob := o.GetJobByName(checkNSName.String())
 	observableJob.Event = evtType
-	observableJob.Job = check
+	observableJob.Job = childJob
 
 	eventChannel := o.GetUpdateChannel()
 	eventChannel <- observableJob
@@ -136,7 +136,7 @@ func (ts *CheckTestSuite) givenCheckResources(amount int) {
 }
 
 func (ts *CheckTestSuite) whenReconcile() (lastResult controllerruntime.Result) {
-	for _, checkName := range ts.CheckNames {
+	for _, check := range ts.GivenChecks {
 		controller := controllers.CheckReconciler{
 			Client: ts.Client,
 			Log:    ts.Logger,
@@ -145,7 +145,7 @@ func (ts *CheckTestSuite) whenReconcile() (lastResult controllerruntime.Result)
 
 		key := types.NamespacedName{
 			Namespace: ts.NS,
-			Name:      checkName,
+			Name:      check.GetMetaObject().GetName(),
 		}
 		request := controllerruntime.Request{
 			NamespacedName: key,