Repository: kubernetes-sigs/cluster-api-provider-openstack

Issue:
No unit tests for load balancer listeners
/kind bug

There are no unit tests for load balancer listeners, and its dependent resources including the reconciler.
We should have them like we have tests for load balancers.

Discussion:
@k8s-triage-robot: The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:
- After 90d of inactivity, `lifecycle/stale` is applied
- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied
- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed

You can:
- Mark this issue as fresh with `/remove-lifecycle stale`
- Close this issue with `/close`
- Offer to help out with [Issue Triage][1]

Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community).

/lifecycle stale

[1]: https://www.kubernetes.dev/docs/guide/issue-triage/@EmilienM: /remove-lifecycle stale@k8s-triage-robot: The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:
- After 90d of inactivity, `lifecycle/stale` is applied
- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied
- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed

You can:
- Mark this issue as fresh with `/remove-lifecycle stale`
- Close this issue with `/close`
- Offer to help out with [Issue Triage][1]

Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community).

/lifecycle stale

[1]: https://www.kubernetes.dev/docs/guide/issue-triage/@EmilienM: /remove-lifecycle stale@k8s-triage-robot: The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:
- After 90d of inactivity, `lifecycle/stale` is applied
- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied
- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed

You can:
- Mark this issue as fresh with `/remove-lifecycle stale`
- Close this issue with `/close`
- Offer to help out with [Issue Triage][1]

Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community).

/lifecycle stale

[1]: https://www.kubernetes.dev/docs/guide/issue-triage/@EmilienM: /remove-lifecycle stale@k8s-triage-robot: The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:
- After 90d of inactivity, `lifecycle/stale` is applied
- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied
- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed

You can:
- Mark this issue as fresh with `/remove-lifecycle stale`
- Close this issue with `/close`
- Offer to help out with [Issue Triage][1]

Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community).

/lifecycle stale

[1]: https://www.kubernetes.dev/docs/guide/issue-triage/@k8s-triage-robot: The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:
- After 90d of inactivity, `lifecycle/stale` is applied
- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied
- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed

You can:
- Mark this issue as fresh with `/remove-lifecycle rotten`
- Close this issue with `/close`
- Offer to help out with [Issue Triage][1]

Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community).

/lifecycle rotten

[1]: https://www.kubernetes.dev/docs/guide/issue-triage/@k8s-triage-robot: The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:
- After 90d of inactivity, `lifecycle/stale` is applied
- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied
- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed

You can:
- Reopen this issue with `/reopen`
- Mark this issue as fresh with `/remove-lifecycle rotten`
- Offer to help out with [Issue Triage][1]

Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community).

/close not-planned

[1]: https://www.kubernetes.dev/docs/guide/issue-triage/@k8s-ci-robot: @k8s-triage-robot: Closing this issue, marking it as "Not Planned".

<details>

In response to [this](https://github.com/kubernetes-sigs/cluster-api-provider-openstack/issues/1906#issuecomment-2824912009):

>The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.
>
>This bot triages issues according to the following rules:
>- After 90d of inactivity, `lifecycle/stale` is applied
>- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied
>- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed
>
>You can:
>- Reopen this issue with `/reopen`
>- Mark this issue as fresh with `/remove-lifecycle rotten`
>- Offer to help out with [Issue Triage][1]
>
>Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community).
>
>/close not-planned
>
>[1]: https://www.kubernetes.dev/docs/guide/issue-triage/


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>

Solution patch:
diff --git a/Makefile b/Makefile
index 4c6837f7..26d97465 100644
--- a/Makefile
+++ b/Makefile
@@ -245,25 +245,28 @@ modules: ## Runs go mod to ensure proper vendoring.
 	cd $(TOOLS_DIR); go mod tidy
 
 .PHONY: generate
-generate: ## Generate code
-	$(MAKE) generate-go
-	$(MAKE) generate-manifests
+generate: generate-controller-gen generate-conversion-gen generate-go generate-manifests ## Generate all generated code
 
 .PHONY: generate-go
 generate-go: $(MOCKGEN)
-	$(MAKE) -B $(CONTROLLER_GEN) $(CONVERSION_GEN)
+	go generate ./...
+
+.PHONY: generate-controller-gen
+generate-controller-gen: $(CONTROLLER_GEN)
 	$(CONTROLLER_GEN) \
 		paths=./api/... \
 		object:headerFile=./hack/boilerplate/boilerplate.generatego.txt
+
+.PHONY: generate-conversion-gen
+capo_module := sigs.k8s.io/cluster-api-provider-openstack
+generate-conversion-gen: $(CONVERSION_GEN)
 	$(CONVERSION_GEN) \
-		--input-dirs=./api/v1alpha1 \
-		--input-dirs=./api/v1alpha5 \
-		--input-dirs=./api/v1alpha6 \
-		--input-dirs=./api/v1alpha7 \
-		--input-dirs=./api/v1beta1 \
+		--input-dirs=$(capo_module)/api/v1alpha5 \
+		--input-dirs=$(capo_module)/api/v1alpha6 \
+		--input-dirs=$(capo_module)/api/v1alpha7 \
 		--output-file-base=zz_generated.conversion \
+		--trim-path-prefix=$(capo_module)/ \
 		--go-header-file=./hack/boilerplate/boilerplate.generatego.txt
-	go generate ./...
 
 .PHONY: generate-manifests
 generate-manifests: $(CONTROLLER_GEN) ## Generate manifests e.g. CRD, RBAC etc.
