Merge pull request openstack-k8s-operators#402 from abays/prevent_port_0

Prevent port 0 from being persisted or causing phantom conflicts

Author: openshift-merge-bot[bot]

api/v1beta1/openstackprovisionserver.go

@@ -25,6 +25,9 @@ func GetExistingProvServerPorts(
 	}
 
 	for _, provServer := range provServerList.Items {
+		if provServer.Spec.Port == 0 {
+			continue
+		}
 		namespacedName := types.NamespacedName{
 			Namespace: provServer.Namespace,
 			Name:      provServer.Name}

api/v1beta1/openstackprovisionserver_types.go

@@ -57,7 +57,7 @@ type OpenStackProvisionServerSpec struct {
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:validation:Minimum=6190
 	// +kubebuilder:validation:Maximum=6220
-	Port int32 `json:"port,omitempty"`
+	Port int32 `json:"port"`
 	// +kubebuilder:validation:Optional
 	// Interface - An optional interface to use instead of the cluster's default provisioning interface (if any)
 	Interface string `json:"interface,omitempty"`

api/v1beta1/openstackprovisionserver_webhook.go

@@ -72,7 +72,10 @@ func (r *OpenStackProvisionServer) ValidateUpdate(old runtime.Object) (admission
 }
 
 func (r *OpenStackProvisionServer) validateCr() error {
-	// TODO: Create a specific context here instead of passing TODO()?
+	if r.Spec.Port == 0 {
+		return fmt.Errorf("no available ports in range %d-%d", ProvisionServerPortStart, ProvisionServerPortEnd)
+	}
+
 	existingPorts, err := GetExistingProvServerPorts(context.TODO(), webhookClient, r)
 	if err != nil {
 		return err

config/samples/baremetal_v1beta1_openstackprovisionserver.yaml

@@ -4,5 +4,4 @@ metadata:
   name: openstackprovisionserver
 spec:
   interface: enp1s0
-  port: 8080
   osImage: edpm-hardened-uefi.qcow2

tests/functional/openstackprovisionserver_webhook_test.go

@@ -17,9 +17,11 @@ package functional
 
 import (
 	"errors"
+	"fmt"
 
 	. "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports
 	. "github.com/onsi/gomega"    //revive:disable:dot-imports
+	baremetalv1 "github.com/openstack-k8s-operators/openstack-baremetal-operator/api/v1beta1"
 	k8s_errors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
@@ -127,6 +129,23 @@ var _ = Describe("OpenStackProvisionServer Webhook", func() {
 		})
 	})
 
+	When("Creating ProvisionServer with port 0 (auto-assign)", func() {
+		It("should auto-assign a valid port via the defaulting webhook", func() {
+			spec := map[string]interface{}{
+				"osImage":             "edpm-hardened-uefi.qcow2",
+				"osContainerImageUrl": "quay.io/podified-antelope-centos9/edpm-hardened-uefi:current-podified",
+				"apacheImageUrl":      "registry.redhat.io/ubi9/httpd-24:latest",
+				"agentImageUrl":       "quay.io/openstack-k8s-operators/openstack-baremetal-operator-agent:latest",
+				"port":                int64(0),
+			}
+			DeferCleanup(th.DeleteInstance, CreateProvisionServer(provisionServerName, spec))
+
+			instance := GetProvisionServerDirect(provisionServerName)
+			Expect(instance.Spec.Port).Should(BeNumerically(">=", baremetalv1.ProvisionServerPortStart))
+			Expect(instance.Spec.Port).Should(BeNumerically("<=", baremetalv1.ProvisionServerPortEnd))
+		})
+	})
+
 	When("Creating ProvisionServer with duplicate port", func() {
 		var provisionServer2Name types.NamespacedName
 
@@ -204,4 +223,43 @@ var _ = Describe("OpenStackProvisionServer Webhook", func() {
 			Expect(instance.Spec.AgentImageURL).ShouldNot(BeEmpty())
 		})
 	})
+
+	When("Port range is exhausted", func() {
+		BeforeEach(func() {
+			spec := map[string]interface{}{
+				"osImage":             "edpm-hardened-uefi.qcow2",
+				"osContainerImageUrl": "quay.io/podified-antelope-centos9/edpm-hardened-uefi:current-podified",
+				"apacheImageUrl":      "registry.redhat.io/ubi9/httpd-24:latest",
+				"agentImageUrl":       "quay.io/openstack-k8s-operators/openstack-baremetal-operator-agent:latest",
+			}
+			portCount := baremetalv1.ProvisionServerPortEnd - baremetalv1.ProvisionServerPortStart + 1
+			for i := 0; i < int(portCount); i++ {
+				name := types.NamespacedName{
+					Name:      fmt.Sprintf("provserver-exhaust-%d", i),
+					Namespace: namespace,
+				}
+				DeferCleanup(th.DeleteInstance, CreateProvisionServer(name, spec))
+			}
+		})
+
+		It("should reject creation when no ports are available", func() {
+			raw := map[string]interface{}{
+				"apiVersion": "baremetal.openstack.org/v1beta1",
+				"kind":       "OpenStackProvisionServer",
+				"metadata": map[string]interface{}{
+					"name":      "provserver-over-limit",
+					"namespace": namespace,
+				},
+				"spec": map[string]interface{}{
+					"osImage":             "edpm-hardened-uefi.qcow2",
+					"osContainerImageUrl": "quay.io/podified-antelope-centos9/edpm-hardened-uefi:current-podified",
+					"apacheImageUrl":      "registry.redhat.io/ubi9/httpd-24:latest",
+					"agentImageUrl":       "quay.io/openstack-k8s-operators/openstack-baremetal-operator-agent:latest",
+				},
+			}
+			unstructuredObj := &unstructured.Unstructured{Object: raw}
+			err := k8sClient.Create(ctx, unstructuredObj)
+			Expect(err).Should(HaveOccurred())
+		})
+	})
 })