From b5c2c07f7936a084cb5ef50437114d12b294713a Mon Sep 17 00:00:00 2001 From: Serge Logvinov Date: Sun, 13 Oct 2024 22:41:45 +0300 Subject: [PATCH] fix: node allocator If a node has a large subnet, such as a /56 or larger, we need to allocate a /64 subnet for each individual node. Signed-off-by: Serge Logvinov --- .github/workflows/build-edge.yaml | 2 +- .github/workflows/release-charts.yaml | 2 +- .github/workflows/release.yaml | 2 +- .golangci.yml | 6 +- pkg/nodeipam/ipam/cloud_allocator.go | 53 +++++++++---- pkg/nodeipam/ipam/cloud_allocator_test.go | 97 +++++++++++++++++++++++ pkg/talos/helper.go | 5 +- pkg/talos/instances.go | 2 +- pkg/talosclient/client.go | 4 +- pkg/utils/net/net.go | 4 +- pkg/utils/net/net_test.go | 4 +- 11 files changed, 149 insertions(+), 32 deletions(-) create mode 100644 pkg/nodeipam/ipam/cloud_allocator_test.go diff --git a/.github/workflows/build-edge.yaml b/.github/workflows/build-edge.yaml index cd1f121..ee77118 100644 --- a/.github/workflows/build-edge.yaml +++ b/.github/workflows/build-edge.yaml @@ -26,7 +26,7 @@ jobs: run: git fetch --prune --unshallow - name: Install Cosign - uses: sigstore/cosign-installer@v3.6.0 + uses: sigstore/cosign-installer@v3.7.0 - name: Set up QEMU uses: docker/setup-qemu-action@v3 with: diff --git a/.github/workflows/release-charts.yaml b/.github/workflows/release-charts.yaml index e6d9cbd..e77abbf 100644 --- a/.github/workflows/release-charts.yaml +++ b/.github/workflows/release-charts.yaml @@ -25,7 +25,7 @@ jobs: - name: Install Helm uses: azure/setup-helm@v4 - name: Install Cosign - uses: sigstore/cosign-installer@v3.6.0 + uses: sigstore/cosign-installer@v3.7.0 - name: Github registry login uses: docker/login-action@v3 diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index dcf89ee..2f760f4 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -21,7 +21,7 @@ jobs: run: git fetch --prune --unshallow - name: Install Cosign - uses: sigstore/cosign-installer@v3.6.0 + uses: sigstore/cosign-installer@v3.7.0 - name: Set up QEMU uses: docker/setup-qemu-action@v3 with: diff --git a/.golangci.yml b/.golangci.yml index 36c029a..290562e 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -57,8 +57,7 @@ linters-settings: # simplify code: gofmt with `-s` option, true by default simplify: true gocyclo: - # minimal code complexity to report, 30 by default (but we recommend 10-20) - min-complexity: 20 + min-complexity: 25 maligned: # print struct with more effective memory layout or not, false by default suggest-new: true @@ -125,8 +124,7 @@ linters-settings: - prefix(github.com/siderolabs) # Groups all imports with the specified Prefix. - prefix(k8s.io) # Groups all imports with the specified Prefix. cyclop: - # the maximal code complexity to report - max-complexity: 20 + max-complexity: 25 gomoddirectives: replace-local: true replace-allow-list: diff --git a/pkg/nodeipam/ipam/cloud_allocator.go b/pkg/nodeipam/ipam/cloud_allocator.go index 9576eb1..94b374c 100644 --- a/pkg/nodeipam/ipam/cloud_allocator.go +++ b/pkg/nodeipam/ipam/cloud_allocator.go @@ -334,7 +334,7 @@ func (r *cloudAllocator) occupyPodCIDRs(ctx context.Context, node *v1.Node) erro if !ok { _, err := r.defineNodeGlobalCIDRs(ctx, node) if err != nil { - return fmt.Errorf("failed to find a CIDRSet for node %s, CIDR %s", node.Name, cidr) + return fmt.Errorf("failed to find a CIDRSet for node %s, CIDR %s: %v", node.Name, cidr, err) } } } @@ -527,6 +527,7 @@ func (r *cloudAllocator) updateCIDRsAllocation(ctx context.Context, nodeName str return err } +// defineNodeGlobalCIDRs returns the global CIDR for the node. func (r *cloudAllocator) defineNodeGlobalCIDRs(ctx context.Context, node *v1.Node) (string, error) { if node == nil { return "", fmt.Errorf("node is nil") @@ -566,14 +567,14 @@ func (r *cloudAllocator) defineNodeGlobalCIDRs(ctx context.Context, node *v1.Nod } } - _, cidr := talosclient.NodeCIDRDiscovery(ipv6, ifaces) - logger.V(4).Info("Node has IPv6 CIDRs", "node", klog.KObj(node), "CIDRs", cidr) + _, cidrs := talosclient.NodeCIDRDiscovery(ipv6, ifaces) + logger.V(4).Info("Node has IPv6 CIDRs", "node", klog.KObj(node), "CIDRs", cidrs) - if len(cidr) > 0 { + if len(cidrs) > 0 { r.lock.Lock() defer r.lock.Unlock() - subnets, err := netutils.ParseCIDRs(cidr) + subnets, err := netutils.ParseCIDRs(cidrs) if err != nil { return "", err } @@ -590,42 +591,60 @@ func (r *cloudAllocator) defineNodeGlobalCIDRs(ctx context.Context, node *v1.Nod } } - for _, subnet := range subnets { - logger.V(4).Info("Add IPv6 to CIDRSet", "node", klog.KObj(node), "CIDR", subnet) + for _, cidr := range cidrs { + mask := netip.MustParsePrefix(cidr).Bits() + if mask == 128 { + continue + } + + logger.V(4).Info("Add IPv6 to CIDRSet", "node", klog.KObj(node), "CIDR", cidr) - err := r.addCIDRSet(subnet) + err := r.addCIDRSet(cidr) if err != nil { - return "", err + return "", fmt.Errorf("error to add CIDRv6 to CIDRSet: %v", err) } } - return cidr[0], nil + return cidrs[0], nil } return "", nil } -func (r *cloudAllocator) addCIDRSet(subnet *net.IPNet) error { - mask, _ := subnet.Mask.Size() +// addCIDRSet adds a new CIDRSet-v6 to the allocator's tracked CIDR sets. +func (r *cloudAllocator) addCIDRSet(cidr string) error { + subnet, err := netip.ParsePrefix(cidr) + if err != nil { + return err + } + + mask := subnet.Bits() switch { case mask < 64: - mask = 64 + subnet, err = subnet.Addr().Prefix(64) + if err != nil { + return err + } + + mask = 80 case mask > 120: return fmt.Errorf("CIDRv6 is too small: %v", subnet.String()) default: mask += 16 } - cidrSet, err := cidrset.NewCIDRSet(subnet, mask) + ip := subnet.Masked().Addr() + net := &net.IPNet{IP: net.ParseIP(ip.String()), Mask: net.CIDRMask(subnet.Bits(), 128)} + + cidrSet, err := cidrset.NewCIDRSet(net, mask) if err != nil { return err } - k := netip.MustParsePrefix(subnet.String()) - + k := subnet.Masked() if _, ok := r.cidrSets[k]; !ok { - r.cidrSets[netip.MustParsePrefix(subnet.String())] = cidrSet + r.cidrSets[k] = cidrSet } return nil diff --git a/pkg/nodeipam/ipam/cloud_allocator_test.go b/pkg/nodeipam/ipam/cloud_allocator_test.go new file mode 100644 index 0000000..49ec073 --- /dev/null +++ b/pkg/nodeipam/ipam/cloud_allocator_test.go @@ -0,0 +1,97 @@ +/* +Copyright 2016 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package ipam + +import ( + "fmt" + "net/netip" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/siderolabs/talos-cloud-controller-manager/pkg/nodeipam/ipam/cidrset" +) + +func TestAddCIDRSet(t *testing.T) { + for _, tt := range []struct { + name string + cidr string + expectedError error + expectedSize int + expectedClusterCIDR netip.Prefix + }{ + { + name: "CIDRv6 with mask size 56", + cidr: "2000::1111:aaaa:bbbb:cccc:123/56", + expectedSize: 1, + expectedClusterCIDR: netip.MustParsePrefix("2000:0:0:1111::/64"), + }, + { + name: "CIDRv6 with mask size 64", + cidr: "2000::aaaa:bbbb:cccc:123/64", + expectedSize: 1, + expectedClusterCIDR: netip.MustParsePrefix("2000::/64"), + }, + { + name: "CIDRv6 with mask size 80", + cidr: "2000::aaaa:bbbb:cccc:123/80", + expectedSize: 1, + expectedClusterCIDR: netip.MustParsePrefix("2000::aaaa:0:0:0/80"), + }, + { + name: "CIDRv6 with mask size 96", + cidr: "2000::aaaa:bbbb:cccc:123/96", + expectedSize: 1, + expectedClusterCIDR: netip.MustParsePrefix("2000::aaaa:bbbb:0:0/96"), + }, + { + name: "CIDRv6 with mask size 112", + cidr: "2000::aaaa:bbbb:cccc:123/112", + expectedSize: 1, + expectedClusterCIDR: netip.MustParsePrefix("2000::aaaa:bbbb:cccc:0/112"), + }, + { + name: "CIDRv6 with mask size 120", + cidr: "2000::aaaa:bbbb:cccc:123/120", + expectedSize: 1, + expectedClusterCIDR: netip.MustParsePrefix("2000::aaaa:bbbb:cccc:100/120"), + }, + { + name: "CIDRv6 with mask size 124", + cidr: "2000::aaaa:bbbb:cccc:123/124", + expectedError: fmt.Errorf("CIDRv6 is too small: 2000::aaaa:bbbb:cccc:123/124"), + }, + } { + t.Run(tt.name, func(t *testing.T) { + cidrSets := make(map[netip.Prefix]*cidrset.CidrSet, 0) + allocator := cloudAllocator{ + cidrSets: cidrSets, + } + err := allocator.addCIDRSet(tt.cidr) + + if tt.expectedError != nil { + assert.Error(t, err) + assert.Equal(t, tt.expectedError, err) + } else { + assert.NoError(t, err) + assert.Len(t, cidrSets, tt.expectedSize) + assert.Contains(t, cidrSets, tt.expectedClusterCIDR, "CIDRSet not found") + assert.NotNil(t, cidrSets[tt.expectedClusterCIDR], "CIDRSet not found") + } + }) + } +} diff --git a/pkg/talos/helper.go b/pkg/talos/helper.go index 95e0e43..f361a56 100644 --- a/pkg/talos/helper.go +++ b/pkg/talos/helper.go @@ -30,6 +30,7 @@ func ipDiscovery(nodeIPs []string, ifaces []network.AddressStatusSpec) (publicIP if iface.LinkName == constants.KubeSpanLinkName || iface.LinkName == constants.SideroLinkName || iface.LinkName == "lo" || + iface.LinkName == "cilium_host" || strings.HasPrefix(iface.LinkName, "dummy") { continue } @@ -88,11 +89,11 @@ func getNodeAddresses(config *cloudConfig, platform string, features *transforme } addresses := []v1.NodeAddress{} - for _, ip := range utilsnet.PreferedDualStackNodeIPs(config.Global.PreferIPv6, nodeIPs) { + for _, ip := range utilsnet.PreferredDualStackNodeIPs(config.Global.PreferIPv6, nodeIPs) { addresses = append(addresses, v1.NodeAddress{Type: v1.NodeInternalIP, Address: ip}) } - publicIPs = utilsnet.PreferedDualStackNodeIPs(config.Global.PreferIPv6, append(publicIPv4s, publicIPv6s...)) + publicIPs = utilsnet.PreferredDualStackNodeIPs(config.Global.PreferIPv6, append(publicIPv4s, publicIPv6s...)) for _, ip := range publicIPs { addresses = append(addresses, v1.NodeAddress{Type: v1.NodeExternalIP, Address: ip}) } diff --git a/pkg/talos/instances.go b/pkg/talos/instances.go index d369eef..72e2cd9 100644 --- a/pkg/talos/instances.go +++ b/pkg/talos/instances.go @@ -98,7 +98,7 @@ func (i *instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloud klog.V(4).InfoS("instances.InstanceMetadata() called", "node", klog.KRef("", node.Name)) if providedIP, ok := node.ObjectMeta.Annotations[cloudproviderapi.AnnotationAlphaProvidedIPAddr]; ok { - nodeIPs := net.PreferedDualStackNodeIPs(i.c.config.Global.PreferIPv6, strings.Split(providedIP, ",")) + nodeIPs := net.PreferredDualStackNodeIPs(i.c.config.Global.PreferIPv6, strings.Split(providedIP, ",")) var ( meta *runtime.PlatformMetadataSpec diff --git a/pkg/talosclient/client.go b/pkg/talosclient/client.go index 4eb864d..ce8bb09 100644 --- a/pkg/talosclient/client.go +++ b/pkg/talosclient/client.go @@ -171,6 +171,7 @@ func NodeIPDiscovery(nodeIPs []string, ifaces []network.AddressStatusSpec) (publ if iface.LinkName == constants.KubeSpanLinkName || iface.LinkName == constants.SideroLinkName || iface.LinkName == "lo" || + iface.LinkName == "cilium_host" || strings.HasPrefix(iface.LinkName, "dummy") { continue } @@ -203,6 +204,7 @@ func NodeCIDRDiscovery(filterIPs []netip.Addr, ifaces []network.AddressStatusSpe if iface.LinkName == constants.KubeSpanLinkName || iface.LinkName == constants.SideroLinkName || iface.LinkName == "lo" || + iface.LinkName == "cilium_host" || strings.HasPrefix(iface.LinkName, "dummy") { continue } @@ -210,7 +212,7 @@ func NodeCIDRDiscovery(filterIPs []netip.Addr, ifaces []network.AddressStatusSpe ip := iface.Address.Addr() if ip.IsGlobalUnicast() && !ip.IsPrivate() { if len(filterIPs) == 0 || slices.Contains(filterIPs, ip) { - cidr := iface.Address.Masked().String() + cidr := iface.Address.String() if ip.Is6() { if slices.Contains(publicCIDRv6s, cidr) { diff --git a/pkg/utils/net/net.go b/pkg/utils/net/net.go index 3426cf3..c62702c 100644 --- a/pkg/utils/net/net.go +++ b/pkg/utils/net/net.go @@ -39,8 +39,8 @@ func SortedNodeIPs(nodeIP string, first, second []string) (res []string) { return res } -// PreferedDualStackNodeIPs returns the first IPv4 and IPv6 addresses from the list of IPs. -func PreferedDualStackNodeIPs(preferIPv6 bool, ips []string) []string { +// PreferredDualStackNodeIPs returns the first IPv4 and IPv6 addresses from the list of IPs. +func PreferredDualStackNodeIPs(preferIPv6 bool, ips []string) []string { var ipv6, ipv4 string for _, ip := range ips { diff --git a/pkg/utils/net/net_test.go b/pkg/utils/net/net_test.go index e2bb92f..b21688d 100644 --- a/pkg/utils/net/net_test.go +++ b/pkg/utils/net/net_test.go @@ -107,7 +107,7 @@ func TestSortedNodeIPs(t *testing.T) { } } -func TestPreferedDualStackNodeIPs(t *testing.T) { +func TestPreferredDualStackNodeIPs(t *testing.T) { t.Parallel() for _, tt := range []struct { //nolint:govet @@ -146,7 +146,7 @@ func TestPreferedDualStackNodeIPs(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - result := utilnet.PreferedDualStackNodeIPs(tt.preferIPv6, tt.nodeIPs) + result := utilnet.PreferredDualStackNodeIPs(tt.preferIPv6, tt.nodeIPs) assert.Equal(t, fmt.Sprintf("%v", tt.expected), fmt.Sprintf("%v", result)) })