diff --git a/cloud/services/disks/client.go b/cloud/services/disks/client.go index b34184a4676..aeddc602d2a 100644 --- a/cloud/services/disks/client.go +++ b/cloud/services/disks/client.go @@ -25,21 +25,21 @@ import ( ) // Client wraps go-sdk -type Client interface { +type client interface { Delete(context.Context, string, string) error } // AzureClient contains the Azure go-sdk Client -type AzureClient struct { +type azureClient struct { disks compute.DisksClient } -var _ Client = &AzureClient{} +var _ client = (*azureClient)(nil) -// NewClient creates a new VM client from subscription ID. -func NewClient(auth azure.Authorizer) *AzureClient { +// newClient creates a new VM client from subscription ID. +func newClient(auth azure.Authorizer) *azureClient { c := newDisksClient(auth.SubscriptionID(), auth.BaseURI(), auth.Authorizer()) - return &AzureClient{c} + return &azureClient{c} } // newDisksClient creates a new disks client from subscription ID. @@ -51,7 +51,7 @@ func newDisksClient(subscriptionID string, baseURI string, authorizer autorest.A } // Delete removes the disk client -func (ac *AzureClient) Delete(ctx context.Context, resourceGroupName, name string) error { +func (ac *azureClient) Delete(ctx context.Context, resourceGroupName, name string) error { future, err := ac.disks.Delete(ctx, resourceGroupName, name) if err != nil { return err diff --git a/cloud/services/disks/disks.go b/cloud/services/disks/disks.go index cbef9be27be..7182978af2b 100644 --- a/cloud/services/disks/disks.go +++ b/cloud/services/disks/disks.go @@ -19,11 +19,33 @@ package disks import ( "context" - azure "sigs.k8s.io/cluster-api-provider-azure/cloud" - + "github.com/go-logr/logr" "github.com/pkg/errors" + + 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 DiskScope + client +} + +// New creates a new disks service. +func New(scope DiskScope) *Service { + return &Service{ + Scope: scope, + client: newClient(scope), + } +} + // 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) error { return nil @@ -33,7 +55,7 @@ func (s *Service) Reconcile(ctx context.Context) error { 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) + err := s.client.Delete(ctx, s.Scope.ResourceGroup(), diskSpec.Name) if err != nil && azure.ResourceNotFound(err) { // already deleted continue diff --git a/cloud/services/disks/disks_test.go b/cloud/services/disks/disks_test.go index d5b10043467..1f72d5cb5c9 100644 --- a/cloud/services/disks/disks_test.go +++ b/cloud/services/disks/disks_test.go @@ -41,12 +41,12 @@ func TestDeleteDisk(t *testing.T) { testcases := []struct { name string expectedError string - expect func(s *mock_disks.MockDiskScopeMockRecorder, m *mock_disks.MockClientMockRecorder) + expect func(s *mock_disks.MockDiskScopeMockRecorder, m *mock_disks.MockclientMockRecorder) }{ { name: "delete the disk", expectedError: "", - expect: func(s *mock_disks.MockDiskScopeMockRecorder, m *mock_disks.MockClientMockRecorder) { + expect: func(s *mock_disks.MockDiskScopeMockRecorder, m *mock_disks.MockclientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) s.DiskSpecs().Return([]azure.DiskSpec{ { @@ -64,7 +64,7 @@ func TestDeleteDisk(t *testing.T) { { name: "disk already deleted", expectedError: "", - expect: func(s *mock_disks.MockDiskScopeMockRecorder, m *mock_disks.MockClientMockRecorder) { + expect: func(s *mock_disks.MockDiskScopeMockRecorder, m *mock_disks.MockclientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) s.DiskSpecs().Return([]azure.DiskSpec{ { @@ -82,7 +82,7 @@ func TestDeleteDisk(t *testing.T) { { 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) { + expect: func(s *mock_disks.MockDiskScopeMockRecorder, m *mock_disks.MockclientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) s.DiskSpecs().Return([]azure.DiskSpec{ { @@ -107,13 +107,13 @@ func TestDeleteDisk(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() scopeMock := mock_disks.NewMockDiskScope(mockCtrl) - clientMock := mock_disks.NewMockClient(mockCtrl) + clientMock := mock_disks.NewMockclient(mockCtrl) tc.expect(scopeMock.EXPECT(), clientMock.EXPECT()) s := &Service{ Scope: scopeMock, - Client: clientMock, + client: clientMock, } err := s.Delete(context.TODO()) @@ -129,23 +129,13 @@ func TestDeleteDisk(t *testing.T) { func TestDiskSpecs(t *testing.T) { testcases := []struct { - name string - azureMachine *infrav1.AzureMachine - expectedDisks []azure.DiskSpec + name string + azureMachineModifyFunc func(*infrav1.AzureMachine) + expectedDisks []azure.DiskSpec }{ { - name: "only os disk", - azureMachine: &infrav1.AzureMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-azure-machine", - }, - Spec: infrav1.AzureMachineSpec{ - OSDisk: infrav1.OSDisk{ - DiskSizeGB: 30, - OSType: "Linux", - }, - }, - }, + name: "only os disk", + azureMachineModifyFunc: func(m *infrav1.AzureMachine) {}, expectedDisks: []azure.DiskSpec{ { Name: "my-azure-machine_OSDisk", @@ -153,19 +143,10 @@ func TestDiskSpecs(t *testing.T) { }, }, { name: "os and data disks", - azureMachine: &infrav1.AzureMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-azure-machine", - }, - Spec: infrav1.AzureMachineSpec{ - OSDisk: infrav1.OSDisk{ - DiskSizeGB: 30, - OSType: "Linux", - }, - DataDisks: []infrav1.DataDisk{{ - NameSuffix: "etcddisk", - }}, - }, + azureMachineModifyFunc: func(m *infrav1.AzureMachine) { + m.Spec.DataDisks = []infrav1.DataDisk{{ + NameSuffix: "etcddisk", + }} }, expectedDisks: []azure.DiskSpec{ { @@ -177,23 +158,14 @@ func TestDiskSpecs(t *testing.T) { }, }, { name: "os and multiple data disks", - azureMachine: &infrav1.AzureMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-azure-machine", - }, - Spec: infrav1.AzureMachineSpec{ - OSDisk: infrav1.OSDisk{ - DiskSizeGB: 30, - OSType: "Linux", + azureMachineModifyFunc: func(m *infrav1.AzureMachine) { + m.Spec.DataDisks = []infrav1.DataDisk{ + { + NameSuffix: "etcddisk", }, - DataDisks: []infrav1.DataDisk{ - { - NameSuffix: "etcddisk", - }, - { - NameSuffix: "otherdisk", - }}, - }, + { + NameSuffix: "otherdisk", + }} }, expectedDisks: []azure.DiskSpec{ { @@ -231,11 +203,25 @@ func TestDiskSpecs(t *testing.T) { Name: "my-machine", }, } + + azureMachine := &infrav1.AzureMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-azure-machine", + }, + Spec: infrav1.AzureMachineSpec{ + OSDisk: infrav1.OSDisk{ + DiskSizeGB: 30, + OSType: "Linux", + }, + }, + } + tc.azureMachineModifyFunc(azureMachine) + initObjects := []runtime.Object{ cluster, machine, azureCluster, - tc.azureMachine, + azureMachine, } client := fake.NewFakeClientWithScheme(scheme, initObjects...) clusterScope, err := scope.NewClusterScope(scope.ClusterScopeParams{ @@ -251,7 +237,7 @@ func TestDiskSpecs(t *testing.T) { Client: client, ClusterScope: clusterScope, Machine: machine, - AzureMachine: tc.azureMachine, + AzureMachine: azureMachine, }) g.Expect(err).NotTo(HaveOccurred()) diff --git a/cloud/services/disks/mock_disks/client_mock.go b/cloud/services/disks/mock_disks/client_mock.go index 5b6c64b44d3..6ed29506b45 100644 --- a/cloud/services/disks/mock_disks/client_mock.go +++ b/cloud/services/disks/mock_disks/client_mock.go @@ -26,31 +26,31 @@ import ( reflect "reflect" ) -// MockClient is a mock of Client interface. -type MockClient struct { +// Mockclient is a mock of client interface. +type Mockclient struct { ctrl *gomock.Controller - recorder *MockClientMockRecorder + recorder *MockclientMockRecorder } -// MockClientMockRecorder is the mock recorder for MockClient. -type MockClientMockRecorder struct { - mock *MockClient +// 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} +// 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 { +func (m *Mockclient) EXPECT() *MockclientMockRecorder { return m.recorder } // Delete mocks base method. -func (m *MockClient) Delete(arg0 context.Context, arg1, arg2 string) error { +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) @@ -58,7 +58,7 @@ func (m *MockClient) Delete(arg0 context.Context, arg1, arg2 string) error { } // Delete indicates an expected call of Delete. -func (mr *MockClientMockRecorder) Delete(arg0, arg1, arg2 interface{}) *gomock.Call { +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) + 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 68da53775e9..fd5f6434b19 100644 --- a/cloud/services/disks/mock_disks/disks_mock.go +++ b/cloud/services/disks/mock_disks/disks_mock.go @@ -15,7 +15,7 @@ limitations under the License. */ // Code generated by MockGen. DO NOT EDIT. -// Source: ../service.go +// Source: ../disks.go // Package mock_disks is a generated GoMock package. package mock_disks diff --git a/cloud/services/disks/mock_disks/doc.go b/cloud/services/disks/mock_disks/doc.go index 5197df51372..ba875a622ce 100644 --- a/cloud/services/disks/mock_disks/doc.go +++ b/cloud/services/disks/mock_disks/doc.go @@ -16,7 +16,7 @@ limitations under the License. // Run go generate to regenerate this mock. //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 ../../../../hack/tools/bin/mockgen -destination disks_mock.go -package mock_disks -source ../disks.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 deleted file mode 100644 index ff8594aac80..00000000000 --- a/cloud/services/disks/service.go +++ /dev/null @@ -1,43 +0,0 @@ -/* -Copyright 2019 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 disks - -import ( - "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 DiskScope - Client -} - -// NewService creates a new service. -func NewService(scope DiskScope) *Service { - return &Service{ - Scope: scope, - Client: NewClient(scope), - } -} diff --git a/controllers/azuremachine_reconciler.go b/controllers/azuremachine_reconciler.go index 866f765e3cb..13221f52f28 100644 --- a/controllers/azuremachine_reconciler.go +++ b/controllers/azuremachine_reconciler.go @@ -54,7 +54,7 @@ func newAzureMachineService(machineScope *scope.MachineScope, clusterScope *scop networkInterfacesSvc: networkinterfaces.NewService(machineScope, cache), virtualMachinesSvc: virtualmachines.NewService(machineScope, cache), roleAssignmentsSvc: roleassignments.NewService(machineScope), - disksSvc: disks.NewService(machineScope), + disksSvc: disks.New(machineScope), publicIPsSvc: publicips.NewService(machineScope), tagsSvc: tags.NewService(machineScope), skuCache: cache,