Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions cmd/catalogd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import (
"github.com/operator-framework/operator-controller/internal/catalogd/storage"
"github.com/operator-framework/operator-controller/internal/catalogd/webhook"
sharedcontrollers "github.com/operator-framework/operator-controller/internal/shared/controllers"
cacheutil "github.com/operator-framework/operator-controller/internal/shared/util/cache"
fsutil "github.com/operator-framework/operator-controller/internal/shared/util/fs"
httputil "github.com/operator-framework/operator-controller/internal/shared/util/http"
imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image"
Expand Down Expand Up @@ -254,6 +255,8 @@ func run(ctx context.Context) error {

cacheOptions := crcache.Options{
ByObject: map[client.Object]crcache.ByObject{},
// Memory optimization: strip managed fields and large annotations from cached objects
DefaultTransform: cacheutil.StripManagedFieldsAndAnnotations(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion:
Just a small nit — this isn’t valid only for Boxcutter since we’re not protecting it with a feature flag.
Could we clarify that in the title? As it stands, it gives the impression that the change only applies when the flag is enabled.
Alternatively, if we don’t want it to be used by default, we could protect the change with the flag.

}

saKey, err := sautil.GetServiceAccount()
Expand Down
9 changes: 8 additions & 1 deletion cmd/operator-controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
k8stypes "k8s.io/apimachinery/pkg/types"
apimachineryrand "k8s.io/apimachinery/pkg/util/rand"
"k8s.io/client-go/discovery"
"k8s.io/client-go/discovery/cached/memory"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
_ "k8s.io/client-go/plugin/pkg/client/auth"
"k8s.io/klog/v2"
Expand Down Expand Up @@ -77,6 +78,7 @@ import (
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1"
"github.com/operator-framework/operator-controller/internal/operator-controller/scheme"
sharedcontrollers "github.com/operator-framework/operator-controller/internal/shared/controllers"
cacheutil "github.com/operator-framework/operator-controller/internal/shared/util/cache"
fsutil "github.com/operator-framework/operator-controller/internal/shared/util/fs"
httputil "github.com/operator-framework/operator-controller/internal/shared/util/http"
imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image"
Expand Down Expand Up @@ -231,6 +233,8 @@ func run() error {
cfg.systemNamespace: {LabelSelector: k8slabels.Everything()},
},
DefaultLabelSelector: k8slabels.Nothing(),
// Memory optimization: strip managed fields and large annotations from cached objects
DefaultTransform: cacheutil.StripManagedFieldsAndAnnotations(),
}

if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) {
Expand Down Expand Up @@ -572,11 +576,14 @@ func setupBoxcutter(
RevisionGenerator: rg,
}

discoveryClient, err := discovery.NewDiscoveryClientForConfig(mgr.GetConfig())
baseDiscoveryClient, err := discovery.NewDiscoveryClientForConfig(mgr.GetConfig())
if err != nil {
return fmt.Errorf("unable to create discovery client: %w", err)
}

// Wrap the discovery client with caching to reduce memory usage from repeated OpenAPI schema fetches
discoveryClient := memory.NewMemCacheClient(baseDiscoveryClient)

trackingCache, err := managedcache.NewTrackingCache(
ctrl.Log.WithName("trackingCache"),
mgr.GetConfig(),
Expand Down
3 changes: 2 additions & 1 deletion internal/catalogd/garbagecollection/garbage_collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ func runGarbageCollection(ctx context.Context, cachePath string, metaClient meta
if err != nil {
return nil, fmt.Errorf("error reading cache directory: %w", err)
}
removed := []string{}
// Pre-allocate removed slice with estimated capacity to avoid reallocation
removed := make([]string, 0, len(cacheDirEntries))
for _, cacheDirEntry := range cacheDirEntries {
if cacheDirEntry.IsDir() && expectedCatalogs.Has(cacheDirEntry.Name()) {
continue
Expand Down
3 changes: 2 additions & 1 deletion internal/catalogd/storage/localdir.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ func (s *LocalDirV1) Store(ctx context.Context, catalog string, fsys fs.FS) erro
}

eg, egCtx := errgroup.WithContext(ctx)
metaChans := []chan *declcfg.Meta{}
// Pre-allocate metaChans with correct capacity to avoid reallocation
metaChans := make([]chan *declcfg.Meta, 0, len(storeMetaFuncs))

for range storeMetaFuncs {
metaChans = append(metaChans, make(chan *declcfg.Meta, 1))
Expand Down
40 changes: 28 additions & 12 deletions internal/operator-controller/applier/boxcutter.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
ocv1 "github.com/operator-framework/operator-controller/api/v1"
"github.com/operator-framework/operator-controller/internal/operator-controller/controllers"
"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
"github.com/operator-framework/operator-controller/internal/shared/util/cache"
)

const (
Expand Down Expand Up @@ -58,14 +59,17 @@ func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease(
return nil, err
}

labels := maps.Clone(obj.GetLabels())
if labels == nil {
labels = map[string]string{}
}
existingLabels := obj.GetLabels()
labels := make(map[string]string, len(existingLabels)+len(objectLabels))
maps.Copy(labels, existingLabels)
maps.Copy(labels, objectLabels)
obj.SetLabels(labels)
obj.SetOwnerReferences(nil) // reset OwnerReferences for migration.

// Memory optimization: strip large annotations and managed fields
// Note: ApplyStripTransform never returns an error in practice
_ = cache.ApplyStripTransform(&obj)

objs = append(objs, ocv1.ClusterExtensionRevisionObject{
Object: obj,
CollisionProtection: ocv1.CollisionProtectionNone, // allow to adopt objects from previous release
Expand Down Expand Up @@ -96,10 +100,9 @@ func (r *SimpleRevisionGenerator) GenerateRevision(
// objectLabels
objs := make([]ocv1.ClusterExtensionRevisionObject, 0, len(plain))
for _, obj := range plain {
labels := maps.Clone(obj.GetLabels())
if labels == nil {
labels = map[string]string{}
}
existingLabels := obj.GetLabels()
labels := make(map[string]string, len(existingLabels)+len(objectLabels))
maps.Copy(labels, existingLabels)
maps.Copy(labels, objectLabels)
obj.SetLabels(labels)

Expand All @@ -115,6 +118,11 @@ func (r *SimpleRevisionGenerator) GenerateRevision(
unstr := unstructured.Unstructured{Object: unstrObj}
unstr.SetGroupVersionKind(gvk)

// Memory optimization: strip large annotations and managed fields
if err := cache.ApplyStripTransform(&unstr); err != nil {
return nil, err
}

objs = append(objs, ocv1.ClusterExtensionRevisionObject{
Object: unstr,
})
Expand Down Expand Up @@ -329,7 +337,8 @@ func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
// ClusterExtensionRevisionPreviousLimit or to the first _active_ revision and deletes trimmed revisions from the cluster.
// NOTE: revisionList must be sorted in chronographical order, from oldest to latest.
func (bc *Boxcutter) setPreviousRevisions(ctx context.Context, latestRevision *ocv1.ClusterExtensionRevision, revisionList []ocv1.ClusterExtensionRevision) error {
trimmedPrevious := make([]ocv1.ClusterExtensionRevisionPrevious, 0)
// Pre-allocate with capacity limit to reduce allocations
trimmedPrevious := make([]ocv1.ClusterExtensionRevisionPrevious, 0, ClusterExtensionRevisionPreviousLimit)
for index, r := range revisionList {
if index < len(revisionList)-ClusterExtensionRevisionPreviousLimit && r.Spec.LifecycleState == ocv1.ClusterExtensionRevisionLifecycleStateArchived {
// Delete oldest CREs from the cluster and list to reach ClusterExtensionRevisionPreviousLimit or latest active revision
Expand Down Expand Up @@ -371,9 +380,16 @@ func latestRevisionNumber(prevRevisions []ocv1.ClusterExtensionRevision) int64 {
}

func splitManifestDocuments(file string) []string {
//nolint:prealloc
var docs []string
for _, manifest := range strings.Split(file, "\n") {
// Estimate: typical manifests have ~50-100 lines per document
// Pre-allocate for reasonable bundle size to reduce allocations
lines := strings.Split(file, "\n")
estimatedDocs := len(lines) / 20 // conservative estimate
if estimatedDocs < 4 {
estimatedDocs = 4
}
docs := make([]string, 0, estimatedDocs)

for _, manifest := range lines {
manifest = strings.TrimSpace(manifest)
if len(manifest) == 0 {
continue
Expand Down
71 changes: 71 additions & 0 deletions internal/shared/util/cache/transform.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package cache
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: perhaps better location for this file is internal/shared/controllers?


import (
"maps"

toolscache "k8s.io/client-go/tools/cache"
crcache "sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
)

// stripAnnotations removes memory-heavy annotations that aren't needed for controller operations.
func stripAnnotations(obj interface{}) (interface{}, error) {
if metaObj, ok := obj.(client.Object); ok {
// Remove the last-applied-configuration annotation which can be very large
// Clone the annotations map to avoid modifying shared references
annotations := metaObj.GetAnnotations()
if annotations != nil {
annotations = maps.Clone(annotations)
delete(annotations, "kubectl.kubernetes.io/last-applied-configuration")
if len(annotations) == 0 {
metaObj.SetAnnotations(nil)
} else {
metaObj.SetAnnotations(annotations)
}
}
}
return obj, nil
}

// StripManagedFieldsAndAnnotations returns a cache transform function that removes
// memory-heavy fields that aren't needed for controller operations.
// This significantly reduces memory usage in informer caches by removing:
// - Managed fields (can be several KB per object)
// - kubectl.kubernetes.io/last-applied-configuration annotation (can be very large)
//
// Use this function as a DefaultTransform in controller-runtime cache.Options
// to reduce memory overhead across all cached objects.
//
// Example:
//
// cacheOptions := cache.Options{
// DefaultTransform: cacheutil.StripManagedFieldsAndAnnotations(),
// }
func StripManagedFieldsAndAnnotations() toolscache.TransformFunc {
// Use controller-runtime's built-in TransformStripManagedFields and compose it
// with our custom annotation stripping transform
managedFieldsTransform := crcache.TransformStripManagedFields()

return func(obj interface{}) (interface{}, error) {
// First strip managed fields using controller-runtime's transform
obj, err := managedFieldsTransform(obj)
if err != nil {
return obj, err
}

// Then strip the large annotations
return stripAnnotations(obj)
}
}

// ApplyStripTransform applies the strip transform directly to an object.
// This is a convenience function for cases where you need to strip fields
// from an object outside of the cache transform context.
//
// Note: This function never returns an error in practice, but returns error
// to satisfy the TransformFunc interface.
func ApplyStripTransform(obj client.Object) error {
transform := StripManagedFieldsAndAnnotations()
_, err := transform(obj)
return err
}
Loading