loft-sh · FabianKramm · Oct 10, 2024 · Oct 9, 2024
diff --git a/pkg/controllers/resources/configmaps/syncer.go b/pkg/controllers/resources/configmaps/syncer.go
@@ -60,10 +60,8 @@ func (s *configMapSyncer) ModifyController(ctx *synccontext.RegisterContext, bui
 }
 
 func (s *configMapSyncer) SyncToHost(ctx *synccontext.SyncContext, event *synccontext.SyncToHostEvent[*corev1.ConfigMap]) (ctrl.Result, error) {
-	createNeeded, err := s.isConfigMapUsed(ctx, event.Virtual)
-	if err != nil {
-		return ctrl.Result{}, err
-	} else if !createNeeded {
+	createNeeded := s.isConfigMapUsed(ctx, event.Virtual)
+	if !createNeeded {
 		return ctrl.Result{}, nil
 	}
 
@@ -72,7 +70,7 @@ func (s *configMapSyncer) SyncToHost(ctx *synccontext.SyncContext, event *syncco
 	}
 
 	pObj := translate.HostMetadata(event.Virtual, s.VirtualToHost(ctx, types.NamespacedName{Name: event.Virtual.Name, Namespace: event.Virtual.Namespace}, event.Virtual))
-	err = pro.ApplyPatchesHostObject(ctx, nil, pObj, event.Virtual, ctx.Config.Sync.ToHost.ConfigMaps.Patches)
+	err := pro.ApplyPatchesHostObject(ctx, nil, pObj, event.Virtual, ctx.Config.Sync.ToHost.ConfigMaps.Patches)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -87,14 +85,12 @@ func (s *configMapSyncer) SyncToVirtual(ctx *synccontext.SyncContext, event *syn
 	}
 
 	vObj := translate.VirtualMetadata(event.Host, s.HostToVirtual(ctx, types.NamespacedName{Name: event.Host.Name, Namespace: event.Host.Namespace}, event.Host))
-	createNeeded, err := s.isConfigMapUsed(ctx, vObj)
-	if err != nil {
-		return ctrl.Result{}, err
-	} else if !createNeeded {
+	createNeeded := s.isConfigMapUsed(ctx, vObj)
+	if !createNeeded {
 		return ctrl.Result{}, nil
 	}
 
-	err = pro.ApplyPatchesVirtualObject(ctx, nil, vObj, event.Host, ctx.Config.Sync.ToHost.ConfigMaps.Patches)
+	err := pro.ApplyPatchesVirtualObject(ctx, nil, vObj, event.Host, ctx.Config.Sync.ToHost.ConfigMaps.Patches)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -103,12 +99,10 @@ func (s *configMapSyncer) SyncToVirtual(ctx *synccontext.SyncContext, event *syn
 }
 
 func (s *configMapSyncer) Sync(ctx *synccontext.SyncContext, event *synccontext.SyncEvent[*corev1.ConfigMap]) (_ ctrl.Result, retErr error) {
-	used, err := s.isConfigMapUsed(ctx, event.Virtual)
-	if err != nil {
-		return ctrl.Result{}, err
-	} else if !used {
+	used := s.isConfigMapUsed(ctx, event.Virtual)
+	if !used {
 		ctx.Log.Infof("delete physical config map %s/%s, because it is not used anymore", event.Host.GetNamespace(), event.Host.GetName())
-		err = ctx.PhysicalClient.Delete(ctx, event.Host)
+		err := ctx.PhysicalClient.Delete(ctx, event.Host)
 		if err != nil {
 			ctx.Log.Infof("error deleting physical object %s/%s in physical cluster: %v", event.Host.GetNamespace(), event.Host.GetName(), err)
 			return ctrl.Result{}, err
@@ -131,13 +125,12 @@ func (s *configMapSyncer) Sync(ctx *synccontext.SyncContext, event *synccontext.
 		}
 	}()
 
-	// check annotations & labels
-	event.Host.Annotations = translate.HostAnnotations(event.Virtual, event.Host)
-
-	// check labels
+	// bi-directional sync of annotations and labels
 	if event.Source == synccontext.SyncEventSourceHost {
+		event.Virtual.Annotations = translate.VirtualAnnotations(event.Host, event.Virtual)
 		event.Virtual.Labels = translate.VirtualLabels(event.Host, event.Virtual)
 	} else {
+		event.Host.Annotations = translate.HostAnnotations(event.Virtual, event.Host)
 		event.Host.Labels = translate.HostLabels(event.Virtual, event.Host)
 	}
 
@@ -147,11 +140,11 @@ func (s *configMapSyncer) Sync(ctx *synccontext.SyncContext, event *synccontext.
 	return ctrl.Result{}, nil
 }
 
-func (s *configMapSyncer) isConfigMapUsed(ctx *synccontext.SyncContext, vObj *corev1.ConfigMap) (bool, error) {
+func (s *configMapSyncer) isConfigMapUsed(ctx *synccontext.SyncContext, vObj *corev1.ConfigMap) bool {
 	if vObj.Annotations[constants.SyncResourceAnnotation] == "true" {
-		return true, nil
+		return true
 	} else if ctx.Config.Sync.ToHost.ConfigMaps.All {
-		return true, nil
+		return true
 	}
 
 	// retrieve references for config map
@@ -163,5 +156,5 @@ func (s *configMapSyncer) isConfigMapUsed(ctx *synccontext.SyncContext, vObj *co
 		},
 	})
 
-	return len(references) > 0, nil
+	return len(references) > 0
 }
diff --git a/pkg/controllers/resources/csistoragecapacities/syncer.go b/pkg/controllers/resources/csistoragecapacities/syncer.go
@@ -147,7 +147,7 @@ func (s *csistoragecapacitySyncer) enqueuePhysical(ctx *synccontext.SyncContext,
 		return
 	}
 
-	name := s.Mapper.HostToVirtual(ctx, types.NamespacedName{Name: obj.GetName(), Namespace: obj.GetNamespace()}, obj)
+	name := s.HostToVirtual(ctx, types.NamespacedName{Name: obj.GetName(), Namespace: obj.GetNamespace()}, obj)
 	if name.Name != "" && name.Namespace != "" {
 		q.Add(reconcile.Request{NamespacedName: name})
 	}

diff --git a/pkg/controllers/resources/endpoints/syncer.go b/pkg/controllers/resources/endpoints/syncer.go
@@ -82,6 +82,14 @@ func (s *endpointsSyncer) Sync(ctx *synccontext.SyncContext, event *synccontext.
 		return ctrl.Result{}, err
 	}
 
+	if event.Source == synccontext.SyncEventSourceHost {
+		event.Virtual.Annotations = translate.VirtualAnnotations(event.Host, event.Virtual, s.excludedAnnotations...)
+		event.Virtual.Labels = translate.VirtualLabels(event.Host, event.Virtual)
+	} else {
+		event.Host.Annotations = translate.HostAnnotations(event.Virtual, event.Host, s.excludedAnnotations...)
+		event.Host.Labels = translate.HostLabels(event.Virtual, event.Host)
+	}
+
 	return ctrl.Result{}, nil
 }
 

diff --git a/pkg/controllers/resources/endpoints/translate.go b/pkg/controllers/resources/endpoints/translate.go
@@ -49,8 +49,5 @@ func (s *endpointsSyncer) translateUpdate(ctx *synccontext.SyncContext, pObj, vO
 	s.translateSpec(ctx, translated)
 	pObj.Subsets = translated.Subsets
 
-	// check annotations & labels
-	pObj.Annotations = translate.HostAnnotations(vObj, pObj, s.excludedAnnotations...)
-	pObj.Labels = translate.HostLabels(vObj, pObj)
 	return nil
 }
diff --git a/pkg/controllers/resources/ingresses/syncer.go b/pkg/controllers/resources/ingresses/syncer.go
@@ -80,7 +80,7 @@ func (s *ingressSyncer) Sync(ctx *synccontext.SyncContext, event *synccontext.Sy
 
 	event.TargetObject().Spec.IngressClassName = event.SourceObject().Spec.IngressClassName
 	event.Virtual.Status = event.Host.Status
-	s.translateUpdate(ctx, event.Host, event.Virtual)
+	s.translateUpdate(ctx, event.Source, event.Host, event.Virtual)
 	return ctrl.Result{}, nil
 }
 

diff --git a/pkg/controllers/resources/ingresses/translate.go b/pkg/controllers/resources/ingresses/translate.go
@@ -40,13 +40,18 @@ func (s *ingressSyncer) TranslateMetadataUpdate(ctx *synccontext.SyncContext, vO
 	return translate.HostAnnotations(vIngress, pObj), translate.HostLabels(vIngress, pObj)
 }
 
-func (s *ingressSyncer) translateUpdate(ctx *synccontext.SyncContext, pObj, vObj *networkingv1.Ingress) {
+func (s *ingressSyncer) translateUpdate(ctx *synccontext.SyncContext, source synccontext.SyncEventSource, pObj, vObj *networkingv1.Ingress) {
 	pObj.Spec = *translateSpec(ctx, vObj.Namespace, &vObj.Spec)
 
-	var translatedAnnotations map[string]string
-	translatedAnnotations, pObj.Labels = s.TranslateMetadataUpdate(ctx, vObj, pObj)
-	translatedAnnotations, _ = resources.TranslateIngressAnnotations(ctx, translatedAnnotations, vObj.Namespace)
-	pObj.Annotations = translatedAnnotations
+	if source == synccontext.SyncEventSourceHost {
+		vObj.Annotations = translate.VirtualAnnotations(pObj, vObj)
+		vObj.Labels = translate.VirtualLabels(pObj, vObj)
+	} else {
+		var translatedAnnotations map[string]string
+		translatedAnnotations, pObj.Labels = s.TranslateMetadataUpdate(ctx, vObj, pObj)
+		translatedAnnotations, _ = resources.TranslateIngressAnnotations(ctx, translatedAnnotations, vObj.Namespace)
+		pObj.Annotations = translatedAnnotations
+	}
 }
 
 func translateSpec(ctx *synccontext.SyncContext, namespace string, vIngressSpec *networkingv1.IngressSpec) *networkingv1.IngressSpec {

diff --git a/pkg/controllers/resources/namespaces/syncer.go b/pkg/controllers/resources/namespaces/syncer.go
@@ -63,7 +63,7 @@ type namespaceSyncer struct {
 var _ syncertypes.Syncer = &namespaceSyncer{}
 
 func (s *namespaceSyncer) Syncer() syncertypes.Sync[client.Object] {
-	return syncer.ToGenericSyncer[*corev1.Namespace](s)
+	return syncer.ToGenericSyncer(s)
 }
 
 func (s *namespaceSyncer) SyncToHost(ctx *synccontext.SyncContext, event *synccontext.SyncToHostEvent[*corev1.Namespace]) (ctrl.Result, error) {

diff --git a/pkg/controllers/resources/networkpolicies/syncer.go b/pkg/controllers/resources/networkpolicies/syncer.go
@@ -67,6 +67,15 @@ func (s *networkPolicySyncer) Sync(ctx *synccontext.SyncContext, event *synccont
 	}()
 
 	s.translateUpdate(event.Host, event.Virtual)
+
+	if event.Source == synccontext.SyncEventSourceHost {
+		event.Virtual.Annotations = translate.VirtualAnnotations(event.Host, event.Virtual)
+		event.Virtual.Labels = translate.VirtualLabels(event.Host, event.Virtual)
+	} else {
+		event.Host.Annotations = translate.HostAnnotations(event.Virtual, event.Host)
+		event.Host.Labels = translate.HostLabels(event.Virtual, event.Host)
+	}
+
 	return ctrl.Result{}, nil
 }
 

diff --git a/pkg/controllers/resources/networkpolicies/translate.go b/pkg/controllers/resources/networkpolicies/translate.go
@@ -19,9 +19,6 @@ func (s *networkPolicySyncer) translateUpdate(pObj, vObj *networkingv1.NetworkPo
 	if translatedSpec := translateSpec(&vObj.Spec, vObj.GetNamespace()); translatedSpec != nil {
 		pObj.Spec = *translatedSpec
 	}
-
-	pObj.Annotations = translate.HostAnnotations(vObj, pObj)
-	pObj.Labels = translate.HostLabels(vObj, pObj)
 }
 
 func translateSpec(spec *networkingv1.NetworkPolicySpec, namespace string) *networkingv1.NetworkPolicySpec {

diff --git a/pkg/controllers/resources/persistentvolumeclaims/syncer.go b/pkg/controllers/resources/persistentvolumeclaims/syncer.go
@@ -171,17 +171,12 @@ func (s *persistentVolumeClaimSyncer) Sync(ctx *synccontext.SyncContext, event *
 	// allow storage size to be increased
 	event.Host.Spec.Resources.Requests = event.Virtual.Spec.Resources.Requests
 
-	// change annotations
+	// bi-directional sync of annotations and labels
 	if event.Source == synccontext.SyncEventSourceHost {
 		event.Virtual.Annotations = translate.VirtualAnnotations(event.Host, event.Virtual, s.excludedAnnotations...)
-	} else {
-		event.Host.Annotations = translate.HostAnnotations(event.Virtual, event.Host, s.excludedAnnotations...)
-	}
-
-	// check labels
-	if event.Source == synccontext.SyncEventSourceHost {
 		event.Virtual.Labels = translate.VirtualLabels(event.Host, event.Virtual)
 	} else {
+		event.Host.Annotations = translate.HostAnnotations(event.Virtual, event.Host, s.excludedAnnotations...)
 		event.Host.Labels = translate.HostLabels(event.Virtual, event.Host)
 	}
 

diff --git a/pkg/controllers/resources/persistentvolumeclaims/translate.go b/pkg/controllers/resources/persistentvolumeclaims/translate.go
@@ -11,19 +11,18 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 )
 
-var (
-	deprecatedStorageClassAnnotation = "volume.beta.kubernetes.io/storage-class"
-)
+var deprecatedStorageClassAnnotation = "volume.beta.kubernetes.io/storage-class"
 
 func (s *persistentVolumeClaimSyncer) translate(ctx *synccontext.SyncContext, vPvc *corev1.PersistentVolumeClaim) (*corev1.PersistentVolumeClaim, error) {
 	pPVC := translate.HostMetadata(vPvc, s.VirtualToHost(ctx, types.NamespacedName{Name: vPvc.GetName(), Namespace: vPvc.GetNamespace()}, vPvc), s.excludedAnnotations...)
 	s.translateSelector(ctx, pPVC)
 
 	if vPvc.Annotations[constants.SkipTranslationAnnotation] != "true" {
 		if pPVC.Spec.DataSource != nil {
-			if pPVC.Spec.DataSource.Kind == "VolumeSnapshot" {
+			switch pPVC.Spec.DataSource.Kind {
+			case "VolumeSnapshot":
 				pPVC.Spec.DataSource.Name = mappings.VirtualToHostName(ctx, pPVC.Spec.DataSource.Name, vPvc.Namespace, mappings.VolumeSnapshots())
-			} else if pPVC.Spec.DataSource.Kind == "PersistentVolumeClaim" {
+			case "PersistentVolumeClaim":
 				pPVC.Spec.DataSource.Name = mappings.VirtualToHostName(ctx, pPVC.Spec.DataSource.Name, vPvc.Namespace, mappings.PersistentVolumeClaims())
 			}
 		}
@@ -34,9 +33,10 @@ func (s *persistentVolumeClaimSyncer) translate(ctx *synccontext.SyncContext, vP
 				namespace = *pPVC.Spec.DataSourceRef.Namespace
 			}
 
-			if pPVC.Spec.DataSourceRef.Kind == "VolumeSnapshot" {
+			switch pPVC.Spec.DataSourceRef.Kind {
+			case "VolumeSnapshot":
 				pPVC.Spec.DataSourceRef.Name = mappings.VirtualToHostName(ctx, pPVC.Spec.DataSourceRef.Name, namespace, mappings.VolumeSnapshots())
-			} else if pPVC.Spec.DataSourceRef.Kind == "PersistentVolumeClaim" {
+			case "PersistentVolumeClaim":
 				pPVC.Spec.DataSourceRef.Name = mappings.VirtualToHostName(ctx, pPVC.Spec.DataSourceRef.Name, namespace, mappings.PersistentVolumeClaims())
 			}
 		}

diff --git a/pkg/controllers/resources/persistentvolumes/syncer.go b/pkg/controllers/resources/persistentvolumes/syncer.go
@@ -141,7 +141,7 @@ func (s *persistentVolumeSyncer) Sync(ctx *synccontext.SyncContext, event *syncc
 	if event.Virtual.GetDeletionTimestamp() != nil {
 		if event.Host.GetDeletionTimestamp() == nil {
 			// check if the PV is dynamically provisioned and the reclaim policy is Delete
-			if !(event.Virtual.Spec.ClaimRef != nil && event.Virtual.Spec.PersistentVolumeReclaimPolicy == corev1.PersistentVolumeReclaimDelete) {
+			if event.Virtual.Spec.ClaimRef == nil || event.Virtual.Spec.PersistentVolumeReclaimPolicy != corev1.PersistentVolumeReclaimDelete {
 				ctx.Log.Infof("delete physical persistent volume %s, because virtual persistent volume is deleted", event.Host.GetName())
 				err := ctx.PhysicalClient.Delete(ctx, event.Host)
 				if err != nil {
@@ -212,6 +212,13 @@ func (s *persistentVolumeSyncer) Sync(ctx *synccontext.SyncContext, event *syncc
 	if event.Virtual.Annotations[constants.HostClusterPersistentVolumeAnnotation] == "" {
 		// TODO: translate the storage secrets
 		event.Host.Spec.StorageClassName = mappings.VirtualToHostName(ctx, event.Virtual.Spec.StorageClassName, "", mappings.StorageClasses())
+	}
+
+	// bi-directional sync of annotations and labels
+	if event.Source == synccontext.SyncEventSourceHost {
+		event.Virtual.Annotations = translate.VirtualAnnotations(event.Host, event.Virtual, s.excludedAnnotations...)
+		event.Virtual.Labels = translate.VirtualLabels(event.Host, event.Virtual)
+	} else {
 		event.Host.Annotations = translate.HostAnnotations(event.Virtual, event.Host, s.excludedAnnotations...)
 		event.Host.Labels = translate.HostLabels(event.Virtual, event.Host)
 	}

diff --git a/pkg/controllers/resources/persistentvolumes/syncer_test.go b/pkg/controllers/resources/persistentvolumes/syncer_test.go
@@ -49,6 +49,16 @@ func TestSync(t *testing.T) {
 			constants.HostClusterPersistentVolumeAnnotation: "testpv",
 		},
 	}
+	backwardPvObjectMeta := metav1.ObjectMeta{
+		Name: "testpv",
+		Annotations: map[string]string{
+			constants.HostClusterPersistentVolumeAnnotation: "testpv",
+			translate.HostNameAnnotation:                    "testpv",
+			translate.KindAnnotation:                        "/v1, Kind=PersistentVolume",
+			translate.NameAnnotation:                        "testpv",
+			translate.UIDAnnotation:                         "",
+		},
+	}
 	basePvWithDelTSObjectMeta := metav1.ObjectMeta{
 		Name:              "testpv",
 		Finalizers:        []string{"kubernetes"},
@@ -86,7 +96,7 @@ func TestSync(t *testing.T) {
 		},
 	}
 	backwardUpdatePPv := &corev1.PersistentVolume{
-		ObjectMeta: basePvObjectMeta,
+		ObjectMeta: backwardPvObjectMeta,
 		Spec: corev1.PersistentVolumeSpec{
 			ClaimRef:         basePPvcReference,
 			StorageClassName: "someStorageClass",
@@ -176,7 +186,7 @@ func TestSync(t *testing.T) {
 		},
 	}
 	backwardRetainPPv := &corev1.PersistentVolume{
-		ObjectMeta: basePvObjectMeta,
+		ObjectMeta: backwardPvObjectMeta,
 		Spec: corev1.PersistentVolumeSpec{
 			PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimRetain,
 			ClaimRef: &corev1.ObjectReference{
@@ -190,10 +200,12 @@ func TestSync(t *testing.T) {
 		},
 	}
 
-	syncertesting.RunTestsWithContext(t, func(vConfig *config.VirtualClusterConfig, pClient *testingutil.FakeIndexClient, vClient *testingutil.FakeIndexClient) *synccontext.RegisterContext {
+	createContext := func(vConfig *config.VirtualClusterConfig, pClient *testingutil.FakeIndexClient, vClient *testingutil.FakeIndexClient) *synccontext.RegisterContext {
 		vConfig.Sync.ToHost.PersistentVolumes.Enabled = true
 		return syncertesting.NewFakeRegisterContext(vConfig, pClient, vClient)
-	}, []*syncertesting.SyncTest{
+	}
+
+	testCases := []*syncertesting.SyncTest{
 		{
 			Name:                 "Create Backward",
 			InitialVirtualState:  []runtime.Object{basePvc},
@@ -456,5 +468,11 @@ func TestSync(t *testing.T) {
 				assert.NilError(t, err)
 			},
 		},
-	})
+	}
+
+	for _, test := range testCases {
+		t.Run(test.Name, func(t *testing.T) {
+			test.Run(t, createContext)
+		})
+	}
 }
diff --git a/pkg/controllers/resources/poddisruptionbudgets/syncer.go b/pkg/controllers/resources/poddisruptionbudgets/syncer.go
@@ -68,6 +68,15 @@ func (s *pdbSyncer) Sync(ctx *synccontext.SyncContext, event *synccontext.SyncEv
 	}()
 
 	s.translateUpdate(event.Host, event.Virtual)
+
+	if event.Source == synccontext.SyncEventSourceHost {
+		event.Virtual.Annotations = translate.VirtualAnnotations(event.Host, event.Virtual)
+		event.Virtual.Labels = translate.VirtualLabels(event.Host, event.Virtual)
+	} else {
+		event.Host.Annotations = translate.HostAnnotations(event.Virtual, event.Host)
+		event.Host.Labels = translate.HostLabels(event.Virtual, event.Host)
+	}
+
 	return ctrl.Result{}, nil
 }
 

diff --git a/pkg/controllers/resources/poddisruptionbudgets/translate.go b/pkg/controllers/resources/poddisruptionbudgets/translate.go
@@ -14,8 +14,6 @@ func (s *pdbSyncer) translate(ctx *synccontext.SyncContext, vObj *policyv1.PodDi
 }
 
 func (s *pdbSyncer) translateUpdate(pObj, vObj *policyv1.PodDisruptionBudget) {
-	pObj.Annotations = translate.HostAnnotations(vObj, pObj)
-	pObj.Labels = translate.HostLabels(vObj, pObj)
 	pObj.Spec.MaxUnavailable = vObj.Spec.MaxUnavailable
 	pObj.Spec.MinAvailable = vObj.Spec.MinAvailable
 	pObj.Spec.Selector = translate.HostLabelSelector(vObj.Spec.Selector)