ds-hostnet-split example refers to undefined CONTOUR_SERVICE and CONTOUR_PORT variables

[bgagnon]

The variables CONTOUR_SERVICE_HOST and CONTOUR_SERVICE_PORT are not defined anywhere, but used

...in deployment/ds-hostnet-split/03-envoy.yaml:

initContainers:
      - args:
        - bootstrap
        - /config/contour.json
        - --xds-address
        - $(CONTOUR_SERVICE_HOST)
        - --xds-port
        - $(CONTOUR_SERVICE_PORT)
        command:
        - contour
        image: gcr.io/heptio-images/contour:master

...and deployment/ds-hostnet-split/03-contour.yaml:

      containers:
      - args:
        - serve
        - --incluster
        - --xds-address=0.0.0.0
        - --xds-port=$(CONTOUR_SERVICE_PORT)
        - --envoy-service-http-port=80
        - --envoy-service-https-port=443
        command: ["contour"]
        image: gcr.io/heptio-images/contour:master

I've replaced them with contour and 8001, respectively.
Is the idea that these should come from a common ConfigMap?

[stevesloka]

Those variables should be auto-injected by Kubernetes which match the Kubernetes service.

Can you exec into an envoy pod and print out the env vars?

[bgagnon]

Gotcha! Kubernetes does inject those Docker-link inspired variables, provided that the Service exists before the DaemonSet and Deployment, as documented here and here.

In my case I created the objects in a different order and did not have those variables in place when the pods started. Furthermore, having missing/empty strings there was not enough to fail the readiness and liveness probes, which would have otherwise resolved the issue automatically some time after the Service appeared.

Isn't this all too flaky to recommend in the canonical deployment manifests? I've long considered service env vars a legacy feature (kubernetes/kubernetes#60099 and spec.enableServiceLinks on PodSpec) due to the inevitable conflicts and ordering problems they introduce.

[stevesloka]

I agree with you @bgagnon that we should look to move away from these ENV variables. We have the sample split deployment yaml files numbered to get around this issue.

I'll open a new issue to look at resolving this which I'd recommend using the internal dns name of the Contour service, however, last time I tried this there was some issues with Contour resolving the name which is why I went with the env vars.

We can dig into that in the new issue and get this updated.

[davecheney]

Thanks for looking into this @stevesloka

…

On 29 May 2019, at 01:05, Steve Sloka ***@***.***> wrote: Closed #1122. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.