From 94d8f065db192a4227a0eb358689088dc2f6cfcb Mon Sep 17 00:00:00 2001 From: Maxim Ivanov Date: Tue, 17 Sep 2024 18:37:10 -0400 Subject: [PATCH] fix(promotions): multiple stages can sync same argocd app Signed-off-by: Maxim Ivanov --- internal/controller/promotion/argocd.go | 51 +++++++++++++++----- internal/controller/promotion/argocd_test.go | 39 +++++++++++---- 2 files changed, 70 insertions(+), 20 deletions(-) diff --git a/internal/controller/promotion/argocd.go b/internal/controller/promotion/argocd.go index 43fcc56e3..38e8e5ded 100644 --- a/internal/controller/promotion/argocd.go +++ b/internal/controller/promotion/argocd.go @@ -57,6 +57,7 @@ type argoCDMechanism struct { desiredSource *argocd.ApplicationSource, desiredSources argocd.ApplicationSources, freightColID string, + stage *kargoapi.Stage, ) error getAuthorizedApplicationFn func( ctx context.Context, @@ -201,6 +202,7 @@ func (a *argoCDMechanism) Promote( desiredSource, desiredSources, promo.Status.FreightCollection.ID, + stage, ); err != nil { return err } @@ -301,7 +303,12 @@ func (a *argoCDMechanism) mustPerformUpdate( // current freight collection. i.e. Not related to the current promotion. var correctFreightColIDFound bool for _, info := range status.Operation.Info { - if info.Name == freightCollectionInfoKey { + if info.Name == fmt.Sprintf( + "%s/%s/%s", + freightCollectionInfoKey, + stage.ObjectMeta.Namespace, + stage.ObjectMeta.Name, + ) { correctFreightColIDFound = info.Value == freightCol.ID break } @@ -374,6 +381,7 @@ func (a *argoCDMechanism) syncApplication( desiredSource *argocd.ApplicationSource, desiredSources argocd.ApplicationSources, freightColID string, + stage *kargoapi.Stage, ) error { // Initiate a "hard" refresh. if app.ObjectMeta.Annotations == nil { @@ -385,22 +393,43 @@ func (a *argoCDMechanism) syncApplication( app.Spec.Source = desiredSource.DeepCopy() app.Spec.Sources = desiredSources.DeepCopy() + kargoKey := fmt.Sprintf("%s/%s/%s", freightCollectionInfoKey, stage.ObjectMeta.Namespace, stage.ObjectMeta.Name) + reason := "Promotion triggered a sync of this Application resource." + + newInfo := make(map[string]string) + defaultInfo := map[string]string{ + "Reason": reason, + kargoKey: freightColID, + } + + if app.Status.OperationState != nil && app.Status.OperationState.Operation.Info != nil { + for _, info := range app.Status.OperationState.Operation.Info { + newInfo[info.Name] = info.Value + } + if newInfo["Reason"] == reason { + newInfo[kargoKey] = freightColID + } else { + newInfo = defaultInfo + } + } else { + newInfo = defaultInfo + } + + infoArray := []*argocd.Info{} + for k, v := range newInfo { + infoArray = append(infoArray, &argocd.Info{ + Name: k, + Value: v, + }) + } + // Initiate a new operation. app.Operation = &argocd.Operation{ InitiatedBy: argocd.OperationInitiator{ Username: applicationOperationInitiator, Automated: true, }, - Info: []*argocd.Info{ - { - Name: "Reason", - Value: "Promotion triggered a sync of this Application resource.", - }, - { - Name: freightCollectionInfoKey, - Value: freightColID, - }, - }, + Info: infoArray, Sync: &argocd.SyncOperation{ Revisions: []string{}, }, diff --git a/internal/controller/promotion/argocd_test.go b/internal/controller/promotion/argocd_test.go index c8ea668b8..687b25ab6 100644 --- a/internal/controller/promotion/argocd_test.go +++ b/internal/controller/promotion/argocd_test.go @@ -271,6 +271,7 @@ func TestArgoCDPromote(t *testing.T) { *argocd.ApplicationSource, argocd.ApplicationSources, string, + *kargoapi.Stage, ) error { return nil }, @@ -452,6 +453,7 @@ func TestArgoCDPromote(t *testing.T) { *argocd.ApplicationSource, argocd.ApplicationSources, string, + *kargoapi.Stage, ) error { return errors.New("something went wrong") }, @@ -538,6 +540,7 @@ func TestArgoCDPromote(t *testing.T) { *argocd.ApplicationSource, argocd.ApplicationSources, string, + *kargoapi.Stage, ) error { return nil }, @@ -990,7 +993,7 @@ func TestArgoCDMustPerformUpdate(t *testing.T) { Username: applicationOperationInitiator, }, Info: []*argocd.Info{{ - Name: freightCollectionInfoKey, + Name: fmt.Sprintf("%s/fake-namespace/fake-name", freightCollectionInfoKey), Value: "wrong-freight-collection", }}, }, @@ -1013,7 +1016,7 @@ func TestArgoCDMustPerformUpdate(t *testing.T) { Username: applicationOperationInitiator, }, Info: []*argocd.Info{{ - Name: freightCollectionInfoKey, + Name: fmt.Sprintf("%s/fake-namespace/fake-name", freightCollectionInfoKey), Value: "wrong-freight-collection", }}, }, @@ -1035,7 +1038,7 @@ func TestArgoCDMustPerformUpdate(t *testing.T) { Username: applicationOperationInitiator, }, Info: []*argocd.Info{{ - Name: freightCollectionInfoKey, + Name: fmt.Sprintf("%s/fake-namespace/fake-name", freightCollectionInfoKey), Value: testFreightCollectionID, }}, }, @@ -1057,7 +1060,7 @@ func TestArgoCDMustPerformUpdate(t *testing.T) { Username: applicationOperationInitiator, }, Info: []*argocd.Info{{ - Name: freightCollectionInfoKey, + Name: fmt.Sprintf("%s/fake-namespace/fake-name", freightCollectionInfoKey), Value: testFreightCollectionID, }}, }, @@ -1083,7 +1086,7 @@ func TestArgoCDMustPerformUpdate(t *testing.T) { Username: applicationOperationInitiator, }, Info: []*argocd.Info{{ - Name: freightCollectionInfoKey, + Name: fmt.Sprintf("%s/fake-namespace/fake-name", freightCollectionInfoKey), Value: testFreightCollectionID, }}, }, @@ -1116,7 +1119,7 @@ func TestArgoCDMustPerformUpdate(t *testing.T) { Username: applicationOperationInitiator, }, Info: []*argocd.Info{{ - Name: freightCollectionInfoKey, + Name: fmt.Sprintf("%s/fake-namespace/fake-name", freightCollectionInfoKey), Value: testFreightCollectionID, }}, }, @@ -1153,7 +1156,7 @@ func TestArgoCDMustPerformUpdate(t *testing.T) { Username: applicationOperationInitiator, }, Info: []*argocd.Info{{ - Name: freightCollectionInfoKey, + Name: fmt.Sprintf("%s/fake-namespace/fake-name", freightCollectionInfoKey), Value: testFreightCollectionID, }}, }, @@ -1189,7 +1192,7 @@ func TestArgoCDMustPerformUpdate(t *testing.T) { Username: applicationOperationInitiator, }, Info: []*argocd.Info{{ - Name: freightCollectionInfoKey, + Name: fmt.Sprintf("%s/fake-namespace/fake-name", freightCollectionInfoKey), Value: testFreightCollectionID, }}, }, @@ -1228,7 +1231,7 @@ func TestArgoCDMustPerformUpdate(t *testing.T) { Username: applicationOperationInitiator, }, Info: []*argocd.Info{{ - Name: freightCollectionInfoKey, + Name: fmt.Sprintf("%s/fake-namespace/fake-name", freightCollectionInfoKey), Value: testFreightCollectionID, }}, }, @@ -1276,6 +1279,10 @@ func TestArgoCDMustPerformUpdate(t *testing.T) { require.True(t, ok) stage := &kargoapi.Stage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-name", + Namespace: "fake-namespace", + }, Spec: kargoapi.StageSpec{ PromotionMechanisms: &kargoapi.PromotionMechanisms{ ArgoCDAppUpdates: []kargoapi.ArgoCDAppUpdate{{ @@ -1317,6 +1324,7 @@ func TestArgoCDSyncApplication(t *testing.T) { desiredSource *argocd.ApplicationSource desiredSources argocd.ApplicationSources assertions func(*testing.T, error) + stage *kargoapi.Stage }{ { name: "error patching Application", @@ -1342,6 +1350,12 @@ func TestArgoCDSyncApplication(t *testing.T) { require.ErrorContains(t, err, "error patching Argo CD Application") require.ErrorContains(t, err, "something went wrong") }, + stage: &kargoapi.Stage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-name", + Namespace: "fake-namespace", + }, + }, }, { name: "success", @@ -1367,6 +1381,12 @@ func TestArgoCDSyncApplication(t *testing.T) { assertions: func(t *testing.T, err error) { require.NoError(t, err) }, + stage: &kargoapi.Stage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-name", + Namespace: "fake-namespace", + }, + }, }, } for _, testCase := range testCases { @@ -1379,6 +1399,7 @@ func TestArgoCDSyncApplication(t *testing.T) { testCase.desiredSource, testCase.desiredSources, "fake-freight-collection-id", + testCase.stage, ), ) })