Merge pull request #1529 from Jeremy-Boyle/backport-ovn-lb-0.7

✨ Backport Provider to release-0.7

Author: k8s-ci-robot

api/v1alpha3/conversion_test.go

@@ -231,6 +231,7 @@ func TestFuzzyConversion(t *testing.T) {
 				v1alpha6Cluster.Spec.AllowAllInClusterTraffic = false
 				v1alpha6Cluster.Spec.DisableAPIServerFloatingIP = false
 				v1alpha6Cluster.Spec.APIServerLoadBalancer.AllowedCIDRs = nil
+				v1alpha6Cluster.Spec.APIServerLoadBalancer.Provider = ""
 				if v1alpha6Cluster.Spec.Bastion != nil {
 					v1alpha6Cluster.Spec.Bastion.Instance.ImageUUID = ""
 					v1alpha6Cluster.Spec.Bastion.Instance.Ports = nil

api/v1alpha4/conversion_test.go

@@ -331,6 +331,8 @@ func TestFuzzyConversion(t *testing.T) {
 
 				v1alpha6Cluster.Spec.APIServerLoadBalancer.AllowedCIDRs = nil
 
+				v1alpha6Cluster.Spec.APIServerLoadBalancer.Provider = ""
+
 				v1alpha6Cluster.Spec.ControlPlaneOmitAvailabilityZone = false
 
 				if v1alpha6Cluster.Spec.Bastion != nil {
@@ -407,6 +409,8 @@ func TestFuzzyConversion(t *testing.T) {
 
 				v1alpha6ClusterTemplate.Spec.Template.Spec.APIServerLoadBalancer.AllowedCIDRs = nil
 
+				v1alpha6ClusterTemplate.Spec.Template.Spec.APIServerLoadBalancer.Provider = ""
+
 				v1alpha6ClusterTemplate.Spec.Template.Spec.ControlPlaneOmitAvailabilityZone = false
 
 				if v1alpha6ClusterTemplate.Spec.Template.Spec.Bastion != nil {

api/v1alpha5/conversion.go

@@ -202,3 +202,8 @@ func Convert_v1alpha6_OpenStackClusterSpec_To_v1alpha5_OpenStackClusterSpec(in *
 	// Our new flag has no equivalent in v1alpha5
 	return autoConvert_v1alpha6_OpenStackClusterSpec_To_v1alpha5_OpenStackClusterSpec(in, out, s)
 }
+
+func Convert_v1alpha6_APIServerLoadBalancer_To_v1alpha5_APIServerLoadBalancer(in *infrav1.APIServerLoadBalancer, out *APIServerLoadBalancer, s conversion.Scope) error {
+	// Provider has been added in v1alpha6 but has no equivalent in v1alpha5
+	return autoConvert_v1alpha6_APIServerLoadBalancer_To_v1alpha5_APIServerLoadBalancer(in, out, s)
+}

api/v1alpha5/zz_generated.conversion.go

@@ -313,4 +313,6 @@ type APIServerLoadBalancer struct {
 	AdditionalPorts []int `json:"additionalPorts,omitempty"`
 	// AllowedCIDRs restrict access to all API-Server listeners to the given address CIDRs.
 	AllowedCIDRs []string `json:"allowedCidrs,omitempty"`
+	// Octavia Provider Used to create load balancer
+	Provider string `json:"provider,omitempty"`
 }

api/v1alpha6/types.go

@@ -4283,6 +4283,9 @@ spec:
                     description: Enabled defines whether a load balancer should be
                       created.
                     type: boolean
+                  provider:
+                    description: Octavia Provider Used to create load balancer
+                    type: string
                 type: object
               apiServerPort:
                 description: APIServerPort is the port on which the listener on the

config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml

@@ -1508,6 +1508,9 @@ spec:
                             description: Enabled defines whether a load balancer should
                               be created.
                             type: boolean
+                          provider:
+                            description: Octavia Provider Used to create load balancer
+                            type: string
                         type: object
                       apiServerPort:
                         description: APIServerPort is the port on which the listener

config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml

@@ -40,9 +40,8 @@ import (
 )
 
 const (
-	networkPrefix               string = "k8s-clusterapi"
-	kubeapiLBSuffix             string = "kubeapi"
-	defaultLoadBalancerProvider string = "amphora"
+	networkPrefix   string = "k8s-clusterapi"
+	kubeapiLBSuffix string = "kubeapi"
 )
 
 const loadBalancerProvisioningStatusActive = "ACTIVE"
@@ -64,13 +63,18 @@ func (s *Service) ReconcileLoadBalancer(openStackCluster *infrav1.OpenStackClust
 		return err
 	}
 
-	// As mostly all LoadBalancer features are only supported on "amphora" we explicitly set the provider
-	// in the LoadBalancer create call to make sure to get the desired features - even if multiple providers exist.
-	var lbProvider string
-	for _, v := range providers {
-		if v.Name == defaultLoadBalancerProvider {
-			lbProvider = v.Name
-			break
+	// 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)
 		}
 	}
 
@@ -124,7 +128,7 @@ func (s *Service) ReconcileLoadBalancer(openStackCluster *infrav1.OpenStackClust
 			return err
 		}
 
-		pool, err := s.getOrCreatePool(openStackCluster, lbPortObjectsName, listener.ID, lb.ID)
+		pool, err := s.getOrCreatePool(openStackCluster, lbPortObjectsName, listener.ID, lb.ID, lb.Provider)
 		if err != nil {
 			return err
 		}
@@ -296,7 +300,7 @@ func validateIPs(openStackCluster *infrav1.OpenStackCluster, definedCIDRs []stri
 	return marshaledCIDRs
 }
 
-func (s *Service) getOrCreatePool(openStackCluster *infrav1.OpenStackCluster, poolName, listenerID, lbID string) (*pools.Pool, error) {
+func (s *Service) getOrCreatePool(openStackCluster *infrav1.OpenStackCluster, poolName, listenerID, lbID, lbProvider string) (*pools.Pool, error) {
 	pool, err := s.checkIfPoolExists(poolName)
 	if err != nil {
 		return nil, err
@@ -308,12 +312,19 @@ func (s *Service) getOrCreatePool(openStackCluster *infrav1.OpenStackCluster, po
 
 	s.scope.Logger.Info(fmt.Sprintf("Creating load balancer pool for listener %q", listenerID), "name", poolName, "lb-id", lbID)
 
+	method := pools.LBMethodRoundRobin
+
+	if lbProvider == "ovn" {
+		method = pools.LBMethodSourceIpPort
+	}
+
 	poolCreateOpts := pools.CreateOpts{
 		Name:       poolName,
 		Protocol:   "TCP",
-		LBMethod:   pools.LBMethodRoundRobin,
+		LBMethod:   method,
 		ListenerID: listenerID,
 	}
+
 	pool, err = s.loadbalancerClient.CreatePool(poolCreateOpts)
 	if err != nil {
 		record.Warnf(openStackCluster, "FailedCreatePool", "Failed to create pool %s: %v", poolName, err)
@@ -351,6 +362,13 @@ func (s *Service) getOrCreateMonitor(openStackCluster *infrav1.OpenStackCluster,
 		Timeout:        5,
 	}
 	monitor, err = s.loadbalancerClient.CreateMonitor(monitorCreateOpts)
+	// Skip creating monitor if it is not supported by Octavia provider
+	if capoerrors.IsNotImplementedError(err) {
+		record.Warnf(openStackCluster, "SkippedCreateMonitor", "Health monitors are not supported with the current Octavia provider.")
+		record.Eventf(openStackCluster, "SkippedCreateMonitor", "Health Monitor is not created as it's not implemented with the current Octavia provider.")
+		return nil
+	}
+
 	if err != nil {
 		record.Warnf(openStackCluster, "FailedCreateMonitor", "Failed to create monitor %s: %v", monitorName, err)
 		return err

pkg/cloud/services/loadbalancer/loadbalancer.go

@@ -84,3 +84,14 @@ func IsConflict(err error) bool {
 
 	return false
 }
+
+func IsNotImplementedError(err error) bool {
+	var errUnexpectedResponseCode gophercloud.ErrUnexpectedResponseCode
+	if errors.As(err, &errUnexpectedResponseCode) {
+		if errUnexpectedResponseCode.Actual == http.StatusNotImplemented {
+			return true
+		}
+	}
+
+	return false
+}