Updating NewCIDRSet return value

pkg/controller/node/ipam/cidrset/cidr_set.go

@@ -48,30 +48,33 @@ const (
 )
 
 var (
-	// ErrCIDRRangeNoCIDRsRemaining occurs when there are no more space
+	// ErrCIDRRangeNoCIDRsRemaining occurs when there is no more space
 	// to allocate CIDR ranges.
 	ErrCIDRRangeNoCIDRsRemaining = errors.New(
 		"CIDR allocation failed; there are no remaining CIDRs left to allocate in the accepted range")
+	// ErrCIDRSetSubNetTooBig occurs when the subnet mask size is too
+	// big compared to the CIDR mask size.
+	ErrCIDRSetSubNetTooBig = errors.New(
+		"New CIDR set failed; the node CIDR size is too big")
 )
 
 // NewCIDRSet creates a new CidrSet.
-func NewCIDRSet(clusterCIDR *net.IPNet, subNetMaskSize int) *CidrSet {
+func NewCIDRSet(clusterCIDR *net.IPNet, subNetMaskSize int) (*CidrSet, error) {
 	clusterMask := clusterCIDR.Mask
 	clusterMaskSize, _ := clusterMask.Size()
 
 	var maxCIDRs int
 	if (clusterCIDR.IP.To4() == nil) && (subNetMaskSize-clusterMaskSize > clusterSubnetMaxDiff) {
-		maxCIDRs = 0
-	} else {
-		maxCIDRs = 1 << uint32(subNetMaskSize-clusterMaskSize)
+		return nil, ErrCIDRSetSubNetTooBig
 	}
+	maxCIDRs = 1 << uint32(subNetMaskSize-clusterMaskSize)
 	return &CidrSet{
 		clusterCIDR:     clusterCIDR,
 		clusterIP:       clusterCIDR.IP,
 		clusterMaskSize: clusterMaskSize,
 		maxCIDRs:        maxCIDRs,
 		subNetMaskSize:  subNetMaskSize,
-	}
+	}, nil
 }
 
 // TODO: Remove this function when upgrading to go 1.9

pkg/controller/node/ipam/cidrset/cidr_set_test.go

