allow scaling custom resource

[wjiec]

This PR implements the features described in #2779.

[wjiec]

The reason for blocking here is that command call the internal/dao/registry::Meta.loadCRDs method during initialization (and as part of the init of the whole app), and if it doesn't block here, it will result in an incorrect list of CRDs being read (and theoretically, other resources will have the same problem...).

But this also creates additional problems, in my cluster with a slightly higher load, a total of 96 CRDs takes about ~2s to synchronize.

I didn't think of any good way (or I'm missing something) to solve this problem, does anyone have any ideas?

[derailed]

@wjiec This could indeed be a problem. As you've noted any cluster with lots of resources will become a pb.
The idea here is to leverage k9s eventual consistance so we would not have to block til all caches are updated.

I think we could perhaps lazy eval the crd instead to gauge whether it is scalable once the user decides to actually view it.
Would this make sense?

[wjiec]

@derailed I've also considered checking if the crd is scalable when the view is created, but consider a scenario where if k9s needs to display a crd resource that supports scalable as soon as it starts up, and at that point the cache may not have finished synced, then factory.Get will either return a NotFound error or will need to block until that crd finish synced. Doesn't seem like an optimal approach in this case either?

The current logic decides whether to wait for the cache to finish sync based on the wait parameter, which I see is false in most cases, and only in some special cases does it have wait = true, which I think might be acceptable?

Or we make the key binding dynamic, and when the crds sync is done, we update the view to add the scale cmd.

What do you think?

[derailed]

@wjiec Thank you for your input and sorry for the delay... I like the idea of eventually enabling the scaling option once the resource is synced. This allows for a non blocking approach.

[derailed]

@wjiec Thank you for taking this on Jayson!

[derailed]

@wjiec This could indeed be a problem. As you've noted any cluster with lots of resources will become a pb.
The idea here is to leverage k9s eventual consistance so we would not have to block til all caches are updated.

I think we could perhaps lazy eval the crd instead to gauge whether it is scalable once the user decides to actually view it.
Would this make sense?

[wjiec]

@derailed I spent some time looking into the logic in Meta, and I found that all resources (including CRDs) are actually loaded during loadPreferred, and that loadCRDs don't bring any additional changes (even the previous assignment of meta.Name = crd.Name resulted in the wrong GVRs being registered to Meta. resMetas).

loadCRDs now wait for the cache to sync and add scaleCat for the corresponding CRD. In scale_extender it is dynamically decided whether to add a Scale command to the view based on the presence or absence of a scaleCat.

What do you think of this?

[derailed]

@wjiec Thank you for taking another pass Jayson!! The catch here is crd loads are currently async. waitForCacheSync will halt the load until all caches are refreshed (not just crds!). This could be a pb on larger clusters and hence I had initially opted to load async thus relying of eventual consistency. Does this make sense or am I missing it?

[wjiec]

@derailed Thank you very much for your reply and reminder, I think I understand what you mean! Sorry, it's my understanding that's wrong. I always thought loadCRDs would only be executed once on k9s startup 😅 I just now understand that the eventual consistency you're talking about says that loadCRDs load a portion at a time until the caches are done synchronizing.

[derailed]

@wjiec Thank you Jayson! No worries. I am glad you've asked!!
These can indeed get a little bit tricky ;(

[derailed]

@wjiec Looks like we have some conflicts. Could you take a peek? Thank you Jayson!

[wjiec]

Hi @derailed, I have resolved these conflicts, please take a look when you have time.

btw, it seems that Dynamic.Get in internal/dao/dynamic.go has not been implemented, which will break the Jump to Owner feature.

[derailed]

@wjiec Nice! Thank you for this update and the heads up on Dynamic!!

I think we should be ok here since I have not yet moved other resources to dynamic.
I think the jump to uses the generic dao vs dynamic.
Am I wrong about that?

In any case, I'll add the get accessor in the next drop.

[derailed]

@wjiec Thank you for the updates! Wondering if we should just roll up this logic in loadPreferred?? ie tune the crd name and add tag as scalable if warranted...

[derailed]

This should use Names.Singular.
Also I think the crd name needs to be unique ie crd.Name+"."+meta.Group

[wjiec]

Sorry I'm late, actually there should be no impact on PR correctness here as client.NewGVRFromMeta only needs Group, Version, VersionName and that's enough. Also, according to [Extend the Kubernetes API with CustomResourceDefinitions](https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/ custom-resource-definitions/) the description of name is name must match the spec fields below, and be in the form: <plural>. <group>, so crd.Spec.Names.Plural should be correct here.

[derailed]

We should add logic here to also update the meta if we have a non-scalabe crd since we need to track the updated meta definition. This is the case where vendorA and vendorB provide CRD named fred.
Perhaps we should rollup this logic in loadPreferred and axe loadCRD all together.
What do you think?

[wjiec]

I think loadCRD should still be necessary, loadPreferred is similar to kubectl api-resources, while loadCRD actually only handles crd resources, if put the logic of loadCRD into loadPreferred maybe still need list + for + process, so I think we can change the name of loadCRD?