From 86369ca71d73901a3ae88c4e5e36a19de75ec618 Mon Sep 17 00:00:00 2001 From: similark <85114352+similark@users.noreply.github.com> Date: Wed, 13 Mar 2024 03:20:28 +0200 Subject: [PATCH] fix(appset): keep reconciling even when params error occurred (#17062) * fix(appset): keep reconcile even when params error occurred Signed-off-by: Or Koren * requeue on generator rendering error Signed-off-by: Or Koren * test ignoring partial rendering errors Signed-off-by: Or Koren * e2e test create app with param error Signed-off-by: Or Koren --------- Signed-off-by: Or Koren Co-authored-by: Blake Pettersson --- .../controllers/applicationset_controller.go | 12 ++- .../applicationset_controller_test.go | 85 +++++++++++++++++ test/e2e/applicationset_test.go | 94 +++++++++++++++++++ 3 files changed, 186 insertions(+), 5 deletions(-) diff --git a/applicationset/controllers/applicationset_controller.go b/applicationset/controllers/applicationset_controller.go index 4f5ac66fc016d..e1275e75d3ba2 100644 --- a/applicationset/controllers/applicationset_controller.go +++ b/applicationset/controllers/applicationset_controller.go @@ -124,18 +124,20 @@ func (r *ApplicationSetReconciler) Reconcile(ctx context.Context, req ctrl.Reque // Log a warning if there are unrecognized generators _ = utils.CheckInvalidGenerators(&applicationSetInfo) // desiredApplications is the main list of all expected Applications from all generators in this appset. - desiredApplications, applicationSetReason, err := r.generateApplications(logCtx, applicationSetInfo) - if err != nil { + desiredApplications, applicationSetReason, generatorsErr := r.generateApplications(logCtx, applicationSetInfo) + if generatorsErr != nil { _ = r.setApplicationSetStatusCondition(ctx, &applicationSetInfo, argov1alpha1.ApplicationSetCondition{ Type: argov1alpha1.ApplicationSetConditionErrorOccurred, - Message: err.Error(), + Message: generatorsErr.Error(), Reason: string(applicationSetReason), Status: argov1alpha1.ApplicationSetConditionStatusTrue, }, parametersGenerated, ) - return ctrl.Result{}, err + if len(desiredApplications) < 1 { + return ctrl.Result{}, generatorsErr + } } parametersGenerated = true @@ -309,7 +311,7 @@ func (r *ApplicationSetReconciler) Reconcile(ctx context.Context, req ctrl.Reque requeueAfter := r.getMinRequeueAfter(&applicationSetInfo) - if len(validateErrors) == 0 { + if len(validateErrors) == 0 && generatorsErr == nil { if err := r.setApplicationSetStatusCondition(ctx, &applicationSetInfo, argov1alpha1.ApplicationSetCondition{ diff --git a/applicationset/controllers/applicationset_controller_test.go b/applicationset/controllers/applicationset_controller_test.go index 81fbad95ac50b..c3c5f3845bea5 100644 --- a/applicationset/controllers/applicationset_controller_test.go +++ b/applicationset/controllers/applicationset_controller_test.go @@ -2423,6 +2423,91 @@ func TestReconcilerValidationProjectErrorBehaviour(t *testing.T) { assert.Error(t, err) } +func TestReconcilerCreateAppsRecoveringRenderError(t *testing.T) { + + scheme := runtime.NewScheme() + err := v1alpha1.AddToScheme(scheme) + assert.Nil(t, err) + err = v1alpha1.AddToScheme(scheme) + assert.Nil(t, err) + + project := v1alpha1.AppProject{ + ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "argocd"}, + } + appSet := v1alpha1.ApplicationSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + Namespace: "argocd", + }, + Spec: v1alpha1.ApplicationSetSpec{ + GoTemplate: true, + Generators: []v1alpha1.ApplicationSetGenerator{ + { + List: &v1alpha1.ListGenerator{ + Elements: []apiextensionsv1.JSON{{ + Raw: []byte(`{"name": "very-good-app"}`), + }, { + Raw: []byte(`{"name": "bad-app"}`), + }}, + }, + }, + }, + Template: v1alpha1.ApplicationSetTemplate{ + ApplicationSetTemplateMeta: v1alpha1.ApplicationSetTemplateMeta{ + Name: "{{ index (splitList \"-\" .name ) 2 }}", + Namespace: "argocd", + }, + Spec: v1alpha1.ApplicationSpec{ + Source: &v1alpha1.ApplicationSource{RepoURL: "https://github.com/argoproj/argocd-example-apps", Path: "guestbook"}, + Project: "default", + Destination: v1alpha1.ApplicationDestination{Server: "https://kubernetes.default.svc"}, + }, + }, + }, + } + + kubeclientset := kubefake.NewSimpleClientset() + argoDBMock := dbmocks.ArgoDB{} + argoObjs := []runtime.Object{&project} + + client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&appSet).WithIndex(&v1alpha1.Application{}, ".metadata.controller", appControllerIndexer).Build() + + r := ApplicationSetReconciler{ + Client: client, + Scheme: scheme, + Renderer: &utils.Render{}, + Recorder: record.NewFakeRecorder(1), + Cache: &fakeCache{}, + Generators: map[string]generators.Generator{ + "List": generators.NewListGenerator(), + }, + ArgoDB: &argoDBMock, + ArgoAppClientset: appclientset.NewSimpleClientset(argoObjs...), + KubeClientset: kubeclientset, + Policy: v1alpha1.ApplicationsSyncPolicySync, + ArgoCDNamespace: "argocd", + } + + req := ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: "argocd", + Name: "name", + }, + } + + // Verify that on generatorsError, no error is returned, but the object is requeued + res, err := r.Reconcile(context.Background(), req) + assert.Nil(t, err) + assert.True(t, res.RequeueAfter == ReconcileRequeueOnValidationError) + + var app v1alpha1.Application + + // make sure good app got created + err = r.Client.Get(context.TODO(), crtclient.ObjectKey{Namespace: "argocd", Name: "app"}, &app) + assert.NoError(t, err) + assert.Equal(t, app.Name, "app") +} + func TestSetApplicationSetStatusCondition(t *testing.T) { scheme := runtime.NewScheme() err := v1alpha1.AddToScheme(scheme) diff --git a/test/e2e/applicationset_test.go b/test/e2e/applicationset_test.go index 5b9b8190c5437..0d4d8ea3498f5 100644 --- a/test/e2e/applicationset_test.go +++ b/test/e2e/applicationset_test.go @@ -523,6 +523,100 @@ func TestSimpleListGeneratorGoTemplate(t *testing.T) { } +func TestCreateApplicationDespiteParamsError(t *testing.T) { + expectedErrorMessage := `failed to execute go template {{.cluster}}-guestbook: template: :1:2: executing "" at <.cluster>: map has no entry for key "cluster"` + expectedConditionsParamsError := []v1alpha1.ApplicationSetCondition{ + { + Type: v1alpha1.ApplicationSetConditionErrorOccurred, + Status: v1alpha1.ApplicationSetConditionStatusTrue, + Message: expectedErrorMessage, + Reason: v1alpha1.ApplicationSetReasonRenderTemplateParamsError, + }, + { + Type: v1alpha1.ApplicationSetConditionParametersGenerated, + Status: v1alpha1.ApplicationSetConditionStatusFalse, + Message: expectedErrorMessage, + Reason: v1alpha1.ApplicationSetReasonErrorOccurred, + }, + { + Type: v1alpha1.ApplicationSetConditionResourcesUpToDate, + Status: v1alpha1.ApplicationSetConditionStatusFalse, + Message: expectedErrorMessage, + Reason: v1alpha1.ApplicationSetReasonRenderTemplateParamsError, + }, + } + expectedApp := argov1alpha1.Application{ + TypeMeta: metav1.TypeMeta{ + Kind: application.ApplicationKind, + APIVersion: "argoproj.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster-guestbook", + Namespace: fixture.TestNamespace(), + Finalizers: []string{"resources-finalizer.argocd.argoproj.io"}, + }, + Spec: argov1alpha1.ApplicationSpec{ + Project: "default", + Source: &argov1alpha1.ApplicationSource{ + RepoURL: "https://github.com/argoproj/argocd-example-apps.git", + TargetRevision: "HEAD", + Path: "guestbook", + }, + Destination: argov1alpha1.ApplicationDestination{ + Server: "https://kubernetes.default.svc", + Namespace: "guestbook", + }, + }, + } + + Given(t). + // Create a ListGenerator-based ApplicationSet + When().Create(v1alpha1.ApplicationSet{ObjectMeta: metav1.ObjectMeta{ + Name: "simple-list-generator", + }, + Spec: v1alpha1.ApplicationSetSpec{ + GoTemplate: true, + GoTemplateOptions: []string{"missingkey=error"}, + Template: v1alpha1.ApplicationSetTemplate{ + ApplicationSetTemplateMeta: v1alpha1.ApplicationSetTemplateMeta{Name: "{{.cluster}}-guestbook"}, + Spec: argov1alpha1.ApplicationSpec{ + Project: "default", + Source: &argov1alpha1.ApplicationSource{ + RepoURL: "https://github.com/argoproj/argocd-example-apps.git", + TargetRevision: "HEAD", + Path: "guestbook", + }, + Destination: argov1alpha1.ApplicationDestination{ + Server: "{{.url}}", + Namespace: "guestbook", + }, + }, + }, + Generators: []v1alpha1.ApplicationSetGenerator{ + { + List: &v1alpha1.ListGenerator{ + Elements: []apiextensionsv1.JSON{ + { + Raw: []byte(`{"cluster": "my-cluster","url": "https://kubernetes.default.svc"}`), + }, + { + Raw: []byte(`{"invalidCluster": "invalid-cluster","url": "https://kubernetes.default.svc"}`), + }}, + }, + }, + }, + }, + }).Then().Expect(ApplicationsExist([]argov1alpha1.Application{expectedApp})). + + // verify the ApplicationSet status conditions were set correctly + Expect(ApplicationSetHasConditions("simple-list-generator", expectedConditionsParamsError)). + + // Delete the ApplicationSet, and verify it deletes the Applications + When(). + Delete().Then().Expect(ApplicationsDoNotExist([]argov1alpha1.Application{expectedApp})) + +} + func TestRenderHelmValuesObject(t *testing.T) { expectedApp := argov1alpha1.Application{