kubeadmiral
kubeadmiral copied to clipboard
`FederatedObject.spec.template` should have type `map[string]interface{}`?
FederatedObject.spec.template
is currently of type k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1.JSON
which stores the raw json bytes. This type was used because controller-gen
refuses to implement support for map[string]interface{}
fields for CRD generation as discussed in https://github.com/kubernetes-sigs/controller-tools/issues/636. However, map[string]interface{}
would be more efficient and ergonomic as it avoids the unmarshaling whenever we need to access the template. We'd like to use map[string]interface{}
if we can find a way to hack controller-gen
to generate CRDs nonetheless.
We could start by looking at post-gen patches (which is already done in config/crds/patches
). Maybe we could ask controller-gen
to ignore the template
field and let the post-gen patches add it?
How often do we need to unmarshal the template? It saves the cost of deserializing them every time federated object is updated (especially status-only updates), and it reduces memory footprint. Any benchmarks to support the argument that late deserializations are more frequent than informer deserializations?
How often do we need to unmarshal the template? It saves the cost of deserializing them every time federated object is updated (especially status-only updates), and it reduces memory footprint. Any benchmarks to support this argument?
No benchmarks, but you do have a valid point.
Currently template
is accessed mainly via the GetTemplateAsUnstructured
method, which does the deserialization. My concern is mainly when the method is invoked multiple times during the reconcile loop, e.g. in event handler's FilterFunc
, UpdateFunc
, then in reconcile
, and potentially in downstream util methods. Ergonomics is an important factor too.
Let's review the GetTemplateAsUnstructured
usages one by one:
- federate: It is only used to retrieve the GVK of the federated object. Could we just parse it as a
metav1.GroupVersionKind
there, or maybe just store the GVK in federated object labels? (Such a label would be useful for users anyway) - follower: Same as above, except we only need
metav1.GroupKind
- nsautoprop: Same as above, used to filter federated objects. (I even wanted to suggest using a filtered watch, but apparently it doesn't work well with shared informers)
- override (image overrider): this is the scope of resource interpreters, see below
- scheduler: We only need the metadata (labels/anotations). Allocating so many hashmaps for the nested objects is still wasteful.
- status/statusaggregator: Same as federate controller, only need
metav1.GroupVersionKind
- sync: OK, this is where we actually need a
map[string]any
in order to execute JSONPatch properly - util/pod: See resource interpreter below
- util/propagationstatus: This compares two JSON objects. Not sure if there are more efficient ways to do this. But this is only called from statusaggregator anyway.
For resource interpreter cases: If we support interpreter plugins written in Go, they'd better be deserialized into their native types. If we support interpreter plugins in some dynamic library or scripting language (WASM, Lua, etc, whatever), passing the object in JSON is probably the most efficient way to cross language boundaries. Allocating it into map[string]any
in advance does not seem to provide any notable benefit.
So given these use cases, the only reasonable use cases would be sync and statusaggregator, which most likely reconcile much less frequently than informer updates.