openshift · openshift-merge-robot · Apr 28, 2022 · Apr 13, 2022 · Apr 22, 2022 · Apr 25, 2022
diff --git a/api/v1alpha4/conversion.go b/api/v1alpha4/conversion.go
@@ -250,7 +250,7 @@ func Convert_v1alpha4_Instance_To_v1alpha5_Instance(in *Instance, out *infrav1.I
 		return err
 	}
 	if in.RootVolume != nil && in.RootVolume.Size > 0 {
-		out.Image = in.RootVolume.SourceUUID
+		out.ImageUUID = in.RootVolume.SourceUUID
 	}
 	return nil
 }
@@ -260,7 +260,7 @@ func Convert_v1alpha5_Instance_To_v1alpha4_Instance(in *infrav1.Instance, out *I
 		return err
 	}
 	if in.RootVolume != nil && in.RootVolume.Size > 0 {
-		out.RootVolume.SourceUUID = in.Image
+		out.RootVolume.SourceUUID = in.ImageUUID
 		out.Image = ""
 	}
 	return nil
@@ -271,7 +271,7 @@ func Convert_v1alpha4_OpenStackMachineSpec_To_v1alpha5_OpenStackMachineSpec(in *
 		return err
 	}
 	if in.RootVolume != nil && in.RootVolume.Size > 0 {
-		out.Image = in.RootVolume.SourceUUID
+		out.ImageUUID = in.RootVolume.SourceUUID
 	}
 	return nil
 }
@@ -281,7 +281,7 @@ func Convert_v1alpha5_OpenStackMachineSpec_To_v1alpha4_OpenStackMachineSpec(in *
 		return err
 	}
 	if in.RootVolume != nil && in.RootVolume.Size > 0 {
-		out.RootVolume.SourceUUID = in.Image
+		out.RootVolume.SourceUUID = in.ImageUUID
 		out.Image = ""
 	}
 	return nil

