Skip to content

Commit 4224175

Browse files
committed
Code clenup and commenting
Signed-off-by: Siddhesh Ghadi <[email protected]>
1 parent f1adb78 commit 4224175

File tree

3 files changed

+56
-21
lines changed

3 files changed

+56
-21
lines changed

cmd/main.go

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ import (
6262
v1alpha1 "github.com/argoproj-labs/argocd-operator/api/v1alpha1"
6363
v1beta1 "github.com/argoproj-labs/argocd-operator/api/v1beta1"
6464
"github.com/argoproj-labs/argocd-operator/pkg/cacheutils"
65-
wc "github.com/argoproj-labs/argocd-operator/pkg/clientwrapper"
65+
cw "github.com/argoproj-labs/argocd-operator/pkg/clientwrapper"
6666
"github.com/argoproj-labs/argocd-operator/version"
6767
//+kubebuilder:scaffold:imports
6868
)
@@ -184,19 +184,19 @@ func main() {
184184
Controller: controllerconfig.Controller{
185185
SkipNameValidation: &skipControllerNameValidation,
186186
},
187+
// Use transformers to strip data from Secrets and ConfigMaps
188+
// that are not tracked by the operator to reduce memory usage.
187189
Cache: cache.Options{
188190
Scheme: scheme,
189191
ByObject: map[crclient.Object]cache.ByObject{
190-
&corev1.Secret{}: {Transform: cacheutils.StripSecretDataTranform()},
192+
&corev1.Secret{}: {Transform: cacheutils.StripSecretDataTransform()},
191193
&corev1.ConfigMap{}: {Transform: cacheutils.StripConfigMapDataTransform()},
192194
},
193195
},
194196
}
195197

196198
if watchedNsCache := getDefaultWatchedNamespacesCacheOptions(); watchedNsCache != nil {
197-
options.Cache = cache.Options{
198-
DefaultNamespaces: watchedNsCache,
199-
}
199+
options.Cache.DefaultNamespaces = watchedNsCache
200200
}
201201

202202
mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), options)
@@ -205,9 +205,17 @@ func main() {
205205
os.Exit(1)
206206
}
207207

208-
liveClient, _ := crclient.New(ctrl.GetConfigOrDie(), crclient.Options{Scheme: mgr.GetScheme()})
209-
client := wc.NewClientWrapper(mgr.GetClient(), liveClient)
210-
//print(client)
208+
liveClient, err := crclient.New(ctrl.GetConfigOrDie(), crclient.Options{Scheme: mgr.GetScheme()})
209+
if err != nil {
210+
setupLog.Error(err, "unable to create live client")
211+
os.Exit(1)
212+
}
213+
214+
// Wraps the controller runtime's default client to provide:
215+
// 1. Fallback to the live client when a Secret/ConfigMap is stripped in the cache.
216+
// 2. Automatic labeling of fetched objects, so they are retained in full form
217+
// in subsequent cache updates and avoid repeated live lookups.
218+
client := cw.NewClientWrapper(mgr.GetClient(), liveClient)
211219

212220
setupLog.Info("Registering Components.")
213221

pkg/cacheutils/utlis.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ import (
77
"github.com/argoproj-labs/argocd-operator/common"
88
)
99

