Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add k8s annotations to the Envoy Proxy Service #377

Closed
arkodg opened this issue Sep 12, 2022 · 7 comments · Fixed by #1115
Closed

Add k8s annotations to the Envoy Proxy Service #377

arkodg opened this issue Sep 12, 2022 · 7 comments · Fixed by #1115
Assignees
Labels
area/api API-related issues area/infra-mgr Issues related to the provisioner used for provisioning the managed Envoy Proxy fleet. kind/enhancement New feature or request
Milestone

Comments

@arkodg
Copy link
Contributor

arkodg commented Sep 12, 2022

Description:
Extend the Envoy Proxy API to allow a user to add more k8s annotations to the Envoy Proxy service.

Relates to #368 (comment)

[optional Relevant Links:]

Any extra documentation required to understand the issue.

@arkodg arkodg added the kind/enhancement New feature or request label Sep 12, 2022
@arkodg arkodg added this to the Backlog milestone Sep 12, 2022
@arkodg arkodg added area/api API-related issues area/infra-mgr Issues related to the provisioner used for provisioning the managed Envoy Proxy fleet. labels Sep 12, 2022
@Xunzhuo
Copy link
Member

Xunzhuo commented Sep 12, 2022

assign, quite busy recently, will work on this tomorrow.

@danehans danehans added the help wanted Extra attention is needed label Sep 13, 2022
@danehans
Copy link
Contributor

@Xunzhuo thanks for offering to help with this issue. Since this is a "Backlog" milestone issue, I suggest working on RC-2 issues since they're more pressing.

@danehans
Copy link
Contributor

danehans commented Oct 26, 2022

Relates to #368 (comment)

IMHO this issue is very different from #368. #368 was required because EG needs a mechanism to correlate a Gateway with its managed infra resources, e.g. Service. If the managed resources were in the same ns as the Gateway, an ownerReference could be used and the owning ns/name label would be unneeded.

I'm not sure if passing annotations to the Service is the best approach for solving this problem. I think it can be error-prone as opposed to creating an API. For example, what if LoadBalancer annotations are specified for an internal (ClusterIP) Service or GCP-specific LoadBalancer annotations are specified when EG runs in AWS?

Thoughts @LukeShu @skriss @youngnick @AliceProxy @Xunzhuo?

@ericgoode
Copy link

I have a slightly different use case. I would like to have the Gateway annotate its LoadBalancer service in order to register with external-dns when the gateway is created. Right now I can manually annotate the service after it is created, but then any time a change is made to the Gateway the annotation is removed from the Service.

So there is two ways that this can be approached. Either A) The gateway manages the annotations that are put onto the Service object or B) the Gateway leaves any existing annotations in place when making changes to the Gateway.

"kubectl annotate service envoy-default-eg-64656661 -n envoy-gateway-system "external-dns.alpha.kubernetes.io/hostname=myhostname.com" will annotate the service and external-dns will see this and register the DNS name with the provider.

@danehans
Copy link
Contributor

@ericgoode External DNS supports Gateway API, so I would expect it to work natively with EG. Have you tested the integration yet?

xref: https://github.com/kubernetes-sigs/external-dns/blob/master/docs/tutorials/gateway-api.md

@ericgoode
Copy link

@ericgoode External DNS supports Gateway API, so I would expect it to work natively with EG. Have you tested the integration yet?

xref: https://github.com/kubernetes-sigs/external-dns/blob/master/docs/tutorials/gateway-api.md

Thank you for pointing this out. I have tested it and it works. My use case listed above for this issue is void.

@arkodg arkodg modified the milestones: Backlog, 0.4.0-rc.1 Feb 15, 2023
@arkodg
Copy link
Contributor Author

arkodg commented Mar 2, 2023

We currently have infrastructure defined in the GatewayClass level tied to the EnvoyProxy CRD using paramsRef https://gateway.envoyproxy.io/v0.3.0/api/config_types.html#kubernetesdeploymentspec
So an annotations knob within a services field makes sense to me.
cross linking with kubernetes-sigs/gateway-api#1653 (comment) where @youngnick highlighted that a notion of infrastructure could be added to the Gateway resource.
Prefer if we started off adding to the EnvoyProxy CRD and later incorporated the upstream proposal

arkodg added a commit to arkodg/gateway that referenced this issue Mar 8, 2023
This PR allows the user to add annotations to the
managed Envoy service and well as the Envoy pods using
the EnvoyProxy resource

* Fixes envoyproxy#377

* Closes envoyproxy#648

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
@arkodg arkodg self-assigned this Mar 9, 2023
@arkodg arkodg removed the help wanted Extra attention is needed label Mar 9, 2023
Xunzhuo pushed a commit that referenced this issue Mar 9, 2023
* Allow user to configure annotations on infra

This PR allows the user to add annotations to the
managed Envoy service and well as the Envoy pods using
the EnvoyProxy resource

* Fixes #377

* Closes #648

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* fix spelling

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* elaborate default

Signed-off-by: Arko Dasgupta <arko@tetrate.io>

---------

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api API-related issues area/infra-mgr Issues related to the provisioner used for provisioning the managed Envoy Proxy fleet. kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants