Merge pull request #123 from tpantelis/fix_two_cluster_export

Ensure first cluster exports service before second cluster

Author: k8s-ci-robot

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)))
+		}
 	})
 }