-
Couldn't load subscription status.
- Fork 1k
feat: Cache transform for Secrets and ConfigMaps to reduce memory #1866
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Siddhesh Ghadi <[email protected]>
Signed-off-by: Siddhesh Ghadi <[email protected]>
Signed-off-by: Siddhesh Ghadi <[email protected]>
pkg/cacheutils/utlis.go
Outdated
| return in, nil | ||
| } | ||
| // Strip data for non-operator secrets | ||
| return &v1.Secret{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than creating a new Object, can we not set the Data field to nil on the same object ? When we create new Secret object. The old object with the data field also exists in memory and waits for Garbage collection to clean it up, isn't it ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @anandf I have updated it according to how you have mentioned in comment
pkg/cacheutils/utlis.go
Outdated
| // StripConfigMapDataTransform returns a TransformFunc that strips the data from ConfigMaps | ||
| // that are not tracked by the operator. This is useful for reducing memory usage | ||
| // when caching ConfigMaps that are not managed by the operator. | ||
| func StripConfigMapDataTransform() clientgotools.TransformFunc { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have a single generic function that can handle both Secret and ConfigMap ? Anyways the function takes in an generic interface{} struct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unified both function
pkg/cacheutils/utlis.go
Outdated
| } | ||
|
|
||
| // IsTrackedByOperator checks if the given labels indicate that the resource is tracked by the operator. | ||
| func IsTrackedByOperator(labels map[string]string) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can it take in a runtime.Object as input instead of labels ? That would be more meaningful as the method checks if a given resource is tracked by operator or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anandrkskd - can we incorporate this suggestion aswell?
Signed-off-by: Anand Kumar Singh <[email protected]>
Signed-off-by: Anand Kumar Singh <[email protected]>
Signed-off-by: Anand Kumar Singh <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to add some tests for the wrapper?
| | `REMOVE_MANAGED_BY_LABEL_ON_ARGOCD_DELETION` | false | When an Argo CD instance is deleted, namespaces managed by that instance (via the `argocd.argoproj.io/managed-by` label ) will retain the label by default. Users can change this behavior by setting the environment variable `REMOVE_MANAGED_BY_LABEL_ON_ARGOCD_DELETION` to `true` in the Subscription. | | ||
| | `ARGOCD_LABEL_SELECTOR` | none | The label selector can be set on argocd-opertor by exporting `ARGOCD_LABEL_SELECTOR` (eg: `export ARGOCD_LABEL_SELECTOR=foo=bar`). The labels can be added to the argocd instances using the command `kubectl label argocd test1 foo=bar -n test-argocd`. This will enable the operator instance to be tailored to oversee only the corresponding ArgoCD instances having the matching label selector. | | ||
| | `LOG_LEVEL` | info | This sets the logging level of the manager (operator) pod. Valid values are "debug", "info", "warn", "error", "panic" and "fatal". | | ||
| | `MEMORY_OPTIMIZATION_ENABLED` | true | Enables memory optimization by stripping data from Secrets and ConfigMaps that are not tracked by the operator, reducing memory usage. Set to `false` to disable this optimization. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| | `MEMORY_OPTIMIZATION_ENABLED` | true | Enables memory optimization by stripping data from Secrets and ConfigMaps that are not tracked by the operator, reducing memory usage. Set to `false` to disable this optimization. | | |
| | `DISABLE_MEMORY_OPTIMIZATION` | false | When set to `true`, disables the memory optimization that strips data from Secrets and ConfigMaps that are not tracked by the operator. This optimization helps reduce memory usage. | |
| if watchedNsCache := getDefaultWatchedNamespacesCacheOptions(); watchedNsCache != nil { | ||
| // Use transformers to strip data from Secrets and ConfigMaps | ||
| // that are not tracked by the operator to reduce memory usage. | ||
| if strings.ToLower(os.Getenv("MEMORY_OPTIMIZATION_ENABLED")) != "false" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets use DISABLE_MEMORY_OPTIMIZATION env var which users should explicitly set to true to disable optimization
| liveClient, err := crclient.New(ctrl.GetConfigOrDie(), crclient.Options{Scheme: mgr.GetScheme()}) | ||
| if err != nil { | ||
| setupLog.Error(err, "unable to create live client") | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| // Wraps the controller runtime's default client to provide: | ||
| // 1. Fallback to the live client when a Secret/ConfigMap is stripped in the cache. | ||
| // 2. Automatic labeling of fetched objects, so they are retained in full form | ||
| // in subsequent cache updates and avoid repeated live lookups. | ||
| client := cw.NewClientWrapper(mgr.GetClient(), liveClient) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic can also be placed behind the DISABLE_MEMORY_OPTIMIZATION check. Use the default client := mgr.GetClient() when memory optimization is disabled.
What type of PR is this?
/kind enhancement
What does this PR do / why we need it:
This PR reduces memory usage in the Argo CD Operator by introducing a cache transform and client wrapper for Secrets and ConfigMaps.
See the attached proposal in this PR for design details, trade-offs, PoC results, and future scope.
Results
These metrics were collected from a test cluster containing 100 ConfigMaps and 100 Secrets, each approximately 1 MB in size. The cluster was running four Argo CD instances and no other workload operators.
With the optimization enabled, operator memory usage dropped from ~350 MB to ~100 MB.
UnOptimized Operator Manager Memory:
Optimized Operator Manager Memory:
However, we could not reduce the startup memory consumption, which remained at ~750 MB in both cases.
We previously attempted another approach in #1795, but it introduced significant complexity and restricted how watches could be set up. Compared to that, this solution provides a better balance between complexity, maintenance overhead, and outcome.