fix(notification-controller): read appprojects from informer cache
Project: argoproj/argo-cd | Language: Go | Concepts: caching, rate-limiting, refactoring | Difficulty: mid
Link: https://github.com/argoproj/argo-cd/pull/28815
The problem
The notification controller in Argo CD was fetching the AppProject resource directly from the Kubernetes API server for every application on every reconcile. This caused significant delays in notification delivery, especially in large installations, due to API rate limiting and serial processing. Additionally, it looked up the AppProject in the application's namespace instead of the control-plane namespace, resulting in unnecessary 404 errors and log spam for apps living outside the default namespace.
Why it's non-trivial
A naive fix might simply cache the result or batch requests, but would miss:
- The need to use the informer cache (which is already kept up to date) instead of live API calls for efficiency.
- The importance of looking up
AppProjectin the correct (control-plane) namespace, not the application's namespace. - Ensuring the code still works for CLI invocations where no informer is available.
- Properly handling rate limits for write operations as well as reads.
The approach
1. Use the informer cache for AppProject lookups
Instead of a live API GET, the controller now serves AppProject lookups from its in-memory informer cache. This is wired by passing the informer to the service:
+ if argocdService != nil {
+ argocdService.SetAppProjectInformer(appProjInformer)
+ }
This change ensures that all lookups during reconciliation are served from memory, eliminating API round-trips.
2. Always key AppProject lookups on the controller namespace
Previously, the lookup used the application's namespace, which was incorrect. Now, it always uses the controller's (argocd) namespace:
-func getAppProj(app *unstructured.Unstructured, appProjInformer cache.SharedIndexInformer) *unstructured.Unstructured {
- projName, ok, err := unstructured.NestedString(app.Object, "spec", "project")
- if !ok || err != nil {
- return nil
- }
- projObj, ok, err := appProjInformer.GetIndexer().GetByKey(fmt.Sprintf("%s/%s", app.GetNamespace(), projName))
+func getAppProj(app *unstructured.Unstructured, appProjInformer cache.SharedIndexInformer, controllerNamespace string) *unstructured.Unstructured {
+ projName, _, err := unstructured.NestedString(app.Object, "spec", "project")
+ if err != nil {
+ return nil
+ }
+ // An empty or absent project means the app belongs to the 'default' project.
+ if projName == "" {
+ projName = "default"
+ }
+ // AppProjects live in the controller namespace, and the informer only watches
+ // that namespace, so key on it rather than the application's namespace (which
+ // misses for apps-in-any-namespace). See argoproj/argo-cd#28137.
+ projObj, ok, err := appProjInformer.GetIndexer().GetByKey(fmt.Sprintf("%s/%s", controllerNamespace, projName))
if !ok || err != nil {
return nil
}
This ensures the correct resource is found regardless of the application's namespace.
3. Apply standard Argo CD client rate limits
The controller now sets higher, tunable QPS/burst limits for its Kubernetes client, matching the rest of Argo CD:
+ // Apply Argo CD's shared client defaults (QPS/burst, transport, timeouts)
+ // like the other components. client-go's defaults (QPS 5 / burst 10) are
+ // far too low and throttle the per-app PATCH that persists the `notified`
+ // annotation; SetK8SConfigDefaults raises them to K8sClientConfigQPS/Burst
+ // (50/100, tunable via ARGOCD_K8S_CLIENT_QPS / ARGOCD_K8S_CLIENT_BURST).
+ if err := v1alpha1.SetK8SConfigDefaults(restConfig); err != nil {
+ return fmt.Errorf("failed to apply K8s client defaults: %w", err)
+ }
This prevents the controller from being a bottleneck due to low default rate limits.
4. Update interfaces and tests
The GetAppProject interface and its mocks/tests are updated to drop the unused namespace parameter and to test the new cache-based logic, including correct fallback and defaulting behaviors.
- GetAppProject(ctx context.Context, projectName string, namespace string) (*unstructured.Unstructured, error)
+ GetAppProject(ctx context.Context, projectName string) (*unstructured.Unstructured, error)
+ SetAppProjectInformer(informer cache.SharedIndexInformer)
And in tests:
+func TestGetAppProj_appInDifferentNamespace(t *testing.T) {
+ // AppProject lives in the controller namespace...
+ ...
+ // ...while the application lives in a different (app) namespace. Keying on the
+ // app namespace would miss the cache; the lookup must use the controller namespace.
+ ...
+}
From the review thread
No notable review discussion on this PR.
Takeaways
- Use informer caches for read-heavy, frequently accessed Kubernetes resources to avoid unnecessary API calls and rate limiting.
- Always resolve resources using their actual storage namespace, not the namespace of referencing objects, to prevent lookup errors and log spam.
- Apply consistent, appropriate client settings (like QPS and burst) across components to avoid hidden bottlenecks and ensure scalability.