From e06c352bdce89d3c7ceeb3723e4b4c27bc7e2d0a Mon Sep 17 00:00:00 2001 From: Carlos Panato Date: Tue, 7 Jul 2020 16:54:35 +0200 Subject: [PATCH] cloud: Refactor Disks scope to interface --- cloud/interfaces.go | 1 + cloud/scope/machine.go | 9 + cloud/services/disks/disks.go | 39 +-- cloud/services/disks/disks_test.go | 153 +++------- .../services/disks/mock_disks/client_mock.go | 64 ++++ cloud/services/disks/mock_disks/disks_mock.go | 277 ++++++++++++++++-- cloud/services/disks/mock_disks/doc.go | 4 +- cloud/services/disks/service.go | 14 +- cloud/types.go | 5 + controllers/azuremachine_reconciler.go | 15 +- 10 files changed, 418 insertions(+), 163 deletions(-) create mode 100644 cloud/services/disks/mock_disks/client_mock.go diff --git a/cloud/interfaces.go b/cloud/interfaces.go index 72a9ad7db7c..31bbb56c050 100644 --- a/cloud/interfaces.go +++ b/cloud/interfaces.go @@ -18,6 +18,7 @@ package azure import ( "context" + "github.com/Azure/go-autorest/autorest" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha3" ) diff --git a/cloud/scope/machine.go b/cloud/scope/machine.go index c1ee3743606..d97c4e67035 100644 --- a/cloud/scope/machine.go +++ b/cloud/scope/machine.go @@ -19,6 +19,7 @@ package scope import ( "context" "encoding/base64" + "github.com/Azure/go-autorest/autorest" "github.com/go-logr/logr" "github.com/pkg/errors" @@ -124,6 +125,14 @@ func (m *MachineScope) NICSpecs() []azure.NICSpec { return []azure.NICSpec{spec} } +// DiskSpecs returns the public IP specs. +func (m *MachineScope) DiskSpecs() []azure.DiskSpec { + spec := azure.DiskSpec{ + Name: azure.GenerateOSDiskName(m.Name()), + } + return []azure.DiskSpec{spec} +} + // Location returns the AzureCluster location. func (m *MachineScope) Location() string { return m.ClusterScope.Location() diff --git a/cloud/services/disks/disks.go b/cloud/services/disks/disks.go index 8240ef6870f..254fdbd137a 100644 --- a/cloud/services/disks/disks.go +++ b/cloud/services/disks/disks.go @@ -19,37 +19,30 @@ package disks import ( "context" - "github.com/pkg/errors" - "k8s.io/klog" azure "sigs.k8s.io/cluster-api-provider-azure/cloud" -) -// Spec specification for disk -type Spec struct { - Name string -} + "github.com/pkg/errors" +) // Reconcile on disk is currently no-op. OS disks should only be deleted and will create with the VM automatically. -func (s *Service) Reconcile(ctx context.Context, spec interface{}) error { +func (s *Service) Reconcile(ctx context.Context) error { return nil } // Delete deletes the disk associated with a VM. -func (s *Service) Delete(ctx context.Context, spec interface{}) error { - diskSpec, ok := spec.(*Spec) - if !ok { - return errors.New("invalid disk specification") - } - klog.V(2).Infof("deleting disk %s", diskSpec.Name) - err := s.Client.Delete(ctx, s.Scope.ResourceGroup(), diskSpec.Name) - if err != nil && azure.ResourceNotFound(err) { - // already deleted - return nil +func (s *Service) Delete(ctx context.Context) error { + for _, diskSpec := range s.Scope.DiskSpecs() { + s.Scope.V(2).Info("deleting disk", "disk", diskSpec.Name) + err := s.Client.Delete(ctx, s.Scope.ResourceGroup(), diskSpec.Name) + if err != nil && azure.ResourceNotFound(err) { + // already deleted + return nil + } + if err != nil { + return errors.Wrapf(err, "failed to delete disk %s in resource group %s", diskSpec.Name, s.Scope.ResourceGroup()) + } + + s.Scope.V(2).Info("successfully deleted disk", "disk", diskSpec.Name) } - if err != nil { - return errors.Wrapf(err, "failed to delete disk %s in resource group %s", diskSpec.Name, s.Scope.ResourceGroup()) - } - - klog.V(2).Infof("successfully deleted disk %s", diskSpec.Name) return nil } diff --git a/cloud/services/disks/disks_test.go b/cloud/services/disks/disks_test.go index 72db34c9e2e..5a85f369151 100644 --- a/cloud/services/disks/disks_test.go +++ b/cloud/services/disks/disks_test.go @@ -21,110 +21,72 @@ import ( "net/http" "testing" + "github.com/Azure/go-autorest/autorest" . "github.com/onsi/gomega" + azure "sigs.k8s.io/cluster-api-provider-azure/cloud" "sigs.k8s.io/cluster-api-provider-azure/cloud/services/disks/mock_disks" - "github.com/Azure/go-autorest/autorest" "github.com/golang/mock/gomock" - network "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-06-01/network" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" - infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha3" - "sigs.k8s.io/cluster-api-provider-azure/cloud/scope" + "k8s.io/klog/klogr" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" - "sigs.k8s.io/controller-runtime/pkg/client/fake" ) func init() { - clusterv1.AddToScheme(scheme.Scheme) -} - -const ( - subscriptionID = "123" -) - -func TestInvalidDiskSpec(t *testing.T) { - g := NewWithT(t) - - mockCtrl := gomock.NewController(t) - defer mockCtrl.Finish() - - disksMock := mock_disks.NewMockClient(mockCtrl) - - cluster := &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{Name: "test-cluster"}, - } - - client := fake.NewFakeClientWithScheme(scheme.Scheme, cluster) - - clusterScope, err := scope.NewClusterScope(scope.ClusterScopeParams{ - AzureClients: scope.AzureClients{ - Authorizer: autorest.NullAuthorizer{}, - }, - Client: client, - Cluster: cluster, - AzureCluster: &infrav1.AzureCluster{ - Spec: infrav1.AzureClusterSpec{ - Location: "test-location", - ResourceGroup: "my-rg", - SubscriptionID: subscriptionID, - NetworkSpec: infrav1.NetworkSpec{ - Vnet: infrav1.VnetSpec{Name: "my-vnet", ResourceGroup: "my-rg"}, - }, - }, - }, - }) - g.Expect(err).NotTo(HaveOccurred()) - - s := &Service{ - Scope: clusterScope, - Client: disksMock, - } - - // Wrong Spec - wrongSpec := &network.PublicIPAddress{} - - err = s.Delete(context.TODO(), &wrongSpec) - g.Expect(err).To(HaveOccurred()) - g.Expect(err).To(MatchError("invalid disk specification")) + _ = clusterv1.AddToScheme(scheme.Scheme) } func TestDeleteDisk(t *testing.T) { testcases := []struct { name string - disksSpec Spec expectedError string - expect func(m *mock_disks.MockClientMockRecorder) + expect func(s *mock_disks.MockDiskScopeMockRecorder, m *mock_disks.MockClientMockRecorder) }{ { - name: "delete the disk", - disksSpec: Spec{ - Name: "my-disk", - }, + name: "delete the disk", expectedError: "", - expect: func(m *mock_disks.MockClientMockRecorder) { - m.Delete(context.TODO(), "my-rg", "my-disk") + expect: func(s *mock_disks.MockDiskScopeMockRecorder, m *mock_disks.MockClientMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + s.DiskSpecs().Return([]azure.DiskSpec{ + { + Name: "my-disk-1", + }, + { + Name: "honk-disk", + }, + }) + s.ResourceGroup().AnyTimes().Return("my-rg") + m.Delete(context.TODO(), "my-rg", "my-disk-1") + m.Delete(context.TODO(), "my-rg", "honk-disk") }, }, { - name: "disk already deleted", - disksSpec: Spec{ - Name: "my-disk", - }, + name: "disk already deleted", expectedError: "", - expect: func(m *mock_disks.MockClientMockRecorder) { - m.Delete(context.TODO(), "my-rg", "my-disk").Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not Found")) + expect: func(s *mock_disks.MockDiskScopeMockRecorder, m *mock_disks.MockClientMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + s.DiskSpecs().Return([]azure.DiskSpec{ + { + Name: "my-disk-1", + }, + }) + s.ResourceGroup().AnyTimes().Return("my-rg") + m.Delete(context.TODO(), "my-rg", "my-disk-1").Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not Found")) }, }, { - name: "error while trying to delete the disk", - disksSpec: Spec{ - Name: "my-disk", - }, - expectedError: "failed to delete disk my-disk in resource group my-rg: #: Internal Server Error: StatusCode=500", - expect: func(m *mock_disks.MockClientMockRecorder) { - m.Delete(context.TODO(), "my-rg", "my-disk").Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) + name: "error while trying to delete the disk", + expectedError: "failed to delete disk my-disk-1 in resource group my-rg: #: Internal Server Error: StatusCode=500", + expect: func(s *mock_disks.MockDiskScopeMockRecorder, m *mock_disks.MockClientMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + s.DiskSpecs().Return([]azure.DiskSpec{ + { + Name: "my-disk-1", + }, + }) + s.ResourceGroup().AnyTimes().Return("my-rg") + m.Delete(context.TODO(), "my-rg", "my-disk-1").Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) }, }, } @@ -133,41 +95,20 @@ func TestDeleteDisk(t *testing.T) { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) + t.Parallel() mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() + scopeMock := mock_disks.NewMockDiskScope(mockCtrl) + clientMock := mock_disks.NewMockClient(mockCtrl) - disksMock := mock_disks.NewMockClient(mockCtrl) - - cluster := &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{Name: "test-cluster"}, - } - - client := fake.NewFakeClientWithScheme(scheme.Scheme, cluster) - - tc.expect(disksMock.EXPECT()) - - clusterScope, err := scope.NewClusterScope(scope.ClusterScopeParams{ - AzureClients: scope.AzureClients{ - Authorizer: autorest.NullAuthorizer{}, - }, - Client: client, - Cluster: cluster, - AzureCluster: &infrav1.AzureCluster{ - Spec: infrav1.AzureClusterSpec{ - Location: "test-location", - ResourceGroup: "my-rg", - SubscriptionID: subscriptionID, - }, - }, - }) - g.Expect(err).NotTo(HaveOccurred()) + tc.expect(scopeMock.EXPECT(), clientMock.EXPECT()) s := &Service{ - Scope: clusterScope, - Client: disksMock, + Scope: scopeMock, + Client: clientMock, } - err = s.Delete(context.TODO(), &tc.disksSpec) + err := s.Delete(context.TODO()) if tc.expectedError != "" { g.Expect(err).To(HaveOccurred()) g.Expect(err).To(MatchError(tc.expectedError)) diff --git a/cloud/services/disks/mock_disks/client_mock.go b/cloud/services/disks/mock_disks/client_mock.go new file mode 100644 index 00000000000..5b6c64b44d3 --- /dev/null +++ b/cloud/services/disks/mock_disks/client_mock.go @@ -0,0 +1,64 @@ +/* +Copyright 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: ../client.go + +// Package mock_disks is a generated GoMock package. +package mock_disks + +import ( + context "context" + gomock "github.com/golang/mock/gomock" + reflect "reflect" +) + +// MockClient is a mock of Client interface. +type MockClient struct { + ctrl *gomock.Controller + recorder *MockClientMockRecorder +} + +// MockClientMockRecorder is the mock recorder for MockClient. +type MockClientMockRecorder struct { + mock *MockClient +} + +// NewMockClient creates a new mock instance. +func NewMockClient(ctrl *gomock.Controller) *MockClient { + mock := &MockClient{ctrl: ctrl} + mock.recorder = &MockClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockClient) EXPECT() *MockClientMockRecorder { + return m.recorder +} + +// Delete mocks base method. +func (m *MockClient) Delete(arg0 context.Context, arg1, arg2 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Delete", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// Delete indicates an expected call of Delete. +func (mr *MockClientMockRecorder) Delete(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockClient)(nil).Delete), arg0, arg1, arg2) +} diff --git a/cloud/services/disks/mock_disks/disks_mock.go b/cloud/services/disks/mock_disks/disks_mock.go index 5b6c64b44d3..7d9d1b09d84 100644 --- a/cloud/services/disks/mock_disks/disks_mock.go +++ b/cloud/services/disks/mock_disks/disks_mock.go @@ -15,50 +15,287 @@ limitations under the License. */ // Code generated by MockGen. DO NOT EDIT. -// Source: ../client.go +// Source: ../service.go // Package mock_disks is a generated GoMock package. package mock_disks import ( - context "context" + autorest "github.com/Azure/go-autorest/autorest" + logr "github.com/go-logr/logr" gomock "github.com/golang/mock/gomock" reflect "reflect" + v1alpha3 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha3" + azure "sigs.k8s.io/cluster-api-provider-azure/cloud" ) -// MockClient is a mock of Client interface. -type MockClient struct { +// MockDiskScope is a mock of DiskScope interface. +type MockDiskScope struct { ctrl *gomock.Controller - recorder *MockClientMockRecorder + recorder *MockDiskScopeMockRecorder } -// MockClientMockRecorder is the mock recorder for MockClient. -type MockClientMockRecorder struct { - mock *MockClient +// MockDiskScopeMockRecorder is the mock recorder for MockDiskScope. +type MockDiskScopeMockRecorder struct { + mock *MockDiskScope } -// NewMockClient creates a new mock instance. -func NewMockClient(ctrl *gomock.Controller) *MockClient { - mock := &MockClient{ctrl: ctrl} - mock.recorder = &MockClientMockRecorder{mock} +// NewMockDiskScope creates a new mock instance. +func NewMockDiskScope(ctrl *gomock.Controller) *MockDiskScope { + mock := &MockDiskScope{ctrl: ctrl} + mock.recorder = &MockDiskScopeMockRecorder{mock} return mock } // EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockClient) EXPECT() *MockClientMockRecorder { +func (m *MockDiskScope) EXPECT() *MockDiskScopeMockRecorder { return m.recorder } -// Delete mocks base method. -func (m *MockClient) Delete(arg0 context.Context, arg1, arg2 string) error { +// Info mocks base method. +func (m *MockDiskScope) Info(msg string, keysAndValues ...interface{}) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Delete", arg0, arg1, arg2) - ret0, _ := ret[0].(error) + varargs := []interface{}{msg} + for _, a := range keysAndValues { + varargs = append(varargs, a) + } + m.ctrl.Call(m, "Info", varargs...) +} + +// Info indicates an expected call of Info. +func (mr *MockDiskScopeMockRecorder) Info(msg interface{}, keysAndValues ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{msg}, keysAndValues...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Info", reflect.TypeOf((*MockDiskScope)(nil).Info), varargs...) +} + +// Enabled mocks base method. +func (m *MockDiskScope) Enabled() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Enabled") + ret0, _ := ret[0].(bool) + return ret0 +} + +// Enabled indicates an expected call of Enabled. +func (mr *MockDiskScopeMockRecorder) Enabled() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Enabled", reflect.TypeOf((*MockDiskScope)(nil).Enabled)) +} + +// Error mocks base method. +func (m *MockDiskScope) Error(err error, msg string, keysAndValues ...interface{}) { + m.ctrl.T.Helper() + varargs := []interface{}{err, msg} + for _, a := range keysAndValues { + varargs = append(varargs, a) + } + m.ctrl.Call(m, "Error", varargs...) +} + +// Error indicates an expected call of Error. +func (mr *MockDiskScopeMockRecorder) Error(err, msg interface{}, keysAndValues ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{err, msg}, keysAndValues...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Error", reflect.TypeOf((*MockDiskScope)(nil).Error), varargs...) +} + +// V mocks base method. +func (m *MockDiskScope) V(level int) logr.InfoLogger { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "V", level) + ret0, _ := ret[0].(logr.InfoLogger) + return ret0 +} + +// V indicates an expected call of V. +func (mr *MockDiskScopeMockRecorder) V(level interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "V", reflect.TypeOf((*MockDiskScope)(nil).V), level) +} + +// WithValues mocks base method. +func (m *MockDiskScope) WithValues(keysAndValues ...interface{}) logr.Logger { + m.ctrl.T.Helper() + varargs := []interface{}{} + for _, a := range keysAndValues { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "WithValues", varargs...) + ret0, _ := ret[0].(logr.Logger) + return ret0 +} + +// WithValues indicates an expected call of WithValues. +func (mr *MockDiskScopeMockRecorder) WithValues(keysAndValues ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WithValues", reflect.TypeOf((*MockDiskScope)(nil).WithValues), keysAndValues...) +} + +// WithName mocks base method. +func (m *MockDiskScope) WithName(name string) logr.Logger { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "WithName", name) + ret0, _ := ret[0].(logr.Logger) + return ret0 +} + +// WithName indicates an expected call of WithName. +func (mr *MockDiskScopeMockRecorder) WithName(name interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WithName", reflect.TypeOf((*MockDiskScope)(nil).WithName), name) +} + +// SubscriptionID mocks base method. +func (m *MockDiskScope) SubscriptionID() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SubscriptionID") + ret0, _ := ret[0].(string) + return ret0 +} + +// SubscriptionID indicates an expected call of SubscriptionID. +func (mr *MockDiskScopeMockRecorder) SubscriptionID() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SubscriptionID", reflect.TypeOf((*MockDiskScope)(nil).SubscriptionID)) +} + +// BaseURI mocks base method. +func (m *MockDiskScope) BaseURI() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "BaseURI") + ret0, _ := ret[0].(string) + return ret0 +} + +// BaseURI indicates an expected call of BaseURI. +func (mr *MockDiskScopeMockRecorder) BaseURI() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BaseURI", reflect.TypeOf((*MockDiskScope)(nil).BaseURI)) +} + +// Authorizer mocks base method. +func (m *MockDiskScope) Authorizer() autorest.Authorizer { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Authorizer") + ret0, _ := ret[0].(autorest.Authorizer) + return ret0 +} + +// Authorizer indicates an expected call of Authorizer. +func (mr *MockDiskScopeMockRecorder) Authorizer() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Authorizer", reflect.TypeOf((*MockDiskScope)(nil).Authorizer)) +} + +// ResourceGroup mocks base method. +func (m *MockDiskScope) ResourceGroup() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ResourceGroup") + ret0, _ := ret[0].(string) + return ret0 +} + +// ResourceGroup indicates an expected call of ResourceGroup. +func (mr *MockDiskScopeMockRecorder) ResourceGroup() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResourceGroup", reflect.TypeOf((*MockDiskScope)(nil).ResourceGroup)) +} + +// ClusterName mocks base method. +func (m *MockDiskScope) ClusterName() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ClusterName") + ret0, _ := ret[0].(string) + return ret0 +} + +// ClusterName indicates an expected call of ClusterName. +func (mr *MockDiskScopeMockRecorder) ClusterName() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClusterName", reflect.TypeOf((*MockDiskScope)(nil).ClusterName)) +} + +// Location mocks base method. +func (m *MockDiskScope) Location() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Location") + ret0, _ := ret[0].(string) + return ret0 +} + +// Location indicates an expected call of Location. +func (mr *MockDiskScopeMockRecorder) Location() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Location", reflect.TypeOf((*MockDiskScope)(nil).Location)) +} + +// AdditionalTags mocks base method. +func (m *MockDiskScope) AdditionalTags() v1alpha3.Tags { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AdditionalTags") + ret0, _ := ret[0].(v1alpha3.Tags) + return ret0 +} + +// AdditionalTags indicates an expected call of AdditionalTags. +func (mr *MockDiskScopeMockRecorder) AdditionalTags() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AdditionalTags", reflect.TypeOf((*MockDiskScope)(nil).AdditionalTags)) +} + +// Vnet mocks base method. +func (m *MockDiskScope) Vnet() *v1alpha3.VnetSpec { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Vnet") + ret0, _ := ret[0].(*v1alpha3.VnetSpec) + return ret0 +} + +// Vnet indicates an expected call of Vnet. +func (mr *MockDiskScopeMockRecorder) Vnet() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Vnet", reflect.TypeOf((*MockDiskScope)(nil).Vnet)) +} + +// NodeSubnet mocks base method. +func (m *MockDiskScope) NodeSubnet() *v1alpha3.SubnetSpec { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "NodeSubnet") + ret0, _ := ret[0].(*v1alpha3.SubnetSpec) + return ret0 +} + +// NodeSubnet indicates an expected call of NodeSubnet. +func (mr *MockDiskScopeMockRecorder) NodeSubnet() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NodeSubnet", reflect.TypeOf((*MockDiskScope)(nil).NodeSubnet)) +} + +// ControlPlaneSubnet mocks base method. +func (m *MockDiskScope) ControlPlaneSubnet() *v1alpha3.SubnetSpec { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ControlPlaneSubnet") + ret0, _ := ret[0].(*v1alpha3.SubnetSpec) + return ret0 +} + +// ControlPlaneSubnet indicates an expected call of ControlPlaneSubnet. +func (mr *MockDiskScopeMockRecorder) ControlPlaneSubnet() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ControlPlaneSubnet", reflect.TypeOf((*MockDiskScope)(nil).ControlPlaneSubnet)) +} + +// DiskSpecs mocks base method. +func (m *MockDiskScope) DiskSpecs() []azure.DiskSpec { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DiskSpecs") + ret0, _ := ret[0].([]azure.DiskSpec) return ret0 } -// Delete indicates an expected call of Delete. -func (mr *MockClientMockRecorder) Delete(arg0, arg1, arg2 interface{}) *gomock.Call { +// DiskSpecs indicates an expected call of DiskSpecs. +func (mr *MockDiskScopeMockRecorder) DiskSpecs() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockClient)(nil).Delete), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DiskSpecs", reflect.TypeOf((*MockDiskScope)(nil).DiskSpecs)) } diff --git a/cloud/services/disks/mock_disks/doc.go b/cloud/services/disks/mock_disks/doc.go index 06ed720865b..5197df51372 100644 --- a/cloud/services/disks/mock_disks/doc.go +++ b/cloud/services/disks/mock_disks/doc.go @@ -15,6 +15,8 @@ limitations under the License. */ // Run go generate to regenerate this mock. -//go:generate ../../../../hack/tools/bin/mockgen -destination disks_mock.go -package mock_disks -source ../client.go Client +//go:generate ../../../../hack/tools/bin/mockgen -destination client_mock.go -package mock_disks -source ../client.go Client +//go:generate ../../../../hack/tools/bin/mockgen -destination disks_mock.go -package mock_disks -source ../service.go DiskScope +//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" //go:generate /usr/bin/env bash -c "cat ../../../../hack/boilerplate/boilerplate.generatego.txt disks_mock.go > _disks_mock.go && mv _disks_mock.go disks_mock.go" package mock_disks //nolint diff --git a/cloud/services/disks/service.go b/cloud/services/disks/service.go index e3114864528..ff8594aac80 100644 --- a/cloud/services/disks/service.go +++ b/cloud/services/disks/service.go @@ -17,17 +17,25 @@ limitations under the License. package disks import ( - "sigs.k8s.io/cluster-api-provider-azure/cloud/scope" + "github.com/go-logr/logr" + azure "sigs.k8s.io/cluster-api-provider-azure/cloud" ) +// DiskScope defines the scope interface for a disk service. +type DiskScope interface { + logr.Logger + azure.ClusterDescriber + DiskSpecs() []azure.DiskSpec +} + // Service provides operations on azure resources type Service struct { - Scope *scope.ClusterScope + Scope DiskScope Client } // NewService creates a new service. -func NewService(scope *scope.ClusterScope) *Service { +func NewService(scope DiskScope) *Service { return &Service{ Scope: scope, Client: NewClient(scope), diff --git a/cloud/types.go b/cloud/types.go index 33ba09f2287..b9428ca8342 100644 --- a/cloud/types.go +++ b/cloud/types.go @@ -37,3 +37,8 @@ type NICSpec struct { VMSize string AcceleratedNetworking *bool } + +// DiskSpec defines the specification for a Disk. +type DiskSpec struct { + Name string +} diff --git a/controllers/azuremachine_reconciler.go b/controllers/azuremachine_reconciler.go index d101af730d6..dfdf6230ddf 100644 --- a/controllers/azuremachine_reconciler.go +++ b/controllers/azuremachine_reconciler.go @@ -42,7 +42,7 @@ type azureMachineService struct { availabilityZonesSvc azure.GetterService networkInterfacesSvc azure.Service virtualMachinesSvc *virtualmachines.Service - disksSvc azure.OldService + disksSvc azure.Service publicIPsSvc azure.Service } @@ -54,7 +54,7 @@ func newAzureMachineService(machineScope *scope.MachineScope, clusterScope *scop availabilityZonesSvc: availabilityzones.NewService(clusterScope), networkInterfacesSvc: networkinterfaces.NewService(machineScope), virtualMachinesSvc: virtualmachines.NewService(clusterScope, machineScope), - disksSvc: disks.NewService(clusterScope), + disksSvc: disks.NewService(machineScope), publicIPsSvc: publicips.NewService(machineScope), } } @@ -100,10 +100,7 @@ func (s *azureMachineService) Delete(ctx context.Context) error { return errors.Wrap(err, "failed to delete public IPs") } - OSDiskSpec := &disks.Spec{ - Name: azure.GenerateOSDiskName(s.machineScope.Name()), - } - err = s.disksSvc.Delete(ctx, OSDiskSpec) + err = s.disksSvc.Delete(ctx) if err != nil { return errors.Wrapf(err, "Failed to delete OS disk of machine %s", s.machineScope.Name()) } @@ -252,10 +249,8 @@ func (s *azureMachineService) reconcileVirtualMachine(ctx context.Context, nicNa if err != nil { return nil, errors.Wrapf(err, "failed to delete machine") } - OSDiskSpec := &disks.Spec{ - Name: azure.GenerateOSDiskName(s.machineScope.Name()), - } - err = s.disksSvc.Delete(ctx, OSDiskSpec) + + err = s.disksSvc.Delete(ctx) if err != nil && !azure.ResourceNotFound(err) { return nil, errors.Wrapf(err, "failed to delete OS disk of machine %s", s.machineScope.Name()) }