Merge pull request #117 from bowei/missing-commit

Code review comments

pkg/context/context.go

@@ -67,7 +67,6 @@ func NewControllerContext(kubeClient kubernetes.Interface, namespace string, res
 
 // HasSynced returns true if all relevant informers has been synced.
 func (ctx *ControllerContext) HasSynced() bool {
-
 	funcs := []func() bool{
 		ctx.IngressInformer.HasSynced,
 		ctx.ServiceInformer.HasSynced,

pkg/controller/cluster_manager.go

@@ -105,7 +105,7 @@ func (c *ClusterManager) shutdown() error {
 // If in performing the checkpoint the cluster manager runs out of quota, a
 // googleapi 403 is returned.
 func (c *ClusterManager) Checkpoint(lbs []*loadbalancers.L7RuntimeInfo, nodeNames []string, backendServicePorts []backends.ServicePort, namedPorts []backends.ServicePort, firewallPorts []int64) ([]*compute.InstanceGroup, error) {
-	glog.V(4).Infof("Checkpoint %q, len(lbs)=%v, len(nodeNames)=%v, lne(backendServicePorts)=%v, len(namedPorts)=%v, len(firewallPorts)=%v", len(lbs), len(nodeNames), len(backendServicePorts), len(namedPorts), len(firewallPorts))
+	glog.V(4).Infof("Checkpoint(%v lbs, %v nodeNames, %v backendServicePorts, %v namedPorts, %v firewallPorts)", len(lbs), len(nodeNames), len(backendServicePorts), len(namedPorts), len(firewallPorts))
 
 	if len(namedPorts) != 0 {
 		// Add the default backend node port to the list of named ports for instance groups.
@@ -180,8 +180,7 @@ func (c *ClusterManager) GC(lbNames []string, nodePorts []backends.ServicePort)
 		if err := c.instancePool.DeleteInstanceGroup(igName); err != err {
 			return err
 		}
-	}
-	if len(lbNames) == 0 {
+		glog.V(2).Infof("Shutting down firewall as there are no loadbalancers")
 		c.firewallPool.Shutdown()
 	}
 

pkg/controller/node.go

@@ -26,15 +26,18 @@ import (
 )
 
 const (
-	nodeResyncPeriod = 1 * time.Second
+	nodeUpdatePeriod = 1 * time.Second
 )
 
 // NodeController synchronizes the state of the nodes to the unmanaged instance
 // groups.
 type NodeController struct {
+	// lister is a cache of the k8s Node resources.
 	lister cache.Indexer
-	queue  utils.TaskQueue
-	cm     *ClusterManager
+	// queue is the TaskQueue used to manage the node worker updates.
+	queue utils.TaskQueue
+	// cm is the shared ClusterManager interface.
+	cm *ClusterManager
 }
 
 // NewNodeController returns a new node update controller.
@@ -48,14 +51,17 @@ func NewNodeController(ctx *context.ControllerContext, cm *ClusterManager) *Node
 	ctx.NodeInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc:    c.queue.Enqueue,
 		DeleteFunc: c.queue.Enqueue,
+		// TODO: watch updates for Nodes going from NotReady to Ready.
+		// Otherwise the controller will wait until the complete
+		// refresh to process the node.
 	})
 
 	return c
 }
 
 // Run a go routine to process updates for the controller.
 func (c *NodeController) Run(stopCh chan struct{}) {
-	go c.queue.Run(nodeResyncPeriod, stopCh)
+	go c.queue.Run(nodeUpdatePeriod, stopCh)
 }
 
 // Run a go routine to process updates for the controller.

pkg/controller/translator/translator_test.go

@@ -291,7 +291,7 @@ func TestGatherFirewallPorts(t *testing.T) {
 		}
 		for _, p := range res {
 			if _, ok := int64ToMap(tc.want)[p]; !ok {
-				t.Errorf("firewall port %v is missing, (got %v, want %v)", p, res, tc.want)
+				t.Errorf("firewall port %v is missing, (got %v, want %v); defaultBackend=%v", p, res, tc.want, tc.defaultBackend)
 			}
 		}
 	}

pkg/controller/utils.go

@@ -211,7 +211,7 @@ func uniq(nodePorts []backends.ServicePort) []backends.ServicePort {
 
 // getReadyNodeNames returns names of schedulable, ready nodes from the node lister.
 func getReadyNodeNames(lister listers.NodeLister) ([]string, error) {
-	nodeNames := []string{}
+	var nodeNames []string
 	nodes, err := lister.ListWithPredicate(utils.NodeIsReady)
 	if err != nil {
 		return nodeNames, err