From 291c83d54461553c51e98af97c05da9558c2e3b0 Mon Sep 17 00:00:00 2001 From: Fabian Kramm Date: Tue, 6 Aug 2024 21:52:19 +0200 Subject: [PATCH] refactor: only rewrite release label --- pkg/controllers/resources/services/syncer.go | 2 +- .../resources/services/translate.go | 2 +- pkg/controllers/servicesync/servicesync.go | 4 +- pkg/server/filters/service.go | 2 +- pkg/util/translate/labels.go | 21 ++++--- pkg/util/translate/translate_test.go | 61 +++++++++++++++---- pkg/util/translate/types.go | 1 - 7 files changed, 64 insertions(+), 29 deletions(-) diff --git a/pkg/controllers/resources/services/syncer.go b/pkg/controllers/resources/services/syncer.go index c621081350..84a426fab7 100644 --- a/pkg/controllers/resources/services/syncer.go +++ b/pkg/controllers/resources/services/syncer.go @@ -137,7 +137,7 @@ func (s *serviceSyncer) Sync(ctx *synccontext.SyncContext, event *synccontext.Sy event.Host.Annotations = updatedAnnotations // translate selector - event.Host.Spec.Selector = translate.HostLabelsMap(event.Virtual.Spec.Selector, event.Host.Spec.Selector, event.Virtual.Namespace) + event.Host.Spec.Selector = translate.HostLabelsMap(event.Virtual.Spec.Selector, event.Host.Spec.Selector, event.Virtual.Namespace, false) return ctrl.Result{}, nil } diff --git a/pkg/controllers/resources/services/translate.go b/pkg/controllers/resources/services/translate.go index 3eb36a49ef..f687ea27cb 100644 --- a/pkg/controllers/resources/services/translate.go +++ b/pkg/controllers/resources/services/translate.go @@ -9,7 +9,7 @@ import ( func (s *serviceSyncer) translate(ctx *synccontext.SyncContext, vObj *corev1.Service) *corev1.Service { newService := translate.HostMetadata(vObj, s.VirtualToHost(ctx, types.NamespacedName{Name: vObj.GetName(), Namespace: vObj.GetNamespace()}, vObj), s.excludedAnnotations...) - newService.Spec.Selector = translate.HostLabelsMap(vObj.Spec.Selector, nil, vObj.Namespace) + newService.Spec.Selector = translate.HostLabelsMap(vObj.Spec.Selector, nil, vObj.Namespace, false) if newService.Spec.ClusterIP != "None" { newService.Spec.ClusterIP = "" } diff --git a/pkg/controllers/servicesync/servicesync.go b/pkg/controllers/servicesync/servicesync.go index 9b9b2bdc7b..96c2f5bec6 100644 --- a/pkg/controllers/servicesync/servicesync.go +++ b/pkg/controllers/servicesync/servicesync.go @@ -155,7 +155,7 @@ func (e *ServiceSyncer) syncServiceWithSelector(ctx context.Context, fromService e.Log.Infof("Add owner reference to host target service %s", to.Name) toService.OwnerReferences = translate.GetOwnerReference(nil) } - toService.Spec.Selector = translate.HostLabelsMap(fromService.Spec.Selector, toService.Spec.Selector, fromService.Namespace) + toService.Spec.Selector = translate.HostLabelsMap(fromService.Spec.Selector, toService.Spec.Selector, fromService.Namespace, false) e.Log.Infof("Create target service %s/%s because it is missing", to.Namespace, to.Name) return ctrl.Result{}, e.To.GetClient().Create(ctx, toService) } else if toService.Labels == nil || toService.Labels[translate.ControllerLabel] != "vcluster" { @@ -165,7 +165,7 @@ func (e *ServiceSyncer) syncServiceWithSelector(ctx context.Context, fromService // rewrite selector targetService := toService.DeepCopy() - targetService.Spec.Selector = translate.HostLabelsMap(fromService.Spec.Selector, toService.Spec.Selector, fromService.Namespace) + targetService.Spec.Selector = translate.HostLabelsMap(fromService.Spec.Selector, toService.Spec.Selector, fromService.Namespace, false) // compare service ports if !apiequality.Semantic.DeepEqual(toService.Spec.Ports, fromService.Spec.Ports) || !apiequality.Semantic.DeepEqual(toService.Spec.Selector, targetService.Spec.Selector) { diff --git a/pkg/server/filters/service.go b/pkg/server/filters/service.go index 70f7f23a75..6058316b0b 100644 --- a/pkg/server/filters/service.go +++ b/pkg/server/filters/service.go @@ -215,7 +215,7 @@ func createService(ctx *synccontext.SyncContext, req *http.Request, decoder enco newService.Annotations = map[string]string{} } newService.Annotations[services.ServiceBlockDeletion] = "true" - newService.Spec.Selector = translate.HostLabelsMap(vService.Spec.Selector, nil, vService.Namespace) + newService.Spec.Selector = translate.HostLabelsMap(vService.Spec.Selector, nil, vService.Namespace, false) err = ctx.PhysicalClient.Create(req.Context(), newService) if err != nil { klog.Infof("Error creating service in physical cluster: %v", err) diff --git a/pkg/util/translate/labels.go b/pkg/util/translate/labels.go index 6def70d0db..567de8668d 100644 --- a/pkg/util/translate/labels.go +++ b/pkg/util/translate/labels.go @@ -8,8 +8,7 @@ import ( ) var translateLabels = map[string]bool{ - // rewrite app & release - VClusterAppLabel: true, + // rewrite release VClusterReleaseLabel: true, // namespace, marker & controlled-by @@ -40,7 +39,7 @@ func VirtualLabel(pLabel string) (string, bool) { return pLabel, true } -func HostLabelsMap(vLabels, pLabels map[string]string, vNamespace string) map[string]string { +func HostLabelsMap(vLabels, pLabels map[string]string, vNamespace string, isMetadata bool) map[string]string { if vLabels == nil { return nil } @@ -57,12 +56,14 @@ func HostLabelsMap(vLabels, pLabels map[string]string, vNamespace string) map[st } } - // check if namespace or cluster-scoped - if vNamespace != "" { - newLabels[MarkerLabel] = VClusterName - newLabels[NamespaceLabel] = vNamespace - } else { - newLabels[MarkerLabel] = Default.MarkerLabelCluster() + // check if we should add namespace and marker label + if isMetadata || pLabels == nil || pLabels[MarkerLabel] != "" { + if vNamespace == "" { + newLabels[MarkerLabel] = Default.MarkerLabelCluster() + } else if Default.SingleNamespaceTarget() { + newLabels[MarkerLabel] = VClusterName + newLabels[NamespaceLabel] = vNamespace + } } // set controller label @@ -201,7 +202,7 @@ func HostLabels(vObj, pObj client.Object) map[string]string { if pObj != nil { pLabels = pObj.GetLabels() } - return HostLabelsMap(vLabels, pLabels, vObj.GetNamespace()) + return HostLabelsMap(vLabels, pLabels, vObj.GetNamespace(), true) } func MergeLabelSelectors(elems ...*metav1.LabelSelector) *metav1.LabelSelector { diff --git a/pkg/util/translate/translate_test.go b/pkg/util/translate/translate_test.go index 0d09185445..255de0cdf1 100644 --- a/pkg/util/translate/translate_test.go +++ b/pkg/util/translate/translate_test.go @@ -10,6 +10,41 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +func TestNotRewritingLabels(t *testing.T) { + pMap := map[string]string{ + "abc": "abc", + "def": "def", + "hij": "hij", + "app": "my-app", + } + + pMap = HostLabelsMap(pMap, pMap, "test", false) + assert.DeepEqual(t, map[string]string{ + "abc": "abc", + "def": "def", + "hij": "hij", + "app": "my-app", + }, pMap) + + pMap = VirtualLabelsMap(pMap, pMap) + assert.DeepEqual(t, map[string]string{ + "abc": "abc", + "def": "def", + "hij": "hij", + "app": "my-app", + }, pMap) + + pMap = HostLabelsMap(pMap, pMap, "test", true) + assert.DeepEqual(t, map[string]string{ + "abc": "abc", + "def": "def", + "hij": "hij", + "app": "my-app", + NamespaceLabel: "test", + MarkerLabel: VClusterName, + }, pMap) +} + func TestAnnotations(t *testing.T) { vObj := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -61,7 +96,7 @@ func TestRecursiveLabelsMap(t *testing.T) { vMaps := []map[string]string{} for i := 0; i <= 10; i++ { vMaps = append(vMaps, maps.Clone(vMap)) - vMap = HostLabelsMap(vMap, nil, fmt.Sprintf("test-%d", i)) + vMap = HostLabelsMap(vMap, nil, fmt.Sprintf("test-%d", i), false) } assert.DeepEqual(t, map[string]string{ @@ -83,15 +118,15 @@ func TestLabelsMap(t *testing.T) { vMap := map[string]string{ "test": "test", "test123": "test123", - "app": "vcluster", - "vcluster.loft.sh/label-suffix-x-a172cedcae": "vcluster123", + "release": "vcluster", + "vcluster.loft.sh/label-suffix-x-a4d451ec23": "vcluster123", } - pMap := HostLabelsMap(vMap, nil, "test") + pMap := HostLabelsMap(vMap, nil, "test", false) assert.DeepEqual(t, map[string]string{ "test": "test", "test123": "test123", - "vcluster.loft.sh/label-suffix-x-a172cedcae": "vcluster", + "vcluster.loft.sh/label-suffix-x-a4d451ec23": "vcluster", MarkerLabel: VClusterName, NamespaceLabel: "test", }, pMap) @@ -103,13 +138,13 @@ func TestLabelsMap(t *testing.T) { "test": "test", "test123": "test123", "other": "other", - "app": "vcluster", - "vcluster.loft.sh/label-suffix-x-a172cedcae": "vcluster123", + "release": "vcluster", + "vcluster.loft.sh/label-suffix-x-a4d451ec23": "vcluster123", }, vMap) - pMap = HostLabelsMap(vMap, pMap, "test") + pMap = HostLabelsMap(vMap, pMap, "test", false) assert.DeepEqual(t, map[string]string{ - "vcluster.loft.sh/label-suffix-x-a172cedcae": "vcluster", + "vcluster.loft.sh/label-suffix-x-a4d451ec23": "vcluster", MarkerLabel: VClusterName, NamespaceLabel: "test", "test": "test", @@ -123,14 +158,14 @@ func TestLabelSelector(t *testing.T) { MatchLabels: map[string]string{ "test": "test", "test123": "test123", - "app": "vcluster", + "release": "vcluster", }, }) assert.DeepEqual(t, &metav1.LabelSelector{ MatchLabels: map[string]string{ "test": "test", "test123": "test123", - "vcluster.loft.sh/label-suffix-x-a172cedcae": "vcluster", + "vcluster.loft.sh/label-suffix-x-a4d451ec23": "vcluster", }, }, pMap) @@ -139,7 +174,7 @@ func TestLabelSelector(t *testing.T) { MatchLabels: map[string]string{ "test": "test", "test123": "test123", - "app": "vcluster", + "release": "vcluster", }, }, vMap) @@ -148,7 +183,7 @@ func TestLabelSelector(t *testing.T) { MatchLabels: map[string]string{ "test": "test", "test123": "test123", - "vcluster.loft.sh/label-suffix-x-a172cedcae": "vcluster", + "vcluster.loft.sh/label-suffix-x-a4d451ec23": "vcluster", }, }, pMap) } diff --git a/pkg/util/translate/types.go b/pkg/util/translate/types.go index 3e4adb98a1..6e9bb8ca01 100644 --- a/pkg/util/translate/types.go +++ b/pkg/util/translate/types.go @@ -13,7 +13,6 @@ var ( ) var ( - VClusterAppLabel = "app" VClusterReleaseLabel = "release" NamespaceLabel = "vcluster.loft.sh/namespace" MarkerLabel = "vcluster.loft.sh/managed-by"