From 68d41338b180d8ac0c9575f77c6b4951ceb3bd87 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 +- go.mod | 6 +- go.sum | 12 +-- 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 +- 13 files changed, 158 insertions(+), 41 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/go.mod b/go.mod index b238385..9e5e9f0 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/cosi-project/runtime v0.6.4 github.com/siderolabs/go-retry v0.3.3 github.com/siderolabs/net v0.4.0 - github.com/siderolabs/talos/pkg/machinery v1.8.0 + github.com/siderolabs/talos/pkg/machinery v1.8.1 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.9.0 gopkg.in/yaml.v3 v3.0.1 @@ -24,7 +24,7 @@ require ( require ( github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 // indirect github.com/NYTimes/gziphandler v1.1.1 // indirect - github.com/ProtonMail/go-crypto v1.1.0-alpha.5.0.20240827111422-b5837fa4476e // indirect + github.com/ProtonMail/go-crypto v1.1.0-beta.0-proton // indirect github.com/ProtonMail/go-mime v0.0.0-20230322103455-7d82a3887f2f // indirect github.com/ProtonMail/gopenpgp/v2 v2.7.5 // indirect github.com/adrg/xdg v0.5.0 // indirect @@ -125,7 +125,7 @@ require ( golang.org/x/time v0.6.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20240903143218-8af14fe29dc1 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20240903143218-8af14fe29dc1 // indirect - google.golang.org/grpc v1.66.2 // indirect + google.golang.org/grpc v1.66.3 // indirect google.golang.org/protobuf v1.34.2 // indirect gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect diff --git a/go.sum b/go.sum index e5b372b..a6f75f8 100644 --- a/go.sum +++ b/go.sum @@ -3,8 +3,8 @@ github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161/go.mod h1:xomTg6 github.com/NYTimes/gziphandler v1.1.1 h1:ZUDjpQae29j0ryrS0u/B8HZfJBtBQHjqw2rQ2cqUQ3I= github.com/NYTimes/gziphandler v1.1.1/go.mod h1:n/CVRwUEOgIxrgPvAQhUUr9oeUtvrhMomdKFjzJNB0c= github.com/ProtonMail/go-crypto v0.0.0-20230717121422-5aa5874ade95/go.mod h1:EjAoLdwvbIOoOQr3ihjnSoLZRtE8azugULFRteWMNc0= -github.com/ProtonMail/go-crypto v1.1.0-alpha.5.0.20240827111422-b5837fa4476e h1:O1cSHAcGcbGEO66Qi2AIJeYmXO8iP4L/PNrbdN+RjJA= -github.com/ProtonMail/go-crypto v1.1.0-alpha.5.0.20240827111422-b5837fa4476e/go.mod h1:rA3QumHc/FZ8pAHreoekgiAbzpNsfQAosU5td4SnOrE= +github.com/ProtonMail/go-crypto v1.1.0-beta.0-proton h1:ZGewsAoeSirbUS5cO8L0FMQA+iSop9xR1nmFYifDBPo= +github.com/ProtonMail/go-crypto v1.1.0-beta.0-proton/go.mod h1:rA3QumHc/FZ8pAHreoekgiAbzpNsfQAosU5td4SnOrE= github.com/ProtonMail/go-mime v0.0.0-20230322103455-7d82a3887f2f h1:tCbYj7/299ekTTXpdwKYF8eBlsYsDVoggDAuAjoK66k= github.com/ProtonMail/go-mime v0.0.0-20230322103455-7d82a3887f2f/go.mod h1:gcr0kNtGBqin9zDW9GOHcVntrwnjrK+qdJ06mWYBybw= github.com/ProtonMail/gopenpgp/v2 v2.7.5 h1:STOY3vgES59gNgoOt2w0nyHBjKViB/qSg7NjbQWPJkA= @@ -208,8 +208,8 @@ github.com/siderolabs/net v0.4.0 h1:1bOgVay/ijPkJz4qct98nHsiB/ysLQU0KLoBC4qLm7I= github.com/siderolabs/net v0.4.0/go.mod h1:/ibG+Hm9HU27agp5r9Q3eZicEfjquzNzQNux5uEk0kM= github.com/siderolabs/protoenc v0.2.1 h1:BqxEmeWQeMpNP3R6WrPqDatX8sM/r4t97OP8mFmg6GA= github.com/siderolabs/protoenc v0.2.1/go.mod h1:StTHxjet1g11GpNAWiATgc8K0HMKiFSEVVFOa/H0otc= -github.com/siderolabs/talos/pkg/machinery v1.8.0 h1:azhBj+Nm9oTgaFgcNaHU8TPS9Oi5OdV1ELNgFAVder8= -github.com/siderolabs/talos/pkg/machinery v1.8.0/go.mod h1:kkrPF7yyhNSg8yJx4RJnk21Hi3APAHm3HQm2M3zfwKI= +github.com/siderolabs/talos/pkg/machinery v1.8.1 h1:oeJQmkLNjEG5jxrzPiC2XMQS5dcg1qZ17p5LKcaCbRM= +github.com/siderolabs/talos/pkg/machinery v1.8.1/go.mod h1:mWTmuUk8G6CdkhUfDmsrIkgPo0G6J5hC/zGazgnyzBg= github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ= github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= github.com/soheilhy/cmux v0.1.5 h1:jjzc5WVemNEDTLwv9tlmemhC73tI08BNOIGwBOo10Js= @@ -367,8 +367,8 @@ google.golang.org/genproto/googleapis/api v0.0.0-20240903143218-8af14fe29dc1 h1: google.golang.org/genproto/googleapis/api v0.0.0-20240903143218-8af14fe29dc1/go.mod h1:qpvKtACPCQhAdu3PyQgV4l3LMXZEtft7y8QcarRsp9I= google.golang.org/genproto/googleapis/rpc v0.0.0-20240903143218-8af14fe29dc1 h1:pPJltXNxVzT4pK9yD8vR9X75DaWYYmLGMsEvBfFQZzQ= google.golang.org/genproto/googleapis/rpc v0.0.0-20240903143218-8af14fe29dc1/go.mod h1:UqMtugtsSgubUsoxbuAoiCXvqvErP7Gf0so0mK9tHxU= -google.golang.org/grpc v1.66.2 h1:3QdXkuq3Bkh7w+ywLdLvM56cmGvQHUMZpiCzt6Rqaoo= -google.golang.org/grpc v1.66.2/go.mod h1:s3/l6xSSCURdVfAnL+TqCNMyTDAGN6+lZeVxnZR128Y= +google.golang.org/grpc v1.66.3 h1:TWlsh8Mv0QI/1sIbs1W36lqRclxrmF+eFJ4DbI0fuhA= +google.golang.org/grpc v1.66.3/go.mod h1:s3/l6xSSCURdVfAnL+TqCNMyTDAGN6+lZeVxnZR128Y= google.golang.org/protobuf v1.34.2 h1:6xV6lTsCfpGD21XK49h7MhtcApnLqkfYgPcdHftf6hg= google.golang.org/protobuf v1.34.2/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWniOlNbLDw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= 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)) })