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

EIP operator should look for annotations, not labels #224

Open
benesch opened this issue Jul 18, 2022 · 2 comments
Open

EIP operator should look for annotations, not labels #224

benesch opened this issue Jul 18, 2022 · 2 comments

Comments

@benesch
Copy link
Member

benesch commented Jul 18, 2022

@pH14 suggests that the EIP operator should use annotations rather than labels to identify the pods that need EIPs:

eip.materialize.cloud/manage=true
eip.materialize.cloud/autocreate_eip=true

I think the idea (@pH14 please correct me) is that this better fits with the Kubernetes models. External systems that act on a resource should store their metadata in annotations; only the object's one true owner should set its labels. Context: https://materializeinc.slack.com/archives/CL68GT3AT/p1658153578418489?thread_ts=1658150176.284539&cid=CL68GT3AT

If we make this change, we should take the opportunity to rename autocreate_eip to autocreate-eip to match Kubernetes convention.

@pH14
Copy link
Contributor

pH14 commented Jul 18, 2022

External systems that act on a resource should store their metadata in annotations; only the object's one true owner should set its labels

yep! that sounds right, some prior art here include external-dns and AWS's load balancer controller

@alex-hunt-materialize
Copy link
Collaborator

Can you filter by annotations? We use labels so that we only watch pods with those labels, not all pods. AFAIK, there isn't a way to filter by annotations in a controller's watch.

only the object's one true owner should set its labels.

The eip-operator never writes these labels. It only reads them to know that it needs to care about that pod. The owner of the pod is already the only one writing these labels.

From the docs at https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/ :

Labels can be used to organize and to select subsets of objects.

Labels allow for efficient queries and watches and are ideal for use in UIs and CLIs. Non-identifying information should be recorded using annotations.

We're using eip.materialize.cloud/manage=true for identification, which seems like a textbook label use case. eip.materialize.cloud/autocreate_eip=true could be moved to an annotation, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants