From e5a549c9411c6f260b0963f8a2e08e55bfb1d721 Mon Sep 17 00:00:00 2001 From: Alan Date: Mon, 20 May 2024 15:53:00 +0200 Subject: [PATCH] feat(layer): align status with kept terraformruns --- api/v1alpha1/common.go | 9 +-- api/v1alpha1/common_test.go | 28 +++------ api/v1alpha1/zz_generated.deepcopy.go | 9 +-- cmd/controllers/start.go | 6 +- deploy/charts/burrito/values.yaml | 33 +---------- internal/burrito/config/config.go | 1 + .../controllers/terraformlayer/controller.go | 47 ++++++++++++--- internal/controllers/terraformlayer/run.go | 56 +----------------- internal/datastore/storage/redis/redis.go | 58 ------------------- ...terraform.padok.cloud_terraformlayers.yaml | 4 +- ...orm.padok.cloud_terraformrepositories.yaml | 4 +- manifests/install.yaml | 8 +-- 12 files changed, 67 insertions(+), 196 deletions(-) delete mode 100644 internal/datastore/storage/redis/redis.go diff --git a/api/v1alpha1/common.go b/api/v1alpha1/common.go index 13e2bda0..9e549c9b 100644 --- a/api/v1alpha1/common.go +++ b/api/v1alpha1/common.go @@ -6,8 +6,7 @@ import ( ) const ( - PlanRunRetention int = 6 - ApplyRunRetention int = 6 + DefaultRunRetention int = 10 ) type OverrideRunnerSpec struct { @@ -31,8 +30,7 @@ type MetadataOverride struct { } type RunHistoryPolicy struct { - KeepLastPlanRuns *int `json:"plan,omitempty"` - KeepLastApplyRuns *int `json:"apply,omitempty"` + KeepLastRuns *int `json:"runs,omitempty"` } type RemediationStrategy struct { @@ -89,8 +87,7 @@ func GetOverrideRunnerSpec(repository *TerraformRepository, layer *TerraformLaye func GetRunHistoryPolicy(repository *TerraformRepository, layer *TerraformLayer) RunHistoryPolicy { return RunHistoryPolicy{ - KeepLastPlanRuns: chooseInt(repository.Spec.RunHistoryPolicy.KeepLastPlanRuns, layer.Spec.RunHistoryPolicy.KeepLastPlanRuns, PlanRunRetention), - KeepLastApplyRuns: chooseInt(repository.Spec.RunHistoryPolicy.KeepLastApplyRuns, layer.Spec.RunHistoryPolicy.KeepLastApplyRuns, ApplyRunRetention), + KeepLastRuns: chooseInt(repository.Spec.RunHistoryPolicy.KeepLastRuns, layer.Spec.RunHistoryPolicy.KeepLastRuns, 10), } } diff --git a/api/v1alpha1/common_test.go b/api/v1alpha1/common_test.go index 933592e4..4da313be 100644 --- a/api/v1alpha1/common_test.go +++ b/api/v1alpha1/common_test.go @@ -1581,15 +1581,13 @@ func TestGetHistoryPolicy(t *testing.T) { &configv1alpha1.TerraformRepository{ Spec: configv1alpha1.TerraformRepositorySpec{ RunHistoryPolicy: configv1alpha1.RunHistoryPolicy{ - KeepLastPlanRuns: intPointer(10), - KeepLastApplyRuns: intPointer(10), + KeepLastRuns: intPointer(10), }, }, }, &configv1alpha1.TerraformLayer{}, configv1alpha1.RunHistoryPolicy{ - KeepLastPlanRuns: intPointer(10), - KeepLastApplyRuns: intPointer(10), + KeepLastRuns: intPointer(10), }, }, { @@ -1598,14 +1596,12 @@ func TestGetHistoryPolicy(t *testing.T) { &configv1alpha1.TerraformLayer{ Spec: configv1alpha1.TerraformLayerSpec{ RunHistoryPolicy: configv1alpha1.RunHistoryPolicy{ - KeepLastPlanRuns: intPointer(10), - KeepLastApplyRuns: intPointer(10), + KeepLastRuns: intPointer(10), }, }, }, configv1alpha1.RunHistoryPolicy{ - KeepLastPlanRuns: intPointer(10), - KeepLastApplyRuns: intPointer(10), + KeepLastRuns: intPointer(10), }, }, { @@ -1613,22 +1609,19 @@ func TestGetHistoryPolicy(t *testing.T) { &configv1alpha1.TerraformRepository{ Spec: configv1alpha1.TerraformRepositorySpec{ RunHistoryPolicy: configv1alpha1.RunHistoryPolicy{ - KeepLastPlanRuns: intPointer(10), - KeepLastApplyRuns: intPointer(10), + KeepLastRuns: intPointer(10), }, }, }, &configv1alpha1.TerraformLayer{ Spec: configv1alpha1.TerraformLayerSpec{ RunHistoryPolicy: configv1alpha1.RunHistoryPolicy{ - KeepLastPlanRuns: intPointer(6), - KeepLastApplyRuns: intPointer(5), + KeepLastRuns: intPointer(5), }, }, }, configv1alpha1.RunHistoryPolicy{ - KeepLastPlanRuns: intPointer(6), - KeepLastApplyRuns: intPointer(5), + KeepLastRuns: intPointer(5), }, }, } @@ -1636,11 +1629,8 @@ func TestGetHistoryPolicy(t *testing.T) { for _, tc := range tt { t.Run(tc.name, func(t *testing.T) { result := configv1alpha1.GetRunHistoryPolicy(tc.repository, tc.layer) - if *tc.expectedHistoryPolicy.KeepLastPlanRuns != *result.KeepLastPlanRuns { - t.Errorf("different plan policy computed: expected %d got %d", *tc.expectedHistoryPolicy.KeepLastPlanRuns, *result.KeepLastPlanRuns) - } - if *tc.expectedHistoryPolicy.KeepLastApplyRuns != *result.KeepLastApplyRuns { - t.Errorf("different apply policy computed: expected %d got %d", *tc.expectedHistoryPolicy.KeepLastApplyRuns, *result.KeepLastApplyRuns) + if *tc.expectedHistoryPolicy.KeepLastRuns != *result.KeepLastRuns { + t.Errorf("different policy computed: expected %d got %d", *tc.expectedHistoryPolicy.KeepLastRuns, *result.KeepLastRuns) } }) } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 418e033f..3630e412 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -198,13 +198,8 @@ func (in *RemediationStrategy) DeepCopy() *RemediationStrategy { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RunHistoryPolicy) DeepCopyInto(out *RunHistoryPolicy) { *out = *in - if in.KeepLastPlanRuns != nil { - in, out := &in.KeepLastPlanRuns, &out.KeepLastPlanRuns - *out = new(int) - **out = **in - } - if in.KeepLastApplyRuns != nil { - in, out := &in.KeepLastApplyRuns, &out.KeepLastApplyRuns + if in.KeepLastRuns != nil { + in, out := &in.KeepLastRuns, &out.KeepLastRuns *out = new(int) **out = **in } diff --git a/cmd/controllers/start.go b/cmd/controllers/start.go index 1d7e168b..42e67979 100644 --- a/cmd/controllers/start.go +++ b/cmd/controllers/start.go @@ -23,9 +23,9 @@ func buildControllersStartCmd(app *burrito.App) *cobra.Command { }, } - defaultDriftDetectionTimer, _ := time.ParseDuration("1h") - defaultOnErrorTimer, _ := time.ParseDuration("30s") - defaultWaitActionTimer, _ := time.ParseDuration("15s") + defaultDriftDetectionTimer, _ := time.ParseDuration("4h") + defaultOnErrorTimer, _ := time.ParseDuration("10s") + defaultWaitActionTimer, _ := time.ParseDuration("5s") defaultFailureGracePeriod, _ := time.ParseDuration("15s") cmd.Flags().StringSliceVar(&app.Config.Controller.Namespaces, "namespaces", []string{"burrito-system"}, "list of namespaces to watch") diff --git a/deploy/charts/burrito/values.yaml b/deploy/charts/burrito/values.yaml index aa3aba26..e84dc7e2 100644 --- a/deploy/charts/burrito/values.yaml +++ b/deploy/charts/burrito/values.yaml @@ -15,14 +15,10 @@ config: burrito: # Burrito controller configuration controller: - # -- By default, the controller will watch all namespaces + # -- By default, the controller will only watch the tenants namespaces namespaces: [] - timers: - driftDetection: 1h - onError: 30s - waitAction: 15s - failureGracePeriod: 15s - terraformMaxRetries: 5 + timers: {} + terraformMaxRetries: 3 types: ["layer", "repository", "run", "pullrequest"] leaderElection: enabled: true @@ -70,29 +66,6 @@ config: runner: sshKnownHostsConfigMapName: burrito-ssh-known-hosts -# redis: -# enabled: true -# metadata: -# labels: -# app.kubernetes.io/component: redis -# app.kubernetes.io/name: burrito-redis -# deployment: -# image: -# repository: redis -# tag: "7.2.4-alpine" -# pullPolicy: Always -# args: [] -# podSecurityContext: -# runAsNonRoot: true -# runAsUser: 999 -# seccompProfile: -# type: RuntimeDefault -# service: -# ports: -# - name: tcp-redis -# port: 6379 -# targetPort: 6379 - hermitcrab: metadata: labels: diff --git a/internal/burrito/config/config.go b/internal/burrito/config/config.go index 426760ee..adae5de2 100644 --- a/internal/burrito/config/config.go +++ b/internal/burrito/config/config.go @@ -70,6 +70,7 @@ type ControllerConfig struct { KubernetesWebhookPort int `mapstructure:"kubernetesWebhookPort"` GithubConfig GithubConfig `mapstructure:"githubConfig"` GitlabConfig GitlabConfig `mapstructure:"gitlabConfig"` + RunParallelism int `mapstructure:"runParallelism"` } type GithubConfig struct { diff --git a/internal/controllers/terraformlayer/controller.go b/internal/controllers/terraformlayer/controller.go index 86ecf82c..4ebf7708 100644 --- a/internal/controllers/terraformlayer/controller.go +++ b/internal/controllers/terraformlayer/controller.go @@ -109,10 +109,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu log.Errorf("failed to get TerraformRepository linked to layer %s: %s", layer.Name, err) return ctrl.Result{RequeueAfter: r.Config.Controller.Timers.OnError}, err } - err = r.cleanupRuns(ctx, layer, repository) - if err != nil { - log.Warningf("failed to cleanup runs for layer %s: %s", layer.Name, err) - } state, conditions := r.GetState(ctx, layer) lastResult, err := r.Datastore.GetPlan(layer.Namespace, layer.Name, layer.Status.LastRun.Name, "", "short") if err != nil { @@ -124,7 +120,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu runHistory := layer.Status.LatestRuns if run != nil { lastRun = getRun(*run) - runHistory = updateLatestRuns(runHistory, *run) + runHistory = updateLatestRuns(runHistory, *run, *configv1alpha1.GetRunHistoryPolicy(repository, layer).KeepLastRuns) } layer.Status = configv1alpha1.TerraformLayerStatus{Conditions: conditions, State: getStateString(state), LastResult: string(lastResult), LastRun: lastRun, LatestRuns: runHistory} err = r.Client.Status().Update(ctx, layer) @@ -132,10 +128,47 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu r.Recorder.Event(layer, corev1.EventTypeWarning, "Reconciliation", "Could not update layer status") log.Errorf("could not update layer %s status: %s", layer.Name, err) } + err = r.cleanupRuns(ctx, layer, repository) + if err != nil { + log.Warningf("failed to cleanup runs for layer %s: %s", layer.Name, err) + } log.Infof("finished reconciliation cycle for layer %s/%s", layer.Namespace, layer.Name) return result, nil } +func (r *Reconciler) cleanupRuns(ctx context.Context, layer *configv1alpha1.TerraformLayer, repository *configv1alpha1.TerraformRepository) error { + historyPolicy := configv1alpha1.GetRunHistoryPolicy(repository, layer) + if len(layer.Status.LatestRuns) < *historyPolicy.KeepLastRuns { + log.Infof("no runs to delete for layer %s", layer.Name) + return nil + } + runs, err := r.getAllRuns(ctx, layer) + if err != nil { + return err + } + runsToKeep := map[string]bool{} + for _, run := range layer.Status.LatestRuns { + runsToKeep[run.Name] = true + } + toDelete := []*configv1alpha1.TerraformRun{} + for _, run := range runs { + if _, ok := runsToKeep[run.Name]; !ok { + toDelete = append(toDelete, run) + } + } + if len(toDelete) == 0 { + log.Infof("no runs to delete for layer %s", layer.Name) + return nil + } + err = deleteAll(ctx, r.Client, toDelete) + if err != nil { + return err + } + log.Infof("deleted %d runs for layer %s", len(toDelete), layer.Name) + r.Recorder.Event(layer, corev1.EventTypeNormal, "Reconciliation", "Cleaned up old runs") + return nil +} + func getRun(run configv1alpha1.TerraformRun) configv1alpha1.TerraformLayerRun { return configv1alpha1.TerraformLayerRun{ Name: run.Name, @@ -145,7 +178,7 @@ func getRun(run configv1alpha1.TerraformRun) configv1alpha1.TerraformLayerRun { } } -func updateLatestRuns(runs []configv1alpha1.TerraformLayerRun, run configv1alpha1.TerraformRun) []configv1alpha1.TerraformLayerRun { +func updateLatestRuns(runs []configv1alpha1.TerraformLayerRun, run configv1alpha1.TerraformRun, keep int) []configv1alpha1.TerraformLayerRun { oldestRun := &configv1alpha1.TerraformLayerRun{ Date: metav1.NewTime(time.Now()), } @@ -157,7 +190,7 @@ func updateLatestRuns(runs []configv1alpha1.TerraformLayerRun, run configv1alpha oldestRunIndex = i } } - if oldestRun == nil || len(runs) < 5 { + if oldestRun == nil || len(runs) < keep { return append(runs, newRun) } rs := append(runs[:oldestRunIndex], runs[oldestRunIndex+1:]...) diff --git a/internal/controllers/terraformlayer/run.go b/internal/controllers/terraformlayer/run.go index 30cf82e6..a2b4f26d 100644 --- a/internal/controllers/terraformlayer/run.go +++ b/internal/controllers/terraformlayer/run.go @@ -3,14 +3,12 @@ package terraformlayer import ( "context" "fmt" - "sort" "strings" "sync" configv1alpha1 "github.com/padok-team/burrito/api/v1alpha1" "github.com/padok-team/burrito/internal/annotations" log "github.com/sirupsen/logrus" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/selection" @@ -62,7 +60,7 @@ func (r *Reconciler) getRun(layer *configv1alpha1.TerraformLayer, repository *co } } -func (r *Reconciler) getAllFinishedRuns(ctx context.Context, layer *configv1alpha1.TerraformLayer, repository *configv1alpha1.TerraformRepository) ([]*configv1alpha1.TerraformRun, error) { +func (r *Reconciler) getAllRuns(ctx context.Context, layer *configv1alpha1.TerraformLayer) ([]*configv1alpha1.TerraformRun, error) { list := &configv1alpha1.TerraformRunList{} labelSelector := labels.NewSelector() for key, value := range GetDefaultLabels(layer) { @@ -85,9 +83,7 @@ func (r *Reconciler) getAllFinishedRuns(ctx context.Context, layer *configv1alph // Keep only runs with state Succeeded or Failed var runs []*configv1alpha1.TerraformRun for _, run := range list.Items { - if run.Status.State == "Succeeded" || run.Status.State == "Failed" { - runs = append(runs, &run) - } + runs = append(runs, &run) } return runs, nil } @@ -125,51 +121,3 @@ func deleteAll(ctx context.Context, c client.Client, objs []*configv1alpha1.Terr return ret } - -func (r *Reconciler) cleanupRuns(ctx context.Context, layer *configv1alpha1.TerraformLayer, repository *configv1alpha1.TerraformRepository) error { - historyPolicy := configv1alpha1.GetRunHistoryPolicy(repository, layer) - - runs, err := r.getAllFinishedRuns(ctx, layer, repository) - if err != nil { - return err - } - sortedRuns := sortAndSplitRunsByAction(runs) - toDelete := []*configv1alpha1.TerraformRun{} - if len(sortedRuns[string(PlanAction)]) <= *historyPolicy.KeepLastPlanRuns { - log.Infof("no plan runs to delete for layer %s", layer.Name) - } else { - toDelete = append(toDelete, sortedRuns[string(PlanAction)][:len(sortedRuns[string(PlanAction)])-*historyPolicy.KeepLastPlanRuns]...) - } - if len(sortedRuns[string(ApplyAction)]) <= *historyPolicy.KeepLastApplyRuns { - log.Infof("no apply runs to delete for layer %s", layer.Name) - } else { - toDelete = append(toDelete, sortedRuns[string(ApplyAction)][:len(sortedRuns[string(ApplyAction)])-*historyPolicy.KeepLastApplyRuns]...) - } - if len(toDelete) == 0 { - log.Infof("no runs to delete for layer %s", layer.Name) - return nil - } - err = deleteAll(ctx, r.Client, toDelete) - if err != nil { - return err - } - log.Infof("deleted %d runs for layer %s", len(toDelete), layer.Name) - r.Recorder.Event(layer, corev1.EventTypeNormal, "Reconciliation", "Cleaned up old runs") - return nil -} - -func sortAndSplitRunsByAction(runs []*configv1alpha1.TerraformRun) map[string][]*configv1alpha1.TerraformRun { - splittedRuns := map[string][]*configv1alpha1.TerraformRun{} - for _, run := range runs { - if _, ok := splittedRuns[run.Spec.Action]; !ok { - splittedRuns[run.Spec.Action] = []*configv1alpha1.TerraformRun{} - } - splittedRuns[run.Spec.Action] = append(splittedRuns[run.Spec.Action], run) - } - for action := range splittedRuns { - sort.Slice(splittedRuns[action], func(i, j int) bool { - return splittedRuns[action][i].CreationTimestamp.Before(&splittedRuns[action][j].CreationTimestamp) - }) - } - return splittedRuns -} diff --git a/internal/datastore/storage/redis/redis.go b/internal/datastore/storage/redis/redis.go deleted file mode 100644 index 2ac52af0..00000000 --- a/internal/datastore/storage/redis/redis.go +++ /dev/null @@ -1,58 +0,0 @@ -package redis - -import ( - "context" - "fmt" - "time" - - "github.com/go-redis/redis/v8" - "github.com/padok-team/burrito/internal/burrito/config" - storageerrors "github.com/padok-team/burrito/internal/datastore/storage/error" -) - -type Storage struct { - Client *redis.Client -} - -func New(config config.Redis) *Storage { - return &Storage{ - Client: redis.NewClient(&redis.Options{ - Addr: fmt.Sprintf("%s:%d", config.Hostname, config.ServerPort), - Password: config.Password, // no password set - DB: config.Database, // use default DB - }), - } -} - -func (s *Storage) Get(key string) ([]byte, error) { - val, err := s.Client.Get(context.TODO(), key).Result() - if err == redis.Nil { - return nil, &storageerrors.StorageError{ - Err: err, - Nil: true, - } - } - if err != nil { - return nil, &storageerrors.StorageError{ - Err: err, - Nil: false, - } - } - return []byte(val), nil -} - -func (s *Storage) Set(key string, value []byte, ttl int) error { - err := s.Client.Set(context.TODO(), key, value, time.Second*time.Duration(ttl)).Err() - if err != nil { - return err - } - return nil -} - -func (s *Storage) Delete(key string) error { - err := s.Client.Del(context.TODO(), key).Err() - if err != nil { - return err - } - return nil -} diff --git a/manifests/crds/config.terraform.padok.cloud_terraformlayers.yaml b/manifests/crds/config.terraform.padok.cloud_terraformlayers.yaml index b8dfa192..4df50976 100644 --- a/manifests/crds/config.terraform.padok.cloud_terraformlayers.yaml +++ b/manifests/crds/config.terraform.padok.cloud_terraformlayers.yaml @@ -2023,9 +2023,7 @@ spec: type: object runHistoryPolicy: properties: - apply: - type: integer - plan: + runs: type: integer type: object terraform: diff --git a/manifests/crds/config.terraform.padok.cloud_terraformrepositories.yaml b/manifests/crds/config.terraform.padok.cloud_terraformrepositories.yaml index e929e218..23df85fd 100644 --- a/manifests/crds/config.terraform.padok.cloud_terraformrepositories.yaml +++ b/manifests/crds/config.terraform.padok.cloud_terraformrepositories.yaml @@ -2009,9 +2009,7 @@ spec: type: object runHistoryPolicy: properties: - apply: - type: integer - plan: + runs: type: integer type: object terraform: diff --git a/manifests/install.yaml b/manifests/install.yaml index c3f25c05..6882ef3e 100644 --- a/manifests/install.yaml +++ b/manifests/install.yaml @@ -1906,9 +1906,7 @@ spec: type: object runHistoryPolicy: properties: - apply: - type: integer - plan: + runs: type: integer type: object terraform: @@ -4069,9 +4067,7 @@ spec: type: object runHistoryPolicy: properties: - apply: - type: integer - plan: + runs: type: integer type: object terraform: