diff --git a/pkg/controller/node/ipam/cidrset/cidr_set.go b/pkg/controller/node/ipam/cidrset/cidr_set.go index c977fd08b8..088c62511b 100644 --- a/pkg/controller/node/ipam/cidrset/cidr_set.go +++ b/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 diff --git a/pkg/controller/node/ipam/cidrset/cidr_set_test.go b/pkg/controller/node/ipam/cidrset/cidr_set_test.go index a79cc100a1..826eeba9bd 100644 --- a/pkg/controller/node/ipam/cidrset/cidr_set_test.go +++ b/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 { diff --git a/pkg/controller/node/ipam/controller.go b/pkg/controller/node/ipam/controller.go index f8a2375678..4b1221b678 100644 --- a/pkg/controller/node/ipam/controller.go +++ b/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 { diff --git a/pkg/controller/node/ipam/controller_test.go b/pkg/controller/node/ipam/controller_test.go index 22baf2081b..14fbb4340f 100644 --- a/pkg/controller/node/ipam/controller_test.go +++ b/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) } diff --git a/pkg/controller/node/ipam/range_allocator.go b/pkg/controller/node/ipam/range_allocator.go index 5fdb90145a..102eac43d8 100644 --- a/pkg/controller/node/ipam/range_allocator.go +++ b/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, diff --git a/pkg/controller/node/ipam/sync/sync_test.go b/pkg/controller/node/ipam/sync/sync_test.go index 15d3e1b5b3..d326848043 100644 --- a/pkg/controller/node/ipam/sync/sync_test.go +++ b/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.