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

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

Merged
merged 4 commits into from
Oct 5, 2024

Conversation

marwanad
Copy link
Contributor

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
Copy link
Contributor

adrienjt commented Aug 31, 2024

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):

func (r upstream) reconcileLabels(clusterSummaryLabels map[string]string) map[string]string {
l := virtualnode.BaseLabels(r.target.Namespace, r.target.Name)
for k, v := range clusterSummaryLabels {
regExp := r.compiledExcludedLabelsRegexp
if regExp == nil || !regExp.MatchString(fmt.Sprintf("%s=%s", k, v)) {
l[k] = v
}
}
return l
}

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).

docs/user_guide/pod_configuration.md Outdated Show resolved Hide resolved
@adrienjt adrienjt merged commit f98c59d into admiraltyio:master Oct 5, 2024
1 check passed
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

Successfully merging this pull request may close these issues.

2 participants