Skip to content

Commit eff6478

Browse files
committed
Require specific microversions based on features
This is an attempt to set the microversion based on what features are needed. For example, if tags are defined for the server, then we need a microversion that supports tags. If volumes that are multiattach capable are used, we need support for that. If no special features are used/needed then we use the fixed minimum version. Signed-off-by: Lennart Jern <[email protected]>
1 parent da4fb46 commit eff6478

File tree

7 files changed

+452
-34
lines changed

7 files changed

+452
-34
lines changed

pkg/clients/compute.go

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,30 @@ import (
3030
"github.com/gophercloud/utils/v2/openstack/clientconfig"
3131

3232
"sigs.k8s.io/cluster-api-provider-openstack/pkg/metrics"
33+
openstackutil "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/openstack"
3334
)
3435

3536
/*
36-
NovaMinimumMicroversion is the minimum Nova microversion supported by CAPO
37-
2.60 corresponds to OpenStack Queens
37+
Constants for specific microversion requirements.
38+
2.60 corresponds to OpenStack Queens and 2.53 to OpenStack Pike,
39+
2.38 is the maximum in OpenStack Newton.
3840
3941
For the canonical description of Nova microversions, see
4042
https://docs.openstack.org/nova/latest/reference/api-microversion-history.html
4143
42-
CAPO uses server tags, which were added in microversion 2.52.
44+
CAPO uses server tags, which were first added in microversion 2.26 and then refined
45+
in 2.52 so it is possible to apply them when creating a server (which is what CAPO does).
46+
We round up to 2.53 here since that takes us to the maximum in Pike.
47+
4348
CAPO supports multiattach volume types, which were added in microversion 2.60.
49+
50+
2.38 was chosen as a base level since it is reasonably old, but not too old.
4451
*/
45-
const NovaMinimumMicroversion = "2.60"
52+
const (
53+
MinimumNovaMicroversion = "2.38"
54+
NovaTagging = "2.53"
55+
NovaMultiAttachVolume = "2.60"
56+
)
4657

