From ccc5fcf41ee7eed0019e34d9baf02f7796085b12 Mon Sep 17 00:00:00 2001 From: michaeljguarino Date: Fri, 6 Dec 2024 18:06:37 -0500 Subject: [PATCH] Fix sha generation for pr automations --- .../controller/prautomation_attributes.go | 2 +- .../controller/prautomation_controller.go | 45 ++++++++++--------- .../prautomation_controller_test.go | 13 ++++-- lib/console/schema/pr_automation.ex | 2 +- 4 files changed, 36 insertions(+), 26 deletions(-) diff --git a/go/controller/internal/controller/prautomation_attributes.go b/go/controller/internal/controller/prautomation_attributes.go index 5f2e62f8b8..8fc1f98efa 100644 --- a/go/controller/internal/controller/prautomation_attributes.go +++ b/go/controller/internal/controller/prautomation_attributes.go @@ -12,7 +12,7 @@ import ( "github.com/pluralsh/console/go/controller/internal/utils" ) -func (in *PrAutomationReconciler) attributes(ctx context.Context, pra *v1alpha1.PrAutomation) (*console.PrAutomationAttributes, error) { +func (in *PrAutomationReconciler) Attributes(ctx context.Context, pra *v1alpha1.PrAutomation) (*console.PrAutomationAttributes, error) { helper := utils.NewConsoleHelper(ctx, in.ConsoleClient, in.Client) clusterID, err := helper.IDFromRef(pra.Spec.ClusterRef, &v1alpha1.Cluster{}) diff --git a/go/controller/internal/controller/prautomation_controller.go b/go/controller/internal/controller/prautomation_controller.go index bfdcd29888..c08053bca0 100644 --- a/go/controller/internal/controller/prautomation_controller.go +++ b/go/controller/internal/controller/prautomation_controller.go @@ -77,16 +77,8 @@ func (in *PrAutomationReconciler) Reconcile(ctx context.Context, req reconcile.R return *result, err } - // Get PrAutomation SHA that can be saved back in the status to check for changes - changed, sha, err := prAutomation.Diff(utils.HashObject) - if err != nil { - logger.Error(err, "unable to calculate prAutomation SHA") - utils.MarkFalse(prAutomation.SetCondition, v1alpha1.SynchronizedConditionType, v1alpha1.SynchronizedConditionReasonError, err.Error()) - return ctrl.Result{}, err - } - // Sync PrAutomation CRD with the Console API - apiPrAutomation, err := in.sync(ctx, prAutomation, changed) + apiPrAutomation, sha, err := in.sync(ctx, prAutomation) if err != nil { if errors.IsNotFound(err) { utils.MarkCondition(prAutomation.SetCondition, v1alpha1.SynchronizedConditionType, metav1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, notFoundOrReadyError) @@ -150,37 +142,50 @@ func (in *PrAutomationReconciler) addOrRemoveFinalizer(ctx context.Context, prAu return nil, nil } -func (in *PrAutomationReconciler) sync(ctx context.Context, prAutomation *v1alpha1.PrAutomation, changed bool) (*console.PrAutomationFragment, error) { +func (in *PrAutomationReconciler) sync(ctx context.Context, prAutomation *v1alpha1.PrAutomation) (pra *console.PrAutomationFragment, sha string, err error) { logger := log.FromContext(ctx) exists, err := in.ConsoleClient.IsPrAutomationExistsByName(ctx, prAutomation.ConsoleName()) if err != nil { - return nil, err + return pra, sha, err } if exists && !prAutomation.Status.HasID() { - return nil, nil + return pra, sha, err } - if err := in.ensure(prAutomation); err != nil { - return nil, err + + if err = in.ensure(prAutomation); err != nil { + return pra, sha, err } - attributes, err := in.attributes(ctx, prAutomation) + + attributes, err := in.Attributes(ctx, prAutomation) if err != nil { - return nil, err + return pra, sha, err + } + + // Get PrAutomation SHA that can be saved back in the status to check for changes + sha, err = utils.HashObject(attributes) + if err != nil { + logger.Error(err, "unable to calculate prAutomation SHA") + utils.MarkFalse(prAutomation.SetCondition, v1alpha1.SynchronizedConditionType, v1alpha1.SynchronizedConditionReasonError, err.Error()) + return pra, sha, err } // Update only if PrAutomation has changed - if changed && exists { + if prAutomation.Status.SHA != nil && *prAutomation.Status.SHA != sha && exists { logger.Info("Updating PR automation") - return in.ConsoleClient.UpdatePrAutomation(ctx, prAutomation.Status.GetID(), *attributes) + pra, err = in.ConsoleClient.UpdatePrAutomation(ctx, prAutomation.Status.GetID(), *attributes) + return pra, sha, err } // Read the PrAutomation from Console API if it already exists if exists { - return in.ConsoleClient.GetPrAutomation(ctx, prAutomation.Status.GetID()) + pra, err = in.ConsoleClient.GetPrAutomation(ctx, prAutomation.Status.GetID()) + return pra, sha, err } logger.Info("Creating PR automation") - return in.ConsoleClient.CreatePrAutomation(ctx, *attributes) + pra, err = in.ConsoleClient.CreatePrAutomation(ctx, *attributes) + return pra, sha, err } func (in *PrAutomationReconciler) updateReadyCondition(prAutomation *v1alpha1.PrAutomation) { diff --git a/go/controller/internal/controller/prautomation_controller_test.go b/go/controller/internal/controller/prautomation_controller_test.go index af84511d9d..47a9025c45 100644 --- a/go/controller/internal/controller/prautomation_controller_test.go +++ b/go/controller/internal/controller/prautomation_controller_test.go @@ -59,7 +59,6 @@ var _ = Describe("PR Automation Controller", func() { Name: scmConnectionName, }, }) - _, prAutomationHash, _ = generatedPrAutomation.Diff(utils.HashObject) prAutomationObjectKey = types.NamespacedName{Name: prAutomationName} ) @@ -104,10 +103,13 @@ var _ = Describe("PR Automation Controller", func() { prAutomation := &v1alpha1.PrAutomation{} err = k8sClient.Get(ctx, prAutomationObjectKey, prAutomation) + attrs, _ := controllerReconciler.Attributes(ctx, prAutomation) + sha, _ := utils.HashObject(attrs) + Expect(err).NotTo(HaveOccurred()) Expect(common.SanitizeStatusConditions(prAutomation.Status)).To(Equal(common.SanitizeStatusConditions(v1alpha1.Status{ ID: lo.ToPtr(prAutomationConsoleID), - SHA: lo.ToPtr(prAutomationHash), + SHA: lo.ToPtr(sha), Conditions: []metav1.Condition{ { Type: v1alpha1.ReadyConditionType.String(), @@ -181,7 +183,6 @@ var _ = Describe("PR Automation Controller", func() { Namespace: clusterNamespace, }, }) - _, prAutomationHash, _ = generatedPrAutomation.Diff(utils.HashObject) prAutomationObjectKey = types.NamespacedName{Name: prAutomationName} ) @@ -239,10 +240,14 @@ var _ = Describe("PR Automation Controller", func() { prAutomation := &v1alpha1.PrAutomation{} err = k8sClient.Get(ctx, prAutomationObjectKey, prAutomation) + + attrs, _ := controllerReconciler.Attributes(ctx, prAutomation) + sha, _ := utils.HashObject(attrs) + Expect(err).NotTo(HaveOccurred()) Expect(common.SanitizeStatusConditions(prAutomation.Status)).To(Equal(common.SanitizeStatusConditions(v1alpha1.Status{ ID: lo.ToPtr(prAutomationConsoleID), - SHA: lo.ToPtr(prAutomationHash), + SHA: lo.ToPtr(sha), Conditions: []metav1.Condition{ { Type: v1alpha1.ReadyConditionType.String(), diff --git a/lib/console/schema/pr_automation.ex b/lib/console/schema/pr_automation.ex index 9871428536..7bc911ee38 100644 --- a/lib/console/schema/pr_automation.ex +++ b/lib/console/schema/pr_automation.ex @@ -126,7 +126,6 @@ defmodule Console.Schema.PrAutomation do |> cast_assoc(:create_bindings) |> put_new_change(:write_policy_id, &Ecto.UUID.generate/0) |> put_new_change(:create_policy_id, &Ecto.UUID.generate/0) - |> validate_required([:name, :title, :message, :connection_id]) |> unique_constraint(:name) |> foreign_key_constraint(:promotion_criteria, name: :promotion_criteria, @@ -138,6 +137,7 @@ defmodule Console.Schema.PrAutomation do |> foreign_key_constraint(:connection_id) |> foreign_key_constraint(:project_id) |> foreign_key_constraint(:catalog_id) + |> validate_required([:name, :title, :message, :connection_id]) end defp update_changeset(model, attrs) do