Stop deleting underlying services when federation service is deleted

federation/pkg/federation-controller/service/servicecontroller.go

@@ -393,41 +393,6 @@ func (s *ServiceController) updateFederationService(key string, cachedService *c
 	return nil, !retryable
 }
 
-func (s *ServiceController) deleteFederationService(cachedService *cachedService) (error, bool) {
-	// handle available clusters one by one
-	var hasErr bool
-	for clusterName, cluster := range s.clusterCache.clientMap {
-		err := s.deleteClusterService(clusterName, cachedService, cluster.clientset)
-		if err != nil {
-			hasErr = true
-		} else if err := s.ensureDnsRecords(clusterName, cachedService); err != nil {
-			hasErr = true
-		}
-	}
-	if hasErr {
-		// detail error has been dumpped inside the loop
-		return fmt.Errorf("Service %s/%s was not successfully updated to all clusters", cachedService.lastState.Namespace, cachedService.lastState.Name), retryable
-	}
-	return nil, !retryable
-}
-
-func (s *ServiceController) deleteClusterService(clusterName string, cachedService *cachedService, clientset *kubeclientset.Clientset) error {
-	service := cachedService.lastState
-	glog.V(4).Infof("Deleting service %s/%s from cluster %s", service.Namespace, service.Name, clusterName)
-	var err error
-	for i := 0; i < clientRetryCount; i++ {
-		err = clientset.Core().Services(service.Namespace).Delete(service.Name, &v1.DeleteOptions{})
-		if err == nil || errors.IsNotFound(err) {
-			glog.V(4).Infof("Service %s/%s deleted from cluster %s", service.Namespace, service.Name, clusterName)
-			delete(cachedService.endpointMap, clusterName)
-			return nil
-		}
-		time.Sleep(cachedService.nextRetryDelay())
-	}
-	glog.V(4).Infof("Failed to delete service %s/%s from cluster %s, %+v", service.Namespace, service.Name, clusterName, err)
-	return err
-}
-
 func (s *ServiceController) ensureClusterService(cachedService *cachedService, clusterName string, service *v1.Service, client *kubeclientset.Clientset) error {
 	var err error
 	var needUpdate bool
@@ -931,31 +896,6 @@ func (s *ServiceController) processServiceUpdate(cachedService *cachedService, s
 // we should retry in that Duration.
 func (s *ServiceController) processServiceDeletion(key string) (error, time.Duration) {
 	glog.V(2).Infof("Process service deletion for %v", key)
-	cachedService, ok := s.serviceCache.get(key)
-	if !ok {
-		return fmt.Errorf("Service %s not in cache even though the watcher thought it was. Ignoring the deletion.", key), doNotRetry
-	}
-	service := cachedService.lastState
-	cachedService.rwlock.Lock()
-	defer cachedService.rwlock.Unlock()
-	s.eventRecorder.Event(service, v1.EventTypeNormal, "DeletingDNSRecord", "Deleting DNS Records")
-	// TODO should we delete dns info here or wait for endpoint changes? prefer here
-	// or we do nothing for service deletion
-	//err := s.dns.balancer.EnsureLoadBalancerDeleted(service)
-	err, retry := s.deleteFederationService(cachedService)
-	if err != nil {
-		message := "Error occurs when deleting federation service"
-		if retry {
-			message += " (will retry): "
-		} else {
-			message += " (will not retry): "
-		}
-		s.eventRecorder.Event(service, v1.EventTypeWarning, "DeletingDNSRecordFailed", message)
-		return err, cachedService.nextRetryDelay()
-	}
-	s.eventRecorder.Event(service, v1.EventTypeNormal, "DeletedDNSRecord", "Deleted DNS Records")
 	s.serviceCache.delete(key)
-
-	cachedService.resetRetryDelay()
 	return nil, doNotRetry
 }

test/e2e/federated-service.go

@@ -110,6 +110,27 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() {
 				}()
 				waitForServiceShardsOrFail(nsName, service, clusters)
 			})
+
+			It("should not be deleted from underlying clusters when it is deleted", func() {
+				framework.SkipUnlessFederated(f.ClientSet)
+				nsName = f.FederationNamespace.Name
+				service = createServiceOrFail(f.FederationClientset_1_5, nsName, FederatedServiceName)
+				By(fmt.Sprintf("Successfully created federated service %q in namespace %q. Waiting for shards to appear in underlying clusters", service.Name, nsName))
+
+				waitForServiceShardsOrFail(nsName, service, clusters)
+
+				By(fmt.Sprintf("Deleting service %s", service.Name))
+				err := f.FederationClientset_1_5.Services(nsName).Delete(service.Name, &v1.DeleteOptions{})
+				framework.ExpectNoError(err, "Error deleting service %q in namespace %q", service.Name, service.Namespace)
+				By(fmt.Sprintf("Deletion of service %q in namespace %q succeeded.", service.Name, nsName))
+				By(fmt.Sprintf("Verifying that services in underlying clusters are not deleted"))
+				for clusterName, clusterClientset := range clusters {
+					_, err := clusterClientset.Core().Services(service.Namespace).Get(service.Name)
+					if err != nil {
+						framework.Failf("Unexpected error in fetching service %s in cluster %s, %s", service.Name, clusterName, err)
+					}
+				}
+			})
 		})
 
 		var _ = Describe("DNS", func() {
@@ -162,6 +183,15 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() {
 				for i, DNSName := range svcDNSNames {
 					discoverService(f, DNSName, true, "federated-service-e2e-discovery-pod-"+strconv.Itoa(i))
 				}
+				By("Verified that DNS rules are working as expected")
+
+				By("Deleting the service to verify that DNS rules still work")
+				err := f.FederationClientset_1_5.Services(nsName).Delete(FederatedServiceName, &v1.DeleteOptions{})
+				framework.ExpectNoError(err, "Error deleting service %q in namespace %q", service.Name, service.Namespace)
+				for i, DNSName := range svcDNSNames {
+					discoverService(f, DNSName, true, "federated-service-e2e-discovery-pod-"+strconv.Itoa(i))
+				}
+				By("Verified that deleting the service does not affect DNS records")
 			})
 
 			Context("non-local federated service", func() {