4758
type ComputeClient interface {
4859
ListAvailabilityZones() ([]availabilityzones.AvailabilityZone, error)
@@ -57,9 +68,14 @@ type ComputeClient interface {
5768
DeleteAttachedInterface(serverID, portID string) error
5869

5970
ListServerGroups() ([]servergroups.ServerGroup, error)
71+
WithMicroversion(required string) (ComputeClient, error)
6072
}
6173

62-
type computeClient struct{ client *gophercloud.ServiceClient }
74+
type computeClient struct {
75+
client *gophercloud.ServiceClient
76+
minVersion string
77+
maxVersion string
78+
}
6379

6480
// NewComputeClient returns a new compute client.
6581
func NewComputeClient(providerClient *gophercloud.ProviderClient, providerClientOpts *clientconfig.ClientOpts) (ComputeClient, error) {
@@ -70,9 +86,25 @@ func NewComputeClient(providerClient *gophercloud.ProviderClient, providerClient
7086
if err != nil {
7187
return nil, fmt.Errorf("failed to create compute service client: %v", err)
7288
}
73-
compute.Microversion = NovaMinimumMicroversion
7489

75-
return &computeClient{compute}, nil
90+
// Find the minimum and maximum versions supported by the server
91+
serviceMin, serviceMax, err := openstackutil.GetSupportedMicroversions(*compute)
92+
if err != nil {
93+
return nil, fmt.Errorf("unable to verify compatible server version: %w", err)
94+
}
95+
96+
supported, err := openstackutil.MicroversionSupported(MinimumNovaMicroversion, serviceMin, serviceMax)
97+
if err != nil {
98+
return nil, fmt.Errorf("unable to verify compatible server version: %w", err)
99+
}
100+
if !supported {
101+
return nil, fmt.Errorf("no compatible server version. CAPO requires %s, but min=%s and max=%s",
102+
MinimumNovaMicroversion, serviceMin, serviceMax)
103+
}
104+
105+
compute.Microversion = MinimumNovaMicroversion
106+
107+
return &computeClient{client: compute, minVersion: serviceMin, maxVersion: serviceMax}, nil
76108
}
77109

78110
func (c computeClient) ListAvailabilityZones() ([]availabilityzones.AvailabilityZone, error) {
@@ -154,6 +186,21 @@ func (c computeClient) ListServerGroups() ([]servergroups.ServerGroup, error) {
154186
return servergroups.ExtractServerGroups(allPages)
155187
}
156188

189+
// WithMicroversion checks that the required Nova microversion is supported and sets it for
190+
// the ComputeClient.
191+
func (c computeClient) WithMicroversion(required string) (ComputeClient, error) {
192+
supported, err := openstackutil.MicroversionSupported(required, c.minVersion, c.maxVersion)
193+
if err != nil {
194+
return nil, err
195+
}
196+
if !supported {
197+
return nil, fmt.Errorf("microversion %s not supported. Min=%s, max=%s", required, c.minVersion, c.maxVersion)
198+
}
199+
versionedClient := c
200+
versionedClient.client.Microversion = required
201+
return versionedClient, nil
202+
}
203+
157204
type computeErrorClient struct{ error }
158205

159206
// NewComputeErrorClient returns a ComputeClient in which every method returns the given error.
@@ -196,3 +243,7 @@ func (e computeErrorClient) DeleteAttachedInterface(_, _ string) error {
196243
func (e computeErrorClient) ListServerGroups() ([]servergroups.ServerGroup, error) {
197244
return nil, e.error
198245
}
246+
247+
func (e computeErrorClient) WithMicroversion(_ string) (ComputeClient, error) {
248+
return nil, e.error
249+
}

pkg/clients/mock/compute.go

Lines changed: 16 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/cloud/services/compute/instance.go

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
orcv1alpha1 "github.com/k-orc/openstack-resource-controller/api/v1alpha1"
3737

3838
infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
39+
"sigs.k8s.io/cluster-api-provider-openstack/pkg/clients"
3940
"sigs.k8s.io/cluster-api-provider-openstack/pkg/record"
4041
capoerrors "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/errors"
4142
"sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/filterconvert"
@@ -104,7 +105,29 @@ func (s *Service) createInstanceImpl(eventObject runtime.Object, instanceSpec *I
104105
}
105106
}
106107

107-
server, err := s.getComputeClient().CreateServer(
108+
compute := s.getComputeClient()
109+
if requiresTagging(instanceSpec) {
110+
s.scope.Logger().V(4).Info("Tagging support is required for creating this Openstack instance")
111+
computeWithTags, err := compute.WithMicroversion(clients.NovaTagging)
112+
if err != nil {
113+
return nil, fmt.Errorf("tagging is not supported by the server: %w", err)
114+
}
115+
compute = computeWithTags
116+
}
117+
118+
multiattachRequired, err := s.requiresMultiattach(blockDevices)
119+
if err != nil {
120+
return nil, fmt.Errorf("error checking if multiattach is required: %w", err)
121+
}
122+
if multiattachRequired {
123+
s.scope.Logger().V(4).Info("Multiattach support is required for creating this Openstack instance")
124+
computeWithMultiattach, err := compute.WithMicroversion(clients.NovaMultiAttachVolume)
125+
if err != nil {
126+
return nil, fmt.Errorf("multiattach is not supported by the server: %w", err)
127+
}
128+
compute = computeWithMultiattach
129+
}
130+
server, err := compute.CreateServer(
108131
keypairs.CreateOptsExt{
109132
CreateOptsBuilder: serverCreateOpts,
110133
KeyName: instanceSpec.SSHKeyName,
@@ -591,3 +614,33 @@ func getTimeout(name string, timeout int) time.Duration {
591614
}
592615
return time.Duration(timeout)
593616
}
617+
618+
// requiresTagging checks if the instanceSpec requires tagging,
619+
// i.e. if it is using tags in some way.
620+
func requiresTagging(instanceSpec *InstanceSpec) bool {
621+
// All AdditionalBlockDevices are always tagged.
622+
if len(instanceSpec.Tags) > 0 || len(instanceSpec.AdditionalBlockDevices) > 0 {
623+
return true
624+
}
625+
return false
626+
}
627+
628+
// requiresMultiattach checks if there is any volume in the blockDevices that requires multiattach.
629+
// Note that we are not checking the default volume type or the volume type configured in the
630+
// image metadata. We assume that these are NOT multiattach.
631+
func (s *Service) requiresMultiattach(blockDevices []servers.BlockDevice) (bool, error) {
632+
for _, blockDevice := range blockDevices {
633+
// Only check volumes
634+
if blockDevice.SourceType == servers.SourceVolume {
635+
volume, err := s.getVolumeClient().GetVolume(blockDevice.UUID)
636+
if err != nil {
637+
return false, err
638+
}
639+
if volume.Multiattach {
640+
return true, nil
641+
}
642+
}
643+
}
644+
// We assume that the default volume type is NOT multiattach
645+
return false, nil
646+
}

0 commit comments

Comments
 (0)