Skip to content
This repository has been archived by the owner on Feb 27, 2023. It is now read-only.

Add ability to add service annotations (towards metallb static ip support) #448

Open
jberger opened this issue Sep 16, 2021 · 9 comments
Open
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@jberger
Copy link

jberger commented Sep 16, 2021

In a previous issue support was added for consuming a loadBalancerIP based on the cloud provider's APIs. While that serves a purpose it is not a generic solution; loadBalancerIP isn't always an output, sometimes it is a useful input. Especially for on-prem loadBalancerIP can be used directly to inform loadbalancer operators like MetalLB what IP addresses should be used. As such having some annotation on the Counter manifest would be much appreciated.

@jberger jberger added the kind/feature Categorizes issue or PR as related to a new feature. label Sep 16, 2021
@youngnick
Copy link
Member

Thanks for this issue @jberger.

It sounds like you want to be able to specify the spec.loadBalancerIP field?

That seems like a reasonable knob to expose in Contour, with the obvious caveat that if your cloud provider / load balancer provider doesn't match up, then it won't work, and it may be tricky for the operator to tell you what happened.

@jberger
Copy link
Author

jberger commented Sep 20, 2021

Yep, that's all I'm asking. Thanks :)

Obviously caveats about provider exist, but the same is true of say PV storage classes etc, you can't select things that aren't possible with your provider, so I think that's consistent.

@youngnick
Copy link
Member

The implementation caveat I need to give you is that we are the process of changing how Contour and the Operator fit together, so this may end up held up behind the "Contour manages the Envoy Service" work. But I definitely agree we should do it.

@Jean-Daniel
Copy link

As a side note, there is also a need to be able to pass arbitrary annotations like metallb.universe.tf/address-pool or metallb.universe.tf/allow-shared-ip

@jamslinger
Copy link

As a side note, there is also a need to be able to pass arbitrary annotations like metallb.universe.tf/address-pool or metallb.universe.tf/allow-shared-ip

I'm running contour-operator alongside MetalLB and it would be really great to have that option. In particular I'm interested in passing metallb.universe.tf/address-pool to get control over what address pool the external IP is chosen from.

As proposed by @jberger, it would also be nice to have the ability to specify an IP manually. However, I'm not sure about spec.loadBalancerIP as that's going to be deprecated while it's encouraged to use implementation-specific annotations for that. In case of MetalLB metallb.universe.tf/loadBalancerIPs has been added recently which allows that.

@jberger
Copy link
Author

jberger commented May 4, 2022

As proposed by @jberger, it would also be nice to have the ability to specify an IP manually. However, I'm not sure about spec.loadBalancerIP as that's going to be deprecated while it's encouraged to use implementation-specific annotations for that. In case of MetalLB metallb.universe.tf/loadBalancerIPs has been added recently which allows that.

That change must have been made just around the time I opened this ticket. By all means, yes, update the ticket to reflect that change upstream.

Actually I would hope that having a CRD field to set service annotations should make the change to contour simpler since that doesn't have a sort of two-way action like a spec field would. Indeed without reading that deprecation ticket I imagine that's what their motivation is too.

@jberger jberger changed the title Add ability to specify loadBalancerIP manually Add ability to add service annotations (towards metallb static ip support) May 4, 2022
@jberger
Copy link
Author

jberger commented May 4, 2022

@skriss
Copy link
Member

skriss commented May 4, 2022

Perhaps it already is possible? https://github.com/projectcontour/contour-operator/blob/main/config/crd/contour/01-crds.yaml#L972-L977

Hey @jberger, what you're seeing there is for the (as yet unreleased) Gateway provisioner, which you can think of as a revamp of the Operator that provisions Contour + Envoy instances based on the Gateway API Gateway resource instead of the Contour CRD. If you want to read more about it, you can check out the (as yet unmerged) updated Gateway API documentation. The v1.21.0-rc.1 manifest for it would be the place to start if you are interested in testing it out.

As far as requesting a static IP for metallb, you actually have 2 different options:

  1. For metallb, simply putting the requested IP in the Gateway's spec.addresses field will work (see https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1alpha2.GatewaySpec) since the Gateway provisioner will use this to populate the Envoy service's spec.loadBalancerIP field, which metallb currently supports.
  2. Alternately, you can specify arbitrary annotations to be placed on the Envoy service, per the CRD spec that you linked above, to provide the IP via metallb annotation, or to request a specific address pool, etc. Details on exactly how you'd configure that are below.

This section of docs describes how to customize your provisioned Contour instances, which is where you'd be able to specify those service annotations. Essentially you'd want something like:

kind: GatewayClass
apiVersion: gateway.networking.k8s.io/v1alpha2
metadata:
  name: contour-with-envoy-service-annotations
spec:
  controllerName: projectcontour.io/gateway-controller
  parametersRef:
    kind: ContourDeployment
    group: projectcontour.io
    name: contour-with-envoy-service-annotations-params
    namespace: projectcontour
---
kind: ContourDeployment
apiVersion: projectcontour.io/v1alpha1
metadata:
  namespace: projectcontour
  name: contour-with-envoy-service-annotations-params
spec:
  envoy:
    networkPublishing:
      serviceAnnotations:
        metallb.universe.tf/address-pool: production-public-ips

And then you'd create a Gateway resource that uses the contour-with-envoy-service-annotations gateway class, which would trigger the Gateway provisioner to spin up a contour+envoy using those annotations on the Envoy service.

It's worth noting that you can still use Ingress or HTTPProxy for routing, even if you use the Gateway provisioner / Gateway resource to provision a Contour, so we hope it's useful for all Contour users, not just those who are using Gateway API for routing.

Final disclaimer - the Gateway provisioner is new, is alpha, and can be expected to have bugs and shortcomings. But if you are interested in testing it out, any/all feedback is welcome!

@jberger
Copy link
Author

jberger commented May 4, 2022

But if you are interested in testing it out, any/all feedback is welcome!

I will be doing so in the upcoming days. I'm deploying a kafka cluster via the banzaicloud operator and it can only do inbound traffic via envoy or istio so I was going to try using contour for it but I remembered this issue from back when I was just playing around with contour for something else and thought I'd check in. Thanks for the reply, I'll let you know how it goes.

https://banzaicloud.com/docs/supertubes/kafka-operator/external-listener/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

5 participants