diff --git a/federation/pkg/federation-controller/service/servicecontroller.go b/federation/pkg/federation-controller/service/servicecontroller.go index 73214ccb9a8..de7ad95f6df 100644 --- a/federation/pkg/federation-controller/service/servicecontroller.go +++ b/federation/pkg/federation-controller/service/servicecontroller.go @@ -387,41 +387,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 @@ -925,31 +890,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 } diff --git a/test/e2e/federated-service.go b/test/e2e/federated-service.go index 6a279783bd0..ae465a1c372 100644 --- a/test/e2e/federated-service.go +++ b/test/e2e/federated-service.go @@ -113,6 +113,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() { @@ -192,6 +213,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() {