From 8018ce12c6a25ab7d49bf707b7e0fd8f340ac27e Mon Sep 17 00:00:00 2001 From: Luca Corrieri Date: Sun, 5 Nov 2023 16:51:41 +0100 Subject: [PATCH] fix(pr): delete temporary layers before discovery --- .../terraformpullrequest/conditions.go | 18 --------------- .../controllers/terraformpullrequest/layer.go | 23 +++++++++++++++++++ .../terraformpullrequest/states.go | 19 +++++---------- 3 files changed, 29 insertions(+), 31 deletions(-) diff --git a/internal/controllers/terraformpullrequest/conditions.go b/internal/controllers/terraformpullrequest/conditions.go index ec1caf160..c5234eebb 100644 --- a/internal/controllers/terraformpullrequest/conditions.go +++ b/internal/controllers/terraformpullrequest/conditions.go @@ -1,15 +1,11 @@ package terraformpullrequest import ( - "context" "time" configv1alpha1 "github.com/padok-team/burrito/api/v1alpha1" "github.com/padok-team/burrito/internal/annotations" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/selection" - "sigs.k8s.io/controller-runtime/pkg/client" ) func (r *Reconciler) IsLastCommitDiscovered(pr *configv1alpha1.TerraformPullRequest) (metav1.Condition, bool) { @@ -143,17 +139,3 @@ func (r *Reconciler) IsCommentUpToDate(pr *configv1alpha1.TerraformPullRequest) condition.Status = metav1.ConditionTrue return condition, true } - -func GetLinkedLayers(cl client.Client, pr *configv1alpha1.TerraformPullRequest) ([]configv1alpha1.TerraformLayer, error) { - layers := configv1alpha1.TerraformLayerList{} - requirement, err := labels.NewRequirement("burrito/managed-by", selection.Equals, []string{pr.Name}) - if err != nil { - return nil, err - } - selector := labels.NewSelector().Add(*requirement) - err = cl.List(context.TODO(), &layers, client.MatchingLabelsSelector{Selector: selector}) - if err != nil { - return nil, err - } - return layers.Items, nil -} diff --git a/internal/controllers/terraformpullrequest/layer.go b/internal/controllers/terraformpullrequest/layer.go index ac95a438b..6e71c2f70 100644 --- a/internal/controllers/terraformpullrequest/layer.go +++ b/internal/controllers/terraformpullrequest/layer.go @@ -5,6 +5,9 @@ import ( "fmt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" + "sigs.k8s.io/controller-runtime/pkg/client" configv1alpha1 "github.com/padok-team/burrito/api/v1alpha1" "github.com/padok-team/burrito/internal/annotations" @@ -96,3 +99,23 @@ func generateTempLayers(pr *configv1alpha1.TerraformPullRequest, layers []config } return list } + +func GetLinkedLayers(cl client.Client, pr *configv1alpha1.TerraformPullRequest) ([]configv1alpha1.TerraformLayer, error) { + layers := configv1alpha1.TerraformLayerList{} + requirement, err := labels.NewRequirement("burrito/managed-by", selection.Equals, []string{pr.Name}) + if err != nil { + return nil, err + } + selector := labels.NewSelector().Add(*requirement) + err = cl.List(context.TODO(), &layers, client.MatchingLabelsSelector{Selector: selector}) + if err != nil { + return nil, err + } + return layers.Items, nil +} + +func (r *Reconciler) deleteTempLayers(ctx context.Context, pr *configv1alpha1.TerraformPullRequest) error { + return r.Client.DeleteAllOf( + ctx, &configv1alpha1.TerraformLayer{}, client.InNamespace(pr.Namespace), client.MatchingLabels{"burrito/managed-by": pr.Name}, + ) +} diff --git a/internal/controllers/terraformpullrequest/states.go b/internal/controllers/terraformpullrequest/states.go index 7d67800c3..f1681dd89 100644 --- a/internal/controllers/terraformpullrequest/states.go +++ b/internal/controllers/terraformpullrequest/states.go @@ -9,7 +9,6 @@ import ( "github.com/padok-team/burrito/internal/annotations" "github.com/padok-team/burrito/internal/controllers/terraformpullrequest/comment" log "github.com/sirupsen/logrus" - "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" ) @@ -55,6 +54,11 @@ type DiscoveryNeeded struct{} func (s *DiscoveryNeeded) getHandler() func(ctx context.Context, r *Reconciler, repository *configv1alpha1.TerraformRepository, pr *configv1alpha1.TerraformPullRequest) ctrl.Result { return func(ctx context.Context, r *Reconciler, repository *configv1alpha1.TerraformRepository, pr *configv1alpha1.TerraformPullRequest) ctrl.Result { + // Delete already existing temporary layers + err := r.deleteTempLayers(ctx, pr) + if err != nil { + log.Errorf("failed to delete temp layers for pull request %s: %s", pr.Name, err) + } layers, err := r.getAffectedLayers(repository, pr) if err != nil { log.Errorf("failed to get affected layers for pull request %s: %s", pr.Name, err) @@ -63,23 +67,12 @@ func (s *DiscoveryNeeded) getHandler() func(ctx context.Context, r *Reconciler, newLayers := generateTempLayers(pr, layers) for _, layer := range newLayers { err := r.Client.Create(ctx, &layer) - // TODO: Fix this, it's not working because of generateName, and with a Name it fails to update - // because you need to inject metadata.resourceVersion. But we want a generateName to avoid - // conflicts with other layers. - // Alternative: delete all linked layers when PR updates - if errors.IsAlreadyExists(err) { - log.Infof("layer %s already exists, updating it", layer.Name) - err = r.Client.Update(ctx, &layer) - if err != nil { - log.Errorf("failed to update layer %s: %s", layer.Name, err) - return ctrl.Result{RequeueAfter: r.Config.Controller.Timers.OnError} - } - } if err != nil { log.Errorf("failed to create layer %s: %s", layer.Name, err) return ctrl.Result{RequeueAfter: r.Config.Controller.Timers.OnError} } } + // TODO: setting this annotation triggers another reconciliation cycle while this one is still running, which is not ideal err = annotations.Add(ctx, r.Client, pr, map[string]string{annotations.LastDiscoveredCommit: pr.Annotations[annotations.LastBranchCommit]}) if err != nil { log.Errorf("failed to add annotation %s to pull request %s: %s", annotations.LastDiscoveredCommit, pr.Name, err)