[OSPRH-11494] Make auto port assignment for OSProvServer cluster-scoped

openstack-k8s-operators · Nov 15, 2024 · 722219c · 722219c
1 parent d69076b
commit 722219c
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 23 deletions.
diff --git a/api/v1beta1/openstackprovisionserver.go b/api/v1beta1/openstackprovisionserver.go
@@ -7,7 +7,7 @@ import (
 	goClient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-// GetExistingProvServerPorts - Get all ports currently in use by all OpenStackProvisionServers in this namespace
+// GetExistingProvServerPorts - Get all ports currently in use by all OpenStackProvisionServers
 func GetExistingProvServerPorts(
 	ctx context.Context,
 	c goClient.Client,
@@ -17,9 +17,7 @@ func GetExistingProvServerPorts(
 
 	provServerList := &OpenStackProvisionServerList{}
 
-	listOpts := []goClient.ListOption{
-		goClient.InNamespace(instance.Namespace),
-	}
+	listOpts := []goClient.ListOption{}
 
 	err := c.List(ctx, provServerList, listOpts...)
 	if err != nil {
@@ -40,11 +38,6 @@ func AssignProvisionServerPort(
 	instance *OpenStackProvisionServer,
 	portStart int32,
 ) error {
-	if instance.Spec.Port != 0 {
-		// Do nothing, already assigned
-		return nil
-	}
-
 	existingPorts, err := GetExistingProvServerPorts(ctx, c, instance)
 	if err != nil {
 		return err

diff --git a/api/v1beta1/openstackprovisionserver_types.go b/api/v1beta1/openstackprovisionserver_types.go
@@ -35,6 +35,8 @@ const (
 	ProvisioningNetworkDisabled  ProvisioningNetwork = "Disabled"
 	// Checksum job hash
 	ChecksumHash = "checksum"
+	// DefaultProvisionPort - The starting default port for the OpenStackProvisionServer's Apache container
+	DefaultProvisionPort = 6190
 )
 
 const (

diff --git a/api/v1beta1/openstackprovisionserver_webhook.go b/api/v1beta1/openstackprovisionserver_webhook.go
@@ -115,4 +115,14 @@ func (r *OpenStackProvisionServer) Default() {
 	if r.Spec.OSImage == "" {
 		r.Spec.OSImage = openstackProvisionServerDefaults.OSImage
 	}
+	if r.Spec.Port == 0 {
+		err := AssignProvisionServerPort(context.TODO(), webhookClient, r, DefaultProvisionPort)
+		if err != nil {
+			// If this occurs, it will also be caught just after this defaulting webhook in the
+			// validating webhook, because that webhook calls the same underlying function that
+			// checks for the availability of ports.  That will cause the create/update of the
+			// CR to fail and halt moving forward.
+			openstackprovisionserverlog.Error(err, "Cannot assign port for OpenStackProvisionServer", "OpenStackProvisionServer", r)
+		}
+	}
 }
diff --git a/controllers/openstackbaremetalset_controller.go b/controllers/openstackbaremetalset_controller.go
@@ -44,7 +44,6 @@ import (
 	"github.com/openstack-k8s-operators/lib-common/modules/common/util"
 	baremetalv1 "github.com/openstack-k8s-operators/openstack-baremetal-operator/api/v1beta1"
 	"github.com/openstack-k8s-operators/openstack-baremetal-operator/pkg/openstackbaremetalset"
-	openstackprovisionserver "github.com/openstack-k8s-operators/openstack-baremetal-operator/pkg/openstackprovisionserver"
 )
 
 // OpenStackBaremetalSetReconciler reconciles a OpenStackBaremetalSet object
@@ -503,24 +502,26 @@ func (r *OpenStackBaremetalSetReconciler) provisionServerCreateOrUpdate(
 	}
 
 	op, err := controllerutil.CreateOrPatch(ctx, helper.GetClient(), provisionServer, func() error {
-		// Assign the prov server its existing port if this is an update, otherwise pick a new one
+		// Leave the prov server's existing port as-is if this is an update, otherwise pick a new one
 		// based on what is available
-		err := baremetalv1.AssignProvisionServerPort(
-			ctx,
-			helper.GetClient(),
-			provisionServer,
-			openstackprovisionserver.DefaultPort,
-		)
-		if err != nil {
-			return err
+		if provisionServer.Spec.Port == 0 {
+			err := baremetalv1.AssignProvisionServerPort(
+				ctx,
+				helper.GetClient(),
+				provisionServer,
+				baremetalv1.DefaultProvisionPort,
+			)
+			if err != nil {
+				return err
+			}
 		}
 		provisionServer.Spec.OSImage = instance.Spec.OSImage
 		provisionServer.Spec.OSContainerImageURL = instance.Spec.OSContainerImageURL
 		provisionServer.Spec.ApacheImageURL = instance.Spec.ApacheImageURL
 		provisionServer.Spec.AgentImageURL = instance.Spec.AgentImageURL
 		provisionServer.Spec.Interface = instance.Spec.ProvisioningInterface
 
-		err = controllerutil.SetControllerReference(instance, provisionServer, helper.GetScheme())
+		err := controllerutil.SetControllerReference(instance, provisionServer, helper.GetScheme())
 		if err != nil {
 			return err
 		}

diff --git a/pkg/openstackprovisionserver/const.go b/pkg/openstackprovisionserver/const.go
@@ -20,9 +20,6 @@ const (
 	// AppLabel -
 	AppLabel = "osp-provisionserver"
 
-	// DefaultPort - The starting default port for the OpenStackProvisionServer's Apache container
-	DefaultPort = 6190
-
 	// HttpdConfPath -
 	HttpdConfPath = "/usr/local/apache2/conf/httpd.conf"
 )