Merge pull request #141 from adrienjt/fix-119

Fix "more than one candidate"

cmd/agent/main.go

@@ -200,12 +200,11 @@ func startOldStyleControllers(
 				targetPodChaperonInformer,
 			),
 			resources.NewUpstreamController(
-				target.GetKey(),
+				target,
 				k,
 				kubeInformerFactory.Core().V1().Nodes(),
 				targetClusterSummaryInformer,
-				nodeStatusUpdaters[target.GetKey()],
-				target.ExcludedLabelsRegexp,
+				nodeStatusUpdaters[target.VirtualNodeName],
 			),
 		)
 	}
@@ -286,7 +285,7 @@ func startWebhook(ctx context.Context, cfg *rest.Config, agentCfg agentconfig.Co
 	utilruntime.Must(err)
 
 	hookServer := webhookMgr.GetWebhookServer()
-	hookServer.Register("/mutate-v1-pod", &webhook.Admission{Handler: proxypod.NewHandler(agentCfg.GetKnownFinalizers())})
+	hookServer.Register("/mutate-v1-pod", &webhook.Admission{Handler: proxypod.NewHandler(agentCfg.GetKnownFinalizersByNamespace())})
 
 	go func() {
 		utilruntime.Must(webhookMgr.Start(ctx))
@@ -296,11 +295,11 @@ func startWebhook(ctx context.Context, cfg *rest.Config, agentCfg agentconfig.Co
 func startVirtualKubeletControllers(ctx context.Context, agentCfg agentconfig.Config, k kubernetes.Interface) map[string]resources.NodeStatusUpdater {
 	nodeStatusUpdaters := make(map[string]resources.NodeStatusUpdater, len(agentCfg.Targets))
 	for _, target := range agentCfg.Targets {
-		n := target.GetKey()
+		t := target
 		p := &node.NodeProvider{}
-		nodeStatusUpdaters[n] = p
+		nodeStatusUpdaters[t.VirtualNodeName] = p
 		go func() {
-			if err := node.Run(ctx, node.Opts{NodeName: n, EnableNodeLease: true}, k, p); err != nil && errors.Cause(err) != context.Canceled {
+			if err := node.Run(ctx, t, k, p); err != nil && errors.Cause(err) != context.Canceled {
 				vklog.G(ctx).Fatal(err)
 			}
 		}()
@@ -312,7 +311,7 @@ func startVirtualKubeletServers(ctx context.Context, agentCfg agentconfig.Config
 	targetConfigs := make(map[string]*rest.Config, len(agentCfg.Targets))
 	targetClients := make(map[string]kubernetes.Interface, len(agentCfg.Targets))
 	for _, target := range agentCfg.Targets {
-		n := target.GetKey()
+		n := target.VirtualNodeName
 		targetConfigs[n] = target.ClientConfig
 		targetClient, err := kubernetes.NewForConfig(target.ClientConfig)
 		utilruntime.Must(err)

pkg/common/constants.go

@@ -72,4 +72,8 @@ var (
 	AnnotationKeyOriginalSelector = KeyPrefix + "original-selector"
 
 	AnnotationKeyRestartedAt = KeyPrefix + "restartedAt"
+
+	LabelKeyTargetNamespace   = KeyPrefix + "target-namespace"
+	LabelKeyTargetName        = KeyPrefix + "target-name"
+	LabelKeyClusterTargetName = KeyPrefix + "cluster-target-name"
 )

pkg/config/agent/agent.go

@@ -1,5 +1,5 @@
 /*
- * Copyright 2021 The Multicluster-Scheduler Authors.
+ * Copyright 2022 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.
@@ -41,7 +41,15 @@ type Config struct {
 func (c Config) GetKnownFinalizers() []string {
 	var knownFinalizers []string
 	for _, target := range c.Targets {
-		knownFinalizers = append(knownFinalizers, target.GetFinalizer())
+		knownFinalizers = append(knownFinalizers, target.Finalizer)
+	}
+	return knownFinalizers
+}
+
+func (c Config) GetKnownFinalizersByNamespace() map[string][]string {
+	knownFinalizers := map[string][]string{}
+	for _, target := range c.Targets {
+		knownFinalizers[target.Namespace] = append(knownFinalizers[target.Namespace], target.Finalizer)
 	}
 	return knownFinalizers
 }
@@ -52,14 +60,13 @@ type Target struct {
 	Self                 bool // optimization to re-use clients, informers, etc.
 	Namespace            string
 	ExcludedLabelsRegexp *string
+	VirtualNodeName      string
+	Finalizer            string
 }
 
-func (t Target) GetKey() string {
-	return name.FromParts(name.Long, []int{0}, []int{1}, "admiralty", t.Namespace, t.Name)
-}
-
-func (t Target) GetFinalizer() string {
-	return common.KeyPrefix + name.FromParts(name.Short, nil, []int{0}, t.Namespace, t.Name)
+func (t *Target) complete() {
+	t.VirtualNodeName = name.FromParts(name.Long, []int{0}, []int{1}, "admiralty", t.Namespace, t.Name)
+	t.Finalizer = common.KeyPrefix + name.FromParts(name.Short, nil, []int{0}, t.Namespace, t.Name)
 }
 
 // until we watch targets at runtime, we can already load them from objects at startup
@@ -114,6 +121,7 @@ func addClusterTarget(ctx context.Context, k *kubernetes.Clientset, agentCfg *Co
 		Self:                 t.Spec.Self,
 		ExcludedLabelsRegexp: t.Spec.ExcludedLabelsRegexp,
 	}
+	c.complete()
 	agentCfg.Targets = append(agentCfg.Targets, c)
 }
 
@@ -142,6 +150,7 @@ func addTarget(ctx context.Context, k *kubernetes.Clientset, agentCfg *Config, t
 		Self:                 t.Spec.Self,
 		ExcludedLabelsRegexp: t.Spec.ExcludedLabelsRegexp,
 	}
+	c.complete()
 	agentCfg.Targets = append(agentCfg.Targets, c)
 }
 

pkg/controllers/feedback/controller.go

@@ -1,5 +1,5 @@
 /*
- * Copyright 2021 The Multicluster-Scheduler Authors.
+ * Copyright 2022 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.
@@ -134,7 +134,7 @@ func (c *reconciler) Handle(obj interface{}) (requeueAfter *time.Duration, err e
 
 	proxyPodTerminating := proxyPod.DeletionTimestamp != nil
 
-	proxyPodHasFinalizer, j := controller.HasFinalizer(proxyPod.Finalizers, c.target.GetFinalizer())
+	proxyPodHasFinalizer, j := controller.HasFinalizer(proxyPod.Finalizers, c.target.Finalizer)
 
 	var candidate *multiclusterv1alpha1.PodChaperon
 	l, err := c.podChaperonsLister.PodChaperons(namespace).List(labels.SelectorFromSet(map[string]string{common.LabelKeyParentUID: string(proxyPod.UID)}))
@@ -151,7 +151,7 @@ func (c *reconciler) Handle(obj interface{}) (requeueAfter *time.Duration, err e
 	didSomething := false
 
 	virtualNodeName := proxypod.GetScheduledClusterName(proxyPod)
-	if proxyPodTerminating || virtualNodeName != "" && virtualNodeName != c.target.GetKey() {
+	if proxyPodTerminating || virtualNodeName != "" && virtualNodeName != c.target.VirtualNodeName {
 		if candidate != nil {
 			if err := c.customclientset.MulticlusterV1alpha1().PodChaperons(namespace).Delete(ctx, candidate.Name, metav1.DeleteOptions{}); err != nil && !errors.IsNotFound(err) {
 				return nil, err
@@ -165,7 +165,7 @@ func (c *reconciler) Handle(obj interface{}) (requeueAfter *time.Duration, err e
 		}
 	}
 
-	if candidate != nil && virtualNodeName == c.target.GetKey() {
+	if candidate != nil && virtualNodeName == c.target.VirtualNodeName {
 		delegate := candidate
 
 		mcProxyPodAnnotations, otherProxyPodAnnotations := common.SplitLabelsOrAnnotations(proxyPod.Annotations)

pkg/controllers/follow/cm.go

@@ -1,5 +1,5 @@
 /*
- * Copyright 2021 The Multicluster-Scheduler Authors.
+ * Copyright 2022 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.
@@ -170,7 +170,7 @@ func (r configMapReconciler) Handle(obj interface{}) (requeueAfter *time.Duratio
 
 	terminating := configMap.DeletionTimestamp != nil
 
-	hasFinalizer, j := controller.HasFinalizer(configMap.Finalizers, r.target.GetFinalizer())
+	hasFinalizer, j := controller.HasFinalizer(configMap.Finalizers, r.target.Finalizer)
 
 	shouldFollow := r.shouldFollow(namespace, name)
 
@@ -238,7 +238,7 @@ func (r configMapReconciler) shouldFollow(namespace string, name string) bool {
 	utilruntime.Must(err)
 	for _, obj := range objs {
 		proxyPod := obj.(*corev1.Pod)
-		if proxypod.GetScheduledClusterName(proxyPod) == r.target.GetKey() {
+		if proxypod.GetScheduledClusterName(proxyPod) == r.target.VirtualNodeName {
 			return true
 		}
 	}
@@ -247,7 +247,7 @@ func (r configMapReconciler) shouldFollow(namespace string, name string) bool {
 
 func (r configMapReconciler) addFinalizer(ctx context.Context, configMap *corev1.ConfigMap) (*corev1.ConfigMap, error) {
 	configMapCopy := configMap.DeepCopy()
-	configMapCopy.Finalizers = append(configMapCopy.Finalizers, r.target.GetFinalizer())
+	configMapCopy.Finalizers = append(configMapCopy.Finalizers, r.target.Finalizer)
 	if configMapCopy.Labels == nil {
 		configMapCopy.Labels = map[string]string{}
 	}

pkg/controllers/follow/ingress/controller.go

@@ -153,7 +153,7 @@ func (r ingressReconciler) Handle(obj interface{}) (requeueAfter *time.Duration,
 
 	terminating := ingress.DeletionTimestamp != nil
 
-	hasFinalizer, j := controller.HasFinalizer(ingress.Finalizers, r.target.GetFinalizer())
+	hasFinalizer, j := controller.HasFinalizer(ingress.Finalizers, r.target.Finalizer)
 
 	shouldFollow := r.shouldFollow(ingress)
 
@@ -256,7 +256,7 @@ func (r ingressReconciler) shouldFollow(ingress *v1.Ingress) bool {
 
 func (r ingressReconciler) addFinalizer(ctx context.Context, ingress *v1.Ingress) (*v1.Ingress, error) {
 	ingressCopy := ingress.DeepCopy()
-	ingressCopy.Finalizers = append(ingressCopy.Finalizers, r.target.GetFinalizer())
+	ingressCopy.Finalizers = append(ingressCopy.Finalizers, r.target.Finalizer)
 	if ingressCopy.Labels == nil {
 		ingressCopy.Labels = map[string]string{}
 	}

pkg/controllers/follow/secret.go

@@ -1,5 +1,5 @@
 /*
- * Copyright 2021 The Multicluster-Scheduler Authors.
+ * Copyright 2022 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.
@@ -169,7 +169,7 @@ func (r secretReconciler) Handle(obj interface{}) (requeueAfter *time.Duration,
 
 	terminating := secret.DeletionTimestamp != nil
 
-	hasFinalizer, j := controller.HasFinalizer(secret.Finalizers, r.target.GetFinalizer())
+	hasFinalizer, j := controller.HasFinalizer(secret.Finalizers, r.target.Finalizer)
 
 	shouldFollow := r.shouldFollow(namespace, name)
 
@@ -233,7 +233,7 @@ func (r secretReconciler) shouldFollow(namespace, name string) bool {
 	utilruntime.Must(err)
 	for _, obj := range objs {
 		proxyPod := obj.(*corev1.Pod)
-		if proxypod.GetScheduledClusterName(proxyPod) == r.target.GetKey() {
+		if proxypod.GetScheduledClusterName(proxyPod) == r.target.VirtualNodeName {
 			return true
 		}
 	}
@@ -242,7 +242,7 @@ func (r secretReconciler) shouldFollow(namespace, name string) bool {
 
 func (r secretReconciler) addFinalizer(ctx context.Context, secret *corev1.Secret) (*corev1.Secret, error) {
 	secretCopy := secret.DeepCopy()
-	secretCopy.Finalizers = append(secretCopy.Finalizers, r.target.GetFinalizer())
+	secretCopy.Finalizers = append(secretCopy.Finalizers, r.target.Finalizer)
 	if secretCopy.Labels == nil {
 		secretCopy.Labels = map[string]string{}
 	}

pkg/controllers/follow/service/controller.go

@@ -1,5 +1,5 @@
 /*
- * Copyright 2021 The Multicluster-Scheduler Authors.
+ * Copyright 2022 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.
@@ -127,7 +127,7 @@ func (r reconciler) Handle(obj interface{}) (requeueAfter *time.Duration, err er
 
 	terminating := svc.DeletionTimestamp != nil
 
-	hasFinalizer, j := controller.HasFinalizer(svc.Finalizers, r.target.GetFinalizer())
+	hasFinalizer, j := controller.HasFinalizer(svc.Finalizers, r.target.Finalizer)
 
 	shouldFollow, originalSelector, err := r.shouldFollow(svc)
 	if err != nil {
@@ -258,7 +258,7 @@ func (r reconciler) shouldFollow(service *corev1.Service) (bool, string, error)
 }
 
 func (r reconciler) addFinalizer(actualCopy *corev1.Service) {
-	actualCopy.Finalizers = append(actualCopy.Finalizers, r.target.GetFinalizer())
+	actualCopy.Finalizers = append(actualCopy.Finalizers, r.target.Finalizer)
 	if actualCopy.Labels == nil {
 		actualCopy.Labels = map[string]string{}
 	}

pkg/controllers/resources/upstream.go

@@ -1,5 +1,5 @@
 /*
- * Copyright 2021 The Multicluster-Scheduler Authors.
+ * Copyright 2022 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.
@@ -23,6 +23,7 @@ import (
 	"regexp"
 	"time"
 
+	"admiralty.io/multicluster-scheduler/pkg/config/agent"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -47,28 +48,27 @@ type NodeStatusUpdater interface {
 }
 
 type upstream struct {
-	targetName string
+	target agent.Target
 
 	kubeclientset kubernetes.Interface
 
 	nodeLister           corelisters.NodeLister
 	clusterSummaryLister listers.ClusterSummaryLister
 	nodeStatusUpdater    NodeStatusUpdater
 
-	excludedLabelsRegexp *regexp.Regexp
+	compiledExcludedLabelsRegexp *regexp.Regexp
 }
 
 func NewUpstreamController(
-	targetName string,
+	target agent.Target,
 	kubeclientset kubernetes.Interface,
 	nodeInformer coreinformers.NodeInformer,
 	clusterSummaryInformer informers.ClusterSummaryInformer,
 	nodeStatusUpdater NodeStatusUpdater,
-	excludedLabelsRegexp *string,
 ) *controller.Controller {
 
 	r := &upstream{
-		targetName:           targetName,
+		target:               target,
 		kubeclientset:        kubeclientset,
 		nodeLister:           nodeInformer.Lister(),
 		clusterSummaryLister: clusterSummaryInformer.Lister(),
@@ -82,21 +82,21 @@ func NewUpstreamController(
 	// so we need to filter here
 	nodeInformer.Informer().AddEventHandler(controller.HandleAddUpdateWith(func(obj interface{}) {
 		node := obj.(*corev1.Node)
-		if node.Name == r.targetName {
+		if node.Name == r.target.VirtualNodeName {
 			c.EnqueueKey(node.Name)
 		}
 	}))
 	clusterSummaryInformer.Informer().AddEventHandler(controller.HandleAllWith(func(_ interface{}) {
-		c.EnqueueKey(targetName)
+		c.EnqueueKey(target.VirtualNodeName)
 	}))
 
-	if excludedLabelsRegexp != nil {
+	if target.ExcludedLabelsRegexp != nil {
 		var err error
-		r.excludedLabelsRegexp, err = regexp.Compile(*excludedLabelsRegexp)
+		r.compiledExcludedLabelsRegexp, err = regexp.Compile(*target.ExcludedLabelsRegexp)
 		if err != nil {
 			// don't crash if regexp cannot be compiled
 			// TODO reject Target at admission
-			utilruntime.HandleError(fmt.Errorf("cannot compile excluded aggregated labels regexp for target %s: %v", targetName, err))
+			utilruntime.HandleError(fmt.Errorf("cannot compile excluded aggregated labels regexp for target %s: %v", target.VirtualNodeName, err))
 		}
 	}
 
@@ -185,9 +185,9 @@ func (r upstream) Handle(key interface{}) (requeueAfter *time.Duration, err erro
 }
 
 func (r upstream) reconcileLabels(clusterSummaryLabels map[string]string) map[string]string {
-	l := virtualnode.BaseLabels()
+	l := virtualnode.BaseLabels(r.target.Namespace, r.target.Name)
 	for k, v := range clusterSummaryLabels {
-		regExp := r.excludedLabelsRegexp
+		regExp := r.compiledExcludedLabelsRegexp
 		if regExp == nil || !regExp.MatchString(fmt.Sprintf("%s=%s", k, v)) {
 			l[k] = v
 		}

pkg/controllers/resources/upstream_test.go

@@ -1,5 +1,5 @@
 /*
- * Copyright 2021 The Multicluster-Scheduler Authors.
+ * Copyright 2022 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.
@@ -20,14 +20,15 @@ import (
 	"regexp"
 	"testing"
 
+	"admiralty.io/multicluster-scheduler/pkg/config/agent"
 	"admiralty.io/multicluster-scheduler/pkg/model/virtualnode"
 	"github.com/stretchr/testify/require"
 )
 
 func Test_upstream_reconcileLabels(t *testing.T) {
 	clusterSummaryLabels := map[string]string{"k1": "v1", "k2": "v2", "prefix.io/k1": "v1"}
 	addBaseLabels := func(m map[string]string) map[string]string {
-		l := virtualnode.BaseLabels()
+		l := virtualnode.BaseLabels("target-namespace", "target-name")
 		for k, v := range m {
 			l[k] = v
 		}
@@ -65,7 +66,8 @@ func Test_upstream_reconcileLabels(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			r := upstream{
-				excludedLabelsRegexp: tt.excludedLabelsRegexp,
+				target:                       agent.Target{Name: "target-name", Namespace: "target-namespace"},
+				compiledExcludedLabelsRegexp: tt.excludedLabelsRegexp,
 			}
 			got := r.reconcileLabels(clusterSummaryLabels)
 			require.Equal(t, tt.want, got)