Support a subset of annotations on the service resources

[bigkraig]

This should allow users to specify custom healthchecks per service and resolve #93.

[coveralls]

Coverage decreased (-0.5%) to 20.867% when pulling 7607db2 on service-annotations into 0ddcba1 on master.

[joshrosso]

ingress -> ingress and service

[joshrosso]

This comment should mention it parses all annotations relevant to the service resource.

Also the annotations.ParseAnnotations should likely be refactored to annotations.ParseIngressAnnotations for consistency.

[or] you could combine these 2 functions as they're quite similar. Just needs to be smarter on which annotations to parse based on what's passed. I'm fine either way, as long as the intent of annotations.ParseServiceAnnotations vs annotations.ParseAnnotations is clear.

[joshrosso]

Minor changes requested. Spinning up a test cluster now to do some verification.

[joshrosso]

The only issue i've hit when testing the controller doesn't fallback on the ingress resource as they should.

For example, I've set my healthcheck-port to 8880. In the ingress resource.

$ kubectl get ing -n echoserver echoserver -o yaml
apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  annotations:
    alb.ingress.kubernetes.io/healthcheck-interval-seconds: "30"
    alb.ingress.kubernetes.io/healthcheck-port: "8880"

But in AWS, it's still using the traffic port as seen below.

If I add this setting to the service, it will be respected.

apiVersion: v1
kind: Service
metadata:
  annotations:
    alb.ingress.kubernetes.io/healthcheck-port: '8880'

But after removal from the service resource, even though that setting is still in the ingress resource, it goes back to the default (traffic port).

$ kubectl get -o yaml -n echoserver svc echoserver
apiVersion: v1
kind: Service
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"v1","kind":"Service","metadata":{"annotations":{},"name":"echoserver","namespace":"echoserver"},"spec":{"ports":[{"port":80,"protocol":"TCP","targetPort":8080}],"selector":{"app":"echoserver"},"type":"NodePort"}}
  creationTimestamp: 2017-11-18T00:33:17Z
  name: echoserver
  namespace: echoserver
  resourceVersion: "6811"
  selfLink: /api/v1/namespaces/echoserver/services/echoserver
  uid: 12541ce2-cbf8-11e7-8ebd-0aeebdf18292
spec:

Perhaps this is a small issue inside of the new annotations.Join?

[bigkraig]

Gotta be. That’s what I get for not writing a test function.

…

On November 17, 2017 at 4:59:15 PM, joshrosso ***@***.***) wrote: The only issue i've hit when testing the controller *doesn't* fallback on the ingress resource as they should. For example, I've set my healthcheck-port to 8880. In the ingress resource. $ kubectl get ing -n echoserver echoserver -o yaml apiVersion: extensions/v1beta1 kind: Ingress metadata: annotations: alb.ingress.kubernetes.io/healthcheck-interval-seconds: "30" alb.ingress.kubernetes.io/healthcheck-port: "8880" But in AWS, it's still using the traffic port as seen below. [image: image] <https://user-images.githubusercontent.com/6200057/32975028-02e716d8-cbb8-11e7-8187-04c65072d6b1.png> If I add this setting to the service, it will be respected. apiVersion: v1 kind: Service metadata: annotations: alb.ingress.kubernetes.io/healthcheck-port: '8880' [image: image] <https://user-images.githubusercontent.com/6200057/32975040-34ad91ba-cbb8-11e7-8f9d-23630f37e2f5.png> But after removal from the service resource, even though that setting is still in the ingress resource, it goes back to the default (traffic port). $ kubectl get -o yaml -n echoserver svc echoserver apiVersion: v1 kind: Service metadata: annotations: kubectl.kubernetes.io/last-applied-configuration: | {"apiVersion":"v1","kind":"Service","metadata":{"annotations":{},"name":"echoserver","namespace":"echoserver"},"spec":{"ports":[{"port":80,"protocol":"TCP","targetPort":8080}],"selector":{"app":"echoserver"},"type":"NodePort"}} creationTimestamp: 2017-11-18T00:33:17Z name: echoserver namespace: echoserver resourceVersion: "6811" selfLink: /api/v1/namespaces/echoserver/services/echoserver uid: 12541ce2-cbf8-11e7-8ebd-0aeebdf18292 spec: [image: image] <https://user-images.githubusercontent.com/6200057/32975064-6a96580c-cbb8-11e7-8162-ba1e981e3afd.png> Perhaps this is a small issue inside of the new annotations.Join? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#269 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAfB8xYYsGwvsg76D8d8x1Xqb9Ce0gQYks5s3ivigaJpZM4QirOm> .

[rahulsen]

Why are we not simply specifying the custom healthcheck per targetGroup inside the ingress file itself ? Why the need to use an annotation from the service definition? Wont this be so much cleaner and intuitive like this:

#ingress yaml
apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: myIngress
  namespace: default
  annotations:
    alb.ingress.kubernetes.io/scheme: internal
spec:
  rules:
  - host: myhost.com
    http:
      paths:
      - path: /path1
        backend:
          serviceName: service1-stg
          servicePort: 8080
          healthCheckPath: /path1/health
      - path: /path2
        backend:
          serviceName: service2-stg
          servicePort: 9000
          healthCheckPath: /path2/health

[bigkraig]

@rahulsen agreed, but its not supported in the ingress spec, i know they are looking at extending it