Fix unit tests for sync-refactoring (#2260)

* wip * ingress tests passing * test: update resources tests for syncer refactor * test: update poddistruptionbudgets tests for syncer refactor * test: update syncer tests for syncer refactor * test: update pod tests for syncer refactor * test: fixes, adds defaults for empty values * Sanitize service spec by type * Fix for conformance tests --------- Co-authored-by: RyanSwanson <[email protected]>
loft-sh · Nov 4, 2024 · 09248b8 · 09248b8
1 parent 73263b1
commit 09248b8
Show file tree

Hide file tree

Showing 19 changed files with 503 additions and 161 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)
 			},
 		},