diff --git a/api/v1alpha4/conversion.go b/api/v1alpha4/conversion.go index 18b97b3a69..484947c066 100644 --- 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 index c0b7b8b919..c90f8dc8ff 100644 --- 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 index 981fd26657..25d532d39d 100644 --- 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 index 71b191ad6d..43b2d6f4a5 100644 --- 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 new file mode 100644 index 0000000000..8a3b387e25 --- /dev/null +++ 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 new file mode 100644 index 0000000000..94b450a461 --- /dev/null +++ b/docs/book/src/topics/crd-changes/v1alpha4-to-v1alpha5.md @@ -0,0 +1,123 @@ + + +**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) + + + +# 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. \ No newline at end of file diff --git a/metadata.yaml b/metadata.yaml index fc980d6916..5379b48deb 100644 --- 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 index 050128efa7..2d2128719c 100644 --- 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 index 9c37662017..c8dbde9283 100644 --- 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 index 74d921a0ca..761dbaef5a 100644 --- 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 } diff --git a/pkg/cloud/services/loadbalancer/loadbalancer_test.go b/pkg/cloud/services/loadbalancer/loadbalancer_test.go index 8667f75be0..8c7de04304 100644 --- a/pkg/cloud/services/loadbalancer/loadbalancer_test.go +++ b/pkg/cloud/services/loadbalancer/loadbalancer_test.go @@ -17,12 +17,14 @@ limitations under the License. package loadbalancer import ( - "fmt" "testing" "github.com/go-logr/logr" "github.com/golang/mock/gomock" + "github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/listeners" "github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/loadbalancers" + "github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/monitors" + "github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/pools" . "github.com/onsi/gomega" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha5" @@ -35,16 +37,21 @@ func Test_ReconcileLoadBalancer(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() - openStackCluster := &infrav1.OpenStackCluster{Status: infrav1.OpenStackClusterStatus{ - ExternalNetwork: &infrav1.Network{ - ID: "aaaaaaaa-bbbb-cccc-dddd-111111111111", + openStackCluster := &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ + DisableAPIServerFloatingIP: true, }, - Network: &infrav1.Network{ - Subnet: &infrav1.Subnet{ - ID: "aaaaaaaa-bbbb-cccc-dddd-222222222222", + Status: infrav1.OpenStackClusterStatus{ + ExternalNetwork: &infrav1.Network{ + ID: "aaaaaaaa-bbbb-cccc-dddd-111111111111", + }, + Network: &infrav1.Network{ + Subnet: &infrav1.Subnet{ + ID: "aaaaaaaa-bbbb-cccc-dddd-222222222222", + }, }, }, - }} + } type serviceFields struct { projectID string networkingClient *mock_networking.MockNetworkClient @@ -59,7 +66,7 @@ func Test_ReconcileLoadBalancer(t *testing.T) { wantError error }{ { - name: "reconcile loadbalancer in non active state should should cause error", + name: "reconcile loadbalancer in non active state should wait for active state", prepareServiceMock: func(sf *serviceFields) { sf.networkingClient = mock_networking.NewMockNetworkClient(mockCtrl) sf.loadbalancerClient = mock_loadbalancer.NewMockLbClient(mockCtrl) @@ -68,17 +75,46 @@ func Test_ReconcileLoadBalancer(t *testing.T) { // add network api call results here }, expectLoadBalancer: func(m *mock_loadbalancer.MockLbClientMockRecorder) { + pendingLB := loadbalancers.LoadBalancer{ + ID: "aaaaaaaa-bbbb-cccc-dddd-333333333333", + Name: "k8s-clusterapi-cluster-AAAAA-kubeapi", + ProvisioningStatus: "PENDING_CREATE", + } + activeLB := pendingLB + activeLB.ProvisioningStatus = "ACTIVE" + // return existing loadbalancer in non-active state - lbList := []loadbalancers.LoadBalancer{ + lbList := []loadbalancers.LoadBalancer{pendingLB} + m.ListLoadBalancers(loadbalancers.ListOpts{Name: pendingLB.Name}).Return(lbList, nil) + + // wait for active loadbalancer by returning active loadbalancer on second call + m.GetLoadBalancer("aaaaaaaa-bbbb-cccc-dddd-333333333333").Return(&pendingLB, nil).Return(&activeLB, nil) + + listenerList := []listeners.Listener{ + { + ID: "aaaaaaaa-bbbb-cccc-dddd-444444444444", + Name: "k8s-clusterapi-cluster-AAAAA-kubeapi-0", + }, + } + m.ListListeners(listeners.ListOpts{Name: listenerList[0].Name}).Return(listenerList, nil) + + poolList := []pools.Pool{ + { + ID: "aaaaaaaa-bbbb-cccc-dddd-555555555555", + Name: "k8s-clusterapi-cluster-AAAAA-kubeapi-0", + }, + } + m.ListPools(pools.ListOpts{Name: poolList[0].Name}).Return(poolList, nil) + + monitorList := []monitors.Monitor{ { - ID: "aaaaaaaa-bbbb-cccc-dddd-333333333333", - Name: "k8s-clusterapi-cluster-AAAAA-kubeapi", - ProvisioningStatus: "PENDING_CREATE", + ID: "aaaaaaaa-bbbb-cccc-dddd-666666666666", + Name: "k8s-clusterapi-cluster-AAAAA-kubeapi-0", }, } - m.ListLoadBalancers(loadbalancers.ListOpts{Name: "k8s-clusterapi-cluster-AAAAA-kubeapi"}).Return(lbList, nil) + m.ListMonitors(monitors.ListOpts{Name: monitorList[0].Name}).Return(monitorList, nil) }, - wantError: fmt.Errorf("load balancer %q is not in expected state %s, current state is %s", "aaaaaaaa-bbbb-cccc-dddd-333333333333", "ACTIVE", "PENDING_CREATE"), + wantError: nil, }, } for _, tt := range lbtests { @@ -90,7 +126,11 @@ func Test_ReconcileLoadBalancer(t *testing.T) { tt.expectNetwork(tt.fields.networkingClient.EXPECT()) tt.expectLoadBalancer(tt.fields.loadbalancerClient.EXPECT()) err := lbs.ReconcileLoadBalancer(openStackCluster, "AAAAA", 0) - g.Expect(err).To(MatchError(tt.wantError)) + if tt.wantError != nil { + g.Expect(err).To(MatchError(tt.wantError)) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } }) } } diff --git a/test/e2e/shared/openstack.go b/test/e2e/shared/openstack.go index 819f6b77ff..b35c514911 100644 --- a/test/e2e/shared/openstack.go +++ b/test/e2e/shared/openstack.go @@ -112,6 +112,10 @@ func dumpOpenStack(_ context.Context, e2eCtx *E2EContext, bootstrapClusterProxyN if err := dumpOpenStackImages(providerClient, clientOpts, logPath); err != nil { _, _ = fmt.Fprintf(GinkgoWriter, "error dumping OpenStack images: %s\n", err) } + + if err := dumpOpenStackPorts(e2eCtx, logPath); err != nil { + _, _ = fmt.Fprintf(GinkgoWriter, "error dumping OpenStack ports: %s\n", err) + } } func dumpOpenStackImages(providerClient *gophercloud.ProviderClient, clientOpts *clientconfig.ClientOpts, logPath string) error { @@ -134,9 +138,25 @@ func dumpOpenStackImages(providerClient *gophercloud.ProviderClient, clientOpts if err != nil { return fmt.Errorf("error marshalling images %v: %s", imagesList, err) } - if err := os.WriteFile(path.Join(logPath, "images.txt"), imagesJSON, 0o600); err != nil { - return fmt.Errorf("error writing seversJSON %s: %s", imagesJSON, err) + if err := os.WriteFile(path.Join(logPath, "images.json"), imagesJSON, 0o600); err != nil { + return fmt.Errorf("error writing images.json %s: %s", imagesJSON, err) + } + return nil +} + +func dumpOpenStackPorts(e2eCtx *E2EContext, logPath string) error { + portsList, err := DumpOpenStackPorts(e2eCtx, ports.ListOpts{}) + if err != nil { + return err + } + portsJSON, err := json.MarshalIndent(portsList, "", " ") + if err != nil { + return fmt.Errorf("error marshalling ports %v: %s", portsList, err) + } + if err := os.WriteFile(path.Join(logPath, "ports.json"), portsJSON, 0o600); err != nil { + return fmt.Errorf("error writing ports.json %s: %s", portsJSON, err) } + return nil }