fix: pruning related issues (#332)

* Fix issue with shadowing Signed-off-by: Jason Parraga <sovietaced@gmail.com> * Sync cronjob updates Signed-off-by: Jason Parraga <sovietaced@gmail.com> * Fix prune command Signed-off-by: Jason Parraga <sovietaced@gmail.com> * service accounts and unit tests for cron jobs Signed-off-by: Jason Parraga <sovietaced@gmail.com> --------- Signed-off-by: Jason Parraga <sovietaced@gmail.com>
armadaproject · Oct 28, 2024 · 77f7bbc · 77f7bbc
1 parent c2d681d
commit 77f7bbc
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 9 deletions.
diff --git a/internal/controller/install/common_helpers.go b/internal/controller/install/common_helpers.go
@@ -196,6 +196,14 @@ func (cc *CommonComponents) ReconcileComponents(newComponents *CommonComponents)
 		cc.PriorityClasses[i].Labels = newComponents.PriorityClasses[i].Labels
 		cc.PriorityClasses[i].Annotations = newComponents.PriorityClasses[i].Annotations
 	}
+
+	if newComponents.CronJob != nil {
+		cc.CronJob.Spec = newComponents.CronJob.Spec
+		cc.CronJob.Annotations = newComponents.CronJob.Annotations
+		cc.CronJob.Labels = newComponents.CronJob.Labels
+	} else {
+		cc.CronJob = nil
+	}
 }
 
 // PostgresConfig is used for scanning postgres section of application config

