Always render LINKERD2_PROXY_INBOUND_PORTS env var in install/inject output#7893
Always render LINKERD2_PROXY_INBOUND_PORTS env var in install/inject output#7893kleimkuhler merged 3 commits intomainfrom
LINKERD2_PROXY_INBOUND_PORTS env var in install/inject output#7893Conversation
Signed-off-by: Kevin Leimkuhler <[email protected]>
Signed-off-by: Kevin Leimkuhler <[email protected]>
kleimkuhler
changed the title
LINKERD2_PROXY_INBOUND_PORTS env var in install/inject output
mateiidavid
left a comment
mateiidavid
left a comment
There was a problem hiding this comment.
Think the tests will be ok once PRs rebased :D
Do you think an integration test here would be useful to make sure pods that do not expose container pods get injected properly and spin up? Think it would be straightforward if we do it as part of the inject test
| valueFrom: | ||
| fieldRef: | ||
| fieldPath: status.podIPs | ||
| {{ if .Values.proxy.podInboundPorts -}} |
There was a problem hiding this comment.
When we list the environment variables, LINKERD2_PROXY_INBOUND_PORTS will not have a corresponding value. I wonder if that's going to be a bit confusing down the line. When debugging, for example, it might be weird not to have a value associated at all with the variable, we wouldn't know whether this is expected behaviour or the injector/values were bad.
- name: LINKERD2_PROXY_INBOUND_PORTS
- name: LINKERD2_PROXY_DESTINATION_PROFILE_SUFFIXES
value: svc.cluster.local.
- name: LINKERD2_PROXY_INBOUND_ACCEPT_KEEPALIVE
value: 10000ms
- name: LINKERD2_PROXY_OUTBOUND_CONNECT_KEEPALIVE
value: 10000ms
- name: LINKERD2_PROXY_INBOUND_PORTS_DISABLE_PROTOCOL_DETECTION
value: 25,587,3306,4444,5432,6379,9300,11211
Would it make sense to have an empty string as a value?
- name: LINKERD2_PROXY_INBOUND_PORTS
value: ""
I'm not sure if we can do this through the templating functions or we'd have to do it in the injector, not sure if k8s itself will strip an empty value but wdyt?
There was a problem hiding this comment.
When we list the environment variables, LINKERD2_PROXY_INBOUND_PORTS will not have a corresponding value.
AFAICT, this change does exactly what you propose by removing the guard around setting the environment variable. So it should be set and empty if there's an empty podInboundPorts. Or are you referring to something else?
There was a problem hiding this comment.
Yeah, sorry, I was referring to seeing the value has been explictly set to empty, e.g
- name: LINKERD2_PROXY_INBOUND_PORTS
value: ""
instead of
- name: LINKERD2_PROXY_INBOUND_PORTS
but I think if the variable is set to an empty value then it'll be stripped in the output? It's not a blocker either way, was just curious if this would make it more clear that it's been intentionally set to empty.
…d-ports Signed-off-by: Kevin Leimkuhler <[email protected]>
4 participants
Closes #7816.
With this change, the
LINKERD2_PROXY_INBOUND_PORTSis always rendered in install/inject output. This means that if a workload does not expose any ports, then the env var is rendered as the empty string.Coupled with linkerd/linkerd2-proxy#1478, no error is printed upon proxy startup.
Signed-off-by: Kevin Leimkuhler [email protected]