Skip to content

Commit 69845a7

Browse files
krzykwaskrzykwas
committed
Use service CIDRs to configure the right set of Pod CIDRs.
Based on the set of service CIDRs, figure out if the cluster's stack type is IPv4, IPv6 or dual-stack. Then, configure the required set of Pod CIDRs. This fixes a bug, where a single-stack IPv4 cluster on a dual-stack network would have both IPv4 and IPv6 addresses assigned to Pods. Moreover it adds support for single-stack IPv6 clusters.
1 parent 4e278fc commit 69845a7

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)