admiraltyio / admiralty

A system of Kubernetes controllers that intelligently schedules workloads across clusters.
https://admiralty.io
Apache License 2.0
673 stars 87 forks source link

feat: allow labels to be copied to delegate pods without prefix #213

Open marwanad opened 1 month ago

marwanad commented 1 month ago

This allows specifying an annotation in a similar fashion to use-constraints-from-spec-for-proxy-pod-scheduling that would allow labels to be copied as is from the proxy pod to the delegate pod without replacing the label key domain with multicluster.admiralty.io/.

The motivation behind this is to enable a multicluster Kueue setup through admiralty. In that setup, admission is gated by ClusterQueues in target clusters. Kueue only operates on pods labeled with kueue.x-k8s.io/queue-name and having admiralty prefix this with multicluster.admiralty.io/ would break this interaction.

adrienjt commented 1 week ago

On my list of ideas, I wanted to implement a simplified mode for cluster topologies where source and target clusters are distinct, in which case prefixing is unnecessary. I like your idea of configuring this at the pod level instead.

What if I want to skip prefixing for all label keys starting with kueue.x-k8s.io/, or skip prefixing for all of the pod's labels, or perhaps skip prefixing based on label values?

For the cluster summary, we have a regexp to exclude node labels from being reconciled (useful on Azure): https://github.com/admiraltyio/admiralty/blob/b704bb7625090af0aacdcf4c1e444d72ce960b8d/pkg/controllers/resources/upstream.go#L187-L196

May I suggest implementing this as a label regexp instead?

annotations:
  no-prefix-label-regexp: ^kueue\.x-k8s\.io/queue-name=

Also, since this is user facing, could you please add a documentation page in the User Guide named Pod Configuration, with a section about label prefixing describing this option, mentioning Kueue as an example use case? We would later expand the page with existing undocumented options (in separate docs-only PRs).