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

Decision: How do we implement *Class objects (IngressClass and GatewayClass) #2809

Closed
youngnick opened this issue Aug 18, 2020 · 20 comments
Closed
Assignees
Labels
area/gateway-api Issues or PRs related to the Gateway (Gateway API working group) API. area/ingress Issues or PRs related to the Ingress API. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@youngnick
Copy link
Member

IngressClass (from Ingress v1) and GatewayClass (from service-apis) are both replacements-with-added-functionality for the kubernetes.io/ingress.class annotation. They both have fields that allow for Class-specific configuration to be applied.

Currently, Contour is configured from a "file" - that is, a ConfigMap, and those config parameters can only be overridden by command-line flags (when present). We're explicitly trying to move away from command line flags for Contour, however.

This issue is to discuss and answer the question: What do we do to implementt he two types of *Class object?

Options off the top of my head to get things started:

  • One Contour deployment === One Class, either as currently configured, IngressClass, or GatewayClass. This would mean that you would need to run three (!) Contours to cover annotation, IngressClass, and GatewayClass. This does not seem ideal.
  • One Contour deployment has multiple Class configuration. This is more flexible, but means that there would need to be a configuration hierarchy of some sort, where some parameters could be overridden by *Class params. How would that work?
  • Something else I can't think of right now.

Ideas welcomed!

@youngnick youngnick added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. area/ingress Issues or PRs related to the Ingress API. area/gateway-api Issues or PRs related to the Gateway (Gateway API working group) API. labels Aug 18, 2020
@youngnick youngnick self-assigned this Aug 18, 2020
@jpeach
Copy link
Contributor

jpeach commented Aug 19, 2020

  • One Contour deployment === One Class, either as currently configured, IngressClass, or GatewayClass. This would mean that you would need to run three (!) Contours to cover annotation, IngressClass, and GatewayClass. This does not seem ideal.

If you had 3 Contours, you would also currently need 3 data planes (Envoy deployments). That seems like a lot :)

/cc @danehans

@stevesloka
Copy link
Member

Once we move to the go-control-plane xDS server, we have very easy control over which envoy gets what configuration. The architectural bit is that Contour would need to think about how to process multiple sets of xDS configuration.

You'd also probably want a way to determine what ports each instance of Envoy gets configured with. I know there's some work on an operator happening, might be a nice way to program out what ip each envoy should bind to, etc.

The biggest use-case of this that has come up is folks wanting an internal/external set of Contours running to handle different types of traffic.

@youngnick
Copy link
Member Author

I think there are a number of key decisions:

  • Do we allow a single Contour instance to have multiple *Class objects (perhaps only at most one of each type)?
  • Do we allow a *Class object to carry configuration?
  • How do those decisions interact with building load balancing configuration from different types of object? (For example, annotation ingress-class, IngressClass, and GatewayClass)

To answer the first two questions, I'm leaning towards answering a more fundamental question: What's a Contour really? Is it a Contour deployment and its associated Envoys, a Contour deployment and its associated configuration and associated Envoys, or should we allow for a single Contour deployment to manage multiple sets of Envoys?

If a "Contour" is a deployment and its associated Envoys, that leans us towards using the *Class configuration mechanisms to control any particular Contour's configuration. That means that a *Class is not just an identifier, but also implies a set of configuration options (which need to interact correctly with anything configured per Contour deployment, and the defaults). If we were to do this, it allows:

  • Multiple *Classes that may have different configuration, served out of the same Contour deployment.
  • It also would work well with this use case to have Contour able to select a set of Envoys based on the *Class somehow. This would allow for a single Contour installation to handle the "internal/external" use case.
    However, it means:
  • All config settings would also need to be namespaced by *Class somehow, and this would need to be plumbed all the way down to call sites that actually use a setting. Each place where a setting is used would need to check the *Class of an object to see if that setting was relevant in that case.
  • If we're adding multiple Envoys as destinations, we would need to have a separate DAG per Envoy deployment managed by the single Contour.