diff --git a/internal/controller/install/lookout_controller.go b/internal/controller/install/lookout_controller.go
@@ -212,7 +212,7 @@ func generateLookoutInstallComponents(
 
 	var cronJob *batchv1.CronJob
 	if enabled := lookout.Spec.DbPruningEnabled; enabled != nil && *enabled {
-		cronJob, err = createLookoutCronJob(lookout)
+		cronJob, err = createLookoutCronJob(lookout, serviceAccountName)
 		if err != nil {
 			return nil, err
 		}
@@ -442,7 +442,7 @@ func createLookoutMigrationJob(lookout *installv1alpha1.Lookout, serviceAccountN
 }
 
 // createLookoutCronJob returns a batch CronJob or an error if the app config is not correct
-func createLookoutCronJob(lookout *installv1alpha1.Lookout) (*batchv1.CronJob, error) {
+func createLookoutCronJob(lookout *installv1alpha1.Lookout, serviceAccountName string) (*batchv1.CronJob, error) {
 	terminationGracePeriodSeconds := int64(0)
 	if lookout.Spec.TerminationGracePeriodSeconds != nil {
 		terminationGracePeriodSeconds = *lookout.Spec.TerminationGracePeriodSeconds
@@ -494,6 +494,7 @@ func createLookoutCronJob(lookout *installv1alpha1.Lookout) (*batchv1.CronJob, e
 							Labels:    AllLabels(lookout.Name, lookout.Labels),
 						},
 						Spec: corev1.PodSpec{
+							ServiceAccountName:            serviceAccountName,
 							RestartPolicy:                 "Never",
 							TerminationGracePeriodSeconds: &terminationGracePeriodSeconds,
 							SecurityContext:               lookout.Spec.PodSecurityContext,

diff --git a/internal/controller/install/lookout_controller_test.go b/internal/controller/install/lookout_controller_test.go
@@ -339,7 +339,7 @@ func TestLookoutReconciler_CreateCronJobErrorDueToApplicationConfig(t *testing.T
 			},
 		},
 	}
-	_, err := createLookoutCronJob(&expectedLookout)
+	_, err := createLookoutCronJob(&expectedLookout, "lookout")
 	assert.Error(t, err)
 	assert.Equal(t, "yaml: line 1: did not find expected ',' or '}'", err.Error())
 }

diff --git a/internal/controller/install/scheduler_controller.go b/internal/controller/install/scheduler_controller.go
@@ -220,7 +220,7 @@ func generateSchedulerInstallComponents(
 
 	var cronJob *batchv1.CronJob
 	if scheduler.Spec.Pruner != nil && scheduler.Spec.Pruner.Enabled {
-		cronJob, err := newSchedulerCronJob(scheduler)
+		cronJob, err = newSchedulerCronJob(scheduler, serviceAccountName)
 		if err != nil {
 			return nil, err
 		}
@@ -462,7 +462,7 @@ func newSchedulerMigrationJob(scheduler *installv1alpha1.Scheduler, serviceAccou
 }
 
 // newSchedulerCronJob returns a batch CronJob or an error if the app config is not correct
-func newSchedulerCronJob(scheduler *installv1alpha1.Scheduler) (*batchv1.CronJob, error) {
+func newSchedulerCronJob(scheduler *installv1alpha1.Scheduler, serviceAccountName string) (*batchv1.CronJob, error) {
 	terminationGracePeriodSeconds := int64(0)
 	if scheduler.Spec.TerminationGracePeriodSeconds != nil {
 		terminationGracePeriodSeconds = *scheduler.Spec.TerminationGracePeriodSeconds
@@ -485,7 +485,7 @@ func newSchedulerCronJob(scheduler *installv1alpha1.Scheduler) (*batchv1.CronJob
 	}
 
 	prunerArgs := []string{
-		"--pruneDatabase",
+		"pruneDatabase",
 		appConfigFlag,
 		appConfigFilepath,
 	}
@@ -530,6 +530,7 @@ func newSchedulerCronJob(scheduler *installv1alpha1.Scheduler) (*batchv1.CronJob
 							Labels:    AllLabels(scheduler.Name, scheduler.Labels),
 						},
 						Spec: corev1.PodSpec{
+							ServiceAccountName:            serviceAccountName,
 							RestartPolicy:                 "Never",
 							TerminationGracePeriodSeconds: &terminationGracePeriodSeconds,
 							SecurityContext:               scheduler.Spec.PodSecurityContext,

diff --git a/internal/controller/install/scheduler_controller_test.go b/internal/controller/install/scheduler_controller_test.go
@@ -527,13 +527,14 @@ func TestSchedulerReconciler_createSchedulerCronJob(t *testing.T) {
 			},
 		},
 	}
-	cronJob, err := newSchedulerCronJob(&schedulerInput)
-	expectedArgs := []string{"--pruneDatabase", appConfigFlag, appConfigFilepath, "--timeout", "10m", "--batchsize", "1000", "--expireAfter", "1d"}
+	cronJob, err := newSchedulerCronJob(&schedulerInput, "sa")
+	expectedArgs := []string{"pruneDatabase", appConfigFlag, appConfigFilepath, "--timeout", "10m", "--batchsize", "1000", "--expireAfter", "1d"}
 	expectedResources := *schedulerInput.Spec.Pruner.Resources
 
 	assert.NoError(t, err)
 	assert.Equal(t, expectedArgs, cronJob.Spec.JobTemplate.Spec.Template.Spec.Containers[0].Args)
 	assert.Equal(t, expectedResources, cronJob.Spec.JobTemplate.Spec.Template.Spec.Containers[0].Resources)
+	assert.Equal(t, "sa", cronJob.Spec.JobTemplate.Spec.Template.Spec.ServiceAccountName)
 }
 
 func TestSchedulerReconciler_createSchedulerIngressGrpc_EmptyHosts(t *testing.T) {
@@ -612,7 +613,7 @@ func TestSchedulerReconciler_createSchedulerCronJobError(t *testing.T) {
 			},
 		},
 	}
-	_, err := newSchedulerCronJob(&expectedScheduler)
+	_, err := newSchedulerCronJob(&expectedScheduler, "scheduler")
 	assert.Error(t, err)
 	assert.Equal(t, "yaml: line 1: did not find expected ',' or '}'", err.Error())
 }