Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT API Discovery #43

Closed
1 of 2 tasks
coryodaniel opened this issue Sep 29, 2019 · 10 comments · Fixed by #49
Closed
1 of 2 tasks

JIT API Discovery #43

coryodaniel opened this issue Sep 29, 2019 · 10 comments · Fixed by #49
Assignees

Comments

@coryodaniel
Copy link
Owner

coryodaniel commented Sep 29, 2019

JIT /api and /apis discovery per comment

  • first request is made for a given gvk, it is discovered and the naming options and scope are cached1
  • http path is generated as it is now, with the cached details
  • subsequent requests build urls from data in the cache

Initial thoughts
Evaluate if there is a non-discovery route that doesn’t require codegen that allows for loose naming of resource kinds and scope detection (namespaced/cluster)

Discovery exists to support various forms of resource names being passed to K8s.Client and how to format URLs with respect to resource scope, but makes development and testing a bit difficult as it requires stubbing our HTTP requests and the discovery phase (see File driver).

This could also serve as a ticket to make testing not as painful :)

@coryodaniel coryodaniel added this to the 0.5 milestone Oct 9, 2019
@gerred
Copy link

gerred commented Oct 11, 2019

@coryodaniel will you flesh your thinking out for me some more? I know that's a lot to ask, but want to get to the same mental framework you're working from on this particular issue. I'd love to contribute some ideas (and hopefully code) here. I love the fact that this uses discovery instead of code gen. I agree certain aspects of development become more difficult, and it would be nice to configure it for production use once everything is known, but...

Thinking of the KUDO use case, where it's much closer to Tiller, dynamic discovery is very helpful there since it's all about the data. I'd like to have a nice balance of schemes that I know about as a developer using k8s/bonny, and schemes that I don't know about but my users may want to use. This includes other custom resources that may be in clusters and aren't core resources.

@coryodaniel
Copy link
Owner Author

I haven’t really thought it through. I def don’t want to do codegen that’s was a nongoal if the project and having to compile against a swagger file really sucks for distributing operators.

Discovery may be something to keep. The current implementation has got loose bolts for sure. I added the dynamic HTTP provider to make testing a bit easier but it still bites me from time-to-time.

Would love to talk through this though! What time zone are you in?

@coryodaniel coryodaniel changed the title Replace cluster discovery w/ suggestions Evaluate Discovery functionality Oct 11, 2019
@gerred
Copy link

gerred commented Oct 11, 2019

I'm in ET. On the Elixir and Kubernetes slacks, but my email is (email redacted) and am 👍 on talking more.

@coryodaniel
Copy link
Owner Author

coryodaniel commented Oct 11, 2019

As I’m rubberducking through this I realized that maybe shipping common YAML stubs for discovery during tests might go a long way.

Maybe even breaking out a K8s.Test app that has the Discovery file driver, dynamic HTTP driver, and some named YAMLs/JSON/configs for all supported k8s versions.

@coryodaniel
Copy link
Owner Author

Dropping links for all the stuff I was gum-flapping about:

@jwietelmann
Copy link

I've been rolling my own code with HTTPoison, and I'm evaluating a switch to k8s right now. FWIW, the loose naming and ability to discover CRDs are what puts this in the running for me (while other libs are a non-starter). In case it helps, this is how I'd frame my own fundamental requirements for a k8 API lib: I want to navigate N clusters and every API and resource in them without knowing ahead of time what versions they are and what extras are installed.

@gerred
Copy link

gerred commented Oct 11, 2019

@coryodaniel we'll need some way to generate out those golden stubs since those aren't published now except by hitting the swagger endpoint.

I like the test discovery stubs. Just dumping a few further thoughts in here. It'd be nice to be able to assert what sort of GVKs I expect. This is a bit leaky with CRD v1beta1 (now that v1 is out) given fields changed between versions, and asserting down to OpenAPIv3 specs seems a little bit ham handed.

@coryodaniel
Copy link
Owner Author

@jwietelmann absolutely agree. That’s what drove me to build this library!

Thinking about it some more, kind of a duh moment. Discovery is necessary because of namespaced resources.

I think the focus should be on making tests a little less bumpy.

The makefile has targets for getting swagger files. We could probably generate the discovery files for any given version.

I think that could be pretty solid for most use cases. For edge cases we could include a mix task that dumps the discovery mocks into your test/support for editing.

@coryodaniel
Copy link
Owner Author

I was thinking a bit more about this, just spit balling here.

I think the work to make testing easier is def worth doing.

As far as the Discovery module, I was thinking that instead of discovery on boot up, replacing with a JIT Discovery module (and cache).

  • first request is made for a given gvk, it is discovered and the naming options and scope are cached1
  • http path is generated as it is now, with the cached details
  • subsequent requests build urls from data in the cache

This would also lower the boot time of the app, I have seen containers take a few seconds to start in 1000+ node clusters.

It would obvi make a small impact on the first request of a given gvk, but its only a single HTTP request to discovery instead of n+2+alot. Currently we make a call to /api and /apis, and then to each resource.

Subresource implentation was tricky in my initial implementatino, need to look into subresources to see if there is anything tricky there with this approach.

Footnotes:

  1. we dont care about .spec and validation because the k8s api will validate the actual body. I think there could be an argument to add a validator middleware down the road.

I think that would support multi-tenant use cases ref'd #44.

@coryodaniel coryodaniel changed the title Evaluate Discovery functionality JIT API Discovery Nov 27, 2019
@coryodaniel
Copy link
Owner Author

This will need to be integrated w/ Request middleware #42 ~K8s.Middleware.Request.ClusterURL

@coryodaniel coryodaniel self-assigned this Nov 28, 2019
@coryodaniel coryodaniel mentioned this issue Nov 28, 2019
4 tasks
@coryodaniel coryodaniel removed this from the 0.5.1 milestone Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants