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

Signed-off-by: Marwan Ahmed <[email protected]>
Signed-off-by: Adrien Trouillaud <[email protected]>
Co-authored-by: Adrien Trouillaud <[email protected]>

docs/user_guide/pod_configuration.md

@@ -0,0 +1,19 @@
+---
+title: Pod Configuration
+custom_edit_url: https://github.com/admiraltyio/admiralty/edit/master/docs/user_guide/pod_configuration.md
+---
+
+
+## Label Prefixing
+Admiralty prefixes delegate pod labels with `multicluster.admiralty.io/` in the target clusters. This behavior is useful in bursting cluster topologies, where the source cluster is also a target cluster, so as not to confuse controllers of proxy pods, e.g., replicasets. The behavior
+can be overridden per pod through the `multicluster.admiralty.io/no-prefix-label-regexp` annotation. This is useful to 
+support components that have functionality that relies on pod labels. For example, webhooks or monitoring resources.
+
+
+:::tip
+One use-case is to prevent prefixing the Kueue queue name label. This can be achieved through:
+```
+multicluster.admiralty.io/no-prefix-label-regexp="^kueue\.x-k8s\.io\/queue-name"
+```
+:::
+

pkg/common/constants.go

@@ -40,6 +40,10 @@ var (
 
 	AnnotationKeyUseConstraintsFromSpecForProxyPodScheduling = KeyPrefix + "use-constraints-from-spec-for-proxy-pod-scheduling"
 
+	// AnnotationNoPrefixLabelRegexp defines a regex that when matched on labels, the label
+	// gets copied as-is to the delegate pod without appending KeyPrefix prefix
+	AnnotationNoPrefixLabelRegexp = KeyPrefix + "no-prefix-label-regexp"
+
 	// annotations on proxy pods (by mutating admission webhook)
 
 	KeyPrefixSourcePod = KeyPrefix + "sourcepod-"

pkg/controllers/follow/service/controller.go

@@ -181,7 +181,10 @@ func (r reconciler) Handle(obj interface{}) (requeueAfter *time.Duration, err er
 			svcCopy.Annotations[common.AnnotationKeyOriginalSelector] = originalSelector
 		}
 		if r.serviceRerouteEnabled {
-			selector, changed := delegatepod.ChangeLabels(svcCopy.Spec.Selector)
+			selector, changed, err := delegatepod.ChangeLabels(svcCopy.Spec.Selector, svc.Annotations[common.AnnotationNoPrefixLabelRegexp])
+			if err != nil {
+				return nil, fmt.Errorf("failed to change labels for mirrored service %s: %v", svcCopy.Name, err)
+			}
 			if changed {
 				needUpdateLocal = true
 				svcCopy.Spec.Selector = selector

pkg/model/delegatepod/model.go

@@ -17,6 +17,8 @@
 package delegatepod
 
 import (
+	"fmt"
+	"regexp"
 	"strings"
 
 	corev1 "k8s.io/api/core/v1"
@@ -48,8 +50,10 @@ func MakeDelegatePod(proxyPod *corev1.Pod, clusterName string) (*v1alpha1.PodCha
 		}
 	}
 
-	labels, _ := ChangeLabels(srcPod.Labels)
-
+	labels, _, err := ChangeLabels(srcPod.Labels, srcPod.Annotations[common.AnnotationNoPrefixLabelRegexp])
+	if err != nil {
+		return nil, fmt.Errorf("failed to change labels for proxy pod %s: %v", proxyPod.Name, err)
+	}
 	delegatePod := &v1alpha1.PodChaperon{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace:    proxyPod.Namespace, // already defaults to "default" (vs. could be empty in srcPod)
@@ -90,11 +94,20 @@ func MakeDelegatePod(proxyPod *corev1.Pod, clusterName string) (*v1alpha1.PodCha
 // The name segment is required and must be 63 characters or less"
 // https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set
 // TODO: resolve conflict two keys have same name but different prefixes
-func ChangeLabels(labels map[string]string) (map[string]string, bool) {
+func ChangeLabels(labels map[string]string, noPrefixLabelRegex string) (map[string]string, bool, error) {
 	changed := false
 	newLabels := make(map[string]string)
+	re, err := regexp.Compile(noPrefixLabelRegex)
+	if err != nil {
+		return nil, false, fmt.Errorf("failed to complie regexp %s: %v", noPrefixLabelRegex, err)
+	}
+
 	for k, v := range labels {
 		keySplit := strings.Split(k, "/") // note: assume no empty key (enforced by Kubernetes)
+		if len(noPrefixLabelRegex) > 0 && re.MatchString(fmt.Sprintf("%s=%s", k, v)) {
+			newLabels[k] = v
+			continue
+		}
 		if len(keySplit) == 1 || keySplit[0] != common.KeyPrefix {
 			changed = true
 			newKey := common.KeyPrefix + keySplit[len(keySplit)-1]
@@ -103,7 +116,7 @@ func ChangeLabels(labels map[string]string) (map[string]string, bool) {
 			newLabels[k] = v
 		}
 	}
-	return newLabels, changed
+	return newLabels, changed, nil
 }
 
 func removeServiceAccount(podSpec *corev1.PodSpec) {

pkg/model/delegatepod/model_test.go

@@ -0,0 +1,109 @@
+/*
+ * Copyright The Multicluster-Scheduler Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package delegatepod
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestChangeLabels(t *testing.T) {
+	tests := []struct {
+		name               string
+		inputLabels        map[string]string
+		noPrefixLabelRegex string
+		outputLabels       map[string]string
+		expectedErr        error
+	}{
+		{
+			name: "mix of keys, no skip",
+			inputLabels: map[string]string{
+				"foo.com/bar":                   "a",
+				"baz.com/foo":                   "b",
+				"multicluster.admiralty.io/baz": "c",
+				"a":                             "d",
+			},
+			noPrefixLabelRegex: "",
+			outputLabels: map[string]string{
+				"multicluster.admiralty.io/bar": "a",
+				"multicluster.admiralty.io/foo": "b",
+				"multicluster.admiralty.io/baz": "c",
+				"multicluster.admiralty.io/a":   "d",
+			},
+		},
+		{
+			name: "skip multiple keys",
+			inputLabels: map[string]string{
+				"foo.com/bar":                   "a",
+				"baz.com/foo":                   "b",
+				"multicluster.admiralty.io/foo": "c",
+				"a":                             "d",
+			},
+			noPrefixLabelRegex: "foo.com/bar|baz.com/foo",
+			outputLabels: map[string]string{
+				"foo.com/bar":                   "a",
+				"baz.com/foo":                   "b",
+				"multicluster.admiralty.io/foo": "c",
+				"multicluster.admiralty.io/a":   "d",
+			},
+		},
+		{
+			name: "skip key/value pair",
+			inputLabels: map[string]string{
+				"foo.com/bar": "a",
+				"baz.com/foo": "b",
+				"a":           "d",
+			},
+			noPrefixLabelRegex: "^a=d$",
+			outputLabels: map[string]string{
+				"multicluster.admiralty.io/bar": "a",
+				"multicluster.admiralty.io/foo": "b",
+				"a":                             "d",
+			},
+		},
+		{
+			name: "skip value",
+			inputLabels: map[string]string{
+				"foo.com/bar": "skip",
+			},
+			noPrefixLabelRegex: "^.*=skip$",
+			outputLabels: map[string]string{
+				"foo.com/bar": "skip",
+			},
+		},
+		{
+			name: "invalid regex lookahead",
+			inputLabels: map[string]string{
+				"foo.com/bar": "a",
+			},
+			noPrefixLabelRegex: "?!foo",
+			outputLabels:       nil,
+			expectedErr:        errors.New("invalid regex"),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			out, _, err := ChangeLabels(tt.inputLabels, tt.noPrefixLabelRegex)
+			if tt.expectedErr != nil {
+				require.Error(t, err)
+			}
+			require.Equal(t, tt.outputLabels, out)
+		})
+	}
+}