Skip to content

Commit

Permalink
refactor: only rewrite release label
Browse files Browse the repository at this point in the history
  • Loading branch information
FabianKramm committed Aug 6, 2024
1 parent f36632b commit 291c83d
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 29 deletions.
2 changes: 1 addition & 1 deletion pkg/controllers/resources/services/syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/resources/services/translate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ""
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/controllers/servicesync/servicesync.go
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/filters/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
21 changes: 11 additions & 10 deletions pkg/util/translate/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ import (
)

var translateLabels = map[string]bool{
// rewrite app & release
VClusterAppLabel: true,
// rewrite release
VClusterReleaseLabel: true,

// namespace, marker & controlled-by
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
61 changes: 48 additions & 13 deletions pkg/util/translate/translate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand All @@ -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)
Expand All @@ -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",
Expand All @@ -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)

Expand All @@ -139,7 +174,7 @@ func TestLabelSelector(t *testing.T) {
MatchLabels: map[string]string{
"test": "test",
"test123": "test123",
"app": "vcluster",
"release": "vcluster",
},
}, vMap)

Expand All @@ -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)
}
1 change: 0 additions & 1 deletion pkg/util/translate/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ var (
)

var (
VClusterAppLabel = "app"
VClusterReleaseLabel = "release"
NamespaceLabel = "vcluster.loft.sh/namespace"
MarkerLabel = "vcluster.loft.sh/managed-by"
Expand Down

0 comments on commit 291c83d

Please sign in to comment.