Skip to content

Commit

Permalink
Merge pull request #230 from abays/default_provserver_ports
Browse files Browse the repository at this point in the history
[OSPRH-11494] Make auto port assignment for OSProvServer cluster-scoped
  • Loading branch information
openshift-merge-bot[bot] authored Nov 18, 2024
2 parents d69076b + 722219c commit a50471a
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 23 deletions.
11 changes: 2 additions & 9 deletions api/v1beta1/openstackprovisionserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions api/v1beta1/openstackprovisionserver_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
10 changes: 10 additions & 0 deletions api/v1beta1/openstackprovisionserver_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
23 changes: 12 additions & 11 deletions controllers/openstackbaremetalset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 0 additions & 3 deletions pkg/openstackprovisionserver/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

0 comments on commit a50471a

Please sign in to comment.