If a "Contour" is a deployment, its configuration, and its associated Envoys, that leans us towards not using the *Class configuration mechanism, and the *Class is a pointer to a specific Contour install only. In this case, you may have many classes pointing to a Contour as you wish, but they all will produce the same behavior.
This allows:

  • supporting multiple types of *Class object with a single Contour object. (basically, you can configure the Class however you like, and Contour should be able to figure out that it's relevant for itself).
    And means:
  • a minimal changeset to our current model
  • leans us towards one Contour == one config == one set of Envoys
  • allows a single Contour to accept config for an arbitrary set of resources, as long as a matching Class can be selected somehow.
  • Additional complexity for the "internal/external" use case - there must be two separate deployments with separate Class, deployment, config, and Envoys.

Personally, I incline towards the latter, because it's more in keeping with Contour itself being simple. We can build additional complexity by reusing a simple building block (that is, a Contour deployment, it's associated config, and a set of Envoys), at the cost of needing to run more things.

Additionally, I think in most cases that you really need different very configuration for Contour, you probably would need a separate set of Envoys for safety. The first approach would also allow this, but at the cost of a significant amount of development time and internal complexity for Contour.

@tsaarni
Copy link
Member

tsaarni commented Oct 7, 2020

We have been assuming the latter (Contour & configuration & associated Envoys) in my team, and we are currently using ingress class name as a mechanism to choose which one to address. I think this model is easy to understand as a user, and like @youngnick mentions, it surely keeps Contour itself simpler.

One question though: would moving configuration to *Class necessarily mean a change of the current model? Could there be a policy that says "all *Classes defined for single controller must refer to same parameters resource"? Maybe its a bit inconvenient and difficult to enforce, but I guess having configuration as a Kubernetes resource could have its advantages as well.

@youngnick
Copy link
Member Author

@jpeach and I discussed this some more yesterday, here's a summary. Please correct me if I've missed anything @jpeach!

We're inclining towards the latter model referenced above, more specifically:

  • A GatewayClass describes a collection of Gateways
  • A collection of Gateways must be mergable (defined in service-apis)
  • A collection of Gateways represents an Envoy deployment.
  • An Envoy deployment is controlled by a single Contour deployment, with a single configuration. This config may be sourced from a file/ConfigMap (as today), or we may look at having other options. In any case, there is one configuration per Contour deployment.
  • Contour (this project, projectcontour/contour) will not prescribe anything for the Gateways other than they are mergeable, with maybe some extra restrictions around IP addresses, to be determined.
  • A single Contour deployment may have up to one each of GatewayClass, IngressClass and ingress.class annotation specified, which will share configuration. These configurations will be overlaid (as is the case now), so if you supply an Ingress that serves foo.com/blog, a HTTPProxy that serves foo.com/api, and a HTTPRoute that serves foo.com/, then all of those configs will end up in Envoy, provided the resources have the correct class specified. Exact matches will overwrite in an overlay fashion, probably in the order HTTPProxy overwrites HTTPRoute overwrites Ingress, although this is subject to change.

The key thing here is that this keeps projectcontour/contour as a HTTP/HTTPS ingress controller only - as is currently the case. We will support the slice of the service-apis that supports this use case (that is, HTTPRoute, and some parts of TCPRoute or similar), but projectcontour/contour will never provision or modify the spec of Gateway resources, only update status information on them.

This leaves space for other components (like projectcontour/contour-operator or other layer 3/4 load balancer provider) to provision Envoy installations for projectcontour/contour to configure. We (being the Project Contour organisation) may in the future look at building something to provide greater functionality around layer3/4 load balancing if we see enough user demand, but that functionality will be delivered outside of projectcontour/contour.

In addition, this model allows easy migration forward and backward between resource types - we may work on a tool similar to projectcontour/ir2proxy for converting between resources if there is a demand for it.

I'd like to hear from other interested parties. @stevesloka, @skriss, @danehans, @Miciah, @tsaarni? Anyone else interested in service-apis implementation is more than welcome to contribute here. I think the path forward is to get some consensus on if this proposed model is up to scratch, and, if it is, then we can move forward with both Ingress v1 and service-apis support.

