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..e49bf803a 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. @@ -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) {