Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Updating NewCIDRSet return a value #45792

Merged
merged 1 commit into from
Nov 9, 2017
Merged

Updating NewCIDRSet return a value #45792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 9 additions & 6 deletions pkg/controller/node/ipam/cidrset/cidr_set.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
45 changes: 33 additions & 12 deletions pkg/controller/node/ipam/cidrset/cidr_set_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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())
Expand All @@ -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

Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -292,7 +302,7 @@ func TestCIDRSet_AllocationOccupied(t *testing.T) {
}
}

var err error
//var err error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove?

_, err = a.AllocateNext()
if err == nil {
t.Fatalf("expected error because of fully-allocated range for %v", tc.description)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
7 changes: 6 additions & 1 deletion pkg/controller/node/ipam/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 4 additions & 1 deletion pkg/controller/node/ipam/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/controller/node/ipam/range_allocator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 6 additions & 3 deletions pkg/controller/node/ipam/sync/sync_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down