Skip to content

Commit 313a5c5

Browse files
khenidakkhenidak
committed
phase 2: ipam filter secondary service cidr
1 parent 93c0682 commit 313a5c5

File tree

11 files changed

+128
-55
lines changed

11 files changed

+128
-55
lines changed

‎api/api-rules/violation_exceptions.list‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,7 @@ API rule violation: names_match,k8s.io/kube-controller-manager/config/v1alpha1,K
636636
API rule violation: names_match,k8s.io/kube-controller-manager/config/v1alpha1,NamespaceControllerConfiguration,ConcurrentNamespaceSyncs
637637
API rule violation: names_match,k8s.io/kube-controller-manager/config/v1alpha1,NamespaceControllerConfiguration,NamespaceSyncPeriod
638638
API rule violation: names_match,k8s.io/kube-controller-manager/config/v1alpha1,NodeIPAMControllerConfiguration,NodeCIDRMaskSize
639+
API rule violation: names_match,k8s.io/kube-controller-manager/config/v1alpha1,NodeIPAMControllerConfiguration,SecondaryServiceCIDR
639640
API rule violation: names_match,k8s.io/kube-controller-manager/config/v1alpha1,NodeIPAMControllerConfiguration,ServiceCIDR
640641
API rule violation: names_match,k8s.io/kube-controller-manager/config/v1alpha1,NodeLifecycleControllerConfiguration,EnableTaintManager
641642
API rule violation: names_match,k8s.io/kube-controller-manager/config/v1alpha1,NodeLifecycleControllerConfiguration,LargeClusterSizeThreshold

‎cmd/kube-controller-manager/app/core.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ func startServiceController(ctx ControllerContext) (http.Handler, bool, error) {
8383
}
8484
func startNodeIpamController(ctx ControllerContext) (http.Handler, bool, error) {
8585
var serviceCIDR *net.IPNet
86+
var secondaryServiceCIDR *net.IPNet
8687

8788
// should we start nodeIPAM
8889
if !ctx.ComponentConfig.KubeCloudShared.AllocateNodeCIDRs {
@@ -118,12 +119,37 @@ func startNodeIpamController(ctx ControllerContext) (http.Handler, bool, error)
118119
}
119120
}
120121

122+
if len(strings.TrimSpace(ctx.ComponentConfig.NodeIPAMController.SecondaryServiceCIDR)) != 0 {
123+
_, secondaryServiceCIDR, err = net.ParseCIDR(ctx.ComponentConfig.NodeIPAMController.SecondaryServiceCIDR)
124+
if err != nil {
125+
klog.Warningf("Unsuccessful parsing of service CIDR %v: %v", ctx.ComponentConfig.NodeIPAMController.SecondaryServiceCIDR, err)
126+
}
127+
}
128+
129+
// the following checks are triggered if both serviceCIDR and secondaryServiceCIDR are provided
130+
if serviceCIDR != nil && secondaryServiceCIDR != nil {
131+
// should have dual stack flag enabled
132+
if !utilfeature.DefaultFeatureGate.Enabled(kubefeatures.IPv6DualStack) {
133+
return nil, false, fmt.Errorf("secondary service cidr is provided and IPv6DualStack feature is not enabled")
134+
}
135+
136+
// should be dual stack (from different IPFamilies)
137+
dualstackServiceCIDR, err := netutils.IsDualStackCIDRs([]*net.IPNet{serviceCIDR, secondaryServiceCIDR})
138+
if err != nil {
139+
return nil, false, fmt.Errorf("failed to perform dualstack check on serviceCIDR and secondaryServiceCIDR error:%v", err)
140+
}
141+
if !dualstackServiceCIDR {
142+
return nil, false, fmt.Errorf("serviceCIDR and secondaryServiceCIDR are not dualstack (from different IPfamiles)")
143+
}
144+
}
145+
121146
nodeIpamController, err := nodeipamcontroller.NewNodeIpamController(
122147
ctx.InformerFactory.Core().V1().Nodes(),
123148
ctx.Cloud,
124149
ctx.ClientBuilder.ClientOrDie("node-controller"),
125150
clusterCIDRs,
126151
serviceCIDR,
152+
secondaryServiceCIDR,
127153
int(ctx.ComponentConfig.NodeIPAMController.NodeCIDRMaskSize),
128154
ipam.CIDRAllocatorType(ctx.ComponentConfig.KubeCloudShared.CIDRAllocatorType),
129155
)

‎cmd/kube-controller-manager/app/options/nodeipamcontroller.go‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ limitations under the License.
1717
package options
1818

