Skip to content

Commit

Permalink
Fix manifest reader inefficiency (#160)
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
michaeljguarino authored Apr 6, 2024
1 parent aee91d2 commit f5aaabe
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 18 deletions.
18 changes: 2 additions & 16 deletions pkg/controller/service/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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")
Expand Down
27 changes: 27 additions & 0 deletions pkg/manifests/template/cache.go
Original file line number Diff line number Diff line change
@@ -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
}
14 changes: 14 additions & 0 deletions pkg/manifests/template/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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())
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/manifests/template/raw.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)
}
Expand Down
1 change: 0 additions & 1 deletion pkg/manifests/template/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit f5aaabe

Please sign in to comment.