From 9ba5609f9b73f00630d998f12f24fae506160561 Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Thu, 30 Jun 2022 18:11:52 +0100 Subject: [PATCH 1/2] Move clients into a separate package --- .../compute/client.go => clients/compute.go} | 75 +++++++++--- .../client.go => clients/loadbalancer.go} | 16 ++- .../mock/compute.go} | 113 +++++++++--------- pkg/clients/mock/doc.go | 26 ++++ .../mock/loadbalancer.go} | 6 +- .../mock/network.go} | 6 +- .../client.go => clients/networking.go} | 18 ++- pkg/cloud/services/compute/instance.go | 3 +- pkg/cloud/services/compute/instance_test.go | 103 ++++++++-------- pkg/cloud/services/compute/instance_types.go | 5 +- .../services/compute/instance_types_test.go | 6 +- pkg/cloud/services/compute/service.go | 40 +------ .../loadbalancer/loadbalancer_test.go | 19 ++- .../loadbalancer/mock_loadbalancer/doc.go | 20 ---- pkg/cloud/services/loadbalancer/service.go | 21 ++-- .../services/networking/floatingip_test.go | 10 +- .../networking/mock_networking/doc.go | 20 ---- pkg/cloud/services/networking/port_test.go | 24 ++-- pkg/cloud/services/networking/service.go | 15 +-- pkg/cloud/services/networking/trunk_test.go | 10 +- test/e2e/shared/openstack.go | 7 +- 21 files changed, 292 insertions(+), 271 deletions(-) rename pkg/{cloud/services/compute/client.go => clients/compute.go} (69%) rename pkg/{cloud/services/loadbalancer/client.go => clients/loadbalancer.go} (94%) rename pkg/{cloud/services/compute/client_mock.go => clients/mock/compute.go} (57%) create mode 100644 pkg/clients/mock/doc.go rename pkg/{cloud/services/loadbalancer/mock_loadbalancer/loadbalancer_service_mock.go => clients/mock/loadbalancer.go} (98%) rename pkg/{cloud/services/networking/mock_networking/client_mock.go => clients/mock/network.go} (99%) rename pkg/{cloud/services/networking/client.go => clients/networking.go} (96%) delete mode 100644 pkg/cloud/services/loadbalancer/mock_loadbalancer/doc.go delete mode 100644 pkg/cloud/services/networking/mock_networking/doc.go diff --git a/pkg/cloud/services/compute/client.go b/pkg/clients/compute.go similarity index 69% rename from pkg/cloud/services/compute/client.go rename to pkg/clients/compute.go index bc5db3e08c..2735b5cd47 100644 --- a/pkg/cloud/services/compute/client.go +++ b/pkg/clients/compute.go @@ -14,10 +14,13 @@ See the License for the specific language governing permissions and limitations under the License. */ -package compute +package clients import ( + "fmt" + "github.com/gophercloud/gophercloud" + "github.com/gophercloud/gophercloud/openstack" "github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumes" "github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/attachinterfaces" "github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/availabilityzones" @@ -26,10 +29,19 @@ import ( "github.com/gophercloud/utils/openstack/compute/v2/flavors" "sigs.k8s.io/cluster-api-provider-openstack/pkg/metrics" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" ) -//go:generate mockgen -package=compute -self_package sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/compute -destination=client_mock.go sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/compute Client -//go:generate /usr/bin/env bash -c "cat ../../../../hack/boilerplate/boilerplate.generatego.txt client_mock.go > _client_mock.go && mv _client_mock.go client_mock.go" +/* +NovaMinimumMicroversion is the minimum Nova microversion supported by CAPO +2.53 corresponds to OpenStack Pike + +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. +*/ +const NovaMinimumMicroversion = "2.53" // ServerExt is the base gophercloud Server with extensions used by InstanceStatus. type ServerExt struct { @@ -37,7 +49,7 @@ type ServerExt struct { availabilityzones.ServerAvailabilityZoneExt } -type Client interface { +type ComputeClient interface { ListAvailabilityZones() ([]availabilityzones.AvailabilityZone, error) ListImages(listOpts images.ListOptsBuilder) ([]images.Image, error) @@ -57,13 +69,40 @@ type Client interface { GetVolume(volumeID string) (*volumes.Volume, error) } -type serviceClient struct { +type computeClient struct { compute *gophercloud.ServiceClient images *gophercloud.ServiceClient volume *gophercloud.ServiceClient } -func (s serviceClient) ListAvailabilityZones() ([]availabilityzones.AvailabilityZone, error) { +// NewComputeClient returns a new compute client. +func NewComputeClient(scope *scope.Scope) (ComputeClient, error) { + compute, err := openstack.NewComputeV2(scope.ProviderClient, gophercloud.EndpointOpts{ + Region: scope.ProviderClientOpts.RegionName, + }) + if err != nil { + return nil, fmt.Errorf("failed to create compute service client: %v", err) + } + compute.Microversion = NovaMinimumMicroversion + + images, err := openstack.NewImageServiceV2(scope.ProviderClient, gophercloud.EndpointOpts{ + Region: scope.ProviderClientOpts.RegionName, + }) + if err != nil { + return nil, fmt.Errorf("failed to create image service client: %v", err) + } + + volume, err := openstack.NewBlockStorageV3(scope.ProviderClient, gophercloud.EndpointOpts{ + Region: scope.ProviderClientOpts.RegionName, + }) + if err != nil { + return nil, fmt.Errorf("failed to create volume service client: %v", err) + } + + return &computeClient{compute, images, volume}, nil +} + +func (s computeClient) ListAvailabilityZones() ([]availabilityzones.AvailabilityZone, error) { mc := metrics.NewMetricPrometheusContext("availability_zone", "list") allPages, err := availabilityzones.List(s.compute).AllPages() if mc.ObserveRequest(err) != nil { @@ -72,7 +111,7 @@ func (s serviceClient) ListAvailabilityZones() ([]availabilityzones.Availability return availabilityzones.ExtractAvailabilityZones(allPages) } -func (s serviceClient) ListImages(listOpts images.ListOptsBuilder) ([]images.Image, error) { +func (s computeClient) ListImages(listOpts images.ListOptsBuilder) ([]images.Image, error) { mc := metrics.NewMetricPrometheusContext("image", "list") pages, err := images.List(s.images, listOpts).AllPages() if mc.ObserveRequest(err) != nil { @@ -81,13 +120,13 @@ func (s serviceClient) ListImages(listOpts images.ListOptsBuilder) ([]images.Ima return images.ExtractImages(pages) } -func (s serviceClient) GetFlavorIDFromName(flavor string) (string, error) { +func (s computeClient) GetFlavorIDFromName(flavor string) (string, error) { mc := metrics.NewMetricPrometheusContext("flavor", "get") flavorID, err := flavors.IDFromName(s.compute, flavor) return flavorID, mc.ObserveRequest(err) } -func (s serviceClient) CreateServer(createOpts servers.CreateOptsBuilder) (*ServerExt, error) { +func (s computeClient) CreateServer(createOpts servers.CreateOptsBuilder) (*ServerExt, error) { var server ServerExt mc := metrics.NewMetricPrometheusContext("server", "create") err := servers.Create(s.compute, createOpts).ExtractInto(&server) @@ -97,13 +136,13 @@ func (s serviceClient) CreateServer(createOpts servers.CreateOptsBuilder) (*Serv return &server, nil } -func (s serviceClient) DeleteServer(serverID string) error { +func (s computeClient) DeleteServer(serverID string) error { mc := metrics.NewMetricPrometheusContext("server", "delete") err := servers.Delete(s.compute, serverID).ExtractErr() return mc.ObserveRequestIgnoreNotFound(err) } -func (s serviceClient) GetServer(serverID string) (*ServerExt, error) { +func (s computeClient) GetServer(serverID string) (*ServerExt, error) { var server ServerExt mc := metrics.NewMetricPrometheusContext("server", "get") err := servers.Get(s.compute, serverID).ExtractInto(&server) @@ -113,7 +152,7 @@ func (s serviceClient) GetServer(serverID string) (*ServerExt, error) { return &server, nil } -func (s serviceClient) ListServers(listOpts servers.ListOptsBuilder) ([]ServerExt, error) { +func (s computeClient) ListServers(listOpts servers.ListOptsBuilder) ([]ServerExt, error) { var serverList []ServerExt mc := metrics.NewMetricPrometheusContext("server", "list") allPages, err := servers.List(s.compute, listOpts).AllPages() @@ -124,7 +163,7 @@ func (s serviceClient) ListServers(listOpts servers.ListOptsBuilder) ([]ServerEx return serverList, err } -func (s serviceClient) ListAttachedInterfaces(serverID string) ([]attachinterfaces.Interface, error) { +func (s computeClient) ListAttachedInterfaces(serverID string) ([]attachinterfaces.Interface, error) { mc := metrics.NewMetricPrometheusContext("server_os_interface", "list") interfaces, err := attachinterfaces.List(s.compute, serverID).AllPages() if mc.ObserveRequest(err) != nil { @@ -133,13 +172,13 @@ func (s serviceClient) ListAttachedInterfaces(serverID string) ([]attachinterfac return attachinterfaces.ExtractInterfaces(interfaces) } -func (s serviceClient) DeleteAttachedInterface(serverID, portID string) error { +func (s computeClient) DeleteAttachedInterface(serverID, portID string) error { mc := metrics.NewMetricPrometheusContext("server_os_interface", "delete") err := attachinterfaces.Delete(s.compute, serverID, portID).ExtractErr() return mc.ObserveRequestIgnoreNotFoundorConflict(err) } -func (s serviceClient) ListVolumes(opts volumes.ListOptsBuilder) ([]volumes.Volume, error) { +func (s computeClient) ListVolumes(opts volumes.ListOptsBuilder) ([]volumes.Volume, error) { mc := metrics.NewMetricPrometheusContext("volume", "list") pages, err := volumes.List(s.volume, opts).AllPages() if mc.ObserveRequest(err) != nil { @@ -148,19 +187,19 @@ func (s serviceClient) ListVolumes(opts volumes.ListOptsBuilder) ([]volumes.Volu return volumes.ExtractVolumes(pages) } -func (s serviceClient) CreateVolume(opts volumes.CreateOptsBuilder) (*volumes.Volume, error) { +func (s computeClient) CreateVolume(opts volumes.CreateOptsBuilder) (*volumes.Volume, error) { mc := metrics.NewMetricPrometheusContext("volume", "create") volume, err := volumes.Create(s.volume, opts).Extract() return volume, mc.ObserveRequest(err) } -func (s serviceClient) DeleteVolume(volumeID string, opts volumes.DeleteOptsBuilder) error { +func (s computeClient) DeleteVolume(volumeID string, opts volumes.DeleteOptsBuilder) error { mc := metrics.NewMetricPrometheusContext("volume", "delete") err := volumes.Delete(s.volume, volumeID, opts).ExtractErr() return mc.ObserveRequestIgnoreNotFound(err) } -func (s serviceClient) GetVolume(volumeID string) (*volumes.Volume, error) { +func (s computeClient) GetVolume(volumeID string) (*volumes.Volume, error) { mc := metrics.NewMetricPrometheusContext("volume", "get") volume, err := volumes.Get(s.volume, volumeID).Extract() return volume, mc.ObserveRequestIgnoreNotFound(err) diff --git a/pkg/cloud/services/loadbalancer/client.go b/pkg/clients/loadbalancer.go similarity index 94% rename from pkg/cloud/services/loadbalancer/client.go rename to pkg/clients/loadbalancer.go index 859c116275..311ea04f6f 100644 --- a/pkg/cloud/services/loadbalancer/client.go +++ b/pkg/clients/loadbalancer.go @@ -14,12 +14,13 @@ See the License for the specific language governing permissions and limitations under the License. */ -package loadbalancer +package clients import ( "fmt" "github.com/gophercloud/gophercloud" + "github.com/gophercloud/gophercloud/openstack" "github.com/gophercloud/gophercloud/openstack/compute/apiversions" "github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/listeners" "github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/loadbalancers" @@ -28,6 +29,7 @@ import ( "github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/providers" "sigs.k8s.io/cluster-api-provider-openstack/pkg/metrics" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" capoerrors "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/errors" ) @@ -59,6 +61,18 @@ type lbClient struct { serviceClient *gophercloud.ServiceClient } +// NewLbClient returns a new loadbalancer client. +func NewLbClient(scope *scope.Scope) (LbClient, error) { + loadbalancerClient, err := openstack.NewLoadBalancerV2(scope.ProviderClient, gophercloud.EndpointOpts{ + Region: scope.ProviderClientOpts.RegionName, + }) + if err != nil { + return nil, fmt.Errorf("failed to create load balancer service client: %v", err) + } + + return &lbClient{loadbalancerClient}, nil +} + func (l lbClient) CreateLoadBalancer(opts loadbalancers.CreateOptsBuilder) (*loadbalancers.LoadBalancer, error) { mc := metrics.NewMetricPrometheusContext("loadbalancer", "create") lb, err := loadbalancers.Create(l.serviceClient, opts).Extract() diff --git a/pkg/cloud/services/compute/client_mock.go b/pkg/clients/mock/compute.go similarity index 57% rename from pkg/cloud/services/compute/client_mock.go rename to pkg/clients/mock/compute.go index 4a525fee94..f0559503d5 100644 --- a/pkg/cloud/services/compute/client_mock.go +++ b/pkg/clients/mock/compute.go @@ -15,10 +15,10 @@ limitations under the License. */ // Code generated by MockGen. DO NOT EDIT. -// Source: sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/compute (interfaces: Client) +// Source: sigs.k8s.io/cluster-api-provider-openstack/pkg/clients (interfaces: ComputeClient) -// Package compute is a generated GoMock package. -package compute +// Package mock is a generated GoMock package. +package mock import ( reflect "reflect" @@ -29,48 +29,49 @@ import ( availabilityzones "github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/availabilityzones" servers "github.com/gophercloud/gophercloud/openstack/compute/v2/servers" images "github.com/gophercloud/gophercloud/openstack/imageservice/v2/images" + clients "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients" ) -// MockClient is a mock of Client interface. -type MockClient struct { +// MockComputeClient is a mock of ComputeClient interface. +type MockComputeClient struct { ctrl *gomock.Controller - recorder *MockClientMockRecorder + recorder *MockComputeClientMockRecorder } -// MockClientMockRecorder is the mock recorder for MockClient. -type MockClientMockRecorder struct { - mock *MockClient +// MockComputeClientMockRecorder is the mock recorder for MockComputeClient. +type MockComputeClientMockRecorder struct { + mock *MockComputeClient } -// NewMockClient creates a new mock instance. -func NewMockClient(ctrl *gomock.Controller) *MockClient { - mock := &MockClient{ctrl: ctrl} - mock.recorder = &MockClientMockRecorder{mock} +// NewMockComputeClient creates a new mock instance. +func NewMockComputeClient(ctrl *gomock.Controller) *MockComputeClient { + mock := &MockComputeClient{ctrl: ctrl} + mock.recorder = &MockComputeClientMockRecorder{mock} return mock } // EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockClient) EXPECT() *MockClientMockRecorder { +func (m *MockComputeClient) EXPECT() *MockComputeClientMockRecorder { return m.recorder } // CreateServer mocks base method. -func (m *MockClient) CreateServer(arg0 servers.CreateOptsBuilder) (*ServerExt, error) { +func (m *MockComputeClient) CreateServer(arg0 servers.CreateOptsBuilder) (*clients.ServerExt, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CreateServer", arg0) - ret0, _ := ret[0].(*ServerExt) + ret0, _ := ret[0].(*clients.ServerExt) ret1, _ := ret[1].(error) return ret0, ret1 } // CreateServer indicates an expected call of CreateServer. -func (mr *MockClientMockRecorder) CreateServer(arg0 interface{}) *gomock.Call { +func (mr *MockComputeClientMockRecorder) CreateServer(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateServer", reflect.TypeOf((*MockClient)(nil).CreateServer), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateServer", reflect.TypeOf((*MockComputeClient)(nil).CreateServer), arg0) } // CreateVolume mocks base method. -func (m *MockClient) CreateVolume(arg0 volumes.CreateOptsBuilder) (*volumes.Volume, error) { +func (m *MockComputeClient) CreateVolume(arg0 volumes.CreateOptsBuilder) (*volumes.Volume, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CreateVolume", arg0) ret0, _ := ret[0].(*volumes.Volume) @@ -79,13 +80,13 @@ func (m *MockClient) CreateVolume(arg0 volumes.CreateOptsBuilder) (*volumes.Volu } // CreateVolume indicates an expected call of CreateVolume. -func (mr *MockClientMockRecorder) CreateVolume(arg0 interface{}) *gomock.Call { +func (mr *MockComputeClientMockRecorder) CreateVolume(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateVolume", reflect.TypeOf((*MockClient)(nil).CreateVolume), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateVolume", reflect.TypeOf((*MockComputeClient)(nil).CreateVolume), arg0) } // DeleteAttachedInterface mocks base method. -func (m *MockClient) DeleteAttachedInterface(arg0, arg1 string) error { +func (m *MockComputeClient) DeleteAttachedInterface(arg0, arg1 string) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "DeleteAttachedInterface", arg0, arg1) ret0, _ := ret[0].(error) @@ -93,13 +94,13 @@ func (m *MockClient) DeleteAttachedInterface(arg0, arg1 string) error { } // DeleteAttachedInterface indicates an expected call of DeleteAttachedInterface. -func (mr *MockClientMockRecorder) DeleteAttachedInterface(arg0, arg1 interface{}) *gomock.Call { +func (mr *MockComputeClientMockRecorder) DeleteAttachedInterface(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAttachedInterface", reflect.TypeOf((*MockClient)(nil).DeleteAttachedInterface), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAttachedInterface", reflect.TypeOf((*MockComputeClient)(nil).DeleteAttachedInterface), arg0, arg1) } // DeleteServer mocks base method. -func (m *MockClient) DeleteServer(arg0 string) error { +func (m *MockComputeClient) DeleteServer(arg0 string) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "DeleteServer", arg0) ret0, _ := ret[0].(error) @@ -107,13 +108,13 @@ func (m *MockClient) DeleteServer(arg0 string) error { } // DeleteServer indicates an expected call of DeleteServer. -func (mr *MockClientMockRecorder) DeleteServer(arg0 interface{}) *gomock.Call { +func (mr *MockComputeClientMockRecorder) DeleteServer(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteServer", reflect.TypeOf((*MockClient)(nil).DeleteServer), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteServer", reflect.TypeOf((*MockComputeClient)(nil).DeleteServer), arg0) } // DeleteVolume mocks base method. -func (m *MockClient) DeleteVolume(arg0 string, arg1 volumes.DeleteOptsBuilder) error { +func (m *MockComputeClient) DeleteVolume(arg0 string, arg1 volumes.DeleteOptsBuilder) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "DeleteVolume", arg0, arg1) ret0, _ := ret[0].(error) @@ -121,13 +122,13 @@ func (m *MockClient) DeleteVolume(arg0 string, arg1 volumes.DeleteOptsBuilder) e } // DeleteVolume indicates an expected call of DeleteVolume. -func (mr *MockClientMockRecorder) DeleteVolume(arg0, arg1 interface{}) *gomock.Call { +func (mr *MockComputeClientMockRecorder) DeleteVolume(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteVolume", reflect.TypeOf((*MockClient)(nil).DeleteVolume), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteVolume", reflect.TypeOf((*MockComputeClient)(nil).DeleteVolume), arg0, arg1) } // GetFlavorIDFromName mocks base method. -func (m *MockClient) GetFlavorIDFromName(arg0 string) (string, error) { +func (m *MockComputeClient) GetFlavorIDFromName(arg0 string) (string, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetFlavorIDFromName", arg0) ret0, _ := ret[0].(string) @@ -136,28 +137,28 @@ func (m *MockClient) GetFlavorIDFromName(arg0 string) (string, error) { } // GetFlavorIDFromName indicates an expected call of GetFlavorIDFromName. -func (mr *MockClientMockRecorder) GetFlavorIDFromName(arg0 interface{}) *gomock.Call { +func (mr *MockComputeClientMockRecorder) GetFlavorIDFromName(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetFlavorIDFromName", reflect.TypeOf((*MockClient)(nil).GetFlavorIDFromName), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetFlavorIDFromName", reflect.TypeOf((*MockComputeClient)(nil).GetFlavorIDFromName), arg0) } // GetServer mocks base method. -func (m *MockClient) GetServer(arg0 string) (*ServerExt, error) { +func (m *MockComputeClient) GetServer(arg0 string) (*clients.ServerExt, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetServer", arg0) - ret0, _ := ret[0].(*ServerExt) + ret0, _ := ret[0].(*clients.ServerExt) ret1, _ := ret[1].(error) return ret0, ret1 } // GetServer indicates an expected call of GetServer. -func (mr *MockClientMockRecorder) GetServer(arg0 interface{}) *gomock.Call { +func (mr *MockComputeClientMockRecorder) GetServer(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetServer", reflect.TypeOf((*MockClient)(nil).GetServer), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetServer", reflect.TypeOf((*MockComputeClient)(nil).GetServer), arg0) } // GetVolume mocks base method. -func (m *MockClient) GetVolume(arg0 string) (*volumes.Volume, error) { +func (m *MockComputeClient) GetVolume(arg0 string) (*volumes.Volume, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetVolume", arg0) ret0, _ := ret[0].(*volumes.Volume) @@ -166,13 +167,13 @@ func (m *MockClient) GetVolume(arg0 string) (*volumes.Volume, error) { } // GetVolume indicates an expected call of GetVolume. -func (mr *MockClientMockRecorder) GetVolume(arg0 interface{}) *gomock.Call { +func (mr *MockComputeClientMockRecorder) GetVolume(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetVolume", reflect.TypeOf((*MockClient)(nil).GetVolume), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetVolume", reflect.TypeOf((*MockComputeClient)(nil).GetVolume), arg0) } // ListAttachedInterfaces mocks base method. -func (m *MockClient) ListAttachedInterfaces(arg0 string) ([]attachinterfaces.Interface, error) { +func (m *MockComputeClient) ListAttachedInterfaces(arg0 string) ([]attachinterfaces.Interface, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ListAttachedInterfaces", arg0) ret0, _ := ret[0].([]attachinterfaces.Interface) @@ -181,13 +182,13 @@ func (m *MockClient) ListAttachedInterfaces(arg0 string) ([]attachinterfaces.Int } // ListAttachedInterfaces indicates an expected call of ListAttachedInterfaces. -func (mr *MockClientMockRecorder) ListAttachedInterfaces(arg0 interface{}) *gomock.Call { +func (mr *MockComputeClientMockRecorder) ListAttachedInterfaces(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListAttachedInterfaces", reflect.TypeOf((*MockClient)(nil).ListAttachedInterfaces), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListAttachedInterfaces", reflect.TypeOf((*MockComputeClient)(nil).ListAttachedInterfaces), arg0) } // ListAvailabilityZones mocks base method. -func (m *MockClient) ListAvailabilityZones() ([]availabilityzones.AvailabilityZone, error) { +func (m *MockComputeClient) ListAvailabilityZones() ([]availabilityzones.AvailabilityZone, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ListAvailabilityZones") ret0, _ := ret[0].([]availabilityzones.AvailabilityZone) @@ -196,13 +197,13 @@ func (m *MockClient) ListAvailabilityZones() ([]availabilityzones.AvailabilityZo } // ListAvailabilityZones indicates an expected call of ListAvailabilityZones. -func (mr *MockClientMockRecorder) ListAvailabilityZones() *gomock.Call { +func (mr *MockComputeClientMockRecorder) ListAvailabilityZones() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListAvailabilityZones", reflect.TypeOf((*MockClient)(nil).ListAvailabilityZones)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListAvailabilityZones", reflect.TypeOf((*MockComputeClient)(nil).ListAvailabilityZones)) } // ListImages mocks base method. -func (m *MockClient) ListImages(arg0 images.ListOptsBuilder) ([]images.Image, error) { +func (m *MockComputeClient) ListImages(arg0 images.ListOptsBuilder) ([]images.Image, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ListImages", arg0) ret0, _ := ret[0].([]images.Image) @@ -211,28 +212,28 @@ func (m *MockClient) ListImages(arg0 images.ListOptsBuilder) ([]images.Image, er } // ListImages indicates an expected call of ListImages. -func (mr *MockClientMockRecorder) ListImages(arg0 interface{}) *gomock.Call { +func (mr *MockComputeClientMockRecorder) ListImages(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListImages", reflect.TypeOf((*MockClient)(nil).ListImages), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListImages", reflect.TypeOf((*MockComputeClient)(nil).ListImages), arg0) } // ListServers mocks base method. -func (m *MockClient) ListServers(arg0 servers.ListOptsBuilder) ([]ServerExt, error) { +func (m *MockComputeClient) ListServers(arg0 servers.ListOptsBuilder) ([]clients.ServerExt, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ListServers", arg0) - ret0, _ := ret[0].([]ServerExt) + ret0, _ := ret[0].([]clients.ServerExt) ret1, _ := ret[1].(error) return ret0, ret1 } // ListServers indicates an expected call of ListServers. -func (mr *MockClientMockRecorder) ListServers(arg0 interface{}) *gomock.Call { +func (mr *MockComputeClientMockRecorder) ListServers(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListServers", reflect.TypeOf((*MockClient)(nil).ListServers), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListServers", reflect.TypeOf((*MockComputeClient)(nil).ListServers), arg0) } // ListVolumes mocks base method. -func (m *MockClient) ListVolumes(arg0 volumes.ListOptsBuilder) ([]volumes.Volume, error) { +func (m *MockComputeClient) ListVolumes(arg0 volumes.ListOptsBuilder) ([]volumes.Volume, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ListVolumes", arg0) ret0, _ := ret[0].([]volumes.Volume) @@ -241,7 +242,7 @@ func (m *MockClient) ListVolumes(arg0 volumes.ListOptsBuilder) ([]volumes.Volume } // ListVolumes indicates an expected call of ListVolumes. -func (mr *MockClientMockRecorder) ListVolumes(arg0 interface{}) *gomock.Call { +func (mr *MockComputeClientMockRecorder) ListVolumes(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListVolumes", reflect.TypeOf((*MockClient)(nil).ListVolumes), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListVolumes", reflect.TypeOf((*MockComputeClient)(nil).ListVolumes), arg0) } diff --git a/pkg/clients/mock/doc.go b/pkg/clients/mock/doc.go new file mode 100644 index 0000000000..e958591f94 --- /dev/null +++ b/pkg/clients/mock/doc.go @@ -0,0 +1,26 @@ +/* +Copyright 2021 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 mock + +//go:generate mockgen -package mock -destination=compute.go sigs.k8s.io/cluster-api-provider-openstack/pkg/clients ComputeClient +//go:generate /usr/bin/env bash -c "cat ../../../hack/boilerplate/boilerplate.generatego.txt compute.go > _compute.go && mv _compute.go compute.go" + +//go:generate mockgen -package mock -destination=network.go sigs.k8s.io/cluster-api-provider-openstack/pkg/clients NetworkClient +//go:generate /usr/bin/env bash -c "cat ../../../hack/boilerplate/boilerplate.generatego.txt network.go > _network.go && mv _network.go network.go" + +//go:generate mockgen -package mock -destination=loadbalancer.go sigs.k8s.io/cluster-api-provider-openstack/pkg/clients LbClient +//go:generate /usr/bin/env bash -c "cat ../../../hack/boilerplate/boilerplate.generatego.txt loadbalancer.go > _loadbalancer.go && mv _loadbalancer.go loadbalancer.go" diff --git a/pkg/cloud/services/loadbalancer/mock_loadbalancer/loadbalancer_service_mock.go b/pkg/clients/mock/loadbalancer.go similarity index 98% rename from pkg/cloud/services/loadbalancer/mock_loadbalancer/loadbalancer_service_mock.go rename to pkg/clients/mock/loadbalancer.go index 54949a738e..96fc02e3ae 100644 --- a/pkg/cloud/services/loadbalancer/mock_loadbalancer/loadbalancer_service_mock.go +++ b/pkg/clients/mock/loadbalancer.go @@ -15,10 +15,10 @@ limitations under the License. */ // Code generated by MockGen. DO NOT EDIT. -// Source: sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/loadbalancer (interfaces: LbClient) +// Source: sigs.k8s.io/cluster-api-provider-openstack/pkg/clients (interfaces: LbClient) -// Package mock_loadbalancer is a generated GoMock package. -package mock_loadbalancer +// Package mock is a generated GoMock package. +package mock import ( reflect "reflect" diff --git a/pkg/cloud/services/networking/mock_networking/client_mock.go b/pkg/clients/mock/network.go similarity index 99% rename from pkg/cloud/services/networking/mock_networking/client_mock.go rename to pkg/clients/mock/network.go index f9aea86719..091a89f3b7 100644 --- a/pkg/cloud/services/networking/mock_networking/client_mock.go +++ b/pkg/clients/mock/network.go @@ -15,10 +15,10 @@ limitations under the License. */ // Code generated by MockGen. DO NOT EDIT. -// Source: sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/networking (interfaces: NetworkClient) +// Source: sigs.k8s.io/cluster-api-provider-openstack/pkg/clients (interfaces: NetworkClient) -// Package mock_networking is a generated GoMock package. -package mock_networking +// Package mock is a generated GoMock package. +package mock import ( reflect "reflect" diff --git a/pkg/cloud/services/networking/client.go b/pkg/clients/networking.go similarity index 96% rename from pkg/cloud/services/networking/client.go rename to pkg/clients/networking.go index 4c2228613c..adffde9c79 100644 --- a/pkg/cloud/services/networking/client.go +++ b/pkg/clients/networking.go @@ -14,10 +14,13 @@ See the License for the specific language governing permissions and limitations under the License. */ -package networking +package clients import ( + "fmt" + "github.com/gophercloud/gophercloud" + "github.com/gophercloud/gophercloud/openstack" "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions" "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/attributestags" "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/floatingips" @@ -30,6 +33,7 @@ import ( "github.com/gophercloud/gophercloud/openstack/networking/v2/subnets" "sigs.k8s.io/cluster-api-provider-openstack/pkg/metrics" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" ) type NetworkClient interface { @@ -89,6 +93,18 @@ type networkClient struct { serviceClient *gophercloud.ServiceClient } +// NewNetworkClient returns an instance of the networking service. +func NewNetworkClient(scope *scope.Scope) (NetworkClient, error) { + serviceClient, err := openstack.NewNetworkV2(scope.ProviderClient, gophercloud.EndpointOpts{ + Region: scope.ProviderClientOpts.RegionName, + }) + if err != nil { + return nil, fmt.Errorf("failed to create networking service providerClient: %v", err) + } + + return networkClient{serviceClient}, nil +} + func (c networkClient) AddRouterInterface(id string, opts routers.AddInterfaceOptsBuilder) (*routers.InterfaceInfo, error) { mc := metrics.NewMetricPrometheusContext("server_os_interface", "create") interfaceInfo, err := routers.AddInterface(c.serviceClient, id, opts).Extract() diff --git a/pkg/cloud/services/compute/instance.go b/pkg/cloud/services/compute/instance.go index d70e2273da..c2b4a8e84e 100644 --- a/pkg/cloud/services/compute/instance.go +++ b/pkg/cloud/services/compute/instance.go @@ -33,6 +33,7 @@ import ( "sigs.k8s.io/cluster-api/util" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha6" + "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/hash" @@ -125,7 +126,7 @@ func (s *Service) CreateInstance(eventObject runtime.Object, openStackCluster *i } func (s *Service) createInstanceImpl(eventObject runtime.Object, openStackCluster *infrav1.OpenStackCluster, instanceSpec *InstanceSpec, clusterName string, retryInterval time.Duration) (*InstanceStatus, error) { - var server *ServerExt + var server *clients.ServerExt accessIPv4 := "" portList := []servers.Network{} diff --git a/pkg/cloud/services/compute/instance_test.go b/pkg/cloud/services/compute/instance_test.go index 86796ad321..99ec5140ee 100644 --- a/pkg/cloud/services/compute/instance_test.go +++ b/pkg/cloud/services/compute/instance_test.go @@ -44,8 +44,9 @@ import ( "k8s.io/utils/pointer" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha6" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients" + mock_clients "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients/mock" "sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/networking" - "sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/networking/mock_networking" "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" ) @@ -182,7 +183,7 @@ func TestService_getServerNetworks(t *testing.T) { name string networkParams []infrav1.NetworkParam want []infrav1.Network - expect func(m *mock_networking.MockNetworkClientMockRecorder) + expect func(m *mock_clients.MockNetworkClientMockRecorder) wantErr bool }{ { @@ -193,7 +194,7 @@ func TestService_getServerNetworks(t *testing.T) { want: []infrav1.Network{ {ID: networkAUUID, Subnet: &infrav1.Subnet{}}, }, - expect: func(m *mock_networking.MockNetworkClientMockRecorder) { + expect: func(m *mock_clients.MockNetworkClientMockRecorder) { }, wantErr: false, }, @@ -206,7 +207,7 @@ func TestService_getServerNetworks(t *testing.T) { {ID: networkAUUID, Subnet: &infrav1.Subnet{}}, {ID: networkBUUID, Subnet: &infrav1.Subnet{}}, }, - expect: func(m *mock_networking.MockNetworkClientMockRecorder) { + expect: func(m *mock_clients.MockNetworkClientMockRecorder) { // List tagged networks (A & B) m.ListNetwork(&testNetworkListOpts). Return([]networks.Network{testNetworkA, testNetworkB}, nil) @@ -225,7 +226,7 @@ func TestService_getServerNetworks(t *testing.T) { {ID: networkAUUID, Subnet: &infrav1.Subnet{ID: subnetA2UUID}}, {ID: networkBUUID, Subnet: &infrav1.Subnet{ID: subnetB1UUID}}, }, - expect: func(m *mock_networking.MockNetworkClientMockRecorder) { + expect: func(m *mock_clients.MockNetworkClientMockRecorder) { // List all tagged subnets in any network (A1, A2, and B1) m.ListSubnet(&testSubnetListOpts). Return([]subnets.Subnet{testSubnetA1, testSubnetA2, testSubnetB1}, nil) @@ -246,7 +247,7 @@ func TestService_getServerNetworks(t *testing.T) { {ID: networkAUUID, Subnet: &infrav1.Subnet{ID: subnetA1UUID}}, {ID: networkAUUID, Subnet: &infrav1.Subnet{ID: subnetA2UUID}}, }, - expect: func(m *mock_networking.MockNetworkClientMockRecorder) { + expect: func(m *mock_clients.MockNetworkClientMockRecorder) { // List tagged subnets in network A (A1 & A2) networkAFilter := testSubnetListOpts networkAFilter.NetworkID = networkAUUID @@ -268,7 +269,7 @@ func TestService_getServerNetworks(t *testing.T) { want: []infrav1.Network{ {ID: networkAUUID, Subnet: &infrav1.Subnet{ID: subnetA1UUID}}, }, - expect: func(m *mock_networking.MockNetworkClientMockRecorder) { + expect: func(m *mock_clients.MockNetworkClientMockRecorder) { }, wantErr: false, }, @@ -288,7 +289,7 @@ func TestService_getServerNetworks(t *testing.T) { {ID: networkAUUID, Subnet: &infrav1.Subnet{ID: subnetA1UUID}}, {ID: networkAUUID, Subnet: &infrav1.Subnet{ID: subnetA2UUID}}, }, - expect: func(m *mock_networking.MockNetworkClientMockRecorder) { + expect: func(m *mock_clients.MockNetworkClientMockRecorder) { // List tagged subnets in network A networkAFilter := testSubnetListOpts networkAFilter.NetworkID = networkAUUID @@ -319,7 +320,7 @@ func TestService_getServerNetworks(t *testing.T) { {ID: networkAUUID, Subnet: &infrav1.Subnet{ID: subnetA2UUID}}, {ID: networkBUUID, Subnet: &infrav1.Subnet{ID: subnetB1UUID}}, }, - expect: func(m *mock_networking.MockNetworkClientMockRecorder) { + expect: func(m *mock_clients.MockNetworkClientMockRecorder) { // List tagged networks (A & B) m.ListNetwork(&testNetworkListOpts). Return([]networks.Network{testNetworkA, testNetworkB}, nil) @@ -345,7 +346,7 @@ func TestService_getServerNetworks(t *testing.T) { {Filter: testNetworkFilter}, }, want: nil, - expect: func(m *mock_networking.MockNetworkClientMockRecorder) { + expect: func(m *mock_clients.MockNetworkClientMockRecorder) { // List tagged networks (none for this test) m.ListNetwork(&testNetworkListOpts).Return([]networks.Network{}, nil) }, @@ -363,7 +364,7 @@ func TestService_getServerNetworks(t *testing.T) { }, }, want: nil, - expect: func(m *mock_networking.MockNetworkClientMockRecorder) { + expect: func(m *mock_clients.MockNetworkClientMockRecorder) { // List tagged subnets in network A networkAFilter := testSubnetListOpts networkAFilter.NetworkID = networkAUUID @@ -379,7 +380,7 @@ func TestService_getServerNetworks(t *testing.T) { }}, }, want: nil, - expect: func(m *mock_networking.MockNetworkClientMockRecorder) { + expect: func(m *mock_clients.MockNetworkClientMockRecorder) { }, wantErr: true, }, @@ -388,7 +389,7 @@ func TestService_getServerNetworks(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { mockCtrl := gomock.NewController(t) - mockNetworkClient := mock_networking.NewMockNetworkClient(mockCtrl) + mockNetworkClient := mock_clients.NewMockNetworkClient(mockCtrl) tt.expect(mockNetworkClient.EXPECT()) networkingService := networking.NewTestService( @@ -419,7 +420,7 @@ func TestService_getImageID(t *testing.T) { testName string imageUUID string imageName string - expect func(m *MockClientMockRecorder) + expect func(m *mock_clients.MockComputeClientMockRecorder) want string wantErr bool }{ @@ -427,7 +428,7 @@ func TestService_getImageID(t *testing.T) { testName: "Return image uuid if uuid given", imageUUID: imageIDC, want: imageIDC, - expect: func(m *MockClientMockRecorder) { + expect: func(m *mock_clients.MockComputeClientMockRecorder) { }, wantErr: false, }, @@ -435,7 +436,7 @@ func TestService_getImageID(t *testing.T) { testName: "Return through uuid if both uuid and name given", imageName: "dummy", imageUUID: imageIDC, - expect: func(m *MockClientMockRecorder) { + expect: func(m *mock_clients.MockComputeClientMockRecorder) { }, want: imageIDC, wantErr: false, @@ -443,7 +444,7 @@ func TestService_getImageID(t *testing.T) { { testName: "Return image ID", imageName: "test-image", - expect: func(m *MockClientMockRecorder) { + expect: func(m *mock_clients.MockComputeClientMockRecorder) { m.ListImages(images.ListOpts{Name: "test-image"}).Return( []images.Image{{ID: imageIDA, Name: "test-image"}}, nil) @@ -454,7 +455,7 @@ func TestService_getImageID(t *testing.T) { { testName: "Return no results", imageName: "test-image", - expect: func(m *MockClientMockRecorder) { + expect: func(m *mock_clients.MockComputeClientMockRecorder) { m.ListImages(images.ListOpts{Name: "test-image"}).Return( []images.Image{}, nil) @@ -465,7 +466,7 @@ func TestService_getImageID(t *testing.T) { { testName: "Return multiple results", imageName: "test-image", - expect: func(m *MockClientMockRecorder) { + expect: func(m *mock_clients.MockComputeClientMockRecorder) { m.ListImages(images.ListOpts{Name: "test-image"}).Return( []images.Image{ {ID: imageIDA, Name: "test-image"}, @@ -478,7 +479,7 @@ func TestService_getImageID(t *testing.T) { { testName: "OpenStack returns error", imageName: "test-image", - expect: func(m *MockClientMockRecorder) { + expect: func(m *mock_clients.MockComputeClientMockRecorder) { m.ListImages(images.ListOpts{Name: "test-image"}).Return( nil, fmt.Errorf("test error")) @@ -490,7 +491,7 @@ func TestService_getImageID(t *testing.T) { for _, tt := range tests { t.Run(tt.testName, func(t *testing.T) { mockCtrl := gomock.NewController(t) - mockComputeClient := NewMockClient(mockCtrl) + mockComputeClient := mock_clients.NewMockComputeClient(mockCtrl) tt.expect(mockComputeClient.EXPECT()) s := Service{ @@ -599,8 +600,8 @@ func TestService_ReconcileInstance(t *testing.T) { } } - returnedServer := func(status string) *ServerExt { - return &ServerExt{ + returnedServer := func(status string) *clients.ServerExt { + return &clients.ServerExt{ Server: servers.Server{ ID: instanceUUID, Name: openStackMachineName, @@ -612,7 +613,7 @@ func TestService_ReconcileInstance(t *testing.T) { } // Expected calls to create a server with a single default port - expectUseExistingDefaultPort := func(networkRecorder *mock_networking.MockNetworkClientMockRecorder) { + expectUseExistingDefaultPort := func(networkRecorder *mock_clients.MockNetworkClientMockRecorder) { // Returning a pre-existing port requires fewer mocks networkRecorder.ListPort(ports.ListOpts{ Name: portName, @@ -625,7 +626,7 @@ func TestService_ReconcileInstance(t *testing.T) { }, nil) } - expectCreatePort := func(networkRecorder *mock_networking.MockNetworkClientMockRecorder, name string, networkID string) { + expectCreatePort := func(networkRecorder *mock_clients.MockNetworkClientMockRecorder, name string, networkID string) { networkRecorder.ListPort(ports.ListOpts{ Name: name, NetworkID: networkID, @@ -654,26 +655,26 @@ func TestService_ReconcileInstance(t *testing.T) { } // Expected calls if we delete the network port - expectCleanupDefaultPort := func(networkRecorder *mock_networking.MockNetworkClientMockRecorder) { + expectCleanupDefaultPort := func(networkRecorder *mock_clients.MockNetworkClientMockRecorder) { networkRecorder.ListExtensions() networkRecorder.DeletePort(portUUID).Return(nil) } // Expected calls when using default image and flavor - expectDefaultImageAndFlavor := func(computeRecorder *MockClientMockRecorder) { + expectDefaultImageAndFlavor := func(computeRecorder *mock_clients.MockComputeClientMockRecorder) { computeRecorder.ListImages(images.ListOpts{Name: imageName}).Return([]images.Image{{ID: imageUUID}}, nil) computeRecorder.GetFlavorIDFromName(flavorName).Return(flavorUUID, nil) } // Expected calls and custom match function for creating a server - expectCreateServer := func(computeRecorder *MockClientMockRecorder, expectedCreateOpts map[string]interface{}, wantError bool) { + expectCreateServer := func(computeRecorder *mock_clients.MockComputeClientMockRecorder, expectedCreateOpts map[string]interface{}, wantError bool) { // 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 // map[string]interface{} of the create options, and then use // gomega to assert the contents of the map, which is more flexible. - computeRecorder.CreateServer(gomock.Any()).DoAndReturn(func(createOpts servers.CreateOptsBuilder) (*ServerExt, error) { + computeRecorder.CreateServer(gomock.Any()).DoAndReturn(func(createOpts servers.CreateOptsBuilder) (*clients.ServerExt, error) { optsMap, err := createOpts.ToServerCreateMap() Expect(err).NotTo(HaveOccurred()) @@ -687,13 +688,13 @@ func TestService_ReconcileInstance(t *testing.T) { } // Expected calls when polling for server creation - expectServerPoll := func(computeRecorder *MockClientMockRecorder, states []string) { + expectServerPoll := func(computeRecorder *mock_clients.MockComputeClientMockRecorder, states []string) { for _, state := range states { computeRecorder.GetServer(instanceUUID).Return(returnedServer(state), nil) } } - expectServerPollSuccess := func(computeRecorder *MockClientMockRecorder) { + expectServerPollSuccess := func(computeRecorder *mock_clients.MockComputeClientMockRecorder) { expectServerPoll(computeRecorder, []string{"ACTIVE"}) } @@ -705,13 +706,13 @@ func TestService_ReconcileInstance(t *testing.T) { } // Expected calls when polling for server creation - expectVolumePoll := func(computeRecorder *MockClientMockRecorder, states []string) { + expectVolumePoll := func(computeRecorder *mock_clients.MockComputeClientMockRecorder, states []string) { for _, state := range states { computeRecorder.GetVolume(volumeUUID).Return(returnedVolume(state), nil) } } - expectVolumePollSuccess := func(computeRecorder *MockClientMockRecorder) { + expectVolumePollSuccess := func(computeRecorder *mock_clients.MockComputeClientMockRecorder) { expectVolumePoll(computeRecorder, []string{"available"}) } @@ -722,13 +723,13 @@ func TestService_ReconcileInstance(t *testing.T) { tests := []struct { name string getInstanceSpec func() *InstanceSpec - expect func(computeRecorder *MockClientMockRecorder, networkRecorder *mock_networking.MockNetworkClientMockRecorder) + expect func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) wantErr bool }{ { name: "Defaults", getInstanceSpec: getDefaultInstanceSpec, - expect: func(computeRecorder *MockClientMockRecorder, networkRecorder *mock_networking.MockNetworkClientMockRecorder) { + expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { expectUseExistingDefaultPort(networkRecorder) expectDefaultImageAndFlavor(computeRecorder) @@ -740,7 +741,7 @@ func TestService_ReconcileInstance(t *testing.T) { { name: "Delete ports on server create error", getInstanceSpec: getDefaultInstanceSpec, - expect: func(computeRecorder *MockClientMockRecorder, networkRecorder *mock_networking.MockNetworkClientMockRecorder) { + expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { expectUseExistingDefaultPort(networkRecorder) expectDefaultImageAndFlavor(computeRecorder) @@ -761,7 +762,7 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(computeRecorder *MockClientMockRecorder, networkRecorder *mock_networking.MockNetworkClientMockRecorder) { + expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { expectDefaultImageAndFlavor(computeRecorder) expectUseExistingDefaultPort(networkRecorder) @@ -779,7 +780,7 @@ func TestService_ReconcileInstance(t *testing.T) { { name: "Poll until server is created", getInstanceSpec: getDefaultInstanceSpec, - expect: func(computeRecorder *MockClientMockRecorder, networkRecorder *mock_networking.MockNetworkClientMockRecorder) { + expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { expectUseExistingDefaultPort(networkRecorder) expectDefaultImageAndFlavor(computeRecorder) @@ -791,7 +792,7 @@ func TestService_ReconcileInstance(t *testing.T) { { name: "Server errors during creation", getInstanceSpec: getDefaultInstanceSpec, - expect: func(computeRecorder *MockClientMockRecorder, networkRecorder *mock_networking.MockNetworkClientMockRecorder) { + expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { expectUseExistingDefaultPort(networkRecorder) expectDefaultImageAndFlavor(computeRecorder) @@ -811,7 +812,7 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(computeRecorder *MockClientMockRecorder, networkRecorder *mock_networking.MockNetworkClientMockRecorder) { + expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { expectUseExistingDefaultPort(networkRecorder) expectDefaultImageAndFlavor(computeRecorder) @@ -857,7 +858,7 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(computeRecorder *MockClientMockRecorder, networkRecorder *mock_networking.MockNetworkClientMockRecorder) { + expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { expectUseExistingDefaultPort(networkRecorder) expectDefaultImageAndFlavor(computeRecorder) @@ -902,7 +903,7 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(computeRecorder *MockClientMockRecorder, networkRecorder *mock_networking.MockNetworkClientMockRecorder) { + expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { expectUseExistingDefaultPort(networkRecorder) expectDefaultImageAndFlavor(computeRecorder) @@ -932,7 +933,7 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(computeRecorder *MockClientMockRecorder, networkRecorder *mock_networking.MockNetworkClientMockRecorder) { + expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { expectDefaultImageAndFlavor(computeRecorder) extensions := []extensions.Extension{ {Extension: common.Extension{Alias: "trunk"}}, @@ -983,8 +984,8 @@ func TestService_ReconcileInstance(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { mockCtrl := gomock.NewController(t) - mockComputeClient := NewMockClient(mockCtrl) - mockNetworkClient := mock_networking.NewMockNetworkClient(mockCtrl) + mockComputeClient := mock_clients.NewMockComputeClient(mockCtrl) + mockNetworkClient := mock_clients.NewMockNetworkClient(mockCtrl) computeRecorder := mockComputeClient.EXPECT() networkRecorder := mockNetworkClient.EXPECT() @@ -1016,7 +1017,7 @@ func TestService_DeleteInstance(t *testing.T) { getDefaultInstanceStatus := func() *InstanceStatus { return &InstanceStatus{ - server: &ServerExt{ + server: &clients.ServerExt{ Server: servers.Server{ ID: instanceUUID, }, @@ -1033,14 +1034,14 @@ func TestService_DeleteInstance(t *testing.T) { eventObject runtime.Object instanceStatus func() *InstanceStatus rootVolume *infrav1.RootVolume - expect func(computeRecorder *MockClientMockRecorder, networkRecorder *mock_networking.MockNetworkClientMockRecorder) + expect func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) wantErr bool }{ { name: "Defaults", eventObject: &infrav1.OpenStackMachine{}, instanceStatus: getDefaultInstanceStatus, - expect: func(computeRecorder *MockClientMockRecorder, networkRecorder *mock_networking.MockNetworkClientMockRecorder) { + expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { computeRecorder.ListAttachedInterfaces(instanceUUID).Return([]attachinterfaces.Interface{ { PortID: portUUID, @@ -1068,7 +1069,7 @@ func TestService_DeleteInstance(t *testing.T) { rootVolume: &infrav1.RootVolume{ Size: 50, }, - expect: func(computeRecorder *MockClientMockRecorder, networkRecorder *mock_networking.MockNetworkClientMockRecorder) { + expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { // Fetch volume by name volumeName := fmt.Sprintf("%s-root", openStackMachineName) computeRecorder.ListVolumes(volumes.ListOpts{ @@ -1089,8 +1090,8 @@ func TestService_DeleteInstance(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { mockCtrl := gomock.NewController(t) - mockComputeClient := NewMockClient(mockCtrl) - mockNetworkClient := mock_networking.NewMockNetworkClient(mockCtrl) + mockComputeClient := mock_clients.NewMockComputeClient(mockCtrl) + mockNetworkClient := mock_clients.NewMockNetworkClient(mockCtrl) computeRecorder := mockComputeClient.EXPECT() networkRecorder := mockNetworkClient.EXPECT() diff --git a/pkg/cloud/services/compute/instance_types.go b/pkg/cloud/services/compute/instance_types.go index 0be72ccc4e..80e0e46dbe 100644 --- a/pkg/cloud/services/compute/instance_types.go +++ b/pkg/cloud/services/compute/instance_types.go @@ -25,6 +25,7 @@ import ( corev1 "k8s.io/api/core/v1" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha6" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients" ) // InstanceSpec defines the fields which can be set on a new OpenStack instance. @@ -59,11 +60,11 @@ type InstanceIdentifier struct { // InstanceStatus represents instance data which has been returned by OpenStack. type InstanceStatus struct { - server *ServerExt + server *clients.ServerExt logger logr.Logger } -func NewInstanceStatusFromServer(server *ServerExt, logger logr.Logger) *InstanceStatus { +func NewInstanceStatusFromServer(server *clients.ServerExt, logger logr.Logger) *InstanceStatus { return &InstanceStatus{server, logger} } diff --git a/pkg/cloud/services/compute/instance_types_test.go b/pkg/cloud/services/compute/instance_types_test.go index 6915f2cc56..a37a25807b 100644 --- a/pkg/cloud/services/compute/instance_types_test.go +++ b/pkg/cloud/services/compute/instance_types_test.go @@ -22,6 +22,8 @@ import ( "github.com/go-logr/logr" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + + "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients" ) // Some arbitrary MAC addresses. @@ -41,8 +43,8 @@ type networkAddress struct { MacAddr string `json:"OS-EXT-IPS:mac_addr"` } -func serverWithAddresses(addresses map[string][]networkAddress) *ServerExt { - var server ServerExt +func serverWithAddresses(addresses map[string][]networkAddress) *clients.ServerExt { + var server clients.ServerExt server.Addresses = make(map[string]interface{}) for network, addressList := range addresses { diff --git a/pkg/cloud/services/compute/service.go b/pkg/cloud/services/compute/service.go index 8e8ba5179a..c1e6918a4d 100644 --- a/pkg/cloud/services/compute/service.go +++ b/pkg/cloud/services/compute/service.go @@ -19,55 +19,23 @@ package compute import ( "fmt" - "github.com/gophercloud/gophercloud" - "github.com/gophercloud/gophercloud/openstack" - + "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients" "sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/networking" "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" ) type Service struct { scope *scope.Scope - computeService Client + computeService clients.ComputeClient networkingService *networking.Service } -/* -NovaMinimumMicroversion is the minimum Nova microversion supported by CAPO -2.53 corresponds to OpenStack Pike - -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. -*/ -const NovaMinimumMicroversion = "2.53" - // NewService returns an instance of the compute service. func NewService(scope *scope.Scope) (*Service, error) { - computeClient, err := openstack.NewComputeV2(scope.ProviderClient, gophercloud.EndpointOpts{ - Region: scope.ProviderClientOpts.RegionName, - }) + computeService, err := clients.NewComputeClient(scope) if err != nil { - return nil, fmt.Errorf("failed to create compute service client: %v", err) + return nil, err } - computeClient.Microversion = NovaMinimumMicroversion - - imagesClient, err := openstack.NewImageServiceV2(scope.ProviderClient, gophercloud.EndpointOpts{ - Region: scope.ProviderClientOpts.RegionName, - }) - if err != nil { - return nil, fmt.Errorf("failed to create image service client: %v", err) - } - - volumeClient, err := openstack.NewBlockStorageV3(scope.ProviderClient, gophercloud.EndpointOpts{ - Region: scope.ProviderClientOpts.RegionName, - }) - if err != nil { - return nil, fmt.Errorf("failed to create volume service client: %v", err) - } - - computeService := serviceClient{computeClient, imagesClient, volumeClient} if scope.ProviderClientOpts.AuthInfo == nil { return nil, fmt.Errorf("authInfo must be set") diff --git a/pkg/cloud/services/loadbalancer/loadbalancer_test.go b/pkg/cloud/services/loadbalancer/loadbalancer_test.go index 12acf025ec..b2a665e011 100644 --- a/pkg/cloud/services/loadbalancer/loadbalancer_test.go +++ b/pkg/cloud/services/loadbalancer/loadbalancer_test.go @@ -30,9 +30,8 @@ import ( . "github.com/onsi/gomega" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha6" - "sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/loadbalancer/mock_loadbalancer" + mock_clients "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients/mock" "sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/networking" - "sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/networking/mock_networking" ) func Test_ReconcileLoadBalancer(t *testing.T) { @@ -56,27 +55,27 @@ func Test_ReconcileLoadBalancer(t *testing.T) { } type serviceFields struct { projectID string - networkingClient *mock_networking.MockNetworkClient - loadbalancerClient *mock_loadbalancer.MockLbClient + networkingClient *mock_clients.MockNetworkClient + loadbalancerClient *mock_clients.MockLbClient } lbtests := []struct { name string fields serviceFields prepareServiceMock func(sf *serviceFields) - expectNetwork func(m *mock_networking.MockNetworkClientMockRecorder) - expectLoadBalancer func(m *mock_loadbalancer.MockLbClientMockRecorder) + expectNetwork func(m *mock_clients.MockNetworkClientMockRecorder) + expectLoadBalancer func(m *mock_clients.MockLbClientMockRecorder) wantError error }{ { name: "reconcile loadbalancer in non active state should wait for active state", prepareServiceMock: func(sf *serviceFields) { - sf.networkingClient = mock_networking.NewMockNetworkClient(mockCtrl) - sf.loadbalancerClient = mock_loadbalancer.NewMockLbClient(mockCtrl) + sf.networkingClient = mock_clients.NewMockNetworkClient(mockCtrl) + sf.loadbalancerClient = mock_clients.NewMockLbClient(mockCtrl) }, - expectNetwork: func(m *mock_networking.MockNetworkClientMockRecorder) { + expectNetwork: func(m *mock_clients.MockNetworkClientMockRecorder) { // add network api call results here }, - expectLoadBalancer: func(m *mock_loadbalancer.MockLbClientMockRecorder) { + expectLoadBalancer: func(m *mock_clients.MockLbClientMockRecorder) { // return loadbalancer providers providers := []providers.Provider{ {Name: "amphora", Description: "The Octavia Amphora driver."}, diff --git a/pkg/cloud/services/loadbalancer/mock_loadbalancer/doc.go b/pkg/cloud/services/loadbalancer/mock_loadbalancer/doc.go deleted file mode 100644 index bc1750a287..0000000000 --- a/pkg/cloud/services/loadbalancer/mock_loadbalancer/doc.go +++ /dev/null @@ -1,20 +0,0 @@ -/* -Copyright 2022 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 mock_loadbalancer //nolint - -//go:generate mockgen -destination=loadbalancer_service_mock.go -package=mock_loadbalancer sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/loadbalancer LbClient -//go:generate /usr/bin/env bash -c "cat ../../../../../hack/boilerplate/boilerplate.generatego.txt loadbalancer_service_mock.go > _loadbalancer_service_mock.go && mv _loadbalancer_service_mock.go loadbalancer_service_mock.go" diff --git a/pkg/cloud/services/loadbalancer/service.go b/pkg/cloud/services/loadbalancer/service.go index d8bd357029..a1c20c21e0 100644 --- a/pkg/cloud/services/loadbalancer/service.go +++ b/pkg/cloud/services/loadbalancer/service.go @@ -20,9 +20,8 @@ import ( "fmt" "github.com/go-logr/logr" - "github.com/gophercloud/gophercloud" - "github.com/gophercloud/gophercloud/openstack" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients" "sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/networking" "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" ) @@ -31,17 +30,15 @@ import ( type Service struct { projectID string scope *scope.Scope - loadbalancerClient LbClient + loadbalancerClient clients.LbClient networkingService *networking.Service } // NewService returns an instance of the loadbalancer service. func NewService(scope *scope.Scope) (*Service, error) { - loadbalancerClient, err := openstack.NewLoadBalancerV2(scope.ProviderClient, gophercloud.EndpointOpts{ - Region: scope.ProviderClientOpts.RegionName, - }) + loadbalancerClient, err := clients.NewLbClient(scope) if err != nil { - return nil, fmt.Errorf("failed to create load balancer service client: %v", err) + return nil, err } networkingService, err := networking.NewService(scope) @@ -50,17 +47,15 @@ func NewService(scope *scope.Scope) (*Service, error) { } return &Service{ - scope: scope, - loadbalancerClient: lbClient{ - serviceClient: loadbalancerClient, - }, - networkingService: networkingService, + scope: scope, + loadbalancerClient: loadbalancerClient, + networkingService: networkingService, }, nil } // NewLoadBalancerTestService returns a Service with no initialization. It should only be used by tests. // It helps to mock the load balancer service in other packages. -func NewLoadBalancerTestService(projectID string, lbClient LbClient, client *networking.Service, logger logr.Logger) *Service { +func NewLoadBalancerTestService(projectID string, lbClient clients.LbClient, client *networking.Service, logger logr.Logger) *Service { return &Service{ projectID: projectID, scope: &scope.Scope{ diff --git a/pkg/cloud/services/networking/floatingip_test.go b/pkg/cloud/services/networking/floatingip_test.go index 417691900e..0e508be19d 100644 --- a/pkg/cloud/services/networking/floatingip_test.go +++ b/pkg/cloud/services/networking/floatingip_test.go @@ -24,7 +24,7 @@ import ( . "github.com/onsi/gomega" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha6" - "sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/networking/mock_networking" + mock_clients "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients/mock" ) func Test_GetOrCreateFloatingIP(t *testing.T) { @@ -34,13 +34,13 @@ func Test_GetOrCreateFloatingIP(t *testing.T) { tests := []struct { name string ip string - expect func(m *mock_networking.MockNetworkClientMockRecorder) + expect func(m *mock_clients.MockNetworkClientMockRecorder) want *floatingips.FloatingIP }{ { name: "creates floating IP when one doesn't already exist", ip: "192.168.111.0", - expect: func(m *mock_networking.MockNetworkClientMockRecorder) { + expect: func(m *mock_clients.MockNetworkClientMockRecorder) { m. ListFloatingIP(floatingips.ListOpts{FloatingIP: "192.168.111.0"}). Return([]floatingips.FloatingIP{}, nil) @@ -56,7 +56,7 @@ func Test_GetOrCreateFloatingIP(t *testing.T) { { name: "finds existing floating IP where one exists", ip: "192.168.111.0", - expect: func(m *mock_networking.MockNetworkClientMockRecorder) { + expect: func(m *mock_clients.MockNetworkClientMockRecorder) { m. ListFloatingIP(floatingips.ListOpts{FloatingIP: "192.168.111.0"}). Return([]floatingips.FloatingIP{{FloatingIP: "192.168.111.0"}}, nil) @@ -72,7 +72,7 @@ func Test_GetOrCreateFloatingIP(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - mockClient := mock_networking.NewMockNetworkClient(mockCtrl) + mockClient := mock_clients.NewMockNetworkClient(mockCtrl) tt.expect(mockClient.EXPECT()) s := Service{ client: mockClient, diff --git a/pkg/cloud/services/networking/mock_networking/doc.go b/pkg/cloud/services/networking/mock_networking/doc.go deleted file mode 100644 index 1a3a98275c..0000000000 --- a/pkg/cloud/services/networking/mock_networking/doc.go +++ /dev/null @@ -1,20 +0,0 @@ -/* -Copyright 2021 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 mock_networking //nolint - -//go:generate mockgen -destination=client_mock.go -package=mock_networking sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/networking NetworkClient -//go:generate /usr/bin/env bash -c "cat ../../../../../hack/boilerplate/boilerplate.generatego.txt client_mock.go > _client_mock.go && mv _client_mock.go client_mock.go" diff --git a/pkg/cloud/services/networking/port_test.go b/pkg/cloud/services/networking/port_test.go index 554b970580..54c8848f51 100644 --- a/pkg/cloud/services/networking/port_test.go +++ b/pkg/cloud/services/networking/port_test.go @@ -29,7 +29,7 @@ import ( . "github.com/onsi/gomega" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha6" - "sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/networking/mock_networking" + mock_clients "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients/mock" ) func Test_GetOrCreatePort(t *testing.T) { @@ -60,7 +60,7 @@ func Test_GetOrCreatePort(t *testing.T) { net infrav1.Network instanceSecurityGroups *[]string tags []string - expect func(m *mock_networking.MockNetworkClientMockRecorder) + expect func(m *mock_clients.MockNetworkClientMockRecorder) // Note the 'wanted' port isn't so important, since it will be whatever we tell ListPort or CreatePort to return. // Mostly in this test suite, we're checking that ListPort/CreatePort is called with the expected port opts. want *ports.Port @@ -75,7 +75,7 @@ func Test_GetOrCreatePort(t *testing.T) { }, nil, []string{}, - func(m *mock_networking.MockNetworkClientMockRecorder) { + func(m *mock_clients.MockNetworkClientMockRecorder) { m. ListPort(ports.ListOpts{ Name: "foo-port-1", @@ -98,7 +98,7 @@ func Test_GetOrCreatePort(t *testing.T) { }, nil, []string{}, - func(m *mock_networking.MockNetworkClientMockRecorder) { + func(m *mock_clients.MockNetworkClientMockRecorder) { m. ListPort(ports.ListOpts{ Name: "foo-port-1", @@ -128,7 +128,7 @@ func Test_GetOrCreatePort(t *testing.T) { }, &instanceSecurityGroups, []string{}, - func(m *mock_networking.MockNetworkClientMockRecorder) { + func(m *mock_clients.MockNetworkClientMockRecorder) { // No ports found m. ListPort(ports.ListOpts{ @@ -182,7 +182,7 @@ func Test_GetOrCreatePort(t *testing.T) { }, nil, nil, - func(m *mock_networking.MockNetworkClientMockRecorder) { + func(m *mock_clients.MockNetworkClientMockRecorder) { portCreateOpts := ports.CreateOpts{ NetworkID: netID, Name: "foo-port-bar", @@ -264,7 +264,7 @@ func Test_GetOrCreatePort(t *testing.T) { }, nil, nil, - func(m *mock_networking.MockNetworkClientMockRecorder) { + func(m *mock_clients.MockNetworkClientMockRecorder) { m. ListPort(ports.ListOpts{ Name: "foo-port-bar", @@ -301,7 +301,7 @@ func Test_GetOrCreatePort(t *testing.T) { }, &instanceSecurityGroups, []string{}, - func(m *mock_networking.MockNetworkClientMockRecorder) { + func(m *mock_clients.MockNetworkClientMockRecorder) { // No ports found m. ListPort(ports.ListOpts{ @@ -332,7 +332,7 @@ func Test_GetOrCreatePort(t *testing.T) { }, nil, []string{"my-instance-tag"}, - func(m *mock_networking.MockNetworkClientMockRecorder) { + func(m *mock_clients.MockNetworkClientMockRecorder) { // No ports found m. ListPort(ports.ListOpts{ @@ -361,7 +361,7 @@ func Test_GetOrCreatePort(t *testing.T) { }, nil, []string{"my-instance-tag"}, - func(m *mock_networking.MockNetworkClientMockRecorder) { + func(m *mock_clients.MockNetworkClientMockRecorder) { // No ports found m. ListPort(ports.ListOpts{ @@ -394,7 +394,7 @@ func Test_GetOrCreatePort(t *testing.T) { }, nil, []string{"my-tag"}, - func(m *mock_networking.MockNetworkClientMockRecorder) { + func(m *mock_clients.MockNetworkClientMockRecorder) { // No ports found m. ListPort(ports.ListOpts{ @@ -434,7 +434,7 @@ func Test_GetOrCreatePort(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - mockClient := mock_networking.NewMockNetworkClient(mockCtrl) + mockClient := mock_clients.NewMockNetworkClient(mockCtrl) tt.expect(mockClient.EXPECT()) s := Service{ client: mockClient, diff --git a/pkg/cloud/services/networking/service.go b/pkg/cloud/services/networking/service.go index e4f65483d3..8e1b307063 100644 --- a/pkg/cloud/services/networking/service.go +++ b/pkg/cloud/services/networking/service.go @@ -21,11 +21,10 @@ import ( "sort" "github.com/go-logr/logr" - "github.com/gophercloud/gophercloud" - "github.com/gophercloud/gophercloud/openstack" "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/attributestags" "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients" "sigs.k8s.io/cluster-api-provider-openstack/pkg/record" "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" ) @@ -40,16 +39,14 @@ const ( // It will create a network related infrastructure for the cluster, like network, subnet, router, security groups. type Service struct { scope *scope.Scope - client NetworkClient + client clients.NetworkClient } // NewService returns an instance of the networking service. func NewService(scope *scope.Scope) (*Service, error) { - serviceClient, err := openstack.NewNetworkV2(scope.ProviderClient, gophercloud.EndpointOpts{ - Region: scope.ProviderClientOpts.RegionName, - }) + networkClient, err := clients.NewNetworkClient(scope) if err != nil { - return nil, fmt.Errorf("failed to create networking service providerClient: %v", err) + return nil, err } if scope.ProviderClientOpts.AuthInfo == nil { @@ -58,12 +55,12 @@ func NewService(scope *scope.Scope) (*Service, error) { return &Service{ scope: scope, - client: networkClient{serviceClient}, + client: networkClient, }, nil } // NewTestService returns a Service with no initialisation. It should only be used by tests. -func NewTestService(projectID string, client NetworkClient, logger logr.Logger) *Service { +func NewTestService(projectID string, client clients.NetworkClient, logger logr.Logger) *Service { return &Service{ scope: &scope.Scope{ ProjectID: projectID, diff --git a/pkg/cloud/services/networking/trunk_test.go b/pkg/cloud/services/networking/trunk_test.go index 289b624443..f41ca9bf8c 100644 --- a/pkg/cloud/services/networking/trunk_test.go +++ b/pkg/cloud/services/networking/trunk_test.go @@ -24,7 +24,7 @@ import ( . "github.com/onsi/gomega" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha6" - "sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/networking/mock_networking" + mock_clients "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients/mock" ) func Test_GetOrCreateTrunk(t *testing.T) { @@ -35,7 +35,7 @@ func Test_GetOrCreateTrunk(t *testing.T) { name string trunkName string portID string - expect func(m *mock_networking.MockNetworkClientMockRecorder) + expect func(m *mock_clients.MockNetworkClientMockRecorder) // Note the 'wanted' port isn't so important, since it will be whatever we tell ListPort or CreatePort to return. // Mostly in this test suite, we're checking that ListPort/CreatePort is called with the expected port opts. want *trunks.Trunk @@ -45,7 +45,7 @@ func Test_GetOrCreateTrunk(t *testing.T) { "return trunk if found", "trunk-1", "port-1", - func(m *mock_networking.MockNetworkClientMockRecorder) { + func(m *mock_clients.MockNetworkClientMockRecorder) { m. ListTrunk(trunks.ListOpts{ Name: "trunk-1", @@ -62,7 +62,7 @@ func Test_GetOrCreateTrunk(t *testing.T) { "creates trunk if not found", "trunk-1", "port-1", - func(m *mock_networking.MockNetworkClientMockRecorder) { + func(m *mock_clients.MockNetworkClientMockRecorder) { // No ports found m. ListTrunk(trunks.ListOpts{ @@ -86,7 +86,7 @@ func Test_GetOrCreateTrunk(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - mockClient := mock_networking.NewMockNetworkClient(mockCtrl) + mockClient := mock_clients.NewMockNetworkClient(mockCtrl) tt.expect(mockClient.EXPECT()) s := Service{ client: mockClient, diff --git a/test/e2e/shared/openstack.go b/test/e2e/shared/openstack.go index fdbfb8c468..1fa790c466 100644 --- a/test/e2e/shared/openstack.go +++ b/test/e2e/shared/openstack.go @@ -51,12 +51,13 @@ import ( "sigs.k8s.io/yaml" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha6" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients" "sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/compute" "sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/provider" ) type ServerExtWithIP struct { - compute.ServerExt + clients.ServerExt ip string } @@ -232,7 +233,7 @@ func DumpOpenStackServers(e2eCtx *E2EContext, filter servers.ListOpts) ([]server return nil, fmt.Errorf("error creating compute client: %v", err) } - computeClient.Microversion = compute.NovaMinimumMicroversion + computeClient.Microversion = clients.NovaMinimumMicroversion allPages, err := servers.List(computeClient, filter).AllPages() if err != nil { return nil, fmt.Errorf("error listing servers: %v", err) @@ -448,7 +449,7 @@ func GetOpenStackServers(e2eCtx *E2EContext, openStackCluster *infrav1.OpenStack return nil, fmt.Errorf("error listing server: %v", err) } - var serverList []compute.ServerExt + var serverList []clients.ServerExt err = servers.ExtractServersInto(allPages, &serverList) if err != nil { return nil, fmt.Errorf("error extracting server: %v", err) From 68474a174b4f655358a4708d5b2ff09b110a78ba Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Tue, 4 Oct 2022 17:57:13 +0100 Subject: [PATCH 2/2] Split ComputeClient into compute, image, and volume Implement lazy initialisation of each in compute service. --- pkg/clients/compute.go | 122 ++++------ pkg/clients/image.go | 66 ++++++ pkg/clients/mock/compute.go | 76 ------ pkg/clients/mock/doc.go | 10 +- pkg/clients/mock/image.go | 66 ++++++ pkg/clients/mock/volume.go | 110 +++++++++ pkg/clients/volume.go | 99 ++++++++ .../services/compute/availabilityzone.go | 2 +- pkg/cloud/services/compute/instance.go | 91 ++++++-- pkg/cloud/services/compute/instance_test.go | 216 ++++++++++-------- pkg/cloud/services/compute/service.go | 74 ++++-- 11 files changed, 643 insertions(+), 289 deletions(-) create mode 100644 pkg/clients/image.go create mode 100644 pkg/clients/mock/image.go create mode 100644 pkg/clients/mock/volume.go create mode 100644 pkg/clients/volume.go diff --git a/pkg/clients/compute.go b/pkg/clients/compute.go index 2735b5cd47..e4396ddf55 100644 --- a/pkg/clients/compute.go +++ b/pkg/clients/compute.go @@ -21,11 +21,9 @@ import ( "github.com/gophercloud/gophercloud" "github.com/gophercloud/gophercloud/openstack" - "github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumes" "github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/attachinterfaces" "github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/availabilityzones" "github.com/gophercloud/gophercloud/openstack/compute/v2/servers" - "github.com/gophercloud/gophercloud/openstack/imageservice/v2/images" "github.com/gophercloud/utils/openstack/compute/v2/flavors" "sigs.k8s.io/cluster-api-provider-openstack/pkg/metrics" @@ -52,8 +50,6 @@ type ServerExt struct { type ComputeClient interface { ListAvailabilityZones() ([]availabilityzones.AvailabilityZone, error) - ListImages(listOpts images.ListOptsBuilder) ([]images.Image, error) - GetFlavorIDFromName(flavor string) (string, error) CreateServer(createOpts servers.CreateOptsBuilder) (*ServerExt, error) DeleteServer(serverID string) error @@ -62,18 +58,9 @@ type ComputeClient interface { ListAttachedInterfaces(serverID string) ([]attachinterfaces.Interface, error) DeleteAttachedInterface(serverID, portID string) error - - ListVolumes(opts volumes.ListOptsBuilder) ([]volumes.Volume, error) - CreateVolume(opts volumes.CreateOptsBuilder) (*volumes.Volume, error) - DeleteVolume(volumeID string, opts volumes.DeleteOptsBuilder) error - GetVolume(volumeID string) (*volumes.Volume, error) } -type computeClient struct { - compute *gophercloud.ServiceClient - images *gophercloud.ServiceClient - volume *gophercloud.ServiceClient -} +type computeClient struct{ client *gophercloud.ServiceClient } // NewComputeClient returns a new compute client. func NewComputeClient(scope *scope.Scope) (ComputeClient, error) { @@ -85,77 +72,54 @@ func NewComputeClient(scope *scope.Scope) (ComputeClient, error) { } compute.Microversion = NovaMinimumMicroversion - images, err := openstack.NewImageServiceV2(scope.ProviderClient, gophercloud.EndpointOpts{ - Region: scope.ProviderClientOpts.RegionName, - }) - if err != nil { - return nil, fmt.Errorf("failed to create image service client: %v", err) - } - - volume, err := openstack.NewBlockStorageV3(scope.ProviderClient, gophercloud.EndpointOpts{ - Region: scope.ProviderClientOpts.RegionName, - }) - if err != nil { - return nil, fmt.Errorf("failed to create volume service client: %v", err) - } - - return &computeClient{compute, images, volume}, nil + return &computeClient{compute}, nil } -func (s computeClient) ListAvailabilityZones() ([]availabilityzones.AvailabilityZone, error) { +func (c computeClient) ListAvailabilityZones() ([]availabilityzones.AvailabilityZone, error) { mc := metrics.NewMetricPrometheusContext("availability_zone", "list") - allPages, err := availabilityzones.List(s.compute).AllPages() + allPages, err := availabilityzones.List(c.client).AllPages() if mc.ObserveRequest(err) != nil { return nil, err } return availabilityzones.ExtractAvailabilityZones(allPages) } -func (s computeClient) ListImages(listOpts images.ListOptsBuilder) ([]images.Image, error) { - mc := metrics.NewMetricPrometheusContext("image", "list") - pages, err := images.List(s.images, listOpts).AllPages() - if mc.ObserveRequest(err) != nil { - return nil, err - } - return images.ExtractImages(pages) -} - -func (s computeClient) GetFlavorIDFromName(flavor string) (string, error) { +func (c computeClient) GetFlavorIDFromName(flavor string) (string, error) { mc := metrics.NewMetricPrometheusContext("flavor", "get") - flavorID, err := flavors.IDFromName(s.compute, flavor) + flavorID, err := flavors.IDFromName(c.client, flavor) return flavorID, mc.ObserveRequest(err) } -func (s computeClient) CreateServer(createOpts servers.CreateOptsBuilder) (*ServerExt, error) { +func (c computeClient) CreateServer(createOpts servers.CreateOptsBuilder) (*ServerExt, error) { var server ServerExt mc := metrics.NewMetricPrometheusContext("server", "create") - err := servers.Create(s.compute, createOpts).ExtractInto(&server) + err := servers.Create(c.client, createOpts).ExtractInto(&server) if mc.ObserveRequest(err) != nil { return nil, err } return &server, nil } -func (s computeClient) DeleteServer(serverID string) error { +func (c computeClient) DeleteServer(serverID string) error { mc := metrics.NewMetricPrometheusContext("server", "delete") - err := servers.Delete(s.compute, serverID).ExtractErr() + err := servers.Delete(c.client, serverID).ExtractErr() return mc.ObserveRequestIgnoreNotFound(err) } -func (s computeClient) GetServer(serverID string) (*ServerExt, error) { +func (c computeClient) GetServer(serverID string) (*ServerExt, error) { var server ServerExt mc := metrics.NewMetricPrometheusContext("server", "get") - err := servers.Get(s.compute, serverID).ExtractInto(&server) + err := servers.Get(c.client, serverID).ExtractInto(&server) if mc.ObserveRequestIgnoreNotFound(err) != nil { return nil, err } return &server, nil } -func (s computeClient) ListServers(listOpts servers.ListOptsBuilder) ([]ServerExt, error) { +func (c computeClient) ListServers(listOpts servers.ListOptsBuilder) ([]ServerExt, error) { var serverList []ServerExt mc := metrics.NewMetricPrometheusContext("server", "list") - allPages, err := servers.List(s.compute, listOpts).AllPages() + allPages, err := servers.List(c.client, listOpts).AllPages() if mc.ObserveRequest(err) != nil { return nil, err } @@ -163,44 +127,56 @@ func (s computeClient) ListServers(listOpts servers.ListOptsBuilder) ([]ServerEx return serverList, err } -func (s computeClient) ListAttachedInterfaces(serverID string) ([]attachinterfaces.Interface, error) { +func (c computeClient) ListAttachedInterfaces(serverID string) ([]attachinterfaces.Interface, error) { mc := metrics.NewMetricPrometheusContext("server_os_interface", "list") - interfaces, err := attachinterfaces.List(s.compute, serverID).AllPages() + interfaces, err := attachinterfaces.List(c.client, serverID).AllPages() if mc.ObserveRequest(err) != nil { return nil, err } return attachinterfaces.ExtractInterfaces(interfaces) } -func (s computeClient) DeleteAttachedInterface(serverID, portID string) error { +func (c computeClient) DeleteAttachedInterface(serverID, portID string) error { mc := metrics.NewMetricPrometheusContext("server_os_interface", "delete") - err := attachinterfaces.Delete(s.compute, serverID, portID).ExtractErr() + err := attachinterfaces.Delete(c.client, serverID, portID).ExtractErr() return mc.ObserveRequestIgnoreNotFoundorConflict(err) } -func (s computeClient) ListVolumes(opts volumes.ListOptsBuilder) ([]volumes.Volume, error) { - mc := metrics.NewMetricPrometheusContext("volume", "list") - pages, err := volumes.List(s.volume, opts).AllPages() - if mc.ObserveRequest(err) != nil { - return nil, err - } - return volumes.ExtractVolumes(pages) +type computeErrorClient struct{ error } + +// NewComputeErrorClient returns a ComputeClient in which every method returns the given error. +func NewComputeErrorClient(e error) ComputeClient { + return computeErrorClient{e} } -func (s computeClient) CreateVolume(opts volumes.CreateOptsBuilder) (*volumes.Volume, error) { - mc := metrics.NewMetricPrometheusContext("volume", "create") - volume, err := volumes.Create(s.volume, opts).Extract() - return volume, mc.ObserveRequest(err) +func (e computeErrorClient) ListAvailabilityZones() ([]availabilityzones.AvailabilityZone, error) { + return nil, e.error } -func (s computeClient) DeleteVolume(volumeID string, opts volumes.DeleteOptsBuilder) error { - mc := metrics.NewMetricPrometheusContext("volume", "delete") - err := volumes.Delete(s.volume, volumeID, opts).ExtractErr() - return mc.ObserveRequestIgnoreNotFound(err) +func (e computeErrorClient) GetFlavorIDFromName(flavor string) (string, error) { + return "", e.error +} + +func (e computeErrorClient) CreateServer(createOpts servers.CreateOptsBuilder) (*ServerExt, error) { + return nil, e.error +} + +func (e computeErrorClient) DeleteServer(serverID string) error { + return e.error +} + +func (e computeErrorClient) GetServer(serverID string) (*ServerExt, error) { + return nil, e.error +} + +func (e computeErrorClient) ListServers(listOpts servers.ListOptsBuilder) ([]ServerExt, error) { + return nil, e.error +} + +func (e computeErrorClient) ListAttachedInterfaces(serverID string) ([]attachinterfaces.Interface, error) { + return nil, e.error } -func (s computeClient) GetVolume(volumeID string) (*volumes.Volume, error) { - mc := metrics.NewMetricPrometheusContext("volume", "get") - volume, err := volumes.Get(s.volume, volumeID).Extract() - return volume, mc.ObserveRequestIgnoreNotFound(err) +func (e computeErrorClient) DeleteAttachedInterface(serverID, portID string) error { + return e.error } diff --git a/pkg/clients/image.go b/pkg/clients/image.go new file mode 100644 index 0000000000..4bf91212cf --- /dev/null +++ b/pkg/clients/image.go @@ -0,0 +1,66 @@ +/* +Copyright 2021 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 clients + +import ( + "fmt" + + "github.com/gophercloud/gophercloud" + "github.com/gophercloud/gophercloud/openstack" + "github.com/gophercloud/gophercloud/openstack/imageservice/v2/images" + + "sigs.k8s.io/cluster-api-provider-openstack/pkg/metrics" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" +) + +type ImageClient interface { + ListImages(listOpts images.ListOptsBuilder) ([]images.Image, error) +} + +type imageClient struct{ client *gophercloud.ServiceClient } + +// NewImageClient returns a new glance client. +func NewImageClient(scope *scope.Scope) (ImageClient, error) { + images, err := openstack.NewImageServiceV2(scope.ProviderClient, gophercloud.EndpointOpts{ + Region: scope.ProviderClientOpts.RegionName, + }) + if err != nil { + return nil, fmt.Errorf("failed to create image service client: %v", err) + } + + return imageClient{images}, nil +} + +func (c imageClient) ListImages(listOpts images.ListOptsBuilder) ([]images.Image, error) { + mc := metrics.NewMetricPrometheusContext("image", "list") + pages, err := images.List(c.client, listOpts).AllPages() + if mc.ObserveRequest(err) != nil { + return nil, err + } + return images.ExtractImages(pages) +} + +type imageErrorClient struct{ error } + +// NewImageErrorClient returns an ImageClient in which every method returns the given error. +func NewImageErrorClient(e error) ImageClient { + return imageErrorClient{e} +} + +func (e imageErrorClient) ListImages(listOpts images.ListOptsBuilder) ([]images.Image, error) { + return nil, e.error +} diff --git a/pkg/clients/mock/compute.go b/pkg/clients/mock/compute.go index f0559503d5..4eb779cb0d 100644 --- a/pkg/clients/mock/compute.go +++ b/pkg/clients/mock/compute.go @@ -24,11 +24,9 @@ import ( reflect "reflect" gomock "github.com/golang/mock/gomock" - volumes "github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumes" attachinterfaces "github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/attachinterfaces" availabilityzones "github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/availabilityzones" servers "github.com/gophercloud/gophercloud/openstack/compute/v2/servers" - images "github.com/gophercloud/gophercloud/openstack/imageservice/v2/images" clients "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients" ) @@ -70,21 +68,6 @@ func (mr *MockComputeClientMockRecorder) CreateServer(arg0 interface{}) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateServer", reflect.TypeOf((*MockComputeClient)(nil).CreateServer), arg0) } -// CreateVolume mocks base method. -func (m *MockComputeClient) CreateVolume(arg0 volumes.CreateOptsBuilder) (*volumes.Volume, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateVolume", arg0) - ret0, _ := ret[0].(*volumes.Volume) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// CreateVolume indicates an expected call of CreateVolume. -func (mr *MockComputeClientMockRecorder) CreateVolume(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateVolume", reflect.TypeOf((*MockComputeClient)(nil).CreateVolume), arg0) -} - // DeleteAttachedInterface mocks base method. func (m *MockComputeClient) DeleteAttachedInterface(arg0, arg1 string) error { m.ctrl.T.Helper() @@ -113,20 +96,6 @@ func (mr *MockComputeClientMockRecorder) DeleteServer(arg0 interface{}) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteServer", reflect.TypeOf((*MockComputeClient)(nil).DeleteServer), arg0) } -// DeleteVolume mocks base method. -func (m *MockComputeClient) DeleteVolume(arg0 string, arg1 volumes.DeleteOptsBuilder) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteVolume", arg0, arg1) - ret0, _ := ret[0].(error) - return ret0 -} - -// DeleteVolume indicates an expected call of DeleteVolume. -func (mr *MockComputeClientMockRecorder) DeleteVolume(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteVolume", reflect.TypeOf((*MockComputeClient)(nil).DeleteVolume), arg0, arg1) -} - // GetFlavorIDFromName mocks base method. func (m *MockComputeClient) GetFlavorIDFromName(arg0 string) (string, error) { m.ctrl.T.Helper() @@ -157,21 +126,6 @@ func (mr *MockComputeClientMockRecorder) GetServer(arg0 interface{}) *gomock.Cal return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetServer", reflect.TypeOf((*MockComputeClient)(nil).GetServer), arg0) } -// GetVolume mocks base method. -func (m *MockComputeClient) GetVolume(arg0 string) (*volumes.Volume, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetVolume", arg0) - ret0, _ := ret[0].(*volumes.Volume) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetVolume indicates an expected call of GetVolume. -func (mr *MockComputeClientMockRecorder) GetVolume(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetVolume", reflect.TypeOf((*MockComputeClient)(nil).GetVolume), arg0) -} - // ListAttachedInterfaces mocks base method. func (m *MockComputeClient) ListAttachedInterfaces(arg0 string) ([]attachinterfaces.Interface, error) { m.ctrl.T.Helper() @@ -202,21 +156,6 @@ func (mr *MockComputeClientMockRecorder) ListAvailabilityZones() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListAvailabilityZones", reflect.TypeOf((*MockComputeClient)(nil).ListAvailabilityZones)) } -// ListImages mocks base method. -func (m *MockComputeClient) ListImages(arg0 images.ListOptsBuilder) ([]images.Image, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ListImages", arg0) - ret0, _ := ret[0].([]images.Image) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// ListImages indicates an expected call of ListImages. -func (mr *MockComputeClientMockRecorder) ListImages(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListImages", reflect.TypeOf((*MockComputeClient)(nil).ListImages), arg0) -} - // ListServers mocks base method. func (m *MockComputeClient) ListServers(arg0 servers.ListOptsBuilder) ([]clients.ServerExt, error) { m.ctrl.T.Helper() @@ -231,18 +170,3 @@ func (mr *MockComputeClientMockRecorder) ListServers(arg0 interface{}) *gomock.C mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListServers", reflect.TypeOf((*MockComputeClient)(nil).ListServers), arg0) } - -// ListVolumes mocks base method. -func (m *MockComputeClient) ListVolumes(arg0 volumes.ListOptsBuilder) ([]volumes.Volume, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ListVolumes", arg0) - ret0, _ := ret[0].([]volumes.Volume) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// ListVolumes indicates an expected call of ListVolumes. -func (mr *MockComputeClientMockRecorder) ListVolumes(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListVolumes", reflect.TypeOf((*MockComputeClient)(nil).ListVolumes), arg0) -} diff --git a/pkg/clients/mock/doc.go b/pkg/clients/mock/doc.go index e958591f94..ce5211b005 100644 --- a/pkg/clients/mock/doc.go +++ b/pkg/clients/mock/doc.go @@ -19,8 +19,14 @@ package mock //go:generate mockgen -package mock -destination=compute.go sigs.k8s.io/cluster-api-provider-openstack/pkg/clients ComputeClient //go:generate /usr/bin/env bash -c "cat ../../../hack/boilerplate/boilerplate.generatego.txt compute.go > _compute.go && mv _compute.go compute.go" -//go:generate mockgen -package mock -destination=network.go sigs.k8s.io/cluster-api-provider-openstack/pkg/clients NetworkClient -//go:generate /usr/bin/env bash -c "cat ../../../hack/boilerplate/boilerplate.generatego.txt network.go > _network.go && mv _network.go network.go" +//go:generate mockgen -package mock -destination=image.go sigs.k8s.io/cluster-api-provider-openstack/pkg/clients ImageClient +//go:generate /usr/bin/env bash -c "cat ../../../hack/boilerplate/boilerplate.generatego.txt image.go > _image.go && mv _image.go image.go" //go:generate mockgen -package mock -destination=loadbalancer.go sigs.k8s.io/cluster-api-provider-openstack/pkg/clients LbClient //go:generate /usr/bin/env bash -c "cat ../../../hack/boilerplate/boilerplate.generatego.txt loadbalancer.go > _loadbalancer.go && mv _loadbalancer.go loadbalancer.go" + +//go:generate mockgen -package mock -destination=network.go sigs.k8s.io/cluster-api-provider-openstack/pkg/clients NetworkClient +//go:generate /usr/bin/env bash -c "cat ../../../hack/boilerplate/boilerplate.generatego.txt network.go > _network.go && mv _network.go network.go" + +//go:generate mockgen -package mock -destination=volume.go sigs.k8s.io/cluster-api-provider-openstack/pkg/clients VolumeClient +//go:generate /usr/bin/env bash -c "cat ../../../hack/boilerplate/boilerplate.generatego.txt volume.go > _volume.go && mv _volume.go volume.go" diff --git a/pkg/clients/mock/image.go b/pkg/clients/mock/image.go new file mode 100644 index 0000000000..c0dab59cc9 --- /dev/null +++ b/pkg/clients/mock/image.go @@ -0,0 +1,66 @@ +/* +Copyright 2022 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. +*/ + +// Code generated by MockGen. DO NOT EDIT. +// Source: sigs.k8s.io/cluster-api-provider-openstack/pkg/clients (interfaces: ImageClient) + +// Package mock is a generated GoMock package. +package mock + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" + images "github.com/gophercloud/gophercloud/openstack/imageservice/v2/images" +) + +// MockImageClient is a mock of ImageClient interface. +type MockImageClient struct { + ctrl *gomock.Controller + recorder *MockImageClientMockRecorder +} + +// MockImageClientMockRecorder is the mock recorder for MockImageClient. +type MockImageClientMockRecorder struct { + mock *MockImageClient +} + +// NewMockImageClient creates a new mock instance. +func NewMockImageClient(ctrl *gomock.Controller) *MockImageClient { + mock := &MockImageClient{ctrl: ctrl} + mock.recorder = &MockImageClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockImageClient) EXPECT() *MockImageClientMockRecorder { + return m.recorder +} + +// ListImages mocks base method. +func (m *MockImageClient) ListImages(arg0 images.ListOptsBuilder) ([]images.Image, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListImages", arg0) + ret0, _ := ret[0].([]images.Image) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListImages indicates an expected call of ListImages. +func (mr *MockImageClientMockRecorder) ListImages(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListImages", reflect.TypeOf((*MockImageClient)(nil).ListImages), arg0) +} diff --git a/pkg/clients/mock/volume.go b/pkg/clients/mock/volume.go new file mode 100644 index 0000000000..511ed06cd1 --- /dev/null +++ b/pkg/clients/mock/volume.go @@ -0,0 +1,110 @@ +/* +Copyright 2022 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. +*/ + +// Code generated by MockGen. DO NOT EDIT. +// Source: sigs.k8s.io/cluster-api-provider-openstack/pkg/clients (interfaces: VolumeClient) + +// Package mock is a generated GoMock package. +package mock + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" + volumes "github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumes" +) + +// MockVolumeClient is a mock of VolumeClient interface. +type MockVolumeClient struct { + ctrl *gomock.Controller + recorder *MockVolumeClientMockRecorder +} + +// MockVolumeClientMockRecorder is the mock recorder for MockVolumeClient. +type MockVolumeClientMockRecorder struct { + mock *MockVolumeClient +} + +// NewMockVolumeClient creates a new mock instance. +func NewMockVolumeClient(ctrl *gomock.Controller) *MockVolumeClient { + mock := &MockVolumeClient{ctrl: ctrl} + mock.recorder = &MockVolumeClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockVolumeClient) EXPECT() *MockVolumeClientMockRecorder { + return m.recorder +} + +// CreateVolume mocks base method. +func (m *MockVolumeClient) CreateVolume(arg0 volumes.CreateOptsBuilder) (*volumes.Volume, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateVolume", arg0) + ret0, _ := ret[0].(*volumes.Volume) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CreateVolume indicates an expected call of CreateVolume. +func (mr *MockVolumeClientMockRecorder) CreateVolume(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateVolume", reflect.TypeOf((*MockVolumeClient)(nil).CreateVolume), arg0) +} + +// DeleteVolume mocks base method. +func (m *MockVolumeClient) DeleteVolume(arg0 string, arg1 volumes.DeleteOptsBuilder) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteVolume", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteVolume indicates an expected call of DeleteVolume. +func (mr *MockVolumeClientMockRecorder) DeleteVolume(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteVolume", reflect.TypeOf((*MockVolumeClient)(nil).DeleteVolume), arg0, arg1) +} + +// GetVolume mocks base method. +func (m *MockVolumeClient) GetVolume(arg0 string) (*volumes.Volume, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetVolume", arg0) + ret0, _ := ret[0].(*volumes.Volume) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetVolume indicates an expected call of GetVolume. +func (mr *MockVolumeClientMockRecorder) GetVolume(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetVolume", reflect.TypeOf((*MockVolumeClient)(nil).GetVolume), arg0) +} + +// ListVolumes mocks base method. +func (m *MockVolumeClient) ListVolumes(arg0 volumes.ListOptsBuilder) ([]volumes.Volume, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListVolumes", arg0) + ret0, _ := ret[0].([]volumes.Volume) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListVolumes indicates an expected call of ListVolumes. +func (mr *MockVolumeClientMockRecorder) ListVolumes(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListVolumes", reflect.TypeOf((*MockVolumeClient)(nil).ListVolumes), arg0) +} diff --git a/pkg/clients/volume.go b/pkg/clients/volume.go new file mode 100644 index 0000000000..378e40a0e4 --- /dev/null +++ b/pkg/clients/volume.go @@ -0,0 +1,99 @@ +/* +Copyright 2021 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 clients + +import ( + "fmt" + + "github.com/gophercloud/gophercloud" + "github.com/gophercloud/gophercloud/openstack" + "github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumes" + + "sigs.k8s.io/cluster-api-provider-openstack/pkg/metrics" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" +) + +type VolumeClient interface { + ListVolumes(opts volumes.ListOptsBuilder) ([]volumes.Volume, error) + CreateVolume(opts volumes.CreateOptsBuilder) (*volumes.Volume, error) + DeleteVolume(volumeID string, opts volumes.DeleteOptsBuilder) error + GetVolume(volumeID string) (*volumes.Volume, error) +} + +type volumeClient struct{ client *gophercloud.ServiceClient } + +// NewVolumeClient returns a new cinder client. +func NewVolumeClient(scope *scope.Scope) (VolumeClient, error) { + volume, err := openstack.NewBlockStorageV3(scope.ProviderClient, gophercloud.EndpointOpts{ + Region: scope.ProviderClientOpts.RegionName, + }) + if err != nil { + return nil, fmt.Errorf("failed to create volume service client: %v", err) + } + + return &volumeClient{volume}, nil +} + +func (c volumeClient) ListVolumes(opts volumes.ListOptsBuilder) ([]volumes.Volume, error) { + mc := metrics.NewMetricPrometheusContext("volume", "list") + pages, err := volumes.List(c.client, opts).AllPages() + if mc.ObserveRequest(err) != nil { + return nil, err + } + return volumes.ExtractVolumes(pages) +} + +func (c volumeClient) CreateVolume(opts volumes.CreateOptsBuilder) (*volumes.Volume, error) { + mc := metrics.NewMetricPrometheusContext("volume", "create") + volume, err := volumes.Create(c.client, opts).Extract() + return volume, mc.ObserveRequest(err) +} + +func (c volumeClient) DeleteVolume(volumeID string, opts volumes.DeleteOptsBuilder) error { + mc := metrics.NewMetricPrometheusContext("volume", "delete") + err := volumes.Delete(c.client, volumeID, opts).ExtractErr() + return mc.ObserveRequestIgnoreNotFound(err) +} + +func (c volumeClient) GetVolume(volumeID string) (*volumes.Volume, error) { + mc := metrics.NewMetricPrometheusContext("volume", "get") + volume, err := volumes.Get(c.client, volumeID).Extract() + return volume, mc.ObserveRequestIgnoreNotFound(err) +} + +type volumeErrorClient struct{ error } + +// NewVolumeErrorClient returns a VolumeClient in which every method returns the given error. +func NewVolumeErrorClient(e error) VolumeClient { + return volumeErrorClient{e} +} + +func (e volumeErrorClient) ListVolumes(opts volumes.ListOptsBuilder) ([]volumes.Volume, error) { + return nil, e.error +} + +func (e volumeErrorClient) CreateVolume(opts volumes.CreateOptsBuilder) (*volumes.Volume, error) { + return nil, e.error +} + +func (e volumeErrorClient) DeleteVolume(volumeID string, opts volumes.DeleteOptsBuilder) error { + return e.error +} + +func (e volumeErrorClient) GetVolume(volumeID string) (*volumes.Volume, error) { + return nil, e.error +} diff --git a/pkg/cloud/services/compute/availabilityzone.go b/pkg/cloud/services/compute/availabilityzone.go index b434ca781f..9fdc510c50 100644 --- a/pkg/cloud/services/compute/availabilityzone.go +++ b/pkg/cloud/services/compute/availabilityzone.go @@ -23,7 +23,7 @@ import ( ) func (s *Service) GetAvailabilityZones() ([]availabilityzones.AvailabilityZone, error) { - availabilityZoneList, err := s.computeService.ListAvailabilityZones() + availabilityZoneList, err := s.getComputeClient().ListAvailabilityZones() if err != nil { return nil, fmt.Errorf("error extracting availability zone list: %v", err) } diff --git a/pkg/cloud/services/compute/instance.go b/pkg/cloud/services/compute/instance.go index c2b4a8e84e..8c4a1852bc 100644 --- a/pkg/cloud/services/compute/instance.go +++ b/pkg/cloud/services/compute/instance.go @@ -67,7 +67,12 @@ func (s *Service) constructNetworks(openStackCluster *infrav1.OpenStackCluster, if port.Network != nil { netID := port.Network.ID if netID == "" { - netIDs, err := s.networkingService.GetNetworkIDsByFilter(port.Network.ToListOpt()) + networkingService, err := s.getNetworkingService() + if err != nil { + return nil, err + } + + netIDs, err := networkingService.GetNetworkIDsByFilter(port.Network.ToListOpt()) if err != nil { return nil, err } @@ -139,7 +144,7 @@ func (s *Service) createInstanceImpl(eventObject runtime.Object, openStackCluste return nil, fmt.Errorf("error getting image ID: %v", err) } - flavorID, err := s.computeService.GetFlavorIDFromName(instanceSpec.Flavor) + flavorID, err := s.getComputeClient().GetFlavorIDFromName(instanceSpec.Flavor) if err != nil { return nil, fmt.Errorf("error getting flavor id from flavor name %s: %v", instanceSpec.Flavor, err) } @@ -160,7 +165,12 @@ func (s *Service) createInstanceImpl(eventObject runtime.Object, openStackCluste return nil, err } - securityGroups, err := s.networkingService.GetSecurityGroups(instanceSpec.SecurityGroups) + networkingService, err := s.getNetworkingService() + if err != nil { + return nil, err + } + + securityGroups, err := networkingService.GetSecurityGroups(instanceSpec.SecurityGroups) if err != nil { return nil, fmt.Errorf("error getting security groups: %v", err) } @@ -174,7 +184,7 @@ func (s *Service) createInstanceImpl(eventObject runtime.Object, openStackCluste iTags = instanceSpec.Tags } portName := getPortName(instanceSpec.Name, network.PortOpts, i) - port, err := s.networkingService.GetOrCreatePort(eventObject, clusterName, portName, network, &securityGroups, iTags) + port, err := networkingService.GetOrCreatePort(eventObject, clusterName, portName, network, &securityGroups, iTags) if err != nil { return nil, err } @@ -201,7 +211,7 @@ func (s *Service) createInstanceImpl(eventObject runtime.Object, openStackCluste // Wait for volume to become available if volume != nil { err = util.PollImmediate(retryIntervalInstanceStatus, instanceCreateTimeout, func() (bool, error) { - createdVolume, err := s.computeService.GetVolume(volume.ID) + createdVolume, err := s.getVolumeClient().GetVolume(volume.ID) if err != nil { if capoerrors.IsRetryable(err) { return false, nil @@ -246,7 +256,7 @@ func (s *Service) createInstanceImpl(eventObject runtime.Object, openStackCluste serverCreateOpts = applyServerGroupID(serverCreateOpts, instanceSpec.ServerGroupID) - server, err = s.computeService.CreateServer(keypairs.CreateOptsExt{ + server, err = s.getComputeClient().CreateServer(keypairs.CreateOptsExt{ CreateOptsBuilder: serverCreateOpts, KeyName: instanceSpec.SSHKeyName, }) @@ -299,7 +309,7 @@ func (s *Service) getVolumeByName(name string) (*volumes.Volume, error) { Name: name, TenantID: s.scope.ProjectID, } - volumeList, err := s.computeService.ListVolumes(listOpts) + volumeList, err := s.getVolumeClient().ListVolumes(listOpts) if err != nil { return nil, fmt.Errorf("error listing volumes: %w", err) } @@ -348,7 +358,7 @@ func (s *Service) getOrCreateRootVolume(eventObject runtime.Object, instanceSpec AvailabilityZone: availabilityZone, VolumeType: rootVolume.VolumeType, } - volume, err = s.computeService.CreateVolume(createOpts) + volume, err = s.getVolumeClient().CreateVolume(createOpts) if err != nil { record.Eventf(eventObject, "FailedCreateVolume", "Failed to create root volume; size=%d imageID=%s err=%v", size, imageID, err) return nil, err @@ -424,7 +434,13 @@ func (s *Service) getServerNetworks(networkParams []infrav1.NetworkParam) ([]inf if netID != "" { subnetOpts.NetworkID = netID } - subnetsByFilter, err := s.networkingService.GetSubnetsByFilter(&subnetOpts) + + networkingService, err := s.getNetworkingService() + if err != nil { + return err + } + + subnetsByFilter, err := networkingService.GetSubnetsByFilter(&subnetOpts) if err != nil { return err } @@ -449,8 +465,14 @@ func (s *Service) getServerNetworks(networkParams []infrav1.NetworkParam) ([]inf } continue } + + networkingService, err := s.getNetworkingService() + if err != nil { + return nil, err + } + opts := networkParam.Filter.ToListOpt() - ids, err := s.networkingService.GetNetworkIDsByFilter(&opts) + ids, err := networkingService.GetNetworkIDsByFilter(&opts) if err != nil { return nil, err } @@ -469,7 +491,7 @@ func (s *Service) getImageIDFromName(imageName string) (string, error) { opts.Name = imageName - allImages, err := s.computeService.ListImages(opts) + allImages, err := s.getImageClient().ListImages(opts) if err != nil { return "", err } @@ -504,7 +526,13 @@ func (s *Service) GetManagementPort(openStackCluster *infrav1.OpenStackCluster, if err != nil { return nil, err } - allPorts, err := s.networkingService.GetPortFromInstanceIP(instanceStatus.ID(), ns.IP(openStackCluster.Status.Network.Name)) + + networkingService, err := s.getNetworkingService() + if err != nil { + return nil, err + } + + allPorts, err := networkingService.GetPortFromInstanceIP(instanceStatus.ID(), ns.IP(openStackCluster.Status.Network.Name)) if err != nil { return nil, fmt.Errorf("lookup management port for server %s: %w", instanceStatus.ID(), err) } @@ -545,13 +573,13 @@ func (s *Service) DeleteInstance(eventObject runtime.Object, instanceStatus *Ins } s.scope.Logger.Info("deleting dangling root volume %s(%s)", volume.Name, volume.ID) - return s.computeService.DeleteVolume(volume.ID, volumes.DeleteOpts{}) + return s.getVolumeClient().DeleteVolume(volume.ID, volumes.DeleteOpts{}) } return nil } - instanceInterfaces, err := s.computeService.ListAttachedInterfaces(instanceStatus.ID()) + instanceInterfaces, err := s.getComputeClient().ListAttachedInterfaces(instanceStatus.ID()) if err != nil { return err } @@ -561,6 +589,11 @@ func (s *Service) DeleteInstance(eventObject runtime.Object, instanceStatus *Ins return fmt.Errorf("obtaining network extensions: %v", err) } + networkingService, err := s.getNetworkingService() + if err != nil { + return err + } + // get and delete trunks for _, port := range instanceInterfaces { if err = s.deleteAttachInterface(eventObject, instanceStatus.InstanceIdentifier(), port.PortID); err != nil { @@ -568,18 +601,18 @@ func (s *Service) DeleteInstance(eventObject runtime.Object, instanceStatus *Ins } if trunkSupported { - if err = s.networkingService.DeleteTrunk(eventObject, port.PortID); err != nil { + if err = networkingService.DeleteTrunk(eventObject, port.PortID); err != nil { return err } } - if err = s.networkingService.DeletePort(eventObject, port.PortID); err != nil { + if err = networkingService.DeletePort(eventObject, port.PortID); err != nil { return err } } // delete port of error instance if instanceStatus.State() == infrav1.InstanceStateError { - if err := s.networkingService.GarbageCollectErrorInstancesPort(eventObject, instanceStatus.Name()); err != nil { + if err := networkingService.GarbageCollectErrorInstancesPort(eventObject, instanceStatus.Name()); err != nil { return err } } @@ -594,12 +627,17 @@ func (s *Service) deletePorts(eventObject runtime.Object, nets []servers.Network } for _, n := range nets { + networkingService, err := s.getNetworkingService() + if err != nil { + return err + } + if trunkSupported { - if err = s.networkingService.DeleteTrunk(eventObject, n.Port); err != nil { + if err = networkingService.DeleteTrunk(eventObject, n.Port); err != nil { return err } } - if err := s.networkingService.DeletePort(eventObject, n.Port); err != nil { + if err := networkingService.DeletePort(eventObject, n.Port); err != nil { return err } } @@ -607,7 +645,7 @@ func (s *Service) deletePorts(eventObject runtime.Object, nets []servers.Network } func (s *Service) deleteAttachInterface(eventObject runtime.Object, instance *InstanceIdentifier, portID string) error { - err := s.computeService.DeleteAttachedInterface(instance.ID, portID) + err := s.getComputeClient().DeleteAttachedInterface(instance.ID, portID) if err != nil { if capoerrors.IsNotFound(err) { record.Eventf(eventObject, "SuccessfulDeleteAttachInterface", "Attach interface did not exist: instance %s, port %s", instance.ID, portID) @@ -627,7 +665,7 @@ func (s *Service) deleteAttachInterface(eventObject runtime.Object, instance *In } func (s *Service) deleteInstance(eventObject runtime.Object, instance *InstanceIdentifier) error { - err := s.computeService.DeleteServer(instance.ID) + err := s.getComputeClient().DeleteServer(instance.ID) if err != nil { if capoerrors.IsNotFound(err) { record.Eventf(eventObject, "SuccessfulDeleteServer", "Server %s with id %s did not exist", instance.Name, instance.ID) @@ -661,7 +699,7 @@ func (s *Service) GetInstanceStatus(resourceID string) (instance *InstanceStatus return nil, fmt.Errorf("resourceId should be specified to get detail") } - server, err := s.computeService.GetServer(resourceID) + server, err := s.getComputeClient().GetServer(resourceID) if err != nil { if capoerrors.IsNotFound(err) { return nil, nil @@ -685,7 +723,7 @@ func (s *Service) GetInstanceStatusByName(eventObject runtime.Object, name strin listOpts = servers.ListOpts{} } - serverList, err := s.computeService.ListServers(listOpts) + serverList, err := s.getComputeClient().ListServers(listOpts) if err != nil { return nil, fmt.Errorf("get server list: %v", err) } @@ -713,7 +751,12 @@ func getTimeout(name string, timeout int) time.Duration { // isTrunkExtSupported verifies trunk setup on the OpenStack deployment. func (s *Service) isTrunkExtSupported() (trunknSupported bool, err error) { - trunkSupport, err := s.networkingService.GetTrunkSupport() + networkingService, err := s.getNetworkingService() + if err != nil { + return false, err + } + + trunkSupport, err := networkingService.GetTrunkSupport() if err != nil { return false, fmt.Errorf("there was an issue verifying whether trunk support is available, Please try again later: %v", err) } diff --git a/pkg/cloud/services/compute/instance_test.go b/pkg/cloud/services/compute/instance_test.go index 99ec5140ee..79248b8b46 100644 --- a/pkg/cloud/services/compute/instance_test.go +++ b/pkg/cloud/services/compute/instance_test.go @@ -396,7 +396,7 @@ func TestService_getServerNetworks(t *testing.T) { "", mockNetworkClient, logr.Discard(), ) s := &Service{ - networkingService: networkingService, + _networkingService: networkingService, } got, err := s.getServerNetworks(tt.networkParams) @@ -420,7 +420,7 @@ func TestService_getImageID(t *testing.T) { testName string imageUUID string imageName string - expect func(m *mock_clients.MockComputeClientMockRecorder) + expect func(m *mock_clients.MockImageClientMockRecorder) want string wantErr bool }{ @@ -428,7 +428,7 @@ func TestService_getImageID(t *testing.T) { testName: "Return image uuid if uuid given", imageUUID: imageIDC, want: imageIDC, - expect: func(m *mock_clients.MockComputeClientMockRecorder) { + expect: func(m *mock_clients.MockImageClientMockRecorder) { }, wantErr: false, }, @@ -436,7 +436,7 @@ func TestService_getImageID(t *testing.T) { testName: "Return through uuid if both uuid and name given", imageName: "dummy", imageUUID: imageIDC, - expect: func(m *mock_clients.MockComputeClientMockRecorder) { + expect: func(m *mock_clients.MockImageClientMockRecorder) { }, want: imageIDC, wantErr: false, @@ -444,7 +444,7 @@ func TestService_getImageID(t *testing.T) { { testName: "Return image ID", imageName: "test-image", - expect: func(m *mock_clients.MockComputeClientMockRecorder) { + expect: func(m *mock_clients.MockImageClientMockRecorder) { m.ListImages(images.ListOpts{Name: "test-image"}).Return( []images.Image{{ID: imageIDA, Name: "test-image"}}, nil) @@ -455,7 +455,7 @@ func TestService_getImageID(t *testing.T) { { testName: "Return no results", imageName: "test-image", - expect: func(m *mock_clients.MockComputeClientMockRecorder) { + expect: func(m *mock_clients.MockImageClientMockRecorder) { m.ListImages(images.ListOpts{Name: "test-image"}).Return( []images.Image{}, nil) @@ -466,7 +466,7 @@ func TestService_getImageID(t *testing.T) { { testName: "Return multiple results", imageName: "test-image", - expect: func(m *mock_clients.MockComputeClientMockRecorder) { + expect: func(m *mock_clients.MockImageClientMockRecorder) { m.ListImages(images.ListOpts{Name: "test-image"}).Return( []images.Image{ {ID: imageIDA, Name: "test-image"}, @@ -479,7 +479,7 @@ func TestService_getImageID(t *testing.T) { { testName: "OpenStack returns error", imageName: "test-image", - expect: func(m *mock_clients.MockComputeClientMockRecorder) { + expect: func(m *mock_clients.MockImageClientMockRecorder) { m.ListImages(images.ListOpts{Name: "test-image"}).Return( nil, fmt.Errorf("test error")) @@ -491,16 +491,16 @@ func TestService_getImageID(t *testing.T) { for _, tt := range tests { t.Run(tt.testName, func(t *testing.T) { mockCtrl := gomock.NewController(t) - mockComputeClient := mock_clients.NewMockComputeClient(mockCtrl) - tt.expect(mockComputeClient.EXPECT()) + mockImageClient := mock_clients.NewMockImageClient(mockCtrl) + tt.expect(mockImageClient.EXPECT()) s := Service{ scope: &scope.Scope{ ProjectID: "", Logger: logr.Discard(), }, - computeService: mockComputeClient, - networkingService: &networking.Service{}, + _imageClient: mockImageClient, + _networkingService: &networking.Service{}, } got, err := s.getImageID(tt.imageUUID, tt.imageName) @@ -661,8 +661,8 @@ func TestService_ReconcileInstance(t *testing.T) { } // Expected calls when using default image and flavor - expectDefaultImageAndFlavor := func(computeRecorder *mock_clients.MockComputeClientMockRecorder) { - computeRecorder.ListImages(images.ListOpts{Name: imageName}).Return([]images.Image{{ID: imageUUID}}, nil) + expectDefaultImageAndFlavor := func(computeRecorder *mock_clients.MockComputeClientMockRecorder, imageRecorder *mock_clients.MockImageClientMockRecorder) { + imageRecorder.ListImages(images.ListOpts{Name: imageName}).Return([]images.Image{{ID: imageUUID}}, nil) computeRecorder.GetFlavorIDFromName(flavorName).Return(flavorUUID, nil) } @@ -706,49 +706,56 @@ func TestService_ReconcileInstance(t *testing.T) { } // Expected calls when polling for server creation - expectVolumePoll := func(computeRecorder *mock_clients.MockComputeClientMockRecorder, states []string) { + expectVolumePoll := func(volumeRecorder *mock_clients.MockVolumeClientMockRecorder, states []string) { for _, state := range states { - computeRecorder.GetVolume(volumeUUID).Return(returnedVolume(state), nil) + volumeRecorder.GetVolume(volumeUUID).Return(returnedVolume(state), nil) } } - expectVolumePollSuccess := func(computeRecorder *mock_clients.MockComputeClientMockRecorder) { - expectVolumePoll(computeRecorder, []string{"available"}) + expectVolumePollSuccess := func(volumeRecorder *mock_clients.MockVolumeClientMockRecorder) { + expectVolumePoll(volumeRecorder, []string{"available"}) } // ******************* // START OF TEST CASES // ******************* + type recorders struct { + compute *mock_clients.MockComputeClientMockRecorder + image *mock_clients.MockImageClientMockRecorder + network *mock_clients.MockNetworkClientMockRecorder + volume *mock_clients.MockVolumeClientMockRecorder + } + tests := []struct { name string getInstanceSpec func() *InstanceSpec - expect func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) + expect func(r *recorders) wantErr bool }{ { name: "Defaults", getInstanceSpec: getDefaultInstanceSpec, - expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { - expectUseExistingDefaultPort(networkRecorder) - expectDefaultImageAndFlavor(computeRecorder) + expect: func(r *recorders) { + expectUseExistingDefaultPort(r.network) + expectDefaultImageAndFlavor(r.compute, r.image) - expectCreateServer(computeRecorder, getDefaultServerMap(), false) - expectServerPollSuccess(computeRecorder) + expectCreateServer(r.compute, getDefaultServerMap(), false) + expectServerPollSuccess(r.compute) }, wantErr: false, }, { name: "Delete ports on server create error", getInstanceSpec: getDefaultInstanceSpec, - expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { - expectUseExistingDefaultPort(networkRecorder) - expectDefaultImageAndFlavor(computeRecorder) + expect: func(r *recorders) { + expectUseExistingDefaultPort(r.network) + expectDefaultImageAndFlavor(r.compute, r.image) - expectCreateServer(computeRecorder, getDefaultServerMap(), true) + expectCreateServer(r.compute, getDefaultServerMap(), true) // Make sure we delete ports - expectCleanupDefaultPort(networkRecorder) + expectCleanupDefaultPort(r.network) }, wantErr: true, }, @@ -762,42 +769,42 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { - expectDefaultImageAndFlavor(computeRecorder) - expectUseExistingDefaultPort(networkRecorder) + expect: func(r *recorders) { + expectDefaultImageAndFlavor(r.compute, r.image) + expectUseExistingDefaultPort(r.network) // Looking up the second port fails - networkRecorder.ListPort(ports.ListOpts{ + r.network.ListPort(ports.ListOpts{ Name: "test-openstack-machine-1", NetworkID: networkUUID, }).Return(nil, fmt.Errorf("test error")) // We should cleanup the first port - expectCleanupDefaultPort(networkRecorder) + expectCleanupDefaultPort(r.network) }, wantErr: true, }, { name: "Poll until server is created", getInstanceSpec: getDefaultInstanceSpec, - expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { - expectUseExistingDefaultPort(networkRecorder) - expectDefaultImageAndFlavor(computeRecorder) + expect: func(r *recorders) { + expectUseExistingDefaultPort(r.network) + expectDefaultImageAndFlavor(r.compute, r.image) - expectCreateServer(computeRecorder, getDefaultServerMap(), false) - expectServerPoll(computeRecorder, []string{"BUILDING", "ACTIVE"}) + expectCreateServer(r.compute, getDefaultServerMap(), false) + expectServerPoll(r.compute, []string{"BUILDING", "ACTIVE"}) }, wantErr: false, }, { name: "Server errors during creation", getInstanceSpec: getDefaultInstanceSpec, - expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { - expectUseExistingDefaultPort(networkRecorder) - expectDefaultImageAndFlavor(computeRecorder) + expect: func(r *recorders) { + expectUseExistingDefaultPort(r.network) + expectDefaultImageAndFlavor(r.compute, r.image) - expectCreateServer(computeRecorder, getDefaultServerMap(), false) - expectServerPoll(computeRecorder, []string{"BUILDING", "ERROR"}) + expectCreateServer(r.compute, getDefaultServerMap(), false) + expectServerPoll(r.compute, []string{"BUILDING", "ERROR"}) // Don't delete ports because the server is created: DeleteInstance will do it }, @@ -812,13 +819,13 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { - expectUseExistingDefaultPort(networkRecorder) - expectDefaultImageAndFlavor(computeRecorder) + expect: func(r *recorders) { + expectUseExistingDefaultPort(r.network) + expectDefaultImageAndFlavor(r.compute, r.image) - computeRecorder.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-root", openStackMachineName)}). + r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-root", openStackMachineName)}). Return([]volumes.Volume{}, nil) - computeRecorder.CreateVolume(volumes.CreateOpts{ + r.volume.CreateVolume(volumes.CreateOpts{ Size: 50, AvailabilityZone: failureDomain, Description: fmt.Sprintf("Root volume for %s", openStackMachineName), @@ -826,7 +833,7 @@ func TestService_ReconcileInstance(t *testing.T) { ImageID: imageUUID, Multiattach: false, }).Return(&volumes.Volume{ID: volumeUUID}, nil) - expectVolumePollSuccess(computeRecorder) + expectVolumePollSuccess(r.volume) createMap := getDefaultServerMap() serverMap := createMap["server"].(map[string]interface{}) @@ -840,8 +847,8 @@ func TestService_ReconcileInstance(t *testing.T) { "boot_index": float64(0), }, } - expectCreateServer(computeRecorder, createMap, false) - expectServerPollSuccess(computeRecorder) + expectCreateServer(r.compute, createMap, false) + expectServerPollSuccess(r.compute) // Don't delete ports because the server is created: DeleteInstance will do it }, @@ -858,13 +865,13 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { - expectUseExistingDefaultPort(networkRecorder) - expectDefaultImageAndFlavor(computeRecorder) + expect: func(r *recorders) { + expectUseExistingDefaultPort(r.network) + expectDefaultImageAndFlavor(r.compute, r.image) - computeRecorder.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-root", openStackMachineName)}). + r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-root", openStackMachineName)}). Return([]volumes.Volume{}, nil) - computeRecorder.CreateVolume(volumes.CreateOpts{ + r.volume.CreateVolume(volumes.CreateOpts{ Size: 50, AvailabilityZone: "test-alternate-az", VolumeType: "test-volume-type", @@ -873,7 +880,7 @@ func TestService_ReconcileInstance(t *testing.T) { ImageID: imageUUID, Multiattach: false, }).Return(&volumes.Volume{ID: volumeUUID}, nil) - expectVolumePollSuccess(computeRecorder) + expectVolumePollSuccess(r.volume) createMap := getDefaultServerMap() serverMap := createMap["server"].(map[string]interface{}) @@ -887,8 +894,8 @@ func TestService_ReconcileInstance(t *testing.T) { "boot_index": float64(0), }, } - expectCreateServer(computeRecorder, createMap, false) - expectServerPollSuccess(computeRecorder) + expectCreateServer(r.compute, createMap, false) + expectServerPollSuccess(r.compute) // Don't delete ports because the server is created: DeleteInstance will do it }, @@ -903,13 +910,13 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { - expectUseExistingDefaultPort(networkRecorder) - expectDefaultImageAndFlavor(computeRecorder) + expect: func(r *recorders) { + expectUseExistingDefaultPort(r.network) + expectDefaultImageAndFlavor(r.compute, r.image) - computeRecorder.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-root", openStackMachineName)}). + r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-root", openStackMachineName)}). Return([]volumes.Volume{}, nil) - computeRecorder.CreateVolume(volumes.CreateOpts{ + r.volume.CreateVolume(volumes.CreateOpts{ Size: 50, AvailabilityZone: failureDomain, Description: fmt.Sprintf("Root volume for %s", openStackMachineName), @@ -917,9 +924,9 @@ func TestService_ReconcileInstance(t *testing.T) { ImageID: imageUUID, Multiattach: false, }).Return(&volumes.Volume{ID: volumeUUID}, nil) - expectVolumePoll(computeRecorder, []string{"creating", "error"}) + expectVolumePoll(r.volume, []string{"creating", "error"}) - expectCleanupDefaultPort(networkRecorder) + expectCleanupDefaultPort(r.network) }, wantErr: true, }, @@ -933,17 +940,17 @@ func TestService_ReconcileInstance(t *testing.T) { } return s }, - expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { - expectDefaultImageAndFlavor(computeRecorder) + expect: func(r *recorders) { + expectDefaultImageAndFlavor(r.compute, r.image) extensions := []extensions.Extension{ {Extension: common.Extension{Alias: "trunk"}}, } - networkRecorder.ListExtensions().Return(extensions, nil) + r.network.ListExtensions().Return(extensions, nil) - expectCreatePort(networkRecorder, portName, networkUUID) + expectCreatePort(r.network, portName, networkUUID) // Check for existing trunk - networkRecorder.ListTrunk(newGomegaMockMatcher( + r.network.ListTrunk(newGomegaMockMatcher( MatchFields(IgnoreExtras, Fields{ "Name": Equal(portName), "PortID": Equal(portUUID), @@ -951,32 +958,32 @@ func TestService_ReconcileInstance(t *testing.T) { )).Return([]trunks.Trunk{}, nil) // Create new trunk - networkRecorder.CreateTrunk(newGomegaMockMatcher(MatchFields(IgnoreExtras, Fields{ + r.network.CreateTrunk(newGomegaMockMatcher(MatchFields(IgnoreExtras, Fields{ "Name": Equal(portName), "PortID": Equal(portUUID), }))).Return(&trunks.Trunk{ PortID: portUUID, ID: trunkUUID, }, nil) - networkRecorder.ReplaceAllAttributesTags("trunks", trunkUUID, attributestags.ReplaceAllOpts{Tags: []string{"test-tag"}}).Return(nil, nil) + r.network.ReplaceAllAttributesTags("trunks", trunkUUID, attributestags.ReplaceAllOpts{Tags: []string{"test-tag"}}).Return(nil, nil) // Looking up the second port fails - networkRecorder.ListPort(ports.ListOpts{ + r.network.ListPort(ports.ListOpts{ Name: "test-openstack-machine-1", NetworkID: networkUUID, }).Return(nil, fmt.Errorf("test error")) - networkRecorder.ListExtensions().Return(extensions, nil) + r.network.ListExtensions().Return(extensions, nil) - networkRecorder.ListTrunk(newGomegaMockMatcher( + r.network.ListTrunk(newGomegaMockMatcher( MatchFields(IgnoreExtras, Fields{ "PortID": Equal(portUUID), }), )).Return([]trunks.Trunk{{ID: trunkUUID}}, nil) // We should cleanup the first port and its trunk - networkRecorder.DeleteTrunk(trunkUUID).Return(nil) - networkRecorder.DeletePort(portUUID).Return(nil) + r.network.DeleteTrunk(trunkUUID).Return(nil) + r.network.DeletePort(portUUID).Return(nil) }, wantErr: true, }, @@ -985,22 +992,28 @@ func TestService_ReconcileInstance(t *testing.T) { t.Run(tt.name, func(t *testing.T) { mockCtrl := gomock.NewController(t) mockComputeClient := mock_clients.NewMockComputeClient(mockCtrl) + mockImageClient := mock_clients.NewMockImageClient(mockCtrl) mockNetworkClient := mock_clients.NewMockNetworkClient(mockCtrl) + mockVolumeClient := mock_clients.NewMockVolumeClient(mockCtrl) computeRecorder := mockComputeClient.EXPECT() + imageRecorder := mockImageClient.EXPECT() networkRecorder := mockNetworkClient.EXPECT() + volumeRecorder := mockVolumeClient.EXPECT() - tt.expect(computeRecorder, networkRecorder) + tt.expect(&recorders{computeRecorder, imageRecorder, networkRecorder, volumeRecorder}) s := Service{ scope: &scope.Scope{ Logger: logr.Discard(), ProjectID: "", }, - computeService: mockComputeClient, - networkingService: networking.NewTestService( + _computeClient: mockComputeClient, + _imageClient: mockImageClient, + _networkingService: networking.NewTestService( "", mockNetworkClient, logr.Discard(), ), + _volumeClient: mockVolumeClient, } // Call CreateInstance with a reduced retry interval to speed up the test _, err := s.createInstanceImpl(&infrav1.OpenStackMachine{}, getDefaultOpenStackCluster(), tt.getInstanceSpec(), "cluster-name", time.Nanosecond) @@ -1029,36 +1042,42 @@ func TestService_DeleteInstance(t *testing.T) { // START OF TEST CASES // ******************* + type recorders struct { + compute *mock_clients.MockComputeClientMockRecorder + network *mock_clients.MockNetworkClientMockRecorder + volume *mock_clients.MockVolumeClientMockRecorder + } + tests := []struct { name string eventObject runtime.Object instanceStatus func() *InstanceStatus rootVolume *infrav1.RootVolume - expect func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) + expect func(r *recorders) wantErr bool }{ { name: "Defaults", eventObject: &infrav1.OpenStackMachine{}, instanceStatus: getDefaultInstanceStatus, - expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { - computeRecorder.ListAttachedInterfaces(instanceUUID).Return([]attachinterfaces.Interface{ + expect: func(r *recorders) { + r.compute.ListAttachedInterfaces(instanceUUID).Return([]attachinterfaces.Interface{ { PortID: portUUID, }, }, nil) - networkRecorder.ListExtensions().Return([]extensions.Extension{{ + r.network.ListExtensions().Return([]extensions.Extension{{ Extension: common.Extension{ Alias: "trunk", }, }}, nil) - computeRecorder.DeleteAttachedInterface(instanceUUID, portUUID).Return(nil) + r.compute.DeleteAttachedInterface(instanceUUID, portUUID).Return(nil) // FIXME: Why we are looking for a trunk when we know the port is not trunked? - networkRecorder.ListTrunk(trunks.ListOpts{PortID: portUUID}).Return([]trunks.Trunk{}, nil) - networkRecorder.DeletePort(portUUID).Return(nil) + r.network.ListTrunk(trunks.ListOpts{PortID: portUUID}).Return([]trunks.Trunk{}, nil) + r.network.DeletePort(portUUID).Return(nil) - computeRecorder.DeleteServer(instanceUUID).Return(nil) - computeRecorder.GetServer(instanceUUID).Return(nil, gophercloud.ErrDefault404{}) + r.compute.DeleteServer(instanceUUID).Return(nil) + r.compute.GetServer(instanceUUID).Return(nil, gophercloud.ErrDefault404{}) }, wantErr: false, }, @@ -1069,10 +1088,10 @@ func TestService_DeleteInstance(t *testing.T) { rootVolume: &infrav1.RootVolume{ Size: 50, }, - expect: func(computeRecorder *mock_clients.MockComputeClientMockRecorder, networkRecorder *mock_clients.MockNetworkClientMockRecorder) { + expect: func(r *recorders) { // Fetch volume by name volumeName := fmt.Sprintf("%s-root", openStackMachineName) - computeRecorder.ListVolumes(volumes.ListOpts{ + r.volume.ListVolumes(volumes.ListOpts{ AllTenants: false, Name: volumeName, TenantID: "", @@ -1082,7 +1101,7 @@ func TestService_DeleteInstance(t *testing.T) { }}, nil) // Delete volume - computeRecorder.DeleteVolume(volumeUUID, volumes.DeleteOpts{}).Return(nil) + r.volume.DeleteVolume(volumeUUID, volumes.DeleteOpts{}).Return(nil) }, wantErr: false, }, @@ -1092,21 +1111,24 @@ func TestService_DeleteInstance(t *testing.T) { mockCtrl := gomock.NewController(t) mockComputeClient := mock_clients.NewMockComputeClient(mockCtrl) mockNetworkClient := mock_clients.NewMockNetworkClient(mockCtrl) + mockVolumeClient := mock_clients.NewMockVolumeClient(mockCtrl) computeRecorder := mockComputeClient.EXPECT() networkRecorder := mockNetworkClient.EXPECT() + volumeRecorder := mockVolumeClient.EXPECT() - tt.expect(computeRecorder, networkRecorder) + tt.expect(&recorders{computeRecorder, networkRecorder, volumeRecorder}) s := Service{ scope: &scope.Scope{ ProjectID: "", Logger: logr.Discard(), }, - computeService: mockComputeClient, - networkingService: networking.NewTestService( + _computeClient: mockComputeClient, + _networkingService: networking.NewTestService( "", mockNetworkClient, logr.Discard(), ), + _volumeClient: mockVolumeClient, } if err := s.DeleteInstance(tt.eventObject, tt.instanceStatus(), openStackMachineName, tt.rootVolume); (err != nil) != tt.wantErr { t.Errorf("Service.DeleteInstance() error = %v, wantErr %v", err, tt.wantErr) diff --git a/pkg/cloud/services/compute/service.go b/pkg/cloud/services/compute/service.go index c1e6918a4d..104c12fe5d 100644 --- a/pkg/cloud/services/compute/service.go +++ b/pkg/cloud/services/compute/service.go @@ -25,30 +25,72 @@ import ( ) type Service struct { - scope *scope.Scope - computeService clients.ComputeClient - networkingService *networking.Service + scope *scope.Scope + _computeClient clients.ComputeClient + _volumeClient clients.VolumeClient + _imageClient clients.ImageClient + _networkingService *networking.Service } // NewService returns an instance of the compute service. func NewService(scope *scope.Scope) (*Service, error) { - computeService, err := clients.NewComputeClient(scope) - if err != nil { - return nil, err - } - if scope.ProviderClientOpts.AuthInfo == nil { return nil, fmt.Errorf("authInfo must be set") } - networkingService, err := networking.NewService(scope) - if err != nil { - return nil, fmt.Errorf("failed to create networking service: %v", err) - } - return &Service{ - scope: scope, - computeService: computeService, - networkingService: networkingService, + scope: scope, }, nil } + +func (s Service) getComputeClient() clients.ComputeClient { + if s._computeClient == nil { + computeClient, err := clients.NewComputeClient(s.scope) + if err != nil { + return clients.NewComputeErrorClient(err) + } + + s._computeClient = computeClient + } + + return s._computeClient +} + +func (s Service) getVolumeClient() clients.VolumeClient { + if s._volumeClient == nil { + volumeClient, err := clients.NewVolumeClient(s.scope) + if err != nil { + return clients.NewVolumeErrorClient(err) + } + + s._volumeClient = volumeClient + } + + return s._volumeClient +} + +func (s Service) getImageClient() clients.ImageClient { + if s._imageClient == nil { + imageClient, err := clients.NewImageClient(s.scope) + if err != nil { + return clients.NewImageErrorClient(err) + } + + s._imageClient = imageClient + } + + return s._imageClient +} + +func (s Service) getNetworkingService() (*networking.Service, error) { + if s._networkingService == nil { + networkingService, err := networking.NewService(s.scope) + if err != nil { + return nil, fmt.Errorf("failed to create networking service: %v", err) + } + + s._networkingService = networkingService + } + + return s._networkingService, nil +}