From 41139c7561953d848bff24982475eaa2dde0be59 Mon Sep 17 00:00:00 2001 From: Luca Corrieri Date: Thu, 21 Sep 2023 15:35:01 +0200 Subject: [PATCH] feat(tfrun): define maxRetries in config --- cmd/controllers/start.go | 1 + deploy/charts/burrito/values.yaml | 1 + internal/burrito/config/config.go | 2 ++ internal/burrito/config/config_test.go | 7 +++++-- internal/burrito/config/testdata/test-config-1.yaml | 1 + internal/controllers/terraformrun/conditions.go | 7 +++---- .../controllers/terraformrun/controller_test.go | 13 +++++++------ 7 files changed, 20 insertions(+), 12 deletions(-) diff --git a/cmd/controllers/start.go b/cmd/controllers/start.go index 88dfef6e1..4d7db5932 100644 --- a/cmd/controllers/start.go +++ b/cmd/controllers/start.go @@ -34,6 +34,7 @@ func buildControllersStartCmd(app *burrito.App) *cobra.Command { cmd.Flags().DurationVar(&app.Config.Controller.Timers.OnError, "on-error-period", defaultOnErrorTimer, "period between two runners launch when an error occurred in the controllers. Must end with s, m or h.") cmd.Flags().DurationVar(&app.Config.Controller.Timers.WaitAction, "wait-action-period", defaultWaitActionTimer, "period between two runners when a layer is locked. Must end with s, m or h.") cmd.Flags().DurationVar(&app.Config.Controller.Timers.FailureGracePeriod, "failure-grace-period", defaultFailureGracePeriod, "initial time before retry, goes exponential function of number failure. Must end with s, m or h.") + cmd.Flags().IntVar(&app.Config.Controller.TerraformMaxRetries, "terraform-max-retries", 5, "default number of retries for terraform actions (can be overriden in CRDs)") cmd.Flags().BoolVar(&app.Config.Controller.LeaderElection.Enabled, "leader-election", true, "whether leader election is enabled or not, default to true") cmd.Flags().StringVar(&app.Config.Controller.LeaderElection.ID, "leader-election-id", "6d185457.terraform.padok.cloud", "lease id used for leader election") cmd.Flags().StringVar(&app.Config.Controller.HealthProbeBindAddress, "health-probe-bind-address", ":8081", "address to bind the metrics server embedded in the controllers") diff --git a/deploy/charts/burrito/values.yaml b/deploy/charts/burrito/values.yaml index fc120169a..f8a704ece 100644 --- a/deploy/charts/burrito/values.yaml +++ b/deploy/charts/burrito/values.yaml @@ -30,6 +30,7 @@ config: onError: 1m waitAction: 1m failureGracePeriod: 15s + terraformMaxRetries: 5 types: ["layer", "repository", "run", "pullrequest"] leaderElection: enabled: true diff --git a/internal/burrito/config/config.go b/internal/burrito/config/config.go index c62eb4577..eed36e0ba 100644 --- a/internal/burrito/config/config.go +++ b/internal/burrito/config/config.go @@ -34,6 +34,7 @@ type WebhookGitlabConfig struct { type ControllerConfig struct { Namespaces []string `mapstructure:"namespaces"` Timers ControllerTimers `mapstructure:"timers"` + TerraformMaxRetries int `mapstructure:"terraformMaxRetries"` Types []string `mapstructure:"types"` LeaderElection LeaderElectionConfig `mapstructure:"leaderElection"` MetricsBindAddress string `mapstructure:"metricsBindAddress"` @@ -180,6 +181,7 @@ func TestConfig() *Config { Database: 0, }, Controller: ControllerConfig{ + TerraformMaxRetries: 5, Timers: ControllerTimers{ DriftDetection: 20 * time.Minute, WaitAction: 5 * time.Minute, diff --git a/internal/burrito/config/config_test.go b/internal/burrito/config/config_test.go index b9c678b5e..61b08feb4 100644 --- a/internal/burrito/config/config_test.go +++ b/internal/burrito/config/config_test.go @@ -59,7 +59,8 @@ func TestConfig_FromYamlFile(t *testing.T) { WaitAction: 1 * time.Minute, FailureGracePeriod: 15 * time.Second, }, - Types: []string{"layer", "repository", "run", "pullrequest"}, + TerraformMaxRetries: 5, + Types: []string{"layer", "repository", "run", "pullrequest"}, LeaderElection: config.LeaderElectionConfig{ Enabled: true, ID: "6d185457.terraform.padok.cloud", @@ -143,6 +144,7 @@ func TestConfig_EnvVarOverrides(t *testing.T) { setEnvVar(t, "BURRITO_CONTROLLER_TIMERS_ONERROR", "30s", &envVarList) setEnvVar(t, "BURRITO_CONTROLLER_TIMERS_WAITACTION", "30s", &envVarList) setEnvVar(t, "BURRITO_CONTROLLER_TIMERS_FAILUREGRACEPERIOD", "1m", &envVarList) + setEnvVar(t, "BURRITO_CONTROLLER_TERRAFORMMAXRETRIES", "32", &envVarList) setEnvVar(t, "BURRITO_CONTROLLER_LEADERELECTION_ID", "other-leader-id", &envVarList) setEnvVar(t, "BURRITO_CONTROLLER_GITHUBCONFIG_APITOKEN", "pr-github-token", &envVarList) setEnvVar(t, "BURRITO_CONTROLLER_GITLABCONFIG_APITOKEN", "mr-gitlab-token", &envVarList) @@ -192,7 +194,8 @@ func TestConfig_EnvVarOverrides(t *testing.T) { WaitAction: 30 * time.Second, FailureGracePeriod: 1 * time.Minute, }, - Types: []string{"layer", "repository"}, + TerraformMaxRetries: 32, + Types: []string{"layer", "repository"}, LeaderElection: config.LeaderElectionConfig{ Enabled: true, ID: "other-leader-id", diff --git a/internal/burrito/config/testdata/test-config-1.yaml b/internal/burrito/config/testdata/test-config-1.yaml index a22b70958..4472a780b 100644 --- a/internal/burrito/config/testdata/test-config-1.yaml +++ b/internal/burrito/config/testdata/test-config-1.yaml @@ -18,6 +18,7 @@ controller: onError: 1m waitAction: 1m failureGracePeriod: 15s + terraformMaxRetries: 5 types: ["layer", "repository", "run", "pullrequest"] leaderElection: enabled: true diff --git a/internal/controllers/terraformrun/conditions.go b/internal/controllers/terraformrun/conditions.go index b4e07cbca..d381c84f7 100644 --- a/internal/controllers/terraformrun/conditions.go +++ b/internal/controllers/terraformrun/conditions.go @@ -44,13 +44,12 @@ func (r *Reconciler) HasStatus(t *configv1alpha1.TerraformRun) (metav1.Condition return condition, false } -func GetMaxRetries(r *configv1alpha1.TerraformRepository, l *configv1alpha1.TerraformLayer) int { +func GetMaxRetries(defaultValue int, r *configv1alpha1.TerraformRepository, l *configv1alpha1.TerraformLayer) int { repo := r.Spec.RemediationStrategy.OnError.MaxRetries layer := l.Spec.RemediationStrategy.OnError.MaxRetries if repo == nil && layer == nil { - // TODO: Default value in config ? - return 5 + return defaultValue } if repo == nil && layer != nil { return *layer @@ -74,7 +73,7 @@ func (r *Reconciler) HasReachedRetryLimit( Status: metav1.ConditionUnknown, LastTransitionTime: metav1.NewTime(time.Now()), } - maxRetries := GetMaxRetries(repo, layer) + maxRetries := GetMaxRetries(r.Config.Controller.TerraformMaxRetries, repo, layer) if run.Status.Retries >= maxRetries { condition.Reason = "HasReachedRetryLimit" condition.Message = fmt.Sprintf("This run has reached the retry limit (%d)", maxRetries) diff --git a/internal/controllers/terraformrun/controller_test.go b/internal/controllers/terraformrun/controller_test.go index af6b98fce..26389a632 100644 --- a/internal/controllers/terraformrun/controller_test.go +++ b/internal/controllers/terraformrun/controller_test.go @@ -479,12 +479,13 @@ var _ = AfterSuite(func() { }) func TestGetMaxRetries(t *testing.T) { + // Config + defaultMaxRetries := 42 // Test case 1: Both repo and layer max retries are nil r1 := &configv1alpha1.TerraformRepository{Spec: configv1alpha1.TerraformRepositorySpec{}} l1 := &configv1alpha1.TerraformLayer{Spec: configv1alpha1.TerraformLayerSpec{}} - // TODO: use config - expectedResult1 := 5 - result1 := controller.GetMaxRetries(r1, l1) + expectedResult1 := defaultMaxRetries + result1 := controller.GetMaxRetries(defaultMaxRetries, r1, l1) if result1 != expectedResult1 { t.Errorf("Test case 1 failed: expected %d, got %d", expectedResult1, result1) } @@ -493,7 +494,7 @@ func TestGetMaxRetries(t *testing.T) { r2 := &configv1alpha1.TerraformRepository{Spec: configv1alpha1.TerraformRepositorySpec{}} l2 := &configv1alpha1.TerraformLayer{Spec: configv1alpha1.TerraformLayerSpec{RemediationStrategy: configv1alpha1.RemediationStrategy{OnError: configv1alpha1.OnErrorRemediationStrategy{MaxRetries: intPtr(3)}}}} expectedResult2 := 3 - result2 := controller.GetMaxRetries(r2, l2) + result2 := controller.GetMaxRetries(defaultMaxRetries, r2, l2) if result2 != expectedResult2 { t.Errorf("Test case 2 failed: expected %d, got %d", expectedResult2, result2) } @@ -502,7 +503,7 @@ func TestGetMaxRetries(t *testing.T) { r3 := &configv1alpha1.TerraformRepository{Spec: configv1alpha1.TerraformRepositorySpec{RemediationStrategy: configv1alpha1.RemediationStrategy{OnError: configv1alpha1.OnErrorRemediationStrategy{MaxRetries: intPtr(7)}}}} l3 := &configv1alpha1.TerraformLayer{Spec: configv1alpha1.TerraformLayerSpec{}} expectedResult3 := 7 - result3 := controller.GetMaxRetries(r3, l3) + result3 := controller.GetMaxRetries(defaultMaxRetries, r3, l3) if result3 != expectedResult3 { t.Errorf("Test case 3 failed: expected %d, got %d", expectedResult3, result3) } @@ -511,7 +512,7 @@ func TestGetMaxRetries(t *testing.T) { r4 := &configv1alpha1.TerraformRepository{Spec: configv1alpha1.TerraformRepositorySpec{RemediationStrategy: configv1alpha1.RemediationStrategy{OnError: configv1alpha1.OnErrorRemediationStrategy{MaxRetries: intPtr(4)}}}} l4 := &configv1alpha1.TerraformLayer{Spec: configv1alpha1.TerraformLayerSpec{RemediationStrategy: configv1alpha1.RemediationStrategy{OnError: configv1alpha1.OnErrorRemediationStrategy{MaxRetries: intPtr(8)}}}} expectedResult4 := 8 - result4 := controller.GetMaxRetries(r4, l4) + result4 := controller.GetMaxRetries(defaultMaxRetries, r4, l4) if result4 != expectedResult4 { t.Errorf("Test case 4 failed: expected %d, got %d", expectedResult4, result4) }