Merge pull request #2129 from FabianKramm/main

refactor: update instead of patch & more tests

pkg/controllers/resources/csidrivers/syncer_test.go

@@ -25,7 +25,8 @@ func TestSync(t *testing.T) {
 		Name: "test-csidriver",
 	}
 	vObjectMeta := metav1.ObjectMeta{
-		Name: "test-csidriver",
+		Name:            "test-csidriver",
+		ResourceVersion: "999",
 	}
 
 	pObj := &storagev1.CSIDriver{

pkg/controllers/resources/csinodes/syncer_test.go

@@ -22,7 +22,8 @@ func TestSync(t *testing.T) {
 		Name: "test-node",
 	}
 	vObjectMeta := metav1.ObjectMeta{
-		Name: "test-node",
+		Name:            "test-node",
+		ResourceVersion: "999",
 	}
 
 	vNode := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "test-node"}}

pkg/controllers/resources/csistoragecapacities/syncer_test.go

@@ -42,6 +42,7 @@ func TestSyncHostStorageClass(t *testing.T) {
 		Labels: map[string]string{
 			"vcluster.loft.sh/namespace": "test",
 		},
+		ResourceVersion: "999",
 	}
 
 	pObj := &storagev1.CSIStorageCapacity{
@@ -171,6 +172,7 @@ func TestSyncStorageClass(t *testing.T) {
 		Labels: map[string]string{
 			"vcluster.loft.sh/namespace": "test",
 		},
+		ResourceVersion: "999",
 	}
 
 	pObj := &storagev1.CSIStorageCapacity{

pkg/controllers/resources/endpoints/syncer_test.go

@@ -27,8 +27,9 @@ func newFakeSyncer(t *testing.T, ctx *synccontext.RegisterContext) (*synccontext
 func TestExistingEndpoints(t *testing.T) {
 	vEndpoints := &corev1.Endpoints{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      "test-endpoints",
-			Namespace: "test",
+			Name:            "test-endpoints",
+			Namespace:       "test",
+			ResourceVersion: "999",
 		},
 		Subsets: []corev1.EndpointSubset{
 			{
@@ -152,8 +153,9 @@ func TestSync(t *testing.T) {
 	}
 	syncedEndpoints := &corev1.Endpoints{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      translate.Default.HostName(nil, baseEndpoints.Name, baseEndpoints.Namespace).Name,
-			Namespace: "test",
+			ResourceVersion: "999",
+			Name:            translate.Default.HostName(nil, baseEndpoints.Name, baseEndpoints.Namespace).Name,
+			Namespace:       "test",
 			Annotations: map[string]string{
 				translate.NameAnnotation:          baseEndpoints.Name,
 				translate.NamespaceAnnotation:     baseEndpoints.Namespace,

pkg/controllers/resources/ingressclasses/syncer_test.go

@@ -21,6 +21,7 @@ func TestSync(t *testing.T) {
 			translate.UIDAnnotation:  "",
 			translate.KindAnnotation: networkingv1.SchemeGroupVersion.WithKind("IngressClass").String(),
 		},
+		ResourceVersion: "999",
 	}
 
 	vObj := &networkingv1.IngressClass{

pkg/controllers/resources/ingresses/syncer_test.go

@@ -119,6 +119,7 @@ func TestSync(t *testing.T) {
 			translate.MarkerLabel:    translate.VClusterName,
 			translate.NamespaceLabel: vObjectMeta.Namespace,
 		},
+		ResourceVersion: "999",
 	}
 	baseIngress := &networkingv1.Ingress{
 		ObjectMeta: vObjectMeta,

pkg/controllers/resources/persistentvolumeclaims/syncer.go

@@ -172,7 +172,11 @@ func (s *persistentVolumeClaimSyncer) Sync(ctx *synccontext.SyncContext, event *
 	event.Host.Spec.Resources.Requests = event.Virtual.Spec.Resources.Requests
 
 	// change annotations
-	event.Host.Annotations = translate.HostAnnotations(event.Virtual, event.Host, s.excludedAnnotations...)
+	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 {

pkg/controllers/resources/runtimeclasses/syncer_test.go

@@ -23,6 +23,7 @@ func TestSync(t *testing.T) {
 			translate.UIDAnnotation:  "",
 			translate.KindAnnotation: nodev1.SchemeGroupVersion.WithKind("RuntimeClass").String(),
 		},
+		ResourceVersion: "999",
 	}
 
 	vObj := &nodev1.RuntimeClass{

pkg/controllers/resources/services/syncer_test.go

@@ -149,6 +149,7 @@ func TestSync(t *testing.T) {
 			Namespace: vObjectMeta.Namespace,
 		},
 		Spec: corev1.ServiceSpec{
+			ExternalName:   "backwardExternal",
 			ClusterIP:      "123:123:123:123",
 			ExternalIPs:    []string{"123:221:123:221"},
 			LoadBalancerIP: "123:213:123:213",

pkg/mappings/generic/recorder_test.go

@@ -187,6 +187,21 @@ func TestRecorderMigrate(t *testing.T) {
 				},
 			},
 		},
+		{
+			Name: "Multi namespace mode - namespace mapper",
+
+			MultiNamespaceMode: true,
+
+			Object: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "host-namespace-1",
+					Annotations: map[string]string{
+						translate.NameAnnotation: "virtual-namespace-1",
+						translate.KindAnnotation: corev1.SchemeGroupVersion.WithKind("Namespace").String(),
+					},
+				},
+			},
+		},
 	}
 
 	for _, testCase := range testCases {

pkg/mappings/resources/namespaces_test.go

@@ -0,0 +1,105 @@
+package resources
+
+import (
+	"context"
+	"testing"
+
+	"github.com/loft-sh/vcluster/pkg/mappings"
+	"github.com/loft-sh/vcluster/pkg/mappings/store"
+	"github.com/loft-sh/vcluster/pkg/scheme"
+	"github.com/loft-sh/vcluster/pkg/syncer/synccontext"
+	testingutil "github.com/loft-sh/vcluster/pkg/util/testing"
+	"github.com/loft-sh/vcluster/pkg/util/translate"
+	"gotest.tools/assert"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
+)
+
+func TestNamespaceMapperMigrate(t *testing.T) {
+	type testCase struct {
+		Name string
+
+		MultiNamespaceMode bool
+
+		Object client.Object
+
+		ExpectedMapping *synccontext.NameMapping
+	}
+	var testCases = []testCase{
+		{
+			Name: "Simple multi-namespace",
+
+			MultiNamespaceMode: true,
+
+			Object: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "host-namespace-1",
+					Annotations: map[string]string{
+						translate.NameAnnotation: "virtual-namespace-1",
+						translate.KindAnnotation: corev1.SchemeGroupVersion.WithKind("Namespace").String(),
+					},
+				},
+			},
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.Name, func(t *testing.T) {
+			storeBackend := store.NewMemoryBackend()
+			mappingsStore, err := store.NewStore(context.TODO(), nil, nil, storeBackend)
+			assert.NilError(t, err)
+
+			vConfig := testingutil.NewFakeConfig()
+			mappingsRegistry := mappings.NewMappingsRegistry(mappingsStore)
+			if testCase.MultiNamespaceMode {
+				translate.Default = translate.NewMultiNamespaceTranslator(testingutil.DefaultTestTargetNamespace)
+				vConfig.Experimental.MultiNamespaceMode.Enabled = true
+			} else {
+				translate.Default = translate.NewSingleNamespaceTranslator(testingutil.DefaultTestTargetNamespace)
+			}
+
+			// check recording
+			registerContext := &synccontext.RegisterContext{
+				Context:         context.TODO(),
+				Config:          vConfig,
+				Mappings:        mappingsRegistry,
+				PhysicalManager: testingutil.NewFakeManager(testingutil.NewFakeClient(scheme.Scheme)),
+				VirtualManager:  testingutil.NewFakeManager(testingutil.NewFakeClient(scheme.Scheme)),
+			}
+
+			// create namespace mapper
+			namespaceMapper, err := CreateNamespacesMapper(registerContext)
+			assert.NilError(t, err)
+			err = mappingsRegistry.AddMapper(namespaceMapper)
+			assert.NilError(t, err)
+
+			// create objects
+			err = registerContext.PhysicalManager.GetClient().Create(registerContext, testCase.Object)
+			assert.NilError(t, err)
+
+			gvk, err := apiutil.GVKForObject(testCase.Object, scheme.Scheme)
+			assert.NilError(t, err)
+
+			// migrate
+			err = namespaceMapper.Migrate(registerContext, namespaceMapper)
+			assert.NilError(t, err)
+
+			// check that objects were correctly migrated
+			mappings, err := storeBackend.List(registerContext)
+			assert.NilError(t, err)
+
+			// check if mapping is correct
+			if testCase.ExpectedMapping != nil {
+				assert.Equal(t, len(mappings), 1)
+				assert.Equal(t, mappings[0].GroupVersionKind.String(), gvk.String())
+				assert.Equal(t, mappings[0].NameMapping.GroupVersionKind.String(), testCase.ExpectedMapping.GroupVersionKind.String())
+				assert.Equal(t, mappings[0].NameMapping.VirtualName.String(), testCase.ExpectedMapping.VirtualName.String())
+				assert.Equal(t, mappings[0].NameMapping.HostName.String(), testCase.ExpectedMapping.HostName.String())
+			} else {
+				assert.Equal(t, len(mappings), 0)
+			}
+		})
+	}
+}

pkg/patcher/patcher.go

@@ -6,8 +6,10 @@ import (
 	"fmt"
 	"reflect"
 
+	jsonpatch "github.com/evanphx/json-patch"
 	"github.com/loft-sh/vcluster/config"
 	"github.com/loft-sh/vcluster/pkg/pro"
+	"github.com/loft-sh/vcluster/pkg/scheme"
 	"github.com/loft-sh/vcluster/pkg/syncer/synccontext"
 	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -220,8 +222,20 @@ func (h *Patcher) patchWholeObject(ctx context.Context, obj client.Object) error
 		return err
 	}
 
-	logPatch(ctx, fmt.Sprintf("Apply %s patch", h.direction), obj, beforeObject, afterObject)
-	return h.client.Patch(ctx, afterObject, client.MergeFrom(beforeObject))
+	patchBytes, err := client.MergeFrom(beforeObject).Data(afterObject)
+	if err != nil {
+		return err
+	} else if string(patchBytes) == "{}" || len(patchBytes) == 0 {
+		return nil
+	}
+
+	err = applyPatch(obj, patchBytes)
+	if err != nil {
+		return fmt.Errorf("apply: %w", err)
+	}
+
+	logPatch(ctx, fmt.Sprintf("Update %s", h.direction), obj, patchBytes)
+	return h.client.Update(ctx, obj)
 }
 
 // patch issues a patch for metadata and spec.
@@ -234,8 +248,20 @@ func (h *Patcher) patch(ctx context.Context, obj client.Object) error {
 		return err
 	}
 
-	logPatch(ctx, fmt.Sprintf("Apply %s patch", h.direction), obj, beforeObject, afterObject)
-	return h.client.Patch(ctx, afterObject, client.MergeFrom(beforeObject))
+	patchBytes, err := client.MergeFrom(beforeObject).Data(afterObject)
+	if err != nil {
+		return err
+	} else if string(patchBytes) == "{}" || len(patchBytes) == 0 {
+		return nil
+	}
+
+	err = applyPatch(obj, patchBytes)
+	if err != nil {
+		return fmt.Errorf("apply: %w", err)
+	}
+
+	logPatch(ctx, fmt.Sprintf("Update %s", h.direction), obj, patchBytes)
+	return h.client.Update(ctx, obj)
 }
 
 // patchStatus issues a patch if the status has changed.
@@ -248,14 +274,26 @@ func (h *Patcher) patchStatus(ctx context.Context, obj client.Object) error {
 		return err
 	}
 
-	logPatch(ctx, fmt.Sprintf("Apply %s status patch", h.direction), obj, beforeObject, afterObject)
-	return h.client.Status().Patch(ctx, afterObject, client.MergeFrom(beforeObject))
+	patchBytes, err := client.MergeFrom(beforeObject).Data(afterObject)
+	if err != nil {
+		return err
+	} else if string(patchBytes) == "{}" || len(patchBytes) == 0 {
+		return nil
+	}
+
+	err = applyPatch(obj, patchBytes)
+	if err != nil {
+		return fmt.Errorf("apply: %w", err)
+	}
+
+	logPatch(ctx, fmt.Sprintf("Update status %s", h.direction), obj, patchBytes)
+	return h.client.Status().Update(ctx, obj)
 }
 
-func logPatch(ctx context.Context, patchMessage string, obj, beforeObject, afterObject client.Object) {
+func logPatch(ctx context.Context, patchMessage string, obj client.Object, patchBytes []byte) {
 	// log patch
-	patchBytes, _ := client.MergeFrom(beforeObject).Data(afterObject)
-	klog.FromContext(ctx).Info(patchMessage, "kind", obj.GetObjectKind().GroupVersionKind().Kind, "object", obj.GetNamespace()+"/"+obj.GetName(), "patch", string(patchBytes))
+	gvk, _ := apiutil.GVKForObject(obj, scheme.Scheme)
+	klog.FromContext(ctx).Info(patchMessage+" "+gvk.Kind+" "+obj.GetNamespace()+"/"+obj.GetName(), "patch", string(patchBytes))
 }
 
 // calculatePatch returns the before/after objects to be given in a controller-runtime patch, scoped down to the absolute necessary.
@@ -329,3 +367,33 @@ func checkNilObject(obj client.Object) error {
 
 	return nil
 }
+
+func applyPatch(obj client.Object, patchBytes []byte) error {
+	unstructuredMap, err := toUnstructured(obj)
+	if err != nil {
+		return fmt.Errorf("to unstructured: %w", err)
+	}
+
+	objBytes, err := json.Marshal(unstructuredMap.Object)
+	if err != nil {
+		return fmt.Errorf("marshal object: %w", err)
+	}
+
+	afterObjBytes, err := jsonpatch.MergePatch(objBytes, patchBytes)
+	if err != nil {
+		return fmt.Errorf("apply merge patch: %w", err)
+	}
+
+	afterObjMap := map[string]interface{}{}
+	err = json.Unmarshal(afterObjBytes, &afterObjMap)
+	if err != nil {
+		return fmt.Errorf("unmarshal applied object: %w", err)
+	}
+
+	err = runtime.DefaultUnstructuredConverter.FromUnstructured(afterObjMap, obj)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}