diff --git a/applicationset/controllers/applicationset_controller.go b/applicationset/controllers/applicationset_controller.go index b477bd1d41007..044262615cf32 100644 --- a/applicationset/controllers/applicationset_controller.go +++ b/applicationset/controllers/applicationset_controller.go @@ -28,9 +28,11 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes" + k8scache "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -85,6 +87,7 @@ type ApplicationSetReconciler struct { SCMRootCAPath string GlobalPreservedAnnotations []string GlobalPreservedLabels []string + Cache cache.Cache } // +kubebuilder:rbac:groups=argoproj.io,resources=applicationsets,verbs=get;list;watch;create;update;patch;delete @@ -582,6 +585,25 @@ func (r *ApplicationSetReconciler) SetupWithManager(mgr ctrl.Manager, enableProg Complete(r) } +func (r *ApplicationSetReconciler) updateCache(ctx context.Context, obj client.Object, logger *log.Entry) { + informer, err := r.Cache.GetInformer(ctx, obj) + if err != nil { + logger.Errorf("failed to get informer: %v", err) + return + } + // The controller runtime abstract away informers creation + // so unfortunately could not find any other way to access informer store. + k8sInformer, ok := informer.(k8scache.SharedInformer) + if !ok { + logger.Error("informer is not a kubernetes informer") + return + } + if err := k8sInformer.GetStore().Update(obj); err != nil { + logger.Errorf("failed to update cache: %v", err) + return + } +} + // createOrUpdateInCluster will create / update application resources in the cluster. // - For new applications, it will call create // - For existing application, it will call update @@ -671,7 +693,7 @@ func (r *ApplicationSetReconciler) createOrUpdateInCluster(ctx context.Context, } continue } - + r.updateCache(ctx, found, appLog) r.Recorder.Eventf(&applicationSet, corev1.EventTypeNormal, fmt.Sprint(action), "%s Application %q", action, generatedApp.Name) appLog.Logf(log.InfoLevel, "%s Application", action) } @@ -829,15 +851,17 @@ func (r *ApplicationSetReconciler) removeFinalizerOnInvalidDestination(ctx conte // If the finalizer length changed (due to filtering out an Argo finalizer), update the finalizer list on the app if len(newFinalizers) != len(app.Finalizers) { - app.Finalizers = newFinalizers + updated := app.DeepCopy() + updated.Finalizers = newFinalizers + if err := r.Client.Patch(ctx, updated, client.MergeFrom(app)); err != nil { + return fmt.Errorf("error updating finalizers: %w", err) + } + r.updateCache(ctx, updated, appLog) + // Application must have updated list of finalizers + updated.DeepCopyInto(app) r.Recorder.Eventf(&applicationSet, corev1.EventTypeNormal, "Updated", "Updated Application %q finalizer before deletion, because application has an invalid destination", app.Name) appLog.Log(log.InfoLevel, "Updating application finalizer before deletion, because application has an invalid destination") - - err := r.Client.Update(ctx, app, &client.UpdateOptions{}) - if err != nil { - return fmt.Errorf("error updating finalizers: %w", err) - } } } diff --git a/applicationset/controllers/applicationset_controller_test.go b/applicationset/controllers/applicationset_controller_test.go index 91e76301e6440..1532fca14f41f 100644 --- a/applicationset/controllers/applicationset_controller_test.go +++ b/applicationset/controllers/applicationset_controller_test.go @@ -19,8 +19,10 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" kubefake "k8s.io/client-go/kubernetes/fake" + k8scache "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/cache" crtclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -40,6 +42,34 @@ import ( "github.com/argoproj/argo-cd/v2/pkg/apis/application" ) +type fakeStore struct { + k8scache.Store +} + +func (f *fakeStore) Update(obj interface{}) error { + return nil +} + +type fakeInformer struct { + k8scache.SharedInformer +} + +func (f *fakeInformer) AddIndexers(indexers k8scache.Indexers) error { + return nil +} + +func (f *fakeInformer) GetStore() k8scache.Store { + return &fakeStore{} +} + +type fakeCache struct { + cache.Cache +} + +func (f *fakeCache) GetInformer(ctx context.Context, obj crtclient.Object) (cache.Informer, error) { + return &fakeInformer{}, nil +} + type generatorMock struct { mock.Mock } @@ -185,6 +215,7 @@ func TestExtractApplications(t *testing.T) { }, Renderer: &rendererMock, KubeClientset: kubefake.NewSimpleClientset(), + Cache: &fakeCache{}, } got, reason, err := r.generateApplications(v1alpha1.ApplicationSet{ @@ -967,6 +998,7 @@ func TestCreateOrUpdateInCluster(t *testing.T) { Client: client, Scheme: scheme, Recorder: record.NewFakeRecorder(len(initObjs) + len(c.expected)), + Cache: &fakeCache{}, } err = r.createOrUpdateInCluster(context.TODO(), c.appSet, c.desiredApps) @@ -1080,6 +1112,7 @@ func TestRemoveFinalizerOnInvalidDestination_FinalizerTypes(t *testing.T) { Scheme: scheme, Recorder: record.NewFakeRecorder(10), KubeClientset: kubeclientset, + Cache: &fakeCache{}, } //settingsMgr := settings.NewSettingsManager(context.TODO(), kubeclientset, "namespace") //argoDB := db.NewDB("namespace", settingsMgr, r.KubeClientset) @@ -1241,6 +1274,7 @@ func TestRemoveFinalizerOnInvalidDestination_DestinationTypes(t *testing.T) { Scheme: scheme, Recorder: record.NewFakeRecorder(10), KubeClientset: kubeclientset, + Cache: &fakeCache{}, } // settingsMgr := settings.NewSettingsManager(context.TODO(), kubeclientset, "argocd") // argoDB := db.NewDB("argocd", settingsMgr, r.KubeClientset) @@ -1452,6 +1486,7 @@ func TestCreateApplications(t *testing.T) { Client: client, Scheme: scheme, Recorder: record.NewFakeRecorder(len(initObjs) + len(c.expected)), + Cache: &fakeCache{}, } err = r.createInCluster(context.TODO(), c.appSet, c.apps) @@ -1659,6 +1694,7 @@ func TestGetMinRequeueAfter(t *testing.T) { Client: client, Scheme: scheme, Recorder: record.NewFakeRecorder(0), + Cache: &fakeCache{}, Generators: map[string]generators.Generator{ "List": &generatorMock10, "Git": &generatorMock1, @@ -1872,6 +1908,7 @@ func TestValidateGeneratedApplications(t *testing.T) { Client: client, Scheme: scheme, Recorder: record.NewFakeRecorder(1), + Cache: &fakeCache{}, Generators: map[string]generators.Generator{}, ArgoDB: &argoDBMock, ArgoCDNamespace: "namespace", @@ -1976,6 +2013,7 @@ func TestReconcilerValidationErrorBehaviour(t *testing.T) { Scheme: scheme, Renderer: &utils.Render{}, Recorder: record.NewFakeRecorder(1), + Cache: &fakeCache{}, Generators: map[string]generators.Generator{ "List": generators.NewListGenerator(), }, @@ -2052,6 +2090,7 @@ func TestSetApplicationSetStatusCondition(t *testing.T) { Scheme: scheme, Renderer: &utils.Render{}, Recorder: record.NewFakeRecorder(1), + Cache: &fakeCache{}, Generators: map[string]generators.Generator{ "List": generators.NewListGenerator(), }, @@ -2126,6 +2165,7 @@ func applicationsUpdateSyncPolicyTest(t *testing.T, applicationsSyncPolicy v1alp Scheme: scheme, Renderer: &utils.Render{}, Recorder: record.NewFakeRecorder(recordBuffer), + Cache: &fakeCache{}, Generators: map[string]generators.Generator{ "List": generators.NewListGenerator(), }, @@ -2295,6 +2335,7 @@ func applicationsDeleteSyncPolicyTest(t *testing.T, applicationsSyncPolicy v1alp Scheme: scheme, Renderer: &utils.Render{}, Recorder: record.NewFakeRecorder(recordBuffer), + Cache: &fakeCache{}, Generators: map[string]generators.Generator{ "List": generators.NewListGenerator(), }, @@ -2475,6 +2516,7 @@ func TestGenerateAppsUsingPullRequestGenerator(t *testing.T) { Client: client, Scheme: scheme, Recorder: record.NewFakeRecorder(1), + Cache: &fakeCache{}, Generators: map[string]generators.Generator{ "PullRequest": &generatorMock, }, @@ -2599,6 +2641,7 @@ func TestPolicies(t *testing.T) { Scheme: scheme, Renderer: &utils.Render{}, Recorder: record.NewFakeRecorder(10), + Cache: &fakeCache{}, Generators: map[string]generators.Generator{ "List": generators.NewListGenerator(), }, @@ -2761,6 +2804,7 @@ func TestSetApplicationSetApplicationStatus(t *testing.T) { Scheme: scheme, Renderer: &utils.Render{}, Recorder: record.NewFakeRecorder(1), + Cache: &fakeCache{}, Generators: map[string]generators.Generator{ "List": generators.NewListGenerator(), }, @@ -3525,6 +3569,7 @@ func TestBuildAppDependencyList(t *testing.T) { Client: client, Scheme: scheme, Recorder: record.NewFakeRecorder(1), + Cache: &fakeCache{}, Generators: map[string]generators.Generator{}, ArgoDB: &argoDBMock, ArgoAppClientset: appclientset.NewSimpleClientset(argoObjs...), @@ -4118,6 +4163,7 @@ func TestBuildAppSyncMap(t *testing.T) { Client: client, Scheme: scheme, Recorder: record.NewFakeRecorder(1), + Cache: &fakeCache{}, Generators: map[string]generators.Generator{}, ArgoDB: &argoDBMock, ArgoAppClientset: appclientset.NewSimpleClientset(argoObjs...), @@ -4777,6 +4823,7 @@ func TestUpdateApplicationSetApplicationStatus(t *testing.T) { Client: client, Scheme: scheme, Recorder: record.NewFakeRecorder(1), + Cache: &fakeCache{}, Generators: map[string]generators.Generator{}, ArgoDB: &argoDBMock, ArgoAppClientset: appclientset.NewSimpleClientset(argoObjs...), @@ -5530,6 +5577,7 @@ func TestUpdateApplicationSetApplicationStatusProgress(t *testing.T) { Client: client, Scheme: scheme, Recorder: record.NewFakeRecorder(1), + Cache: &fakeCache{}, Generators: map[string]generators.Generator{}, ArgoDB: &argoDBMock, ArgoAppClientset: appclientset.NewSimpleClientset(argoObjs...), diff --git a/applicationset/utils/createOrUpdate.go b/applicationset/utils/createOrUpdate.go index a9775eb7994c5..096be5a9a97d3 100644 --- a/applicationset/utils/createOrUpdate.go +++ b/applicationset/utils/createOrUpdate.go @@ -45,7 +45,11 @@ func CreateOrUpdate(ctx context.Context, c client.Client, obj client.Object, f c return controllerutil.OperationResultCreated, nil } - existing := obj.DeepCopyObject() + existingObj := obj.DeepCopyObject() + existing, ok := existingObj.(client.Object) + if !ok { + panic(fmt.Errorf("existing object is not a client.Object")) + } if err := mutate(f, key, obj); err != nil { return controllerutil.OperationResultNone, err } @@ -79,7 +83,7 @@ func CreateOrUpdate(ctx context.Context, c client.Client, obj client.Object, f c return controllerutil.OperationResultNone, nil } - if err := c.Update(ctx, obj); err != nil { + if err := c.Patch(ctx, obj, client.MergeFrom(existing)); err != nil { return controllerutil.OperationResultNone, err } return controllerutil.OperationResultUpdated, nil diff --git a/cmd/argocd-applicationset-controller/commands/applicationset_controller.go b/cmd/argocd-applicationset-controller/commands/applicationset_controller.go index ea23f3a8a1588..9939cf6a4972c 100644 --- a/cmd/argocd-applicationset-controller/commands/applicationset_controller.go +++ b/cmd/argocd-applicationset-controller/commands/applicationset_controller.go @@ -218,6 +218,7 @@ func NewCommand() *cobra.Command { SCMRootCAPath: scmRootCAPath, GlobalPreservedAnnotations: globalPreservedAnnotations, GlobalPreservedLabels: globalPreservedLabels, + Cache: mgr.GetCache(), }).SetupWithManager(mgr, enableProgressiveSyncs, maxConcurrentReconciliations); err != nil { log.Error(err, "unable to create controller", "controller", "ApplicationSet") os.Exit(1)