Adds Network Publishing API

[danehans]

Updates the Contour API type to support network endpoint management.

Note: The API design supports managing Envoy network endpoints with the ability to add Contour network endpoints in the future.

Fixes: #70

Signed-off-by: Daneyon Hansen daneyonhansen@gmail.com

[danehans]

In the future, fields will be added to support config knobs for the different endpoint publishing types. EndpointPublishing will become a union type and Type will be the union discriminator.

[jpeach]

Approving, since this seems like a very reasonable API to me. Not merging yet, since I think the EndpointPublishing nomenclature is confusing and I'd like to give people a chance to suggest alternatives.

[danehans]

@jpeach during yesterday's community meeting we discussed whether it's necessary to manage the network endpoints of Contour. @stevesloka provided a use-case where Contour runs in a separate cluster from the managed Envoys, but we agreed that's not applicable since each cluster would run contour-operator. If Contour network endpoint publishing is not required today, should this API be designed to support the use case in the future. For example:

Current

spec:
  endpointPublishing:
    type: LoadBalancerService # Publishes Envoy's network endpoints using a k8s LB svc.

Here is an example of an API that can support Envoy (dataPlane) and Contour (controlPlane) network endpoint publishing.
Option 1

spec:
  endpointPublishing:
    dataPlane:
      type: LoadBalancerService # Publishes Envoy's network endpoints using a k8s LB svc.
    # future (if needed)
    controlPlane:
      type: LoadBalancerService # Publishes Contour's network endpoints using a k8s LB svc.

Here is an example of an API that abstracts the control and data planes at spec:
Option 2

spec:
  controlPlane:
    endpointPublishing:
      type: LoadBalancerService # Publishes Envoy's network endpoints using a k8s LB svc.
  # future (if needed)
  dataPlane:
    endpointPublishing:
      type: LoadBalancerService # Publishes Envoy's network endpoints using a k8s LB svc.

I anticipate future data-plane and control-plane specific requirements that the API will need to support. I'm beginning to think it makes sense to separate data/control planes at spec. All other spec-level fields will be applicable to both the control and data planes. Thoughts? cc: @stevesloka @skriss @Miciah

[danehans]

I created this sheet to get input from the community for naming the API.

[stevesloka]

nit: Should we drop Service from these names and just call them LoadBalancer, or NodePort to match Kubernetes?

[danehans]

@stevesloka as I mention in #151 (comment), future types may not use a k8s Service resource. I thought it made sense to use types that are backed by a k8s service to include "Service" in the name. Take LoadBalancerService for example, what if a future LoadBalancer type is added that creates a hardware load balancer outside the k8s cluster?

[stevesloka]

Maybe add ClusterIP for those who might need an internal ingress controller not exposed outside the cluster?

[danehans]

@stevesloka I agree that ClusterIP or ClusterIPService will be a supported type. I would like to keep the initial API represented in this PR as minimal as possible and add types as needed. To provide parity with Contour's rendered manifest, this initial API needs to only support LoadBalancerService and NodePortService.

[danehans]

@jpeach @stevesloka although a k8s Service will back the EndpointPublishing types in the initial API, other non Service-based types may be added in the future. For example, HostNetwork would set hostNetwork: true for the Envoy DaemonSet and not create a Service. I agree that Endpoint may cause confusion, so let me see try to figure out a better name. Feel free to provide any naming suggestions in #151 (comment).

[danehans]

@stevesloka @jpeach @Miciah commit 0652869 renames the API to avoid confusion with the k8s Endpoints API. The commit also redesigns the API to support managing Contour network publishing (if the need arises). Here are a few examples of a Contour with the proposed API changes:

A Default Contour

apiVersion: operator.projectcontour.io/v1alpha1
kind: Contour
metadata:
  name: contour-sample
spec: {} # Envoy will use container ports 80/443 and be published using a k8s LB service.

A Contour with a NodePort Service

apiVersion: operator.projectcontour.io/v1alpha1
kind: Contour
metadata:
  name: contour-sample
spec:
  networkPublishing:
    envoy:
      type: NodePortService

A Contour with Non-Standard Container Ports

apiVersion: operator.projectcontour.io/v1alpha1
kind: Contour
metadata:
  name: contour-sample
spec:
  networkPublishing:
    envoy:
      httpContainerPort: 8080
      httpsContainerPort: 8443

[youngnick]

I'm in a similar boat to @jpeach on this one. I think the API itself looks great, but suggest that we reuse the Service APIs naming and go with GatewayPublishing instead.

I can certainly be talked out of that, but I think we're heading that way anyway, so let's just call it the same thing.

[stevesloka]

I think this looks good now pending any other comments (I know there's been a bunch of questions back & forth). =)

[danehans]

Per #171, Contour will be utilized by Service APIs for gatewayclass.spec.parametersRef. Since this PR will be used for provisioning Contour using the Contour CRD or Service APIs, I would like to proceed with the NetworkPublishing API naming scheme. @youngnick @jpeach PTAL and let me know if you have any questions or concerns.

[danehans]

Per projectcontour/contour#3263, commit 9226ee0 updates the NetworkPublishing API to support a list of container ports instead of specifying individual fields for http and https container ports. This commit also updates the default port numbers to match upstream, i.e. 8443 for secure and 8080 for insecure. cc: @youngnick @stevesloka

[stevesloka]

Is the operator going to take on specific implementations (e.g. https://kubernetes.io/docs/concepts/services-networking/service/#internal-load-balancer).

If the goal isn't to take those on then possibly a new ProviderSetting (or something) are a better spot to allow users to define those specific exceptions. We might need both to cover anyone who might need specific annotations but aren't yet supported in the operator.

[danehans]

@stevesloka yes, the operator will support provider-specific LB settings through ProviderParameters, i.e. LB connection tuning. All the major cloud providers support internal/external LB scoping, so I think it's appropriate to define scope outside ProviderParameters.

[stevesloka]

Just one small question and it looks great to me! Thanks for the work on this @danehans! 🎉

I think we should just implement it at this point and see how things shake out while we're still in alpha/beta versions.

[danehans]

@youngnick @stevesloka @skriss do you agree that at least 2 ContainerPorts must be defined to support Envoy's insecure and secure services? I'm unsure if it's possible or if a use-case exists to run the insecure/secure services on the same container port. Do you agree that 6 container ports is an appropriate maximum?

[youngnick]

I think those are totally acceptable numbers for now. We can change this later.

[danehans]

Commit f1fa042 changes MaxItem=2 and godocs for the containerPort field based on feedback from the maintainers call.

[skriss]

nothing to add here, LGTM