Ensure first cluster exports service before second cluster

...in two cluster conformance tests. Previously, the tests exported
the first cluster in JustBeforeEach but that was executed after the
JustBeforeEach in the twoClusterTestDriver so the second cluster was
actually exported first.

In addition, the verification of service type was put back in for the
service type conflict conformance test. This had been removed by
#119. To make the
conflict resolution deterministic for an implementation that uses
the first observed ServiceExport, the test setup now ensures the
ServiceImport is created before deploying on the second cluster.

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>

Author: tpantelis

conformance/clusterip_service_dns.go

@@ -32,10 +32,6 @@ import (
 var _ = Describe("", Label(OptionalLabel, DNSLabel, ClusterIPLabel), func() {
 	t := newTestDriver()
 
-	JustBeforeEach(func() {
-		t.createServiceExport(&clients[0], newHelloServiceExport())
-	})
-
 	Specify("A DNS lookup of the <service>.<ns>.svc.clusterset.local domain for a ClusterIP service should resolve to the "+
 		"clusterset IP", func() {
 		AddReportEntry(SpecRefReportEntry, "https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/1645-multi-cluster-services-api#dns")

conformance/conformance_suite.go

@@ -144,10 +144,11 @@ func setupClients() error {
 }
 
 type testDriver struct {
-	namespace       string
-	helloService    *corev1.Service
-	helloDeployment *appsv1.Deployment
-	requestPod      *corev1.Pod
+	namespace         string
+	helloService      *corev1.Service
+	helloDeployment   *appsv1.Deployment
+	requestPod        *corev1.Pod
+	autoExportService bool
 }
 
 func newTestDriver() *testDriver {
@@ -158,6 +159,7 @@ func newTestDriver() *testDriver {
 		t.helloService = newHelloService()
 		t.helloDeployment = newHelloDeployment()
 		t.requestPod = newRequestPod()
+		t.autoExportService = true
 	})
 
 	JustBeforeEach(func() {
@@ -178,6 +180,10 @@ func newTestDriver() *testDriver {
 		for _, client := range clients {
 			t.startRequestPod(ctx, client)
 		}
+
+		if t.autoExportService {
+			t.createServiceExport(&clients[0], newHelloServiceExport())
+		}
 	})
 
 	AfterEach(func() {
@@ -353,11 +359,21 @@ func newTwoClusterTestDriver(t *testDriver) *twoClusterTestDriver {
 
 		tt.helloService2 = newHelloService()
 		tt.helloServiceExport2 = newHelloServiceExport()
+		t.autoExportService = false
 	})
 
 	JustBeforeEach(func() {
+		t.createServiceExport(&clients[0], newHelloServiceExport())
+
+		// The conflict resolution policy in the MCS spec (KEP 1645) allows an implementation to favor maintaining
+		// service continuity and avoiding potentially disruptive changes, as such, an implementation may choose the
+		// first observed exported service when resolving conflicts. To support this, verify the ServiceImport is
+		// created on the first cluster prior to deploying on the second cluster.
+		t.awaitServiceImport(&clients[0], helloServiceName, false, nil)
+
 		// Delay a little before deploying on the second cluster to ensure the first cluster's ServiceExport timestamp
-		// is older so conflict checking is deterministic. Make the delay at least 1 sec as creation timestamps have seconds granularity.
+		// is older so conflict checking is deterministic for implementations that use the timestamp when resolving conflicts.
+		// Make the delay at least 1 sec as creation timestamps have seconds granularity.
 		time.Sleep(1100 * time.Millisecond)
 
 		t.deployHelloService(&clients[1], tt.helloService2)

conformance/connectivity.go

@@ -29,6 +29,10 @@ import (
 var _ = Describe("", func() {
 	t := newTestDriver()
 
+	BeforeEach(func() {
+		t.autoExportService = false
+	})
+
 	Context("Connectivity to a service that is not exported", func() {
 		It("should be inaccessible", Label(RequiredLabel), func() {
 			AddReportEntry(SpecRefReportEntry, "https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/1645-multi-cluster-services-api#exporting-services")

conformance/endpoint_slice.go

@@ -34,10 +34,6 @@ const K8sEndpointSliceManagedByName = "endpointslice-controller.k8s.io"
 var _ = Describe("", Label(OptionalLabel, EndpointSliceLabel), func() {
 	t := newTestDriver()
 
-	JustBeforeEach(func() {
-		t.createServiceExport(&clients[0], newHelloServiceExport())
-	})
-
 	Specify("Exporting a service should create an MCS EndpointSlice in the service's namespace in each cluster with the "+
 		"required MCS labels. Unexporting should delete the EndpointSlice.", func() {
 		AddReportEntry(SpecRefReportEntry, "https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/1645-multi-cluster-services-api#using-endpointslice-objects-to-track-endpoints")

conformance/headless_service_dns.go

@@ -45,10 +45,6 @@ var _ = Describe("", Label(OptionalLabel, DNSLabel, HeadlessLabel), func() {
 		t.helloDeployment.Spec.Replicas = ptr.To(int32(replicas))
 	})
 
-	JustBeforeEach(func() {
-		t.createServiceExport(&clients[0], newHelloServiceExport())
-	})
-
 	Specify("A DNS query of the <service>.<ns>.svc.clusterset.local domain for a headless service should return the "+
 		"ready endpoint addresses of all the backing pods", func() {
 		AddReportEntry(SpecRefReportEntry, "https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/1645-multi-cluster-services-api#dns")

conformance/service_import.go

@@ -44,10 +44,6 @@ func testGeneralServiceImport() {
 		helloServiceExport = newHelloServiceExport()
 	})
 
-	JustBeforeEach(func() {
-		t.createServiceExport(&clients[0], helloServiceExport)
-	})
-
 	assertHasKeyValues := func(g Gomega, actual, expected map[string]string) {
 		for k, v := range expected {
 			g.Expect(actual).To(HaveKeyWithValue(k, v), reportNonConformant(""))
@@ -148,17 +144,8 @@ func testGeneralServiceImport() {
 }
 
 func testClusterIPServiceImport() {
-	var helloServiceExport *v1alpha1.ServiceExport
 	t := newTestDriver()
 
-	BeforeEach(func() {
-		helloServiceExport = newHelloServiceExport()
-	})
-
-	JustBeforeEach(func() {
-		t.createServiceExport(&clients[0], helloServiceExport)
-	})
-
 	Specify("Exporting a ClusterIP service should create a ServiceImport of type ClusterSetIP in the service's namespace in each cluster. "+
 		"Unexporting should delete the ServiceImport", Label(RequiredLabel), func() {
 		AddReportEntry(SpecRefReportEntry, "https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/1645-multi-cluster-services-api#importing-services")
@@ -267,10 +254,6 @@ func testHeadlessServiceImport() {
 		t.helloService.Spec.ClusterIP = corev1.ClusterIPNone
 	})
 
-	JustBeforeEach(func() {
-		t.createServiceExport(&clients[0], newHelloServiceExport())
-	})
-
 	Specify("Exporting a headless service should create a ServiceImport of type Headless in the service's namespace in each cluster. "+
 		"Unexporting should delete the ServiceImport", Label(RequiredLabel), func() {
 		AddReportEntry(SpecRefReportEntry, "https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/1645-multi-cluster-services-api#service-types")
@@ -309,10 +292,6 @@ func testExternalNameService() {
 		t.helloService.Spec.ExternalName = "example.com"
 	})
 
-	JustBeforeEach(func() {
-		t.createServiceExport(&clients[0], newHelloServiceExport())
-	})
-
 	Specify("Exporting an ExternalName service should set ServiceExport Valid condition to False", Label(RequiredLabel), func() {
 		AddReportEntry(SpecRefReportEntry, "https://github.com/kubernetes/enhancements/blob/master/keps/sig-multicluster/1645-multi-cluster-services-api/README.md#service-types")
 
@@ -329,15 +308,18 @@ func testServiceTypeConflict() {
 		t.helloService2.Spec.ClusterIP = corev1.ClusterIPNone
 	})
 
-	JustBeforeEach(func() {
-		t.createServiceExport(&clients[0], newHelloServiceExport())
-	})
-
 	Specify("A service exported on two clusters with conflicting headlessness should apply the conflict resolution policy and "+
 		"report a Conflict condition on the ServiceExport", Label(RequiredLabel), func() {
 		AddReportEntry(SpecRefReportEntry, "https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/1645-multi-cluster-services-api#headlessness")
 
 		t.awaitServiceExportCondition(&clients[0], v1alpha1.ServiceExportConditionConflict, metav1.ConditionTrue)
 		t.awaitServiceExportCondition(&clients[1], v1alpha1.ServiceExportConditionConflict, metav1.ConditionTrue)
+
+		for i := range clients {
+			serviceImport := t.awaitServiceImport(&clients[i], helloServiceName, true, nil)
+
+			Expect(serviceImport.Spec.Type).To(Equal(v1alpha1.ClusterSetIP), reportNonConformant(
+				fmt.Sprintf("ServiceImport on cluster %q has type %q", clients[i].name, serviceImport.Spec.Type)))
+		}
 	})
 }