gateway icon indicating copy to clipboard operation
gateway copied to clipboard

Labels or Annotations for Service-to-Gateway Object References

Open danehans opened this issue 3 years ago • 7 comments

https://github.com/envoyproxy/gateway/pull/352 adds gateway owning labels. These labels are applied to the Envoy service and reference the gateway ns/name used to create the service. The Kube provider gateway controller watches services and reconciles the correct gateway, e.g. update gateway.status.addresses when the Envoy service is assigned a load-balancer IP. Labels were chosen instead of annotations based on guidance from the Kubernetes Annotations/Labels docs:

Labels can be used to select objects and to find collections of objects that satisfy certain conditions. In contrast, annotations are not used to identify and select objects. The metadata in an annotation can be small or large, structured or unstructured, and can include characters not permitted by labels.

Should we proceed with Labels or Annotations to solve this problem?

danehans avatar Sep 08 '22 22:09 danehans

Usually, labels are used when you're searching or watching for things though, so ideally we'd put a label on the Gateway(s) as well, and then take a value from the label on the Service (the controllerName string maybe?) and when reconciling a Service, enqueue updates for all the Gateways that might need it.

That's only because there is (possibly) a many-to-one relationship between Gateways and Service though.

youngnick avatar Sep 09 '22 05:09 youngnick

Usually, labels are used when you're searching or watching for things though...

This aligns with the Kube docs for using labels instead of annotations:

Labels can be used to select objects...

We are using labels to select objects, e.g. using the Gateway owning labels to select the Gateway(s) that need status.addresses updated.

and when reconciling a Service, enqueue updates for all the Gateways that might need it.

This sounds like an optimization to #352, filtering events (Services) before enqueuing them. I think this makes sense as a follow-on to #352 since large clusters can have thousands of Services.

danehans avatar Sep 09 '22 15:09 danehans

This is interesting as it touches on something we are looking into. We are trying to pass some annotations from the Gateway to the service that are linked to AWS ELB annotations. For example:

      service.beta.kubernetes.io/aws-load-balancer-name: setme
      service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags: setme
      service.beta.kubernetes.io/aws-load-balancer-internal: "false"
      service.beta.kubernetes.io/aws-load-balancer-scheme: "internet-facing"
      service.beta.kubernetes.io/aws-load-balancer-nlb-target-type: "ip"
      service.beta.kubernetes.io/aws-load-balancer-type: external

The main issue that we are facing is that this default setup without the annotations would create a classic loadbalancer on AWS and we are trying to create an NLB instead. How would you go about resolving this?

zlahham avatar Sep 12 '22 09:09 zlahham

@zlahham https://github.com/envoyproxy/gateway/pull/350 added metadata support to the Infra IR and https://github.com/envoyproxy/gateway/pull/352 provides an example implementation of how these labels can be applied to the Envoy Service.

For your use case, I prefer using the EnvoyProxy API instead of labeling the Gateway. The driving factor for this recommendation is to avoid Gateway annotation sprawl. Thoughts @arkodg @LukeShu @skriss @youngnick?

danehans avatar Sep 12 '22 16:09 danehans

For your use case, I prefer using the EnvoyProxy API instead of labeling the Gateway. The driving factor for this recommendation is to avoid Gateway annotation sprawl. Thoughts @arkodg @LukeShu @skriss @youngnick?

+1 raised https://github.com/envoyproxy/gateway/issues/377 to track it

arkodg avatar Sep 12 '22 17:09 arkodg

@zlahham would you like to work on resolving #377?

danehans avatar Sep 12 '22 17:09 danehans

Yes, I agree with the approach in #377.

It doesn't really matter where the label lives, I guess, and labels rather than annotations is fine.

youngnick avatar Sep 14 '22 05:09 youngnick

I'm closing this issue since owning Gateway ns/name labels were added.

danehans avatar Oct 26 '22 22:10 danehans