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 no special features are used/needed
then we use the fixed minimum version

Signed-off-by: Lennart Jern <[email protected]>

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
+}

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
+}