From 722219c86e954d8bae16d06d438e92d9264eb4c3 Mon Sep 17 00:00:00 2001 From: Andrew Bays Date: Thu, 14 Nov 2024 16:29:25 +0000 Subject: [PATCH] [OSPRH-11494] Make auto port assignment for OSProvServer cluster-scoped --- api/v1beta1/openstackprovisionserver.go | 11 ++------- api/v1beta1/openstackprovisionserver_types.go | 2 ++ .../openstackprovisionserver_webhook.go | 10 ++++++++ .../openstackbaremetalset_controller.go | 23 ++++++++++--------- pkg/openstackprovisionserver/const.go | 3 --- 5 files changed, 26 insertions(+), 23 deletions(-) diff --git a/api/v1beta1/openstackprovisionserver.go b/api/v1beta1/openstackprovisionserver.go index 7bb6b33..3c5a18f 100644 --- 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 index 24ee943..bde8b2a 100644 --- 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 index 5e4782f..fd4c503 100644 --- 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 index 25ab6aa..4936f16 100644 --- 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,16 +502,18 @@ 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 @@ -520,7 +521,7 @@ func (r *OpenStackBaremetalSetReconciler) provisionServerCreateOrUpdate( 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 index ee8ed76..1daf0bd 100644 --- 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" )