Skip to content

Commit

Permalink
Avoid walking entire schema tree
Browse files Browse the repository at this point in the history
  • Loading branch information
Jordan Olshevski committed Jan 2, 2024
1 parent 0765f21 commit fe16ca5
Showing 1 changed file with 52 additions and 16 deletions.
68 changes: 52 additions & 16 deletions internal/controllers/reconciliation/discoverycache.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"sync"

"github.com/go-logr/logr"
openapi_v2 "github.com/google/gnostic-models/openapiv2"
"gopkg.in/yaml.v2"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/discovery"
Expand All @@ -17,10 +19,11 @@ import (
// discoveryCache is useful to prevent excessive QPS to the discovery APIs while
// still allowing dynamic refresh of the openapi spec on cache misses.
type discoveryCache struct {
mut sync.Mutex
client discovery.DiscoveryInterface
fillWhenNotFound bool
current openapi.Resources
mut sync.Mutex
client discovery.DiscoveryInterface
fillWhenNotFound bool
currentResources openapi.Resources
currentSupportedTypes map[schema.GroupVersionKind]struct{}
}

func newDicoveryCache(rc *rest.Config, qps float32, fillWhenNotFound bool) (*discoveryCache, error) {
Expand All @@ -33,7 +36,7 @@ func newDicoveryCache(rc *rest.Config, qps float32, fillWhenNotFound bool) (*dis
}
disc.UseLegacyDiscovery = true // don't bother with aggregated APIs since they may be unavailable

d := &discoveryCache{client: disc, fillWhenNotFound: fillWhenNotFound}
d := &discoveryCache{client: disc, fillWhenNotFound: fillWhenNotFound, currentSupportedTypes: map[schema.GroupVersionKind]struct{}{}}
return d, nil
}

Expand All @@ -45,17 +48,16 @@ func (d *discoveryCache) Get(ctx context.Context, gvk schema.GroupVersionKind) (
// Older versions of Kubernetes don't include CRDs in the openapi spec, so on those versions we cannot invalidate the cache if a resource is not found.
// However, on newer versions we expect every resource to exist in the spec so retries are safe and often necessary.
for i := 0; i < 2; i++ {
if d.current == nil {
// TODO: is this called when it shouldn't be? Need to enable debug logs
if d.currentResources == nil {
logger.V(1).Info("filling discovery cache")
if err := d.fillUnlocked(ctx); err != nil {
return nil, err
}
}

model := d.current.LookupResource(gvk)
model := d.currentResources.LookupResource(gvk)
if model == nil && d.fillWhenNotFound {
d.current = nil // invalidate cache - retrieve fresh schema on next attempt
d.currentResources = nil // invalidate cache - retrieve fresh schema on next attempt
continue
}
return d.checkSupportUnlocked(ctx, gvk, model)
Expand All @@ -72,7 +74,8 @@ func (d *discoveryCache) fillUnlocked(ctx context.Context) error {
if err != nil {
return err
}
d.current = resources
d.currentResources = resources
d.currentSupportedTypes = buildSupportedTypesMap(doc)
return nil
}

Expand All @@ -83,13 +86,46 @@ func (d *discoveryCache) checkSupportUnlocked(ctx context.Context, gvk schema.Gr
return nil, nil
}

// TODO: Something here is consuming a lot of cpu cycles

for _, c := range d.current.GetConsumes(gvk, "PATCH") {
if c == string(types.StrategicMergePatchType) {
return model, nil
}
if _, ok := d.currentSupportedTypes[gvk]; ok {
return model, nil
}

return nil, nil // doesn't support strategic merge
}

func buildSupportedTypesMap(doc *openapi_v2.Document) map[schema.GroupVersionKind]struct{} {
// This is copied and adapted from the kubectl openapi package
// Originally it walked the entire tree for every lookup, we have optimized it down to a single map lookup.
m := make(map[schema.GroupVersionKind]struct{})
for _, path := range doc.GetPaths().GetPath() {
for _, ex := range path.GetValue().GetPatch().GetVendorExtension() {
if ex.GetValue().GetYaml() == "" ||
ex.GetName() != "x-kubernetes-group-version-kind" {
continue
}

var value map[string]string
err := yaml.Unmarshal([]byte(ex.GetValue().GetYaml()), &value)
if err != nil {
continue
}

gvk := schema.GroupVersionKind{
Group: value["group"],
Version: value["version"],
Kind: value["kind"],
}
var supported bool
for _, c := range path.GetValue().GetPatch().GetConsumes() {
if c == string(types.StrategicMergePatchType) {
supported = true
break
}
}
if supported {
m[gvk] = struct{}{}
}
}
}
return m
}

0 comments on commit fe16ca5

Please sign in to comment.