Merge pull request #37370 from gmarek/test-refactor

pkg/controller/node/BUILD

@@ -20,18 +20,15 @@ go_library(
         "metrics.go",
         "nodecontroller.go",
         "rate_limited_queue.go",
-        "test_utils.go",
     ],
     tags = ["automanaged"],
     deps = [
         "//pkg/api:go_default_library",
         "//pkg/api/errors:go_default_library",
-        "//pkg/api/resource:go_default_library",
         "//pkg/api/v1:go_default_library",
         "//pkg/apis/meta/v1:go_default_library",
         "//pkg/client/cache:go_default_library",
         "//pkg/client/clientset_generated/release_1_5:go_default_library",
-        "//pkg/client/clientset_generated/release_1_5/fake:go_default_library",
         "//pkg/client/clientset_generated/release_1_5/typed/core/v1:go_default_library",
         "//pkg/client/record:go_default_library",
         "//pkg/cloudprovider:go_default_library",
@@ -39,9 +36,7 @@ go_library(
         "//pkg/fields:go_default_library",
         "//pkg/kubelet/util/format:go_default_library",
         "//pkg/labels:go_default_library",
-        "//pkg/runtime:go_default_library",
         "//pkg/types:go_default_library",
-        "//pkg/util/clock:go_default_library",
         "//pkg/util/errors:go_default_library",
         "//pkg/util/flowcontrol:go_default_library",
         "//pkg/util/metrics:go_default_library",
@@ -51,7 +46,6 @@ go_library(
         "//pkg/util/system:go_default_library",
         "//pkg/util/wait:go_default_library",
         "//pkg/version:go_default_library",
-        "//pkg/watch:go_default_library",
         "//vendor:github.com/golang/glog",
         "//vendor:github.com/prometheus/client_golang/prometheus",
     ],
@@ -81,6 +75,7 @@ go_test(
         "//pkg/cloudprovider/providers/fake:go_default_library",
         "//pkg/controller:go_default_library",
         "//pkg/controller/informers:go_default_library",
+        "//pkg/controller/node/testutil:go_default_library",
         "//pkg/types:go_default_library",
         "//pkg/util/diff:go_default_library",
         "//pkg/util/flowcontrol:go_default_library",

pkg/controller/node/cidr_allocator_test.go

@@ -23,6 +23,7 @@ import (
 
 	"k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/release_1_5/fake"
+	"k8s.io/kubernetes/pkg/controller/node/testutil"
 	"k8s.io/kubernetes/pkg/util/wait"
 )
 
@@ -30,9 +31,9 @@ const (
 	nodePollInterval = 100 * time.Millisecond
 )
 
-func waitForUpdatedNodeWithTimeout(nodeHandler *FakeNodeHandler, number int, timeout time.Duration) error {
+func waitForUpdatedNodeWithTimeout(nodeHandler *testutil.FakeNodeHandler, number int, timeout time.Duration) error {
 	return wait.Poll(nodePollInterval, timeout, func() (bool, error) {
-		if len(nodeHandler.getUpdatedNodesCopy()) >= number {
+		if len(nodeHandler.GetUpdatedNodesCopy()) >= number {
 			return true, nil
 		}
 		return false, nil
@@ -42,7 +43,7 @@ func waitForUpdatedNodeWithTimeout(nodeHandler *FakeNodeHandler, number int, tim
 func TestAllocateOrOccupyCIDRSuccess(t *testing.T) {
 	testCases := []struct {
 		description           string
-		fakeNodeHandler       *FakeNodeHandler
+		fakeNodeHandler       *testutil.FakeNodeHandler
 		clusterCIDR           *net.IPNet
 		serviceCIDR           *net.IPNet
 		subNetMaskSize        int
@@ -51,7 +52,7 @@ func TestAllocateOrOccupyCIDRSuccess(t *testing.T) {
 	}{
 		{
 			description: "When there's no ServiceCIDR return first CIDR in range",
-			fakeNodeHandler: &FakeNodeHandler{
+			fakeNodeHandler: &testutil.FakeNodeHandler{
 				Existing: []*v1.Node{
 					{
 						ObjectMeta: v1.ObjectMeta{
@@ -71,7 +72,7 @@ func TestAllocateOrOccupyCIDRSuccess(t *testing.T) {
 		},
 		{
 			description: "Correctly filter out ServiceCIDR",
-			fakeNodeHandler: &FakeNodeHandler{
+			fakeNodeHandler: &testutil.FakeNodeHandler{
 				Existing: []*v1.Node{
 					{
 						ObjectMeta: v1.ObjectMeta{
@@ -95,7 +96,7 @@ func TestAllocateOrOccupyCIDRSuccess(t *testing.T) {
 		},
 		{
 			description: "Correctly ignore already allocated CIDRs",
-			fakeNodeHandler: &FakeNodeHandler{
+			fakeNodeHandler: &testutil.FakeNodeHandler{
 				Existing: []*v1.Node{
 					{
 						ObjectMeta: v1.ObjectMeta{
@@ -121,7 +122,7 @@ func TestAllocateOrOccupyCIDRSuccess(t *testing.T) {
 
 	testFunc := func(tc struct {
 		description           string
-		fakeNodeHandler       *FakeNodeHandler
+		fakeNodeHandler       *testutil.FakeNodeHandler
 		clusterCIDR           *net.IPNet
 		serviceCIDR           *net.IPNet
 		subNetMaskSize        int
@@ -152,7 +153,7 @@ func TestAllocateOrOccupyCIDRSuccess(t *testing.T) {
 		}
 		found := false
 		seenCIDRs := []string{}
-		for _, updatedNode := range tc.fakeNodeHandler.getUpdatedNodesCopy() {
+		for _, updatedNode := range tc.fakeNodeHandler.GetUpdatedNodesCopy() {
 			seenCIDRs = append(seenCIDRs, updatedNode.Spec.PodCIDR)
 			if updatedNode.Spec.PodCIDR == tc.expectedAllocatedCIDR {
 				found = true
@@ -173,7 +174,7 @@ func TestAllocateOrOccupyCIDRSuccess(t *testing.T) {
 func TestAllocateOrOccupyCIDRFailure(t *testing.T) {
 	testCases := []struct {
 		description     string
-		fakeNodeHandler *FakeNodeHandler
+		fakeNodeHandler *testutil.FakeNodeHandler
 		clusterCIDR     *net.IPNet
 		serviceCIDR     *net.IPNet
 		subNetMaskSize  int
@@ -181,7 +182,7 @@ func TestAllocateOrOccupyCIDRFailure(t *testing.T) {
 	}{
 		{
 			description: "When there's no ServiceCIDR return first CIDR in range",
-			fakeNodeHandler: &FakeNodeHandler{
+			fakeNodeHandler: &testutil.FakeNodeHandler{
 				Existing: []*v1.Node{
 					{
 						ObjectMeta: v1.ObjectMeta{
@@ -203,7 +204,7 @@ func TestAllocateOrOccupyCIDRFailure(t *testing.T) {
 
 	testFunc := func(tc struct {
 		description     string
-		fakeNodeHandler *FakeNodeHandler
+		fakeNodeHandler *testutil.FakeNodeHandler
 		clusterCIDR     *net.IPNet
 		serviceCIDR     *net.IPNet
 		subNetMaskSize  int
@@ -231,11 +232,11 @@ func TestAllocateOrOccupyCIDRFailure(t *testing.T) {
 		}
 		// We don't expect any updates, so just sleep for some time
 		time.Sleep(time.Second)
-		if len(tc.fakeNodeHandler.getUpdatedNodesCopy()) != 0 {
-			t.Fatalf("%v: unexpected update of nodes: %v", tc.description, tc.fakeNodeHandler.getUpdatedNodesCopy())
+		if len(tc.fakeNodeHandler.GetUpdatedNodesCopy()) != 0 {
+			t.Fatalf("%v: unexpected update of nodes: %v", tc.description, tc.fakeNodeHandler.GetUpdatedNodesCopy())
 		}
 		seenCIDRs := []string{}
-		for _, updatedNode := range tc.fakeNodeHandler.getUpdatedNodesCopy() {
+		for _, updatedNode := range tc.fakeNodeHandler.GetUpdatedNodesCopy() {
 			if updatedNode.Spec.PodCIDR != "" {
 				seenCIDRs = append(seenCIDRs, updatedNode.Spec.PodCIDR)
 			}
@@ -253,7 +254,7 @@ func TestAllocateOrOccupyCIDRFailure(t *testing.T) {
 func TestReleaseCIDRSuccess(t *testing.T) {
 	testCases := []struct {
 		description                      string
-		fakeNodeHandler                  *FakeNodeHandler
+		fakeNodeHandler                  *testutil.FakeNodeHandler
 		clusterCIDR                      *net.IPNet
 		serviceCIDR                      *net.IPNet
 		subNetMaskSize                   int
@@ -264,7 +265,7 @@ func TestReleaseCIDRSuccess(t *testing.T) {
 	}{
 		{
 			description: "Correctly release preallocated CIDR",
-			fakeNodeHandler: &FakeNodeHandler{
+			fakeNodeHandler: &testutil.FakeNodeHandler{
 				Existing: []*v1.Node{
 					{
 						ObjectMeta: v1.ObjectMeta{
@@ -287,7 +288,7 @@ func TestReleaseCIDRSuccess(t *testing.T) {
 		},
 		{
 			description: "Correctly recycle CIDR",
-			fakeNodeHandler: &FakeNodeHandler{
+			fakeNodeHandler: &testutil.FakeNodeHandler{
 				Existing: []*v1.Node{
 					{
 						ObjectMeta: v1.ObjectMeta{
@@ -311,7 +312,7 @@ func TestReleaseCIDRSuccess(t *testing.T) {
 
 	testFunc := func(tc struct {
 		description                      string
-		fakeNodeHandler                  *FakeNodeHandler
+		fakeNodeHandler                  *testutil.FakeNodeHandler
 		clusterCIDR                      *net.IPNet
 		serviceCIDR                      *net.IPNet
 		subNetMaskSize                   int
@@ -351,8 +352,8 @@ func TestReleaseCIDRSuccess(t *testing.T) {
 			}
 			// We don't expect any updates here
 			time.Sleep(time.Second)
-			if len(tc.fakeNodeHandler.getUpdatedNodesCopy()) != 0 {
-				t.Fatalf("%v: unexpected update of nodes: %v", tc.description, tc.fakeNodeHandler.getUpdatedNodesCopy())
+			if len(tc.fakeNodeHandler.GetUpdatedNodesCopy()) != 0 {
+				t.Fatalf("%v: unexpected update of nodes: %v", tc.description, tc.fakeNodeHandler.GetUpdatedNodesCopy())
 			}
 		}
 
@@ -378,7 +379,7 @@ func TestReleaseCIDRSuccess(t *testing.T) {
 
 		found := false
 		seenCIDRs := []string{}
-		for _, updatedNode := range tc.fakeNodeHandler.getUpdatedNodesCopy() {
+		for _, updatedNode := range tc.fakeNodeHandler.GetUpdatedNodesCopy() {
 			seenCIDRs = append(seenCIDRs, updatedNode.Spec.PodCIDR)
 			if updatedNode.Spec.PodCIDR == tc.expectedAllocatedCIDRSecondRound {
 				found = true

pkg/controller/node/testutil/BUILD

+package(default_visibility = ["//visibility:public"])
+
+licenses(["notice"])
+
+load(
+    "@io_bazel_rules_go//go:def.bzl",
+    "go_binary",
+    "go_library",
+    "go_test",
+    "cgo_library",
+)
+
+go_library(
+    name = "go_default_library",
+    srcs = ["test_utils.go"],
+    tags = ["automanaged"],
+    deps = [
+        "//pkg/api:go_default_library",
+        "//pkg/api/errors:go_default_library",
+        "//pkg/api/resource:go_default_library",
+        "//pkg/api/v1:go_default_library",
+        "//pkg/apis/meta/v1:go_default_library",
+        "//pkg/client/clientset_generated/release_1_5/fake:go_default_library",
+        "//pkg/client/clientset_generated/release_1_5/typed/core/v1:go_default_library",
+        "//pkg/runtime:go_default_library",
+        "//pkg/util/clock:go_default_library",
+        "//pkg/util/node:go_default_library",
+        "//pkg/util/sets:go_default_library",
+        "//pkg/watch:go_default_library",
+    ],
+)

pkg/controller/node/test_utils.go → pkg/controller/node/testutil/test_utils.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package node
+package testutil
 
 import (
 	"errors"
@@ -56,7 +56,7 @@ type FakeNodeHandler struct {
 
 	// Synchronization
 	lock           sync.Mutex
-	deleteWaitChan chan struct{}
+	DeleteWaitChan chan struct{}
 }
 
 type FakeLegacyHandler struct {
@@ -64,7 +64,8 @@ type FakeLegacyHandler struct {
 	n *FakeNodeHandler
 }
 
-func (c *FakeNodeHandler) getUpdatedNodesCopy() []*v1.Node {
+// GetUpdatedNodesCopy returns a slice of Nodes with updates applied.
+func (c *FakeNodeHandler) GetUpdatedNodesCopy() []*v1.Node {
 	c.lock.Lock()
 	defer c.lock.Unlock()
 	updatedNodesCopy := make([]*v1.Node, len(c.UpdatedNodes), len(c.UpdatedNodes))
@@ -74,14 +75,17 @@ func (c *FakeNodeHandler) getUpdatedNodesCopy() []*v1.Node {
 	return updatedNodesCopy
 }
 
+// Core returns fake CoreInterface.
 func (c *FakeNodeHandler) Core() v1core.CoreV1Interface {
 	return &FakeLegacyHandler{c.Clientset.Core(), c}
 }
 
+// Nodes return fake NodeInterfaces.
 func (m *FakeLegacyHandler) Nodes() v1core.NodeInterface {
 	return m.n
 }
 
+// Create adds a new Node to the fake store.
 func (m *FakeNodeHandler) Create(node *v1.Node) (*v1.Node, error) {
 	m.lock.Lock()
 	defer func() {
@@ -102,6 +106,7 @@ func (m *FakeNodeHandler) Create(node *v1.Node) (*v1.Node, error) {
 	}
 }
 
+// Get returns a Node from the fake store.
 func (m *FakeNodeHandler) Get(name string) (*v1.Node, error) {
 	m.lock.Lock()
 	defer func() {
@@ -123,6 +128,7 @@ func (m *FakeNodeHandler) Get(name string) (*v1.Node, error) {
 	return nil, nil
 }
 
+// List returns a list of Nodes from the fake store.
 func (m *FakeNodeHandler) List(opts v1.ListOptions) (*v1.NodeList, error) {
 	m.lock.Lock()
 	defer func() {
@@ -152,23 +158,26 @@ func (m *FakeNodeHandler) List(opts v1.ListOptions) (*v1.NodeList, error) {
 	return nodeList, nil
 }
 
+// Delete delets a Node from the fake store.
 func (m *FakeNodeHandler) Delete(id string, opt *v1.DeleteOptions) error {
 	m.lock.Lock()
 	defer func() {
 		m.RequestCount++
-		if m.deleteWaitChan != nil {
-			m.deleteWaitChan <- struct{}{}
+		if m.DeleteWaitChan != nil {
+			m.DeleteWaitChan <- struct{}{}
 		}
 		m.lock.Unlock()
 	}()
-	m.DeletedNodes = append(m.DeletedNodes, newNode(id))
+	m.DeletedNodes = append(m.DeletedNodes, NewNode(id))
 	return nil
 }
 
+// DeleteCollection deletes a collection of Nodes from the fake store.
 func (m *FakeNodeHandler) DeleteCollection(opt *v1.DeleteOptions, listOpts v1.ListOptions) error {
 	return nil
 }
 
+// Update updates a Node in the fake store.
 func (m *FakeNodeHandler) Update(node *v1.Node) (*v1.Node, error) {
 	m.lock.Lock()
 	defer func() {
@@ -186,6 +195,7 @@ func (m *FakeNodeHandler) Update(node *v1.Node) (*v1.Node, error) {
 	return node, nil
 }
 
+// UpdateStatus updates a status of a Node in the fake store.
 func (m *FakeNodeHandler) UpdateStatus(node *v1.Node) (*v1.Node, error) {
 	m.lock.Lock()
 	defer func() {
@@ -197,15 +207,18 @@ func (m *FakeNodeHandler) UpdateStatus(node *v1.Node) (*v1.Node, error) {
 	return node, nil
 }
 
+// PatchStatus patches a status of a Node in the fake store.
 func (m *FakeNodeHandler) PatchStatus(nodeName string, data []byte) (*v1.Node, error) {
 	m.RequestCount++
 	return &v1.Node{}, nil
 }
 
+// Watch watches Nodes in a fake store.
 func (m *FakeNodeHandler) Watch(opts v1.ListOptions) (watch.Interface, error) {
 	return watch.NewFake(), nil
 }
 
+// Patch patches a Node in the fake store.
 func (m *FakeNodeHandler) Patch(name string, pt api.PatchType, data []byte, subresources ...string) (*v1.Node, error) {
 	return nil, nil
 }
@@ -213,18 +226,21 @@ func (m *FakeNodeHandler) Patch(name string, pt api.PatchType, data []byte, subr
 // FakeRecorder is used as a fake during testing.
 type FakeRecorder struct {
 	source v1.EventSource
-	events []*v1.Event
+	Events []*v1.Event
 	clock  clock.Clock
 }
 
+// Event emits a fake event to the fake recorder
 func (f *FakeRecorder) Event(obj runtime.Object, eventtype, reason, message string) {
 	f.generateEvent(obj, metav1.Now(), eventtype, reason, message)
 }
 
+// Eventf emits a fake formatted event to the fake recorder
 func (f *FakeRecorder) Eventf(obj runtime.Object, eventtype, reason, messageFmt string, args ...interface{}) {
 	f.Event(obj, eventtype, reason, fmt.Sprintf(messageFmt, args...))
 }
 
+// PastEventf is a no-op
 func (f *FakeRecorder) PastEventf(obj runtime.Object, timestamp metav1.Time, eventtype, reason, messageFmt string, args ...interface{}) {
 }
 
@@ -235,9 +251,9 @@ func (f *FakeRecorder) generateEvent(obj runtime.Object, timestamp metav1.Time,
 	}
 	event := f.makeEvent(ref, eventtype, reason, message)
 	event.Source = f.source
-	if f.events != nil {
+	if f.Events != nil {
 		fmt.Println("write event")
-		f.events = append(f.events, event)
+		f.Events = append(f.Events, event)
 	}
 }
 
@@ -263,15 +279,17 @@ func (f *FakeRecorder) makeEvent(ref *v1.ObjectReference, eventtype, reason, mes
 	}
 }
 
+// NewFakeRecorder returns a pointer to a newly constructed FakeRecorder.
 func NewFakeRecorder() *FakeRecorder {
 	return &FakeRecorder{
 		source: v1.EventSource{Component: "nodeControllerTest"},
-		events: []*v1.Event{},
+		Events: []*v1.Event{},
 		clock:  clock.NewFakeClock(time.Now()),
 	}
 }
 
-func newNode(name string) *v1.Node {
+// NewNode is a helper function for creating Nodes for testing.
+func NewNode(name string) *v1.Node {
 	return &v1.Node{
 		ObjectMeta: v1.ObjectMeta{Name: name},
 		Spec: v1.NodeSpec{
@@ -286,7 +304,8 @@ func newNode(name string) *v1.Node {
 	}
 }
 
-func newPod(name, host string) *v1.Pod {
+// NewPod is a helper function for creating Pods for testing.
+func NewPod(name, host string) *v1.Pod {
 	pod := &v1.Pod{
 		ObjectMeta: v1.ObjectMeta{
 			Namespace: "default",
@@ -317,8 +336,8 @@ func contains(node *v1.Node, nodes []*v1.Node) bool {
 	return false
 }
 
-// Returns list of zones for all Nodes stored in FakeNodeHandler
-func getZones(nodeHandler *FakeNodeHandler) []string {
+// GetZones returns list of zones for all Nodes stored in FakeNodeHandler
+func GetZones(nodeHandler *FakeNodeHandler) []string {
 	nodes, _ := nodeHandler.List(v1.ListOptions{})
 	zones := sets.NewString()
 	for _, node := range nodes.Items {
@@ -327,6 +346,7 @@ func getZones(nodeHandler *FakeNodeHandler) []string {
 	return zones.List()
 }
 
-func createZoneID(region, zone string) string {
+// CreateZoneID returns a single zoneID for a given region and zone.
+func CreateZoneID(region, zone string) string {
 	return region + ":\x00:" + zone
 }