minimize sync()'s done by the controllers (#399)

* minimize sync() done by the controllers

* deprecate --config-sync-period

* review comments

Author: murali-reddy

cmd/kube-router/kube-router_test.go

@@ -45,5 +45,6 @@ func TestMainHelp(t *testing.T) {
 
 	if !bytes.Contains(docBuf.Bytes(), exp) {
 		t.Errorf("docs/README.md 'command line options' section does not match `kube-router --help`.\nExpected:\n%s", exp)
+		t.Errorf("\nGot:\n%s", docBuf.Bytes())
 	}
 }

docs/README.md

@@ -107,7 +107,6 @@ Usage of kube-router:
       --cleanup-config                   Cleanup iptables rules, ipvs, ipset configuration and exit.
       --cluster-asn uint                 ASN number under which cluster nodes will run iBGP.
       --cluster-cidr string              CIDR range of pods in the cluster. It is used to identify traffic originating from and destinated to pods.
-      --config-sync-period duration      The delay between apiserver configuration synchronizations (e.g. '5s', '1m').  Must be greater than 0. (default 1m0s)
       --enable-ibgp                      Enables peering with nodes with the same ASN, if disabled will only peer with external BGP peers (default true)
       --enable-overlay                   When enable-overlay set to true, IP-in-IP tunneling is used for pod-to-pod networking across nodes in different subnets. When set to false no tunneling is used and routing infrastrcture is expected to route traffic for pod-to-pod networking across nodes in different subnets (default true)
       --enable-pod-egress                SNAT traffic from Pods to destinations outside the cluster. (default true)
@@ -116,8 +115,8 @@ Usage of kube-router:
       --health-port uint16               Health check port, 0 = Disabled (default 20244)
   -h, --help                             Print usage information.
       --hostname-override string         Overrides the NodeName of the node. Set this if kube-router is unable to determine your NodeName automatically.
-      --iptables-sync-period duration    The delay between iptables rule synchronizations (e.g. '5s', '1m'). Must be greater than 0. (default 1m0s)
-      --ipvs-sync-period duration        The delay between ipvs config synchronizations (e.g. '5s', '1m', '2h22m'). Must be greater than 0. (default 1m0s)
+      --iptables-sync-period duration    The delay between iptables rule synchronizations (e.g. '5s', '1m'). Must be greater than 0. (default 5m0s)
+      --ipvs-sync-period duration        The delay between ipvs config synchronizations (e.g. '5s', '1m', '2h22m'). Must be greater than 0. (default 5m0s)
       --kubeconfig string                Path to kubeconfig file with authorization information (the master location is set by the master flag).
       --masquerade-all                   SNAT all traffic to cluster IP/node port.
       --master string                    The address of the Kubernetes API server (overrides any value in kubeconfig).
@@ -129,7 +128,7 @@ Usage of kube-router:
       --peer-router-ips ipSlice          The ip address of the external router to which all nodes will peer and advertise the cluster ip and pod cidr's. (default [])
       --peer-router-multihop-ttl uint8   Enable eBGP multihop supports -- sets multihop-ttl. (Relevant only if ttl >= 2)
       --peer-router-passwords strings    Password for authenticating against the BGP peer defined with "--peer-router-ips".
-      --routes-sync-period duration      The delay between route updates and advertisements (e.g. '5s', '1m', '2h22m'). Must be greater than 0. (default 1m0s)
+      --routes-sync-period duration      The delay between route updates and advertisements (e.g. '5s', '1m', '2h22m'). Must be greater than 0. (default 5m0s)
       --run-firewall                     Enables Network Policy -- sets up iptables to provide ingress firewall for pods. (default true)
       --run-router                       Enables Pod Networking -- Advertises and learns the routes to Pods via iBGP. (default true)
       --run-service-proxy                Enables Service Proxy -- sets up IPVS for Kubernetes Services. (default true)

pkg/cmd/kube-router.go

@@ -106,7 +106,7 @@ func (kr *KubeRouter) Run() error {
 		kr.Config.MetricsEnabled = false
 	}
 
-	informerFactory := informers.NewSharedInformerFactory(kr.Client, kr.Config.ConfigSyncPeriod)
+	informerFactory := informers.NewSharedInformerFactory(kr.Client, 0)
 
 	svcInformer := informerFactory.Core().V1().Services().Informer()
 	epInformer := informerFactory.Core().V1().Endpoints().Informer()
