From ba5d0a5552f8bf8c28f2fb563594e200b4e9e1dc Mon Sep 17 00:00:00 2001
From: Emilien Macchi <emacchi@redhat.com>
Date: Tue, 19 Nov 2024 10:11:40 -0500
Subject: [PATCH 01/16] Better conditions for creating Floating IPs

When a floating is created, we need to make sure that
`OpenStackCluster.Spec.DisableExternalNetwork` is not set to `True`.
Otherwise, we'll have a nil pointer error.

* Add a check in `reconcileBastion` to check that external network is
  not disabled before creating the floating IP for the bastion.
* Add a check in `reconcileControlPlaneEndpoint` and
  `reconcileAPIServerLoadBalancer` to check that external
  network is not disabled (alongside the DisableAPIServerFloatingIP
  check) before creating the floating IP for the API server endpoint.
* Add a safeguard in `GetOrCreateFloatingIP` to return a proper error
  (instead of a nil pointer error) when
  `openStackCluster.Status.ExternalNetwork` is nil.
* Add API CEL to check that when DisableExternalNetwork is set and true,
  the bastion (if defined) doesn't have a floating IP defined and also
  that disableAPIServerFloatingIP (when set) is not False.

(cherry picked from commit 5429b4b8b80a59795846e969cef4da4937ce5530)
---
 api/v1beta1/openstackcluster_types.go         |  2 ++
 ...re.cluster.x-k8s.io_openstackclusters.yaml | 10 ++++++++
 ...er.x-k8s.io_openstackclustertemplates.yaml | 10 ++++++++
 controllers/openstackcluster_controller.go    | 10 +++++---
 controllers/openstackmachine_controller.go    |  2 +-
 .../src/clusteropenstack/configuration.md     |  4 ++++
 pkg/cloud/services/networking/floatingip.go   |  5 ++++
 .../apivalidations/openstackcluster_test.go   | 23 +++++++++++++++++++
 8 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/api/v1beta1/openstackcluster_types.go b/api/v1beta1/openstackcluster_types.go
index d6745e5ccd..5da4d77111 100644
--- a/api/v1beta1/openstackcluster_types.go
+++ b/api/v1beta1/openstackcluster_types.go
@@ -31,6 +31,8 @@ const (
 )
 
 // OpenStackClusterSpec defines the desired state of OpenStackCluster.
