Add externalTrafficPolicy support for services (excluding the UI service) #464
Conversation
Fix server-service test - add missing '
| --set 'server.service.externalTrafficPolicy=local' \ | ||
| --set 'server.ha.enabled=true' \ | ||
| . | tee /dev/stderr | | ||
| yq -r '.spec.externalTrafficPolicy | tee /dev/stderr) |
There was a problem hiding this comment.
This is missing a ' and so are the other tests you added here, which is causing failures when running the tests.
There was a problem hiding this comment.
Just committed the change from my phone :)
Add missing '
externalTrafficPolicy is set to local and service type is NodePort otherwise don'tset externalTrafficPolicy
Remove test that isn't necessary - changed documentation instead
| @@ -15,6 +15,11 @@ metadata: | |||
| annotations: | |||
| {{ template "vault.service.annotations" .}} | |||
| spec: | |||
| {{- if and (.Values.server.service.externalTrafficPolicy ) (print .Values.server.service.externalTrafficPolicy | lower | title | eq "Local" )}} | |||
There was a problem hiding this comment.
Seems to be a missing update here.
| @@ -157,3 +157,53 @@ load _helpers | |||
| yq -r '.spec.ports | map(select(.port==8200)) | .[] .name' | tee /dev/stderr) | |||
| [ "${actual}" = "https" ] | |||
| } | |||
|
|
|||
| @test "server/ha-active-Service: vault externalTrafficPolicy set to Local lowercase" { | |||
There was a problem hiding this comment.
I think just testing that the value provided is honoured is good enough here. The two cases are:
- empty string
"" - non empty value like
Cluster
There was a problem hiding this comment.
@benashz So you suggest adding a default value of "Cluster" for instance ?
I'm not handling the case where an empty string isn't provided.
This would probably fall back to K8S defaults (Which currently I'm not sure what is).
There was a problem hiding this comment.
@benashz Just checked - made a new svc with a value not in the correct case - got the following error:
The Service "my-service" is invalid: spec.externalTrafficPolicy: Invalid value: "local": ExternalTrafficPolicy must be empty, Cluster or Local.
So - empty is just fine.
Non empty value would fail if it isn't Local or Cluster.
|
@shayfisher thank for your effort on this PR. I have moved the work to #626 |
No description provided.