From ce24807fddd11492ba79212c96c158d87c4f5258 Mon Sep 17 00:00:00 2001 From: Dan Childers Date: Thu, 9 Jul 2026 15:20:16 -0400 Subject: [PATCH 1/2] Add platform-os-max-allowed-address-pairs arg Currently, the maxmium allowed address pairs per port is hardcoded to OpenStack's default of 10. The `platform-os-max-allowed-address-pairs` flag allows users to customize the configuration, mirroring the setting in their installation's neutron.conf. The patch allows any custom configuration greater than zero. Finally, the default of 10 is still used as a fallback if the user doesn't configure `platform-os-max-allowed-address-pairs` Signed-off-by: Dan Childers --- cmd/cloud-network-config-controller/main.go | 1 + pkg/cloudprovider/cloudprovider.go | 3 ++- pkg/cloudprovider/openstack.go | 19 ++++++++++++++++--- pkg/cloudprovider/openstack_test.go | 16 ++++++++-------- 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/cmd/cloud-network-config-controller/main.go b/cmd/cloud-network-config-controller/main.go index 486479a57..f8962e6ae 100644 --- a/cmd/cloud-network-config-controller/main.go +++ b/cmd/cloud-network-config-controller/main.go @@ -264,6 +264,7 @@ func init() { flag.StringVar(&platformCfg.AzureEnvironment, "platform-azure-environment", "AzurePublicCloud", "The Azure environment name, used to select API endpoints") flag.StringVar(&platformCfg.AWSCAOverride, "platform-aws-ca-override", "", "Path to a separate CA bundle to use when connecting to the AWS API") flag.StringVar(&kubeConfig, "kubeconfig", "", "Path to a kubeconfig. Only required if out-of-cluster.") + flag.IntVar(&platformCfg.OSMaxAllowedAddressPairs, "platform-os-max-allowed-address-pairs", 0, "Maximum number of allowed address pairs per OpenStack port. Overrides the Neutron default of 10.") flag.Parse() // Verify required arguments diff --git a/pkg/cloudprovider/cloudprovider.go b/pkg/cloudprovider/cloudprovider.go index 32ca861f6..2a4b99575 100644 --- a/pkg/cloudprovider/cloudprovider.go +++ b/pkg/cloudprovider/cloudprovider.go @@ -94,7 +94,8 @@ type CloudProviderConfig struct { Region string // region, only used by AWS AWSCAOverride string - AzureEnvironment string // The azure "environment", which is a set of API endpoints + AzureEnvironment string // The azure "environment", which is a set of API endpoints + OSMaxAllowedAddressPairs int } type CloudProvider struct { diff --git a/pkg/cloudprovider/openstack.go b/pkg/cloudprovider/openstack.go index 7175753c4..4e759db1c 100644 --- a/pkg/cloudprovider/openstack.go +++ b/pkg/cloudprovider/openstack.go @@ -51,7 +51,7 @@ const ( // to the default value of 10, always; and we might want to document that OSP environments must set // max_allowed_address_pairs >= 10. For more details, see: // https://github.com/openstack/neutron/blob/800f863ccc502b334cb2dd79ec54066440e43e27/neutron/conf/extensions/allowedaddresspairs.py#L21 - openstackMaxCapacity = 10 + defaultOpenstackMaxCapacity = 10 ) // OpenStack implements the API wrapper for talking @@ -549,8 +549,9 @@ func (o *OpenStack) GetNodeEgressIPConfiguration(node *corev1.Node, cpicIPs sets // The interface is keyed by a neutron UUID. // If multiple IPv4 repectively multiple IPv6 subnets are attached to the same port, throw an error. // The IP capacity is per port. The definition of this field does unfortunately not play very well with the way how -// neutron operates as there is no such thing as a per port quota or limit. Therefore we set a ceiling of -// `openstackMaxCapacity`. The number of unique IP addresses in allowed_address_pair and fixed_ips is subtracted from +// neutron operates as there is no such thing as a per port quota or limit. However, a quota can be set to mirror +// the max_allowed_address_pairs configuration in neutron.conf, and when unset, it will default to +// `defaultOpenstackMaxCapacity`. The number of unique IP addresses in allowed_address_pair and fixed_ips is subtracted from // that ceiling. func (o *OpenStack) getNeutronPortNodeEgressIPConfiguration(p neutronports.Port, cpicIPs sets.Set[string]) (*NodeEgressIPConfiguration, error) { var ipv4, ipv6 string @@ -597,7 +598,19 @@ func (o *OpenStack) getNeutronPortNodeEgressIPConfiguration(p neutronports.Port, } } + var openstackMaxCapacity int + + if o.cfg.OSMaxAllowedAddressPairs > 0 { + openstackMaxCapacity = o.cfg.OSMaxAllowedAddressPairs + } else { + openstackMaxCapacity = defaultOpenstackMaxCapacity + } + c := openstackMaxCapacity + cloudPrivateIPsCount - len(p.AllowedAddressPairs) + if c < 0 { + return nil, fmt.Errorf("max allowed address pairs (%d) is too small for port %s which already has (%d) allowed address pairs and (%d) managed by CNCC", + openstackMaxCapacity, p.ID, len(p.AllowedAddressPairs), cloudPrivateIPsCount) + } return &NodeEgressIPConfiguration{ Interface: p.ID, IFAddr: ifAddr{ diff --git a/pkg/cloudprovider/openstack_test.go b/pkg/cloudprovider/openstack_test.go index 232729c3c..f2c69dda5 100644 --- a/pkg/cloudprovider/openstack_test.go +++ b/pkg/cloudprovider/openstack_test.go @@ -685,7 +685,7 @@ func TestOpenStackPlugin(t *testing.T) { IPv6: "2000::/64", }, Capacity: capacity{ - IP: ptr.To(openstackMaxCapacity), + IP: ptr.To(defaultOpenstackMaxCapacity), }, }, } @@ -812,7 +812,7 @@ func TestGetNodeEgressIPConfiguration(t *testing.T) { IPv6: "2000::/64", }, Capacity: capacity{ - IP: ptr.To(openstackMaxCapacity), + IP: ptr.To(defaultOpenstackMaxCapacity), }, }, }, @@ -848,7 +848,7 @@ func TestGetNodeEgressIPConfiguration(t *testing.T) { IPv6: "2001::/64", }, Capacity: capacity{ - IP: ptr.To(openstackMaxCapacity), + IP: ptr.To(defaultOpenstackMaxCapacity), }, }, }, @@ -892,7 +892,7 @@ func TestGetNodeEgressIPConfiguration(t *testing.T) { IPv6: "2000::/64", }, Capacity: capacity{ - IP: ptr.To(openstackMaxCapacity), + IP: ptr.To(defaultOpenstackMaxCapacity), }, }, }, @@ -939,7 +939,7 @@ func TestGetNodeEgressIPConfiguration(t *testing.T) { IPv6: "2000::/64", }, Capacity: capacity{ - IP: ptr.To(openstackMaxCapacity), + IP: ptr.To(defaultOpenstackMaxCapacity), }, }, }, @@ -951,7 +951,7 @@ func TestGetNodeEgressIPConfiguration(t *testing.T) { IPv6: "2001::/64", }, Capacity: capacity{ - IP: ptr.To(openstackMaxCapacity), + IP: ptr.To(defaultOpenstackMaxCapacity), }, }, }, @@ -1036,7 +1036,7 @@ func TestGetNeutronPortNodeEgressIPConfiguration(t *testing.T) { IPv6: "2000::/64", }, Capacity: capacity{ - IP: ptr.To(openstackMaxCapacity - 5), // 5 allowed_address_pairs configured on the port. + IP: ptr.To(defaultOpenstackMaxCapacity - 5), // 5 allowed_address_pairs configured on the port. }, }, }, @@ -1049,7 +1049,7 @@ func TestGetNeutronPortNodeEgressIPConfiguration(t *testing.T) { IPv6: "2000::/64", }, Capacity: capacity{ - IP: ptr.To(openstackMaxCapacity - 2), // excluding 2 allowed_address_pairs configured on the port. + IP: ptr.To(defaultOpenstackMaxCapacity - 2), // excluding 2 allowed_address_pairs configured on the port. }, }, // Configure IPs with 3 ips are within neutron subnet, 1 ip outside neutron subnet. From d1a1b288ee2fb8003b0023db2ebbb7fb218a6bc4 Mon Sep 17 00:00:00 2001 From: Dan Childers Date: Wed, 15 Jul 2026 11:18:59 -0400 Subject: [PATCH 2/2] Add unit test coverage for the new platform-os-max-allowed-address-pairs arg Coverage includes testing the following combinations of conditions: - custom positive value overrides default - unset flag uses default capacity - custom value is too small Signed-off-by: Dan Childers --- pkg/cloudprovider/openstack_test.go | 83 ++++++++++++++++++++++++++++- 1 file changed, 82 insertions(+), 1 deletion(-) diff --git a/pkg/cloudprovider/openstack_test.go b/pkg/cloudprovider/openstack_test.go index f2c69dda5..e49bf803a 100644 --- a/pkg/cloudprovider/openstack_test.go +++ b/pkg/cloudprovider/openstack_test.go @@ -1066,7 +1066,7 @@ func TestGetNeutronPortNodeEgressIPConfiguration(t *testing.T) { } for i, tc := range tcs { - cpicIPs := sets.New[string](tc.cloudPrivateIPConfigs...) + cpicIPs := sets.New(tc.cloudPrivateIPConfigs...) nodeEgressIPConfig, err := o.getNeutronPortNodeEgressIPConfiguration(tc.port, cpicIPs) if err != nil { if !strings.Contains(err.Error(), tc.errString) { @@ -1081,6 +1081,87 @@ func TestGetNeutronPortNodeEgressIPConfiguration(t *testing.T) { } } +func TestGetNeutronPortNodeEgressIPConfigurationMaxAllowedAddressPairs(t *testing.T) { + th.SetupHTTP() + defer th.TeardownHTTP() + HandleSubnetList(t) + HandlePortListAndCreation(t) + + tcs := []struct { + name string + maxAllowedAddrPairs int + port neutronports.Port + nodeEgressIPConfig NodeEgressIPConfiguration + cloudPrivateIPConfigs []string + errString string + }{ + { + name: "custom positive value overrides default", + maxAllowedAddrPairs: 20, + port: portMap["9ab428d4-58f8-42d7-9672-90c3f5641f83"], + nodeEgressIPConfig: NodeEgressIPConfiguration{ + Interface: "9ab428d4-58f8-42d7-9672-90c3f5641f83", + IFAddr: ifAddr{ + IPv4: "192.0.2.0/24", + IPv6: "2000::/64", + }, + Capacity: capacity{ + IP: ptr.To(20 - 5), // 5 allowed_address_pairs on the port + }, + }, + }, + { + name: "max too small for existing address pairs", + maxAllowedAddrPairs: 3, + port: portMap["9ab428d4-58f8-42d7-9672-90c3f5641f83"], + errString: "max allowed address pairs (3) is too small for port 9ab428d4-58f8-42d7-9672-90c3f5641f83 which already has (5) allowed address pairs and (0) managed by CNCC", + }, + { + name: "unset flag uses default capacity", + maxAllowedAddrPairs: 0, + port: portMap["9ab428d4-58f8-42d7-9672-90c3f5641f83"], + nodeEgressIPConfig: NodeEgressIPConfiguration{ + Interface: "9ab428d4-58f8-42d7-9672-90c3f5641f83", + IFAddr: ifAddr{ + IPv4: "192.0.2.0/24", + IPv6: "2000::/64", + }, + Capacity: capacity{ + IP: ptr.To(defaultOpenstackMaxCapacity - 5), + }, + }, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + o := OpenStack{ + CloudProvider: CloudProvider{ + cfg: CloudProviderConfig{ + OSMaxAllowedAddressPairs: tc.maxAllowedAddrPairs, + }, + }, + novaClient: testclient.ServiceClient(), + neutronClient: testclient.ServiceClient(), + } + cpicIPs := sets.New(tc.cloudPrivateIPConfigs...) + nodeEgressIPConfig, err := o.getNeutronPortNodeEgressIPConfiguration(tc.port, cpicIPs) + if err != nil { + if tc.errString == "" || !strings.Contains(err.Error(), tc.errString) { + t.Fatalf("Received unexpected error, err: %q, expected to contain: %q", err, tc.errString) + } + return + } + if tc.errString != "" { + t.Fatalf("Expected error containing %q but got none", tc.errString) + } + if !reflect.DeepEqual(*nodeEgressIPConfig, tc.nodeEgressIPConfig) { + t.Fatalf("Received unexpected nodeEgressIPConfig.\nExpected: %v\nGot: %v", tc.nodeEgressIPConfig, nodeEgressIPConfig) + } + }) + } +} + // TestAllowUnAllowIPAddressOnNeutronPort tests both allowIPAddressOnNeutronPort and // unAllowIPAddressOnNeutronPort. func TestAllowUnAllowIPAddressOnNeutronPort(t *testing.T) {