Unittests

controllers/gce/instances/fakes.go

@@ -27,30 +27,50 @@ import (
 // NewFakeInstanceGroups creates a new FakeInstanceGroups.
 func NewFakeInstanceGroups(nodes sets.String) *FakeInstanceGroups {
 	return &FakeInstanceGroups{
-		instances:  nodes,
-		listResult: getInstanceList(nodes),
-		namer:      utils.Namer{},
+		instances:        nodes,
+		listResult:       getInstanceList(nodes),
+		namer:            utils.Namer{},
+		zonesToInstances: map[string][]string{},
 	}
 }
 
 // InstanceGroup fakes
 
+// FakeZoneLister records zones for nodes.
+type FakeZoneLister struct {
+	Zones []string
+}
+
+// ListZones returns the list of zones.
+func (z *FakeZoneLister) ListZones() ([]string, error) {
+	return z.Zones, nil
+}
+
+// GetZoneForNode returns the only zone stored in the fake zone lister.
+func (z *FakeZoneLister) GetZoneForNode(name string) (string, error) {
+	// TODO: evolve as required, it's currently needed just to satisfy the
+	// interface in unittests that don't care about zones. See unittests in
+	// controller/util_test for actual zoneLister testing.
+	return z.Zones[0], nil
+}
+
 // FakeInstanceGroups fakes out the instance groups api.
 type FakeInstanceGroups struct {
-	instances      sets.String
-	instanceGroups []*compute.InstanceGroup
-	Ports          []int64
-	getResult      *compute.InstanceGroup
-	listResult     *compute.InstanceGroupsListInstances
-	calls          []int
-	namer          utils.Namer
+	instances        sets.String
+	instanceGroups   []*compute.InstanceGroup
+	Ports            []int64
+	getResult        *compute.InstanceGroup
+	listResult       *compute.InstanceGroupsListInstances
+	calls            []int
+	namer            utils.Namer
+	zonesToInstances map[string][]string
 }
 
 // GetInstanceGroup fakes getting an instance group from the cloud.
 func (f *FakeInstanceGroups) GetInstanceGroup(name, zone string) (*compute.InstanceGroup, error) {
 	f.calls = append(f.calls, utils.Get)
 	for _, ig := range f.instanceGroups {
-		if ig.Name == name {
+		if ig.Name == name && ig.Zone == zone {
 			return ig, nil
 		}
 	}
@@ -60,7 +80,7 @@ func (f *FakeInstanceGroups) GetInstanceGroup(name, zone string) (*compute.Insta
 
 // CreateInstanceGroup fakes instance group creation.
 func (f *FakeInstanceGroups) CreateInstanceGroup(name, zone string) (*compute.InstanceGroup, error) {
-	newGroup := &compute.InstanceGroup{Name: name, SelfLink: name}
+	newGroup := &compute.InstanceGroup{Name: name, SelfLink: name, Zone: zone}
 	f.instanceGroups = append(f.instanceGroups, newGroup)
 	return newGroup, nil
 }
@@ -92,13 +112,35 @@ func (f *FakeInstanceGroups) ListInstancesInInstanceGroup(name, zone string, sta
 func (f *FakeInstanceGroups) AddInstancesToInstanceGroup(name, zone string, instanceNames []string) error {
 	f.calls = append(f.calls, utils.AddInstances)
 	f.instances.Insert(instanceNames...)
+	if _, ok := f.zonesToInstances[zone]; !ok {
+		f.zonesToInstances[zone] = []string{}
+	}
+	f.zonesToInstances[zone] = append(f.zonesToInstances[zone], instanceNames...)
 	return nil
 }
 
+// GetInstancesByZone returns the zone to instances map.
+func (f *FakeInstanceGroups) GetInstancesByZone() map[string][]string {
+	return f.zonesToInstances
+}
+
 // RemoveInstancesFromInstanceGroup fakes removing instances from an instance group.
 func (f *FakeInstanceGroups) RemoveInstancesFromInstanceGroup(name, zone string, instanceNames []string) error {
 	f.calls = append(f.calls, utils.RemoveInstances)
 	f.instances.Delete(instanceNames...)
+	l, ok := f.zonesToInstances[zone]
+	if !ok {
+		return nil
+	}
+	newIns := []string{}
+	delIns := sets.NewString(instanceNames...)
+	for _, oldIns := range l {
+		if delIns.Has(oldIns) {
+			continue
+		}
+		newIns = append(newIns, oldIns)
+	}
+	f.zonesToInstances[zone] = newIns
 	return nil
 }
 

controllers/gce/instances/instances.go

@@ -50,6 +50,9 @@ func NewNodePool(cloud InstanceGroups, defaultZone string) NodePool {
 	return &Instances{cloud, storage.NewInMemoryPool(), nil}
 }
 
+// Init initializes the instance pool. The given zoneLister is used to list
+// all zones that require an instance group, and to lookup which zone a
+// given Kubernetes node is in so we can add it to the right instance group.
 func (i *Instances) Init(zl zoneLister) {
 	i.zoneLister = zl
 }
@@ -191,7 +194,7 @@ func (i *Instances) Remove(groupName string, names []string) error {
 
 // Sync syncs kubernetes instances with the instances in the instance group.
 func (i *Instances) Sync(nodes []string) (err error) {
-	glog.V(1).Infof("Syncing nodes %v", nodes)
+	glog.V(4).Infof("Syncing nodes %v", nodes)
 
 	defer func() {
 		// The node pool is only responsible for syncing nodes to instance
@@ -207,9 +210,9 @@ func (i *Instances) Sync(nodes []string) (err error) {
 	}()
 
 	pool := i.snapshotter.Snapshot()
-	for name := range pool {
+	for igName := range pool {
 		gceNodes := sets.NewString()
-		gceNodes, err = i.list(name)
+		gceNodes, err = i.list(igName)
 		if err != nil {
 			return err
 		}
@@ -223,14 +226,14 @@ func (i *Instances) Sync(nodes []string) (err error) {
 		addNodes := kubeNodes.Difference(gceNodes).List()
 		if len(removeNodes) != 0 {
 			if err = i.Remove(
-				name, gceNodes.Difference(kubeNodes).List()); err != nil {
+				igName, gceNodes.Difference(kubeNodes).List()); err != nil {
 				return err
 			}
 		}
 
 		if len(addNodes) != 0 {
 			if err = i.Add(
-				name, kubeNodes.Difference(gceNodes).List()); err != nil {
+				igName, kubeNodes.Difference(gceNodes).List()); err != nil {
 				return err
 			}
 		}

controllers/gce/instances/instances_test.go

@@ -24,10 +24,16 @@ import (
 
 const defaultZone = "default-zone"
 
+func newNodePool(f *FakeInstanceGroups, zone string) NodePool {
+	pool := NewNodePool(f, zone)
+	pool.Init(&FakeZoneLister{[]string{zone}})
+	return pool
+}
+
 func TestNodePoolSync(t *testing.T) {
 	f := NewFakeInstanceGroups(sets.NewString(
 		[]string{"n1", "n2"}...))
-	pool := NewNodePool(f, defaultZone)
+	pool := newNodePool(f, defaultZone)
 	pool.AddInstanceGroup("test", 80)
 
 	// KubeNodes: n1
@@ -46,7 +52,7 @@ func TestNodePoolSync(t *testing.T) {
 	// Try to add n2 to the instance group.
 
 	f = NewFakeInstanceGroups(sets.NewString([]string{"n1"}...))
-	pool = NewNodePool(f, defaultZone)
+	pool = newNodePool(f, defaultZone)
 	pool.AddInstanceGroup("test", 80)
 
 	f.calls = []int{}
@@ -62,7 +68,7 @@ func TestNodePoolSync(t *testing.T) {
 	// Do nothing.
 
 	f = NewFakeInstanceGroups(sets.NewString([]string{"n1", "n2"}...))
-	pool = NewNodePool(f, defaultZone)
+	pool = newNodePool(f, defaultZone)
 	pool.AddInstanceGroup("test", 80)
 
 	f.calls = []int{}