Skip to content

Commit

Permalink
WIP: Require specific microversions based on features
Browse files Browse the repository at this point in the history
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
  • Loading branch information
lentzi90 committed Oct 5, 2023
1 parent ade1ed0 commit f931f03
Show file tree
Hide file tree
Showing 7 changed files with 296 additions and 9 deletions.
62 changes: 55 additions & 7 deletions pkg/clients/compute.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,28 @@ import (
uflavors "github.com/gophercloud/utils/openstack/compute/v2/flavors"

"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.
*/
const NovaMinimumMicroversion = "2.60"
const (
MinimumNovaMicroversion = "2.38"
NovaTagging = "2.53"
MultiAttachVolume = "2.60"
)

// ServerExt is the base gophercloud Server with extensions used by InstanceStatus.
type ServerExt struct {
Expand All @@ -60,9 +69,14 @@ type ComputeClient interface {

ListAttachedInterfaces(serverID string) ([]attachinterfaces.Interface, error)
DeleteAttachedInterface(serverID, portID string) error
RequireMicroversion(required string) 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) {
Expand All @@ -72,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) {
Expand Down Expand Up @@ -148,6 +178,20 @@ func (c computeClient) DeleteAttachedInterface(serverID, portID string) error {
return mc.ObserveRequestIgnoreNotFoundorConflict(err)
}

// RequireMicroversion checks that the required Nova microversion is supported and sets it for
// the ComputeClient.
func (c computeClient) RequireMicroversion(required string) error {
supported, err := openstackutil.MicroversionSupported(required, c.minVersion, c.maxVersion)
if err != nil {
return err
}
if !supported {
return fmt.Errorf("microversion %s not supported. Min=%s, max=%s", required, c.minVersion, c.maxVersion)
}
c.client.Microversion = required
return nil
}

type computeErrorClient struct{ error }

// NewComputeErrorClient returns a ComputeClient in which every method returns the given error.
Expand Down Expand Up @@ -186,3 +230,7 @@ func (e computeErrorClient) ListAttachedInterfaces(_ string) ([]attachinterfaces
func (e computeErrorClient) DeleteAttachedInterface(_, _ string) error {
return e.error
}

func (e computeErrorClient) RequireMicroversion(_ string) error {
return e.error
}
14 changes: 14 additions & 0 deletions pkg/clients/mock/compute.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 8 additions & 1 deletion pkg/cloud/services/compute/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,14 @@ func (s *Service) createInstanceImpl(eventObject runtime.Object, openStackCluste

serverCreateOpts = applyServerGroupID(serverCreateOpts, instanceSpec.ServerGroupID)

server, err = s.getComputeClient().CreateServer(keypairs.CreateOptsExt{
compute := s.getComputeClient()
if len(instanceSpec.Tags) > 0 {
err = compute.RequireMicroversion(clients.NovaTagging)
if err != nil {
return nil, fmt.Errorf("tagging is not supported by the server: %w", err)
}
}
server, err = compute.CreateServer(keypairs.CreateOptsExt{
CreateOptsBuilder: serverCreateOpts,
KeyName: instanceSpec.SSHKeyName,
})
Expand Down
19 changes: 19 additions & 0 deletions pkg/cloud/services/compute/instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package compute

import (
"encoding/base64"
"errors"
"fmt"
"testing"
"time"
Expand Down Expand Up @@ -387,6 +388,8 @@ func TestService_ReconcileInstance(t *testing.T) {

// Expected calls and custom match function for creating a server
expectCreateServer := func(computeRecorder *mock.MockComputeClientMockRecorder, expectedCreateOpts map[string]interface{}, wantError bool) {
computeRecorder.RequireMicroversion(clients.NovaTagging).Return(nil)

// This nonsense is because ConfigDrive is a bool pointer, so we
// can't assert its exact contents with gomock.
// Instead we call ToServerCreateMap() on it to obtain a
Expand All @@ -406,6 +409,10 @@ func TestService_ReconcileInstance(t *testing.T) {
})
}

expectUnsupportedMicroversion := func(computeRecorder *mock.MockComputeClientMockRecorder) {
computeRecorder.RequireMicroversion(clients.NovaTagging).Return(errors.New("unsupported microversion"))
}

// Expected calls when polling for server creation
expectServerPoll := func(computeRecorder *mock.MockComputeClientMockRecorder, states []string) {
for _, state := range states {
Expand Down Expand Up @@ -888,6 +895,18 @@ func TestService_ReconcileInstance(t *testing.T) {
},
wantErr: true,
},
{
name: "Unsupported Nova microversion",
getInstanceSpec: getDefaultInstanceSpec,
expect: func(r *recorders) {
expectUseExistingDefaultPort(r.network)
expectDefaultImageAndFlavor(r.compute, r.image)

expectUnsupportedMicroversion(r.compute)
expectCleanupDefaultPort(r.network)
},
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
97 changes: 97 additions & 0 deletions pkg/utils/openstack/microversion.go
Original file line number Diff line number Diff line change
@@ -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 (
"fmt"
"strconv"
"strings"

"github.com/gophercloud/gophercloud"
)

// 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("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, min, err := parseMicroversion(minVersion)
if err != nil {
return false, err
}
maxMajor, max, 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 <= max) && (v >= min) {
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
}
97 changes: 97 additions & 0 deletions pkg/utils/openstack/microversion_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
7 changes: 6 additions & 1 deletion test/e2e/shared/openstack.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,12 @@ func DumpOpenStackServers(e2eCtx *E2EContext, filter servers.ListOpts) ([]server
return nil, fmt.Errorf("error creating compute client: %v", err)
}

computeClient.Microversion = clients.NovaMinimumMicroversion
// 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()
if err != nil {
return nil, fmt.Errorf("error listing servers: %v", err)
Expand Down

0 comments on commit f931f03

Please sign in to comment.