@@ -47,8 +47,10 @@ func TestCIDRSetFullyAllocated(t *testing.T) {
 	}
 	for _, tc := range cases {
 		_, clusterCIDR, _ := net.ParseCIDR(tc.clusterCIDRStr)
-		a := NewCIDRSet(clusterCIDR, tc.subNetMaskSize)
-
+		a, err := NewCIDRSet(clusterCIDR, tc.subNetMaskSize)
+		if err != nil {
+			t.Fatalf("unexpected error: %v for %v", err, tc.description)
+		}
 		p, err := a.AllocateNext()
 		if err != nil {
 			t.Fatalf("unexpected error: %v for %v", err, tc.description)
@@ -196,7 +198,10 @@ func TestIndexToCIDRBlock(t *testing.T) {
 	}
 	for _, tc := range cases {
 		_, clusterCIDR, _ := net.ParseCIDR(tc.clusterCIDRStr)
-		a := NewCIDRSet(clusterCIDR, tc.subnetMaskSize)
+		a, err := NewCIDRSet(clusterCIDR, tc.subnetMaskSize)
+		if err != nil {
+			t.Fatalf("error for %v ", tc.description)
+		}
 		cidr := a.indexToCIDRBlock(tc.index)
 		if cidr.String() != tc.CIDRBlock {
 			t.Fatalf("error for %v index %d %s", tc.description, tc.index, cidr.String())
@@ -220,7 +225,10 @@ func TestCIDRSet_RandomishAllocation(t *testing.T) {
 	}
 	for _, tc := range cases {
 		_, clusterCIDR, _ := net.ParseCIDR(tc.clusterCIDRStr)
-		a := NewCIDRSet(clusterCIDR, 24)
+		a, err := NewCIDRSet(clusterCIDR, 24)
+		if err != nil {
+			t.Fatalf("Error allocating CIDRSet for %v", tc.description)
+		}
 		// allocate all the CIDRs
 		var cidrs []*net.IPNet
 
@@ -232,7 +240,7 @@ func TestCIDRSet_RandomishAllocation(t *testing.T) {
 			}
 		}
 
-		var err error
+		//var err error
 		_, err = a.AllocateNext()
 		if err == nil {
 			t.Fatalf("expected error because of fully-allocated range for %v", tc.description)
@@ -278,8 +286,10 @@ func TestCIDRSet_AllocationOccupied(t *testing.T) {
 	}
 	for _, tc := range cases {
 		_, clusterCIDR, _ := net.ParseCIDR(tc.clusterCIDRStr)
-		a := NewCIDRSet(clusterCIDR, 24)
-
+		a, err := NewCIDRSet(clusterCIDR, 24)
+		if err != nil {
+			t.Fatalf("Error allocating CIDRSet for %v", tc.description)
+		}
 		// allocate all the CIDRs
 		var cidrs []*net.IPNet
 		var numCIDRs = 256
@@ -292,7 +302,7 @@ func TestCIDRSet_AllocationOccupied(t *testing.T) {
 			}
 		}
 
-		var err error
+		//var err error
 		_, err = a.AllocateNext()
 		if err == nil {
 			t.Fatalf("expected error because of fully-allocated range for %v", tc.description)
@@ -457,8 +467,10 @@ func TestGetBitforCIDR(t *testing.T) {
 			t.Fatalf("unexpected error: %v for %v", err, tc.description)
 		}
 
-		cs := NewCIDRSet(clusterCIDR, tc.subNetMaskSize)
-
+		cs, err := NewCIDRSet(clusterCIDR, tc.subNetMaskSize)
+		if err != nil {
+			t.Fatalf("Error allocating CIDRSet for %v", tc.description)
+		}
 		_, subnetCIDR, err := net.ParseCIDR(tc.subNetCIDRStr)
 		if err != nil {
 			t.Fatalf("unexpected error: %v for %v", err, tc.description)
@@ -625,7 +637,10 @@ func TestOccupy(t *testing.T) {
 			t.Fatalf("unexpected error: %v for %v", err, tc.description)
 		}
 
-		cs := NewCIDRSet(clusterCIDR, tc.subNetMaskSize)
+		cs, err := NewCIDRSet(clusterCIDR, tc.subNetMaskSize)
+		if err != nil {
+			t.Fatalf("Error allocating CIDRSet for %v", tc.description)
+		}
 
 		_, subnetCIDR, err := net.ParseCIDR(tc.subNetCIDRStr)
 		if err != nil {
@@ -686,7 +701,13 @@ func TestCIDRSetv6(t *testing.T) {
 	}
 	for _, tc := range cases {
 		_, clusterCIDR, _ := net.ParseCIDR(tc.clusterCIDRStr)
-		a := NewCIDRSet(clusterCIDR, tc.subNetMaskSize)
+		a, err := NewCIDRSet(clusterCIDR, tc.subNetMaskSize)
+		if err != nil {
+			if tc.expectErr {
+				continue
+			}
+			t.Fatalf("Error allocating CIDRSet for %v", tc.description)
+		}
 
 		p, err := a.AllocateNext()
 		if err == nil && tc.expectErr {

pkg/controller/node/ipam/controller.go

@@ -76,11 +76,16 @@ func NewController(
 		return nil, fmt.Errorf("cloud IPAM controller does not support %q provider", cloud.ProviderName())
 	}
 
+	set, err := cidrset.NewCIDRSet(clusterCIDR, nodeCIDRMaskSize)
+	if err != nil {
+		return nil, err
+	}
+
 	c := &Controller{
 		config:  config,
 		adapter: newAdapter(kubeClient, gceCloud),
 		syncers: make(map[string]*nodesync.NodeSync),
-		set:     cidrset.NewCIDRSet(clusterCIDR, nodeCIDRMaskSize),
+		set:     set,
 	}
 
 	if err := occupyServiceCIDR(c.set, clusterCIDR, serviceCIDR); err != nil {

pkg/controller/node/ipam/controller_test.go

@@ -37,7 +37,10 @@ TestCase:
 		{"10.2.0.0/24"},
 	} {
 		serviceCIDR := test.MustParseCIDR(tc.serviceCIDR)
-		set := cidrset.NewCIDRSet(test.MustParseCIDR(clusterCIDR), 24)
+		set, err := cidrset.NewCIDRSet(test.MustParseCIDR(clusterCIDR), 24)
+		if err != nil {
+			t.Errorf("test case %+v: NewCIDRSet() = %v, want nil", tc, err)
+		}
 		if err := occupyServiceCIDR(set, test.MustParseCIDR(clusterCIDR), serviceCIDR); err != nil {
 			t.Errorf("test case %+v: occupyServiceCIDR() = %v, want nil", tc, err)
 		}

pkg/controller/node/ipam/range_allocator.go

@@ -70,9 +70,13 @@ func NewCIDRRangeAllocator(client clientset.Interface, clusterCIDR *net.IPNet, s
 	glog.V(0).Infof("Sending events to api server.")
 	eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{Interface: v1core.New(client.CoreV1().RESTClient()).Events("")})
 
+	set, err := cidrset.NewCIDRSet(clusterCIDR, subNetMaskSize)
+	if err != nil {
+		return nil, err
+	}
 	ra := &rangeAllocator{
 		client:                client,
-		cidrs:                 cidrset.NewCIDRSet(clusterCIDR, subNetMaskSize),
+		cidrs:                 set,
 		clusterCIDR:           clusterCIDR,
 		nodeCIDRUpdateChannel: make(chan nodeAndCIDR, cidrUpdateQueueSize),
 		recorder:              recorder,

pkg/controller/node/ipam/sync/sync_test.go

@@ -194,7 +194,8 @@ func TestNodeSyncUpdate(t *testing.T) {
 			wantError: false,
 		},
 	} {
-		sync := New(&tc.fake, &tc.fake, &tc.fake, tc.mode, "node1", cidrset.NewCIDRSet(clusterCIDRRange, 24))
+		cidr, _ := cidrset.NewCIDRSet(clusterCIDRRange, 24)
+		sync := New(&tc.fake, &tc.fake, &tc.fake, tc.mode, "node1", cidr)
 		doneChan := make(chan struct{})
 
 		// Do a single step of the loop.
@@ -225,7 +226,8 @@ func TestNodeSyncResync(t *testing.T) {
 		resyncTimeout: time.Millisecond,
 		reportChan:    make(chan struct{}),
 	}
-	sync := New(fake, fake, fake, SyncFromCluster, "node1", cidrset.NewCIDRSet(clusterCIDRRange, 24))
+	cidr, _ := cidrset.NewCIDRSet(clusterCIDRRange, 24)
+	sync := New(fake, fake, fake, SyncFromCluster, "node1", cidr)
 	doneChan := make(chan struct{})
 
 	go sync.Loop(doneChan)
@@ -267,7 +269,8 @@ func TestNodeSyncDelete(t *testing.T) {
 			},
 		},
 	} {
-		sync := New(&tc.fake, &tc.fake, &tc.fake, tc.mode, "node1", cidrset.NewCIDRSet(clusterCIDRRange, 24))
+		cidr, _ := cidrset.NewCIDRSet(clusterCIDRRange, 24)
+		sync := New(&tc.fake, &tc.fake, &tc.fake, tc.mode, "node1", cidr)
 		doneChan := make(chan struct{})
 
 		// Do a single step of the loop.