+// +kubebuilder:validation:XValidation:rule="has(self.disableExternalNetwork) && self.disableExternalNetwork ? !has(self.bastion) || !has(self.bastion.floatingIP) : true",message="bastion floating IP cannot be set when disableExternalNetwork is true"
+// +kubebuilder:validation:XValidation:rule="has(self.disableExternalNetwork) && self.disableExternalNetwork ? has(self.disableAPIServerFloatingIP) && self.disableAPIServerFloatingIP : true",message="disableAPIServerFloatingIP cannot be false when disableExternalNetwork is true"
 type OpenStackClusterSpec struct {
 	// ManagedSubnets describe OpenStack Subnets to be created. Cluster actuator will create a network,
 	// subnets with the defined CIDR, and a router connected to these subnets. Currently only one IPv4
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml
index fd240c51f5..80778a76da 100644
--- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml
+++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml
@@ -6570,6 +6570,16 @@ spec:
             required:
             - identityRef
             type: object
+            x-kubernetes-validations:
+            - message: bastion floating IP cannot be set when disableExternalNetwork
+                is true
+              rule: 'has(self.disableExternalNetwork) && self.disableExternalNetwork
+                ? !has(self.bastion) || !has(self.bastion.floatingIP) : true'
+            - message: disableAPIServerFloatingIP cannot be false when disableExternalNetwork
+                is true
+              rule: 'has(self.disableExternalNetwork) && self.disableExternalNetwork
+                ? has(self.disableAPIServerFloatingIP) && self.disableAPIServerFloatingIP
+                : true'
           status:
             description: OpenStackClusterStatus defines the observed state of OpenStackCluster.
             properties:
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml
index 96680265b6..3155b8c054 100644
--- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml
+++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml
@@ -4027,6 +4027,16 @@ spec:
                     required:
                     - identityRef
                     type: object
+                    x-kubernetes-validations:
+                    - message: bastion floating IP cannot be set when disableExternalNetwork
+                        is true
+                      rule: 'has(self.disableExternalNetwork) && self.disableExternalNetwork
+                        ? !has(self.bastion) || !has(self.bastion.floatingIP) : true'
+                    - message: disableAPIServerFloatingIP cannot be false when disableExternalNetwork
+                        is true
+                      rule: 'has(self.disableExternalNetwork) && self.disableExternalNetwork
+                        ? has(self.disableAPIServerFloatingIP) && self.disableAPIServerFloatingIP
+                        : true'
                 required:
                 - spec
                 type: object
diff --git a/controllers/openstackcluster_controller.go b/controllers/openstackcluster_controller.go
index 7429958805..96846f58ca 100644
--- a/controllers/openstackcluster_controller.go
+++ b/controllers/openstackcluster_controller.go
@@ -507,7 +507,11 @@ func reconcileBastion(scope *scope.WithLogger, cluster *clusterv1.Cluster, openS
 		return nil, err
 	}
 
-	return bastionAddFloatingIP(openStackCluster, clusterResourceName, port, networkingService)
+	if !ptr.Deref(openStackCluster.Spec.DisableExternalNetwork, false) {
+		return bastionAddFloatingIP(openStackCluster, clusterResourceName, port, networkingService)
+	}
+
+	return nil, nil
 }
 
 func bastionAddFloatingIP(openStackCluster *infrav1.OpenStackCluster, clusterResourceName string, port *ports.Port, networkingService *networking.Service) (*reconcile.Result, error) {
@@ -841,9 +845,9 @@ func reconcileControlPlaneEndpoint(scope *scope.WithLogger, networkingService *n
 	case openStackCluster.Spec.ControlPlaneEndpoint != nil && openStackCluster.Spec.ControlPlaneEndpoint.IsValid():
 		host = openStackCluster.Spec.ControlPlaneEndpoint.Host
 
-	// API server load balancer is disabled, but floating IP is not. Create
+	// API server load balancer is disabled, but external netowork and floating IP are not. Create
 	// a floating IP to be attached directly to a control plane host.
-	case !ptr.Deref(openStackCluster.Spec.DisableAPIServerFloatingIP, false):
+	case !ptr.Deref(openStackCluster.Spec.DisableAPIServerFloatingIP, false) && !ptr.Deref(openStackCluster.Spec.DisableExternalNetwork, false):
 		fp, err := networkingService.GetOrCreateFloatingIP(openStackCluster, openStackCluster, clusterResourceName, openStackCluster.Spec.APIServerFloatingIP)
 		if err != nil {
 			handleUpdateOSCError(openStackCluster, fmt.Errorf("floating IP cannot be got or created: %w", err))
diff --git a/controllers/openstackmachine_controller.go b/controllers/openstackmachine_controller.go
index 230ecd5342..8638b72f5a 100644
--- a/controllers/openstackmachine_controller.go
+++ b/controllers/openstackmachine_controller.go
@@ -708,7 +708,7 @@ func (r *OpenStackMachineReconciler) reconcileAPIServerLoadBalancer(scope *scope
 			conditions.MarkFalse(openStackMachine, infrav1.APIServerIngressReadyCondition, infrav1.LoadBalancerMemberErrorReason, clusterv1.ConditionSeverityError, "Reconciling load balancer member failed: %v", err)
 			return fmt.Errorf("reconcile load balancer member: %w", err)
 		}
-	} else if !ptr.Deref(openStackCluster.Spec.DisableAPIServerFloatingIP, false) {
+	} else if !ptr.Deref(openStackCluster.Spec.DisableAPIServerFloatingIP, false) && !ptr.Deref(openStackCluster.Spec.DisableExternalNetwork, false) {
 		var floatingIPAddress *string
 		switch {
 		case openStackCluster.Spec.ControlPlaneEndpoint != nil && openStackCluster.Spec.ControlPlaneEndpoint.IsValid():
diff --git a/docs/book/src/clusteropenstack/configuration.md b/docs/book/src/clusteropenstack/configuration.md
index 193720927e..3a0439c584 100644
--- a/docs/book/src/clusteropenstack/configuration.md
+++ b/docs/book/src/clusteropenstack/configuration.md
@@ -270,6 +270,8 @@ associated to the first controller node and the other controller nodes have no f
 to any other controller node. So we recommend to only set one controller node when floating IP is needed,
 or please consider using load balancer instead, see [issue #1265](https://github.com/kubernetes-sigs/cluster-api-provider-openstack/issues/1265) for further information.
 
+Note: `spec.disableExternalNetwork` must be unset or set to `false` to allow the API server to have a floating IP.
+
 ### Disabling the API server floating IP
 
 It is possible to provision a cluster without a floating IP for the API server by setting
@@ -715,6 +717,8 @@ spec:
     floatingIP: <Floating IP address>
 ```
 
+Note: A floating IP can only be added if `OpenStackCluster.Spec.DisableExternalNetwork` is not set or set to `false`.
+
 If `managedSecurityGroups` is set to a non-nil value (e.g. `{}`), security group rule opening 22/tcp is added to security groups for bastion, controller, and worker nodes respectively. Otherwise, you have to add `securityGroups` to the `bastion` in `OpenStackCluster` spec and `OpenStackMachineTemplate` spec template respectively.
 
 ### Making changes to the bastion host
diff --git a/pkg/cloud/services/networking/floatingip.go b/pkg/cloud/services/networking/floatingip.go
index 4e611f7fe0..dc474b846d 100644
--- a/pkg/cloud/services/networking/floatingip.go
+++ b/pkg/cloud/services/networking/floatingip.go
@@ -38,6 +38,11 @@ func (s *Service) GetOrCreateFloatingIP(eventObject runtime.Object, openStackClu
 	var err error
 	var fpCreateOpts floatingips.CreateOpts
 
+	// This is a safeguard, we shouldn't reach it and if we do, it's something to fix in the caller of the function.
+	if openStackCluster.Status.ExternalNetwork == nil {
+		return nil, fmt.Errorf("external network not found")
+	}
+
 	if ptr.Deref(ip, "") != "" {
 		fp, err = s.GetFloatingIP(*ip)
 		if err != nil {
diff --git a/test/e2e/suites/apivalidations/openstackcluster_test.go b/test/e2e/suites/apivalidations/openstackcluster_test.go
index 76bcbb40bf..943af0605c 100644
--- a/test/e2e/suites/apivalidations/openstackcluster_test.go
+++ b/test/e2e/suites/apivalidations/openstackcluster_test.go
@@ -169,6 +169,29 @@ var _ = Describe("OpenStackCluster API validations", func() {
 			}
 			Expect(createObj(cluster)).NotTo(Succeed(), "OpenStackCluster creation should not succeed")
 		})
+
+		It("should not allow a bastion floating IP with DisableExternalNetwork set to true", func() {
+			cluster.Spec.Bastion = &infrav1.Bastion{
+				Enabled: ptr.To(true),
+				Spec: &infrav1.OpenStackMachineSpec{
+					Flavor: "flavor-name",
+					Image: infrav1.ImageParam{
+						Filter: &infrav1.ImageFilter{
+							Name: ptr.To("fake-image"),
+						},
+					},
+				},
+				FloatingIP: ptr.To("10.0.0.0"),
+			}
+			cluster.Spec.DisableExternalNetwork = ptr.To(true)
+			Expect(createObj(cluster)).ToNot(Succeed(), "OpenStackCluster creation should not succeed")
+		})
+
+		It("should not allow DisableAPIServerFloatingIP to be false with DisableExternalNetwork set to true", func() {
+			cluster.Spec.DisableAPIServerFloatingIP = ptr.To(false)
+			cluster.Spec.DisableExternalNetwork = ptr.To(true)
+			Expect(createObj(cluster)).ToNot(Succeed(), "OpenStackCluster creation should not succeed")
+		})
 	})
 
 	Context("v1alpha7", func() {

From c62b21f2831e503a9313824da13cbfd7d82039e0 Mon Sep 17 00:00:00 2001
From: Lennart Jern <lennart.jern@est.tech>
Date: Thu, 28 Nov 2024 12:32:25 +0200
Subject: [PATCH 02/16] Fix deletion of cluster when bastion image missing

If the bastion image is missing or ambiguous, we do not have a bastion.
In that case, we must continue with the deletion or be permanently
stuck.

Signed-off-by: Lennart Jern <lennart.jern@est.tech>
---
 controllers/openstackcluster_controller.go | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/controllers/openstackcluster_controller.go b/controllers/openstackcluster_controller.go
index 96846f58ca..484a24c15c 100644
--- a/controllers/openstackcluster_controller.go
+++ b/controllers/openstackcluster_controller.go
@@ -152,9 +152,11 @@ func (r *OpenStackClusterReconciler) reconcileDelete(ctx context.Context, scope
 	// A bastion may have been created if cluster initialisation previously reached populating the network status
 	// We attempt to delete it even if no status was written, just in case
 	if openStackCluster.Status.Network != nil {
-		// Attempt to resolve bastion resources before delete. We don't need to worry about starting if the resources have changed on update.
+		// Attempt to resolve bastion resources before delete.
+		// Even if we fail, we need to continue with the deletion or risk getting stuck.
+		// For example, if the image doesn't exist, we do not have a bastion.
 		if _, err := resolveBastionResources(scope, clusterResourceName, openStackCluster); err != nil {
-			return reconcile.Result{}, err
+			scope.Logger().Info("Failed to resolve bastion, continuing.", "error", err)
 		}
 
 		if err := deleteBastion(scope, cluster, openStackCluster); err != nil {

From 372727961ded59a9d4f66cfda7329786dffc0a1b Mon Sep 17 00:00:00 2001
From: Emilien Macchi <emacchi@redhat.com>
Date: Tue, 26 Nov 2024 15:33:41 -0500
Subject: [PATCH 03/16] Re-work release process

---
 .github/workflows/release.yaml | 44 ++++++++++++++++++++++++++++++++++
 .gitignore                     |  3 +++
 Makefile                       | 42 ++++++++++++++++++++++++++++----
 RELEASE.md                     |  5 +---
 4 files changed, 86 insertions(+), 8 deletions(-)
 create mode 100644 .github/workflows/release.yaml

diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml
new file mode 100644
index 0000000000..5ded0c65ed
--- /dev/null
+++ b/.github/workflows/release.yaml
@@ -0,0 +1,44 @@
+name: release
+
+on:
+  push:
+    # Sequence of patterns matched against refs/tags
+    tags:
+    - 'v*' # Push events to matching v*, i.e. v1.0, v20.15.10
+
+permissions:
+  contents: write # Allow to create a release.
+
+jobs:
+  build:
+    name: create draft release
+    runs-on: ubuntu-latest
+    steps:
+      - name: Set env
+        run:  echo "RELEASE_TAG=${GITHUB_REF:10}" >> $GITHUB_ENV
+      - name: checkout code
+        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # tag=v4.2.2
+        with:
+          fetch-depth: 0
+      - name: Calculate go version
+        run: echo "go_version=$(make go-version)" >> $GITHUB_ENV
+      - name: Set up Go
+        uses: actions/setup-go@41dfa10bad2bb2ae585af6ee5bb4d7d973ad74ed # tag=v5.1.0
+        with:
+          go-version: ${{ env.go_version }}
+      - name: generate release artifacts
+        run: |
+          make release
+      - name: generate release notes
+        # Ignore failures for release-notes generation so they could still get
+        # generated manually before publishing.
+        run: |
+          make generate-release-notes || echo "Failed to generate release notes" >> _releasenotes/${{ env.RELEASE_TAG }}.md
+        env:
+          GH_TOKEN: ${{ github.token }}
+      - name: Release
+        uses: softprops/action-gh-release@01570a1f39cb168c169c802c3bceb9e93fb10974 # tag=v2.1.0
+        with:
+          draft: true
+          files: out/*
+          body_path: _releasenotes/${{ env.RELEASE_TAG }}.md
diff --git a/.gitignore b/.gitignore
index 039565c86f..02582c507b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -74,6 +74,9 @@ cscope.*
 *.test
 /hack/.test-cmd-auth
 
+# Generated release notes
+_releasenotes
+
 # JUnit test output from ginkgo e2e tests
 /junit*.xml
 
diff --git a/Makefile b/Makefile
index 8cc3ec63c2..e352fff5c7 100644
--- a/Makefile
+++ b/Makefile
@@ -25,6 +25,9 @@ export GOTOOLCHAIN=go1.22.8
 export GO111MODULE=on
 unexport GOPATH
 
+# Go
+GO_VERSION ?= 1.22.7
+
 # Directories.
 ARTIFACTS ?= $(REPO_ROOT)/_artifacts
 TOOLS_DIR := hack/tools
@@ -346,8 +349,25 @@ staging-manifests:
 ##@ Release
 ## --------------------------------------
 
+ifneq (,$(findstring -,$(RELEASE_TAG)))                                                                                                 
+    PRE_RELEASE=true                                                                                                                    
+endif                                                                                                                                   
+PREVIOUS_TAG ?= $(shell git tag -l | grep -E "^v[0-9]+\.[0-9]+\.[0-9]+$$" | sort -V | grep -B1 $(RELEASE_TAG) | head -n 1 2>/dev/null)
+## set by Prow, ref name of the base branch, e.g., main
+RELEASE_DIR := out
+RELEASE_NOTES_DIR := _releasenotes
+
+.PHONY: $(RELEASE_DIR)
 $(RELEASE_DIR):
-	mkdir -p $@
+	mkdir -p $(RELEASE_DIR)/
+ 
+.PHONY: $(RELEASE_NOTES_DIR)
+$(RELEASE_NOTES_DIR):
+	mkdir -p $(RELEASE_NOTES_DIR)/
+ 
+.PHONY: $(BUILD_DIR)
+$(BUILD_DIR):
+	@mkdir -p $(BUILD_DIR)
 
 .PHONY: list-staging-releases
 list-staging-releases: ## List staging images for image promotion
@@ -422,9 +442,14 @@ upload-gh-artifacts: $(GH) ## Upload artifacts to Github release
 release-alias-tag: # Adds the tag to the last build tag.
 	gcloud container images add-tag -q $(CONTROLLER_IMG):$(TAG) $(CONTROLLER_IMG):$(RELEASE_ALIAS_TAG)
 
-.PHONY: release-notes
-release-notes: $(RELEASE_NOTES) ## Generate release notes
-	$(RELEASE_NOTES) $(RELEASE_NOTES_ARGS)
+.PHONY: generate-release-notes ## Generate release notes
+generate-release-notes: $(RELEASE_NOTES_DIR) $(RELEASE_NOTES)
+	# Reset the file
+	echo -n > $(RELEASE_NOTES_DIR)/$(RELEASE_TAG).md
+	if [ -n "${PRE_RELEASE}" ]; then \
+	echo -e ":rotating_light: This is a RELEASE CANDIDATE. Use it only for testing purposes. If you find any bugs, file an [issue](https://github.com/kubernetes-sigs/cluster-api-provider-openstack/issues/new/choose).\n" >> $(RELEASE_NOTES_DIR)/$(RELEASE_TAG).md; \
+	fi
+	"$(RELEASE_NOTES)" --from=$(PREVIOUS_TAG) >> $(RELEASE_NOTES_DIR)/$(RELEASE_TAG).md
 
 .PHONY: templates
 templates: ## Generate cluster templates
@@ -533,3 +558,12 @@ compile-e2e: ## Test e2e compilation
 
 .PHONY: FORCE
 FORCE:
+
+## --------------------------------------    
+## Helpers
+## --------------------------------------
+
+##@ helpers:
+
+go-version: ## Print the go version we use to compile our binaries and images
+	@echo $(GO_VERSION)
diff --git a/RELEASE.md b/RELEASE.md
index a86fd79693..df00a8103b 100644
--- a/RELEASE.md
+++ b/RELEASE.md
@@ -70,10 +70,7 @@ The content of the release notes differs depending on the type of release, speci
    for review and the promotion of the image.
 1. Run `make release` to build artifacts to be attached to the GitHub release.
 1. Generate and finalize the release notes and save them for the next step.
-   - Run `make release-notes RELEASE_NOTES_ARGS="--from <tag>"`.
-     - Depending on the type of release, substitute `<tag>` with the following:
-       - Stable releases: tag of the last stable release
-       - Pre-releases*: tag of the latest pre-release (or last stable release if there isn't one)
+   - Run `make release-notes`.
    - Pay close attention to the `## :question: Sort these by hand` section, as it contains items that need to be manually sorted.
 1. Create a draft release in GitHub based on the tag created above
     - Name the release `Release [VERSION]` where VERSION is the full version string.

From 1fa0c34ca3eb2d172ec672af0479ae9cd9a88423 Mon Sep 17 00:00:00 2001
From: Lennart Jern <lennart.jern@est.tech>
Date: Thu, 14 Nov 2024 13:42:35 +0200
Subject: [PATCH 04/16] Add libvirt resource type for create_devstack.sh

This makes it possible to set up a local devstack on libvirt. It is
probably not useful for running the full e2e suite (unless you have a
very beefy PC), but it is enough for development with Tilt.

Signed-off-by: Lennart Jern <lennart.jern@est.tech>
---
 hack/ci/create_devstack.sh |   9 ++-
 hack/ci/libvirt.sh         | 140 +++++++++++++++++++++++++++++++++++++
 2 files changed, 148 insertions(+), 1 deletion(-)
 create mode 100755 hack/ci/libvirt.sh

diff --git a/hack/ci/create_devstack.sh b/hack/ci/create_devstack.sh
index 77ac0f05bf..f238fd23c9 100755
--- a/hack/ci/create_devstack.sh
+++ b/hack/ci/create_devstack.sh
@@ -47,6 +47,9 @@ PRIVATE_NETWORK_CIDR=${PRIVATE_NETWORK_CIDR:-"10.0.3.0/24"}
 CONTROLLER_IP=${CONTROLLER_IP:-"10.0.3.15"}
 WORKER_IP=${WORKER_IP:-"10.0.3.16"}
 
+SKIP_INIT_INFRA=${SKIP_INIT_INFRA:-}
+SKIP_SECONDARY_AZ=${SKIP_SECONDARY_AZ:-}
+
 PRIMARY_AZ=testaz1
 SECONDARY_AZ=testaz2
 
@@ -273,7 +276,11 @@ function main() {
     # is available, and wait if it is not.
     #
     # For efficiency, tests which require multi-AZ SHOULD run as late as possible.
-    create_worker
+    if [[ -n "${SKIP_SECONDARY_AZ:-}" ]]; then
+        echo "Skipping worker creation..."
+    else
+        create_worker
+    fi
 
     public_ip=$(get_public_ip)
     cat << EOF > "${REPO_ROOT_ABSOLUTE}/clouds.yaml"
diff --git a/hack/ci/libvirt.sh b/hack/ci/libvirt.sh
new file mode 100755
index 0000000000..4dc70b29a9
--- /dev/null
+++ b/hack/ci/libvirt.sh
@@ -0,0 +1,140 @@
+#!/usr/bin/env bash
+
+# Copyright 2024 The Kubernetes Authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# 	http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# hack script for preparing libvirt to run cluster-api-provider-openstack e2e
+
+set -x -o errexit -o nounset -o pipefail
+
+# Required environment variables:
+# SSH_PUBLIC_KEY_FILE
+# SSH_PRIVATE_KEY_FILE
+# LIBVIRT_NETWORK_NAME
+
+function cloud_init {
+    LIBVIRT_NETWORK_NAME=${LIBVIRT_NETWORK_NAME:-${CLUSTER_NAME}-network}
+    LIBVIRT_IMAGE_NAME=${LIBVIRT_IMAGE_NAME:-ubuntu-2204-lts}
+
+    LIBVIRT_MEMORY=${LIBVIRT_MEMORY:-8192}
+    LIBVIRT_MEMORY_controller=${LIBVIRT_MEMORY_controller:-$LIBVIRT_MEMORY}
+    LIBVIRT_MEMORY_worker=${LIBVIRT_MEMORY_worker:-$LIBVIRT_MEMORY}
+
+    LIBVIRT_VCPU=${LIBVIRT_VCPU:-4}
+    LIBVIRT_VCPU_controller=${LIBVIRT_VCPU_controller:-$LIBVIRT_VCPU}
+    LIBVIRT_VCPU_worker=${LIBVIRT_VCPU_worker:-$LIBVIRT_VCPU}
+
+    LIBVIRT_MAC_controller="00:60:2f:32:81:00"
+    LIBVIRT_MAC_worker="00:60:2f:32:81:01"
+}
+
+function init_infrastructure() {
+    if ! virsh net-info "${LIBVIRT_NETWORK_NAME}" &>/dev/null; then
+        virsh net-define <(cat <<EOF
+<network>
+  <name>${LIBVIRT_NETWORK_NAME}</name>
+  <forward mode='nat'>
+    <nat>
+      <port start='1024' end='65535'/>
+    </nat>
+  </forward>
+  <bridge name="capobr0" stp="on" delay="0"/>
+  <ip address="${PRIVATE_NETWORK_CIDR%/*}" netmask="255.255.255.0">
+    <dhcp>
+      <range start="${PRIVATE_NETWORK_CIDR%.*}.10" end="${PRIVATE_NETWORK_CIDR%.*}.199"/>
+      <host mac="${LIBVIRT_MAC_controller}" name='controller' ip="${CONTROLLER_IP}"/>
+      <host mac="${LIBVIRT_MAC_worker}" name='worker' ip="${WORKER_IP}"/>
+    </dhcp>
+  </ip>
+</network>
+EOF
+)
+        virsh net-start "${LIBVIRT_NETWORK_NAME}"
+        virsh net-autostart "${LIBVIRT_NETWORK_NAME}"
+    fi
+
+    if [ ! -f "/tmp/${LIBVIRT_IMAGE_NAME}.qcow2" ]; then
+        curl -o "/tmp/${LIBVIRT_IMAGE_NAME}.qcow2" https://cloud-images.ubuntu.com/releases/jammy/release/ubuntu-22.04-server-cloudimg-amd64.img
+    fi
+}
+
+function create_vm {
+    local name=$1 && shift
+    local ip=$1 && shift
+    local userdata=$1 && shift
+    local public=$1 && shift
+
+    local memory=LIBVIRT_MEMORY_${name}
+    memory=${!memory}
+    local vcpu=LIBVIRT_VCPU_${name}
+    vcpu=${!vcpu}
+    local servername="${CLUSTER_NAME}-${name}"
+    local mac=LIBVIRT_MAC_${name}
+    mac=${!mac}
+
+    # Values which weren't initialised if we skipped init_infrastructure. Use names instead.
+    networkid=${networkid:-${LIBVIRT_NETWORK_NAME}}
+    volumeid=${volumeid:-${LIBVIRT_IMAGE_NAME}_${name}.qcow2}
+
+    sudo cp "/tmp/${LIBVIRT_IMAGE_NAME}.qcow2" "/var/lib/libvirt/images/${volumeid}"
+    sudo qemu-img resize "/var/lib/libvirt/images/${volumeid}" +200G
+
+    local serverid
+    local serverid
+    if ! virsh dominfo "${servername}" &>/dev/null; then
+        sudo virt-install \
+            --name "${servername}" \
+            --memory "${memory}" \
+            --vcpus "${vcpu}" \
+            --import \
+            --disk "/var/lib/libvirt/images/${volumeid},format=qcow2,bus=virtio" \
+            --network network="${networkid}",mac="${mac}" \
+            --os-variant=ubuntu22.04 \
+            --graphics none \
+            --cloud-init user-data="${userdata}" \
+            --noautoconsole
+    fi
+}
+
+function get_public_ip {
+    echo "${CONTROLLER_IP}"
+}
+
+function get_mtu {
+    # Set MTU statically for libvirt
+    echo 1500
+}
+
+function get_ssh_public_key_file {
+    echo "${SSH_PUBLIC_KEY_FILE}"
+}
+
+function get_ssh_private_key_file {
+    # Allow this to be unbound. This is handled in create_devstack.sh
+    echo "${SSH_PRIVATE_KEY_FILE:-}"
+}
+
+function cloud_cleanup {
+    for serverid in $(virsh list --all --name | grep -E "${CLUSTER_NAME}-controller|${CLUSTER_NAME}-worker"); do
+        virsh destroy "${serverid}"
+        virsh undefine "${serverid}" --remove-all-storage
+    done
+
+    for networkid in $(virsh net-list --name | grep -E "${CLUSTER_NAME}"); do
+        virsh net-destroy "${networkid}"
+        virsh net-undefine "${networkid}"
+    done
+
+    true
+}

From 523cf06c2a0868ecc560a87bb9fcf986bcf03cc9 Mon Sep 17 00:00:00 2001
From: Lennart Jern <lennart.jern@est.tech>
Date: Wed, 4 Dec 2024 08:39:10 +0200
Subject: [PATCH 05/16] Run devstack on ubuntu 24.04 and bump to 2024.2

This is an attempt to make it more stable.

Signed-off-by: Lennart Jern <lennart.jern@est.tech>
(cherry picked from commit 5a86c5a49d2e145cdd8f72f0663bc05b51e467d1)
---
 hack/ci/create_devstack.sh | 2 +-
 hack/ci/gce-project.sh     | 2 +-
 hack/ci/libvirt.sh         | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hack/ci/create_devstack.sh b/hack/ci/create_devstack.sh
index f238fd23c9..60c8ce7f62 100755
--- a/hack/ci/create_devstack.sh
+++ b/hack/ci/create_devstack.sh
@@ -31,7 +31,7 @@ source "${scriptdir}/${RESOURCE_TYPE}.sh"
 
 CLUSTER_NAME=${CLUSTER_NAME:-"capo-e2e"}
 
-OPENSTACK_RELEASE=${OPENSTACK_RELEASE:-"2023.2"}
+OPENSTACK_RELEASE=${OPENSTACK_RELEASE:-"2024.2"}
 OPENSTACK_ENABLE_HORIZON=${OPENSTACK_ENABLE_HORIZON:-"false"}
 
 # Devstack will create a provider network using this range
diff --git a/hack/ci/gce-project.sh b/hack/ci/gce-project.sh
index 1aae2de9b9..c915f40ce0 100755
--- a/hack/ci/gce-project.sh
+++ b/hack/ci/gce-project.sh
@@ -95,7 +95,7 @@ function create_vm {
         --zone "$GCP_ZONE" \
         --enable-nested-virtualization \
         --image-project ubuntu-os-cloud \
-        --image-family ubuntu-2204-lts \
+        --image-family ubuntu-2404-lts-amd64 \
         --boot-disk-size 200G \
         --boot-disk-type pd-ssd \
         --can-ip-forward \
diff --git a/hack/ci/libvirt.sh b/hack/ci/libvirt.sh
index 4dc70b29a9..d6b8d1ab64 100755
--- a/hack/ci/libvirt.sh
+++ b/hack/ci/libvirt.sh
@@ -25,7 +25,7 @@ set -x -o errexit -o nounset -o pipefail
 
 function cloud_init {
     LIBVIRT_NETWORK_NAME=${LIBVIRT_NETWORK_NAME:-${CLUSTER_NAME}-network}
-    LIBVIRT_IMAGE_NAME=${LIBVIRT_IMAGE_NAME:-ubuntu-2204-lts}
+    LIBVIRT_IMAGE_NAME=${LIBVIRT_IMAGE_NAME:-ubuntu-2404-lts}
 
     LIBVIRT_MEMORY=${LIBVIRT_MEMORY:-8192}
     LIBVIRT_MEMORY_controller=${LIBVIRT_MEMORY_controller:-$LIBVIRT_MEMORY}
@@ -65,7 +65,7 @@ EOF
     fi
 
     if [ ! -f "/tmp/${LIBVIRT_IMAGE_NAME}.qcow2" ]; then
-        curl -o "/tmp/${LIBVIRT_IMAGE_NAME}.qcow2" https://cloud-images.ubuntu.com/releases/jammy/release/ubuntu-22.04-server-cloudimg-amd64.img
+        curl -o "/tmp/${LIBVIRT_IMAGE_NAME}.qcow2" https://cloud-images.ubuntu.com/releases/noble/release/ubuntu-24.04-server-cloudimg-amd64.img
     fi
 }
 

From d3fd177b3c17b1f501451e28a37c365e21fc9237 Mon Sep 17 00:00:00 2001
From: Stephen Finucane <stephenfin@redhat.com>
Date: Thu, 5 Dec 2024 17:04:01 +0000
Subject: [PATCH 06/16] test: Temporarily disable deep image inspection in Nova

While we investigate the issues with flatcar images further [1].

[1] https://bugs.launchpad.net/nova/+bug/2091114

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
(cherry picked from commit 0a28aaeece6005752c59cfcfe429da819d120652)
---
 hack/ci/cloud-init/controller.yaml.tpl | 6 ++++++
 hack/ci/cloud-init/worker.yaml.tpl     | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/hack/ci/cloud-init/controller.yaml.tpl b/hack/ci/cloud-init/controller.yaml.tpl
index a3b8bb23b3..14e6fdef65 100644
--- a/hack/ci/cloud-init/controller.yaml.tpl
+++ b/hack/ci/cloud-init/controller.yaml.tpl
@@ -89,6 +89,12 @@
     # query_placement_for_availability_zone is the default from Xena
     query_placement_for_availability_zone = True
 
+    [workarounds]
+    # FIXME(stephenfin): This is temporary while we get to the bottom of
+    # https://bugs.launchpad.net/nova/+bug/2091114 It should not be kept after
+    # we bump to 2025.1
+    disable_deep_image_inspection = True
+
     [[post-config|$CINDER_CONF]]
     [DEFAULT]
     storage_availability_zone = ${PRIMARY_AZ}
diff --git a/hack/ci/cloud-init/worker.yaml.tpl b/hack/ci/cloud-init/worker.yaml.tpl
index ccbf79411d..6e4a5b44e6 100644
--- a/hack/ci/cloud-init/worker.yaml.tpl
+++ b/hack/ci/cloud-init/worker.yaml.tpl
@@ -47,6 +47,12 @@
     [DEFAULT]
     cpu_allocation_ratio = 2.0
 
+    [workarounds]
+    # FIXME(stephenfin): This is temporary while we get to the bottom of
+    # https://bugs.launchpad.net/nova/+bug/2091114 It should not be kept after
+    # we bump to 2025.1
+    disable_deep_image_inspection = True
+
     [[post-config|$CINDER_CONF]]
     [DEFAULT]
     storage_availability_zone = ${SECONDARY_AZ}

From aee4a46609efbd9bd373133d157d8510910c9f73 Mon Sep 17 00:00:00 2001
From: Emilien Macchi <emacchi@redhat.com>
Date: Mon, 9 Dec 2024 08:32:39 -0500
Subject: [PATCH 07/16] ci/devstack: host tuning

Configuring the following in devstack:

```
ENABLE_SYSCTL_MEM_TUNING="True"
ENABLE_SYSCTL_NET_TUNING="True"
ENABLE_ZSWAP="True"
```

That tuning is what is done in the `devstack-platform-ubuntu-noble`
job.
Example:
https://storage.gra.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_d17/936669/2/check/devstack-platform-ubuntu-noble/d178dad/controller/logs/local_conf.txt
---
 hack/ci/cloud-init/controller.yaml.tpl | 5 +++++
 hack/ci/cloud-init/worker.yaml.tpl     | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/hack/ci/cloud-init/controller.yaml.tpl b/hack/ci/cloud-init/controller.yaml.tpl
index 14e6fdef65..e1c554e63f 100644
--- a/hack/ci/cloud-init/controller.yaml.tpl
+++ b/hack/ci/cloud-init/controller.yaml.tpl
@@ -12,6 +12,11 @@
     VERBOSE=True
     LOG_COLOR=True
 
+    # Host tuning
+    ENABLE_SYSCTL_MEM_TUNING="True"
+    ENABLE_SYSCTL_NET_TUNING="True"
+    ENABLE_ZSWAP="True"
+
     # Octavia
     enable_plugin octavia https://github.com/openstack/octavia stable/${OPENSTACK_RELEASE}
     enable_plugin octavia-dashboard https://github.com/openstack/octavia-dashboard stable/${OPENSTACK_RELEASE}
diff --git a/hack/ci/cloud-init/worker.yaml.tpl b/hack/ci/cloud-init/worker.yaml.tpl
index 6e4a5b44e6..9084b8aad7 100644
--- a/hack/ci/cloud-init/worker.yaml.tpl
+++ b/hack/ci/cloud-init/worker.yaml.tpl
@@ -11,6 +11,11 @@
     VERBOSE=True
     LOG_COLOR=True
 
+    # Host tuning
+    ENABLE_SYSCTL_MEM_TUNING="True"
+    ENABLE_SYSCTL_NET_TUNING="True"
+    ENABLE_ZSWAP="True"
+
     DATABASE_PASSWORD=secretdatabase
     RABBIT_PASSWORD=secretrabbit
     ADMIN_PASSWORD=secretadmin

From e2807f5309e160531ae88b7058fc2a8cdee9e7c6 Mon Sep 17 00:00:00 2001
From: Emilien Macchi <emacchi@redhat.com>
Date: Wed, 11 Dec 2024 10:42:05 -0500
Subject: [PATCH 08/16] devstack: build OVN from source

In this PR we'll control the version of OVN to align with the upstream
devstack jobs building OVN and OVS from LTS versions.
This is largely a workaround to address some timeout issues that we
recently have and that with a controlled version of OVN we hope to
reduce.
---
 hack/ci/cloud-init/controller.yaml.tpl | 8 ++++++++
 hack/ci/cloud-init/worker.yaml.tpl     | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/hack/ci/cloud-init/controller.yaml.tpl b/hack/ci/cloud-init/controller.yaml.tpl
index e1c554e63f..3cd465bb13 100644
--- a/hack/ci/cloud-init/controller.yaml.tpl
+++ b/hack/ci/cloud-init/controller.yaml.tpl
@@ -48,6 +48,14 @@
     OVN_L3_CREATE_PUBLIC_NETWORK="True"
     Q_AGENT="ovn"
 
+    # WORKAROUND:
+    # 	https://github.com/kubernetes-sigs/cluster-api-provider-openstack/issues/2320
+    # 	OVN built from source using LTS versions. Should be removed once OVS is more stable without the pin.
+    # 	https://opendev.org/openstack/neutron/src/commit/83de306105f9329e24c97c4af6c3886de20e7d70/zuul.d/tempest-multinode.yaml#L603-L604
+    OVN_BUILD_FROM_SOURCE=True
+    OVN_BRANCH=branch-24.03
+    OVS_BRANCH=branch-3.3
+
     # Octavia
     ENABLED_SERVICES+=,octavia,o-api,o-cw,o-hm,o-hk,o-da
 
diff --git a/hack/ci/cloud-init/worker.yaml.tpl b/hack/ci/cloud-init/worker.yaml.tpl
index 9084b8aad7..0a34b69a22 100644
--- a/hack/ci/cloud-init/worker.yaml.tpl
+++ b/hack/ci/cloud-init/worker.yaml.tpl
@@ -44,6 +44,14 @@
     Q_ML2_PLUGIN_MECHANISM_DRIVERS="ovn,logger"
     Q_AGENT="ovn"
 
+    # WORKAROUND:
+    # 	https://github.com/kubernetes-sigs/cluster-api-provider-openstack/issues/2320
+    # 	OVN built from source using LTS versions. Should be removed once OVS is more stable without the pin.
+    # 	https://opendev.org/openstack/neutron/src/commit/83de306105f9329e24c97c4af6c3886de20e7d70/zuul.d/tempest-multinode.yaml#L603-L604
+    OVN_BUILD_FROM_SOURCE=True
+    OVN_BRANCH=branch-24.03
+    OVS_BRANCH=branch-3.3
+
     # Additional services
     ENABLED_SERVICES+=${OPENSTACK_ADDITIONAL_SERVICES}
     DISABLED_SERVICES+=${OPENSTACK_DISABLED_SERVICES}

From 39eea8099b153a66beab092449f681f937a591f9 Mon Sep 17 00:00:00 2001
From: Lennart Jern <lennart.jern@est.tech>
Date: Wed, 11 Dec 2024 12:51:05 +0200
Subject: [PATCH 09/16] CI: Increase IOPS for devstack disk

This is an attempt to see if disk performance is what makes our CI
unstable.

Signed-off-by: Lennart Jern <lennart.jern@est.tech>
---
 hack/ci/gce-project.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hack/ci/gce-project.sh b/hack/ci/gce-project.sh
index c915f40ce0..22d4a059ca 100755
--- a/hack/ci/gce-project.sh
+++ b/hack/ci/gce-project.sh
@@ -96,7 +96,7 @@ function create_vm {
         --enable-nested-virtualization \
         --image-project ubuntu-os-cloud \
         --image-family ubuntu-2404-lts-amd64 \
-        --boot-disk-size 200G \
+        --boot-disk-size 500G \
         --boot-disk-type pd-ssd \
         --can-ip-forward \
         --tags http-server,https-server,novnc,openstack-apis \

From 5816690c07c46f8f80dc765006c15614e0a38c4a Mon Sep 17 00:00:00 2001
From: Lennart Jern <lennart.jern@est.tech>
Date: Wed, 11 Dec 2024 12:41:00 +0200
Subject: [PATCH 10/16] Devstack: Make boot volume optional on openstack

This makes it possible to choose if the devstack should be created with
or without volumes when running on openstack. It also bumps the default
image name to ubuntu-2404-lts to match the other resource types.

Signed-off-by: Lennart Jern <lennart.jern@est.tech>
---
 hack/ci/openstack.sh | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/hack/ci/openstack.sh b/hack/ci/openstack.sh
index 49e4f61bcc..f69f66e7a8 100755
--- a/hack/ci/openstack.sh
+++ b/hack/ci/openstack.sh
@@ -24,13 +24,14 @@ set -x -o errexit -o nounset -o pipefail
 # SSH_PRIVATE_KEY_FILE
 # OPENSTACK_PUBLIC_NETWORK
 # OPENSTACK_PUBLIC_IP (optional, will be created on OPENSTACK_PUBLIC_NETWORK if not defined)
+# USE_VOLUMES (optional, default true)
 
 function cloud_init {
     OPENSTACK_NETWORK_NAME=${OPENSTACK_NETWORK_NAME:-${CLUSTER_NAME}-network}
     OPENSTACK_SUBNET_NAME=${OPENSTACK_SUBNET_NAME:-${CLUSTER_NAME}-subnet}
     OPENSTACK_SECGROUP_NAME=${OPENSTACK_SECGROUP_NAME:-${CLUSTER_NAME}-secgroup}
     OPENSTACK_ROUTER_NAME=${OPENSTACK_ROUTER_NAME:-${CLUSTER_NAME}-router}
-    OPENSTACK_IMAGE_NAME=${OPENSTACK_IMAGE_NAME:-ubuntu-2204-lts}
+    OPENSTACK_IMAGE_NAME=${OPENSTACK_IMAGE_NAME:-ubuntu-2404-lts}
 
     OPENSTACK_FLAVOR=${OPENSTACK_FLAVOR:-m1.xlarge}
     OPENSTACK_FLAVOR_controller=${OPENSTACK_FLAVOR_controller:-$OPENSTACK_FLAVOR}
@@ -109,16 +110,21 @@ function create_vm {
     secgroupid=${secgroupid:-${OPENSTACK_SECGROUP_NAME}}
     imageid=${imageid:-${OPENSTACK_IMAGE_NAME}}
 
-    local volumename="${CLUSTER_NAME}-${name}"
-    local volumeid
-    if ! volumeid=$(openstack volume show "$volumename" -f value -c id 2>/dev/null)
-    then
-        volumeid=$(openstack volume create -f value -c id --size 200  \
-            --bootable --image "$imageid" "$volumename")
-        while [ "$(openstack volume show "$volumename" -f value -c status 2>/dev/null)" != "available" ]; do
-            echo "Waiting for volume to become available"
-            sleep 5
-        done
+    local storage_medium_flag="--image=$imageid"
+
+    if [ "${USE_VOLUMES:-true}" == "true" ]; then
+      local volumename="${CLUSTER_NAME}-${name}"
+      local volumeid
+      if ! volumeid=$(openstack volume show "$volumename" -f value -c id 2>/dev/null)
+      then
+          volumeid=$(openstack volume create -f value -c id --size 200  \
+              --bootable --image "$imageid" "$volumename")
+          while [ "$(openstack volume show "$volumename" -f value -c status 2>/dev/null)" != "available" ]; do
+              echo "Waiting for volume to become available"
+              sleep 5
+          done
+      fi
+      storage_medium_flag="--volume=$volumeid"
     fi
 
     local serverid
@@ -126,7 +132,7 @@ function create_vm {
     then
         serverid=$(openstack server create -f value -c id \
             --os-compute-api-version 2.52 --tag "$CLUSTER_NAME" \
-            --flavor "$flavor" --volume "$volumeid" \
+            --flavor "$flavor" "$storage_medium_flag" \
             --nic net-id="$networkid",v4-fixed-ip="$ip" \
             --security-group "$secgroupid" \
             --user-data "$userdata" \

From fb4c8a3b7e5d0250578e780e3567dfae8864ff64 Mon Sep 17 00:00:00 2001
From: Lennart Jern <lennart.jern@est.tech>
Date: Tue, 14 Jan 2025 12:05:00 +0000
Subject: [PATCH 11/16] Update cloudbuild image

The image used in the postsubmit job that builds staging images was outdated.
It had an image with go1.18 and could not build the image.

Signed-off-by: Lennart Jern <lennart.jern@est.tech>
---
 cloudbuild-nightly.yaml | 2 +-
 cloudbuild.yaml         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cloudbuild-nightly.yaml b/cloudbuild-nightly.yaml
index a95860d3ad..14ed94de8e 100644
--- a/cloudbuild-nightly.yaml
+++ b/cloudbuild-nightly.yaml
@@ -4,7 +4,7 @@ options:
   substitution_option: ALLOW_LOOSE
   machineType: 'N1_HIGHCPU_8'
 steps:
-  - name: 'gcr.io/k8s-staging-test-infra/gcb-docker-gcloud:v20220609-2e4c91eb7e'
+  - name: 'gcr.io/k8s-staging-test-infra/gcb-docker-gcloud:v20241229-5dc092c636'
     entrypoint: make
     env:
     - DOCKER_CLI_EXPERIMENTAL=enabled
diff --git a/cloudbuild.yaml b/cloudbuild.yaml
index 919539d8e1..49ec67656a 100644
--- a/cloudbuild.yaml
+++ b/cloudbuild.yaml
@@ -4,7 +4,7 @@ options:
   substitution_option: ALLOW_LOOSE
   machineType: 'N1_HIGHCPU_8'
 steps:
-  - name: 'gcr.io/k8s-staging-test-infra/gcb-docker-gcloud:v20220609-2e4c91eb7e'
+  - name: 'gcr.io/k8s-staging-test-infra/gcb-docker-gcloud:v20241229-5dc092c636'
     entrypoint: make
     env:
     - DOCKER_CLI_EXPERIMENTAL=enabled

From 1f1136fde333dd65cfb43c4ad8c6500e850fdb25 Mon Sep 17 00:00:00 2001
From: Simon Ostendorf <simon.ostendorf@cancom.de>
Date: Wed, 18 Dec 2024 10:50:45 +0100
Subject: [PATCH 12/16] fix: create lbaas in specified subnet

Signed-off-by: Lennart Jern <lennart.jern@est.tech>
---
 controllers/openstackcluster_controller.go      | 13 ++++++++++---
 pkg/cloud/services/loadbalancer/loadbalancer.go |  5 +++--
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/controllers/openstackcluster_controller.go b/controllers/openstackcluster_controller.go
index 484a24c15c..3251a95b40 100644
--- a/controllers/openstackcluster_controller.go
+++ b/controllers/openstackcluster_controller.go
@@ -666,14 +666,19 @@ func resolveLoadBalancerNetwork(openStackCluster *infrav1.OpenStackCluster, netw
 
 			// Filter out only relevant subnets specified by the spec
 			lbNetStatus.Subnets = []infrav1.Subnet{}
-			for _, s := range lbSpec.Subnets {
+			for i := range lbSpec.Subnets {
+				s := lbSpec.Subnets[i]
 				matchFound := false
 				for _, subnetID := range lbNet.Subnets {
-					if s.ID != nil && subnetID == *s.ID {
+					subnet, err := networkingService.GetSubnetByParam(&s)
+					if s.ID != nil && subnetID == *s.ID && err == nil {
 						matchFound = true
 						lbNetStatus.Subnets = append(
 							lbNetStatus.Subnets, infrav1.Subnet{
-								ID: *s.ID,
+								ID:   subnet.ID,
+								Name: subnet.Name,
+								CIDR: subnet.CIDR,
+								Tags: subnet.Tags,
 							})
 					}
 				}
@@ -682,6 +687,8 @@ func resolveLoadBalancerNetwork(openStackCluster *infrav1.OpenStackCluster, netw
 					return fmt.Errorf("no subnet match was found in the specified network (specified subnet: %v, available subnets: %v)", s, lbNet.Subnets)
 				}
 			}
+
+			openStackCluster.Status.APIServerLoadBalancer.LoadBalancerNetwork = lbNetStatus
 		}
 	}
 
diff --git a/pkg/cloud/services/loadbalancer/loadbalancer.go b/pkg/cloud/services/loadbalancer/loadbalancer.go
index ccdca0c0d9..91e972ee7d 100644
--- a/pkg/cloud/services/loadbalancer/loadbalancer.go
+++ b/pkg/cloud/services/loadbalancer/loadbalancer.go
@@ -294,10 +294,11 @@ func (s *Service) getOrCreateAPILoadBalancer(openStackCluster *infrav1.OpenStack
 	if vipNetworkID == "" && vipSubnetID == "" {
 		// keep the default and create the VIP on the first cluster subnet
 		vipSubnetID = openStackCluster.Status.Network.Subnets[0].ID
+		s.scope.Logger().Info("No load balancer network specified, creating load balancer in the default subnet", "subnetID", vipSubnetID, "name", loadBalancerName)
+	} else {
+		s.scope.Logger().Info("Creating load balancer in subnet", "subnetID", vipSubnetID, "name", loadBalancerName)
 	}
 
-	s.scope.Logger().Info("Creating load balancer in subnet", "subnetID", vipSubnetID, "name", loadBalancerName)
-
 	providers, err := s.loadbalancerClient.ListLoadBalancerProviders()
 	if err != nil {
 		return nil, err

From c09d580ea7e0d2a5f2d37efb948eade7d63c049a Mon Sep 17 00:00:00 2001
From: Emilien Macchi <emacchi@redhat.com>
Date: Tue, 14 Jan 2025 08:22:03 -0500
Subject: [PATCH 13/16] Update OWNERS_ALIASES

---
 OWNERS_ALIASES | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/OWNERS_ALIASES b/OWNERS_ALIASES
index 3c9d48a297..d2efce1b97 100644
--- a/OWNERS_ALIASES
+++ b/OWNERS_ALIASES
@@ -20,7 +20,8 @@ aliases:
     - vincepri
   cluster-api-openstack-maintainers:
     - emilienm
-    - jichenjc
     - lentzi90
     - mdbooth
   cluster-api-openstack-reviewers:
+  cluster-api-openstack-emeritus-maintainers:
+    - jichenjc

From 68e7b13c236f5af71e997f078511720888ce9f4e Mon Sep 17 00:00:00 2001
From: Lennart Jern <lennart.jern@est.tech>
Date: Fri, 24 Jan 2025 11:05:01 +0000
Subject: [PATCH 14/16] Ensure that existing ports also have correct tags and
 trunks

If port creation fails in the middle, and cleanup also fails,
then we may end up with ports with missing tags or trunks.
This could happen when hitting rate-limits for example or
if there is a network outage. This commit addresses the issue
by going through existing ports and ensuring that they have
correct tags and trunks.

Co-authored-by: Huy Mai <huy.mai@est.tech>
Signed-off-by: Lennart Jern <lennart.jern@est.tech>
---
 controllers/openstackcluster_controller.go    |   6 +-
 .../openstackcluster_controller_test.go       |   6 +
 controllers/openstackmachine_controller.go    |   6 +-
 pkg/cloud/services/networking/port.go         | 117 ++++++++++++++----
 pkg/cloud/services/networking/port_test.go    | 112 ++++++++++++++++-
 pkg/cloud/services/networking/service.go      |  18 +--
 test/e2e/data/e2e_conf.yaml                   |   1 -
 7 files changed, 207 insertions(+), 59 deletions(-)

diff --git a/controllers/openstackcluster_controller.go b/controllers/openstackcluster_controller.go
index 3251a95b40..9ec63477bc 100644
--- a/controllers/openstackcluster_controller.go
+++ b/controllers/openstackcluster_controller.go
@@ -616,11 +616,7 @@ func getOrCreateBastionPorts(openStackCluster *infrav1.OpenStackCluster, network
 		return errors.New("bastion resources are nil")
 	}
 
-	if len(desiredPorts) == len(resources.Ports) {
-		return nil
-	}
-
-	err := networkingService.CreatePorts(openStackCluster, desiredPorts, resources)
+	err := networkingService.EnsurePorts(openStackCluster, desiredPorts, resources)
 	if err != nil {
 		return fmt.Errorf("failed to create ports for bastion %s: %w", bastionName(openStackCluster.Name), err)
 	}
diff --git a/controllers/openstackcluster_controller_test.go b/controllers/openstackcluster_controller_test.go
index 21dc0dbe3d..362794e13a 100644
--- a/controllers/openstackcluster_controller_test.go
+++ b/controllers/openstackcluster_controller_test.go
@@ -277,6 +277,8 @@ var _ = Describe("OpenStackCluster controller", func() {
 		server.Status = "ACTIVE"
 
 		networkClientRecorder := mockScopeFactory.NetworkClient.EXPECT()
+		// One list for adopting and one for ensuring the ports and tags are correct
+		networkClientRecorder.ListPort(gomock.Any()).Return([]ports.Port{{ID: "portID1"}}, nil)
 		networkClientRecorder.ListPort(gomock.Any()).Return([]ports.Port{{ID: "portID1"}}, nil)
 
 		computeClientRecorder := mockScopeFactory.ComputeClient.EXPECT()
@@ -362,6 +364,7 @@ var _ = Describe("OpenStackCluster controller", func() {
 
 		networkClientRecorder := mockScopeFactory.NetworkClient.EXPECT()
 		networkClientRecorder.ListPort(gomock.Any()).Return([]ports.Port{{ID: "portID1"}}, nil)
+		networkClientRecorder.ListPort(gomock.Any()).Return([]ports.Port{{ID: "portID1"}}, nil)
 
 		computeClientRecorder := mockScopeFactory.ComputeClient.EXPECT()
 		computeClientRecorder.GetServer("adopted-fip-bastion-uuid").Return(&server, nil)
@@ -445,6 +448,9 @@ var _ = Describe("OpenStackCluster controller", func() {
 		computeClientRecorder := mockScopeFactory.ComputeClient.EXPECT()
 		computeClientRecorder.GetServer("requeue-bastion-uuid").Return(&server, nil)
 
+		networkClientRecorder := mockScopeFactory.NetworkClient.EXPECT()
+		networkClientRecorder.ListPort(gomock.Any()).Return([]ports.Port{{ID: "portID1"}}, nil)
+
 		res, err := reconcileBastion(scope, capiCluster, testCluster)
 		Expect(testCluster.Status.Bastion).To(Equal(&infrav1.BastionStatus{
 			ID:    "requeue-bastion-uuid",
diff --git a/controllers/openstackmachine_controller.go b/controllers/openstackmachine_controller.go
index 8638b72f5a..81faaaaef7 100644
--- a/controllers/openstackmachine_controller.go
+++ b/controllers/openstackmachine_controller.go
@@ -752,11 +752,7 @@ func getOrCreateMachinePorts(openStackMachine *infrav1.OpenStackMachine, network
 	}
 	desiredPorts := resolved.Ports
 
-	if len(desiredPorts) == len(resources.Ports) {
-		return nil
-	}
-
-	if err := networkingService.CreatePorts(openStackMachine, desiredPorts, resources); err != nil {
+	if err := networkingService.EnsurePorts(openStackMachine, desiredPorts, resources); err != nil {
 		return fmt.Errorf("creating ports: %w", err)
 	}
 
diff --git a/pkg/cloud/services/networking/port.go b/pkg/cloud/services/networking/port.go
index 0c86e7854b..310685c966 100644
--- a/pkg/cloud/services/networking/port.go
+++ b/pkg/cloud/services/networking/port.go
@@ -123,7 +123,61 @@ func (s *Service) GetPortForExternalNetwork(instanceID string, externalNetworkID
 	return nil, nil
 }
 
-func (s *Service) CreatePort(eventObject runtime.Object, portSpec *infrav1.ResolvedPortSpec) (*ports.Port, error) {
+// ensurePortTagsAndTrunk ensures that the provided port has the tags and trunk defined in portSpec.
+func (s *Service) ensurePortTagsAndTrunk(port *ports.Port, eventObject runtime.Object, portSpec *infrav1.ResolvedPortSpec) error {
+	wantedTags := uniqueSortedTags(portSpec.Tags)
+	actualTags := uniqueSortedTags(port.Tags)
+	// Only replace tags if there is a difference
+	if !slices.Equal(wantedTags, actualTags) && len(wantedTags) > 0 {
+		if err := s.replaceAllAttributesTags(eventObject, portResource, port.ID, wantedTags); err != nil {
+			record.Warnf(eventObject, "FailedReplaceTags", "Failed to replace port tags %s: %v", port.Name, err)
+			return err
+		}
+	}
+	if ptr.Deref(portSpec.Trunk, false) {
+		trunk, err := s.getOrCreateTrunkForPort(eventObject, port)
+		if err != nil {
+			record.Warnf(eventObject, "FailedCreateTrunk", "Failed to create trunk for port %s: %v", port.Name, err)
+			return err
+		}
+
+		if !slices.Equal(wantedTags, trunk.Tags) {
+			if err = s.replaceAllAttributesTags(eventObject, trunkResource, trunk.ID, wantedTags); err != nil {
+				record.Warnf(eventObject, "FailedReplaceTags", "Failed to replace trunk tags %s: %v", port.Name, err)
+				return err
+			}
+		}
+	}
+	return nil
+}
+
+// EnsurePort ensure that a port defined with portSpec Name and NetworkID exists,
+// and that the port has suitable tags and trunk. If the PortStatus is already known,
+// use the ID when filtering for existing ports.
+func (s *Service) EnsurePort(eventObject runtime.Object, portSpec *infrav1.ResolvedPortSpec, portStatus infrav1.PortStatus) (*ports.Port, error) {
+	opts := ports.ListOpts{
+		Name:      portSpec.Name,
+		NetworkID: portSpec.NetworkID,
+	}
+	if portStatus.ID != "" {
+		opts.ID = portStatus.ID
+	}
+
+	existingPorts, err := s.client.ListPort(opts)
+	if err != nil {
+		return nil, fmt.Errorf("searching for existing port for server: %v", err)
+	}
+	if len(existingPorts) > 1 {
+		return nil, fmt.Errorf("multiple ports found with name \"%s\"", portSpec.Name)
+	}
+
+	if len(existingPorts) == 1 {
+		port := &existingPorts[0]
+		if err = s.ensurePortTagsAndTrunk(port, eventObject, portSpec); err != nil {
+			return nil, err
+		}
+		return port, nil
+	}
 	var addressPairs []ports.AddressPair
 	if !ptr.Deref(portSpec.DisablePortSecurity, false) {
 		for _, ap := range portSpec.AllowedAddressPairs {
@@ -196,24 +250,10 @@ func (s *Service) CreatePort(eventObject runtime.Object, portSpec *infrav1.Resol
 		return nil, err
 	}
 
-	if len(portSpec.Tags) > 0 {
-		if err = s.replaceAllAttributesTags(eventObject, portResource, port.ID, portSpec.Tags); err != nil {
-			record.Warnf(eventObject, "FailedReplaceTags", "Failed to replace port tags %s: %v", portSpec.Name, err)
-			return nil, err
-		}
+	if err = s.ensurePortTagsAndTrunk(port, eventObject, portSpec); err != nil {
+		return nil, err
 	}
 	record.Eventf(eventObject, "SuccessfulCreatePort", "Created port %s with id %s", port.Name, port.ID)
-	if ptr.Deref(portSpec.Trunk, false) {
-		trunk, err := s.getOrCreateTrunkForPort(eventObject, port)
-		if err != nil {
-			record.Warnf(eventObject, "FailedCreateTrunk", "Failed to create trunk for port %s: %v", port.Name, err)
-			return nil, err
-		}
-		if err = s.replaceAllAttributesTags(eventObject, trunkResource, trunk.ID, portSpec.Tags); err != nil {
-			record.Warnf(eventObject, "FailedReplaceTags", "Failed to replace trunk tags %s: %v", port.Name, err)
-			return nil, err
-		}
-	}
 
 	return port, nil
 }
@@ -324,23 +364,30 @@ func getPortName(baseName string, portSpec *infrav1.PortOpts, netIndex int) stri
 	return fmt.Sprintf("%s-%d", baseName, netIndex)
 }
 
-func (s *Service) CreatePorts(eventObject runtime.Object, desiredPorts []infrav1.ResolvedPortSpec, resources *infrav1.MachineResources) error {
+// EnsurePorts ensures that every one of desiredPorts is created and has
+// expected trunk and tags.
+func (s *Service) EnsurePorts(eventObject runtime.Object, desiredPorts []infrav1.ResolvedPortSpec, resources *infrav1.MachineResources) error {
 	for i := range desiredPorts {
-		// Skip creation of ports which already exist
+		// If we already created the port, make use of the status
+		portStatus := infrav1.PortStatus{}
 		if i < len(resources.Ports) {
-			continue
+			portStatus = resources.Ports[i]
 		}
-
-		portSpec := &desiredPorts[i]
-		// Events are recorded in CreatePort
-		port, err := s.CreatePort(eventObject, portSpec)
+		// Events are recorded in EnsurePort
+		port, err := s.EnsurePort(eventObject, &desiredPorts[i], portStatus)
 		if err != nil {
 			return err
 		}
 
-		resources.Ports = append(resources.Ports, infrav1.PortStatus{
-			ID: port.ID,
-		})
+		// If we already have the status, replace it,
+		// otherwise append it.
+		if i < len(resources.Ports) {
+			resources.Ports[i] = portStatus
+		} else {
+			resources.Ports = append(resources.Ports, infrav1.PortStatus{
+				ID: port.ID,
+			})
+		}
 	}
 
 	return nil
@@ -609,3 +656,19 @@ func (s *Service) AdoptPorts(scope *scope.WithLogger, desiredPorts []infrav1.Res
 
 	return nil
 }
+
+// uniqueSortedTags returns a new, sorted slice where any duplicates have been removed.
+func uniqueSortedTags(tags []string) []string {
+	// remove duplicate values from tags
+	tagsMap := make(map[string]string)
+	for _, t := range tags {
+		tagsMap[t] = t
+	}
+
+	uniqueTags := []string{}
+	for k := range tagsMap {
+		uniqueTags = append(uniqueTags, k)
+	}
+	slices.Sort(uniqueTags)
+	return uniqueTags
+}
diff --git a/pkg/cloud/services/networking/port_test.go b/pkg/cloud/services/networking/port_test.go
index bbdc1cd018..2cee4c84a3 100644
--- a/pkg/cloud/services/networking/port_test.go
+++ b/pkg/cloud/services/networking/port_test.go
@@ -40,7 +40,7 @@ import (
 	"sigs.k8s.io/cluster-api-provider-openstack/pkg/scope"
 )
 
-func Test_CreatePort(t *testing.T) {
+func Test_EnsurePort(t *testing.T) {
 	// Arbitrary values used in the tests
 	const (
 		netID               = "7fd24ceb-788a-441f-ad0a-d8e2f5d31a1d"
@@ -59,8 +59,8 @@ func Test_CreatePort(t *testing.T) {
 		name   string
 		port   infrav1.ResolvedPortSpec
 		expect func(m *mock.MockNetworkClientMockRecorder, g Gomega)
-		// Note the 'wanted' port isn't so important, since it will be whatever we tell ListPort or CreatePort to return.
-		// Mostly in this test suite, we're checking that CreatePort is called with the expected port opts.
+		// Note the 'wanted' port isn't so important, since it will be whatever we tell ListPort or EnsurePort to return.
+		// Mostly in this test suite, we're checking that EnsurePort is called with the expected port opts.
 		want    *ports.Port
 		wantErr bool
 	}{
@@ -156,6 +156,10 @@ func Test_CreatePort(t *testing.T) {
 					},
 				}
 
+				m.ListPort(ports.ListOpts{
+					Name:      "foo-port-1",
+					NetworkID: netID,
+				}).Return(nil, nil)
 				// The following allows us to use gomega to
 				// compare the argument instead of gomock.
 				// Gomock's output in the case of a mismatch is
@@ -183,6 +187,10 @@ func Test_CreatePort(t *testing.T) {
 				expectedCreateOpts = portsbinding.CreateOptsExt{
 					CreateOptsBuilder: expectedCreateOpts,
 				}
+				m.ListPort(ports.ListOpts{
+					Name:      "test-port",
+					NetworkID: netID,
+				}).Return(nil, nil)
 				m.CreatePort(gomock.Any()).DoAndReturn(func(builder ports.CreateOptsBuilder) (*ports.Port, error) {
 					gotCreateOpts := builder.(portsbinding.CreateOptsExt)
 					g.Expect(gotCreateOpts).To(Equal(expectedCreateOpts), cmp.Diff(gotCreateOpts, expectedCreateOpts))
@@ -219,6 +227,10 @@ func Test_CreatePort(t *testing.T) {
 				expectedCreateOpts = portsbinding.CreateOptsExt{
 					CreateOptsBuilder: expectedCreateOpts,
 				}
+				m.ListPort(ports.ListOpts{
+					Name:      "test-port",
+					NetworkID: netID,
+				}).Return(nil, nil)
 				m.CreatePort(gomock.Any()).DoAndReturn(func(builder ports.CreateOptsBuilder) (*ports.Port, error) {
 					gotCreateOpts := builder.(portsbinding.CreateOptsExt)
 					g.Expect(gotCreateOpts).To(Equal(expectedCreateOpts), cmp.Diff(gotCreateOpts, expectedCreateOpts))
@@ -261,6 +273,10 @@ func Test_CreatePort(t *testing.T) {
 				expectedCreateOpts = portsbinding.CreateOptsExt{
 					CreateOptsBuilder: expectedCreateOpts,
 				}
+				m.ListPort(ports.ListOpts{
+					Name:      "test-port",
+					NetworkID: netID,
+				}).Return(nil, nil)
 				m.CreatePort(gomock.Any()).DoAndReturn(func(builder ports.CreateOptsBuilder) (*ports.Port, error) {
 					gotCreateOpts := builder.(portsbinding.CreateOptsExt)
 					g.Expect(gotCreateOpts).To(Equal(expectedCreateOpts), cmp.Diff(gotCreateOpts, expectedCreateOpts))
@@ -270,7 +286,7 @@ func Test_CreatePort(t *testing.T) {
 			want: &ports.Port{ID: portID},
 		},
 		{
-			name: "tags and trunk",
+			name: "create port with tags and trunk",
 			port: infrav1.ResolvedPortSpec{
 				Name:      "test-port",
 				NetworkID: netID,
@@ -287,6 +303,10 @@ func Test_CreatePort(t *testing.T) {
 					CreateOptsBuilder: expectedCreateOpts,
 				}
 
+				m.ListPort(ports.ListOpts{
+					Name:      "test-port",
+					NetworkID: netID,
+				}).Return(nil, nil)
 				// Create the port
 				m.CreatePort(gomock.Any()).DoAndReturn(func(builder ports.CreateOptsBuilder) (*ports.Port, error) {
 					gotCreateOpts := builder.(portsbinding.CreateOptsExt)
@@ -318,6 +338,87 @@ func Test_CreatePort(t *testing.T) {
 			},
 			want: &ports.Port{ID: portID, Name: "test-port"},
 		},
+		{
+			name: "port with tags and trunk already exists",
+			port: infrav1.ResolvedPortSpec{
+				Name:      "test-port",
+				NetworkID: netID,
+				Tags:      []string{"tag1", "tag2"},
+				Trunk:     ptr.To(true),
+			},
+			expect: func(m *mock.MockNetworkClientMockRecorder, _ types.Gomega) {
+				m.ListPort(ports.ListOpts{
+					Name:      "test-port",
+					NetworkID: netID,
+				}).Return([]ports.Port{{
+					ID:        portID,
+					Name:      "test-port",
+					NetworkID: netID,
+					Tags:      []string{"tag1", "tag2"},
+				}}, nil)
+
+				// Look for existing trunk
+				m.ListTrunk(trunks.ListOpts{
+					PortID: portID,
+					Name:   "test-port",
+				}).Return([]trunks.Trunk{{
+					ID:   trunkID,
+					Tags: []string{"tag1", "tag2"},
+				}}, nil)
+			},
+			want: &ports.Port{
+				ID:        portID,
+				Name:      "test-port",
+				NetworkID: netID,
+				Tags:      []string{"tag1", "tag2"},
+			},
+		},
+		{
+			name: "partial port missing tags and trunk",
+			port: infrav1.ResolvedPortSpec{
+				Name:      "test-port",
+				NetworkID: netID,
+				Tags:      []string{"tag1", "tag2"},
+				Trunk:     ptr.To(true),
+			},
+			expect: func(m *mock.MockNetworkClientMockRecorder, _ types.Gomega) {
+				m.ListPort(ports.ListOpts{
+					Name:      "test-port",
+					NetworkID: netID,
+				}).Return([]ports.Port{{
+					ID:        portID,
+					Name:      "test-port",
+					NetworkID: netID,
+				}}, nil)
+
+				// Tag the port
+				m.ReplaceAllAttributesTags("ports", portID, attributestags.ReplaceAllOpts{
+					Tags: []string{"tag1", "tag2"},
+				})
+
+				// Look for existing trunk
+				m.ListTrunk(trunks.ListOpts{
+					PortID: portID,
+					Name:   "test-port",
+				}).Return([]trunks.Trunk{}, nil)
+
+				// Create the trunk
+				m.CreateTrunk(trunks.CreateOpts{
+					PortID: portID,
+					Name:   "test-port",
+				}).Return(&trunks.Trunk{ID: trunkID}, nil)
+
+				// Tag the trunk
+				m.ReplaceAllAttributesTags("trunks", trunkID, attributestags.ReplaceAllOpts{
+					Tags: []string{"tag1", "tag2"},
+				})
+			},
+			want: &ports.Port{
+				ID:        portID,
+				Name:      "test-port",
+				NetworkID: netID,
+			},
+		},
 	}
 
 	eventObject := &infrav1.OpenStackMachine{}
@@ -333,9 +434,10 @@ func Test_CreatePort(t *testing.T) {
 			s := Service{
 				client: mockClient,
 			}
-			got, err := s.CreatePort(
+			got, err := s.EnsurePort(
 				eventObject,
 				&tt.port,
+				infrav1.PortStatus{},
 			)
 			if tt.wantErr {
 				g.Expect(err).To(HaveOccurred())
diff --git a/pkg/cloud/services/networking/service.go b/pkg/cloud/services/networking/service.go
index 3a01c48b85..d1b206fde2 100644
--- a/pkg/cloud/services/networking/service.go
+++ b/pkg/cloud/services/networking/service.go
@@ -18,7 +18,6 @@ package networking
 
 import (
 	"fmt"
-	"sort"
 
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/attributestags"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -65,28 +64,15 @@ func (s *Service) replaceAllAttributesTags(eventObject runtime.Object, resourceT
 		record.Warnf(eventObject, "FailedReplaceAllAttributesTags", "Invalid resourceType argument in function call")
 		panic(fmt.Errorf("invalid argument: resourceType, %s, does not match allowed arguments: %s or %s", resourceType, trunkResource, portResource))
 	}
-	// remove duplicate values from tags
-	tagsMap := make(map[string]string)
-	for _, t := range tags {
-		tagsMap[t] = t
-	}
-
-	uniqueTags := []string{}
-	for k := range tagsMap {
-		uniqueTags = append(uniqueTags, k)
-	}
-
-	// Sort the tags so that we always get fixed order of tags to make UT easier
-	sort.Strings(uniqueTags)
 
 	_, err := s.client.ReplaceAllAttributesTags(resourceType, resourceID, attributestags.ReplaceAllOpts{
-		Tags: uniqueTags,
+		Tags: tags,
 	})
 	if err != nil {
 		record.Warnf(eventObject, "FailedReplaceAllAttributesTags", "Failed to replace all attributestags, %s: %v", resourceID, err)
 		return err
 	}
 
-	record.Eventf(eventObject, "SuccessfulReplaceAllAttributeTags", "Replaced all attributestags for %s with tags %s", resourceID, uniqueTags)
+	record.Eventf(eventObject, "SuccessfulReplaceAllAttributeTags", "Replaced all attributestags for %s with tags %s", resourceID, tags)
 	return nil
 }
diff --git a/test/e2e/data/e2e_conf.yaml b/test/e2e/data/e2e_conf.yaml
index f7aa4214e0..c57568a8d6 100644
--- a/test/e2e/data/e2e_conf.yaml
+++ b/test/e2e/data/e2e_conf.yaml
@@ -1,4 +1,3 @@
----
 # E2E test scenario using local dev images and manifests built from the source tree for following providers:
 # - cluster-api
 # - bootstrap kubeadm

From 23d4eb6ba3a1a99c5695ec851e9dc95368172e53 Mon Sep 17 00:00:00 2001
From: okozachenko1203 <okozachenko@vexxhost.com>
Date: Wed, 16 Apr 2025 20:40:41 +1000
Subject: [PATCH 15/16] allow switching from filter.name to id in
 openstackclusterspec network and subnets

---
 pkg/webhooks/openstackcluster_webhook.go      |  68 ++++
 pkg/webhooks/openstackcluster_webhook_test.go | 304 ++++++++++++++++++
 2 files changed, 372 insertions(+)

diff --git a/pkg/webhooks/openstackcluster_webhook.go b/pkg/webhooks/openstackcluster_webhook.go
index 910f761ebb..6334b0a419 100644
--- a/pkg/webhooks/openstackcluster_webhook.go
+++ b/pkg/webhooks/openstackcluster_webhook.go
@@ -72,6 +72,60 @@ func (*openStackClusterWebhook) ValidateCreate(_ context.Context, objRaw runtime
 	return aggregateObjErrors(newObj.GroupVersionKind().GroupKind(), newObj.Name, allErrs)
 }
 
+// allowSubnetFilterToIDTransition checks if changes to OpenStackCluster.Spec.Subnets
+// are transitioning from a Filter-based definition to an ID-based one, and whether
+// those transitions are valid based on the current status.network.subnets.
+//
+// This function only allows Filter → ID transitions when the filter name in the old
+// spec matches the subnet name in status, and the new ID matches the corresponding subnet ID.
+//
+// Returns true if all such transitions are valid; false otherwise.
+func allowSubnetFilterToIDTransition(oldObj, newObj *infrav1.OpenStackCluster) bool {
+	if newObj.Spec.Network == nil || oldObj.Spec.Network == nil || oldObj.Status.Network == nil {
+		return false
+	}
+
+	if len(newObj.Spec.Subnets) != len(oldObj.Spec.Subnets) || len(oldObj.Status.Network.Subnets) == 0 {
+		return false
+	}
+
+	for i := range newObj.Spec.Subnets {
+		oldSubnet := oldObj.Spec.Subnets[i]
+		newSubnet := newObj.Spec.Subnets[i]
+
+		// Allow Filter → ID only if both values match a known subnet in status
+		if oldSubnet.Filter != nil && newSubnet.ID != nil && newSubnet.Filter == nil {
+			matchFound := false
+			for _, statusSubnet := range oldObj.Status.Network.Subnets {
+				if oldSubnet.Filter.Name == statusSubnet.Name && *newSubnet.ID == statusSubnet.ID {
+					matchFound = true
+					break
+				}
+			}
+			if !matchFound {
+				return false
+			}
+		}
+
+		// Reject any change from ID → Filter
+		if oldSubnet.ID != nil && newSubnet.Filter != nil {
+			return false
+		}
+
+		// Reject changes to Filter or ID if they do not match the old values
+		if oldSubnet.Filter != nil && newSubnet.Filter != nil &&
+			oldSubnet.Filter.Name != newSubnet.Filter.Name {
+			return false
+		}
+		if oldSubnet.ID != nil && newSubnet.ID != nil &&
+			*oldSubnet.ID != *newSubnet.ID {
+			return false
+		}
+	}
+
+	return true
+}
+
 // ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type.
 func (*openStackClusterWebhook) ValidateUpdate(_ context.Context, oldObjRaw, newObjRaw runtime.Object) (admission.Warnings, error) {
 	var allErrs field.ErrorList
@@ -154,6 +208,20 @@ func (*openStackClusterWebhook) ValidateUpdate(_ context.Context, oldObjRaw, new
 		oldObj.Spec.APIServerFloatingIP = nil
 	}
 
+	// Allow changes from filter to id for spec.network and spec.subnets
+	if newObj.Spec.Network != nil && oldObj.Spec.Network != nil && oldObj.Status.Network != nil {
+		// Allow change from spec.network.subnets from filter to id if it matches the current subnets.
+		if allowSubnetFilterToIDTransition(oldObj, newObj) {
+			oldObj.Spec.Subnets = nil
+			newObj.Spec.Subnets = nil
+		}
+		// Allow change from spec.network.filter to spec.network.id only if it matches the current network.
+		if ptr.Deref(newObj.Spec.Network.ID, "") == oldObj.Status.Network.ID {
+			newObj.Spec.Network = nil
+			oldObj.Spec.Network = nil
+		}
+	}
+
 	if !reflect.DeepEqual(oldObj.Spec, newObj.Spec) {
 		allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "cannot be modified"))
 	}
diff --git a/pkg/webhooks/openstackcluster_webhook_test.go b/pkg/webhooks/openstackcluster_webhook_test.go
index edd60296a3..4ece188c87 100644
--- a/pkg/webhooks/openstackcluster_webhook_test.go
+++ b/pkg/webhooks/openstackcluster_webhook_test.go
@@ -512,6 +512,310 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "Switching OpenStackCluster.Spec.Network from filter.name to id is allowed when they refer to the same network",
+			oldTemplate: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					IdentityRef: infrav1.OpenStackIdentityReference{
+						Name:      "foobar",
+						CloudName: "foobar",
+					},
+					Network: &infrav1.NetworkParam{
+						Filter: &infrav1.NetworkFilter{
+							Name: "testnetworkname",
+						},
+					},
+				},
+				Status: infrav1.OpenStackClusterStatus{
+					Network: &infrav1.NetworkStatusWithSubnets{
+						NetworkStatus: infrav1.NetworkStatus{
+							ID:   "testnetworkid",
+							Name: "testnetworkname",
+						},
+					},
+				},
+			},
+			newTemplate: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					IdentityRef: infrav1.OpenStackIdentityReference{
+						Name:      "foobar",
+						CloudName: "foobar",
+					},
+					Network: &infrav1.NetworkParam{
+						ID: ptr.To("testnetworkid"),
+					},
+				},
+				Status: infrav1.OpenStackClusterStatus{
+					Network: &infrav1.NetworkStatusWithSubnets{
+						NetworkStatus: infrav1.NetworkStatus{
+							ID:   "testnetworkid",
+							Name: "testnetworkname",
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "Switching OpenStackCluster.Spec.Network from filter.name to id is not allowed when they refer to different networks",
+			oldTemplate: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					IdentityRef: infrav1.OpenStackIdentityReference{
+						Name:      "foobar",
+						CloudName: "foobar",
+					},
+					Network: &infrav1.NetworkParam{
+						Filter: &infrav1.NetworkFilter{
+							Name: "testetworkname",
+						},
+					},
+				},
+				Status: infrav1.OpenStackClusterStatus{
+					Network: &infrav1.NetworkStatusWithSubnets{
+						NetworkStatus: infrav1.NetworkStatus{
+							ID:   "testetworkid1",
+							Name: "testnetworkname",
+						},
+					},
+				},
+			},
+			newTemplate: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					IdentityRef: infrav1.OpenStackIdentityReference{
+						Name:      "foobar",
+						CloudName: "foobar",
+					},
+					Network: &infrav1.NetworkParam{
+						ID: ptr.To("testetworkid2"),
+					},
+				},
+				Status: infrav1.OpenStackClusterStatus{
+					Network: &infrav1.NetworkStatusWithSubnets{
+						NetworkStatus: infrav1.NetworkStatus{
+							ID:   "testetworkid1",
+							Name: "testnetworkname",
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "Switching OpenStackCluster.Spec.Subnets from filter.name to id is allowed when they refer to the same subnet",
+			oldTemplate: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					IdentityRef: infrav1.OpenStackIdentityReference{
+						Name:      "foobar",
+						CloudName: "foobar",
+					},
+					Network: &infrav1.NetworkParam{
+						ID: ptr.To("net-123"),
+					},
+					Subnets: []infrav1.SubnetParam{
+						{
+							Filter: &infrav1.SubnetFilter{
+								Name: "test-subnet",
+							},
+						},
+					},
+				},
+				Status: infrav1.OpenStackClusterStatus{
+					Network: &infrav1.NetworkStatusWithSubnets{
+						NetworkStatus: infrav1.NetworkStatus{
+							ID:   "net-123",
+							Name: "testnetwork",
+						},
+						Subnets: []infrav1.Subnet{
+							{
+								ID:   "subnet-123",
+								Name: "test-subnet",
+							},
+						},
+					},
+				},
+			},
+			newTemplate: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					IdentityRef: infrav1.OpenStackIdentityReference{
+						Name:      "foobar",
+						CloudName: "foobar",
+					},
+					Network: &infrav1.NetworkParam{
+						ID: ptr.To("net-123"),
+					},
+					Subnets: []infrav1.SubnetParam{
+						{
+							ID: ptr.To("subnet-123"),
+						},
+					},
+				},
+				Status: infrav1.OpenStackClusterStatus{
+					Network: &infrav1.NetworkStatusWithSubnets{
+						NetworkStatus: infrav1.NetworkStatus{
+							ID:   "net-123",
+							Name: "testnetwork",
+						},
+						Subnets: []infrav1.Subnet{
+							{
+								ID:   "subnet-123",
+								Name: "test-subnet",
+							},
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "Switching OpenStackCluster.Spec.Subnets from filter.name to id is not allowed when they refer to different subnets",
+			oldTemplate: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					IdentityRef: infrav1.OpenStackIdentityReference{
+						Name:      "foobar",
+						CloudName: "foobar",
+					},
+					Network: &infrav1.NetworkParam{
+						ID: ptr.To("net-123"),
+					},
+					Subnets: []infrav1.SubnetParam{
+						{
+							Filter: &infrav1.SubnetFilter{
+								Name: "test-subnet",
+							},
+						},
+					},
+				},
+				Status: infrav1.OpenStackClusterStatus{
+					Network: &infrav1.NetworkStatusWithSubnets{
+						NetworkStatus: infrav1.NetworkStatus{
+							ID:   "net-123",
+							Name: "testnetwork",
+						},
+						Subnets: []infrav1.Subnet{
+							{
+								ID:   "subnet-123",
+								Name: "test-subnet",
+							},
+						},
+					},
+				},
+			},
+			newTemplate: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					IdentityRef: infrav1.OpenStackIdentityReference{
+						Name:      "foobar",
+						CloudName: "foobar",
+					},
+					Network: &infrav1.NetworkParam{
+						ID: ptr.To("net-123"),
+					},
+					Subnets: []infrav1.SubnetParam{
+						{
+							ID: ptr.To("wrong-subnet"),
+						},
+					},
+				},
+				Status: infrav1.OpenStackClusterStatus{
+					Network: &infrav1.NetworkStatusWithSubnets{
+						NetworkStatus: infrav1.NetworkStatus{
+							ID:   "net-123",
+							Name: "testnetwork",
+						},
+						Subnets: []infrav1.Subnet{
+							{
+								ID:   "subnet-123",
+								Name: "test-subnet",
+							},
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "Switching one OpenStackCluster.Spec.Subnets entry from filter to a mismatched ID (from another subnet) should be rejected, even if other subnets remain unchanged",
+			oldTemplate: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					IdentityRef: infrav1.OpenStackIdentityReference{
+						Name:      "foobar",
+						CloudName: "foobar",
+					},
+					Network: &infrav1.NetworkParam{
+						ID: ptr.To("net-123"),
+					},
+					Subnets: []infrav1.SubnetParam{
+						{
+							Filter: &infrav1.SubnetFilter{
+								Name: "test-subnet-1",
+							},
+						},
+						{
+							Filter: &infrav1.SubnetFilter{
+								Name: "test-subnet-2",
+							},
+						},
+					},
+				},
+				Status: infrav1.OpenStackClusterStatus{
+					Network: &infrav1.NetworkStatusWithSubnets{
+						NetworkStatus: infrav1.NetworkStatus{
+							ID:   "net-123",
+							Name: "testnetwork",
+						},
+						Subnets: []infrav1.Subnet{
+							{
+								ID:   "test-subnet-id-1",
+								Name: "test-subnet-1",
+							},
+							{
+								ID:   "test-subnet-id-2",
+								Name: "test-subnet-2",
+							},
+						},
+					},
+				},
+			},
+			newTemplate: &infrav1.OpenStackCluster{
+				Spec: infrav1.OpenStackClusterSpec{
+					IdentityRef: infrav1.OpenStackIdentityReference{
+						Name:      "foobar",
+						CloudName: "foobar",
+					},
+					Network: &infrav1.NetworkParam{
+						ID: ptr.To("net-123"),
+					},
+					Subnets: []infrav1.SubnetParam{
+						{
+							ID: ptr.To("test-subnet-id-2"),
+						},
+						{
+							Filter: &infrav1.SubnetFilter{
+								Name: "test-subnet-2",
+							},
+						},
+					},
+				},
+				Status: infrav1.OpenStackClusterStatus{
+					Network: &infrav1.NetworkStatusWithSubnets{
+						NetworkStatus: infrav1.NetworkStatus{
+							ID:   "net-123",
+							Name: "testnetwork",
+						},
+						Subnets: []infrav1.Subnet{
+							{
+								ID:   "test-subnet-id-1",
+								Name: "test-subnet-1",
+							},
+							{
+								ID:   "test-subnet-id-2",
+								Name: "test-subnet-2",
+							},
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

From 04a75f4f3f8775dc40901bc9e1294a95ee17576d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Andr=C3=A9?= <m.andre@redhat.com>
Date: Mon, 23 Jun 2025 14:49:22 +0200
Subject: [PATCH 16/16] CARRY: Re-add slices package

New commits were added that depend on slices functions available in go
1.21.
---
 pkg/cloud/services/networking/port.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pkg/cloud/services/networking/port.go b/pkg/cloud/services/networking/port.go
index f5959a5057..cda98aacc4 100644
--- a/pkg/cloud/services/networking/port.go
+++ b/pkg/cloud/services/networking/port.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"slices"
 	"strings"
 	"time"