🐛 fix endless updates of cronjobs (#999)

* make sure concurrency policies are updated Signed-off-by: Ivan Milchev <[email protected]> * fix endless updates of cronjobs Signed-off-by: Ivan Milchev <[email protected]> * fix tests and add more Signed-off-by: Ivan Milchev <[email protected]> * simplify code Signed-off-by: Ivan Milchev <[email protected]> --------- Signed-off-by: Ivan Milchev <[email protected]>
mondoohq · Jan 12, 2024 · 5477938 · 5477938
1 parent 648c289
commit 5477938
Show file tree

Hide file tree

Showing 15 changed files with 226 additions and 189 deletions.
diff --git a/controllers/container_image/deployment_handler.go b/controllers/container_image/deployment_handler.go
@@ -124,14 +124,10 @@ func (n *DeploymentHandler) syncCronJob(ctx context.Context) error {
 
 	if created {
 		logger.Info("Created CronJob", "namespace", desired.Namespace, "name", desired.Name)
-		err = mondoo.UpdateMondooAuditConfig(ctx, n.KubeClient, n.Mondoo, logger)
-		if err != nil {
-			logger.Error(err, "Failed to update MondooAuditConfig", "namespace", n.Mondoo.Namespace, "name", n.Mondoo.Name)
-			return err
-		}
 	} else if !k8s.AreCronJobsEqual(*existing, *desired) {
 		existing.Spec.JobTemplate = desired.Spec.JobTemplate
 		existing.Spec.Schedule = desired.Spec.Schedule
+		existing.Spec.ConcurrencyPolicy = desired.Spec.ConcurrencyPolicy
 		existing.SetOwnerReferences(desired.GetOwnerReferences())
 
 		// Remove any old jobs because they won't be updated when the cronjob changes

diff --git a/controllers/container_image/deployment_handler_test.go b/controllers/container_image/deployment_handler_test.go
@@ -138,30 +138,6 @@ func (s *DeploymentHandlerSuite) TestReconcile_CreateWithCustomSchedule() {
 	s.Equal(created.Spec.Schedule, customSchedule)
 }
 
-func (s *DeploymentHandlerSuite) TestReconcile_CreateWithCustomScheduleFail() {
-	d := s.createDeploymentHandler()
-	mondooAuditConfig := &s.auditConfig
-	s.NoError(d.KubeClient.Create(s.ctx, mondooAuditConfig))
-
-	customSchedule := "this is not valid"
-	s.auditConfig.Spec.Containers.Schedule = customSchedule
-
-	result, err := d.Reconcile(s.ctx)
-	s.NoError(err)
-	s.True(result.IsZero())
-
-	image, err := s.containerImageResolver.CnspecImage("", "", false)
-	s.NoError(err)
-
-	expected := CronJob(image, "", test.KubeSystemNamespaceUid, "", &s.auditConfig, mondoov1alpha2.MondooOperatorConfig{})
-	created := &batchv1.CronJob{}
-	created.Name = expected.Name
-	created.Namespace = expected.Namespace
-	s.NoError(d.KubeClient.Get(s.ctx, client.ObjectKeyFromObject(created), created))
-
-	s.NotEqual(created.Spec.Schedule, customSchedule)
-}
-
 func (s *DeploymentHandlerSuite) TestReconcile_Create_PrivateRegistriesSecret() {
 	d := s.createDeploymentHandler()
 	mondooAuditConfig := &s.auditConfig

diff --git a/controllers/container_image/resources.go b/controllers/container_image/resources.go
@@ -6,10 +6,9 @@ package container_image
 import (
 	"fmt"
 	"strings"
-	"time"
 
 	// That's the mod k8s relies on https://github.com/kubernetes/kubernetes/blob/master/go.mod#L63
-	"github.com/robfig/cron/v3"
+
 	"go.mondoo.com/cnquery/v9/providers-sdk/v1/inventory"
 	"go.mondoo.com/mondoo-operator/api/v1alpha2"
 	"go.mondoo.com/mondoo-operator/pkg/constants"
@@ -48,30 +47,14 @@ func CronJob(image, integrationMrn, clusterUid, privateImageScanningSecretName s
 	envVars := feature_flags.AllFeatureFlagsAsEnv()
 	envVars = append(envVars, corev1.EnvVar{Name: "MONDOO_AUTO_UPDATE", Value: "false"})
 
-	// We want to start the cron job one minute after it was enabled.
-	cronStart := time.Now().Add(1 * time.Minute)
-	cronTab := fmt.Sprintf("%d %d * * *", cronStart.Minute(), cronStart.Hour())
-	if m.Spec.Containers.Schedule != "" {
-		_, err := cron.ParseStandard(m.Spec.Containers.Schedule)
-		if err != nil {
-			logger.Error(err, "invalid cron schedule specified in MondooAuditConfig Spec.Containers.Schedule; using default")
-		} else {
-			logger.Info("using cron custom schedule", "crontab", m.Spec.Containers.Schedule)
-			cronTab = m.Spec.Containers.Schedule
-		}
-	} else {
-		logger.Info("using default cron schedule", "crontab", cronTab)
-		m.Spec.Containers.Schedule = cronTab
-	}
-
 	cronjob := &batchv1.CronJob{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      CronJobName(m.Name),
 			Namespace: m.Namespace,
 			Labels:    ls,
 		},
 		Spec: batchv1.CronJobSpec{
-			Schedule:          cronTab,
+			Schedule:          m.Spec.Containers.Schedule,
 			ConcurrencyPolicy: batchv1.ForbidConcurrent,
 			JobTemplate: batchv1.JobTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{Labels: ls},

diff --git a/controllers/k8s_scan/deployment_handler.go b/controllers/k8s_scan/deployment_handler.go
@@ -91,14 +91,10 @@ func (n *DeploymentHandler) syncCronJob(ctx context.Context) error {
 
 	if created {
 		logger.Info("Created CronJob", "namespace", desired.Namespace, "name", desired.Name)
-		err = mondoo.UpdateMondooAuditConfig(ctx, n.KubeClient, n.Mondoo, logger)
-		if err != nil {
-			logger.Error(err, "Failed to update MondooAuditConfig", "namespace", n.Mondoo.Namespace, "name", n.Mondoo.Name)
-			return err
-		}
 	} else if !k8s.AreCronJobsEqual(*existing, *desired) {
 		existing.Spec.JobTemplate = desired.Spec.JobTemplate
 		existing.Spec.Schedule = desired.Spec.Schedule
+		existing.Spec.ConcurrencyPolicy = desired.Spec.ConcurrencyPolicy
 		existing.SetOwnerReferences(desired.GetOwnerReferences())
 
 		// Remove any old jobs because they won't be updated when the cronjob changes

diff --git a/controllers/k8s_scan/deployment_handler_test.go b/controllers/k8s_scan/deployment_handler_test.go
@@ -351,26 +351,6 @@ func (s *DeploymentHandlerSuite) TestReconcile_CreateWithDefaultSchedule() {
 	created.Name = expected.Name
 	created.Namespace = expected.Namespace
 	s.NoError(d.KubeClient.Get(s.ctx, client.ObjectKeyFromObject(created), created))
-
-	// Wait a bit to longer, to later check, whether the CronJob schedule was changed.
-	time.Sleep(61 * time.Second)
-
-	s.scanApiStoreMock.EXPECT().Add(&scan_api_store.ScanApiStoreAddOpts{
-		Url:   scanApiUrl,
-		Token: "token",
-	}).Times(1)
-	result, err = d.Reconcile(s.ctx)
-	s.NoError(err)
-	s.True(result.IsZero())
-
-	s.NoError(d.KubeClient.Get(s.ctx, client.ObjectKeyFromObject(created), created))
-	foundMAC := &mondoov1alpha2.MondooAuditConfig{}
-	foundMAC.Name = mondooAuditConfig.Name
-	foundMAC.Namespace = mondooAuditConfig.Namespace
-	s.NoError(d.KubeClient.Get(s.ctx, client.ObjectKeyFromObject(foundMAC), foundMAC))
-
-	s.Equal(created.Spec.Schedule, expected.Spec.Schedule)
-	s.Equal(foundMAC.Spec.KubernetesResources.Schedule, expected.Spec.Schedule)
 }
 
 func (s *DeploymentHandlerSuite) TestReconcile_CreateWithCustomSchedule() {
@@ -404,37 +384,6 @@ func (s *DeploymentHandlerSuite) TestReconcile_CreateWithCustomSchedule() {
 	s.Equal(created.Spec.Schedule, customSchedule)
 }
 
-func (s *DeploymentHandlerSuite) TestReconcile_CreateWithCustomScheduleFail() {
-	d := s.createDeploymentHandler()
-	mondooAuditConfig := &s.auditConfig
-	s.NoError(d.KubeClient.Create(s.ctx, mondooAuditConfig))
-
-	scanApiUrl := scanapi.ScanApiServiceUrl(*d.Mondoo)
-	s.scanApiStoreMock.EXPECT().Add(&scan_api_store.ScanApiStoreAddOpts{
-		Url:   scanApiUrl,
-		Token: "token",
-	}).Times(1)
-
-	customSchedule := "this is not valid"
-	s.auditConfig.Spec.KubernetesResources.Schedule = customSchedule
-
-	result, err := d.Reconcile(s.ctx)
-	s.NoError(err)
-	s.True(result.IsZero())
-
-	image, err := s.containerImageResolver.CnspecImage("", "", false)
-	s.NoError(err)
-
-	expected := CronJob(image, "", test.KubeSystemNamespaceUid, &s.auditConfig)
-
-	created := &batchv1.CronJob{}
-	created.Name = expected.Name
-	created.Namespace = expected.Namespace
-	s.NoError(d.KubeClient.Get(s.ctx, client.ObjectKeyFromObject(created), created))
-
-	s.NotEqual(created.Spec.Schedule, customSchedule)
-}
-
 func (s *DeploymentHandlerSuite) createDeploymentHandler() DeploymentHandler {
 	return DeploymentHandler{
 		KubeClient:             s.fakeClientBuilder.Build(),

diff --git a/controllers/k8s_scan/resources.go b/controllers/k8s_scan/resources.go
@@ -6,10 +6,9 @@ package k8s_scan
 import (
 	"fmt"
 	"strings"
-	"time"
 
 	// That's the mod k8s relies on https://github.com/kubernetes/kubernetes/blob/master/go.mod#L63
-	"github.com/robfig/cron/v3"
+
 	"go.mondoo.com/mondoo-operator/api/v1alpha2"
 	"go.mondoo.com/mondoo-operator/controllers/scanapi"
 	"go.mondoo.com/mondoo-operator/pkg/feature_flags"
@@ -24,20 +23,6 @@ const CronJobNameSuffix = "-k8s-scan"
 
 func CronJob(image, integrationMrn, clusterUid string, m *v1alpha2.MondooAuditConfig) *batchv1.CronJob {
 	ls := CronJobLabels(*m)
-
-	cronTab := fmt.Sprintf("%d * * * *", time.Now().Add(1*time.Minute).Minute())
-	if m.Spec.KubernetesResources.Schedule != "" {
-		_, err := cron.ParseStandard(m.Spec.KubernetesResources.Schedule)
-		if err != nil {
-			logger.Error(err, "invalid cron schedule specified in MondooAuditConfig Spec.KubernetesResources.Schedule; using default")
-		} else {
-			logger.Info("using cron custom schedule", "crontab", m.Spec.KubernetesResources.Schedule)
-			cronTab = m.Spec.KubernetesResources.Schedule
-		}
-	} else {
-		logger.Info("using default cron schedule", "crontab", cronTab)
-		m.Spec.KubernetesResources.Schedule = cronTab
-	}
 	scanApiUrl := scanapi.ScanApiServiceUrl(*m)
 
 	containerArgs := []string{
@@ -73,7 +58,7 @@ func CronJob(image, integrationMrn, clusterUid string, m *v1alpha2.MondooAuditCo
 			Labels:    ls,
 		},
 		Spec: batchv1.CronJobSpec{
-			Schedule:          cronTab,
+			Schedule:          m.Spec.KubernetesResources.Schedule,
 			ConcurrencyPolicy: batchv1.ForbidConcurrent,
 			JobTemplate: batchv1.JobTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{Labels: ls},

diff --git a/controllers/mondooauditconfig_controller.go b/controllers/mondooauditconfig_controller.go
@@ -5,6 +5,7 @@ package controllers
 
 import (
 	"context"
+	"fmt"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -166,6 +167,30 @@ func (r *MondooAuditConfigReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		}
 	}
 
+	// Set the default cron tab if none is set
+	shouldUpdate := false
+	if mondooAuditConfig.Spec.Nodes.Enable && mondooAuditConfig.Spec.Nodes.Schedule == "" {
+		mondooAuditConfig.Spec.Nodes.Schedule = fmt.Sprintf("%d * * * *", time.Now().Add(1*time.Minute).Minute())
+		shouldUpdate = true
+	}
+	if mondooAuditConfig.Spec.KubernetesResources.Enable && mondooAuditConfig.Spec.KubernetesResources.Schedule == "" {
+		mondooAuditConfig.Spec.KubernetesResources.Schedule = fmt.Sprintf("%d * * * *", time.Now().Add(1*time.Minute).Minute())
+		shouldUpdate = true
+	}
+	if mondooAuditConfig.Spec.Containers.Enable && mondooAuditConfig.Spec.Containers.Schedule == "" {
+		cronStart := time.Now().Add(1 * time.Minute)
+		mondooAuditConfig.Spec.Containers.Schedule = fmt.Sprintf("%d %d * * *", cronStart.Minute(), cronStart.Hour())
+		shouldUpdate = true
+	}
+	if shouldUpdate {
+		err := r.Update(ctx, mondooAuditConfig)
+		if err != nil {
+			log.Error(reconcileError, "failed to update MondooAuditConfig with default schedule")
+			return ctrl.Result{}, reconcileError
+		}
+		return ctrl.Result{Requeue: true}, nil
+	}
+
 	mondooAuditConfigCopy := mondooAuditConfig.DeepCopy()
 
 	// Conditions might be updated before this reconciler reaches the end