Merge pull request #41254 from shashidharatd/federation-service-e2e-1

Automatic merge from submit-queue (batch tested with PRs 45052, 44983, 41254)

[Federation][e2e] Add 2 new testcases to federation service e2e

**What this PR does / why we need it**:
Add 2 new test cases for federation services
- Federation service updation should update clustered service
- Federation service controller should recreate service shard in cluster, if it gets deleted.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #27623, #35827
Handles one of the tasks discussed in #41253

**Special notes for your reviewer**:

**Release note**:
`NONE`

cc @kubernetes/sig-federation-bugs, @nikhiljindal @madhusudancs

test/e2e_federation/service.go

@@ -108,7 +108,7 @@ var _ = framework.KubeDescribe("Federated Services [Feature:Federation]", func()
 				}
 			})
 
-			It("should create matching services in underlying clusters", func() {
+			It("should create and update matching services in underlying clusters", func() {
 				fedframework.SkipUnlessFederated(f.ClientSet)
 				service = createServiceOrFail(f.FederationClientset, nsName, FederatedServiceName)
 				defer func() { // Cleanup
@@ -116,7 +116,13 @@ var _ = framework.KubeDescribe("Federated Services [Feature:Federation]", func()
 					err := f.FederationClientset.Services(nsName).Delete(service.Name, &metav1.DeleteOptions{})
 					framework.ExpectNoError(err, "Error deleting service %q in namespace %q", service.Name, nsName)
 				}()
+				By(fmt.Sprintf("Wait for service shards to be created in all clusters for service \"%s/%s\"", nsName, service.Name))
 				waitForServiceShardsOrFail(nsName, service, clusters)
+				framework.Logf("Successfully created and synced service \"%s/%s\" to all clusters", nsName, service.Name)
+				By(fmt.Sprintf("Update federated service \"%s/%s\"", nsName, service.Name))
+				service = updateServiceOrFail(f.FederationClientset, nsName, FederatedServiceName)
+				waitForServiceShardsOrFail(nsName, service, clusters)
+				framework.Logf("Successfully updated and synced service \"%s/%s\" to clusters", nsName, service.Name)
 			})
 
 			It("should be deleted from underlying clusters when OrphanDependents is false", func() {
@@ -138,6 +144,28 @@ var _ = framework.KubeDescribe("Federated Services [Feature:Federation]", func()
 				verifyCascadingDeletionForService(f.FederationClientset, clusters, nil, nsName)
 				By(fmt.Sprintf("Verified that services were not deleted from underlying clusters"))
 			})
+
+			It("should recreate service shard in underlying clusters when service shard is deleted", func() {
+				fedframework.SkipUnlessFederated(f.ClientSet)
+				service = createServiceOrFail(f.FederationClientset, nsName, FederatedServiceName)
+				defer func() {
+					// Cleanup
+					By(fmt.Sprintf("Deleting service %q in namespace %q", service.Name, nsName))
+					err := f.FederationClientset.Services(nsName).Delete(service.Name, &metav1.DeleteOptions{})
+					framework.ExpectNoError(err, "Error deleting service %q in namespace %q", service.Name, nsName)
+				}()
+				By(fmt.Sprintf("Wait for service shards to be created in all clusters for service \"%s/%s\"", nsName, service.Name))
+				waitForServiceShardsOrFail(nsName, service, clusters)
+				framework.Logf("Successfully created and synced service \"%s/%s\" to all clusters", nsName, service.Name)
+
+				By(fmt.Sprintf("Deleting a service shard in one underlying cluster"))
+				primaryClusterName := clusters[0].Name
+				err := deleteServiceShard(clusters[0], nsName, FederatedServiceName)
+				framework.ExpectNoError(err, fmt.Sprintf("while deleting service shard %q in cluster %q", FederatedServiceName, primaryClusterName))
+
+				waitForServiceShardsOrFail(nsName, service, clusters)
+				framework.Logf("Successfully recreated service shard \"%s/%s\" in %q cluster", nsName, service.Name, primaryClusterName)
+			})
 		})
 
 		var _ = Describe("DNS", func() {
@@ -330,18 +358,41 @@ func verifyCascadingDeletionForService(clientset *fedclientset.Clientset, cluste
 	}
 }
 
+func updateServiceOrFail(clientset *fedclientset.Clientset, namespace, name string) *v1.Service {
+	service, err := clientset.Services(namespace).Get(name, metav1.GetOptions{})
+	framework.ExpectNoError(err, "Getting service %q in namespace %q", name, namespace)
+	service.Spec.Selector["name"] = "update-demo"
+	newService, err := clientset.Services(namespace).Update(service)
+	By(fmt.Sprintf("Successfully updated federated service %q in namespace %q", name, namespace))
+	return newService
+}
+
+func deleteServiceShard(c *fedframework.Cluster, namespace, service string) error {
+	err := c.Clientset.Services(namespace).Delete(service, &metav1.DeleteOptions{})
+	if err != nil && !errors.IsNotFound(err) {
+		framework.Logf("Failed to delete service %q in namespace %q, in cluster %q", service, namespace, c.Name)
+		return err
+	}
+	By(fmt.Sprintf("Service %q in namespace %q in cluster %q deleted", service, namespace, c.Name))
+	return nil
+}
+
 // equivalent returns true if the two services are equivalent.  Fields which are expected to differ between
-// federated services and the underlying cluster services (e.g. ClusterIP, LoadBalancerIP etc) are ignored.
+// federated services and the underlying cluster services (e.g. ClusterIP, NodePort) are ignored.
 func equivalent(federationService, clusterService v1.Service) bool {
-	// TODO: I think that we need a DeepCopy here to avoid clobbering our parameters.
 	clusterService.Spec.ClusterIP = federationService.Spec.ClusterIP
-	clusterService.Spec.ExternalIPs = federationService.Spec.ExternalIPs
-	clusterService.Spec.LoadBalancerIP = federationService.Spec.LoadBalancerIP
-	clusterService.Spec.LoadBalancerSourceRanges = federationService.Spec.LoadBalancerSourceRanges
-	// N.B. We cannot iterate over the port objects directly, as their values
-	// only get copied and our updates will get lost.
 	for i := range clusterService.Spec.Ports {
 		clusterService.Spec.Ports[i].NodePort = federationService.Spec.Ports[i].NodePort
 	}
-	return reflect.DeepEqual(clusterService.Spec, federationService.Spec)
+
+	if federationService.Name != clusterService.Name || federationService.Namespace != clusterService.Namespace {
+		return false
+	}
+	if !reflect.DeepEqual(federationService.Labels, clusterService.Labels) && (len(federationService.Labels) != 0 || len(clusterService.Labels) != 0) {
+		return false
+	}
+	if !reflect.DeepEqual(federationService.Spec, clusterService.Spec) {
+		return false
+	}
+	return true
 }