1919
import (
20+
"fmt"
21+
"strings"
22+
2023
"github.com/spf13/pflag"
2124

2225
nodeipamconfig "k8s.io/kubernetes/pkg/controller/nodeipam/config"
@@ -32,7 +35,6 @@ func (o *NodeIPAMControllerOptions) AddFlags(fs *pflag.FlagSet) {
3235
if o == nil {
3336
return
3437
}
35-
3638
fs.StringVar(&o.ServiceCIDR, "service-cluster-ip-range", o.ServiceCIDR, "CIDR Range for Services in cluster. Requires --allocate-node-cidrs to be true")
3739
fs.Int32Var(&o.NodeCIDRMaskSize, "node-cidr-mask-size", o.NodeCIDRMaskSize, "Mask size for node cidr in cluster.")
3840
}
@@ -43,7 +45,15 @@ func (o *NodeIPAMControllerOptions) ApplyTo(cfg *nodeipamconfig.NodeIPAMControll
4345
return nil
4446
}
4547

46-
cfg.ServiceCIDR = o.ServiceCIDR
48+
// split the cidrs list and assign primary and secondary
49+
serviceCIDRList := strings.Split(o.ServiceCIDR, ",")
50+
if len(serviceCIDRList) > 0 {
51+
cfg.ServiceCIDR = serviceCIDRList[0]
52+
}
53+
if len(serviceCIDRList) > 1 {
54+
cfg.SecondaryServiceCIDR = serviceCIDRList[1]
55+
}
56+
4757
cfg.NodeCIDRMaskSize = o.NodeCIDRMaskSize
4858

4959
return nil
@@ -54,7 +64,12 @@ func (o *NodeIPAMControllerOptions) Validate() []error {
5464
if o == nil {
5565
return nil
5666
}
67+
errs := make([]error, 0)
68+
69+
serviceCIDRList := strings.Split(o.ServiceCIDR, ",")
70+
if len(serviceCIDRList) > 2 {
71+
errs = append(errs, fmt.Errorf("--service-cluster-ip-range can not contain more than two entries"))
72+
}
5773

58-
errs := []error{}
5974
return errs
6075
}

‎pkg/controller/nodeipam/config/types.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ package config
2020
type NodeIPAMControllerConfiguration struct {
2121
// serviceCIDR is CIDR Range for Services in cluster.
2222
ServiceCIDR string
23+
// secondaryServiceCIDR is CIDR Range for Services in cluster. This is used in dual stack clusters. SecondaryServiceCIDR must be of different IP family than ServiceCIDR
24+
SecondaryServiceCIDR string
2325
// NodeCIDRMaskSize is the mask size for node cidr in cluster.
2426
NodeCIDRMaskSize int32
2527
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,15 +89,15 @@ type CIDRAllocator interface {
8989
}
9090

9191
// New creates a new CIDR range allocator.
92-
func New(kubeClient clientset.Interface, cloud cloudprovider.Interface, nodeInformer informers.NodeInformer, allocatorType CIDRAllocatorType, clusterCIDRs []*net.IPNet, serviceCIDR *net.IPNet, nodeCIDRMaskSize int) (CIDRAllocator, error) {
92+
func New(kubeClient clientset.Interface, cloud cloudprovider.Interface, nodeInformer informers.NodeInformer, allocatorType CIDRAllocatorType, clusterCIDRs []*net.IPNet, serviceCIDR *net.IPNet, secondaryServiceCIDR *net.IPNet, nodeCIDRMaskSize int) (CIDRAllocator, error) {
9393
nodeList, err := listNodes(kubeClient)
9494
if err != nil {
9595
return nil, err
9696
}
9797

9898
switch allocatorType {
9999
case RangeAllocatorType:
100-
return NewCIDRRangeAllocator(kubeClient, nodeInformer, clusterCIDRs, serviceCIDR, nodeCIDRMaskSize, nodeList)
100+
return NewCIDRRangeAllocator(kubeClient, nodeInformer, clusterCIDRs, serviceCIDR, secondaryServiceCIDR, nodeCIDRMaskSize, nodeList)
101101
case CloudAllocatorType:
102102
return NewCloudCIDRAllocator(kubeClient, cloud, nodeInformer)
103103
default:

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ type rangeAllocator struct {
7171
// Caller must always pass in a list of existing nodes so the new allocator.
7272
// Caller must ensure that ClusterCIDRs are semantically correct e.g (1 for non DualStack, 2 for DualStack etc..)
7373
// can initialize its CIDR map. NodeList is only nil in testing.
74-
func NewCIDRRangeAllocator(client clientset.Interface, nodeInformer informers.NodeInformer, clusterCIDRs []*net.IPNet, serviceCIDR *net.IPNet, subNetMaskSize int, nodeList *v1.NodeList) (CIDRAllocator, error) {
74+
func NewCIDRRangeAllocator(client clientset.Interface, nodeInformer informers.NodeInformer, clusterCIDRs []*net.IPNet, serviceCIDR *net.IPNet, secondaryServiceCIDR *net.IPNet, subNetMaskSize int, nodeList *v1.NodeList) (CIDRAllocator, error) {
7575
if client == nil {
7676
klog.Fatalf("kubeClient is nil when starting NodeController")
7777
}
@@ -110,6 +110,12 @@ func NewCIDRRangeAllocator(client clientset.Interface, nodeInformer informers.No
110110
klog.V(0).Info("No Service CIDR provided. Skipping filtering out service addresses.")
111111
}
112112

113+
if secondaryServiceCIDR != nil {
114+
ra.filterOutServiceRange(secondaryServiceCIDR)
115+
} else {
116+
klog.V(0).Info("No Secondary Service CIDR provided. Skipping filtering out secondary service addresses.")
117+
}
118+
113119
if nodeList != nil {
114120
for _, node := range nodeList.Items {
115121
if len(node.Spec.PodCIDRs) == 0 {
@@ -295,6 +301,7 @@ func (r *rangeAllocator) filterOutServiceRange(serviceCIDR *net.IPNet) {
295301
// serviceCIDR) or vice versa (which means that serviceCIDR contains
296302
// clusterCIDR).
297303
for idx, cidr := range r.clusterCIDRs {
304+
// if they don't overlap then ignore the filtering
298305
if !cidr.Contains(serviceCIDR.IP.Mask(cidr.Mask)) && !serviceCIDR.Contains(cidr.IP.Mask(serviceCIDR.Mask)) {
299306
continue
300307
}

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

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,12 @@ func getFakeNodeInformer(fakeNodeHandler *testutil.FakeNodeHandler) coreinformer
6060
}
6161

6262
type testCase struct {
63-
description string
64-
fakeNodeHandler *testutil.FakeNodeHandler
65-
clusterCIDRs []*net.IPNet
66-
serviceCIDR *net.IPNet
67-
subNetMaskSize int
63+
description string
64+
fakeNodeHandler *testutil.FakeNodeHandler
65+
clusterCIDRs []*net.IPNet
66+
serviceCIDR *net.IPNet
67+
secondaryServiceCIDR *net.IPNet
68+
subNetMaskSize int
6869
// key is index of the cidr allocated
6970
expectedAllocatedCIDR map[int]string
7071
allocatedCIDRs map[int][]string
@@ -89,8 +90,9 @@ func TestAllocateOrOccupyCIDRSuccess(t *testing.T) {
8990
_, clusterCIDR, _ := net.ParseCIDR("127.123.234.0/24")
9091
return []*net.IPNet{clusterCIDR}
9192
}(),
92-
serviceCIDR: nil,
93-
subNetMaskSize: 30,
93+
serviceCIDR: nil,
94+
secondaryServiceCIDR: nil,
95+
subNetMaskSize: 30,
9496
expectedAllocatedCIDR: map[int]string{
9597
0: "127.123.234.0/30",
9698
},
@@ -115,7 +117,8 @@ func TestAllocateOrOccupyCIDRSuccess(t *testing.T) {
115117
_, serviceCIDR, _ := net.ParseCIDR("127.123.234.0/26")
116118
return serviceCIDR
117119
}(),
118-
subNetMaskSize: 30,
120+
secondaryServiceCIDR: nil,
121+
subNetMaskSize: 30,
119122
// it should return first /30 CIDR after service range
120123
expectedAllocatedCIDR: map[int]string{
121124
0: "127.123.234.64/30",
@@ -141,7 +144,8 @@ func TestAllocateOrOccupyCIDRSuccess(t *testing.T) {
141144
_, serviceCIDR, _ := net.ParseCIDR("127.123.234.0/26")
142145
return serviceCIDR
143146
}(),
144-
subNetMaskSize: 30,
147+
secondaryServiceCIDR: nil,
148+
subNetMaskSize: 30,
145149
allocatedCIDRs: map[int][]string{
146150
0: {"127.123.234.64/30", "127.123.234.68/30", "127.123.234.72/30", "127.123.234.80/30"},
147151
},
@@ -170,6 +174,7 @@ func TestAllocateOrOccupyCIDRSuccess(t *testing.T) {
170174
_, serviceCIDR, _ := net.ParseCIDR("127.123.234.0/26")
171175
return serviceCIDR
172176
}(),
177+
secondaryServiceCIDR: nil,
173178
},
174179
{
175180
description: "Dualstack CIDRs v6,v4",
@@ -192,6 +197,7 @@ func TestAllocateOrOccupyCIDRSuccess(t *testing.T) {
192197
_, serviceCIDR, _ := net.ParseCIDR("127.123.234.0/26")
193198
return serviceCIDR
194199
}(),
200+
secondaryServiceCIDR: nil,
195201
},
196202

197203
{
@@ -216,13 +222,14 @@ func TestAllocateOrOccupyCIDRSuccess(t *testing.T) {
216222
_, serviceCIDR, _ := net.ParseCIDR("127.123.234.0/26")
217223
return serviceCIDR
218224
}(),
225+
secondaryServiceCIDR: nil,
219226
},
220227
}
221228

222229
// test function
223230
testFunc := func(tc testCase) {
224231
// Initialize the range allocator.
225-
allocator, err := NewCIDRRangeAllocator(tc.fakeNodeHandler, getFakeNodeInformer(tc.fakeNodeHandler), tc.clusterCIDRs, tc.serviceCIDR, tc.subNetMaskSize, nil)
232+
allocator, err := NewCIDRRangeAllocator(tc.fakeNodeHandler, getFakeNodeInformer(tc.fakeNodeHandler), tc.clusterCIDRs, tc.serviceCIDR, tc.secondaryServiceCIDR, tc.subNetMaskSize, nil)
226233
if err != nil {
227234
t.Errorf("%v: failed to create CIDRRangeAllocator with error %v", tc.description, err)
228235
return
@@ -298,8 +305,9 @@ func TestAllocateOrOccupyCIDRFailure(t *testing.T) {
298305
_, clusterCIDR, _ := net.ParseCIDR("127.123.234.0/28")
299306
return []*net.IPNet{clusterCIDR}
300307
}(),
301-
serviceCIDR: nil,
302-
subNetMaskSize: 30,
308+
serviceCIDR: nil,
309+
secondaryServiceCIDR: nil,
310+
subNetMaskSize: 30,
303311
allocatedCIDRs: map[int][]string{
304312
0: {"127.123.234.0/30", "127.123.234.4/30", "127.123.234.8/30", "127.123.234.12/30"},
305313
},
@@ -308,7 +316,7 @@ func TestAllocateOrOccupyCIDRFailure(t *testing.T) {
308316

309317
testFunc := func(tc testCase) {
310318
// Initialize the range allocator.
311-
allocator, err := NewCIDRRangeAllocator(tc.fakeNodeHandler, getFakeNodeInformer(tc.fakeNodeHandler), tc.clusterCIDRs, tc.serviceCIDR, tc.subNetMaskSize, nil)
319+
allocator, err := NewCIDRRangeAllocator(tc.fakeNodeHandler, getFakeNodeInformer(tc.fakeNodeHandler), tc.clusterCIDRs, tc.serviceCIDR, tc.secondaryServiceCIDR, tc.subNetMaskSize, nil)
312320
if err != nil {
313321
t.Logf("%v: failed to create CIDRRangeAllocator with error %v", tc.description, err)
314322
}
@@ -369,6 +377,7 @@ type releaseTestCase struct {
369377
fakeNodeHandler *testutil.FakeNodeHandler
370378
clusterCIDRs []*net.IPNet
371379
serviceCIDR *net.IPNet
380+
secondaryServiceCIDR *net.IPNet
372381
subNetMaskSize int
373382
expectedAllocatedCIDRFirstRound map[int]string
374383
expectedAllocatedCIDRSecondRound map[int]string
@@ -394,8 +403,9 @@ func TestReleaseCIDRSuccess(t *testing.T) {
394403
_, clusterCIDR, _ := net.ParseCIDR("127.123.234.0/28")
395404
return []*net.IPNet{clusterCIDR}
396405
}(),
397-
serviceCIDR: nil,
398-
subNetMaskSize: 30,
406+
serviceCIDR: nil,
407+
secondaryServiceCIDR: nil,
408+
subNetMaskSize: 30,
399409
allocatedCIDRs: map[int][]string{
400410
0: {"127.123.234.0/30", "127.123.234.4/30", "127.123.234.8/30", "127.123.234.12/30"},
401411
},
@@ -423,8 +433,9 @@ func TestReleaseCIDRSuccess(t *testing.T) {
423433
_, clusterCIDR, _ := net.ParseCIDR("127.123.234.0/28")
424434
return []*net.IPNet{clusterCIDR}
425435
}(),
426-
serviceCIDR: nil,
427-
subNetMaskSize: 30,
436+
serviceCIDR: nil,
437+
secondaryServiceCIDR: nil,
438+
subNetMaskSize: 30,
428439
allocatedCIDRs: map[int][]string{
429440
0: {"127.123.234.4/30", "127.123.234.8/30", "127.123.234.12/30"},
430441
},
@@ -442,7 +453,7 @@ func TestReleaseCIDRSuccess(t *testing.T) {
442453

443454
testFunc := func(tc releaseTestCase) {
444455
// Initialize the range allocator.
445-
allocator, _ := NewCIDRRangeAllocator(tc.fakeNodeHandler, getFakeNodeInformer(tc.fakeNodeHandler), tc.clusterCIDRs, tc.serviceCIDR, tc.subNetMaskSize, nil)
456+
allocator, _ := NewCIDRRangeAllocator(tc.fakeNodeHandler, getFakeNodeInformer(tc.fakeNodeHandler), tc.clusterCIDRs, tc.serviceCIDR, tc.secondaryServiceCIDR, tc.subNetMaskSize, nil)
446457
rangeAllocator, ok := allocator.(*rangeAllocator)
447458
if !ok {
448459
t.Logf("%v: found non-default implementation of CIDRAllocator, skipping white-box test...", tc.description)

‎pkg/controller/nodeipam/node_ipam_controller.go‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,11 @@ const (
5353
type Controller struct {
5454
allocatorType ipam.CIDRAllocatorType
5555

56-
cloud cloudprovider.Interface
57-
clusterCIDRs []*net.IPNet
58-
serviceCIDR *net.IPNet
59-
kubeClient clientset.Interface
56+
cloud cloudprovider.Interface
57+
clusterCIDRs []*net.IPNet
58+
serviceCIDR *net.IPNet
59+
secondaryServiceCIDR *net.IPNet
60+
kubeClient clientset.Interface
6061
// Method for easy mocking in unittest.
6162
lookupIP func(host string) ([]net.IP, error)
6263

@@ -79,6 +80,7 @@ func NewNodeIpamController(
7980
kubeClient clientset.Interface,
8081
clusterCIDRs []*net.IPNet,
8182
serviceCIDR *net.IPNet,
83+
secondaryServiceCIDR *net.IPNet,
8284
nodeCIDRMaskSize int,
8385
allocatorType ipam.CIDRAllocatorType) (*Controller, error) {
8486

@@ -119,20 +121,21 @@ func NewNodeIpamController(
119121
}
120122

121123
ic := &Controller{
122-
cloud: cloud,
123-
kubeClient: kubeClient,
124-
lookupIP: net.LookupIP,
125-
clusterCIDRs: clusterCIDRs,
126-
serviceCIDR: serviceCIDR,
127-
allocatorType: allocatorType,
124+
cloud: cloud,
125+
kubeClient: kubeClient,
126+
lookupIP: net.LookupIP,
127+
clusterCIDRs: clusterCIDRs,
128+
serviceCIDR: serviceCIDR,
129+
secondaryServiceCIDR: secondaryServiceCIDR,
130+
allocatorType: allocatorType,
128131
}
129132

130133
// TODO: Abstract this check into a generic controller manager should run method.
131134
if ic.allocatorType == ipam.IPAMFromClusterAllocatorType || ic.allocatorType == ipam.IPAMFromCloudAllocatorType {
132135
startLegacyIPAM(ic, nodeInformer, cloud, kubeClient, clusterCIDRs, serviceCIDR, nodeCIDRMaskSize)
133136
} else {
134137
var err error
135-
ic.cidrAllocator, err = ipam.New(kubeClient, cloud, nodeInformer, ic.allocatorType, clusterCIDRs, ic.serviceCIDR, nodeCIDRMaskSize)
138+
ic.cidrAllocator, err = ipam.New(kubeClient, cloud, nodeInformer, ic.allocatorType, clusterCIDRs, ic.serviceCIDR, ic.secondaryServiceCIDR, nodeCIDRMaskSize)
136139
if err != nil {
137140
return nil, err
138141
}

0 commit comments

Comments
 (0)