deis / router

Edge router for Deis Workflow
https://deis.com
MIT License
80 stars 57 forks source link

Trouble setting service_annotations via chart values file #325

Closed vdice closed 7 years ago

vdice commented 7 years ago

I've recently attempted setting various service_annotations for the router service via a values.yaml -- both via Workflow's global version and Router's. The former errors out while the latter appears successful yet the annotations don't show up in resulting service resource:

Via Workflow's values.yaml:

Relevant snippet:

router:
  dhparam: ""
  # Any custom router annotations(https://github.com/deis/router#annotations)
  # which need to be applied can be specified as key-value pairs under "deployment_annotations"
  deployment_annotations:
    #<example-key>: <example-value>

  # Any custom annotations for k8s services like http://kubernetes.io/docs/user-guide/services/#ssl-support-on-aws
  # which need to be applied can be specified as key-value pairs under "service_annotations"
  service_annotations:
    router.deis.io/nginx.proxyBuffers.number: "10"
    router.deis.io/nginx.proxyBuffers.size: "5k"
    router.deis.io/nginx.proxyBuffers.busySize: "10k"
    router.deis.io/connectTimeout: "35s"

Helm install attempt:

$ helm install --namespace deis --name workflow workflow-dev/workflow -f values.yaml
2017/03/02 14:16:26 warning: destination for service_annotations is a table. Ignoring non-table value <nil>
Error: release workflow failed: Service in version "v1" cannot be handled as a Service: [pos 196]: json: expect char '"' but got char '1'

Via Router's values.yaml:

Relevant snippet:

# Any custom annotations for k8s services like http://kubernetes.io/docs/user-guide/services/#ssl-support-on-aws
# which need to be applied can be specified as key-value pairs under "service_annotations"
service_annotations:
  router.deis.io/nginx.proxyBuffers.number: "10"
  router.deis.io/nginx.proxyBuffers.size: "5k"
  router.deis.io/nginx.proxyBuffers.busySize: "10k"
  router.deis.io/connectTimeout: "35s"

Helm install is successful but when we check out the deis-router service resource we don't see the annotations intended to be set:

$ helm install --namespace deis --name workflow workflow/
...
$ kd get svc deis-router -o yaml
apiVersion: v1
kind: Service
metadata:
  creationTimestamp: 2017-03-02T21:18:17Z
  labels:
    heritage: deis
  name: deis-router
  namespace: deis
  resourceVersion: "19624"
  selfLink: /api/v1/namespaces/deis/services/deis-router
  uid: c0d2148e-ff8d-11e6-ba37-1644177bcf32
spec:
  clusterIP: 10.0.0.240
  ports:
  - name: http
    nodePort: 31264
    port: 80
    protocol: TCP
    targetPort: 8080
  - name: https
    nodePort: 30161
    port: 443
    protocol: TCP
    targetPort: 6443
  - name: builder
    nodePort: 30178
    port: 2222
    protocol: TCP
    targetPort: 2222
  - name: healthz
    nodePort: 31671
    port: 9090
    protocol: TCP
    targetPort: 9090
  selector:
    app: deis-router
  sessionAffinity: None
  type: LoadBalancer
status:
  loadBalancer: {}

Note: some of the attempted annotations are related to PR https://github.com/deis/router/pull/323

Helm version v2.2.1

krancour commented 7 years ago

@vdice... those annotations don't get added to the router's service. They get added to the applications' services.

vdice commented 7 years ago

@krancour thank you for the clarification on that detail. I think we still have an issue, most likely helm and/or templating related, in attempting to set service_annotations via Workflow's global values.yaml (see helm install error in description). Hoping to garner similar experiences and if so can create an actual issue...

krancour commented 7 years ago

@vdice I owe you an apology...

those annotations don't get added to the router's service. They get added to the applications' services.

I don't know what I was thinking when I said that. I was confusing this issue as being wrapped up with something else I was working on...

What I said above simply isn't the case at all.

Sorry about that and I'll look into this.

vdice commented 7 years ago

A practical fix has been issued in #328 and remaining questions have been answered in #323 so closing.