diff --git a/pkg/cloud/services/loadbalancer/loadbalancer.go b/pkg/cloud/services/loadbalancer/loadbalancer.go
index 1d0812c3..38dda5ec 100644
--- a/pkg/cloud/services/loadbalancer/loadbalancer.go
+++ b/pkg/cloud/services/loadbalancer/loadbalancer.go
@@ -76,42 +76,7 @@ func (s *Service) ReconcileLoadBalancer(openStackCluster *infrav1.OpenStackClust
 		openStackCluster.Status.APIServerLoadBalancer = lbStatus
 	}
 
-	var fixedIPAddress string
-	var err error
-
-	switch {
-	case lbStatus.InternalIP != "":
-		fixedIPAddress = lbStatus.InternalIP
-	case openStackCluster.Spec.APIServerFixedIP != "":
-		fixedIPAddress = openStackCluster.Spec.APIServerFixedIP
-	case openStackCluster.Spec.DisableAPIServerFloatingIP && openStackCluster.Spec.ControlPlaneEndpoint.IsValid():
-		fixedIPAddress, err = lookupHost(openStackCluster.Spec.ControlPlaneEndpoint.Host)
-		if err != nil {
-			return false, fmt.Errorf("lookup host: %w", err)
-		}
-	}
-
-	providers, err := s.loadbalancerClient.ListLoadBalancerProviders()
-	if err != nil {
-		return false, err
-	}
-
-	// Choose the selected provider if it is set in cluster spec, if not, omit the field and Octavia will use the default provider.
-	lbProvider := ""
-	if openStackCluster.Spec.APIServerLoadBalancer.Provider != "" {
-		for _, v := range providers {
-			if v.Name == openStackCluster.Spec.APIServerLoadBalancer.Provider {
-				lbProvider = v.Name
-				break
-			}
-		}
-		if lbProvider == "" {
-			record.Warnf(openStackCluster, "OctaviaProviderNotFound", "Provider specified for Octavia not found.")
-			record.Eventf(openStackCluster, "OctaviaProviderNotFound", "Provider %s specified for Octavia not found, using the default provider.", openStackCluster.Spec.APIServerLoadBalancer.Provider)
-		}
-	}
-
-	lb, err := s.getOrCreateLoadBalancer(openStackCluster, loadBalancerName, openStackCluster.Status.Network.Subnets[0].ID, clusterName, fixedIPAddress, lbProvider)
+	lb, err := s.getOrCreateAPILoadBalancer(openStackCluster, clusterName)
 	if err != nil {
 		return false, err
 	}
