Merge pull request #15814 from hakman/number_of_subnets

Split network CIDR into even bigger subnets

pkg/util/subnet/subnet.go

@@ -45,6 +45,11 @@ func BelongsTo(parent *net.IPNet, child *net.IPNet) bool {
 	return childMasked.Equal(parentMasked)
 }
 
+// SplitInto1 splits the parent IPNet into 1 subnet
+func SplitInto1(parent *net.IPNet) ([]*net.IPNet, error) {
+	return SplitInto(0, parent)
+}
+
 // SplitInto2 splits the parent IPNet into 2 subnets
 func SplitInto2(parent *net.IPNet) ([]*net.IPNet, error) {
 	return SplitInto(1, parent)
@@ -61,13 +66,9 @@ func SplitInto8(parent *net.IPNet) ([]*net.IPNet, error) {
 }
 
 // SplitInto splits the parent IPNet into subnets with the specified number of additional bits in the prefix.
-func SplitInto(additionalBits int, parent *net.IPNet) ([]*net.IPNet, error) {
-	if additionalBits < 1 || additionalBits > 3 {
-		return nil, fmt.Errorf("additionalBits value must be between 1 and 3, not %d", additionalBits)
-	}
-
+func SplitInto(additionalBits uint, parent *net.IPNet) ([]*net.IPNet, error) {
 	networkLength, _ := parent.Mask.Size()
-	networkLength += additionalBits
+	networkLength += int(additionalBits)
 
 	var subnets []*net.IPNet
 	for i := 0; i < 1<<additionalBits; i++ {

tests/integration/create_cluster/cilium-eni/expected-v1alpha2.yaml

@@ -52,7 +52,7 @@ spec:
   - 0.0.0.0/0
   - ::/0
   subnets:
-  - cidr: 172.20.128.0/17
+  - cidr: 172.20.0.0/16
     name: us-test-1a
     type: Public
     zone: us-test-1a

tests/integration/create_cluster/complex/expected-v1alpha2.yaml

@@ -47,7 +47,7 @@ spec:
   sshAccess:
   - 1.2.3.4/32
   subnets:
-  - cidr: 172.20.128.0/17
+  - cidr: 172.20.0.0/16
     name: us-test-1a
     type: Public
     zone: us-test-1a

tests/integration/create_cluster/ha/expected-v1alpha2.yaml

@@ -60,15 +60,15 @@ spec:
   - 0.0.0.0/0
   - ::/0
   subnets:
-  - cidr: 172.20.64.0/18
+  - cidr: 172.20.0.0/18
     name: us-test-1a
     type: Public
     zone: us-test-1a
-  - cidr: 172.20.128.0/18
+  - cidr: 172.20.64.0/18
     name: us-test-1b
     type: Public
     zone: us-test-1b
-  - cidr: 172.20.192.0/18
+  - cidr: 172.20.128.0/18
     name: us-test-1c
     type: Public
     zone: us-test-1c

tests/integration/create_cluster/ha_encrypt/expected-v1alpha2.yaml

@@ -60,15 +60,15 @@ spec:
   - 0.0.0.0/0
   - ::/0
   subnets:
-  - cidr: 172.20.64.0/18
+  - cidr: 172.20.0.0/18
     name: us-test-1a
     type: Public
     zone: us-test-1a
-  - cidr: 172.20.128.0/18
+  - cidr: 172.20.64.0/18
     name: us-test-1b
     type: Public
     zone: us-test-1b
-  - cidr: 172.20.192.0/18
+  - cidr: 172.20.128.0/18
     name: us-test-1c
     type: Public
     zone: us-test-1c

tests/integration/create_cluster/ha_openstack/expected-v1alpha2.yaml

@@ -68,7 +68,7 @@ spec:
   - 0.0.0.0/0
   - ::/0
   subnets:
-  - cidr: 10.0.128.0/17
+  - cidr: 10.0.0.0/16
     name: us-test1
     type: Public
     zone: us-test1

tests/integration/create_cluster/ha_shared_zone/expected-v1alpha2.yaml

@@ -60,7 +60,7 @@ spec:
   - 0.0.0.0/0
   - ::/0
   subnets:
-  - cidr: 172.20.128.0/17
+  - cidr: 172.20.0.0/16
     name: us-test-1a
     type: Public
     zone: us-test-1a

tests/integration/create_cluster/ha_shared_zones/expected-v1alpha2.yaml

@@ -72,11 +72,11 @@ spec:
   - 0.0.0.0/0
   - ::/0
   subnets:
-  - cidr: 172.20.64.0/18
+  - cidr: 172.20.0.0/17
     name: us-test-1a
     type: Public
     zone: us-test-1a
-  - cidr: 172.20.128.0/18
+  - cidr: 172.20.128.0/17
     name: us-test-1b
     type: Public
     zone: us-test-1b

tests/integration/create_cluster/karpenter/expected-v1alpha2.yaml

@@ -54,15 +54,15 @@ spec:
   - 0.0.0.0/0
   - ::/0
   subnets:
-  - cidr: 172.20.64.0/18
+  - cidr: 172.20.0.0/18
     name: us-test-1a
     type: Public
     zone: us-test-1a
-  - cidr: 172.20.128.0/18
+  - cidr: 172.20.64.0/18
     name: us-test-1b
     type: Public
     zone: us-test-1b
-  - cidr: 172.20.192.0/18
+  - cidr: 172.20.128.0/18
     name: us-test-1c
     type: Public
     zone: us-test-1c

tests/integration/create_cluster/minimal-1.23/expected-v1alpha2.yaml

@@ -48,7 +48,7 @@ spec:
   - 0.0.0.0/0
   - ::/0
   subnets:
-  - cidr: 172.20.128.0/17
+  - cidr: 172.20.0.0/16
     name: us-test-1a
     type: Public
     zone: us-test-1a

tests/integration/create_cluster/minimal-1.24/expected-v1alpha2.yaml

@@ -48,7 +48,7 @@ spec:
   - 0.0.0.0/0
   - ::/0
   subnets:
-  - cidr: 172.20.128.0/17
+  - cidr: 172.20.0.0/16
     name: us-test-1a
     type: Public
     zone: us-test-1a

tests/integration/create_cluster/minimal-1.25/expected-v1alpha2.yaml

@@ -48,7 +48,7 @@ spec:
   - 0.0.0.0/0
   - ::/0
   subnets:
-  - cidr: 172.20.128.0/17
+  - cidr: 172.20.0.0/16
     name: us-test-1a
     type: Public
     zone: us-test-1a

tests/integration/create_cluster/minimal-1.26-arm64/expected-v1alpha2.yaml

@@ -48,7 +48,7 @@ spec:
   - 0.0.0.0/0
   - ::/0
   subnets:
-  - cidr: 172.20.128.0/17
+  - cidr: 172.20.0.0/16
     name: us-test-1a
     type: Public
     zone: us-test-1a

tests/integration/create_cluster/minimal-1.26-irsa/expected-v1alpha2.yaml

@@ -52,7 +52,7 @@ spec:
   - 0.0.0.0/0
   - ::/0
   subnets:
-  - cidr: 172.20.128.0/17
+  - cidr: 172.20.0.0/16
     name: us-test-1a
     type: Public
     zone: us-test-1a

tests/integration/create_cluster/minimal-1.26/expected-v1alpha2.yaml

@@ -48,7 +48,7 @@ spec:
   - 0.0.0.0/0
   - ::/0
   subnets:
-  - cidr: 172.20.128.0/17
+  - cidr: 172.20.0.0/16
     name: us-test-1a
     type: Public
     zone: us-test-1a

tests/integration/create_cluster/minimal-1.27/expected-v1alpha2.yaml

@@ -48,7 +48,7 @@ spec:
   - 0.0.0.0/0
   - ::/0
   subnets:
-  - cidr: 172.20.128.0/17
+  - cidr: 172.20.0.0/16
     name: us-test-1a
     type: Public
     zone: us-test-1a

tests/integration/create_cluster/minimal_feature-gates/expected-v1alpha2.yaml

@@ -72,7 +72,7 @@ spec:
   - 0.0.0.0/0
   - ::/0
   subnets:
-  - cidr: 172.20.128.0/17
+  - cidr: 172.20.0.0/16
     name: us-test-1a
     type: Public
     zone: us-test-1a

tests/integration/create_cluster/overrides/expected-v1alpha2.yaml

@@ -51,7 +51,7 @@ spec:
   - 0.0.0.0/0
   - ::/0
   subnets:
-  - cidr: 172.20.128.0/17
+  - cidr: 172.20.0.0/16
     name: us-test-1a
     type: Public
     zone: us-test-1a

tests/integration/create_cluster/shared_vpc/expected-v1alpha2.yaml

@@ -49,7 +49,7 @@ spec:
   - 0.0.0.0/0
   - ::/0
   subnets:
-  - cidr: 10.8.0.0/13
+  - cidr: 10.0.0.0/12
     name: us-test-1a
     type: Public
     zone: us-test-1a

upup/pkg/fi/cloudup/subnets.go

@@ -131,10 +131,21 @@ func assignCIDRsToSubnets(c *kops.Cluster, cloud fi.Cloud) error {
 		}
 	}
 
+	// Assign a consistent order
+	sort.Sort(ByZone(bigSubnets))
+	sort.Sort(ByZone(littleSubnets))
+
+	// Check how many subnet slices are needed
+	cidrCount := len(bigSubnets)
+	if len(littleSubnets) > 0 {
+		cidrCount += 1
+	}
 	var bigCIDRs []*net.IPNet
-	if len(bigSubnets)+1 <= 2 {
+	if cidrCount <= 1 {
+		bigCIDRs, err = subnet.SplitInto1(cidr)
+	} else if cidrCount <= 2 {
 		bigCIDRs, err = subnet.SplitInto2(cidr)
-	} else if len(bigSubnets)+1 <= 4 {
+	} else if cidrCount <= 4 {
 		bigCIDRs, err = subnet.SplitInto4(cidr)
 	} else {
 		bigCIDRs, err = subnet.SplitInto8(cidr)
@@ -164,16 +175,30 @@ func assignCIDRsToSubnets(c *kops.Cluster, cloud fi.Cloud) error {
 		return fmt.Errorf("could not find any non-overlapping CIDRs in parent NetworkCIDR; cannot automatically assign CIDR to subnet")
 	}
 
-	littleCIDRs, err := subnet.SplitInto8(bigCIDRs[0])
-	if err != nil {
-		return err
+	// Assign CIDRs to little subnets
+	if len(littleSubnets) > 0 {
+		littleCIDRs, err := subnet.SplitInto8(bigCIDRs[0])
+		if err != nil {
+			return err
+		}
+		bigCIDRs = bigCIDRs[1:]
+
+		for _, subnet := range littleSubnets {
+			if subnet.CIDR != "" {
+				continue
+			}
+
+			if len(littleCIDRs) == 0 {
+				return fmt.Errorf("insufficient (little) CIDRs remaining for automatic CIDR allocation to subnet %q", subnet.Name)
+			}
+			subnet.CIDR = littleCIDRs[0].String()
+			klog.Infof("Assigned CIDR %s to subnet %s", subnet.CIDR, subnet.Name)
+
+			littleCIDRs = littleCIDRs[1:]
+		}
 	}
-	bigCIDRs = bigCIDRs[1:]
-
-	// Assign a consistent order
-	sort.Sort(ByZone(bigSubnets))
-	sort.Sort(ByZone(littleSubnets))
 
+	// Assign CIDRs to big subnets
 	for _, subnet := range bigSubnets {
 		if subnet.CIDR != "" {
 			continue
@@ -191,20 +216,6 @@ func assignCIDRsToSubnets(c *kops.Cluster, cloud fi.Cloud) error {
 		bigCIDRs = bigCIDRs[1:]
 	}
 
-	for _, subnet := range littleSubnets {
-		if subnet.CIDR != "" {
-			continue
-		}
-
-		if len(littleCIDRs) == 0 {
-			return fmt.Errorf("insufficient (little) CIDRs remaining for automatic CIDR allocation to subnet %q", subnet.Name)
-		}
-		subnet.CIDR = littleCIDRs[0].String()
-		klog.Infof("Assigned CIDR %s to subnet %s", subnet.CIDR, subnet.Name)
-
-		littleCIDRs = littleCIDRs[1:]
-	}
-
 	return nil
 }
 

upup/pkg/fi/cloudup/subnets_test.go

@@ -39,35 +39,35 @@ func Test_AssignSubnets(t *testing.T) {
 			subnets: []kops.ClusterSubnetSpec{
 				{Name: "a", Zone: "a", CIDR: "", Type: kops.SubnetTypePublic},
 			},
-			expected: []string{"10.128.0.0/9"},
+			expected: []string{"10.0.0.0/8"},
 		},
 		{
 			subnets: []kops.ClusterSubnetSpec{
 				{Name: "a", Zone: "a", CIDR: "", Type: kops.SubnetTypePublic},
 				{Name: "b", Zone: "b", CIDR: "", Type: kops.SubnetTypePublic},
 			},
-			expected: []string{"10.64.0.0/10", "10.128.0.0/10"},
+			expected: []string{"10.0.0.0/9", "10.128.0.0/9"},
 		},
 		{
 			subnets: []kops.ClusterSubnetSpec{
 				{Name: "a", Zone: "b", CIDR: "", Type: kops.SubnetTypePublic},
 				{Name: "b", Zone: "a", CIDR: "", Type: kops.SubnetTypePublic},
 			},
-			expected: []string{"10.128.0.0/10", "10.64.0.0/10"},
+			expected: []string{"10.128.0.0/9", "10.0.0.0/9"},
 		},
 		{
 			subnets: []kops.ClusterSubnetSpec{
 				{Name: "a", Zone: "a", CIDR: "10.64.0.0/11", Type: kops.SubnetTypePublic},
 				{Name: "b", Zone: "b", CIDR: "", Type: kops.SubnetTypePublic},
 			},
-			expected: []string{"10.64.0.0/11", "10.128.0.0/10"},
+			expected: []string{"10.64.0.0/11", "10.128.0.0/9"},
 		},
 		{
 			subnets: []kops.ClusterSubnetSpec{
 				{Name: "a", Zone: "a", CIDR: "10.0.0.0/9", Type: kops.SubnetTypePublic},
 				{Name: "b", Zone: "b", CIDR: "", Type: kops.SubnetTypePublic},
 			},
-			expected: []string{"10.0.0.0/9", "10.192.0.0/10"},
+			expected: []string{"10.0.0.0/9", "10.128.0.0/9"},
 		},
 
 		{
@@ -94,7 +94,7 @@ func Test_AssignSubnets(t *testing.T) {
 			subnets: []kops.ClusterSubnetSpec{
 				{Name: "a", Zone: "a", CIDR: "", Type: kops.SubnetTypePrivate},
 			},
-			expected: []string{"10.128.0.0/9"},
+			expected: []string{"10.0.0.0/8"},
 		},
 		{
 			subnets: []kops.ClusterSubnetSpec{
@@ -106,7 +106,7 @@ func Test_AssignSubnets(t *testing.T) {
 			subnets: []kops.ClusterSubnetSpec{
 				{Name: "a", Zone: "a", CIDR: "", IPv6CIDR: "/64#0", Type: kops.SubnetTypeDualStack},
 			},
-			expected: []string{"10.128.0.0/9"},
+			expected: []string{"10.0.0.0/8"},
 		},
 		{
 			subnets: []kops.ClusterSubnetSpec{