Contour: Address not set on Ingress resource

[nphmuller]

Contour does not set the IP value on the Ingress resource. This value is necessary for services like ExternalDNS to work properly.

Version

The version that https://j.hept.io/contour-deployment-norbac produces. Atm:
docker.io/envoyproxy/envoy-alpine:v1.6.0
gcr.io/heptio-images/contour:master (pointing to b9ede01f025)

Repro

1. Setup Contour in your cluster.
   kubectl apply -f kubectl apply -f https://j.hept.io/contour-deployment-norbac
2. Deploy a simple service with Ingress rules.
   kubectl apply -f https://gist.githubusercontent.com/nphmuller/1c980c72e2119b1482e19eb619def08f/raw/10331f4bbc18abe36a0c8c841bf70ab04a38eb92/service-with-ingress.yaml
3. (A) kubectl get ingress nginx
4. (B) kubectl get ingress nginx -o yaml

Expected

A. ADDRESS column should have external IP of Contour load balancer service, but is empty.
B. status should have value:

status:
  loadBalancer:
    ingress:
    - ip: %External IP-of-Contour-LoadBalancer-service%

but has value:

status:
  loadBalancer: {}

[davecheney]

Yup, this is a known limitation and a feature request that keeps getting bumped from release to release. Please don’t close his issue, I’ll close the other as a duplicate as you have put more details in this one.

On 18 May 2018, at 23:41, Nick [e-mail] wrote:

Contour does not set the IP value on the Ingress resource. This value is necessary for services like ExternalDNS to work properly.

Version

The version that https://j.hept.io/contour-deployment-norbac produces. Atm:
docker.io/envoyproxy/envoy-alpine:v1.6.0
gcr.io/heptio-images/contour:master (pointing to b9ede01f025)

Repro

Setup Contour in your cluster.
kubectl apply -f kubectl apply -f https://j.hept.io/contour-deployment-norbac
Deploy a simple service with Ingress rules.
kubectl apply -f https://gist.githubusercontent.com/nphmuller/1c980c72e2119b1482e19eb619def08f/raw/10331f4bbc18abe36a0c8c841bf70ab04a38eb92/service-with-ingress.yaml
(A) kubectl get ingress nginx
(B) kubectl get ingress nginx -o yaml
Expected

A. ADDRESS column should have external IP of Contour load balancer service, but is empty.
B. status should have value:

status:
loadBalancer:
ingress:
- ip: %External IP-of-Contour-LoadBalancer-service%
but has value:

status:
loadBalancer: {}
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[piclemx]

@davecheney do you know if this will be done soon?

[davecheney]

I’m sorry this won’t make it for 0.6. I know it is important but resources are limited.

On 1 Jun 2018, at 23:21, Alexandre Picard-Lemieux [e-mail] wrote:

@davecheney do you know if this will be done soon?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[nphmuller]

@davecheney Still appreciate all the effort! 👍

[davecheney]

This should be possible to cover as part of the IngressRoute refactor. No ETA on a milestone.

[vorozhko]

Hi @davecheney,
do you have any update on the issue?

I am using external-dns with Ingress and it require ADDRESS field to be set otherwise it will not create DNS records.

[davecheney]

I’m sorry I don’t have an eta when this issue will be worked on.

[davecheney]

I'm sorry to report that at this stage in the development freeze without a design we won't be able to deliver this for rc.2. Moving to the backlog for prioritisation after Contour 1.0

[stevesloka]

I was going to pick this up, my thought here was to do the following:

1. Contour watches for all Services right now, when it finds service with an external LB attached, then look to see if that matches the namespace where Contour is deployed. This will be the IP/DNS name that should get attached to any Ingress object.

This assumption also assumes there's only one service in the namespace where Contour is deployed that is type LoadBalancer. We could also filter on the service name envoy, but not sure if that's too specific.

The status should also only get updated if the ingress class matches the Ingress resource.

[pschulten]

@stevesloka Thanks for picking this up. My current nginx-ingress has three different services of type LB for three different ACM certs and different subnets in the same namespace, would I then need three different namespaces and Contour deployments?

[stevesloka]

The problem I'm trying to think through @pschulten is contour is deployed in a "split" model by default, meaning Contour is in a deployment separate from Envoy in a daemonset. I can't think of a good way to tie the two together with other than by convention (i.e. in the namespace). I don't think we should add a flag to Contour to tell what the envoy service name is.

In your example, we'd have three services type LoadBalancer, how would I know which one matched which instance of Contour?

If we were deployed to the same pod this wouldn't be a problem.

[youngnick]

As much as I don't like the idea either, I can't think of another way to address this problem without adding flags, due to the separated deployment model. For Contour to be able to update the Ingress object per the spec, we have to know what to update it with. Something similar will probably be necessary for HTTPProxy as well.

[rochacon]

I also believe a new flag/config would be a more appropriate approach for this. I'm not sure if Contour should try to "guess" the Envoy service to get the address from. Adding the flag can make this behavior more precise/explicit and we can keep the current "no address" behavior as the default (empty value) since we have reached 1.0 (great job btw 🎉).

@pschulten I don't know the details about the LB split, but if you need isolation you should definitely split your Contour deployments per LB. Users, specially malicious ones, can ignore the certificate validation and force a Host header to reach services that were supposed to be protected by the subnet/lb split. Again, I don't know the details but is something to keep in mind.
Also, if all you need is ACM certificate split, an externally managed (i.e. Terraform) AWS ALB might be a better and cheaper solution for you.

[jpeach]

@stevesloka Probably worth doing a small prior art survey to see how other ingress controllers solve this problem. IIRC ingress-nginx uses a command flag.

[youngnick]

I'm picking this up now that Dave has left the team.

The current design of this has taken a lot of cues from the nginx-ingress-controller, speficially the --update-status, --publish-service, and --publish-status-address, see the command line reference.

Contour's current design is as follows:

• there is a new IngressStatusWriter goroutine added, that watches a channel for status updates, and writes them back to the apiserver (once it is elected leader).
• There is a new Service Load Balancer watcher that, in the example deployment, watches the envoy service in ProjectContour for the load balancer address, and sends it down the channel to IngressStatusWriter. This is configured with via command-line flags, with defaults to the example deployment.

Todo:

• [ ] command line flags that allow specification of a manual address or domain name. (#2387)
• [x] Handle ingress class correctly ( OnUpdate in the LoadBalancer status watcher amongst others) (#2388)

Done:

• in-cluster manual testing to try and find any flakiness with the current method

[youngnick]

Just to provide an update aside from commit messages:

The main part of this work is complete for the example deployment - Contour will now look for the Envoy service and expose the IP from its Loadbalancer-type Service in Ingress records that are in scope.

The last piece is to complete #2387, allowing manual specification of the details rather than pulling them from a Service.