diff --git a/docs/openstack-cloud-controller-manager/using-openstack-cloud-controller-manager.md b/docs/openstack-cloud-controller-manager/using-openstack-cloud-controller-manager.md index cdb8663dcd..538999e1b3 100644 --- a/docs/openstack-cloud-controller-manager/using-openstack-cloud-controller-manager.md +++ b/docs/openstack-cloud-controller-manager/using-openstack-cloud-controller-manager.md @@ -327,6 +327,9 @@ Although the openstack-cloud-controller-manager was initially implemented with N loadbalancer, then populate its listeners, pools and members. This is a compatibility option at the expense of increased load on the OpenStack API. Default: false +* `provider-requires-nodeports` + This option is for cloud providers that do not require nodeport access for accessing nodes from their externalLBs. E.g. when using IPIP tunneling where k8s node IP is only used for filling the outer IP header (inner dstIP is the VIP itself). Default: true + NOTE: * environment variable `OCCM_WAIT_LB_ACTIVE_STEPS` is used to provide steps of waiting loadbalancer to be ready. Current default wait steps is 23 and setup the environment variable overrides default value. Refer to [Backoff.Steps](https://pkg.go.dev/k8s.io/apimachinery/pkg/util/wait#Backoff) for further information. diff --git a/pkg/ingress/config/config.go b/pkg/ingress/config/config.go index 8658fae925..173bbdea7f 100644 --- a/pkg/ingress/config/config.go +++ b/pkg/ingress/config/config.go @@ -62,4 +62,9 @@ type octaviaConfig struct { // the load balancer instead of the bulk update API call. // Default is false. ProviderRequiresSerialAPICalls bool `mapstructure:"provider-requires-serial-api-calls"` + + // (Optional) If the provider requires nodeports for accessing the nodes or allows usage + // of `allocateLoadBalancerNodePorts=false`. + // Default is true. + ProviderRequiresNodeports bool `mapstructure:"provider-requires-nodeports"` } diff --git a/pkg/openstack/loadbalancer.go b/pkg/openstack/loadbalancer.go index c67ba1f1b4..855e07ab4b 100644 --- a/pkg/openstack/loadbalancer.go +++ b/pkg/openstack/loadbalancer.go @@ -1033,10 +1033,18 @@ func (lbaas *LbaasV2) buildBatchUpdateMemberOpts(ctx context.Context, port corev memberSubnetID = nil } - if port.NodePort != 0 { // It's 0 when AllocateLoadBalancerNodePorts=False + protoPort := port.NodePort + if !lbaas.opts.ProviderRequiresNodeports { + // overwrite protoPort as NodePort might be 0 + protoPort = port.Port + } + + // protoPort set when provider doesn't require nodePorts (and e.g. AllocateLoadBalancerNodePorts=false) + // or when NodePort is set + if protoPort > 0 { member := v2pools.BatchUpdateMemberOpts{ Address: addr, - ProtocolPort: int(port.NodePort), + ProtocolPort: int(protoPort), Name: &node.Name, SubnetID: memberSubnetID, } diff --git a/pkg/openstack/loadbalancer_test.go b/pkg/openstack/loadbalancer_test.go index 27304a45ad..fcc3fd1c52 100644 --- a/pkg/openstack/loadbalancer_test.go +++ b/pkg/openstack/loadbalancer_test.go @@ -3,11 +3,12 @@ package openstack import ( "context" "fmt" - "k8s.io/utils/ptr" "reflect" "sort" "testing" + "k8s.io/utils/ptr" + "github.com/gophercloud/gophercloud/v2" "github.com/gophercloud/gophercloud/v2/openstack/loadbalancer/v2/listeners" v2monitors "github.com/gophercloud/gophercloud/v2/openstack/loadbalancer/v2/monitors" @@ -1905,16 +1906,22 @@ func TestBuildBatchUpdateMemberOpts(t *testing.T) { }, }, } + + defaultLBaasOpts := LoadBalancerOpts{ + ProviderRequiresNodeports: true, + } + testCases := []struct { name string nodes []*corev1.Node port corev1.ServicePort svcConf *serviceConfig + lbOpts *LoadBalancerOpts expectedLen int expectedNewMembersCount int }{ { - name: "NodePortequalszero", + name: "NodePort equals zero", nodes: []*corev1.Node{node1, node2}, port: corev1.ServicePort{NodePort: 0}, svcConf: &serviceConfig{ @@ -1925,6 +1932,21 @@ func TestBuildBatchUpdateMemberOpts(t *testing.T) { expectedLen: 0, expectedNewMembersCount: 0, }, + { + name: "NodesPort equals zero, providerRequiresNodeports=false", + nodes: []*corev1.Node{node1, node2}, + port: corev1.ServicePort{NodePort: 0, Port: 80}, + svcConf: &serviceConfig{ + preferredIPFamily: corev1.IPv4Protocol, + lbMemberSubnetID: "subnet-12345-test", + healthCheckNodePort: 8081, + }, + lbOpts: &LoadBalancerOpts{ + ProviderRequiresNodeports: false, + }, + expectedLen: 2, + expectedNewMembersCount: 2, + }, { name: "Valid nodes, canUseHTTPMonitor=false", nodes: []*corev1.Node{node1, node2}, @@ -1987,7 +2009,15 @@ func TestBuildBatchUpdateMemberOpts(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - lbaas := &LbaasV2{} + lbaas := &LbaasV2{ + LoadBalancer: LoadBalancer{ + opts: defaultLBaasOpts, + }, + } + // overwrite default options + if tc.lbOpts != nil { + lbaas.opts = *tc.lbOpts + } members, newMembers, err := lbaas.buildBatchUpdateMemberOpts(context.TODO(), tc.port, tc.nodes, tc.svcConf) assert.Len(t, members, tc.expectedLen) assert.NoError(t, err) diff --git a/pkg/openstack/openstack.go b/pkg/openstack/openstack.go index ac07d50345..cdca28d407 100644 --- a/pkg/openstack/openstack.go +++ b/pkg/openstack/openstack.go @@ -120,6 +120,7 @@ type LoadBalancerOpts struct { MaxSharedLB int `gcfg:"max-shared-lb"` // Number of Services in maximum can share a single load balancer. Default 2 ContainerStore string `gcfg:"container-store"` // Used to specify the store of the tls-container-ref ProviderRequiresSerialAPICalls bool `gcfg:"provider-requires-serial-api-calls"` // default false, the provider supports the "bulk update" API call + ProviderRequiresNodeports bool `gcfg:"provider-requires-nodeports"` // default true, the provider requires nodeports for accessing nodes // revive:disable:var-naming TlsContainerRef string `gcfg:"default-tls-container-ref"` // reference to a tls container // revive:enable:var-naming @@ -227,6 +228,7 @@ func ReadConfig(config io.Reader) (Config, error) { cfg.LoadBalancer.ContainerStore = "barbican" cfg.LoadBalancer.MaxSharedLB = 2 cfg.LoadBalancer.ProviderRequiresSerialAPICalls = false + cfg.LoadBalancer.ProviderRequiresNodeports = true err := gcfg.FatalOnly(gcfg.ReadInto(&cfg, config)) if err != nil {