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 d2a0fe7cbf..f6a6f7bc78 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 4c040c5ea9..1e2caf0f15 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 @@ -359,8 +362,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 @@ -435,9 +455,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 @@ -557,3 +582,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/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 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 "`. - - Depending on the type of release, substitute `` 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. 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 diff --git a/controllers/openstackcluster_controller.go b/controllers/openstackcluster_controller.go index 96846f58ca..9ec63477bc 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 { @@ -614,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) } @@ -664,14 +662,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, }) } } @@ -680,6 +683,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/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/hack/ci/cloud-init/controller.yaml.tpl b/hack/ci/cloud-init/controller.yaml.tpl index a3b8bb23b3..3cd465bb13 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} @@ -43,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 @@ -89,6 +102,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..0a34b69a22 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 @@ -39,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} @@ -47,6 +60,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} diff --git a/hack/ci/create_devstack.sh b/hack/ci/create_devstack.sh index 77ac0f05bf..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 @@ -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/gce-project.sh b/hack/ci/gce-project.sh index 1aae2de9b9..22d4a059ca 100755 --- a/hack/ci/gce-project.sh +++ b/hack/ci/gce-project.sh @@ -95,8 +95,8 @@ function create_vm { --zone "$GCP_ZONE" \ --enable-nested-virtualization \ --image-project ubuntu-os-cloud \ - --image-family ubuntu-2204-lts \ - --boot-disk-size 200G \ + --image-family ubuntu-2404-lts-amd64 \ + --boot-disk-size 500G \ --boot-disk-type pd-ssd \ --can-ip-forward \ --tags http-server,https-server,novnc,openstack-apis \ diff --git a/hack/ci/libvirt.sh b/hack/ci/libvirt.sh new file mode 100755 index 0000000000..d6b8d1ab64 --- /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-2404-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 < + ${LIBVIRT_NETWORK_NAME} + + + + + + + + + + + + + + +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/noble/release/ubuntu-24.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 +} 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" \ 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 diff --git a/pkg/cloud/services/networking/port.go b/pkg/cloud/services/networking/port.go index 33fd22294a..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" @@ -123,7 +124,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 +251,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 +365,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 +657,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/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) { 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