10-
func StripSecretDataTranform() clientgotools.TransformFunc {
10+
// StripSecretDataTransform returns a TransformFunc that strips the data from Secrets
11+
// that are not tracked by the operator. This is useful for reducing memory usage
12+
// when caching Secrets that are not managed by the operator.
13+
func StripSecretDataTransform() clientgotools.TransformFunc {
1114
return func(in interface{}) (interface{}, error) {
1215
if s, ok := in.(*v1.Secret); ok {
1316
if IsTrackedByOperator(s.Labels) {
@@ -28,6 +31,9 @@ func StripSecretDataTranform() clientgotools.TransformFunc {
2831
}
2932
}
3033

34+
// StripConfigMapDataTransform returns a TransformFunc that strips the data from ConfigMaps
35+
// that are not tracked by the operator. This is useful for reducing memory usage
36+
// when caching ConfigMaps that are not managed by the operator.
3137
func StripConfigMapDataTransform() clientgotools.TransformFunc {
3238
return func(in interface{}) (interface{}, error) {
3339
if cm, ok := in.(*v1.ConfigMap); ok {
@@ -47,6 +53,7 @@ func StripConfigMapDataTransform() clientgotools.TransformFunc {
4753
}
4854
}
4955

56+
// IsTrackedByOperator checks if the given labels indicate that the resource is tracked by the operator.
5057
func IsTrackedByOperator(labels map[string]string) bool {
5158
trackedLabels := []string{
5259
common.ArgoCDTrackedByOperatorLabel,

pkg/clientwrapper/client.go

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,63 +9,74 @@ import (
99

1010
"github.com/argoproj-labs/argocd-operator/common"
1111
"github.com/argoproj-labs/argocd-operator/pkg/cacheutils"
12+
logf "sigs.k8s.io/controller-runtime/pkg/log"
1213
)
1314

15+
var log = logf.Log.WithName("clientwrapper")
16+
1417
// ClientWrapper wraps a cached client and only falls back to
1518
// live GET when the cached object appears stripped OR is missing required labels.
19+
// We currently override GET only, but could extend to other methods if needed.
1620
type ClientWrapper struct {
1721
ctrlclient.Client // cached client
1822
liveClient ctrlclient.Client // direct API client
1923
}
2024

25+
// NewClientWrapper creates a new ClientWrapper instance.
2126
func NewClientWrapper(cached, live ctrlclient.Client) *ClientWrapper {
2227
return &ClientWrapper{
2328
Client: cached,
2429
liveClient: live,
2530
}
2631
}
2732

33+
// Get first tries to get from the cached client, and only falls back to live GET
34+
// if the cached object appears stripped or is missing required labels.
35+
// After a live GET, it also ensures the object has the tracking label (best-effort).
2836
func (cw *ClientWrapper) Get(ctx context.Context, key types.NamespacedName, obj ctrlclient.Object, opts ...ctrlclient.GetOption) error {
29-
// 1) Cache read (no live fallback here on error)
37+
3038
if err := cw.Client.Get(ctx, key, obj, opts...); err != nil {
3139
return err
3240
}
3341

34-
// 2) Post-read check: only specific kinds may need live refresh
3542
switch o := obj.(type) {
3643
case *corev1.Secret:
3744
if secretNeedsLiveRefresh(o) {
38-
// Re-fetch from live to get the full object
45+
3946
if err := cw.liveClient.Get(ctx, key, obj, opts...); err != nil {
4047
return err
4148
}
42-
// Ensure it becomes tracked for future full-caching (best-effort)
49+
4350
cw.ensureTrackedLabel(ctx, obj)
4451
}
52+
4553
case *corev1.ConfigMap:
4654
if configmapNeedsLiveRefresh(o) {
47-
// Re-fetch from live to get the full object
55+
4856
if err := cw.liveClient.Get(ctx, key, obj, opts...); err != nil {
4957
return err
5058
}
51-
// Ensure it becomes tracked for future full-caching (best-effort)
59+
5260
cw.ensureTrackedLabel(ctx, obj)
5361
}
5462

55-
// add more kinds here (e.g., *corev1.ConfigMap) if you apply similar striping
63+
// add more kinds here (e.g., *corev1.Deployment) if applying similar striping
64+
5665
}
5766

5867
return nil
5968
}
6069

6170
// secretNeedsLiveRefresh returns true if the cached secret looks stripped or untracked.
6271
func secretNeedsLiveRefresh(s *corev1.Secret) bool {
63-
// Untracked → we likely cached a slimmed object; refresh live.
72+
6473
if !cacheutils.IsTrackedByOperator(s.GetLabels()) {
6574
return true
6675
}
67-
// Heuristic: a "slimmed" secret from our transform has nil Data/StringData.
68-
// (Note: a legitimately empty secret may also match; adjust if you add a marker label/annotation.)
76+
77+
// Heuristic: a "stripped" Secret from our transform has nil Data/StringData.
78+
// A truly empty Secret may also match, but that only triggers an extra live GET,
79+
// which is rare and acceptable.
6980
if s.Data == nil && s.StringData == nil {
7081
return true
7182
}
@@ -77,13 +88,17 @@ func configmapNeedsLiveRefresh(cm *corev1.ConfigMap) bool {
7788
if !cacheutils.IsTrackedByOperator(cm.GetLabels()) {
7889
return true
7990
}
91+
92+
// Heuristic: a "stripped" ConfigMap from our transform has nil Data/BinaryData.
93+
// A truly empty ConfigMap may also match, but that only triggers an extra live GET,
94+
// which is rare and acceptable.
8095
if cm.Data == nil && cm.BinaryData == nil {
8196
return true
8297
}
8398
return false
8499
}
85100

86-
// ensureTrackedLabel adds the operator tracking label (best-effort, ignores patch error).
101+
// ensureTrackedLabel adds the operator tracking label.
87102
func (cw *ClientWrapper) ensureTrackedLabel(ctx context.Context, obj ctrlclient.Object) {
88103
if cacheutils.IsTrackedByOperator(obj.GetLabels()) {
89104
return
@@ -97,5 +112,10 @@ func (cw *ClientWrapper) ensureTrackedLabel(ctx context.Context, obj ctrlclient.
97112
labels[common.ArgoCDTrackedByOperatorLabel] = common.ArgoCDAppName
98113
obj.SetLabels(labels)
99114

100-
_ = cw.liveClient.Patch(ctx, obj, ctrlclient.MergeFrom(orig)) // best-effort
115+
// Best-effort patch to add the operator tracking label.
116+
// Non-fatal: a later reconcile will reattempt if this fails.
117+
err := cw.liveClient.Patch(ctx, obj, ctrlclient.MergeFrom(orig))
118+
if err != nil {
119+
log.Error(err, "failed to add operator tracking label to object", "name", obj.GetName(), "namespace", obj.GetNamespace())
120+
}
101121
}

0 commit comments

Comments
 (0)