From 01234f4db9c39209f592a96c0ed7a0060e1da6aa Mon Sep 17 00:00:00 2001 From: Matthias Baur Date: Tue, 10 Dec 2024 12:40:33 +0100 Subject: [PATCH] Refactor to proper openstackclient --- internal/openstackclient/openstackclient.go | 190 ++++++++++++++++++ .../openstackclient/openstackclient_test.go | 38 ++++ provider.go | 120 ++--------- utils.go | 31 --- utils_test.go | 26 --- 5 files changed, 248 insertions(+), 157 deletions(-) create mode 100644 internal/openstackclient/openstackclient.go create mode 100644 internal/openstackclient/openstackclient_test.go diff --git a/internal/openstackclient/openstackclient.go b/internal/openstackclient/openstackclient.go new file mode 100644 index 0000000..247490d --- /dev/null +++ b/internal/openstackclient/openstackclient.go @@ -0,0 +1,190 @@ +package openstackclient + +import ( + "context" + "crypto/tls" + "fmt" + "os" + + "github.com/gophercloud/gophercloud/v2" + "github.com/gophercloud/gophercloud/v2/openstack" + "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servers" + "github.com/gophercloud/gophercloud/v2/openstack/config" + "github.com/gophercloud/gophercloud/v2/openstack/config/clouds" + "github.com/gophercloud/gophercloud/v2/openstack/image/v2/images" + "github.com/gophercloud/gophercloud/v2/openstack/utils" + "github.com/mitchellh/mapstructure" +) + +type AuthConfig struct { + // AuthFromEnv specifies whether to use environment variables for auth + AuthFromEnv bool + + // Cloud is the name of the cloud config from clouds.yaml to use + Cloud string + + // CloudsConfig is the path to the clouds.yaml file + CloudsConfig string + + // NovaMicroversion is the microversion of the OpenStack Nova client. Default 2.79 (which should be ok for Train+) + NovaMicroversion string +} + +// Some good known properties useful for setting up ConnectInfo +// +// See also: https://docs.openstack.org/glance/latest/admin/useful-image-properties.html +type ImageProperties struct { + // Architecture that must be supported by the hypervisor. + Architecture string `json:"architecture,omitempty" mapstructure:"architecture,omitempty"` + + // OSType is the operating system installed on the image. + OSType string `json:"os_type,omitempty" mapstructure:"os_type,omitempty"` + + // OSDistro is the common name of the operating system distribution in lowercase + OSDistro string `json:"os_distro,omitempty" mapstructure:"os_distro,omitempty"` + + // OSVersion is the operating system version as specified by the distributor. + OSVersion string `json:"os_version,omitempty" mapstructure:"os_version,omitempty"` + + // OSAdminUser is the default admin user name for the operating system + OSAdminUser string `json:"os_admin_user,omitempty" mapstructure:"os_admin_user,omitempty"` +} + +type Client interface { + GetImageProperties(ctx context.Context, imageRef string) (*ImageProperties, error) + ShowServerConsoleOutput(ctx context.Context, serverId string) (string, error) + GetServer(ctx context.Context, serverId string) (*servers.Server, error) + ListServers(ctx context.Context) ([]servers.Server, error) + CreateServer(ctx context.Context, spec servers.CreateOptsBuilder, hintOpts servers.SchedulerHintOptsBuilder) (*servers.Server, error) + DeleteServer(ctx context.Context, serverId string) error +} + +var _ Client = (*client)(nil) + +type client struct { + compute *gophercloud.ServiceClient + image *gophercloud.ServiceClient +} + +func New(authConfig AuthConfig) (Client, error) { + if authConfig.NovaMicroversion == "" { + authConfig.NovaMicroversion = "2.79" // Train+ + } + + providerClient, endpointOps, err := newProviderClient(authConfig) + if err != nil { + return nil, err + } + + computeClient, err := openstack.NewComputeV2(providerClient, endpointOps) + if err != nil { + return nil, err + } + + _computeClient, err := utils.RequireMicroversion(context.TODO(), *computeClient, authConfig.NovaMicroversion) + if err != nil { + return nil, fmt.Errorf("failed to request microversion %s for OpenStack Nova: %w", authConfig.NovaMicroversion, err) + } + + computeClient = &_computeClient + + imageClient, err := openstack.NewImageV2(providerClient, endpointOps) + if err != nil { + return nil, err + } + + return &client{ + compute: computeClient, + image: imageClient, + }, nil +} + +func newProviderClient(authConfig AuthConfig) (*gophercloud.ProviderClient, gophercloud.EndpointOpts, error) { + var endpointOps gophercloud.EndpointOpts + var authOptions gophercloud.AuthOptions + var providerClient *gophercloud.ProviderClient + + if authConfig.AuthFromEnv { + var err error + endpointOps = gophercloud.EndpointOpts{Region: os.Getenv("OS_REGION_NAME")} + authOptions, err = openstack.AuthOptionsFromEnv() + if err != nil { + return nil, gophercloud.EndpointOpts{}, fmt.Errorf("failed to get auth options from environment: %w", err) + } + authOptions.AllowReauth = true + + providerClient, err = openstack.AuthenticatedClient(context.Background(), authOptions) + if err != nil { + return nil, gophercloud.EndpointOpts{}, fmt.Errorf("failed to connect to OpenStack Keystone: %w", err) + } + } else { + var err error + var tlsCfg *tls.Config + cloudOpts := []clouds.ParseOption{clouds.WithCloudName(authConfig.Cloud)} + if authConfig.CloudsConfig != "" { + cloudOpts = append(cloudOpts, clouds.WithLocations(authConfig.CloudsConfig)) + } + + authOptions, endpointOps, tlsCfg, err = clouds.Parse(cloudOpts...) + if err != nil { + return nil, gophercloud.EndpointOpts{}, fmt.Errorf("failed to parse clouds.yaml: %w", err) + } + + // plugin is a long running process. force allow reauth + authOptions.AllowReauth = true + + providerClient, err = config.NewProviderClient(context.TODO(), authOptions, config.WithTLSConfig(tlsCfg)) + if err != nil { + return nil, gophercloud.EndpointOpts{}, fmt.Errorf("failed to connect to OpenStack Keystone: %w", err) + } + } + + return providerClient, endpointOps, nil +} + +func (c *client) GetImageProperties(ctx context.Context, imageRef string) (*ImageProperties, error) { + image, err := images.Get(ctx, c.image, imageRef).Extract() + if err != nil { + return nil, fmt.Errorf("failed to get image %s: %w", imageRef, err) + } + + out := new(ImageProperties) + err = mapstructure.Decode(image.Properties, out) + if err != nil { + return nil, fmt.Errorf("failed to parse properties: %w", err) + } + + return out, nil +} + +func (c *client) ShowServerConsoleOutput(ctx context.Context, serverId string) (string, error) { + return servers.ShowConsoleOutput(ctx, c.compute, serverId, servers.ShowConsoleOutputOpts{ + Length: 100, + }).Extract() +} + +func (c *client) GetServer(ctx context.Context, serverId string) (*servers.Server, error) { + return servers.Get(ctx, c.compute, serverId).Extract() +} + +func (c *client) ListServers(ctx context.Context) ([]servers.Server, error) { + page, err := servers.List(c.compute, nil).AllPages(ctx) + if err != nil { + return nil, fmt.Errorf("server listing error: %w", err) + } + + allServers, err := servers.ExtractServers(page) + if err != nil { + return nil, fmt.Errorf("server listing extract error: %w", err) + } + + return allServers, nil +} + +func (c *client) CreateServer(ctx context.Context, spec servers.CreateOptsBuilder, hintOpts servers.SchedulerHintOptsBuilder) (*servers.Server, error) { + return servers.Create(ctx, c.compute, spec, hintOpts).Extract() +} + +func (c *client) DeleteServer(ctx context.Context, serverId string) error { + return servers.Delete(ctx, c.compute, serverId).ExtractErr() +} diff --git a/internal/openstackclient/openstackclient_test.go b/internal/openstackclient/openstackclient_test.go new file mode 100644 index 0000000..0c942d0 --- /dev/null +++ b/internal/openstackclient/openstackclient_test.go @@ -0,0 +1,38 @@ +package openstackclient + +import ( + "context" + "os" + "testing" + + "github.com/gophercloud/gophercloud/v2/testhelper" + thclient "github.com/gophercloud/gophercloud/v2/testhelper/client" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetImageProperties(t *testing.T) { + assert := assert.New(t) + + img, err := os.ReadFile("../../testdata/image_get.json") + require.NoError(t, err) + + testhelper.SetupHTTP() + defer testhelper.TeardownHTTP() + + testhelper.ServeFile(t, "", "", "application/json", string(img)) + + client := &client{ + compute: thclient.ServiceClient(), + image: thclient.ServiceClient(), + } + + ctx := context.TODO() + props, err := client.GetImageProperties(ctx, "1da9661c-953e-424d-a1e5-834a8174b198") + assert.NoError(err) + if assert.NotNil(props) { + assert.Equal("core", props.OSAdminUser) + } + + t.Log(props) +} diff --git a/provider.go b/provider.go index a2e9536..93263a2 100644 --- a/provider.go +++ b/provider.go @@ -3,22 +3,16 @@ package fpoc import ( "bytes" "context" - "crypto/tls" "errors" "fmt" - "os" "path" "sync/atomic" "time" - "github.com/gophercloud/gophercloud/v2" - "github.com/gophercloud/gophercloud/v2/openstack" "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servers" - "github.com/gophercloud/gophercloud/v2/openstack/config" - clouds "github.com/gophercloud/gophercloud/v2/openstack/config/clouds" - osutil "github.com/gophercloud/gophercloud/v2/openstack/utils" "github.com/hashicorp/go-hclog" "github.com/jinzhu/copier" + "github.com/sardinasystems/fleeting-plugin-openstack/internal/openstackclient" "gitlab.com/gitlab-org/fleeting/fleeting/connector" "gitlab.com/gitlab-org/fleeting/fleeting/provider" @@ -28,6 +22,8 @@ const MetadataKey = "fleeting-cluster" var _ provider.InstanceGroup = (*InstanceGroup)(nil) +var newClient = openstackclient.New + type InstanceGroup struct { Cloud string `json:"cloud"` // cloud to use CloudsConfig string `json:"clouds_config"` // optional: path to clouds.yaml @@ -39,10 +35,10 @@ type InstanceGroup struct { BootTimeS string `json:"boot_time"` // optional: wait some time before report machine as available BootTime time.Duration - computeClient *gophercloud.ServiceClient + client openstackclient.Client settings provider.Settings log hclog.Logger - imgProps *ImageProperties + imgProps *openstackclient.ImageProperties sshPubKey string instanceCounter atomic.Int32 } @@ -51,39 +47,25 @@ func (g *InstanceGroup) Init(ctx context.Context, log hclog.Logger, settings pro g.log = log.With("name", g.Name, "cloud", g.Cloud) g.log.Debug("Initializing fleeting-plugin-openstack") - providerClient, endpointOps, err := g.getProviderClient(ctx) - if err != nil { - return provider.ProviderInfo{}, err - } + var err error + g.client, err = newClient(openstackclient.AuthConfig{ + AuthFromEnv: g.AuthFromEnv, + Cloud: g.Cloud, + CloudsConfig: g.CloudsConfig, + NovaMicroversion: g.NovaMicroversion, + }) - cli, err := openstack.NewComputeV2(providerClient, endpointOps) if err != nil { - return provider.ProviderInfo{}, fmt.Errorf("Failed to connect to OpenStack Nova: %w", err) - } - - if g.NovaMicroversion == "" { - g.NovaMicroversion = "2.79" // Train+ - } - - ncli, err := osutil.RequireMicroversion(ctx, *cli, g.NovaMicroversion) - if err != nil { - return provider.ProviderInfo{}, fmt.Errorf("Failed to request microversion %s for OpenStack Nova: %w", g.NovaMicroversion, err) + return provider.ProviderInfo{}, err } - g.computeClient = &ncli - _, err = g.ServerSpec.ToServerCreateMap() if err != nil { return provider.ProviderInfo{}, fmt.Errorf("Failed to check server_spec: %w", err) } if g.ServerSpec.ImageRef != "" { - imgCli, err := openstack.NewImageV2(providerClient, endpointOps) - if err != nil { - return provider.ProviderInfo{}, fmt.Errorf("Failed to get OpenStack Glance: %w", err) - } - - imgProps, err := GetImageProperties(ctx, imgCli, g.ServerSpec.ImageRef) + imgProps, err := g.client.GetImageProperties(ctx, g.ServerSpec.ImageRef) if err != nil { return provider.ProviderInfo{}, err } @@ -126,53 +108,6 @@ func (g *InstanceGroup) Init(ctx context.Context, log hclog.Logger, settings pro }, nil } -func (g *InstanceGroup) getProviderClient(ctx context.Context) (*gophercloud.ProviderClient, gophercloud.EndpointOpts, error) { - var endpointOps gophercloud.EndpointOpts - var authOptions gophercloud.AuthOptions - var providerClient *gophercloud.ProviderClient - - if g.AuthFromEnv { - g.log.Debug("Using env vars for auth") - - var err error - endpointOps = gophercloud.EndpointOpts{Region: os.Getenv("OS_REGION_NAME")} - authOptions, err = openstack.AuthOptionsFromEnv() - if err != nil { - return nil, gophercloud.EndpointOpts{}, fmt.Errorf("Failed to get auth options from environment: %w", err) - } - authOptions.AllowReauth = true - - providerClient, err = openstack.AuthenticatedClient(ctx, authOptions) - if err != nil { - return nil, gophercloud.EndpointOpts{}, fmt.Errorf("Failed to connect to OpenStack Keystone: %w", err) - } - } else { - g.log.Debug("Using clouds.yaml for auth") - - var err error - var tlsCfg *tls.Config - cloudOpts := []clouds.ParseOption{clouds.WithCloudName(g.Cloud)} - if g.CloudsConfig != "" { - cloudOpts = append(cloudOpts, clouds.WithLocations(g.CloudsConfig)) - } - - authOptions, endpointOps, tlsCfg, err = clouds.Parse(cloudOpts...) - if err != nil { - return nil, gophercloud.EndpointOpts{}, fmt.Errorf("Failed to parse clouds.yaml: %w", err) - } - - // plugin is a long running process. force allow reauth - authOptions.AllowReauth = true - - providerClient, err = config.NewProviderClient(ctx, authOptions, config.WithTLSConfig(tlsCfg)) - if err != nil { - return nil, gophercloud.EndpointOpts{}, fmt.Errorf("Failed to connect to OpenStack Keystone: %w", err) - } - } - - return providerClient, endpointOps, nil -} - func (g *InstanceGroup) Update(ctx context.Context, update func(instance string, state provider.State)) error { instances, err := g.getInstances(ctx) @@ -202,9 +137,7 @@ func (g *InstanceGroup) Update(ctx context.Context, update func(instance string, // treat all nodes running long enough as Running state = provider.StateRunning } else { - log, err := servers.ShowConsoleOutput(ctx, g.computeClient, srv.ID, servers.ShowConsoleOutputOpts{ - Length: 100, - }).Extract() + log, err := g.client.ShowServerConsoleOutput(ctx, srv.ID) if err != nil { reterr = errors.Join(reterr, err) continue @@ -252,7 +185,7 @@ func (g *InstanceGroup) Decrease(ctx context.Context, instances []string) (succe succeeded = make([]string, 0, len(instances)) for _, id := range instances { - err2 := g.deleteInstance(ctx, id) + err2 := g.client.DeleteServer(ctx, id) if err2 != nil { g.log.Error("Failed to delete instance", "err", err2, "id", id) err = errors.Join(err, err2) @@ -268,14 +201,9 @@ func (g *InstanceGroup) Decrease(ctx context.Context, instances []string) (succe } func (g *InstanceGroup) getInstances(ctx context.Context) ([]servers.Server, error) { - page, err := servers.List(g.computeClient, nil).AllPages(ctx) - if err != nil { - return nil, fmt.Errorf("Server listing error: %w", err) - } - - allServers, err := servers.ExtractServers(page) + allServers, err := g.client.ListServers(ctx) if err != nil { - return nil, fmt.Errorf("Server listing extract error: %w", err) + return nil, err } filteredServers := make([]servers.Server, 0, len(allServers)) @@ -318,7 +246,7 @@ func (g *InstanceGroup) createInstance(ctx context.Context) (string, error) { } } - srv, err := servers.Create(ctx, g.computeClient, spec, hintOpts).Extract() + srv, err := g.client.CreateServer(ctx, spec, hintOpts) if err != nil { return "", err } @@ -326,16 +254,8 @@ func (g *InstanceGroup) createInstance(ctx context.Context) (string, error) { return srv.ID, nil } -func (g *InstanceGroup) deleteInstance(ctx context.Context, id string) error { - return servers.Delete(ctx, g.computeClient, id).ExtractErr() -} - -func (g *InstanceGroup) getInstance(ctx context.Context, id string) (*servers.Server, error) { - return servers.Get(ctx, g.computeClient, id).Extract() -} - func (g *InstanceGroup) ConnectInfo(ctx context.Context, instanceID string) (provider.ConnectInfo, error) { - srv, err := g.getInstance(ctx, instanceID) + srv, err := g.client.GetServer(ctx, instanceID) if err != nil { return provider.ConnectInfo{}, fmt.Errorf("Failed to get server %s: %w", instanceID, err) } diff --git a/utils.go b/utils.go index 7f48d19..a0d8fe6 100644 --- a/utils.go +++ b/utils.go @@ -1,7 +1,6 @@ package fpoc import ( - "context" "encoding/json" "fmt" "maps" @@ -11,9 +10,7 @@ import ( igncfg "github.com/coreos/ignition/v2/config/v3_4" igntyp "github.com/coreos/ignition/v2/config/v3_4/types" "github.com/coreos/vcontext/report" - "github.com/gophercloud/gophercloud/v2" "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servers" - "github.com/gophercloud/gophercloud/v2/openstack/image/v2/images" "github.com/mitchellh/mapstructure" ) @@ -143,34 +140,6 @@ func IsIgnitionFinished(log string) bool { return false } -// Some good known properties useful for setting up ConnectInfo -// -// See also: https://docs.openstack.org/glance/latest/admin/useful-image-properties.html -type ImageProperties struct { - Architecture string `json:"architecture,omitempty" mapstructure:"architecture,omitempty"` - OSType string `json:"os_type,omitempty" mapstructure:"os_type,omitempty"` - OSDistro string `json:"os_distro,omitempty" mapstructure:"os_distro,omitempty"` - OSVersion string `json:"os_version,omitempty" mapstructure:"os_version,omitempty"` - OSAdminUser string `json:"os_admin_user,omitempty" mapstructure:"os_admin_user,omitempty"` - - // Extra map[string]any `mapstructure:",remain"` -} - -func GetImageProperties(ctx context.Context, cli *gophercloud.ServiceClient, imageID string) (*ImageProperties, error) { - image, err := images.Get(ctx, cli, imageID).Extract() - if err != nil { - return nil, fmt.Errorf("failed to get image %s: %w", imageID, err) - } - - out := new(ImageProperties) - err = mapstructure.Decode(image.Properties, out) - if err != nil { - return nil, fmt.Errorf("failed to parse properties: %w", err) - } - - return out, nil -} - func InsertSSHKeyIgn(spec *ExtCreateOpts, username, pubKey string) error { var cfg igntyp.Config var err error diff --git a/utils_test.go b/utils_test.go index 72ffe6d..d69333d 100644 --- a/utils_test.go +++ b/utils_test.go @@ -1,13 +1,10 @@ package fpoc import ( - "context" "encoding/json" "os" "testing" - "github.com/gophercloud/gophercloud/v2/testhelper" - tc "github.com/gophercloud/gophercloud/v2/testhelper/client" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -111,29 +108,6 @@ func TestExtCreateOpts(t *testing.T) { //t.Log(string(req)) } -func TestGetImageProperties(t *testing.T) { - assert := assert.New(t) - - img, err := os.ReadFile("./testdata/image_get.json") - require.NoError(t, err) - - testhelper.SetupHTTP() - defer testhelper.TeardownHTTP() - - testhelper.ServeFile(t, "", "", "application/json", string(img)) - - ctx := context.TODO() - imgCli := tc.ServiceClient() - - props, err := GetImageProperties(ctx, imgCli, "1da9661c-953e-424d-a1e5-834a8174b198") - assert.NoError(err) - if assert.NotNil(props) { - assert.Equal("core", props.OSAdminUser) - } - - t.Log(props) -} - func TestInsertSSHKeyIgn(t *testing.T) { testCases := []struct { name string