Skip to content

Commit 2ba796f

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #652 from krzykwas/master
Configure Pod CIDRs based on service CIDRs
2 parents 8b4e4f8 + 69845a7 commit 2ba796f

File tree

7 files changed

+711
-72
lines changed

7 files changed

+711
-72
lines changed

‎cmd/cloud-controller-manager/BUILD

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ load(
44
"@io_bazel_rules_go//go:def.bzl",
55
"go_binary",
66
"go_library",
7+
"go_test",
78
)
89
load("//defs:version.bzl", "version_x_defs")
910

@@ -53,3 +54,16 @@ go_library(
5354
load("//defs:container.bzl", "image")
5455

5556
image(binary = ":cloud-controller-manager")
57+
58+
go_test(
59+
name = "cloud-controller-manager_test",
60+
srcs = ["nodeipamcontroller_test.go"],
61+
embed = [":cloud-controller-manager_lib"],
62+
deps = [
63+
"//pkg/controller/nodeipam/config",
64+
"//vendor/k8s.io/cloud-provider",
65+
"//vendor/k8s.io/cloud-provider/app/config",
66+
"//vendor/k8s.io/cloud-provider/config",
67+
"//vendor/k8s.io/controller-manager/app",
68+
],
69+
)

‎cmd/cloud-controller-manager/nodeipamcontroller.go

Lines changed: 39 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -78,60 +78,56 @@ func startNodeIpamController(ccmConfig *cloudcontrollerconfig.CompletedConfig, n
7878
return nil, false, fmt.Errorf("the AllocateNodeCIDRs is not enabled")
7979
}
8080

81-
// the CloudAllocator ipam does not need service or cluster cidrs since it obtains the data from the cloud directly
82-
// for backward compatiblity reasons we don't fail if there are any of those values configured
83-
if ipam.CIDRAllocatorType(ccmConfig.ComponentConfig.KubeCloudShared.CIDRAllocatorType) != ipam.CloudAllocatorType {
84-
// failure: bad cidrs in config
85-
cidrs, dualStack, err := processCIDRs(ccmConfig.ComponentConfig.KubeCloudShared.ClusterCIDR)
86-
if err != nil {
87-
return nil, false, err
88-
}
89-
clusterCIDRs = cidrs
90-
91-
// failure: more than one cidr but they are not configured as dual stack
92-
if len(clusterCIDRs) > 1 && !dualStack {
93-
return nil, false, fmt.Errorf("len of ClusterCIDRs==%v and they are not configured as dual stack (at least one from each IPFamily", len(clusterCIDRs))
94-
}
81+
// failure: bad cidrs in config
82+
cidrs, dualStack, err := processCIDRs(ccmConfig.ComponentConfig.KubeCloudShared.ClusterCIDR)
83+
if err != nil {
84+
return nil, false, err
85+
}
86+
clusterCIDRs = cidrs
9587

96-
// failure: more than cidrs is not allowed even with dual stack
97-
if len(clusterCIDRs) > 2 {
98-
return nil, false, fmt.Errorf("len of clusters is:%v > more than max allowed of 2", len(clusterCIDRs))
99-
}
88+
// failure: more than one cidr but they are not configured as dual stack
89+
if len(clusterCIDRs) > 1 && !dualStack {
90+
return nil, false, fmt.Errorf("len of ClusterCIDRs==%v and they are not configured as dual stack (at least one from each IPFamily", len(clusterCIDRs))
91+
}
10092

101-
// service cidr processing
102-
if len(strings.TrimSpace(nodeIPAMConfig.ServiceCIDR)) != 0 {
103-
_, serviceCIDR, err = net.ParseCIDR(nodeIPAMConfig.ServiceCIDR)
104-
if err != nil {
105-
klog.Warningf("Unsuccessful parsing of service CIDR %v: %v", nodeIPAMConfig.ServiceCIDR, err)
106-
}
107-
}
93+
// failure: more than 2 cidrs is not allowed even with dual stack
94+
if len(clusterCIDRs) > 2 {
95+
return nil, false, fmt.Errorf("len of clusters cidrs is:%v > more than max allowed of 2", len(clusterCIDRs))
96+
}
10897

109-
if len(strings.TrimSpace(nodeIPAMConfig.SecondaryServiceCIDR)) != 0 {
110-
_, secondaryServiceCIDR, err = net.ParseCIDR(nodeIPAMConfig.SecondaryServiceCIDR)
111-
if err != nil {
112-
klog.Warningf("Unsuccessful parsing of service CIDR %v: %v", nodeIPAMConfig.SecondaryServiceCIDR, err)
113-
}
98+
// service cidr processing
99+
if len(strings.TrimSpace(nodeIPAMConfig.ServiceCIDR)) != 0 {
100+
_, serviceCIDR, err = net.ParseCIDR(nodeIPAMConfig.ServiceCIDR)
101+
if err != nil {
102+
klog.Warningf("Unsuccessful parsing of service CIDR %v: %v", nodeIPAMConfig.ServiceCIDR, err)
114103
}
104+
}
115105

116-
// the following checks are triggered if both serviceCIDR and secondaryServiceCIDR are provided
117-
if serviceCIDR != nil && secondaryServiceCIDR != nil {
118-
// should be dual stack (from different IPFamilies)
119-
dualstackServiceCIDR, err := netutils.IsDualStackCIDRs([]*net.IPNet{serviceCIDR, secondaryServiceCIDR})
120-
if err != nil {
121-
return nil, false, fmt.Errorf("failed to perform dualstack check on serviceCIDR and secondaryServiceCIDR error:%v", err)
122-
}
123-
if !dualstackServiceCIDR {
124-
return nil, false, fmt.Errorf("serviceCIDR and secondaryServiceCIDR are not dualstack (from different IPfamiles)")
125-
}
106+
if len(strings.TrimSpace(nodeIPAMConfig.SecondaryServiceCIDR)) != 0 {
107+
_, secondaryServiceCIDR, err = net.ParseCIDR(nodeIPAMConfig.SecondaryServiceCIDR)
108+
if err != nil {
109+
klog.Warningf("Unsuccessful parsing of service CIDR %v: %v", nodeIPAMConfig.SecondaryServiceCIDR, err)
126110
}
111+
}
127112

128-
// get list of node cidr mask sizes
129-
nodeCIDRMaskSizes, err = setNodeCIDRMaskSizes(nodeIPAMConfig, clusterCIDRs)
113+
// the following checks are triggered if both serviceCIDR and secondaryServiceCIDR are provided
114+
if serviceCIDR != nil && secondaryServiceCIDR != nil {
115+
// should be dual stack (from different IPFamilies)
116+
dualstackServiceCIDR, err := netutils.IsDualStackCIDRs([]*net.IPNet{serviceCIDR, secondaryServiceCIDR})
130117
if err != nil {
131-
return nil, false, err
118+
return nil, false, fmt.Errorf("failed to perform dualstack check on serviceCIDR and secondaryServiceCIDR error:%v", err)
119+
}
120+
if !dualstackServiceCIDR {
121+
return nil, false, fmt.Errorf("serviceCIDR and secondaryServiceCIDR are not dualstack (from different IPfamiles)")
132122
}
133123
}
134124

125+
// get list of node cidr mask sizes
126+
nodeCIDRMaskSizes, err = setNodeCIDRMaskSizes(nodeIPAMConfig, clusterCIDRs)
127+
if err != nil {
128+
return nil, false, err
129+
}
130+
135131
kubeConfig := ccmConfig.Complete().Kubeconfig
136132
kubeConfig.ContentType = "application/json" // required to serialize Networks to json
137133
networkClient, err := networkclientset.NewForConfig(kubeConfig)

‎cmd/cloud-controller-manager/nodeipamcontroller_test.go

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
package main
2+
3+
import (
4+
"testing"
5+
6+
cloudprovider "k8s.io/cloud-provider"
7+
nodeipamconfig "k8s.io/cloud-provider-gcp/pkg/controller/nodeipam/config"
8+
cloudcontrollerconfig "k8s.io/cloud-provider/app/config"
9+
"k8s.io/cloud-provider/config"
10+
genericcontrollermanager "k8s.io/controller-manager/app"
11+
)
12+
13+
type fakeCloudProvider struct{}
14+
15+
// Implements cloudprovider.Interface.
16+
var _ cloudprovider.Interface = &fakeCloudProvider{}
17+
18+
func (f *fakeCloudProvider) Initialize(clientBuilder cloudprovider.ControllerClientBuilder, stop <-chan struct{}) {
19+
}
20+
21+
func (f *fakeCloudProvider) LoadBalancer() (cloudprovider.LoadBalancer, bool) {
22+
return nil, false
23+
}
24+
25+
func (f *fakeCloudProvider) Instances() (cloudprovider.Instances, bool) {
26+
return nil, false
27+
}
28+
29+
func (f *fakeCloudProvider) InstancesV2() (cloudprovider.InstancesV2, bool) {
30+
return nil, false
31+
}
32+
33+
func (f *fakeCloudProvider) Zones() (cloudprovider.Zones, bool) {
34+
return nil, false
35+
}
36+
37+
func (f *fakeCloudProvider) Clusters() (cloudprovider.Clusters, bool) {
38+
return nil, false
39+
}
40+
41+
func (f *fakeCloudProvider) Routes() (cloudprovider.Routes, bool) {
42+
return nil, false
43+
}
44+
45+
func (f *fakeCloudProvider) ProviderName() string {
46+
return "fake"
47+
}
48+
49+
func (f *fakeCloudProvider) HasClusterID() bool {
50+
return false
51+
}
52+
53+
func TestStartNodeIpamController(t *testing.T) {
54+
testCases := []struct {
55+
desc string
56+
ccmConfig *cloudcontrollerconfig.Config
57+
nodeIPAMConfig nodeipamconfig.NodeIPAMControllerConfiguration
58+
wantErr bool
59+
}{
60+
{
61+
desc: "Allocate node CIDRs disabled",
62+
ccmConfig: &cloudcontrollerconfig.Config{
63+
ComponentConfig: config.CloudControllerManagerConfiguration{
64+
KubeCloudShared: config.KubeCloudSharedConfiguration{
65+
AllocateNodeCIDRs: false,
66+
},
67+
},
68+
},
69+
wantErr: true,
70+
},
71+
{
72+
desc: "Unparseable cluster CIDRs",
73+
ccmConfig: &cloudcontrollerconfig.Config{
74+
ComponentConfig: config.CloudControllerManagerConfiguration{
75+
KubeCloudShared: config.KubeCloudSharedConfiguration{
76+
AllocateNodeCIDRs: true,
77+
ClusterCIDR: "invalid",
78+
},
79+
},
80+
},
81+
wantErr: true,
82+
},
83+
{
84+
desc: "Multiple same stack type cluster CIDRs - ipv4",
85+
ccmConfig: &cloudcontrollerconfig.Config{
86+
ComponentConfig: config.CloudControllerManagerConfiguration{
87+
KubeCloudShared: config.KubeCloudSharedConfiguration{
88+
AllocateNodeCIDRs: true,
89+
ClusterCIDR: "10.0.0.0/16,10.1.0.0/16",
90+
},
91+
},
92+
},
93+
wantErr: true,
94+
},
95+
{
96+
desc: "Multiple same stack type cluster CIDRs - ipv6",
97+
ccmConfig: &cloudcontrollerconfig.Config{
98+
ComponentConfig: config.CloudControllerManagerConfiguration{
99+
KubeCloudShared: config.KubeCloudSharedConfiguration{
100+
AllocateNodeCIDRs: true,
101+
ClusterCIDR: "2001:db8::/112,2001:db9::/112",
102+
},
103+
},
104+
},
105+
wantErr: true,
106+
},
107+
{
108+
desc: "More than 2 cluster CIDRs",
109+
ccmConfig: &cloudcontrollerconfig.Config{
110+
ComponentConfig: config.CloudControllerManagerConfiguration{
111+
KubeCloudShared: config.KubeCloudSharedConfiguration{
112+
AllocateNodeCIDRs: true,
113+
ClusterCIDR: "10.0.0.0/16,10.1.0.0/16,10.2.0.0/16",
114+
},
115+
},
116+
},
117+
wantErr: true,
118+
},
119+
{
120+
desc: "Primary and secondary service CIDR same stack type - ipv4",
121+
ccmConfig: &cloudcontrollerconfig.Config{
122+
ComponentConfig: config.CloudControllerManagerConfiguration{
123+
KubeCloudShared: config.KubeCloudSharedConfiguration{
124+
AllocateNodeCIDRs: true,
125+
ClusterCIDR: "10.0.0.0/16",
126+
},
127+
},
128+
},
129+
nodeIPAMConfig: nodeipamconfig.NodeIPAMControllerConfiguration{
130+
ServiceCIDR: "10.0.0.0/16",
131+
SecondaryServiceCIDR: "10.1.0.0/16",
132+
},
133+
wantErr: true,
134+
},
135+
{
136+
desc: "Primary and secondary service CIDR same stack type - ipv6",
137+
ccmConfig: &cloudcontrollerconfig.Config{
138+
ComponentConfig: config.CloudControllerManagerConfiguration{
139+
KubeCloudShared: config.KubeCloudSharedConfiguration{
140+
AllocateNodeCIDRs: true,
141+
ClusterCIDR: "10.0.0.0/16",
142+
},
143+
},
144+
},
145+
nodeIPAMConfig: nodeipamconfig.NodeIPAMControllerConfiguration{
146+
ServiceCIDR: "2001:db8::/112",
147+
SecondaryServiceCIDR: "2001:db9::/112",
148+
},
149+
wantErr: true,
150+
},
151+
{
152+
desc: "NodeCIDRMaskSize used with a dual stack cluster",
153+
ccmConfig: &cloudcontrollerconfig.Config{
154+
ComponentConfig: config.CloudControllerManagerConfiguration{
155+
KubeCloudShared: config.KubeCloudSharedConfiguration{
156+
AllocateNodeCIDRs: true,
157+
ClusterCIDR: "10.0.0.0/16,2001:aa::/112",
158+
},
159+
},
160+
},
161+
nodeIPAMConfig: nodeipamconfig.NodeIPAMControllerConfiguration{
162+
NodeCIDRMaskSize: 10,
163+
},
164+
wantErr: true,
165+
},
166+
{
167+
desc: "NodeCIDRMaskSize and NodeCIDRMaskSizeIPv4 used together",
168+
ccmConfig: &cloudcontrollerconfig.Config{
169+
ComponentConfig: config.CloudControllerManagerConfiguration{
170+
KubeCloudShared: config.KubeCloudSharedConfiguration{
171+
AllocateNodeCIDRs: true,
172+
ClusterCIDR: "10.0.0.0/16",
173+
},
174+
},
175+
},
176+
nodeIPAMConfig: nodeipamconfig.NodeIPAMControllerConfiguration{
177+
NodeCIDRMaskSize: 10,
178+
NodeCIDRMaskSizeIPv4: 4,
179+
},
180+
wantErr: true,
181+
},
182+
{
183+
desc: "NodeCIDRMaskSize and NodeCIDRMaskSizeIPv6 used together",
184+
ccmConfig: &cloudcontrollerconfig.Config{
185+
ComponentConfig: config.CloudControllerManagerConfiguration{
186+
KubeCloudShared: config.KubeCloudSharedConfiguration{
187+
AllocateNodeCIDRs: true,
188+
ClusterCIDR: "10.0.0.0/16",
189+
},
190+
},
191+
},
192+
nodeIPAMConfig: nodeipamconfig.NodeIPAMControllerConfiguration{
193+
NodeCIDRMaskSize: 10,
194+
NodeCIDRMaskSizeIPv6: 4,
195+
},
196+
wantErr: true,
197+
},
198+
}
199+
200+
for _, tc := range testCases {
201+
t.Run(tc.desc, func(t *testing.T) {
202+
ctx := genericcontrollermanager.ControllerContext{}
203+
_, _, err := startNodeIpamController(tc.ccmConfig.Complete(), tc.nodeIPAMConfig, ctx, &fakeCloudProvider{})
204+
205+
if err == nil && tc.wantErr {
206+
t.Fatalf("startNodeIpamController succeeded, want error")
207+
}
208+
if err != nil && !tc.wantErr {
209+
t.Fatalf("startNodeIpamController(): %v", err)
210+
}
211+
})
212+
}
213+
}

‎pkg/controller/nodeipam/ipam/cidr_allocator.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ func New(kubeClient clientset.Interface, cloud cloudprovider.Interface, nodeInfo
118118
case RangeAllocatorType:
119119
return NewCIDRRangeAllocator(kubeClient, nodeInformer, allocatorParams, nodeList)
120120
case CloudAllocatorType:
121-
return NewCloudCIDRAllocator(kubeClient, cloud, nwInformer, gnpInformer, nodeInformer)
121+
return NewCloudCIDRAllocator(kubeClient, cloud, nwInformer, gnpInformer, nodeInformer, allocatorParams)
122122
default:
123123
return nil, fmt.Errorf("invalid CIDR allocator type: %v", allocatorType)
124124
}

0 commit comments

Comments
 (0)