diff --git a/pkg/controllers/resources/endpoints/syncer_test.go b/pkg/controllers/resources/endpoints/syncer_test.go index 0b9c7890b..f3557fd39 100644 --- a/pkg/controllers/resources/endpoints/syncer_test.go +++ b/pkg/controllers/resources/endpoints/syncer_test.go @@ -213,7 +213,12 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*endpointsSyncer).Sync(syncCtx, synccontext.NewSyncEvent(syncedEndpoints, updatedEndpoints)) + _, err := syncer.(*endpointsSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + syncedEndpoints, + syncedEndpoints, + baseEndpoints, + updatedEndpoints, + )) assert.NilError(t, err) }, }, diff --git a/pkg/controllers/resources/ingresses/syncer_test.go b/pkg/controllers/resources/ingresses/syncer_test.go index c3f9e5f21..3e760f3f0 100644 --- a/pkg/controllers/resources/ingresses/syncer_test.go +++ b/pkg/controllers/resources/ingresses/syncer_test.go @@ -256,7 +256,7 @@ func TestSync(t *testing.T) { vIngress := baseIngress.DeepCopy() vIngress.ResourceVersion = "999" - _, err := syncer.(*ingressSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(backwardUpdateIngress, backwardUpdateIngress, vIngress, vIngress)) + _, err := syncer.(*ingressSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(baseIngress.DeepCopy(), backwardUpdateIngress, vIngress, vIngress)) assert.NilError(t, err) err = syncCtx.VirtualClient.Get(syncCtx, types.NamespacedName{Namespace: vIngress.Namespace, Name: vIngress.Name}, vIngress) @@ -346,7 +346,6 @@ func TestSync(t *testing.T) { Annotations: map[string]string{ "nginx.ingress.kubernetes.io/auth-secret": translate.Default.HostName(nil, "my-secret", baseIngress.Namespace).Name, "nginx.ingress.kubernetes.io/auth-tls-secret": createdIngress.Namespace + "/" + translate.Default.HostName(nil, "my-secret", baseIngress.Namespace).Name, - "vcluster.loft.sh/managed-annotations": "nginx.ingress.kubernetes.io/auth-secret\nnginx.ingress.kubernetes.io/auth-tls-secret", "vcluster.loft.sh/object-name": baseIngress.Name, "vcluster.loft.sh/object-namespace": baseIngress.Namespace, translate.UIDAnnotation: "", @@ -369,7 +368,7 @@ func TestSync(t *testing.T) { err = syncCtx.PhysicalClient.Get(syncCtx, types.NamespacedName{Name: createdIngress.Name, Namespace: createdIngress.Namespace}, pIngress) assert.NilError(t, err) - _, err = syncer.(*ingressSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(pIngress, pIngress, vIngress, vIngress)) + _, err = syncer.(*ingressSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(pIngress, pIngress, baseIngress.DeepCopy(), vIngress)) assert.NilError(t, err) }, }, @@ -422,7 +421,6 @@ func TestSync(t *testing.T) { Namespace: createdIngress.Namespace, Labels: createdIngress.Labels, Annotations: map[string]string{ - "vcluster.loft.sh/managed-annotations": "alb.ingress.kubernetes.io/actions.ssl-redirect-x-test-x-suffix\nalb.ingress.kubernetes.io/actions.testservice-x-test-x-suffix\nnginx.ingress.kubernetes.io/auth-secret", "nginx.ingress.kubernetes.io/auth-secret": translate.Default.HostName(nil, "my-secret", baseIngress.Namespace).Name, "vcluster.loft.sh/object-name": baseIngress.Name, "vcluster.loft.sh/object-namespace": baseIngress.Namespace, @@ -448,7 +446,7 @@ func TestSync(t *testing.T) { err = syncCtx.PhysicalClient.Get(syncCtx, types.NamespacedName{Name: createdIngress.Name, Namespace: createdIngress.Namespace}, pIngress) assert.NilError(t, err) - _, err = syncer.(*ingressSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(pIngress, pIngress, vIngress, vIngress)) + _, err = syncer.(*ingressSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(pIngress, pIngress, baseIngress.DeepCopy(), vIngress)) assert.NilError(t, err) }, }, diff --git a/pkg/controllers/resources/networkpolicies/syncer_test.go b/pkg/controllers/resources/networkpolicies/syncer_test.go index 40094a625..3942b633c 100644 --- a/pkg/controllers/resources/networkpolicies/syncer_test.go +++ b/pkg/controllers/resources/networkpolicies/syncer_test.go @@ -249,11 +249,8 @@ func TestSync(t *testing.T) { }, }, { - Name: "Update forward", - InitialVirtualState: []runtime.Object{&networkingv1.NetworkPolicy{ - ObjectMeta: vObjectMeta, - Spec: vBaseSpec, - }}, + Name: "Update forward", + InitialVirtualState: []runtime.Object{vBaseNetworkPolicy.DeepCopy()}, InitialPhysicalState: []runtime.Object{&networkingv1.NetworkPolicy{ ObjectMeta: pObjectMeta, Spec: networkingv1.NetworkPolicySpec{}, @@ -269,16 +266,22 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - pNetworkPolicy := &networkingv1.NetworkPolicy{ + pNetworkPolicyOld := &networkingv1.NetworkPolicy{ ObjectMeta: pObjectMeta, Spec: networkingv1.NetworkPolicySpec{}, } + pNetworkPolicy := pNetworkPolicyOld.DeepCopy() pNetworkPolicy.ResourceVersion = "999" - _, err := syncer.(*networkPolicySyncer).Sync(syncCtx, synccontext.NewSyncEvent(pNetworkPolicy, &networkingv1.NetworkPolicy{ - ObjectMeta: vObjectMeta, - Spec: vBaseSpec, - })) + vNetworkPolicyOld := vBaseNetworkPolicy + vNetworkPolicy := vNetworkPolicyOld.DeepCopy() + + _, err := syncer.(*networkPolicySyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + pNetworkPolicyOld, + pNetworkPolicy, + vNetworkPolicyOld, + vNetworkPolicy, + )) assert.NilError(t, err) }, }, @@ -294,10 +297,20 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) + + vNetworkPolicyOld := vBaseNetworkPolicy.DeepCopy() vNetworkPolicy := vBaseNetworkPolicy.DeepCopy() vNetworkPolicy.ResourceVersion = "999" - _, err := syncer.(*networkPolicySyncer).Sync(syncCtx, synccontext.NewSyncEvent(pBaseNetworkPolicy.DeepCopy(), vNetworkPolicy)) + pNetworkPolicyOld := pBaseNetworkPolicy.DeepCopy() + pNetworkPolicy := pBaseNetworkPolicy.DeepCopy() + + _, err := syncer.(*networkPolicySyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + pNetworkPolicyOld, + pNetworkPolicy, + vNetworkPolicyOld, + vNetworkPolicy, + )) assert.NilError(t, err) }, }, diff --git a/pkg/controllers/resources/persistentvolumeclaims/syncer_test.go b/pkg/controllers/resources/persistentvolumeclaims/syncer_test.go index bbed7bc9c..9294d8243 100644 --- a/pkg/controllers/resources/persistentvolumeclaims/syncer_test.go +++ b/pkg/controllers/resources/persistentvolumeclaims/syncer_test.go @@ -79,14 +79,13 @@ func TestSync(t *testing.T) { Name: pObjectMeta.Name, Namespace: pObjectMeta.Namespace, Annotations: map[string]string{ - translate.NameAnnotation: vObjectMeta.Name, - translate.NamespaceAnnotation: vObjectMeta.Namespace, - translate.UIDAnnotation: "", - translate.KindAnnotation: corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim").String(), - translate.HostNamespaceAnnotation: pObjectMeta.Namespace, - translate.HostNameAnnotation: pObjectMeta.Name, - translate.ManagedAnnotationsAnnotation: "otherAnnotationKey", - "otherAnnotationKey": "update this", + translate.NameAnnotation: vObjectMeta.Name, + translate.NamespaceAnnotation: vObjectMeta.Namespace, + translate.UIDAnnotation: "", + translate.KindAnnotation: corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim").String(), + translate.HostNamespaceAnnotation: pObjectMeta.Namespace, + translate.HostNameAnnotation: pObjectMeta.Name, + "otherAnnotationKey": "update this", }, Labels: pObjectMeta.Labels, }, @@ -186,7 +185,20 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*persistentVolumeClaimSyncer).Sync(syncCtx, synccontext.NewSyncEvent(createdPvc.DeepCopy(), updatePvc.DeepCopy())) + + pObjOld := createdPvc.DeepCopy() + pObj := createdPvc.DeepCopy() + + vObjOld := updatePvc.DeepCopy() + vObjOld.ObjectMeta.SetAnnotations(nil) + vObj := updatePvc.DeepCopy() + + _, err := syncer.(*persistentVolumeClaimSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + pObjOld, + pObj, + vObjOld, + vObj, + )) assert.NilError(t, err) }, }, @@ -202,7 +214,12 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*persistentVolumeClaimSyncer).Sync(syncCtx, synccontext.NewSyncEvent(createdPvc.DeepCopy(), basePvc.DeepCopy())) + _, err := syncer.(*persistentVolumeClaimSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + createdPvc, + createdPvc.DeepCopy(), + basePvc, + basePvc.DeepCopy(), + )) assert.NilError(t, err) }, }, @@ -234,7 +251,18 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*persistentVolumeClaimSyncer).Sync(syncCtx, synccontext.NewSyncEvent(backwardUpdateAnnotationsPvc.DeepCopy(), basePvc.DeepCopy())) + pObjOld := backwardUpdateAnnotationsPvc + pObj := backwardUpdateAnnotationsPvc.DeepCopy() + + vObjOld := basePvc + vObj := basePvc.DeepCopy() + + _, err := syncer.(*persistentVolumeClaimSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + pObjOld, + pObj, + vObjOld, + vObj, + )) assert.NilError(t, err) }, }, @@ -251,7 +279,18 @@ func TestSync(t *testing.T) { Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) syncer.(*persistentVolumeClaimSyncer).useFakePersistentVolumes = true - _, err := syncer.(*persistentVolumeClaimSyncer).Sync(syncCtx, synccontext.NewSyncEvent(backwardUpdateStatusPvc.DeepCopy(), basePvc.DeepCopy())) + + pObjOld := backwardUpdateStatusPvc.DeepCopy() + pObj := backwardUpdateStatusPvc.DeepCopy() + vObjOld := basePvc.DeepCopy() + vObj := basePvc.DeepCopy() + + _, err := syncer.(*persistentVolumeClaimSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + pObjOld, + pObj, + vObjOld, + vObj, + )) assert.NilError(t, err) }, }, diff --git a/pkg/controllers/resources/persistentvolumes/syncer_test.go b/pkg/controllers/resources/persistentvolumes/syncer_test.go index 10a37e96a..6bf23668f 100644 --- a/pkg/controllers/resources/persistentvolumes/syncer_test.go +++ b/pkg/controllers/resources/persistentvolumes/syncer_test.go @@ -199,6 +199,15 @@ func TestSync(t *testing.T) { Phase: corev1.VolumeReleased, }, } + capacityVPv := &corev1.PersistentVolume{ + ObjectMeta: basePPv.ObjectMeta, + Spec: corev1.PersistentVolumeSpec{ + Capacity: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("5Gi"), + }, + ClaimRef: basePPv.Spec.ClaimRef, + }, + } createContext := func(vConfig *config.VirtualClusterConfig, pClient *testingutil.FakeIndexClient, vClient *testingutil.FakeIndexClient) *synccontext.RegisterContext { vConfig.Sync.ToHost.PersistentVolumes.Enabled = true @@ -274,9 +283,18 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncContext, syncer := newFakeSyncer(t, ctx) - backwardUpdatePPv := backwardUpdatePPv.DeepCopy() - baseVPv := baseVPv.DeepCopy() - _, err := syncer.Sync(syncContext, synccontext.NewSyncEvent(backwardUpdatePPv, baseVPv)) + + pObjOld := backwardUpdatePPv + pObj := backwardUpdatePPv.DeepCopy() + vObjOld := baseVPv + vObj := baseVPv.DeepCopy() + + _, err := syncer.Sync(syncContext, synccontext.NewSyncEventWithOld( + pObjOld, + pObj, + vObjOld, + vObj, + )) assert.NilError(t, err) err = syncContext.VirtualClient.Get(ctx, types.NamespacedName{Name: baseVPv.Name}, baseVPv) @@ -285,7 +303,14 @@ func TestSync(t *testing.T) { err = syncContext.PhysicalClient.Get(ctx, types.NamespacedName{Name: backwardUpdatePPv.Name}, backwardUpdatePPv) assert.NilError(t, err) - _, err = syncer.Sync(syncContext, synccontext.NewSyncEvent(backwardUpdatePPv, baseVPv)) + pObj2 := backwardUpdatePPv + vObj2 := baseVPv + _, err = syncer.Sync(syncContext, synccontext.NewSyncEventWithOld( + pObjOld, + pObj2, + vObjOld, + vObj2, + )) assert.NilError(t, err) }, }, @@ -319,7 +344,18 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncContext, syncer := newFakeSyncer(t, ctx) - _, err := syncer.Sync(syncContext, synccontext.NewSyncEvent(basePPv, baseVPv)) + + pObjOld := basePPv + pObj := basePPv + vObjOld := baseVPv + vObj := baseVPv + + _, err := syncer.Sync(syncContext, synccontext.NewSyncEventWithOld( + pObjOld, + pObj, + vObjOld, + vObj, + )) assert.NilError(t, err) }, }, @@ -329,15 +365,7 @@ func TestSync(t *testing.T) { &corev1.PersistentVolumeClaim{ ObjectMeta: basePvc.ObjectMeta, }, - &corev1.PersistentVolume{ - ObjectMeta: baseVPv.ObjectMeta, - Spec: corev1.PersistentVolumeSpec{ - Capacity: corev1.ResourceList{ - corev1.ResourceStorage: resource.MustParse("5Gi"), - }, - ClaimRef: baseVPv.Spec.ClaimRef, - }, - }, + capacityVPv, }, InitialPhysicalState: []runtime.Object{ &corev1.PersistentVolume{ @@ -371,7 +399,7 @@ func TestSync(t *testing.T) { ExpectedPhysicalState: map[schema.GroupVersionKind][]runtime.Object{ corev1.SchemeGroupVersion.WithKind("PersistentVolume"): { &corev1.PersistentVolume{ - ObjectMeta: basePPv.ObjectMeta, + ObjectMeta: backwardPvObjectMeta, Spec: corev1.PersistentVolumeSpec{ Capacity: corev1.ResourceList{ corev1.ResourceStorage: resource.MustParse("20Gi"), @@ -384,15 +412,20 @@ func TestSync(t *testing.T) { Sync: func(ctx *synccontext.RegisterContext) { syncContext, syncer := newFakeSyncer(t, ctx) - vPv := &corev1.PersistentVolume{} - err := syncContext.VirtualClient.Get(ctx, types.NamespacedName{Name: baseVPv.Name}, vPv) - assert.NilError(t, err) + vObjOld := capacityVPv - pPv := &corev1.PersistentVolume{} - err = syncContext.PhysicalClient.Get(ctx, types.NamespacedName{Name: basePPv.Name}, pPv) + vObj := vObjOld.DeepCopy() + vObj.Spec.Capacity = corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("20Gi"), + } + + pObjOld := &corev1.PersistentVolume{} + err := syncContext.PhysicalClient.Get(ctx, types.NamespacedName{Name: basePPv.Name}, pObjOld) assert.NilError(t, err) - _, err = syncer.Sync(syncContext, synccontext.NewSyncEventWithOld(pPv, pPv, vPv, vPv)) + pObj := pObjOld.DeepCopy() + + _, err = syncer.Sync(syncContext, synccontext.NewSyncEventWithOld(pObjOld, pObj, vObjOld, vObj)) assert.NilError(t, err) }, }, @@ -408,9 +441,18 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncContext, syncer := newFakeSyncer(t, ctx) - backwardRetainPPv := backwardRetainPPv.DeepCopy() - backwardRetainInitialVPv := backwardRetainInitialVPv.DeepCopy() - _, err := syncer.Sync(syncContext, synccontext.NewSyncEvent(backwardRetainPPv, backwardRetainInitialVPv)) + + pObjOld := backwardRetainPPv + pObj := backwardRetainPPv.DeepCopy() + vObjOld := backwardRetainInitialVPv + vObj := backwardRetainInitialVPv.DeepCopy() + + _, err := syncer.Sync(syncContext, synccontext.NewSyncEventWithOld( + pObjOld, + pObj, + vObjOld, + vObj, + )) assert.NilError(t, err) }, }, diff --git a/pkg/controllers/resources/poddisruptionbudgets/syncer_test.go b/pkg/controllers/resources/poddisruptionbudgets/syncer_test.go index 35b94ea76..a0b52869e 100644 --- a/pkg/controllers/resources/poddisruptionbudgets/syncer_test.go +++ b/pkg/controllers/resources/poddisruptionbudgets/syncer_test.go @@ -116,7 +116,12 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*pdbSyncer).Sync(syncCtx, synccontext.NewSyncEvent(hostClusterSyncedPDB.DeepCopy(), vclusterUpdatedPDB.DeepCopy())) + _, err := syncer.(*pdbSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + hostClusterSyncedPDB.DeepCopy(), + hostClusterSyncedPDB.DeepCopy(), + vclusterUpdatedPDB.DeepCopy(), + vclusterUpdatedPDB.DeepCopy(), + )) assert.NilError(t, err) }, }, @@ -136,7 +141,12 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*pdbSyncer).Sync(syncCtx, synccontext.NewSyncEvent(hostClusterSyncedPDB.DeepCopy(), vclusterUpdatedSelectorPDB.DeepCopy())) + _, err := syncer.(*pdbSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + hostClusterSyncedPDB.DeepCopy(), + hostClusterSyncedPDB.DeepCopy(), + vclusterUpdatedSelectorPDB.DeepCopy(), + vclusterUpdatedSelectorPDB.DeepCopy(), + )) assert.NilError(t, err) }, }, diff --git a/pkg/controllers/resources/pods/syncer_test.go b/pkg/controllers/resources/pods/syncer_test.go index 5e990448c..250760af7 100644 --- a/pkg/controllers/resources/pods/syncer_test.go +++ b/pkg/controllers/resources/pods/syncer_test.go @@ -278,7 +278,12 @@ func TestSyncTable(t *testing.T) { if tC.syncToHost { _, err = syncer.(*podSyncer).SyncToHost(syncCtx, synccontext.NewSyncToHostEvent(vPodInitial.DeepCopy())) } else { - _, err = syncer.(*podSyncer).Sync(syncCtx, synccontext.NewSyncEvent(pPodInitial.DeepCopy(), vPodInitial.DeepCopy())) + _, err = syncer.(*podSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + pPodInitial.DeepCopy(), + pPodInitial.DeepCopy(), + vPodInitial.DeepCopy(), + vPodInitial.DeepCopy(), + )) } assert.NilError(t, err) diff --git a/pkg/controllers/resources/secrets/syncer_test.go b/pkg/controllers/resources/secrets/syncer_test.go index e73f2629e..46b38591a 100644 --- a/pkg/controllers/resources/secrets/syncer_test.go +++ b/pkg/controllers/resources/secrets/syncer_test.go @@ -127,7 +127,10 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncContext, syncer := newFakeSyncer(t, ctx) - _, err := syncer.(*secretSyncer).Sync(syncContext, synccontext.NewSyncEvent(syncedSecret, updatedSecret)) + syncedSecret := syncedSecret.DeepCopy() + updatedSecret := updatedSecret.DeepCopy() + baseSecret := baseSecret.DeepCopy() + _, err := syncer.(*secretSyncer).Sync(syncContext, synccontext.NewSyncEventWithOld(syncedSecret, syncedSecret, baseSecret, updatedSecret)) assert.NilError(t, err) }, }, @@ -144,7 +147,7 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncContext, syncer := newFakeSyncer(t, ctx) - _, err := syncer.(*secretSyncer).Sync(syncContext, synccontext.NewSyncEvent(syncedSecret, updatedSecret)) + _, err := syncer.(*secretSyncer).Sync(syncContext, synccontext.NewSyncEvent(syncedSecret.DeepCopy(), updatedSecret.DeepCopy())) assert.NilError(t, err) }, }, diff --git a/pkg/controllers/resources/services/syncer.go b/pkg/controllers/resources/services/syncer.go index c650b03dd..912365a9f 100644 --- a/pkg/controllers/resources/services/syncer.go +++ b/pkg/controllers/resources/services/syncer.go @@ -15,6 +15,7 @@ import ( syncertypes "github.com/loft-sh/vcluster/pkg/syncer/types" "github.com/loft-sh/vcluster/pkg/util/translate" corev1 "k8s.io/api/core/v1" + apiequality "k8s.io/apimachinery/pkg/api/equality" kerrors "k8s.io/apimachinery/pkg/api/errors" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/klog/v2" @@ -109,6 +110,9 @@ func (s *serviceSyncer) Sync(ctx *synccontext.SyncContext, event *synccontext.Sy return ctrl.Result{}, fmt.Errorf("new syncer patcher: %w", err) } defer func() { + AlignSpecWithServiceType(event.Virtual) + AlignSpecWithServiceType(event.Host) + if err := patch.Patch(ctx, event.Host, event.Virtual); err != nil { retErr = utilerrors.NewAggregate([]error{retErr, err}) } @@ -117,6 +121,13 @@ func (s *serviceSyncer) Sync(ctx *synccontext.SyncContext, event *synccontext.Sy } }() + event.Virtual.Spec.Type, event.Host.Spec.Type = patcher.CopyBidirectional( + event.VirtualOld.Spec.Type, + event.Virtual.Spec.Type, + event.HostOld.Spec.Type, + event.Host.Spec.Type, + ) + // update spec bidirectionally event.Virtual.Spec.ExternalIPs, event.Host.Spec.ExternalIPs = patcher.CopyBidirectional( event.VirtualOld.Spec.ExternalIPs, @@ -124,6 +135,12 @@ func (s *serviceSyncer) Sync(ctx *synccontext.SyncContext, event *synccontext.Sy event.HostOld.Spec.ExternalIPs, event.Host.Spec.ExternalIPs, ) + event.Virtual.Spec.ExternalName, event.Host.Spec.ExternalName = patcher.CopyBidirectional( + event.VirtualOld.Spec.ExternalName, + event.Virtual.Spec.ExternalName, + event.HostOld.Spec.ExternalName, + event.Host.Spec.ExternalName, + ) event.Virtual.Spec.LoadBalancerIP, event.Host.Spec.LoadBalancerIP = patcher.CopyBidirectional( event.VirtualOld.Spec.LoadBalancerIP, event.Virtual.Spec.LoadBalancerIP, @@ -142,18 +159,6 @@ func (s *serviceSyncer) Sync(ctx *synccontext.SyncContext, event *synccontext.Sy event.HostOld.Spec.PublishNotReadyAddresses, event.Host.Spec.PublishNotReadyAddresses, ) - event.Virtual.Spec.Type, event.Host.Spec.Type = patcher.CopyBidirectional( - event.VirtualOld.Spec.Type, - event.Virtual.Spec.Type, - event.HostOld.Spec.Type, - event.Host.Spec.Type, - ) - event.Virtual.Spec.ExternalName, event.Host.Spec.ExternalName = patcher.CopyBidirectional( - event.VirtualOld.Spec.ExternalName, - event.Virtual.Spec.ExternalName, - event.HostOld.Spec.ExternalName, - event.Host.Spec.ExternalName, - ) event.Virtual.Spec.ExternalTrafficPolicy, event.Host.Spec.ExternalTrafficPolicy = patcher.CopyBidirectional( event.VirtualOld.Spec.ExternalTrafficPolicy, event.Virtual.Spec.ExternalTrafficPolicy, @@ -190,7 +195,6 @@ func (s *serviceSyncer) Sync(ctx *synccontext.SyncContext, event *synccontext.Sy // bi-directional sync of annotations and labels event.Virtual.Annotations, event.Host.Annotations = translate.AnnotationsBidirectionalUpdate(event, s.excludedAnnotations...) - event.Virtual.Labels, event.Host.Labels = translate.LabelsBidirectionalUpdate(event) // remove the ServiceBlockDeletion annotation if it's not needed if event.Virtual.Spec.ClusterIP == event.Host.Spec.ClusterIP { @@ -198,12 +202,12 @@ func (s *serviceSyncer) Sync(ctx *synccontext.SyncContext, event *synccontext.Sy } // translate selector - event.Virtual.Spec.Selector, event.Host.Spec.Selector = translate.LabelsBidirectionalUpdateMaps( - event.VirtualOld.Spec.Selector, - event.Virtual.Spec.Selector, - event.HostOld.Spec.Selector, - event.Host.Spec.Selector, - ) + // TODO: ryan - convert to bidirectional + if !apiequality.Semantic.DeepEqual(event.VirtualOld.Spec.Selector, event.Virtual.Spec.Selector) { + event.Host.Spec.Selector = translate.HostLabelsMap(event.Virtual.Spec.Selector, event.Host.Spec.Selector, event.Virtual.Namespace, false) + } else { + event.Virtual.Spec.Selector = translate.VirtualLabelsMap(event.Host.Spec.Selector, event.Virtual.Spec.Selector) + } return ctrl.Result{}, nil } diff --git a/pkg/controllers/resources/services/syncer_test.go b/pkg/controllers/resources/services/syncer_test.go index 970807d28..dfa01f40d 100644 --- a/pkg/controllers/resources/services/syncer_test.go +++ b/pkg/controllers/resources/services/syncer_test.go @@ -6,17 +6,14 @@ import ( "github.com/loft-sh/vcluster/pkg/specialservices" "github.com/loft-sh/vcluster/pkg/syncer/synccontext" syncertesting "github.com/loft-sh/vcluster/pkg/syncer/testing" - "gotest.tools/assert" - "k8s.io/apimachinery/pkg/util/intstr" - - "k8s.io/apimachinery/pkg/types" - "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" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" ) func TestSync(t *testing.T) { @@ -64,15 +61,14 @@ func TestSync(t *testing.T) { }, PublishNotReadyAddresses: true, Type: corev1.ServiceTypeNodePort, - ExternalName: "external", ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeLocal, SessionAffinity: corev1.ServiceAffinityClientIP, - LoadBalancerSourceRanges: []string{"backwardRange"}, SessionAffinityConfig: &corev1.SessionAffinityConfig{ ClientIP: &corev1.ClientIPConfig{}, }, HealthCheckNodePort: 112, } + updateForwardService := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: vObjectMeta.Name, @@ -86,14 +82,13 @@ func TestSync(t *testing.T) { Name: pObjectMeta.Name, Namespace: pObjectMeta.Namespace, Annotations: map[string]string{ - translate.NameAnnotation: vObjectMeta.Name, - translate.NamespaceAnnotation: vObjectMeta.Namespace, - translate.UIDAnnotation: "", - translate.KindAnnotation: corev1.SchemeGroupVersion.WithKind("Service").String(), - translate.HostNamespaceAnnotation: pObjectMeta.Namespace, - translate.HostNameAnnotation: pObjectMeta.Name, - translate.ManagedAnnotationsAnnotation: "a", - "a": "b", + translate.NameAnnotation: vObjectMeta.Name, + translate.NamespaceAnnotation: vObjectMeta.Namespace, + translate.UIDAnnotation: "", + translate.KindAnnotation: corev1.SchemeGroupVersion.WithKind("Service").String(), + translate.HostNamespaceAnnotation: pObjectMeta.Namespace, + translate.HostNameAnnotation: pObjectMeta.Name, + "a": "b", }, Labels: pObjectMeta.Labels, }, @@ -118,9 +113,8 @@ func TestSync(t *testing.T) { Annotations: pObjectMeta.Annotations, }, Spec: corev1.ServiceSpec{ - ExternalName: "backwardExternal", - ExternalIPs: []string{"123:221:123:221"}, - LoadBalancerIP: "123:213:123:213", + Type: corev1.ServiceTypeExternalName, + ExternalName: "backwardExternal", }, } updatedBackwardSpecService := &corev1.Service{ @@ -129,9 +123,8 @@ func TestSync(t *testing.T) { Namespace: vObjectMeta.Namespace, }, Spec: corev1.ServiceSpec{ - ExternalName: "backwardExternal", - ExternalIPs: []string{"123:221:123:221"}, - LoadBalancerIP: "123:213:123:213", + Type: corev1.ServiceTypeExternalName, + ExternalName: "backwardExternal", }, } updateBackwardSpecRecreateService := &corev1.Service{ @@ -259,8 +252,7 @@ func TestSync(t *testing.T) { vServiceClusterIPFromExternal := &corev1.Service{ ObjectMeta: vObjectMeta, Spec: corev1.ServiceSpec{ - ExternalName: "test.com", - Type: corev1.ServiceTypeClusterIP, + Type: corev1.ServiceTypeClusterIP, Ports: []corev1.ServicePort{ { Name: "http", @@ -279,9 +271,8 @@ func TestSync(t *testing.T) { pServiceClusterIPFromExternal := &corev1.Service{ ObjectMeta: pObjectMeta, Spec: corev1.ServiceSpec{ - ExternalName: "test.com", - Type: corev1.ServiceTypeClusterIP, - Ports: vServiceClusterIPFromExternal.Spec.Ports, + Type: corev1.ServiceTypeClusterIP, + Ports: vServiceClusterIPFromExternal.Spec.Ports, }, } selectorKey := "test" @@ -311,7 +302,7 @@ func TestSync(t *testing.T) { }, } - syncertesting.RunTests(t, []*syncertesting.SyncTest{ + tests := []*syncertesting.SyncTest{ { Name: "Create Forward", InitialVirtualState: []runtime.Object{baseService.DeepCopy()}, @@ -323,6 +314,7 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) + baseService := baseService.DeepCopy() _, err := syncer.(*serviceSyncer).SyncToHost(syncCtx, synccontext.NewSyncToHostEvent(baseService)) assert.NilError(t, err) }, @@ -339,7 +331,10 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(pServicePorts1.DeepCopy(), pServicePorts1.DeepCopy(), vServicePorts1.DeepCopy(), vServicePorts1.DeepCopy())) + pObjOld := baseService.DeepCopy() + pObjNew := pServicePorts1.DeepCopy() + vObj := vServicePorts1.DeepCopy() + _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(pObjOld, pObjNew, vObj, vObj)) assert.NilError(t, err) }, }, @@ -355,23 +350,31 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEvent(pServicePorts2.DeepCopy(), vServicePorts1.DeepCopy())) + pObj := pServicePorts2.DeepCopy() + vObjOld := baseService.DeepCopy() + vObjNew := vServicePorts1.DeepCopy() + _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(pObj, pObj, vObjOld, vObjNew)) assert.NilError(t, err) }, }, { Name: "Update forward", - InitialVirtualState: []runtime.Object{updateForwardService.DeepCopy()}, InitialPhysicalState: []runtime.Object{createdByServerService.DeepCopy()}, - ExpectedVirtualState: map[schema.GroupVersionKind][]runtime.Object{ - corev1.SchemeGroupVersion.WithKind("Service"): {updateForwardService.DeepCopy()}, - }, ExpectedPhysicalState: map[schema.GroupVersionKind][]runtime.Object{ corev1.SchemeGroupVersion.WithKind("Service"): {updatedForwardService.DeepCopy()}, }, + + InitialVirtualState: []runtime.Object{updateForwardService.DeepCopy()}, + ExpectedVirtualState: map[schema.GroupVersionKind][]runtime.Object{ + corev1.SchemeGroupVersion.WithKind("Service"): {updateForwardService.DeepCopy()}, + }, + Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEvent(createdByServerService.DeepCopy(), updateForwardService.DeepCopy())) + pObjOld := createdByServerService.DeepCopy() + vObjOld := createdService.DeepCopy() + vObj := updateForwardService.DeepCopy() + _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(pObjOld, pObjOld, vObjOld, vObj)) assert.NilError(t, err) }, }, @@ -387,7 +390,9 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEvent(createdService.DeepCopy(), baseService.DeepCopy())) + pObj := createdService.DeepCopy() + vObj := baseService.DeepCopy() + _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(pObj, pObj, vObj, vObj)) assert.NilError(t, err) }, }, @@ -403,19 +408,20 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - baseService := baseService.DeepCopy() - updateBackwardSpecService := updateBackwardSpecService.DeepCopy() - _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(updateBackwardSpecService, updateBackwardSpecService, baseService, baseService)) + pObjOld := baseService.DeepCopy() + vObj := baseService.DeepCopy() + pObjNew := updateBackwardSpecService.DeepCopy() + _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(pObjOld, pObjNew, vObj, vObj)) assert.NilError(t, err) - err = ctx.VirtualManager.GetClient().Get(ctx, types.NamespacedName{Namespace: baseService.Namespace, Name: baseService.Name}, baseService) + err = ctx.VirtualManager.GetClient().Get(ctx, types.NamespacedName{Namespace: pObjOld.Namespace, Name: pObjOld.Name}, pObjOld) assert.NilError(t, err) - err = ctx.PhysicalManager.GetClient().Get(ctx, types.NamespacedName{Namespace: updateBackwardSpecService.Namespace, Name: updateBackwardSpecService.Name}, updateBackwardSpecService) + err = ctx.PhysicalManager.GetClient().Get(ctx, types.NamespacedName{Namespace: pObjNew.Namespace, Name: pObjNew.Name}, pObjNew) assert.NilError(t, err) - baseService.Spec.ExternalName = updateBackwardSpecService.Spec.ExternalName - _, err = syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(updateBackwardSpecService.DeepCopy(), updateBackwardSpecService.DeepCopy(), baseService.DeepCopy(), baseService.DeepCopy())) + pObjOld.Spec.ExternalName = pObjNew.Spec.ExternalName + _, err = syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(pObjNew, pObjNew, vObj, vObj)) assert.NilError(t, err) }, }, @@ -431,19 +437,19 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - baseService := baseService.DeepCopy() - updateBackwardSpecRecreateService := updateBackwardSpecRecreateService.DeepCopy() - _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEvent(updateBackwardSpecRecreateService, baseService)) + pObj := updateBackwardSpecRecreateService.DeepCopy() + vObj := baseService.DeepCopy() + _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(baseService.DeepCopy(), pObj, vObj, vObj)) assert.NilError(t, err) - err = ctx.VirtualManager.GetClient().Get(ctx, types.NamespacedName{Namespace: baseService.Namespace, Name: baseService.Name}, baseService) + err = ctx.VirtualManager.GetClient().Get(ctx, types.NamespacedName{Namespace: vObj.Namespace, Name: vObj.Name}, vObj) assert.NilError(t, err) - err = ctx.PhysicalManager.GetClient().Get(ctx, types.NamespacedName{Namespace: updateBackwardSpecRecreateService.Namespace, Name: updateBackwardSpecRecreateService.Name}, updateBackwardSpecRecreateService) + err = ctx.PhysicalManager.GetClient().Get(ctx, types.NamespacedName{Namespace: pObj.Namespace, Name: pObj.Name}, pObj) assert.NilError(t, err) - baseService.Spec.ExternalName = updateBackwardSpecService.Spec.ExternalName - _, err = syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(updateBackwardSpecRecreateService.DeepCopy(), updateBackwardSpecRecreateService.DeepCopy(), baseService.DeepCopy(), baseService.DeepCopy())) + pObj.Spec.ExternalName = updateBackwardSpecService.Spec.ExternalName + _, err = syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(pObj.DeepCopy(), pObj.DeepCopy(), vObj.DeepCopy(), vObj.DeepCopy())) assert.NilError(t, err) }, }, @@ -459,7 +465,10 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEvent(updateBackwardStatusService.DeepCopy(), baseService.DeepCopy())) + pObjOld := updateBackwardSpecService.DeepCopy() + pObjNew := updateBackwardStatusService.DeepCopy() + vObj := baseService.DeepCopy() + _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(pObjOld, pObjNew, vObj, vObj)) assert.NilError(t, err) }, }, @@ -475,7 +484,9 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEvent(createdService.DeepCopy(), baseService.DeepCopy())) + pObj := createdService.DeepCopy() + vObj := baseService.DeepCopy() + _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(pObj, pObj, vObj, vObj)) assert.NilError(t, err) }, }, @@ -563,7 +574,11 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEvent(pServiceExternal.DeepCopy(), vServiceClusterIPFromExternal.DeepCopy())) + vObjOld := baseService.DeepCopy() + vObjNew := vServiceClusterIPFromExternal.DeepCopy() + pObj := pServiceExternal.DeepCopy() + + _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(pObj, pObj, vObjOld, vObjNew)) assert.NilError(t, err) }, }, @@ -579,9 +594,14 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEvent(pServiceExternal.DeepCopy(), vServiceNodePortFromExternal.DeepCopy())) + pObjOld := pServiceExternal.DeepCopy() + pObjNew := pServiceExternal.DeepCopy() + vObjOld := baseService.DeepCopy() + vObjNew := vServiceNodePortFromExternal.DeepCopy() + _, err := syncer.(*serviceSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld(pObjOld, pObjNew, vObjOld, vObjNew)) assert.NilError(t, err) }, }, - }) + } + syncertesting.RunTests(t, tests) } diff --git a/pkg/controllers/resources/services/translate.go b/pkg/controllers/resources/services/translate.go index a37aa4c9a..92951c9c6 100644 --- a/pkg/controllers/resources/services/translate.go +++ b/pkg/controllers/resources/services/translate.go @@ -50,3 +50,72 @@ func StripNodePorts(vObj *corev1.Service) { vObj.Spec.Ports[i].NodePort = 0 } } + +// AlignSpecWithServiceType removes any fields that are invalid for the specific service type +func AlignSpecWithServiceType(svc *corev1.Service) { + if svc == nil || svc.Spec.Type == "" { + return + } + + // Default to ClusterIP if type is not specified + if svc.Spec.Type == "" { + svc.Spec.Type = corev1.ServiceTypeClusterIP + } + + switch svc.Spec.Type { + case corev1.ServiceTypeClusterIP: + cleanClusterIPFields(svc) + case corev1.ServiceTypeNodePort: + cleanNodePortFields(svc) + case corev1.ServiceTypeLoadBalancer: + cleanLoadBalancerFields(svc) + case corev1.ServiceTypeExternalName: + cleanExternalNameFields(svc) + } +} + +func cleanClusterIPFields(svc *corev1.Service) { + // Clear fields not valid for ClusterIP + svc.Spec.ExternalTrafficPolicy = "" + svc.Spec.HealthCheckNodePort = 0 + svc.Spec.LoadBalancerIP = "" + svc.Spec.LoadBalancerSourceRanges = nil + svc.Spec.LoadBalancerClass = nil + svc.Spec.ExternalName = "" + svc.Spec.ExternalIPs = nil + svc.Spec.AllocateLoadBalancerNodePorts = nil +} + +func cleanNodePortFields(svc *corev1.Service) { + // NodePort can have all ClusterIP fields plus some additional ones + // Clear fields not valid for NodePort + svc.Spec.LoadBalancerIP = "" + svc.Spec.LoadBalancerSourceRanges = nil + svc.Spec.LoadBalancerClass = nil + svc.Spec.ExternalName = "" +} + +func cleanLoadBalancerFields(svc *corev1.Service) { + // LoadBalancer can have all NodePort fields plus some additional ones + // Only need to clear ExternalName as it inherits from NodePort + svc.Spec.ExternalName = "" +} + +func cleanExternalNameFields(svc *corev1.Service) { + // ExternalName services should only have metadata, type, and externalName + svc.Spec.Ports = nil + svc.Spec.ClusterIP = "" + svc.Spec.ExternalIPs = nil + svc.Spec.LoadBalancerIP = "" + svc.Spec.LoadBalancerSourceRanges = nil + svc.Spec.LoadBalancerClass = nil + svc.Spec.ExternalTrafficPolicy = "" + svc.Spec.HealthCheckNodePort = 0 + svc.Spec.PublishNotReadyAddresses = false + svc.Spec.SessionAffinity = "" + svc.Spec.SessionAffinityConfig = nil + svc.Spec.IPFamilies = nil + svc.Spec.IPFamilyPolicy = nil + svc.Spec.AllocateLoadBalancerNodePorts = nil + svc.Spec.InternalTrafficPolicy = nil +} diff --git a/pkg/controllers/resources/storageclasses/syncer_test.go b/pkg/controllers/resources/storageclasses/syncer_test.go index eecd659c8..5ee3df1ce 100644 --- a/pkg/controllers/resources/storageclasses/syncer_test.go +++ b/pkg/controllers/resources/storageclasses/syncer_test.go @@ -100,7 +100,12 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*storageClassSyncer).Sync(syncCtx, synccontext.NewSyncEvent(pObject.DeepCopy(), vObjectUpdated.DeepCopy())) + _, err := syncer.(*storageClassSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + pObject.DeepCopy(), + pObject.DeepCopy(), + vObject.DeepCopy(), + vObjectUpdated.DeepCopy(), + )) assert.NilError(t, err) }, }, diff --git a/pkg/controllers/resources/volumesnapshotcontents/syncer_test.go b/pkg/controllers/resources/volumesnapshotcontents/syncer_test.go index c3d2245e2..5be059dc5 100644 --- a/pkg/controllers/resources/volumesnapshotcontents/syncer_test.go +++ b/pkg/controllers/resources/volumesnapshotcontents/syncer_test.go @@ -225,7 +225,12 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := newFakeSyncer(t, ctx) - _, err := syncer.Sync(syncCtx, synccontext.NewSyncEvent(pDynamic.DeepCopy(), vDynamic.DeepCopy())) + _, err := syncer.Sync(syncCtx, synccontext.NewSyncEventWithOld( + pDynamic.DeepCopy(), + pDynamic.DeepCopy(), + vDynamic.DeepCopy(), + vDynamic.DeepCopy(), + )) assert.NilError(t, err) }, }, @@ -241,7 +246,12 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := newFakeSyncer(t, ctx) - _, err := syncer.Sync(syncCtx, synccontext.NewSyncEvent(pDynamic.DeepCopy(), vInvalidMutation.DeepCopy())) + _, err := syncer.Sync(syncCtx, synccontext.NewSyncEventWithOld( + pDynamic.DeepCopy(), + pDynamic.DeepCopy(), + vDynamic.DeepCopy(), + vInvalidMutation.DeepCopy(), + )) assert.NilError(t, err) }, }, @@ -257,7 +267,12 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := newFakeSyncer(t, ctx) - _, err := syncer.Sync(syncCtx, synccontext.NewSyncEvent(pWithStatus.DeepCopy(), vWithGCFinalizer.DeepCopy())) + _, err := syncer.Sync(syncCtx, synccontext.NewSyncEventWithOld( + pWithStatus.DeepCopy(), + pWithStatus.DeepCopy(), + vWithGCFinalizer.DeepCopy(), + vWithGCFinalizer.DeepCopy(), + )) assert.NilError(t, err) }, }, @@ -273,7 +288,12 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := newFakeSyncer(t, ctx) - _, err := syncer.Sync(syncCtx, synccontext.NewSyncEvent(pModifiedDeletionPolicy.DeepCopy(), vModifiedDeletionPolicy.DeepCopy())) + _, err := syncer.Sync(syncCtx, synccontext.NewSyncEventWithOld( + pPreProvisioned.DeepCopy(), + pModifiedDeletionPolicy.DeepCopy(), + vModifiedDeletionPolicy.DeepCopy(), + vModifiedDeletionPolicy.DeepCopy(), + )) assert.NilError(t, err) }, }, @@ -289,7 +309,12 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := newFakeSyncer(t, ctx) - _, err := syncer.Sync(syncCtx, synccontext.NewSyncEvent(pPreProvisioned.DeepCopy(), vDeleting.DeepCopy())) + _, err := syncer.Sync(syncCtx, synccontext.NewSyncEventWithOld( + pPreProvisioned.DeepCopy(), + pPreProvisioned.DeepCopy(), + vDeleting.DeepCopy(), + vDeleting.DeepCopy(), + )) assert.NilError(t, err) }, }, @@ -321,7 +346,12 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := newFakeSyncer(t, ctx) - _, err := syncer.Sync(syncCtx, synccontext.NewSyncEvent(pDeletingWithOneFinalizer.DeepCopy(), vDeletingWithMoreFinalizers.DeepCopy())) + _, err := syncer.Sync(syncCtx, synccontext.NewSyncEventWithOld( + pDeletingWithOneFinalizer.DeepCopy(), + pDeletingWithOneFinalizer.DeepCopy(), + vDeletingWithMoreFinalizers.DeepCopy(), + vDeletingWithMoreFinalizers.DeepCopy(), + )) assert.NilError(t, err) }, }, @@ -337,7 +367,12 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := newFakeSyncer(t, ctx) - _, err := syncer.Sync(syncCtx, synccontext.NewSyncEvent(pDeletingWithStatus.DeepCopy(), vDeletingWithOneFinalizer.DeepCopy())) + _, err := syncer.Sync(syncCtx, synccontext.NewSyncEventWithOld( + pDeletingWithStatus.DeepCopy(), + pDeletingWithStatus.DeepCopy(), + vDeletingWithOneFinalizer.DeepCopy(), + vDeletingWithOneFinalizer.DeepCopy(), + )) assert.NilError(t, err) }, }, diff --git a/pkg/controllers/resources/volumesnapshots/syncer_test.go b/pkg/controllers/resources/volumesnapshots/syncer_test.go index 1d29fae83..f44b47b65 100644 --- a/pkg/controllers/resources/volumesnapshots/syncer_test.go +++ b/pkg/controllers/resources/volumesnapshots/syncer_test.go @@ -156,7 +156,12 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*volumeSnapshotSyncer).Sync(syncCtx, synccontext.NewSyncEvent(pPVSourceSnapshot.DeepCopy(), vVSCSourceSnapshot.DeepCopy())) + _, err := syncer.(*volumeSnapshotSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + pPVSourceSnapshot.DeepCopy(), + pPVSourceSnapshot.DeepCopy(), + vVSCSourceSnapshot.DeepCopy(), + vVSCSourceSnapshot.DeepCopy(), + )) assert.NilError(t, err) }, }, @@ -172,7 +177,12 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*volumeSnapshotSyncer).Sync(syncCtx, synccontext.NewSyncEvent(pWithNilClass.DeepCopy(), vPVSourceSnapshot.DeepCopy())) + _, err := syncer.(*volumeSnapshotSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + pWithNilClass.DeepCopy(), + pWithNilClass.DeepCopy(), + vPVSourceSnapshot.DeepCopy(), + vPVSourceSnapshot.DeepCopy(), + )) assert.NilError(t, err) }, }, @@ -188,7 +198,12 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*volumeSnapshotSyncer).Sync(syncCtx, synccontext.NewSyncEvent(pWithNilClass.DeepCopy(), vWithNilClass.DeepCopy())) + _, err := syncer.(*volumeSnapshotSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + pWithNilClass.DeepCopy(), + pWithNilClass.DeepCopy(), + vWithNilClass.DeepCopy(), + vWithNilClass.DeepCopy(), + )) assert.NilError(t, err) }, }, @@ -204,7 +219,12 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*volumeSnapshotSyncer).Sync(syncCtx, synccontext.NewSyncEvent(pWithFinalizers, vPVSourceSnapshot.DeepCopy())) + _, err := syncer.(*volumeSnapshotSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + pWithFinalizers, + pWithFinalizers, + vPVSourceSnapshot.DeepCopy(), + vPVSourceSnapshot.DeepCopy(), + )) assert.NilError(t, err) }, }, @@ -220,7 +240,12 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*volumeSnapshotSyncer).Sync(syncCtx, synccontext.NewSyncEvent(pWithStatus, vPVSourceSnapshot.DeepCopy())) + _, err := syncer.(*volumeSnapshotSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + pWithStatus, + pWithStatus, + vPVSourceSnapshot.DeepCopy(), + vPVSourceSnapshot.DeepCopy(), + )) assert.NilError(t, err) }, }, @@ -236,7 +261,12 @@ func TestSync(t *testing.T) { }, Sync: func(ctx *synccontext.RegisterContext) { syncCtx, syncer := syncertesting.FakeStartSyncer(t, ctx, New) - _, err := syncer.(*volumeSnapshotSyncer).Sync(syncCtx, synccontext.NewSyncEvent(pPVSourceSnapshot.DeepCopy(), vDeletingSnapshot)) + _, err := syncer.(*volumeSnapshotSyncer).Sync(syncCtx, synccontext.NewSyncEventWithOld( + pPVSourceSnapshot.DeepCopy(), + pPVSourceSnapshot.DeepCopy(), + vDeletingSnapshot, + vDeletingSnapshot, + )) assert.NilError(t, err) }, }, diff --git a/pkg/patcher/sync.go b/pkg/patcher/sync.go index 9a11761c1..e9cdd4e4b 100644 --- a/pkg/patcher/sync.go +++ b/pkg/patcher/sync.go @@ -3,6 +3,7 @@ package patcher import ( "encoding/json" "fmt" + "github.com/loft-sh/vcluster/pkg/util/generics" jsonpatch "github.com/evanphx/json-patch/v5" apiequality "k8s.io/apimachinery/pkg/api/equality" @@ -11,9 +12,9 @@ import ( func CopyBidirectional[T any](virtualOld, virtual, hostOld, host T) (T, T) { newVirtual := virtual newHost := host - if !apiequality.Semantic.DeepEqual(virtualOld, virtual) { + if generics.IsNilOrEmpty(host) || !apiequality.Semantic.DeepEqual(virtualOld, virtual) { newHost = virtual - } else if !apiequality.Semantic.DeepEqual(hostOld, host) { + } else if generics.IsNilOrEmpty(virtual) || !apiequality.Semantic.DeepEqual(hostOld, host) { newVirtual = host } @@ -25,9 +26,9 @@ func MergeBidirectional[T any](virtualOld, virtual, hostOld, host T) (T, T, erro newVirtual := virtual newHost := host - if !apiequality.Semantic.DeepEqual(virtualOld, virtual) { + if generics.IsNilOrEmpty(host) || !apiequality.Semantic.DeepEqual(virtualOld, virtual) { newHost, err = MergeChangesInto(virtualOld, virtual, host) - } else if !apiequality.Semantic.DeepEqual(hostOld, host) { + } else if generics.IsNilOrEmpty(virtual) || !apiequality.Semantic.DeepEqual(hostOld, host) { newVirtual, err = MergeChangesInto(hostOld, host, virtual) } @@ -35,6 +36,10 @@ func MergeBidirectional[T any](virtualOld, virtual, hostOld, host T) (T, T, erro } func MergeChangesInto[T any](oldValue, newValue, outValue T) (T, error) { + if generics.IsNilOrEmpty(outValue) { + return newValue, nil + } + var ret T oldValueBytes, err := json.Marshal(oldValue) if err != nil { @@ -50,9 +55,6 @@ func MergeChangesInto[T any](oldValue, newValue, outValue T) (T, error) { if err != nil { return ret, fmt.Errorf("marshal out value: %w", err) } - if string(outBytes) == "null" { - outBytes = []byte("{}") - } patchBytes, err := jsonpatch.CreateMergePatch(oldValueBytes, newValueBytes) if err != nil { diff --git a/pkg/syncer/syncer_test.go b/pkg/syncer/syncer_test.go index 66f952a12..fff0ae1bd 100644 --- a/pkg/syncer/syncer_test.go +++ b/pkg/syncer/syncer_test.go @@ -422,6 +422,9 @@ func TestReconcile(t *testing.T) { Namespace: namespaceInVClusterA, UID: "123", }, + Data: map[string][]byte{ + "datakey1": []byte("datavalue1"), + }, }, }, }, @@ -446,6 +449,9 @@ func TestReconcile(t *testing.T) { translate.NamespaceLabel: namespaceInVClusterA, }, }, + Data: map[string][]byte{ + "datakey1": []byte("datavalue1"), + }, }, }, }, diff --git a/pkg/util/generics/generics.go b/pkg/util/generics/generics.go new file mode 100644 index 000000000..24b032d3f --- /dev/null +++ b/pkg/util/generics/generics.go @@ -0,0 +1,46 @@ +package generics + +import "reflect" + +func IsNilOrEmpty[T any](v T) bool { + i := interface{}(v) + + // Check if the value is nil + if i == nil { + return true + } + + rv := reflect.ValueOf(i) + + // Check if it's nil for types that can be nil + switch rv.Kind() { + case reflect.Ptr, reflect.Interface, reflect.Slice, reflect.Map, reflect.Func, reflect.Chan: + if rv.IsNil() { + return true + } + default: + } + + // Check if it's a zero value + zeroValue := reflect.Zero(rv.Type()).Interface() + return reflect.DeepEqual(i, zeroValue) +} + +func Filter[T any](slice []T, predicate func(T) bool) []T { + var filtered []T + for _, item := range slice { + if predicate(item) { + filtered = append(filtered, item) + } + } + return filtered +} + +func Every[T any](slice []T, predicate func(T) bool) bool { + for _, item := range slice { + if !predicate(item) { + return false + } + } + return true +} diff --git a/pkg/util/translate/labels.go b/pkg/util/translate/labels.go index 49b5f3c31..dad937fe2 100644 --- a/pkg/util/translate/labels.go +++ b/pkg/util/translate/labels.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/loft-sh/vcluster/pkg/syncer/synccontext" + "github.com/loft-sh/vcluster/pkg/util/generics" "github.com/loft-sh/vcluster/pkg/util/stringutil" apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -228,8 +229,14 @@ func MergeLabelSelectors(elems ...*metav1.LabelSelector) *metav1.LabelSelector { func AnnotationsBidirectionalUpdateFunction[T client.Object](event *synccontext.SyncEvent[T], transformFromHost, transformToHost func(key string, value interface{}) (string, interface{})) (map[string]string, map[string]string) { excludeAnnotations := []string{HostNameAnnotation, HostNamespaceAnnotation, NameAnnotation, UIDAnnotation, KindAnnotation, NamespaceAnnotation, ManagedAnnotationsAnnotation, ManagedLabelsAnnotation} newVirtual := maps.Clone(event.Virtual.GetAnnotations()) + if generics.IsNilOrEmpty(newVirtual) { + newVirtual = map[string]string{} + } newHost := maps.Clone(event.Host.GetAnnotations()) - if !apiequality.Semantic.DeepEqual(event.VirtualOld.GetAnnotations(), event.Virtual.GetAnnotations()) { + if generics.IsNilOrEmpty(newHost) { + newHost = map[string]string{} + } + if generics.IsNilOrEmpty(newHost) || !apiequality.Semantic.DeepEqual(event.VirtualOld.GetAnnotations(), event.Virtual.GetAnnotations()) { newHost = mergeMaps(event.VirtualOld.GetAnnotations(), event.Virtual.GetAnnotations(), event.Host.GetAnnotations(), func(key string, value interface{}) (string, interface{}) { if stringutil.Contains(excludeAnnotations, key) { return "", nil @@ -239,7 +246,7 @@ func AnnotationsBidirectionalUpdateFunction[T client.Object](event *synccontext. return transformToHost(key, value) }) - } else if !apiequality.Semantic.DeepEqual(event.HostOld.GetAnnotations(), event.Host.GetAnnotations()) { + } else if generics.IsNilOrEmpty(newVirtual) || !apiequality.Semantic.DeepEqual(event.HostOld.GetAnnotations(), event.Host.GetAnnotations()) { newVirtual = mergeMaps(event.HostOld.GetAnnotations(), event.Host.GetAnnotations(), event.Virtual.GetAnnotations(), func(key string, value interface{}) (string, interface{}) { if stringutil.Contains(excludeAnnotations, key) { return "", nil @@ -275,7 +282,7 @@ func LabelsBidirectionalUpdateFunction[T client.Object](event *synccontext.SyncE func LabelsBidirectionalUpdateFunctionMaps(virtualOld, virtual, hostOld, host map[string]string, transformFromHost, transformToHost func(key string, value interface{}) (string, interface{})) (map[string]string, map[string]string) { newVirtual := virtual newHost := host - if !apiequality.Semantic.DeepEqual(virtualOld, virtual) { + if generics.IsNilOrEmpty(newVirtual) || !apiequality.Semantic.DeepEqual(virtualOld, virtual) { newHost = mergeMaps(virtualOld, virtual, host, func(key string, value interface{}) (string, interface{}) { key = HostLabel(key) if transformToHost == nil { @@ -284,7 +291,7 @@ func LabelsBidirectionalUpdateFunctionMaps(virtualOld, virtual, hostOld, host ma return transformToHost(key, value) }) - } else if !apiequality.Semantic.DeepEqual(hostOld, host) { + } else if generics.IsNilOrEmpty(newVirtual) || !apiequality.Semantic.DeepEqual(hostOld, host) { newVirtual = mergeMaps(hostOld, host, virtual, func(key string, value interface{}) (string, interface{}) { key, _ = VirtualLabel(key) if transformFromHost == nil { @@ -322,10 +329,10 @@ func LabelsBidirectionalUpdateMaps(virtualOld, virtual, hostOld, host map[string return LabelsBidirectionalUpdateFunctionMaps(virtualOld, virtual, hostOld, host, excludeFn, excludeFn) } -func mergeMaps(beforeMap, afterMap, targetMap map[string]string, transformKey func(key string, value interface{}) (string, interface{})) map[string]string { - retMap := maps.Clone(targetMap) - if retMap == nil { - retMap = map[string]string{} +func mergeMaps(beforeMap, afterMap, targetMap map[string]string, transformKey func(key string, value interface{}) (string, interface{})) (retMap map[string]string) { + // If the target map is empty merge with an empty before map to get all the changes + if retMap = maps.Clone(targetMap); retMap == nil { + return mergeMaps(map[string]string{}, afterMap, map[string]string{}, transformKey) } // get diff map diff --git a/pkg/util/translate/translate.go b/pkg/util/translate/translate.go index 426d9782a..c4a2f7681 100644 --- a/pkg/util/translate/translate.go +++ b/pkg/util/translate/translate.go @@ -104,6 +104,9 @@ func HostAnnotations(vObj, pObj client.Object, excluded ...string) map[string]st toAnnotations := map[string]string{} if pObj != nil { toAnnotations = pObj.GetAnnotations() + if toAnnotations == nil { + toAnnotations = map[string]string{} + } } retMap := applyAnnotations(vObj.GetAnnotations(), toAnnotations, excluded...)