@@ -121,23 +86,20 @@ func (s *Service) ReconcileLoadBalancer(openStackCluster *infrav1.OpenStackClust
 	lbStatus.InternalIP = lb.VipAddress
 	lbStatus.Tags = lb.Tags
 
-	if err := s.waitForLoadBalancerActive(lb.ID); err != nil {
-		return false, fmt.Errorf("load balancer %q with id %s is not active after timeout: %v", loadBalancerName, lb.ID, err)
+	if lb.ProvisioningStatus != loadBalancerProvisioningStatusActive {
+		var err error
+		lb, err = s.waitForLoadBalancerActive(lb.ID)
+		if err != nil {
+			return false, fmt.Errorf("load balancer %q with id %s is not active after timeout: %v", loadBalancerName, lb.ID, err)
+		}
 	}
 
 	if !openStackCluster.Spec.DisableAPIServerFloatingIP {
-		var floatingIPAddress string
-		switch {
-		case lbStatus.IP != "":
-			floatingIPAddress = lbStatus.IP
-		case openStackCluster.Spec.APIServerFloatingIP != "":
-			floatingIPAddress = openStackCluster.Spec.APIServerFloatingIP
-		case openStackCluster.Spec.ControlPlaneEndpoint.IsValid():
-			floatingIPAddress, err = lookupHost(openStackCluster.Spec.ControlPlaneEndpoint.Host)
-			if err != nil {
-				return false, fmt.Errorf("lookup host: %w", err)
-			}
+		floatingIPAddress, err := getAPIServerFloatingIP(openStackCluster)
+		if err != nil {
+			return false, err
 		}
+
 		fp, err := s.networkingService.GetOrCreateFloatingIP(openStackCluster, openStackCluster, clusterName, floatingIPAddress)
 		if err != nil {
 			return false, err
@@ -153,71 +115,190 @@ func (s *Service) ReconcileLoadBalancer(openStackCluster *infrav1.OpenStackClust
 		}
 	}
 
-	allowedCIDRs := []string{}
-	// To reduce API calls towards OpenStack API, let's handle the CIDR support verification for all Ports only once.
-	allowedCIDRsSupported := false
-	octaviaVersions, err := s.loadbalancerClient.ListOctaviaVersions()
+	allowedCIDRsSupported, err := s.isAllowsCIDRSSupported(lb)
 	if err != nil {
 		return false, err
 	}
-	// The current version is always the last one in the list.
-	octaviaVersion := octaviaVersions[len(octaviaVersions)-1].ID
-	if openstackutil.IsOctaviaFeatureSupported(octaviaVersion, openstackutil.OctaviaFeatureVIPACL, lbProvider) {
-		allowedCIDRsSupported = true
+
+	// AllowedCIDRs will be nil if allowed CIDRs is not supported by the Octavia provider
+	if allowedCIDRsSupported {
+		lbStatus.AllowedCIDRs = getCanonicalAllowedCIDRs(openStackCluster)
+	} else {
+		lbStatus.AllowedCIDRs = nil
 	}
 
 	portList := []int{apiServerPort}
 	portList = append(portList, openStackCluster.Spec.APIServerLoadBalancer.AdditionalPorts...)
 	for _, port := range portList {
-		lbPortObjectsName := fmt.Sprintf("%s-%d", loadBalancerName, port)
+		if err := s.reconcileAPILoadBalancerListener(lb, openStackCluster, clusterName, port); err != nil {
+			return false, err
+		}
+	}
+
+	return false, nil
+}
+
+// getAPIServerVIPAddress gets the VIP address for the API server from wherever it is specified.
+// Returns an empty string if the VIP address is not specified and it should be allocated automatically.
+func getAPIServerVIPAddress(openStackCluster *infrav1.OpenStackCluster) (string, error) {
+	switch {
+	// We only use call this function when creating the loadbalancer, so this case should never be used
+	case openStackCluster.Status.APIServerLoadBalancer != nil && openStackCluster.Status.APIServerLoadBalancer.InternalIP != "":
+		return openStackCluster.Status.APIServerLoadBalancer.InternalIP, nil
+
+	// Explicit fixed IP in the cluster spec
+	case openStackCluster.Spec.APIServerFixedIP != "":
+		return openStackCluster.Spec.APIServerFixedIP, nil
 
-		listener, err := s.getOrCreateListener(openStackCluster, lbPortObjectsName, lb.ID, port)
+	// If we are using the VIP as the control plane endpoint, use any value explicitly set on the control plane endpoint
+	case openStackCluster.Spec.DisableAPIServerFloatingIP && openStackCluster.Spec.ControlPlaneEndpoint.IsValid():
+		fixedIPAddress, err := lookupHost(openStackCluster.Spec.ControlPlaneEndpoint.Host)
 		if err != nil {
-			return false, err
+			return "", fmt.Errorf("lookup host: %w", err)
 		}
+		return fixedIPAddress, nil
+	}
+
+	return "", nil
+}
+
+// getAPIServerFloatingIP gets the floating IP from wherever it is specified.
+// Returns an empty string if the floating IP is not specified and it should be allocated automatically.
+func getAPIServerFloatingIP(openStackCluster *infrav1.OpenStackCluster) (string, error) {
+	switch {
+	// The floating IP was created previously
+	case openStackCluster.Status.APIServerLoadBalancer != nil && openStackCluster.Status.APIServerLoadBalancer.IP != "":
+		return openStackCluster.Status.APIServerLoadBalancer.IP, nil
 
-		pool, err := s.getOrCreatePool(openStackCluster, lbPortObjectsName, listener.ID, lb.ID, lb.Provider)
+	// Explicit floating IP in the cluster spec
+	case openStackCluster.Spec.APIServerFloatingIP != "":
+		return openStackCluster.Spec.APIServerFloatingIP, nil
+
+	// An IP address is specified explicitly in the control plane endpoint
+	case openStackCluster.Spec.ControlPlaneEndpoint.IsValid():
+		floatingIPAddress, err := lookupHost(openStackCluster.Spec.ControlPlaneEndpoint.Host)
 		if err != nil {
-			return false, err
+			return "", fmt.Errorf("lookup host: %w", err)
 		}
+		return floatingIPAddress, nil
+	}
 
-		if err := s.getOrCreateMonitor(openStackCluster, lbPortObjectsName, pool.ID, lb.ID); err != nil {
-			return false, err
+	return "", nil
+}
+
+// getCanonicalAllowedCIDRs gets a filtered list of CIDRs which should be allowed to access the API server loadbalancer.
+// Invalid CIDRs are filtered from the list and emil a warning event.
+// It returns a canonical representation that can be directly compared with other canonicalized lists.
+func getCanonicalAllowedCIDRs(openStackCluster *infrav1.OpenStackCluster) []string {
+	allowedCIDRs := []string{}
+
+	if len(openStackCluster.Spec.APIServerLoadBalancer.AllowedCIDRs) > 0 {
+		allowedCIDRs = append(allowedCIDRs, openStackCluster.Spec.APIServerLoadBalancer.AllowedCIDRs...)
+
+		// In the first reconciliation loop, only the Ready field is set in openStackCluster.Status
+		// All other fields are empty/nil
+		if openStackCluster.Status.Bastion != nil {
+			if openStackCluster.Status.Bastion.FloatingIP != "" {
+				allowedCIDRs = append(allowedCIDRs, openStackCluster.Status.Bastion.FloatingIP)
+			}
+
+			if openStackCluster.Status.Bastion.IP != "" {
+				allowedCIDRs = append(allowedCIDRs, openStackCluster.Status.Bastion.IP)
+			}
 		}
 
-		if allowedCIDRsSupported {
-			// Skip reconciliation if network status is nil (e.g. during clusterctl move)
-			if openStackCluster.Status.Network != nil {
-				if err := s.getOrUpdateAllowedCIDRS(openStackCluster, listener); err != nil {
-					return false, err
+		if openStackCluster.Status.Network != nil {
+			for _, subnet := range openStackCluster.Status.Network.Subnets {
+				if subnet.CIDR != "" {
+					allowedCIDRs = append(allowedCIDRs, subnet.CIDR)
 				}
-				allowedCIDRs = listener.AllowedCIDRs
+			}
+
+			if openStackCluster.Status.Router != nil && len(openStackCluster.Status.Router.IPs) > 0 {
+				allowedCIDRs = append(allowedCIDRs, openStackCluster.Status.Router.IPs...)
 			}
 		}
 	}
-	lbStatus.AllowedCIDRs = allowedCIDRs
 
-	return false, nil
+	// Filter invalid CIDRs and convert any IPs into CIDRs.
+	validCIDRs := []string{}
+	for _, v := range allowedCIDRs {
+		switch {
+		case utilsnet.IsIPv4String(v):
+			validCIDRs = append(validCIDRs, v+"/32")
+		case utilsnet.IsIPv4CIDRString(v):
+			validCIDRs = append(validCIDRs, v)
+		default:
+			record.Warnf(openStackCluster, "FailedIPAddressValidation", "%s is not a valid IPv4 nor CIDR address and will not get applied to allowed_cidrs", v)
+		}
+	}
+
+	// Sort and remove duplicates
+	return capostrings.Canonicalize(validCIDRs)
+}
+
+// isAllowsCIDRSSupported returns true if Octavia supports allowed CIDRs for the loadbalancer provider in use.
+func (s *Service) isAllowsCIDRSSupported(lb *loadbalancers.LoadBalancer) (bool, error) {
+	octaviaVersions, err := s.loadbalancerClient.ListOctaviaVersions()
+	if err != nil {
+		return false, err
+	}
+	// The current version is always the last one in the list.
+	octaviaVersion := octaviaVersions[len(octaviaVersions)-1].ID
+
+	return openstackutil.IsOctaviaFeatureSupported(octaviaVersion, openstackutil.OctaviaFeatureVIPACL, lb.Provider), nil
 }
 
-func (s *Service) getOrCreateLoadBalancer(openStackCluster *infrav1.OpenStackCluster, loadBalancerName, subnetID, clusterName, vipAddress, provider string) (*loadbalancers.LoadBalancer, error) {
+// getOrCreateAPILoadBalancer returns an existing API loadbalancer if it already exists, or creates a new one if it does not.
+func (s *Service) getOrCreateAPILoadBalancer(openStackCluster *infrav1.OpenStackCluster, clusterName string) (*loadbalancers.LoadBalancer, error) {
+	loadBalancerName := getLoadBalancerName(clusterName)
 	lb, err := s.checkIfLbExists(loadBalancerName)
 	if err != nil {
 		return nil, err
 	}
-
 	if lb != nil {
 		return lb, nil
 	}
 
+	if openStackCluster.Status.Network == nil {
+		return nil, fmt.Errorf("network is not yet available in OpenStackCluster.Status")
+	}
+
+	// Create the VIP on the first cluster subnet
+	subnetID := openStackCluster.Status.Network.Subnets[0].ID
 	s.scope.Logger().Info("Creating load balancer in subnet", "subnetID", subnetID, "name", loadBalancerName)
 
+	providers, err := s.loadbalancerClient.ListLoadBalancerProviders()
+	if err != nil {
+		return nil, err
+	}
+
+	// Choose the selected provider if it is set in cluster spec, if not, omit the field and Octavia will use the default provider.
+	lbProvider := ""
+	if openStackCluster.Spec.APIServerLoadBalancer.Provider != "" {
+		for _, v := range providers {
+			if v.Name == openStackCluster.Spec.APIServerLoadBalancer.Provider {
+				lbProvider = v.Name
+				break
+			}
+		}
+		if lbProvider == "" {
+			record.Warnf(openStackCluster, "OctaviaProviderNotFound", "Provider specified for Octavia not found.")
+			record.Eventf(openStackCluster, "OctaviaProviderNotFound", "Provider %s specified for Octavia not found, using the default provider.", openStackCluster.Spec.APIServerLoadBalancer.Provider)
+		}
+	}
+
+	vipAddress, err := getAPIServerVIPAddress(openStackCluster)
+	if err != nil {
+		return nil, err
+	}
+
 	lbCreateOpts := loadbalancers.CreateOpts{
 		Name:        loadBalancerName,
 		VipSubnetID: subnetID,
 		VipAddress:  vipAddress,
 		Description: names.GetDescription(clusterName),
-		Provider:    provider,
+		Provider:    lbProvider,
 		Tags:        openStackCluster.Spec.Tags,
 	}
 	lb, err = s.loadbalancerClient.CreateLoadBalancer(lbCreateOpts)
@@ -230,7 +311,45 @@ func (s *Service) getOrCreateLoadBalancer(openStackCluster *infrav1.OpenStackClu
 	return lb, nil
 }
 
-func (s *Service) getOrCreateListener(openStackCluster *infrav1.OpenStackCluster, listenerName, lbID string, port int) (*listeners.Listener, error) {
+// reconcileAPILoadBalancerListener ensures that the listener on the given port exists and is configured correctly.
+func (s *Service) reconcileAPILoadBalancerListener(lb *loadbalancers.LoadBalancer, openStackCluster *infrav1.OpenStackCluster, clusterName string, port int) error {
+	loadBalancerName := getLoadBalancerName(clusterName)
+	lbPortObjectsName := fmt.Sprintf("%s-%d", loadBalancerName, port)
+
+	if openStackCluster.Status.APIServerLoadBalancer == nil {
+		return fmt.Errorf("APIServerLoadBalancer is not yet available in OpenStackCluster.Status")
+	}
+
+	allowedCIDRs := openStackCluster.Status.APIServerLoadBalancer.AllowedCIDRs
+
+	listener, err := s.getOrCreateListener(openStackCluster, lbPortObjectsName, lb.ID, allowedCIDRs, port)
+	if err != nil {
+		return err
+	}
+
+	pool, err := s.getOrCreatePool(openStackCluster, lbPortObjectsName, listener.ID, lb.ID, lb.Provider)
+	if err != nil {
+		return err
+	}
+
+	if err := s.getOrCreateMonitor(openStackCluster, lbPortObjectsName, pool.ID, lb.ID); err != nil {
+		return err
+	}
+
+	// allowedCIDRs is nil if allowedCIDRs is not supported by the Octavia provider
+	// A non-nil empty slice is an explicitly empty list
+	if allowedCIDRs != nil {
+		if err := s.getOrUpdateAllowedCIDRs(openStackCluster, listener, allowedCIDRs); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// getOrCreateListener returns an existing listener for the given loadbalancer
+// and port if it already exists, or creates a new one if it does not.
+func (s *Service) getOrCreateListener(openStackCluster *infrav1.OpenStackCluster, listenerName, lbID string, allowedCIDRs []string, port int) (*listeners.Listener, error) {
 	listener, err := s.checkIfListenerExists(listenerName)
 	if err != nil {
 		return nil, err
@@ -248,6 +367,7 @@ func (s *Service) getOrCreateListener(openStackCluster *infrav1.OpenStackCluster
 		ProtocolPort:   port,
 		LoadbalancerID: lbID,
 		Tags:           openStackCluster.Spec.Tags,
+		AllowedCIDRs:   allowedCIDRs,
 	}
 	listener, err = s.loadbalancerClient.CreateListener(listenerCreateOpts)
 	if err != nil {
@@ -255,7 +375,7 @@ func (s *Service) getOrCreateListener(openStackCluster *infrav1.OpenStackCluster
 		return nil, err
 	}
 
-	if err := s.waitForLoadBalancerActive(lbID); err != nil {
+	if _, err := s.waitForLoadBalancerActive(lbID); err != nil {
 		record.Warnf(openStackCluster, "FailedCreateListener", "Failed to create listener %s with id %s: wait for load balancer active %s: %v", listenerName, listener.ID, lbID, err)
 		return nil, err
 	}
@@ -269,42 +389,9 @@ func (s *Service) getOrCreateListener(openStackCluster *infrav1.OpenStackCluster
 	return listener, nil
 }
 
-func (s *Service) getOrUpdateAllowedCIDRS(openStackCluster *infrav1.OpenStackCluster, listener *listeners.Listener) error {
-	allowedCIDRs := []string{}
-
-	if len(openStackCluster.Spec.APIServerLoadBalancer.AllowedCIDRs) > 0 {
-		allowedCIDRs = append(allowedCIDRs, openStackCluster.Spec.APIServerLoadBalancer.AllowedCIDRs...)
-
-		// In the first reconciliation loop, only the Ready field is set in openStackCluster.Status
-		// All other fields are empty/nil
-		if openStackCluster.Status.Bastion != nil {
-			if openStackCluster.Status.Bastion.FloatingIP != "" {
-				allowedCIDRs = append(allowedCIDRs, openStackCluster.Status.Bastion.FloatingIP)
-			}
-
-			if openStackCluster.Status.Bastion.IP != "" {
-				allowedCIDRs = append(allowedCIDRs, openStackCluster.Status.Bastion.IP)
-			}
-		}
-
-		if openStackCluster.Status.Network != nil {
-			for _, subnet := range openStackCluster.Status.Network.Subnets {
-				if subnet.CIDR != "" {
-					allowedCIDRs = append(allowedCIDRs, subnet.CIDR)
-				}
-			}
-
-			if len(openStackCluster.Status.Router.IPs) > 0 {
-				allowedCIDRs = append(allowedCIDRs, openStackCluster.Status.Router.IPs...)
-			}
-		}
-	}
-
-	// Validate CIDRs and convert any given IP into a CIDR.
-	allowedCIDRs = validateIPs(openStackCluster, allowedCIDRs)
-
+// getOrUpdateAllowedCIDRs ensures that the allowed CIDRs configured on a listener correspond to the expected list.
+func (s *Service) getOrUpdateAllowedCIDRs(openStackCluster *infrav1.OpenStackCluster, listener *listeners.Listener, allowedCIDRs []string) error {
 	// Sort and remove duplicates
-	allowedCIDRs = capostrings.Canonicalize(allowedCIDRs)
 	listener.AllowedCIDRs = capostrings.Canonicalize(listener.AllowedCIDRs)
 
 	if !slices.Equal(allowedCIDRs, listener.AllowedCIDRs) {
@@ -330,24 +417,6 @@ func (s *Service) getOrUpdateAllowedCIDRS(openStackCluster *infrav1.OpenStackClu
 	return nil
 }
 
-// validateIPs validates given IPs/CIDRs and removes non valid network objects.
-func validateIPs(openStackCluster *infrav1.OpenStackCluster, definedCIDRs []string) []string {
-	marshaledCIDRs := []string{}
-
-	for _, v := range definedCIDRs {
-		switch {
-		case utilsnet.IsIPv4String(v):
-			marshaledCIDRs = append(marshaledCIDRs, v+"/32")
-		case utilsnet.IsIPv4CIDRString(v):
-			marshaledCIDRs = append(marshaledCIDRs, v)
-		default:
-			record.Warnf(openStackCluster, "FailedIPAddressValidation", "%s is not a valid IPv4 nor CIDR address and will not get applied to allowed_cidrs", v)
-		}
-	}
-
-	return marshaledCIDRs
-}
-
 func (s *Service) getOrCreatePool(openStackCluster *infrav1.OpenStackCluster, poolName, listenerID, lbID string, lbProvider string) (*pools.Pool, error) {
 	pool, err := s.checkIfPoolExists(poolName)
 	if err != nil {
@@ -379,7 +448,7 @@ func (s *Service) getOrCreatePool(openStackCluster *infrav1.OpenStackCluster, po
 		return nil, err
 	}
 
-	if err := s.waitForLoadBalancerActive(lbID); err != nil {
+	if _, err := s.waitForLoadBalancerActive(lbID); err != nil {
 		record.Warnf(openStackCluster, "FailedCreatePool", "Failed to create pool %s with id %s: wait for load balancer active %s: %v", poolName, pool.ID, lbID, err)
 		return nil, err
 	}
@@ -421,7 +490,7 @@ func (s *Service) getOrCreateMonitor(openStackCluster *infrav1.OpenStackCluster,
 		return err
 	}
 
-	if err = s.waitForLoadBalancerActive(lbID); err != nil {
+	if _, err = s.waitForLoadBalancerActive(lbID); err != nil {
 		record.Warnf(openStackCluster, "FailedCreateMonitor", "Failed to create monitor %s with id %s: wait for load balancer active %s: %v", monitorName, monitor.ID, lbID, err)
 		return err
 	}
@@ -474,14 +543,14 @@ func (s *Service) ReconcileLoadBalancerMember(openStackCluster *infrav1.OpenStac
 			s.scope.Logger().Info("Deleting load balancer member because the IP of the machine changed", "name", name)
 
 			// lb member changed so let's delete it so we can create it again with the correct IP
-			err = s.waitForLoadBalancerActive(lbID)
+			_, err = s.waitForLoadBalancerActive(lbID)
 			if err != nil {
 				return err
 			}
 			if err := s.loadbalancerClient.DeletePoolMember(pool.ID, lbMember.ID); err != nil {
 				return err
 			}
-			err = s.waitForLoadBalancerActive(lbID)
+			_, err = s.waitForLoadBalancerActive(lbID)
 			if err != nil {
 				return err
 			}
@@ -497,7 +566,7 @@ func (s *Service) ReconcileLoadBalancerMember(openStackCluster *infrav1.OpenStac
 			Tags:         openStackCluster.Spec.Tags,
 		}
 
-		if err := s.waitForLoadBalancerActive(lbID); err != nil {
+		if _, err := s.waitForLoadBalancerActive(lbID); err != nil {
 			return err
 		}
 
@@ -505,7 +574,7 @@ func (s *Service) ReconcileLoadBalancerMember(openStackCluster *infrav1.OpenStac
 			return err
 		}
 
-		if err := s.waitForLoadBalancerActive(lbID); err != nil {
+		if _, err := s.waitForLoadBalancerActive(lbID); err != nil {
 			return err
 		}
 	}
@@ -598,14 +667,14 @@ func (s *Service) DeleteLoadBalancerMember(openStackCluster *infrav1.OpenStackCl
 
 		if lbMember != nil {
 			// lb member changed so let's delete it so we can create it again with the correct IP
-			err = s.waitForLoadBalancerActive(lbID)
+			_, err = s.waitForLoadBalancerActive(lbID)
 			if err != nil {
 				return err
 			}
 			if err := s.loadbalancerClient.DeletePoolMember(pool.ID, lbMember.ID); err != nil {
 				return err
 			}
-			err = s.waitForLoadBalancerActive(lbID)
+			_, err = s.waitForLoadBalancerActive(lbID)
 			if err != nil {
 				return err
 			}
@@ -681,15 +750,22 @@ var backoff = wait.Backoff{
 }
 
 // Possible LoadBalancer states are documented here: https://docs.openstack.org/api-ref/load-balancer/v2/index.html#prov-status
-func (s *Service) waitForLoadBalancerActive(id string) error {
+func (s *Service) waitForLoadBalancerActive(id string) (*loadbalancers.LoadBalancer, error) {
+	var lb *loadbalancers.LoadBalancer
+
 	s.scope.Logger().Info("Waiting for load balancer", "id", id, "targetStatus", "ACTIVE")
-	return wait.ExponentialBackoff(backoff, func() (bool, error) {
-		lb, err := s.loadbalancerClient.GetLoadBalancer(id)
+	err := wait.ExponentialBackoff(backoff, func() (bool, error) {
+		var err error
+		lb, err = s.loadbalancerClient.GetLoadBalancer(id)
 		if err != nil {
 			return false, err
 		}
 		return lb.ProvisioningStatus == loadBalancerProvisioningStatusActive, nil
 	})
+	if err != nil {
+		return nil, err
+	}
+	return lb, nil
 }
 
 func (s *Service) waitForListener(id, target string) error {



Test patch:
diff --git a/pkg/cloud/services/loadbalancer/loadbalancer_test.go b/pkg/cloud/services/loadbalancer/loadbalancer_test.go
index d4a6b69a..0c9996ca 100644
--- a/pkg/cloud/services/loadbalancer/loadbalancer_test.go
+++ b/pkg/cloud/services/loadbalancer/loadbalancer_test.go
@@ -18,6 +18,7 @@ package loadbalancer
 
 import (
 	"errors"
+	"fmt"
 	"net"
 	"testing"
 
@@ -37,6 +38,8 @@ import (
 	"sigs.k8s.io/cluster-api-provider-openstack/pkg/scope"
 )
 
+const apiHostname = "api.test-cluster.test"
+
 func Test_ReconcileLoadBalancer(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()
@@ -45,7 +48,7 @@ func Test_ReconcileLoadBalancer(t *testing.T) {
 	lookupHost = func(host string) (addrs string, err error) {
 		if net.ParseIP(host) != nil {
 			return host, nil
-		} else if host == "api.test-cluster.test" {
+		} else if host == apiHostname {
 			ips := []string{"192.168.100.10"}
 			return ips[0], nil
 		}
@@ -56,7 +59,7 @@ func Test_ReconcileLoadBalancer(t *testing.T) {
 		Spec: infrav1.OpenStackClusterSpec{
 			DisableAPIServerFloatingIP: true,
 			ControlPlaneEndpoint: clusterv1.APIEndpoint{
-				Host: "api.test-cluster.test",
+				Host: apiHostname,
 				Port: 6443,
 			},
 		},
@@ -83,13 +86,6 @@ func Test_ReconcileLoadBalancer(t *testing.T) {
 				// add network api call results here
 			},
 			expectLoadBalancer: func(m *mock.MockLbClientMockRecorder) {
-				// return loadbalancer providers
-				providers := []providers.Provider{
-					{Name: "amphora", Description: "The Octavia Amphora driver."},
-					{Name: "octavia", Description: "Deprecated alias of the Octavia Amphora driver."},
-				}
-				m.ListLoadBalancerProviders().Return(providers, nil)
-
 				pendingLB := loadbalancers.LoadBalancer{
 					ID:                 "aaaaaaaa-bbbb-cccc-dddd-333333333333",
 					Name:               "k8s-clusterapi-cluster-AAAAA-kubeapi",
@@ -159,3 +155,349 @@ func Test_ReconcileLoadBalancer(t *testing.T) {
 		})
 	}
 }
+
+func Test_getAPIServerVIPAddress(t *testing.T) {
+	// Stub the call to net.LookupHost
+	lookupHost = func(host string) (addrs string, err error) {
+		if net.ParseIP(host) != nil {
+			return host, nil
+		} else if host == apiHostname {
+			ips := []string{"192.168.100.10"}
+			return ips[0], nil
+		}
+		return "", errors.New("Unknown Host " + host)
+	}
+	tests := []struct {
+		name             string
+		openStackCluster *infrav1.OpenStackCluster
+		want             string
+		wantError        bool
+	}{
+		{
+			name:             "empty cluster returns empty VIP",
+			openStackCluster: &infrav1.OpenStackCluster{},
+			want:             "",
+			wantError:        false,
+		},
+		{
+			name: "API server VIP is InternalIP",
+			openStackCluster: &infrav1.OpenStackCluster{
+				Status: infrav1.OpenStackClusterStatus{
+					APIServerLoadBalancer: &infrav1.LoadBalancer{
+						InternalIP: "1.2.3.4",
+					},
+				},
+			},
+			want:      "1.2.3.4",
+			wantError: false,
+		},
+		{
+			name: "API server VIP is API Server Fixed IP",
+			openStackCluster: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					APIServerFixedIP: "1.2.3.4",
+				},
+			},
+			want:      "1.2.3.4",
+			wantError: false,
+		},
+		{
+			name: "API server VIP with valid control plane endpoint",
+			openStackCluster: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					DisableAPIServerFloatingIP: true,
+					ControlPlaneEndpoint: clusterv1.APIEndpoint{
+						Host: apiHostname,
+						Port: 6443,
+					},
+				},
+			},
+			want:      "192.168.100.10",
+			wantError: false,
+		},
+		{
+			name: "API server VIP with invalid control plane endpoint",
+			openStackCluster: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					DisableAPIServerFloatingIP: true,
+					ControlPlaneEndpoint: clusterv1.APIEndpoint{
+						Host: "invalid-api.test-cluster.test",
+						Port: 6443,
+					},
+				},
+			},
+			wantError: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			got, err := getAPIServerVIPAddress(tt.openStackCluster)
+			if tt.wantError {
+				g.Expect(err).To(HaveOccurred())
+			} else {
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(got).To(Equal(tt.want))
+			}
+		})
+	}
+}
+
+func Test_getAPIServerFloatingIP(t *testing.T) {
+	// Stub the call to net.LookupHost
+	lookupHost = func(host string) (addrs string, err error) {
+		if net.ParseIP(host) != nil {
+			return host, nil
+		} else if host == apiHostname {
+			ips := []string{"192.168.100.10"}
+			return ips[0], nil
+		}
+		return "", errors.New("Unknown Host " + host)
+	}
+	tests := []struct {
+		name             string
+		openStackCluster *infrav1.OpenStackCluster
+		want             string
+		wantError        bool
+	}{
+		{
+			name:             "empty cluster returns empty FIP",
+			openStackCluster: &infrav1.OpenStackCluster{},
+			want:             "",
+			wantError:        false,
+		},
+		{
+			name: "API server FIP is API Server LB IP",
+			openStackCluster: &infrav1.OpenStackCluster{
+				Status: infrav1.OpenStackClusterStatus{
+					APIServerLoadBalancer: &infrav1.LoadBalancer{
+						IP: "1.2.3.4",
+					},
+				},
+			},
+			want:      "1.2.3.4",
+			wantError: false,
+		},
+		{
+			name: "API server FIP is API Server Floating IP",
+			openStackCluster: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					APIServerFloatingIP: "1.2.3.4",
+				},
+			},
+			want:      "1.2.3.4",
+			wantError: false,
+		},
+		{
+			name: "API server FIP with valid control plane endpoint",
+			openStackCluster: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					ControlPlaneEndpoint: clusterv1.APIEndpoint{
+						Host: apiHostname,
+						Port: 6443,
+					},
+				},
+			},
+			want:      "192.168.100.10",
+			wantError: false,
+		},
+		{
+			name: "API server FIP with invalid control plane endpoint",
+			openStackCluster: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					ControlPlaneEndpoint: clusterv1.APIEndpoint{
+						Host: "invalid-api.test-cluster.test",
+						Port: 6443,
+					},
+				},
+			},
+			wantError: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			got, err := getAPIServerFloatingIP(tt.openStackCluster)
+			if tt.wantError {
+				g.Expect(err).To(HaveOccurred())
+			} else {
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(got).To(Equal(tt.want))
+			}
+		})
+	}
+}
+
+func Test_getCanonicalAllowedCIDRs(t *testing.T) {
+	tests := []struct {
+		name             string
+		openStackCluster *infrav1.OpenStackCluster
+		want             []string
+	}{
+		{
+			name:             "allowed CIDRs are empty",
+			openStackCluster: &infrav1.OpenStackCluster{},
+			want:             []string{},
+		},
+		{
+			name: "allowed CIDRs are set",
+			openStackCluster: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					APIServerLoadBalancer: infrav1.APIServerLoadBalancer{
+						AllowedCIDRs: []string{"1.2.3.4/32"},
+					},
+				},
+			},
+			want: []string{"1.2.3.4/32"},
+		},
+		{
+			name: "allowed CIDRs are set with bastion",
+			openStackCluster: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					APIServerLoadBalancer: infrav1.APIServerLoadBalancer{
+						AllowedCIDRs: []string{"1.2.3.4/32"},
+					},
+				},
+				Status: infrav1.OpenStackClusterStatus{
+					Bastion: &infrav1.BastionStatus{
+						FloatingIP: "1.2.3.5",
+						IP:         "192.168.0.1",
+					},
+				},
+			},
+			want: []string{"1.2.3.4/32", "1.2.3.5/32", "192.168.0.1/32"},
+		},
+		{
+			name: "allowed CIDRs are set with network status",
+			openStackCluster: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					APIServerLoadBalancer: infrav1.APIServerLoadBalancer{
+						AllowedCIDRs: []string{"1.2.3.4/32"},
+					},
+				},
+				Status: infrav1.OpenStackClusterStatus{
+					Network: &infrav1.NetworkStatusWithSubnets{
+						Subnets: []infrav1.Subnet{
+							{
+								CIDR: "192.168.0.0/24",
+							},
+						},
+					},
+				},
+			},
+			want: []string{"1.2.3.4/32", "192.168.0.0/24"},
+		},
+		{
+			name: "allowed CIDRs are set with network status and router IP",
+			openStackCluster: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					APIServerLoadBalancer: infrav1.APIServerLoadBalancer{
+						AllowedCIDRs: []string{"1.2.3.4/32"},
+					},
+				},
+				Status: infrav1.OpenStackClusterStatus{
+					Network: &infrav1.NetworkStatusWithSubnets{
+						Subnets: []infrav1.Subnet{
+							{
+								CIDR: "192.168.0.0/24",
+							},
+						},
+					},
+					Router: &infrav1.Router{
+						IPs: []string{"1.2.3.5"},
+					},
+				},
+			},
+			want: []string{"1.2.3.4/32", "1.2.3.5/32", "192.168.0.0/24"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			got := getCanonicalAllowedCIDRs(tt.openStackCluster)
+			g.Expect(got).To(Equal(tt.want))
+		})
+	}
+}
+
+func Test_getOrCreateAPILoadBalancer(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+
+	octaviaProviders := []providers.Provider{
+		{
+			Name: "ovn",
+		},
+	}
+	lbtests := []struct {
+		name               string
+		openStackCluster   *infrav1.OpenStackCluster
+		expectLoadBalancer func(m *mock.MockLbClientMockRecorder)
+		want               *loadbalancers.LoadBalancer
+		wantError          error
+	}{
+		{
+			name:             "nothing exists",
+			openStackCluster: &infrav1.OpenStackCluster{},
+			expectLoadBalancer: func(m *mock.MockLbClientMockRecorder) {
+				m.ListLoadBalancers(gomock.Any()).Return([]loadbalancers.LoadBalancer{}, nil)
+			},
+			want:      &loadbalancers.LoadBalancer{},
+			wantError: fmt.Errorf("network is not yet available in OpenStackCluster.Status"),
+		},
+		{
+			name:             "loadbalancer already exists",
+			openStackCluster: &infrav1.OpenStackCluster{},
+			expectLoadBalancer: func(m *mock.MockLbClientMockRecorder) {
+				m.ListLoadBalancers(gomock.Any()).Return([]loadbalancers.LoadBalancer{{ID: "AAAAA"}}, nil)
+			},
+			want: &loadbalancers.LoadBalancer{
+				ID: "AAAAA",
+			},
+		},
+		{
+			name: "loadbalancer created",
+			openStackCluster: &infrav1.OpenStackCluster{
+				Status: infrav1.OpenStackClusterStatus{
+					Network: &infrav1.NetworkStatusWithSubnets{
+						Subnets: []infrav1.Subnet{
+							{ID: "aaaaaaaa-bbbb-cccc-dddd-222222222222"},
+							{ID: "aaaaaaaa-bbbb-cccc-dddd-333333333333"},
+						},
+					},
+				},
+			},
+			expectLoadBalancer: func(m *mock.MockLbClientMockRecorder) {
+				m.ListLoadBalancers(gomock.Any()).Return([]loadbalancers.LoadBalancer{}, nil)
+				m.ListLoadBalancerProviders().Return(octaviaProviders, nil)
+				m.CreateLoadBalancer(gomock.Any()).Return(&loadbalancers.LoadBalancer{
+					ID: "AAAAA",
+				}, nil)
+			},
+			want: &loadbalancers.LoadBalancer{
+				ID: "AAAAA",
+			},
+		},
+	}
+	for _, tt := range lbtests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			mockScopeFactory := scope.NewMockScopeFactory(mockCtrl, "", logr.Discard())
+			lbs, err := NewService(mockScopeFactory)
+			g.Expect(err).NotTo(HaveOccurred())
+
+			tt.expectLoadBalancer(mockScopeFactory.LbClient.EXPECT())
+			lb, err := lbs.getOrCreateAPILoadBalancer(tt.openStackCluster, "AAAAA")
+			if tt.wantError != nil {
+				g.Expect(err).To(MatchError(tt.wantError))
+			} else {
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(lb).To(Equal(tt.want))
+			}
+		})
+	}
+}