diff --git a/api/v1alpha4/conversion_test.go b/api/v1alpha4/conversion_test.go
@@ -270,7 +270,26 @@ func TestFuzzyConversion(t *testing.T) {
 			func(v1alpha5MachineSpec *infrav1.OpenStackMachineSpec, c fuzz.Continue) {
 				c.FuzzNoCustom(v1alpha5MachineSpec)
 
-				v1alpha5MachineSpec.ImageUUID = ""
+				// In v1alpha4 boot from volume only supports
+				// image by UUID, and boot from local only
+				// suppots image by name
+				if v1alpha5MachineSpec.RootVolume != nil && v1alpha5MachineSpec.RootVolume.Size > 0 {
+					v1alpha5MachineSpec.Image = ""
+				} else {
+					v1alpha5MachineSpec.ImageUUID = ""
+				}
+			},
+			func(v1alpha5Instance *infrav1.Instance, c fuzz.Continue) {
+				c.FuzzNoCustom(v1alpha5Instance)
+
+				// In v1alpha4 boot from volume only supports
+				// image by UUID, and boot from local only
+				// suppots image by name
+				if v1alpha5Instance.RootVolume != nil && v1alpha5Instance.RootVolume.Size > 0 {
+					v1alpha5Instance.Image = ""
+				} else {
+					v1alpha5Instance.ImageUUID = ""
+				}
 			},
 			func(v1alpha5RootVolume *infrav1.RootVolume, c fuzz.Continue) {
 				c.FuzzNoCustom(v1alpha5RootVolume)

diff --git a/controllers/openstackcluster_controller.go b/controllers/openstackcluster_controller.go
@@ -372,9 +372,11 @@ func bastionToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, clusterNa
 
 	instanceSpec.SecurityGroups = openStackCluster.Spec.Bastion.Instance.SecurityGroups
 	if openStackCluster.Spec.ManagedSecurityGroups {
-		instanceSpec.SecurityGroups = append(instanceSpec.SecurityGroups, infrav1.SecurityGroupParam{
-			UUID: openStackCluster.Status.BastionSecurityGroup.ID,
-		})
+		if openStackCluster.Status.BastionSecurityGroup != nil {
+			instanceSpec.SecurityGroups = append(instanceSpec.SecurityGroups, infrav1.SecurityGroupParam{
+				UUID: openStackCluster.Status.BastionSecurityGroup.ID,
+			})
+		}
 	}
 
 	instanceSpec.Networks = openStackCluster.Spec.Bastion.Instance.Networks

diff --git a/docs/book/src/SUMMARY.md b/docs/book/src/SUMMARY.md
@@ -7,5 +7,7 @@
     - [external cloud provider](./topics/external-cloud-provider.md)
     - [move from bootstrap](./topics/mover.md)
     - [trouble shooting](./topics/troubleshooting.md)
+    - [CRD Changes](./topics/crd-changes/index.md)
+        - [v1alpha4 to v1alpha5](./topics/crd-changes/v1alpha4-to-v1alpha5.md)
 - [Development](./development/development.md)
 - [Hacking CI](./development/ci.md)
diff --git a/docs/book/src/topics/crd-changes/index.md b/docs/book/src/topics/crd-changes/index.md
@@ -0,0 +1,5 @@
+# CRD Changes
+
+### Conversions
+
+CAPO is able to automatically convert your old resources into new API versions.
diff --git a/docs/book/src/topics/crd-changes/v1alpha4-to-v1alpha5.md b/docs/book/src/topics/crd-changes/v1alpha4-to-v1alpha5.md
@@ -0,0 +1,123 @@
+<!-- START doctoc generated TOC please keep comment here to allow auto update -->
+<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
+**Table of Contents**  *generated with [DocToc](https://github.com/thlorenz/doctoc)*
+
+- [v1alpha4 compared to v1alpha5](#v1alpha4-compared-to-v1alpha5)
+  - [Conversion from v1alpha4 to v1alpha5](#conversion-from-v1alpha4-to-v1alpha5)
+  - [API Changes](#api-changes)
+    - [`OpenStackCluster`](#openstackcluster)
+      - [Managed API LoadBalancer](#managed-api-loadbalancer)
+      - [Major Changes to Ports and Networks](#major-changes-to-ports-and-networks)
+    - [`OpenStackMachine`](#openstackmachine)
+      - [Rename of `status.error{Reason,Message}` to `status.failure{Reason,Message}`](#rename-of-statuserrorreasonmessage-to-statusfailurereasonmessage)
+      - [Changes to `rootVolume`](#changes-to-rootvolume)
+
+<!-- END doctoc generated TOC please keep comment here to allow auto update -->
+
+# v1alpha4 compared to v1alpha5
+
+## Migration
+
+
+All users are encouraged to migrate their usage of the CAPO CRDs from older versions to `v1alpha5`. This includes yaml files and source code. As CAPO implements automatic conversions between the CRD versions, this migration can happen after installing the new CAPO release.
+
+## API Changes
+
+This only documents backwards incompatible changes. Fields that were added to v1alpha5 are not listed here.
+
+### `OpenStackCluster`
+
+#### Managed API LoadBalancer
+
+The fields related to the managed API LoadBalancer were moved into a seperate object:
+
+```yaml
+apiVersion: infrastructure.cluster.x-k8s.io/v1alpha4
+kind: OpenStackCluster
+spec:
+  managedAPIServerLoadBalancer: true
+  apiServerLoadBalancerAdditionalPorts: [443]
+```
+
+becomes:
+
+```yaml
+apiVersion: infrastructure.cluster.x-k8s.io/v1alpha5
+kind: OpenStackCluster
+spec:
+  apiServerLoadBalancer:
+    enabled: true
+    additionalPorts: [443]
+```
+
+### `OpenStackMachine`
+
+#### Major Changes to Ports and Networks
+
+When using Ports it is now possible to specify network and subnet by filter instead of just ID. As a consequence, the relevant ID fields are now moved into the new filter specifications:
+
+```yaml
+ports:
+  - networkId: d-e-a-d
+    fixedIPs:
+      - subnetId: b-e-e-f
+```
+
+becomes:
+
+```yaml
+ports:
+  - network:
+      id: d-e-a-d
+    fixedIPs:
+      subnet:
+        id: b-e-e-f
+```
+
+Networks are now deprecated. With one exception, all functionality of Networks is now available for Ports. Consequently, Networks will be removed from the API in a future release.
+
+The ability of a Network to add multiple ports with a single directive will not be added to Ports. When moving to Ports, all ports must be added explicitly. Specifically, when evaluating the network or subnet filter of a Network, if there are multiple matches we will add all of these to the server. By contrast we raise an error if the network or subnet filter of a Port does not return exactly one result.
+
+`tenantId` was previously a synonym for `projectId` in both network and subnet filters. This has now been removed. Use `projectId` instead.
+
+The following fields are removed from network and subnet filters without replacement:
+
+- status
+- adminStateUp
+- shared
+- marker
+- limit
+- sortKey
+- sortDir
+- subnetPoolId
+
+#### Rename of `status.error{Reason,Message}` to `status.failure{Reason,Message}`
+
+The actual fields were previously already renamed, but we still used the `error` prefix in JSON. This was done to align with CAPI, where these fields were [renamed in v1alpha3](https://cluster-api.sigs.k8s.io/developer/providers/v1alpha2-to-v1alpha3.html#external-objects-will-need-to-rename-statuserrorreason-and-statuserrormessage).
+
+```yaml
+apiVersion: infrastructure.cluster.x-k8s.io/v1alpha4
+kind: OpenStackMachine
+status:
+  errorReason: UpdateError
+  errorMessage: Something when wrong
+```
+
+becomes:
+
+```yaml
+apiVersion: infrastructure.cluster.x-k8s.io/v1alpha5
+kind: OpenStackMachine
+status:
+  failureReason: UpdateError
+  failureMessage: Something when wrong
+```
+
+#### Changes to `rootVolume`
+
+The following fields were removed without replacement:
+
+- `rootVolume.deviceType`
+- `rootVolume.sourceType`
+
+Additionally, `rootVolume.sourceUUID` has been replaced by using `ImageUUID` or `Image` from the OpenStackMachine as appropriate.
diff --git a/metadata.yaml b/metadata.yaml
@@ -10,3 +10,6 @@ releaseSeries:
 - major: 0
   minor: 5
   contract: v1beta1
+- major: 0
+  minor: 6
+  contract: v1beta1
diff --git a/pkg/cloud/services/compute/instance.go b/pkg/cloud/services/compute/instance.go
@@ -188,20 +188,6 @@ func (s *Service) createInstanceImpl(eventObject runtime.Object, openStackCluste
 		})
 	}
 
-	var serverCreateOpts servers.CreateOptsBuilder = servers.CreateOpts{
-		Name:             instanceSpec.Name,
-		ImageRef:         imageID,
-		FlavorRef:        flavorID,
-		AvailabilityZone: instanceSpec.FailureDomain,
-		Networks:         portList,
-		UserData:         []byte(instanceSpec.UserData),
-		SecurityGroups:   securityGroups,
-		Tags:             instanceSpec.Tags,
-		Metadata:         instanceSpec.Metadata,
-		ConfigDrive:      &instanceSpec.ConfigDrive,
-		AccessIPv4:       accessIPv4,
-	}
-
 	volume, err := s.getOrCreateRootVolume(eventObject, instanceSpec, imageID)
 	if err != nil {
 		return nil, fmt.Errorf("error in get or create root volume: %w", err)
@@ -235,6 +221,26 @@ func (s *Service) createInstanceImpl(eventObject runtime.Object, openStackCluste
 		}
 	}
 
+	// Don't set ImageRef on the server if we're booting from volume
+	var serverImageRef string
+	if volume == nil {
+		serverImageRef = imageID
+	}
+
+	var serverCreateOpts servers.CreateOptsBuilder = servers.CreateOpts{
+		Name:             instanceSpec.Name,
+		ImageRef:         serverImageRef,
+		FlavorRef:        flavorID,
+		AvailabilityZone: instanceSpec.FailureDomain,
+		Networks:         portList,
+		UserData:         []byte(instanceSpec.UserData),
+		SecurityGroups:   securityGroups,
+		Tags:             instanceSpec.Tags,
+		Metadata:         instanceSpec.Metadata,
+		ConfigDrive:      &instanceSpec.ConfigDrive,
+		AccessIPv4:       accessIPv4,
+	}
+
 	serverCreateOpts = applyRootVolume(serverCreateOpts, volume)
 
 	serverCreateOpts = applyServerGroupID(serverCreateOpts, instanceSpec.ServerGroupID)

diff --git a/pkg/cloud/services/compute/instance_test.go b/pkg/cloud/services/compute/instance_test.go
@@ -834,6 +834,7 @@ func TestService_ReconcileInstance(t *testing.T) {
 
 				createMap := getDefaultServerMap()
 				serverMap := createMap["server"].(map[string]interface{})
+				serverMap["imageRef"] = ""
 				serverMap["block_device_mapping_v2"] = []map[string]interface{}{
 					{
 						"delete_on_termination": true,
@@ -880,6 +881,7 @@ func TestService_ReconcileInstance(t *testing.T) {
 
 				createMap := getDefaultServerMap()
 				serverMap := createMap["server"].(map[string]interface{})
+				serverMap["imageRef"] = ""
 				serverMap["block_device_mapping_v2"] = []map[string]interface{}{
 					{
 						"delete_on_termination": true,

diff --git a/pkg/cloud/services/loadbalancer/loadbalancer.go b/pkg/cloud/services/loadbalancer/loadbalancer.go
@@ -58,8 +58,8 @@ func (s *Service) ReconcileLoadBalancer(openStackCluster *infrav1.OpenStackClust
 	if err != nil {
 		return err
 	}
-	if lb.ProvisioningStatus != loadBalancerProvisioningStatusActive {
-		return fmt.Errorf("load balancer %q is not in expected state %s, current state is %s", lb.ID, loadBalancerProvisioningStatusActive, lb.ProvisioningStatus)
+	if err := s.waitForLoadBalancerActive(lb.ID); err != nil {
+		return fmt.Errorf("load balancer %q with id %s is not active after timeout: %v", loadBalancerName, lb.ID, err)
 	}
 
 	var lbFloatingIP string
@@ -133,11 +133,6 @@ func (s *Service) getOrCreateLoadBalancer(openStackCluster *infrav1.OpenStackClu
 		return nil, err
 	}
 
-	if err := s.waitForLoadBalancerActive(lb.ID); err != nil {
-		record.Warnf(openStackCluster, "FailedCreateLoadBalancer", "Failed to create load balancer %s with id %s: wait for load balancer active: %v", loadBalancerName, lb.ID, err)
-		return nil, err
-	}
-
 	record.Eventf(openStackCluster, "SuccessfulCreateLoadBalancer", "Created load balancer %s with id %s", loadBalancerName, lb.ID)
 	return lb, nil
 }