@youngnick
Copy link
Member Author

This issue blocks #2139 and #2287.

@skriss
Copy link
Member

skriss commented Oct 20, 2020

A collection of Gateways represents an Envoy deployment.

Can you give an example of where you'd have one Envoy deployment corresponding to >1 Gateways? I'm not quite following that.

@youngnick
Copy link
Member Author

Gateways have listeners, and certain sets of listeners can be combined, either in the same Gateway, or in multiple gateways.

This theoretically would allow us to expose multiple secure or multiple insecure ports, which people have asked for.

In short, anything you can do in one Gateway by having multiple Listeners, you can do in multiple gateways of one listener each, as they may be merged.

@Miciah
Copy link

Miciah commented Oct 20, 2020

Options off the top of my head to get things started:

  • One Contour deployment === One Class, either as currently configured, IngressClass, or GatewayClass. This would mean that you would need to run three (!) Contours to cover annotation, IngressClass, and GatewayClass. This does not seem ideal.

I'm confused by this statement. Don't the annotation and IngressClass refer to the same thing? That is, aren't kubectl annotate ingresses/bar kubernetes.io/ingress.class=foo and kubectl patch ingresses/bar --patch='{"spec":{"ingressClassName":"foo"}}' two ways of doing the same thing, namely configuring the Ingress named "bar" to use the IngressClass named "foo"?

Presumably IngressClass and GatewayClass are distinct namespaces though (that is, an IngressClass named "foo" and a GatewayClass named "foo" describe distinct things), which makes matters interesting for clusters where users want to use both Ingress v1 and Service APIs. @danehans, should we bring this use-case up for discussion on the next Service APIs call (or has it been addressed previously)?

@danehans
Copy link
Contributor

@Miciah
Copy link

Miciah commented Oct 20, 2020

In short, anything you can do in one Gateway by having multiple Listeners, you can do in multiple gateways of one listener each, as they may be merged.

I think this is changing (that is, Service APIs may change to prohibit merging): kubernetes-sigs/gateway-api#399

@youngnick
Copy link
Member Author

I'm confused by this statement. Don't the annotation and IngressClass refer to the same thing? That is, aren't kubectl annotate ingresses/bar kubernetes.io/ingress.class=foo and kubectl patch ingresses/bar --patch='{"spec":{"ingressClassName":"foo"}}' two ways of doing the same thing, namely configuring the Ingress named "bar" to use the IngressClass named "foo"?

I think that the field may behave that way if you don't create an IngressClass object? I will need to go an check the API again, it's been a while. I think the key part is that in the model I'm proposing, it should be possible to have the same Contour installation deal with either Ingress objects (however their class is determined) and service-apis objects.

@skriss
Copy link
Member

skriss commented Oct 21, 2020

