From f5aaabedda074de018d571d4669d3f0b73f96d04 Mon Sep 17 00:00:00 2001 From: michaeljguarino Date: Sat, 6 Apr 2024 11:35:17 -0400 Subject: [PATCH] Fix manifest reader inefficiency (#160) For some reason this only has popped up when trying to sync opa constraints but there are two major inefficiencies w/ manifest reader right now: * has to go through RESTMapper to check namespacing (this can sometimes result in a network call, so caching is a meaningful optimization) * Apparently it doubles the array on each cycle because it calls read on its own result list. Think we've only used small sets of files for raw resources prior so never became an issue, but it's definitely an issue at large file sets (as you suddenly have a result of like 48k) --- pkg/controller/service/reconciler.go | 18 ++---------------- pkg/manifests/template/cache.go | 27 +++++++++++++++++++++++++++ pkg/manifests/template/common.go | 14 ++++++++++++++ pkg/manifests/template/raw.go | 4 +++- pkg/manifests/template/reader.go | 1 - 5 files changed, 46 insertions(+), 18 deletions(-) create mode 100644 pkg/manifests/template/cache.go diff --git a/pkg/controller/service/reconciler.go b/pkg/controller/service/reconciler.go index 924edbb4..a25b10d9 100644 --- a/pkg/controller/service/reconciler.go +++ b/pkg/controller/service/reconciler.go @@ -278,9 +278,10 @@ func (s *ServiceReconciler) Reconcile(ctx context.Context, id string) (result re return } + logger.Info("Fetching manifests", "service", svc.Name) manifests, err := s.ManifestCache.Fetch(s.UtilFactory, svc) if err != nil { - logger.Error(err, "failed to parse manifests") + logger.Error(err, "failed to parse manifests", "service", svc.Name) return } manifests = postProcess(manifests) @@ -294,21 +295,6 @@ func (s *ServiceReconciler) Reconcile(ctx context.Context, id string) (result re vcache := manis.VersionCache(manifests) - if svc.DeletedAt != nil { - logger.Info("Deleting service", "name", svc.Name, "namespace", svc.Namespace) - ch := s.Destroyer.Run(ctx, inv, apply.DestroyerOptions{ - InventoryPolicy: inventory.PolicyAdoptIfNoInventory, - DryRunStrategy: common.DryRunNone, - DeleteTimeout: 20 * time.Second, - DeletePropagationPolicy: metav1.DeletePropagationBackground, - EmitStatusEvents: true, - ValidationPolicy: 1, - }) - - err = s.UpdatePruneStatus(ctx, svc, ch, vcache) - return - } - logger.Info("Apply service", "name", svc.Name, "namespace", svc.Namespace) if err = s.CheckNamespace(svc.Namespace); err != nil { logger.Error(err, "failed to check namespace") diff --git a/pkg/manifests/template/cache.go b/pkg/manifests/template/cache.go new file mode 100644 index 00000000..d137a0f7 --- /dev/null +++ b/pkg/manifests/template/cache.go @@ -0,0 +1,27 @@ +package template + +import ( + "k8s.io/apimachinery/pkg/runtime/schema" +) + +var ( + namespacedCache = &gvkCache{cache: map[schema.GroupVersionKind]bool{}} +) + +type gvkCache struct { + cache map[schema.GroupVersionKind]bool +} + +func (c *gvkCache) Store(gvk schema.GroupVersionKind, namespaced bool) { + c.cache[gvk] = namespaced +} + +func (c *gvkCache) Namespaced(gvk schema.GroupVersionKind) bool { + val, ok := c.cache[gvk] + return ok && val +} + +func (c *gvkCache) Present(gvk schema.GroupVersionKind) bool { + _, ok := c.cache[gvk] + return ok +} diff --git a/pkg/manifests/template/common.go b/pkg/manifests/template/common.go index e5f62a0e..19810cc1 100644 --- a/pkg/manifests/template/common.go +++ b/pkg/manifests/template/common.go @@ -31,6 +31,18 @@ func setNamespaces(mapper meta.RESTMapper, objs []*unstructured.Unstructured, continue } + gvk := obj.GroupVersionKind() + if namespacedCache.Present(gvk) { + if namespacedCache.Namespaced(gvk) && obj.GetNamespace() == "" { + obj.SetNamespace(defaultNamespace) + } + + if !namespacedCache.Namespaced(gvk) && obj.GetNamespace() != "" { + obj.SetNamespace("") + } + continue + } + // Look up the scope of the resource so we know if the resource // should have a namespace set or not. scope, err := object.LookupResourceScope(obj, crdObjs, mapper) @@ -60,11 +72,13 @@ func setNamespaces(mapper meta.RESTMapper, objs []*unstructured.Unstructured, } } } + namespacedCache.Store(gvk, true) case meta.RESTScopeRoot: if ns := obj.GetNamespace(); ns != "" { obj.SetNamespace("") fmt.Printf("Found cluster scoped resource %s with namespace %s, coerced to un-namespaced\n", obj.GetName(), ns) } + namespacedCache.Store(gvk, false) default: return fmt.Errorf("unknown RESTScope %q", scope.Name()) } diff --git a/pkg/manifests/template/raw.go b/pkg/manifests/template/raw.go index ccaeb47c..47f6cbf5 100644 --- a/pkg/manifests/template/raw.go +++ b/pkg/manifests/template/raw.go @@ -85,6 +85,7 @@ func (r *raw) Render(svc *console.GetServiceDeploymentForAgent_ServiceDeployment if info.IsDir() { return nil } + if ext := strings.ToLower(filepath.Ext(info.Name())); !lo.Contains(extensions, ext) { return nil } @@ -111,7 +112,8 @@ func (r *raw) Render(svc *console.GetServiceDeploymentForAgent_ServiceDeployment Reader: r, ReaderOptions: readerOptions, } - items, err := mReader.Read(res) + items, err := mReader.Read([]*unstructured.Unstructured{}) + if err != nil { return fmt.Errorf("failed to parse %s: %w", rpath, err) } diff --git a/pkg/manifests/template/reader.go b/pkg/manifests/template/reader.go index e2ca726f..9c1c70b4 100644 --- a/pkg/manifests/template/reader.go +++ b/pkg/manifests/template/reader.go @@ -66,7 +66,6 @@ func (r *StreamManifestReader) Read(objs []*unstructured.Unstructured) ([]*unstr } objs = manifestreader.FilterLocalConfig(objs) - err = setNamespaces(r.Mapper, objs, r.Namespace, r.EnforceNamespace) return objs, err }