From 77f7bbceb623bcb9455635f649b2ef4a275a024f Mon Sep 17 00:00:00 2001 From: Jason Parraga Date: Sun, 27 Oct 2024 20:06:48 -0700 Subject: [PATCH] fix: pruning related issues (#332) * Fix issue with shadowing Signed-off-by: Jason Parraga * Sync cronjob updates Signed-off-by: Jason Parraga * Fix prune command Signed-off-by: Jason Parraga * service accounts and unit tests for cron jobs Signed-off-by: Jason Parraga --------- Signed-off-by: Jason Parraga --- internal/controller/install/common_helpers.go | 8 ++++++++ internal/controller/install/lookout_controller.go | 5 +++-- internal/controller/install/lookout_controller_test.go | 2 +- internal/controller/install/scheduler_controller.go | 7 ++++--- internal/controller/install/scheduler_controller_test.go | 7 ++++--- 5 files changed, 20 insertions(+), 9 deletions(-) diff --git a/internal/controller/install/common_helpers.go b/internal/controller/install/common_helpers.go index 6d5f328..f2bca45 100644 --- 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 index ef0926e..ba817bc 100644 --- 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 index 72ba1dc..70cbb46 100644 --- 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 index 0609c40..9953fc2 100644 --- 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 index 0001638..f1581e6 100644 --- 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()) }