From d4fa23feb820cc6f55fd9eecbe710c7bdbe1b31a Mon Sep 17 00:00:00 2001 From: xiangpengzhao Date: Wed, 6 Jul 2016 00:02:49 -0400 Subject: [PATCH 1/3] Check return value when calling ensureDnsRecords --- .../federation-controller/service/servicecontroller.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/federation/pkg/federation-controller/service/servicecontroller.go b/federation/pkg/federation-controller/service/servicecontroller.go index 9cdc911bc0751..17911b3d3d727 100644 --- a/federation/pkg/federation-controller/service/servicecontroller.go +++ b/federation/pkg/federation-controller/service/servicecontroller.go @@ -728,8 +728,12 @@ func (s *ServiceController) lockedUpdateDNSRecords(service *cachedService, clust for key := range s.clusterCache.clientMap { for _, clusterName := range clusterNames { if key == clusterName { - s.ensureDnsRecords(clusterName, service) - ensuredCount += 1 + err := s.ensureDnsRecords(clusterName, service) + if err != nil { + glog.Errorf("Failed to ensure DNS records for service %v in cluster %s due to %v", service, clusterName, err) + } else { + ensuredCount += 1 + } } } } From dfd12276dff17e35feb1e5d1de4d877c97e9d0e0 Mon Sep 17 00:00:00 2001 From: xiangpengzhao Date: Thu, 21 Jul 2016 22:35:12 -0400 Subject: [PATCH 2/3] Keep track of missed and unensured counts --- .../service/servicecontroller.go | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/federation/pkg/federation-controller/service/servicecontroller.go b/federation/pkg/federation-controller/service/servicecontroller.go index 17911b3d3d727..61c0418d9b54d 100644 --- a/federation/pkg/federation-controller/service/servicecontroller.go +++ b/federation/pkg/federation-controller/service/servicecontroller.go @@ -724,22 +724,24 @@ func (s *ServiceController) lockedUpdateDNSRecords(service *cachedService, clust if !wantsDNSRecords(service.appliedState) { return nil } - ensuredCount := 0 + + var err error + missedCount := 0 + unensuredCount := 0 for key := range s.clusterCache.clientMap { for _, clusterName := range clusterNames { if key == clusterName { - err := s.ensureDnsRecords(clusterName, service) - if err != nil { - glog.Errorf("Failed to ensure DNS records for service %v in cluster %s due to %v", service, clusterName, err) - } else { - ensuredCount += 1 + if err = s.ensureDnsRecords(clusterName, service); err != nil { + unensuredCount += 1 } + } else { + missedCount += 1 } } } - if ensuredCount < len(clusterNames) { - return fmt.Errorf("Failed to update DNS records for %d of %d clusters for service %v due to missing clients for those clusters", - len(clusterNames)-ensuredCount, len(clusterNames), service) + if missedCount > 0 || unensuredCount > 0 { + return fmt.Errorf("Failed to update DNS records for %d clusters for service %v due to missing clients[missed count: %d] or ensuring DNS records error[unensured count: %d] %v", + len(clusterNames), service, missedCount, unensuredCount, err) } return nil } From 4ccd90d7418f764cc9a42abb0dd305a0732edb38 Mon Sep 17 00:00:00 2001 From: xiangpengzhao Date: Fri, 29 Jul 2016 22:01:38 -0400 Subject: [PATCH 3/3] Log error in each loop it appears and fix wrong count of missed cluster. --- .../service/servicecontroller.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/federation/pkg/federation-controller/service/servicecontroller.go b/federation/pkg/federation-controller/service/servicecontroller.go index 61c0418d9b54d..a7d929e613b15 100644 --- a/federation/pkg/federation-controller/service/servicecontroller.go +++ b/federation/pkg/federation-controller/service/servicecontroller.go @@ -725,23 +725,25 @@ func (s *ServiceController) lockedUpdateDNSRecords(service *cachedService, clust return nil } - var err error - missedCount := 0 + ensuredCount := 0 unensuredCount := 0 for key := range s.clusterCache.clientMap { for _, clusterName := range clusterNames { if key == clusterName { - if err = s.ensureDnsRecords(clusterName, service); err != nil { + err := s.ensureDnsRecords(clusterName, service) + if err != nil { unensuredCount += 1 + glog.V(4).Infof("Failed to update DNS records for service %v from cluster %s: %v", service, clusterName, err) + } else { + ensuredCount += 1 } - } else { - missedCount += 1 } } } + missedCount := len(clusterNames) - ensuredCount - unensuredCount if missedCount > 0 || unensuredCount > 0 { - return fmt.Errorf("Failed to update DNS records for %d clusters for service %v due to missing clients[missed count: %d] or ensuring DNS records error[unensured count: %d] %v", - len(clusterNames), service, missedCount, unensuredCount, err) + return fmt.Errorf("Failed to update DNS records for %d clusters for service %v due to missing clients [missed count: %d] and/or failing to ensure DNS records [unensured count: %d]", + len(clusterNames), service, missedCount, unensuredCount) } return nil }