Skip to content

Commit

Permalink
chore(backport release-0.8): fix(promotion): always hard refresh Argo…
Browse files Browse the repository at this point in the history
…CD App (#2398)

Co-authored-by: Hidde Beydals <[email protected]>
  • Loading branch information
akuitybot and hiddeco authored Aug 6, 2024
1 parent 2b085fb commit a7f3635
Show file tree
Hide file tree
Showing 2 changed files with 160 additions and 8 deletions.
40 changes: 40 additions & 0 deletions internal/controller/promotion/argocd.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ type argoCDMechanism struct {
*argocd.ApplicationSource,
argocd.ApplicationSources,
) (argocd.OperationPhase, bool, error)
hardRefreshApplicationFn func(context.Context, *argocd.Application) error
updateApplicationSourcesFn func(
context.Context,
*argocd.Application,
Expand Down Expand Up @@ -86,6 +87,7 @@ func newArgoCDMechanism(kargoClient, argocdClient client.Client) Mechanism {
}
a.buildDesiredSourcesFn = a.buildDesiredSources
a.mustPerformUpdateFn = a.mustPerformUpdate
a.hardRefreshApplicationFn = a.hardRefreshApplication
a.updateApplicationSourcesFn = a.updateApplicationSources
a.getAuthorizedApplicationFn = a.getAuthorizedApplication
a.applyArgoCDSourceUpdateFn = a.applyArgoCDSourceUpdate
Expand Down Expand Up @@ -147,6 +149,15 @@ func (a *argoCDMechanism) Promote(
return nil, newFreight, err
}

// If we have no desired source(s), we only need to perform a hard refresh.
if desiredSource == nil && len(desiredSources) == 0 {
if err = a.hardRefreshApplicationFn(ctx, app); err != nil {
return nil, newFreight, err
}
updateResults = append(updateResults, argocd.OperationSucceeded)
continue
}

// Check if the update needs to be performed and retrieve its phase.
phase, mustUpdate, err := a.mustPerformUpdateFn(
ctx,
Expand Down Expand Up @@ -335,6 +346,32 @@ func (a *argoCDMechanism) mustPerformUpdate(
return status.Phase, false, nil
}

func (a *argoCDMechanism) hardRefreshApplication(
ctx context.Context,
app *argocd.Application,
) error {
// Create a patch for the Application.
patch := client.MergeFrom(app.DeepCopy())

// Initiate a "hard" refresh.
if app.ObjectMeta.Annotations == nil {
app.ObjectMeta.Annotations = make(map[string]string, 1)
}
app.ObjectMeta.Annotations[argocd.AnnotationKeyRefresh] = string(argocd.RefreshTypeHard)

// Patch the Application with the changes from above.
if err := a.argoCDAppPatchFn(
ctx,
app,
patch,
); err != nil {
return fmt.Errorf("error patching Argo CD Application %q: %w", app.Name, err)
}
logging.LoggerFromContext(ctx).Debug("refreshed Argo CD Application", "app", app.Name)

return nil
}

func (a *argoCDMechanism) updateApplicationSources(
ctx context.Context,
app *argocd.Application,
Expand All @@ -345,6 +382,9 @@ func (a *argoCDMechanism) updateApplicationSources(
patch := client.MergeFrom(app.DeepCopy())

// Initiate a "hard" refresh.
if app.ObjectMeta.Annotations == nil {
app.ObjectMeta.Annotations = make(map[string]string, 1)
}
app.ObjectMeta.Annotations[argocd.AnnotationKeyRefresh] = string(argocd.RefreshTypeHard)

// Update the desired source(s) in the Argo CD Application.
Expand Down
128 changes: 120 additions & 8 deletions internal/controller/promotion/argocd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ func TestNewArgoCDMechanism(t *testing.T) {
require.NotNil(t, apm.argocdClient)
require.NotNil(t, apm.buildDesiredSourcesFn)
require.NotNil(t, apm.mustPerformUpdateFn)
require.NotNil(t, apm.hardRefreshApplicationFn)
require.NotNil(t, apm.updateApplicationSourcesFn)
require.NotNil(t, apm.getAuthorizedApplicationFn)
require.NotNil(t, apm.applyArgoCDSourceUpdateFn)
Expand Down Expand Up @@ -173,7 +174,7 @@ func TestArgoCDPromote(t *testing.T) {
},
},
{
name: "error determining if update is necessary",
name: "error performing hard refresh",
promoMech: &argoCDMechanism{
argocdClient: fake.NewFakeClient(),
getAuthorizedApplicationFn: func(
Expand All @@ -193,6 +194,54 @@ func TestArgoCDPromote(t *testing.T) {
) (*argocd.ApplicationSource, argocd.ApplicationSources, error) {
return nil, nil, nil
},
hardRefreshApplicationFn: func(
context.Context,
*argocd.Application,
) error {
return errors.New("something went wrong")
},
},
stage: &kargoapi.Stage{
Spec: kargoapi.StageSpec{
PromotionMechanisms: &kargoapi.PromotionMechanisms{
ArgoCDAppUpdates: []kargoapi.ArgoCDAppUpdate{
{},
},
},
},
},
assertions: func(
t *testing.T,
_ *kargoapi.PromotionStatus,
newFreightIn []kargoapi.FreightReference,
newFreightOut []kargoapi.FreightReference,
err error,
) {
require.ErrorContains(t, err, "something went wrong")
require.Equal(t, newFreightIn, newFreightOut)
},
},
{
name: "error determining if update is necessary",
promoMech: &argoCDMechanism{
argocdClient: fake.NewFakeClient(),
getAuthorizedApplicationFn: func(
context.Context,
string,
string,
metav1.ObjectMeta,
) (*argocd.Application, error) {
return &argocd.Application{}, nil
},
buildDesiredSourcesFn: func(
context.Context,
*kargoapi.Stage,
*kargoapi.ArgoCDAppUpdate,
*argocd.Application,
[]kargoapi.FreightReference,
) (*argocd.ApplicationSource, argocd.ApplicationSources, error) {
return &argocd.ApplicationSource{}, nil, nil
},
mustPerformUpdateFn: func(
context.Context,
*kargoapi.Stage,
Expand Down Expand Up @@ -244,7 +293,7 @@ func TestArgoCDPromote(t *testing.T) {
*argocd.Application,
[]kargoapi.FreightReference,
) (*argocd.ApplicationSource, argocd.ApplicationSources, error) {
return nil, nil, nil
return &argocd.ApplicationSource{}, nil, nil
},
mustPerformUpdateFn: func(
context.Context,
Expand Down Expand Up @@ -305,7 +354,7 @@ func TestArgoCDPromote(t *testing.T) {
*argocd.Application,
[]kargoapi.FreightReference,
) (*argocd.ApplicationSource, argocd.ApplicationSources, error) {
return nil, nil, nil
return &argocd.ApplicationSource{}, nil, nil
},
mustPerformUpdateFn: func(
context.Context,
Expand Down Expand Up @@ -359,7 +408,7 @@ func TestArgoCDPromote(t *testing.T) {
*argocd.Application,
[]kargoapi.FreightReference,
) (*argocd.ApplicationSource, argocd.ApplicationSources, error) {
return nil, nil, nil
return &argocd.ApplicationSource{}, nil, nil
},
mustPerformUpdateFn: func(
context.Context,
Expand Down Expand Up @@ -413,7 +462,7 @@ func TestArgoCDPromote(t *testing.T) {
*argocd.Application,
[]kargoapi.FreightReference,
) (*argocd.ApplicationSource, argocd.ApplicationSources, error) {
return nil, nil, nil
return &argocd.ApplicationSource{}, nil, nil
},
mustPerformUpdateFn: func(
context.Context,
Expand Down Expand Up @@ -479,7 +528,7 @@ func TestArgoCDPromote(t *testing.T) {
*argocd.Application,
[]kargoapi.FreightReference,
) (*argocd.ApplicationSource, argocd.ApplicationSources, error) {
return nil, nil, nil
return &argocd.ApplicationSource{}, nil, nil
},
mustPerformUpdateFn: func() func(
context.Context,
Expand Down Expand Up @@ -557,7 +606,7 @@ func TestArgoCDPromote(t *testing.T) {
*argocd.Application,
[]kargoapi.FreightReference,
) (*argocd.ApplicationSource, argocd.ApplicationSources, error) {
return nil, nil, nil
return &argocd.ApplicationSource{}, nil, nil
},
mustPerformUpdateFn: func(
context.Context,
Expand Down Expand Up @@ -610,7 +659,7 @@ func TestArgoCDPromote(t *testing.T) {
*argocd.Application,
[]kargoapi.FreightReference,
) (*argocd.ApplicationSource, argocd.ApplicationSources, error) {
return nil, nil, nil
return &argocd.ApplicationSource{}, nil, nil
},
mustPerformUpdateFn: func(
context.Context,
Expand Down Expand Up @@ -1178,6 +1227,69 @@ func TestArgoCDMustPerformUpdate(t *testing.T) {
}
}

func TestArgoCDHardRefreshApplication(t *testing.T) {
testCases := []struct {
name string
promoMech *argoCDMechanism
app *argocd.Application
desiredSource *argocd.ApplicationSource
desiredSources argocd.ApplicationSources
assertions func(*testing.T, error)
}{
{
name: "error patching Application",
promoMech: &argoCDMechanism{
argoCDAppPatchFn: func(
context.Context,
client.Object,
client.Patch,
...client.PatchOption,
) error {
return errors.New("something went wrong")
},
},
app: &argocd.Application{},
assertions: func(t *testing.T, err error) {
require.ErrorContains(t, err, "error patching Argo CD Application")
require.ErrorContains(t, err, "something went wrong")
},
},
{
name: "success",
promoMech: &argoCDMechanism{
argoCDAppPatchFn: func(
context.Context,
client.Object,
client.Patch,
...client.PatchOption,
) error {
return nil
},
},
app: &argocd.Application{
ObjectMeta: metav1.ObjectMeta{
Name: "fake-name",
Namespace: "fake-namespace",
},
},
assertions: func(t *testing.T, err error) {
require.NoError(t, err)
},
},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
testCase.assertions(
t,
testCase.promoMech.hardRefreshApplication(
context.Background(),
testCase.app,
),
)
})
}
}

func TestArgoCDUpdateApplicationSources(t *testing.T) {
testCases := []struct {
name string
Expand Down

0 comments on commit a7f3635

Please sign in to comment.