diff --git a/pkg/clients/compute.go b/pkg/clients/compute.go index 85b63256ce..2bd2a056a1 100644 --- a/pkg/clients/compute.go +++ b/pkg/clients/compute.go @@ -30,19 +30,30 @@ import ( "github.com/gophercloud/utils/v2/openstack/clientconfig" "sigs.k8s.io/cluster-api-provider-openstack/pkg/metrics" + openstackutil "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/openstack" ) /* -NovaMinimumMicroversion is the minimum Nova microversion supported by CAPO -2.60 corresponds to OpenStack Queens +Constants for specific microversion requirements. +2.60 corresponds to OpenStack Queens and 2.53 to OpenStack Pike, +2.38 is the maximum in OpenStack Newton. For the canonical description of Nova microversions, see https://docs.openstack.org/nova/latest/reference/api-microversion-history.html -CAPO uses server tags, which were added in microversion 2.52. +CAPO uses server tags, which were first added in microversion 2.26 and then refined +in 2.52 so it is possible to apply them when creating a server (which is what CAPO does). +We round up to 2.53 here since that takes us to the maximum in Pike. + CAPO supports multiattach volume types, which were added in microversion 2.60. + +2.38 was chosen as a base level since it is reasonably old, but not too old. */ -const NovaMinimumMicroversion = "2.60" +const ( + MinimumNovaMicroversion = "2.38" + NovaTagging = "2.53" + MultiAttachVolume = "2.60" +) type ComputeClient interface { ListAvailabilityZones() ([]availabilityzones.AvailabilityZone, error) @@ -57,9 +68,14 @@ type ComputeClient interface { DeleteAttachedInterface(serverID, portID string) error ListServerGroups() ([]servergroups.ServerGroup, error) + WithMicroversion(required string) (ComputeClient, error) } -type computeClient struct{ client *gophercloud.ServiceClient } +type computeClient struct { + client *gophercloud.ServiceClient + minVersion string + maxVersion string +} // NewComputeClient returns a new compute client. func NewComputeClient(providerClient *gophercloud.ProviderClient, providerClientOpts *clientconfig.ClientOpts) (ComputeClient, error) { @@ -70,9 +86,25 @@ func NewComputeClient(providerClient *gophercloud.ProviderClient, providerClient if err != nil { return nil, fmt.Errorf("failed to create compute service client: %v", err) } - compute.Microversion = NovaMinimumMicroversion - return &computeClient{compute}, nil + // Find the minimum and maximum versions supported by the server + serviceMin, serviceMax, err := openstackutil.GetSupportedMicroversions(*compute) + if err != nil { + return nil, fmt.Errorf("unable to verify compatible server version: %w", err) + } + + supported, err := openstackutil.MicroversionSupported(MinimumNovaMicroversion, serviceMin, serviceMax) + if err != nil { + return nil, fmt.Errorf("unable to verify compatible server version: %w", err) + } + if !supported { + return nil, fmt.Errorf("no compatible server version. CAPO requires %s, but min=%s and max=%s", + MinimumNovaMicroversion, serviceMin, serviceMax) + } + + compute.Microversion = MinimumNovaMicroversion + + return &computeClient{client: compute, minVersion: serviceMin, maxVersion: serviceMax}, nil } func (c computeClient) ListAvailabilityZones() ([]availabilityzones.AvailabilityZone, error) { @@ -154,6 +186,21 @@ func (c computeClient) ListServerGroups() ([]servergroups.ServerGroup, error) { return servergroups.ExtractServerGroups(allPages) } +// WithMicroversion checks that the required Nova microversion is supported and sets it for +// the ComputeClient. +func (c computeClient) WithMicroversion(required string) (ComputeClient, error) { + supported, err := openstackutil.MicroversionSupported(required, c.minVersion, c.maxVersion) + if err != nil { + return nil, err + } + if !supported { + return nil, fmt.Errorf("microversion %s not supported. Min=%s, max=%s", required, c.minVersion, c.maxVersion) + } + versionedClient := c + versionedClient.client.Microversion = required + return versionedClient, nil +} + type computeErrorClient struct{ error } // NewComputeErrorClient returns a ComputeClient in which every method returns the given error. @@ -196,3 +243,7 @@ func (e computeErrorClient) DeleteAttachedInterface(_, _ string) error { func (e computeErrorClient) ListServerGroups() ([]servergroups.ServerGroup, error) { return nil, e.error } + +func (e computeErrorClient) WithMicroversion(_ string) (ComputeClient, error) { + return nil, e.error +} diff --git a/pkg/clients/mock/compute.go b/pkg/clients/mock/compute.go index 7bf2e5c742..2ad388233b 100644 --- a/pkg/clients/mock/compute.go +++ b/pkg/clients/mock/compute.go @@ -33,6 +33,7 @@ import ( servergroups "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servergroups" servers "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servers" gomock "go.uber.org/mock/gomock" + clients "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients" ) // MockComputeClient is a mock of ComputeClient interface. @@ -191,3 +192,18 @@ func (mr *MockComputeClientMockRecorder) ListServers(listOpts any) *gomock.Call mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListServers", reflect.TypeOf((*MockComputeClient)(nil).ListServers), listOpts) } + +// WithMicroversion mocks base method. +func (m *MockComputeClient) WithMicroversion(required string) (clients.ComputeClient, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "WithMicroversion", required) + ret0, _ := ret[0].(clients.ComputeClient) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// WithMicroversion indicates an expected call of WithMicroversion. +func (mr *MockComputeClientMockRecorder) WithMicroversion(required any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WithMicroversion", reflect.TypeOf((*MockComputeClient)(nil).WithMicroversion), required) +} diff --git a/pkg/cloud/services/compute/instance.go b/pkg/cloud/services/compute/instance.go index afaaa7396d..8b21c672ef 100644 --- a/pkg/cloud/services/compute/instance.go +++ b/pkg/cloud/services/compute/instance.go @@ -36,6 +36,7 @@ import ( orcv1alpha1 "github.com/k-orc/openstack-resource-controller/api/v1alpha1" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients" "sigs.k8s.io/cluster-api-provider-openstack/pkg/record" capoerrors "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/errors" "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/filterconvert" @@ -104,7 +105,16 @@ func (s *Service) createInstanceImpl(eventObject runtime.Object, instanceSpec *I } } - server, err := s.getComputeClient().CreateServer( + compute := s.getComputeClient() + if requiresTagging(instanceSpec) { + s.scope.Logger().V(4).Info("Tagging support is required for creating this Openstack instance") + computeWithTags, err := compute.WithMicroversion(clients.NovaTagging) + if err != nil { + return nil, fmt.Errorf("tagging is not supported by the server: %w", err) + } + compute = computeWithTags + } + server, err := compute.CreateServer( keypairs.CreateOptsExt{ CreateOptsBuilder: serverCreateOpts, KeyName: instanceSpec.SSHKeyName, @@ -591,3 +601,13 @@ func getTimeout(name string, timeout int) time.Duration { } return time.Duration(timeout) } + +// requiresTagging checks if the instanceSpec requires tagging, +// i.e. if it is using tags in some way. +func requiresTagging(instanceSpec *InstanceSpec) bool { + // All AdditionalBlockDevices are always tagged. + if len(instanceSpec.Tags) > 0 || len(instanceSpec.AdditionalBlockDevices) > 0 { + return true + } + return false +} diff --git a/pkg/cloud/services/compute/instance_test.go b/pkg/cloud/services/compute/instance_test.go index 3de57656bb..50988574bb 100644 --- a/pkg/cloud/services/compute/instance_test.go +++ b/pkg/cloud/services/compute/instance_test.go @@ -40,6 +40,7 @@ import ( orcv1alpha1 "github.com/k-orc/openstack-resource-controller/api/v1alpha1" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients" "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients/mock" "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" capoerrors "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/errors" @@ -380,7 +381,8 @@ func TestService_ReconcileInstance(t *testing.T) { } // Expected calls and custom match function for creating a server - expectCreateServer := func(g Gomega, computeRecorder *mock.MockComputeClientMockRecorder, expectedCreateOpts servers.CreateOptsBuilder, expectedSchedulerHintOpts servers.SchedulerHintOptsBuilder, wantError bool) { + expectCreateServer := func(g Gomega, computeRecorder *mock.MockComputeClientMockRecorder, expectedCreateOpts servers.CreateOptsBuilder, expectedSchedulerHintOpts servers.SchedulerHintOptsBuilder, computeClient clients.ComputeClient, wantError bool) { + computeRecorder.WithMicroversion(clients.NovaTagging).Return(computeClient, nil) computeRecorder.CreateServer(gomock.Any(), gomock.Any()).DoAndReturn(func(createOpts servers.CreateOptsBuilder, schedulerHintOpts servers.SchedulerHintOptsBuilder) (*servers.Server, error) { createOptsMap, _ := createOpts.ToServerCreateMap() expectedCreateOptsMap, _ := expectedCreateOpts.ToServerCreateMap() @@ -397,6 +399,10 @@ func TestService_ReconcileInstance(t *testing.T) { }) } + expectUnsupportedMicroversion := func(computeRecorder *mock.MockComputeClientMockRecorder) { + computeRecorder.WithMicroversion(clients.NovaTagging).Return(nil, errors.New("unsupported microversion")) + } + returnedVolume := func(uuid string, status string) *volumes.Volume { return &volumes.Volume{ ID: uuid, @@ -429,14 +435,14 @@ func TestService_ReconcileInstance(t *testing.T) { tests := []struct { name string getInstanceSpec func() *InstanceSpec - expect func(g Gomega, r *recorders) + expect func(g Gomega, r *recorders, factory *scope.MockScopeFactory) wantErr bool }{ { name: "Defaults", getInstanceSpec: getDefaultInstanceSpec, - expect: func(g Gomega, r *recorders) { - expectCreateServer(g, r.compute, withSSHKey(getDefaultServerCreateOpts()), getDefaultSchedulerHintOpts(), false) + expect: func(g Gomega, r *recorders, factory *scope.MockScopeFactory) { + expectCreateServer(g, r.compute, withSSHKey(getDefaultServerCreateOpts()), getDefaultSchedulerHintOpts(), factory.ComputeClient, false) }, wantErr: false, }, @@ -449,7 +455,7 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(g Gomega, r *recorders) { + expect: func(g Gomega, r *recorders, factory *scope.MockScopeFactory) { r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-root", openStackMachineName)}). Return([]volumes.Volume{}, nil) r.volume.CreateVolume(volumes.CreateOpts{ @@ -471,7 +477,7 @@ func TestService_ReconcileInstance(t *testing.T) { DestinationType: "volume", }, } - expectCreateServer(g, r.compute, withSSHKey(createOpts), getDefaultSchedulerHintOpts(), false) + expectCreateServer(g, r.compute, withSSHKey(createOpts), getDefaultSchedulerHintOpts(), factory.ComputeClient, false) // Don't delete ports because the server is created: DeleteInstance will do it }, @@ -491,7 +497,7 @@ func TestService_ReconcileInstance(t *testing.T) { s.RootVolume.Type = "test-volume-type" return s }, - expect: func(g Gomega, r *recorders) { + expect: func(g Gomega, r *recorders, factory *scope.MockScopeFactory) { r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-root", openStackMachineName)}). Return([]volumes.Volume{}, nil) r.volume.CreateVolume(volumes.CreateOpts{ @@ -515,7 +521,7 @@ func TestService_ReconcileInstance(t *testing.T) { DestinationType: "volume", }, } - expectCreateServer(g, r.compute, withSSHKey(createOpts), getDefaultSchedulerHintOpts(), false) + expectCreateServer(g, r.compute, withSSHKey(createOpts), getDefaultSchedulerHintOpts(), factory.ComputeClient, false) // Don't delete ports because the server is created: DeleteInstance will do it }, @@ -534,7 +540,7 @@ func TestService_ReconcileInstance(t *testing.T) { s.RootVolume.Type = "test-volume-type" return s }, - expect: func(g Gomega, r *recorders) { + expect: func(g Gomega, r *recorders, factory *scope.MockScopeFactory) { r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-root", openStackMachineName)}). Return([]volumes.Volume{}, nil) r.volume.CreateVolume(volumes.CreateOpts{ @@ -558,7 +564,7 @@ func TestService_ReconcileInstance(t *testing.T) { DestinationType: "volume", }, } - expectCreateServer(g, r.compute, withSSHKey(createOpts), getDefaultSchedulerHintOpts(), false) + expectCreateServer(g, r.compute, withSSHKey(createOpts), getDefaultSchedulerHintOpts(), factory.ComputeClient, false) // Don't delete ports because the server is created: DeleteInstance will do it }, @@ -573,7 +579,7 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(_ Gomega, r *recorders) { + expect: func(_ Gomega, r *recorders, _ *scope.MockScopeFactory) { r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-root", openStackMachineName)}). Return([]volumes.Volume{}, nil) r.volume.CreateVolume(volumes.CreateOpts{ @@ -614,7 +620,7 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(g Gomega, r *recorders) { + expect: func(g Gomega, r *recorders, factory *scope.MockScopeFactory) { r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-root", openStackMachineName)}). Return([]volumes.Volume{}, nil) r.volume.CreateVolume(volumes.CreateOpts{ @@ -662,7 +668,7 @@ func TestService_ReconcileInstance(t *testing.T) { Tag: "local-device", }, } - expectCreateServer(g, r.compute, withSSHKey(createOpts), getDefaultSchedulerHintOpts(), false) + expectCreateServer(g, r.compute, withSSHKey(createOpts), getDefaultSchedulerHintOpts(), factory.ComputeClient, false) // Don't delete ports because the server is created: DeleteInstance will do it }, @@ -693,7 +699,7 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(g Gomega, r *recorders) { + expect: func(g Gomega, r *recorders, factory *scope.MockScopeFactory) { r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-etcd", openStackMachineName)}). Return([]volumes.Volume{}, nil) r.volume.CreateVolume(volumes.CreateOpts{ @@ -730,7 +736,7 @@ func TestService_ReconcileInstance(t *testing.T) { Tag: "data", }, } - expectCreateServer(g, r.compute, withSSHKey(createOpts), getDefaultSchedulerHintOpts(), false) + expectCreateServer(g, r.compute, withSSHKey(createOpts), getDefaultSchedulerHintOpts(), factory.ComputeClient, false) // Don't delete ports because the server is created: DeleteInstance will do it }, @@ -758,7 +764,7 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(g Gomega, r *recorders) { + expect: func(g Gomega, r *recorders, factory *scope.MockScopeFactory) { r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-etcd", openStackMachineName)}). Return([]volumes.Volume{}, nil) r.volume.CreateVolume(volumes.CreateOpts{ @@ -788,7 +794,7 @@ func TestService_ReconcileInstance(t *testing.T) { Tag: "etcd", }, } - expectCreateServer(g, r.compute, withSSHKey(createOpts), getDefaultSchedulerHintOpts(), false) + expectCreateServer(g, r.compute, withSSHKey(createOpts), getDefaultSchedulerHintOpts(), factory.ComputeClient, false) // Don't delete ports because the server is created: DeleteInstance will do it }, @@ -809,7 +815,7 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(_ Gomega, _ *recorders) { + expect: func(_ Gomega, _ *recorders, _ *scope.MockScopeFactory) { }, wantErr: true, }, @@ -828,7 +834,7 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(g Gomega, r *recorders) { + expect: func(g Gomega, r *recorders, factory *scope.MockScopeFactory) { createOpts := getDefaultServerCreateOpts() schedulerHintOpts := servers.SchedulerHintOpts{ Group: serverGroupUUID, @@ -836,7 +842,7 @@ func TestService_ReconcileInstance(t *testing.T) { "custom_hint": true, }, } - expectCreateServer(g, r.compute, withSSHKey(createOpts), schedulerHintOpts, false) + expectCreateServer(g, r.compute, withSSHKey(createOpts), schedulerHintOpts, factory.ComputeClient, false) }, wantErr: false, }, @@ -855,7 +861,7 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(g Gomega, r *recorders) { + expect: func(g Gomega, r *recorders, factory *scope.MockScopeFactory) { createOpts := getDefaultServerCreateOpts() schedulerHintOpts := servers.SchedulerHintOpts{ Group: serverGroupUUID, @@ -863,7 +869,7 @@ func TestService_ReconcileInstance(t *testing.T) { "custom_hint": 1, }, } - expectCreateServer(g, r.compute, withSSHKey(createOpts), schedulerHintOpts, false) + expectCreateServer(g, r.compute, withSSHKey(createOpts), schedulerHintOpts, factory.ComputeClient, false) }, wantErr: false, }, @@ -882,7 +888,7 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(g Gomega, r *recorders) { + expect: func(g Gomega, r *recorders, factory *scope.MockScopeFactory) { createOpts := getDefaultServerCreateOpts() schedulerHintOpts := servers.SchedulerHintOpts{ Group: serverGroupUUID, @@ -890,10 +896,18 @@ func TestService_ReconcileInstance(t *testing.T) { "custom_hint": "custom hint", }, } - expectCreateServer(g, r.compute, withSSHKey(createOpts), schedulerHintOpts, false) + expectCreateServer(g, r.compute, withSSHKey(createOpts), schedulerHintOpts, factory.ComputeClient, false) }, wantErr: false, }, + { + name: "Unsupported Nova microversion", + getInstanceSpec: getDefaultInstanceSpec, + expect: func(_ Gomega, r *recorders, _ *scope.MockScopeFactory) { + expectUnsupportedMicroversion(r.compute) + }, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -907,7 +921,7 @@ func TestService_ReconcileInstance(t *testing.T) { networkRecorder := mockScopeFactory.NetworkClient.EXPECT() volumeRecorder := mockScopeFactory.VolumeClient.EXPECT() - tt.expect(g, &recorders{computeRecorder, imageRecorder, networkRecorder, volumeRecorder}) + tt.expect(g, &recorders{computeRecorder, imageRecorder, networkRecorder, volumeRecorder}, mockScopeFactory) s, err := NewService(scope.NewWithLogger(mockScopeFactory, log)) if err != nil { diff --git a/pkg/utils/openstack/microversion.go b/pkg/utils/openstack/microversion.go new file mode 100644 index 0000000000..72478a40e7 --- /dev/null +++ b/pkg/utils/openstack/microversion.go @@ -0,0 +1,98 @@ +/* +Copyright 2023 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. +*/ + +package openstack + +import ( + "context" + "fmt" + "strconv" + "strings" + + "github.com/gophercloud/gophercloud/v2" +) + +// GetSupportedMicroversions returns the minimum and maximum microversion that is supported by the ServiceClient Endpoint. +func GetSupportedMicroversions(client gophercloud.ServiceClient) (string, string, error) { + type valueResp struct { + ID string `json:"id"` + Status string `json:"status"` + Version string `json:"version"` + MinVersion string `json:"min_version"` + } + + type response struct { + Version valueResp `json:"version"` + } + var resp response + _, err := client.Request(context.TODO(), "GET", client.Endpoint, &gophercloud.RequestOpts{ + JSONResponse: &resp, + OkCodes: []int{200, 300}, + }) + if err != nil { + return "", "", err + } + + return resp.Version.MinVersion, resp.Version.Version, nil +} + +// MicroversionSupported checks if a microversion falls in the supported interval. +// It returns true if the version is within the interval and false otherwise. +func MicroversionSupported(version string, minVersion string, maxVersion string) (bool, error) { + // Parse the version X.Y into X and Y integers that are easier to compare. + vMajor, v, err := parseMicroversion(version) + if err != nil { + return false, err + } + minMajor, minimum, err := parseMicroversion(minVersion) + if err != nil { + return false, err + } + maxMajor, maximum, err := parseMicroversion(maxVersion) + if err != nil { + return false, err + } + + // Check that the major version number is supported. + if (vMajor < minMajor) || (vMajor > maxMajor) { + return false, err + } + + // Check that the minor version number is supported + if (v <= maximum) && (v >= minimum) { + return true, nil + } + + return false, nil +} + +// parseMicroversion parses the version X.Y into separate integers X and Y. +// For example, "2.53" becomes 2 and 53. +func parseMicroversion(version string) (int, int, error) { + parts := strings.Split(version, ".") + if len(parts) != 2 { + return 0, 0, fmt.Errorf("invalid microversion format: %q", version) + } + major, err := strconv.Atoi(parts[0]) + if err != nil { + return 0, 0, err + } + minor, err := strconv.Atoi(parts[1]) + if err != nil { + return 0, 0, err + } + return major, minor, nil +} diff --git a/pkg/utils/openstack/microversion_test.go b/pkg/utils/openstack/microversion_test.go new file mode 100644 index 0000000000..212870db09 --- /dev/null +++ b/pkg/utils/openstack/microversion_test.go @@ -0,0 +1,97 @@ +/* +Copyright 2023 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. +*/ + +package openstack + +import "testing" + +type microversionSupported struct { + Version string + MinVersion string + MaxVersion string + Supported bool + Error bool +} + +func TestMicroversionSupported(t *testing.T) { + tests := []microversionSupported{ + { + // Checking min version + Version: "2.1", + MinVersion: "2.1", + MaxVersion: "2.90", + Supported: true, + Error: false, + }, + { + // Checking max version + Version: "2.90", + MinVersion: "2.1", + MaxVersion: "2.90", + Supported: true, + Error: false, + }, + { + // Checking too high version + Version: "2.95", + MinVersion: "2.1", + MaxVersion: "2.90", + Supported: false, + Error: false, + }, + { + // Checking too low version + Version: "2.1", + MinVersion: "2.53", + MaxVersion: "2.90", + Supported: false, + Error: false, + }, + { + // Invalid version + Version: "2.1.53", + MinVersion: "2.53", + MaxVersion: "2.90", + Supported: false, + Error: true, + }, + { + // No microversions supported + Version: "2.1", + MinVersion: "", + MaxVersion: "", + Supported: false, + Error: true, + }, + } + + for _, test := range tests { + supported, err := MicroversionSupported(test.Version, test.MinVersion, test.MaxVersion) + if test.Error { + if err == nil { + t.Error("Expected error but got none!") + } + } else { + if err != nil { + t.Errorf("Expected no error but got %s", err.Error()) + } + } + if test.Supported != supported { + t.Errorf("Expected supported=%t to be %t, when version=%s, min=%s and max=%s", + supported, test.Supported, test.Version, test.MinVersion, test.MaxVersion) + } + } +} diff --git a/test/e2e/shared/openstack.go b/test/e2e/shared/openstack.go index 1e60b7163a..49b1cdf8b1 100644 --- a/test/e2e/shared/openstack.go +++ b/test/e2e/shared/openstack.go @@ -260,7 +260,12 @@ func DumpOpenStackServers(e2eCtx *E2EContext, filter servers.ListOpts) ([]server return nil, fmt.Errorf("error creating compute client: %v", err) } - computeClient.Microversion = clients.NovaMinimumMicroversion + computeClient.Microversion = clients.MinimumNovaMicroversion + // TODO: We have a ServieClient here (not ComputeClient), which means we do not have access to `RequireMicroversion`. + // Maybe we can fix it by implementing `RequireMicroversion` in gophercloud? + if filter.Tags != "" || filter.TagsAny != "" || filter.NotTags != "" || filter.NotTagsAny != "" { + computeClient.Microversion = clients.NovaTagging + } allPages, err := servers.List(computeClient, filter).AllPages(context.TODO()) if err != nil { return nil, fmt.Errorf("error listing servers: %v", err)