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

Make Ambassador "Kubernetes-Native" with Custom Resource Definitions (CRD) #482

Closed
adrienjt opened this issue Jun 1, 2018 · 11 comments
Closed
Assignees

Comments

@adrienjt
Copy link

adrienjt commented Jun 1, 2018

I wish I could just kubectl get mappings.

Are you all planning to use CRDs (or even an aggregated API)? https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/custom-resources/

@richarddli
Copy link
Contributor

Yes, this is something we'd like to do. (Ambassador was originally written after TPRs were deprecated and before CRDs existed.) It's a little ways off, though, as we're migrating to Envoy's ADS and v2 config YAML first.

@esmet
Copy link
Contributor

esmet commented Mar 24, 2019

Bumping this to revive the conversation.

@richarddli / @kflynn Would the Ambassador team be open to hearing a design proposal and/or community contribution for this?

I think there are a few things that will need to happen to this work:
a) Create CRD yaml files, include them in the helm chart. extend entrypoint.sh to watch for these new Kinds
a) diagd support for native objects from kubewatch, instead of Service objects with an annotation. Based on a light reading of the code, i think this won't be terribly hard.
c) Expand documentation

@richarddli
Copy link
Contributor

I was pondering #1363 this weekend, and it did make me wonder if it would make sense to:

  1. Support ingress as a resource (this shouldn't be hard; the semantics are fairly limited). This would reduce the "on-ramp" if you will to Ambassador if coming from other ingress controllers. My view on this has changed given the consensus in the community to formalize the ingress spec out of beta as-is.
  2. Use CRDs to give access to the full feature set of Ambassador.

@esmet What is your specific motivation for wanting to support CRDs? I think it would be helpful to clearly articulate the end user benefits of the migration (is it just kubectl get mappings and friends? is there something else?) I also think we would need to be clear to end users if we want them to migrate away from mappings on services.

TL;DR. I don't think technically this is hard. I do have questions around what we would recommend as the end UX though and when you should use a CRD versus service annotation.

@esmet
Copy link
Contributor

esmet commented Mar 24, 2019

I think there are a few benefits of using a CRD over annotations. Some of these match to my use case today, some may become useful in the future, so I'm speaking mostly theoretically here.

This list is somewhat quick and off the top of my head. I'd love to continue this conversation on Slack - hit me up whenever.

  • CRDs can be RBAC'd separately from Services. This allows for separation of control for service authors (eg: product developers) and software networking operators (eg: infrastructure devleopers).
  • Ambassador can use label selection on CRDs for ambassador-id, ensuring that kube API load and Ambassador update load scales linearly with the set of matching CRDs, instead of the set of all Services. This would make something like Support for label selection in watt/kubewatch #1292 a first-class design feature, instead of an optimization.
  • I think CRDs can specify their own schema/validation logic, allowing developers to know when their specs are invalid more easily. Since Ambassador's config objects are all-or-nothing, a mistake can lead to an outage, and hopefully we can prevent that by rejecting invalid updates before they land. I'm hand waving here a little but I think this is something we can do more easily with CRDs than with annotations.
  • CRDs match the Kubernetes object model more naturally, and may help prevent confusion. For instance, an annotation on a Service object may refer to any other service, though in practice we typically don't do this. With a CRD, the mapping to Service reference is also explicit, and it's very clear why. With an annotation, it's intuitive that the Service mapping shouldn't be necessary (because the developer almost always means "map to this service", but it is.
  • Competing products prefer CRDs (eg: Istio, Gloo) and I think Ambassador would be even better if it could match this user experience, too.

@esmet
Copy link
Contributor

esmet commented Mar 24, 2019

Here's the validation I had in mind: https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/#validation I admit that I haven't done full research on this yet so I'll probably get to that on Tuesday

@richarddli
Copy link
Contributor

OK, so based on the above, I think you're suggesting a incremental migration path away from Service annotations towards CRDs, then, as the long term goal for everyone, given the advantages above?

@esmet
Copy link
Contributor

esmet commented Mar 24, 2019

Yes, I think so.

A service annotation can be the easy path: write an annotation on a Service that you want a Mapping for, and you don't need to specify the service (implied to be this?)

@richarddli
Copy link
Contributor

It turns out we're going to have to migrate to need to introduce a CRD model with the next iteration of kubewatch (aka watt).

So a good starting point could be to just sketch out how you'd want the CRD to work? We can go back and forth here.

@volatilemolotov
Copy link

This feature would be great especially when you handle services that are deployed via charts and CD. Lets say you have an externally maintained chart (prometheus or something similar) and want to deploy alongside your app using Jenkins. One should edit the prometheus service manually after the deploy (if you want to be consistent with adding your mapping onto services they map to instead of creating an another redundant dummy service to deploy the mapping from CD). CRD would allow just to pack a object of a ambassador.io/Mapping type which has the mapping defined. One could also attach a label (id) and ambassador should have a configurable setting of which labels to seek in order to parse the mapping (effectively replacing ambassador_id)

@kflynn
Copy link
Member

kflynn commented May 9, 2019

A very, very quick test implies that this involves some ResourceFetcher tweaks. Shouldn't be too horrible.

@kflynn kflynn self-assigned this May 17, 2019
@kflynn
Copy link
Member

kflynn commented May 20, 2019

Done in 0.70.0.

@kflynn kflynn closed this as completed May 20, 2019
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

No branches or pull requests

5 participants