Few comments/questions:

  • Assuming I understand correctly, then the Gateway stuff makes sense to me - it corresponds to an Envoy deployment, not a Contour, and Contour is just 1:1 with an Envoy deployment. It'd be interesting to see if there's room for a generic Envoy operator to manage deployment & lifecycle of it.
  • Supporting a single ingress class per Contour seems reasonable (whether it's specified via annotation or field).
  • It sounds like we're saying that, at least for now, Contour will not look at any config defined in the IngressClass CR, but instead will continue to be configured as it is today, i.e. by config file?
  • I also agree with, at least for now, keeping Contour focused on L7 ingress. Among other reasons, I think trying to increase scope now might spread the current team too thin.

@youngnick
Copy link
Member Author

The maintainers discussed this in a catchup yesterday, and I believe this was the consensus we arrived at:

  • projectcontour/contour will support reading from Gateways in so far as it's required to use the HTTPRoute and TLSRoute objects that form part of service-apis. This continues our current focus on L7 and HTTP/HTTPS. If and when we build something that offers TCPRoute and UDPRoute support, that will be a separate part of the projectcontour Github organisation.
  • A set of gateways ( roughly a GatewayClass) represents an Envoy deployment in our infrastructure model. However, projectcontour/contour will assume that a GatewayClass and associated Gateway(s) are provisioned by "something else". Right now, we have the projectcontour/contour-operator repo in a pre-alpha state, but conceivably this could be used to fill this need in the medium term.
  • For now, we will have the Contour deployment configured only by file/ConfigMap, as today, rather than taking config from the GatewayClass itself. We may reevaluate this decision after we have more real-world experience with the service-apis.
  • With respect to IngressClass, we will also not use the spot in that API for holding configuration.

@jpeach, @skriss, @stevesloka, did I miss anything here? We'll leave this open to give the community a chance to comment, but I propose lazy consensus of one week, after which time, if there are no objections, we'll consider this decision resolved.

@stevesloka
Copy link
Member

If and when we build something that offers TCPRoute and UDPRoute support, that will be a separate part of the projectcontour Github organisation.

To me this is up in the air at the moment as to if it's a new project or just part of Contour, but agree it's an implementation detail for later on. The current focus is on what Contour does today (e.g. HTTP L7).

With respect to IngressClass, we will also not use the spot in that API for holding configuration.

What do you mean by this just to be clear @youngnick?

@jpeach
Copy link
Contributor

jpeach commented Oct 24, 2020

The maintainers discussed this in a catchup yesterday, and I believe this was the consensus we arrived at:

  • projectcontour/contour will support reading from Gateways in so far as it's required to use the HTTPRoute and TLSRoute objects that form part of service-apis. This continues our current focus on L7 and HTTP/HTTPS. If and when we build something that offers TCPRoute and UDPRoute support, that will be a separate part of the projectcontour Github organisation.

This is a reasonable starting point. However, there's no way for several components to program the same Envoy data plane, so we may find that we need to revisit this.

  • A set of gateways ( roughly a GatewayClass) represents an Envoy deployment in our infrastructure model. However, projectcontour/contour will assume that a GatewayClass and associated Gateway(s) are provisioned by "something else". Right now, we have the projectcontour/contour-operator repo in a pre-alpha state, but conceivably this could be used to fill this need in the medium term.

I agree that a GatewayClass corresponds to a Envoy data plane deployment. I think the missing piece here is a controller to associate the data plane with the class. That is - you need to be able to deploy a data plane, then later on attach a Contour controller to the corresponding gateway class. The Envoy pods would automatically be reconfigured to connect to the Contour that is attached to the class.

  • For now, we will have the Contour deployment configured only by file/ConfigMap, as today, rather than taking config from the GatewayClass itself. We may reevaluate this decision after we have more real-world experience with the service-apis.
  • With respect to IngressClass, we will also not use the spot in that API for holding configuration.

Yep, I think we can make it work.

@jpeach, @skriss, @stevesloka, did I miss anything here? We'll leave this open to give the community a chance to comment, but I propose lazy consensus of one week, after which time, if there are no objections, we'll consider this decision resolved.

+1

@youngnick
Copy link
Member Author

youngnick commented Oct 25, 2020

With respect to IngressClass, we will also not use the spot in that API for holding configuration.

What do you mean by this just to be clear @youngnick?

There is a parameters field in the IngressClass object that is a TypedLocalObjectReference - I'm proposing that we do not use that at the moment, and stick with our current configured-by-file/ConfigMap setup.

@skriss
Copy link
Member

skriss commented Oct 26, 2020

SGTM. Agree that we don't yet need to make a decision on where TCPRoute/UDPRoute code would live if we decide to built it.

@youngnick
Copy link
Member Author

Okay, it's been a little more than a week, so I'm going to mark this as decided, and close it. This means that we can unblock both the service-apis work and the Ingress V1 work.

@danehans
Copy link
Contributor

danehans commented Jan 6, 2021

xref: #3213

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/gateway-api Issues or PRs related to the Gateway (Gateway API working group) API. area/ingress Issues or PRs related to the Ingress API. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

No branches or pull requests

7 participants