@@ -115,7 +115,7 @@ func (kr *KubeRouter) Run() error {
 	nsInformer := informerFactory.Core().V1().Namespaces().Informer()
 	npInformer := informerFactory.Networking().V1().NetworkPolicies().Informer()
 
-	go informerFactory.Start(stopCh)
+	informerFactory.Start(stopCh)
 	informerFactory.WaitForCacheSync(stopCh)
 
 	if kr.Config.RunFirewall {

pkg/controllers/network_policy_controller.go

@@ -150,30 +150,31 @@ func (npc *NetworkPolicyController) Run(healthChan chan<- *ControllerHeartbeat,
 // OnPodUpdate handles updates to pods from the Kubernetes api server
 func (npc *NetworkPolicyController) OnPodUpdate(obj interface{}) {
 	pod := obj.(*api.Pod)
-	glog.V(2).Infof("Received pod update namespace:%s pod name:%s", pod.Namespace, pod.Name)
+	glog.V(2).Infof("Received update to pod: %s/%s", pod.Namespace, pod.Name)
 
 	err := npc.Sync()
 	if err != nil {
-		glog.Errorf("Error syncing on pod update: %s", err)
+		glog.Errorf("Error syncing network policy for the update to pod: %s/%s Error: %s", pod.Namespace, pod.Name, err)
 	}
 }
 
 // OnNetworkPolicyUpdate handles updates to network policy from the kubernetes api server
 func (npc *NetworkPolicyController) OnNetworkPolicyUpdate(obj interface{}) {
+	netpol := obj.(*networking.NetworkPolicy)
+	glog.V(2).Infof("Received update for network policy: %s/%s", netpol.Namespace, netpol.Name)
 	err := npc.Sync()
 	if err != nil {
-		glog.Errorf("Error syncing on network policy update: %s", err)
+		glog.Errorf("Error syncing network policy for the update to network policy: %s/%s Error: %s", netpol.Namespace, netpol.Name, err)
 	}
 }
 
 // OnNamespaceUpdate handles updates to namespace from kubernetes api server
 func (npc *NetworkPolicyController) OnNamespaceUpdate(obj interface{}) {
+	namespace := obj.(*api.Namespace)
 	// namespace (and annotations on it) has no significance in GA ver of network policy
 	if npc.v1NetworkPolicy {
 		return
 	}
-
-	namespace := obj.(*api.Namespace)
 	glog.V(2).Infof("Received update for namespace: %s", namespace.Name)
 
 	err := npc.Sync()

pkg/controllers/network_routes_controller.go

@@ -261,10 +261,11 @@ func (nrc *NetworkRoutingController) Run(healthChan chan<- *ControllerHeartbeat,
 			glog.Errorf("failed to get routes to advertise/withdraw %s", err)
 		}
 
+		glog.V(1).Infof("Performing periodic sync of service VIP routes")
 		nrc.advertiseVIPs(toAdvertise)
 		nrc.withdrawVIPs(toWithdraw)
 
-		glog.V(1).Info("Performing periodic sync of the routes")
+		glog.V(1).Info("Performing periodic sync of pod CIDR routes")
 		err = nrc.advertisePodRoute()
 		if err != nil {
 			glog.Errorf("Error advertising route: %s", err.Error())
@@ -1438,6 +1439,7 @@ func (nrc *NetworkRoutingController) OnServiceUpdate(obj interface{}) {
 		return
 	}
 
+	glog.V(1).Infof("Received update to service: %s/%s from watch API", svc.Namespace, svc.Name)
 	toAdvertise, toWithdraw, err := nrc.getVIPsForService(svc, true)
 	if err != nil {
 		glog.Errorf("error getting routes for service: %s, err: %s", svc.Name, err)
@@ -1464,9 +1466,10 @@ func (nrc *NetworkRoutingController) OnServiceDelete(obj interface{}) {
 		return
 	}
 
+	glog.V(1).Infof("Received event to delete service: %s/%s from watch API", svc.Namespace, svc.Name)
 	toAdvertise, toWithdraw, err := nrc.getVIPsForService(svc, true)
 	if err != nil {
-		glog.Errorf("failed to get clean up routes for deleted service %s", svc.Name)
+		glog.Errorf("failed to get clean up routes for deleted service: %s/%s", svc.Namespace, svc.Name)
 		return
 	}
 

pkg/controllers/network_services_controller.go

@@ -353,8 +353,6 @@ func (nsc *NetworkServicesController) OnEndpointsUpdate(obj interface{}) {
 	nsc.mu.Lock()
 	defer nsc.mu.Unlock()
 
-	glog.V(1).Info("Received endpoints update from watch API")
-
 	ep, ok := obj.(*api.Endpoints)
 	if !ok {
 		glog.Error("could not convert endpoints update object to *v1.Endpoints")
@@ -365,14 +363,19 @@ func (nsc *NetworkServicesController) OnEndpointsUpdate(obj interface{}) {
 		return
 	}
 
-	// build new endpoints map to reflect the change
+	glog.V(1).Infof("Received update to endpoint: %s/%s from watch API", ep.Namespace, ep.Name)
+
+	// build new service and endpoints map to reflect the change
+	newServiceMap := nsc.buildServicesInfo()
 	newEndpointsMap := nsc.buildEndpointsInfo()
 
 	if len(newEndpointsMap) != len(nsc.endpointsMap) || !reflect.DeepEqual(newEndpointsMap, nsc.endpointsMap) {
 		nsc.endpointsMap = newEndpointsMap
+		nsc.serviceMap = newServiceMap
+		glog.V(1).Infof("Syncing IPVS services sync for update to endpoint: %s/%s", ep.Namespace, ep.Name)
 		nsc.syncIpvsServices(nsc.serviceMap, nsc.endpointsMap)
 	} else {
-		glog.V(1).Info("Skipping ipvs server sync on endpoints because nothing changed")
+		glog.V(1).Infof("Skipping IPVS services sync on endpoint: %s/%s update as nothing changed", ep.Namespace, ep.Name)
 	}
 }
 
@@ -381,16 +384,25 @@ func (nsc *NetworkServicesController) OnServiceUpdate(obj interface{}) {
 	nsc.mu.Lock()
 	defer nsc.mu.Unlock()
 
-	glog.V(1).Info("Received service update from watch API")
+	svc, ok := obj.(*api.Service)
+	if !ok {
+		glog.Error("could not convert service update object to *v1.Service")
+		return
+	}
+
+	glog.V(1).Infof("Received update to service: %s/%s from watch API", svc.Namespace, svc.Name)
 
-	// build new services map to reflect the change
+	// build new service and endpoints map to reflect the change
 	newServiceMap := nsc.buildServicesInfo()
+	newEndpointsMap := nsc.buildEndpointsInfo()
 
 	if len(newServiceMap) != len(nsc.serviceMap) || !reflect.DeepEqual(newServiceMap, nsc.serviceMap) {
+		nsc.endpointsMap = newEndpointsMap
 		nsc.serviceMap = newServiceMap
+		glog.V(1).Infof("Syncing IPVS services sync on update to service: %s/%s", svc.Namespace, svc.Name)
 		nsc.syncIpvsServices(nsc.serviceMap, nsc.endpointsMap)
 	} else {
-		glog.V(1).Info("Skipping ipvs server sync on service update because nothing changed")
+		glog.V(1).Infof("Skipping syncing IPVS services for update to service: %s/%s as nothing changed", svc.Namespace, svc.Name)
 	}
 }
 
@@ -532,7 +544,6 @@ func (nsc *NetworkServicesController) syncIpvsServices(serviceInfoMap serviceInf
 		if !svc.skipLbIps {
 			extIPSet = extIPSet.Union(sets.NewString(svc.loadBalancerIPs...))
 		}
-		glog.V(2).Infof("Service \"%s\" using extIPSet: %v", svc.name, extIPSet.List())
 
 		for _, externalIP := range extIPSet.List() {
 			var externalIpServiceId string

pkg/options/options.go

@@ -15,7 +15,6 @@ type KubeRouterConfig struct {
 	CleanupConfig           bool
 	ClusterAsn              uint
 	ClusterCIDR             string
-	ConfigSyncPeriod        time.Duration
 	EnableiBGP              bool
 	EnableOverlay           bool
 	EnablePodEgress         bool
@@ -48,10 +47,10 @@ type KubeRouterConfig struct {
 }
 
 func NewKubeRouterConfig() *KubeRouterConfig {
-	return &KubeRouterConfig{ConfigSyncPeriod: 1 * time.Minute,
-		IpvsSyncPeriod:     1 * time.Minute,
-		IPTablesSyncPeriod: 1 * time.Minute,
-		RoutesSyncPeriod:   1 * time.Minute,
+	return &KubeRouterConfig{
+		IpvsSyncPeriod:     5 * time.Minute,
+		IPTablesSyncPeriod: 5 * time.Minute,
+		RoutesSyncPeriod:   5 * time.Minute,
 		EnableOverlay:      true,
 	}
 }
@@ -79,8 +78,6 @@ func (s *KubeRouterConfig) AddFlags(fs *pflag.FlagSet) {
 		"CIDR range of pods in the cluster. It is used to identify traffic originating from and destinated to pods.")
 	fs.BoolVar(&s.EnablePodEgress, "enable-pod-egress", true,
 		"SNAT traffic from Pods to destinations outside the cluster.")
-	fs.DurationVar(&s.ConfigSyncPeriod, "config-sync-period", s.ConfigSyncPeriod,
-		"The delay between apiserver configuration synchronizations (e.g. '5s', '1m').  Must be greater than 0.")
 	fs.DurationVar(&s.IPTablesSyncPeriod, "iptables-sync-period", s.IPTablesSyncPeriod,
 		"The delay between iptables rule synchronizations (e.g. '5s', '1m'). Must be greater than 0.")
 	fs.DurationVar(&s.IpvsSyncPeriod, "ipvs-sync-period", s.IpvsSyncPeriod,