diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go index f514e0d88d6..49f9ab08a31 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -40,6 +40,7 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/azure/services/loadbalancers" "sigs.k8s.io/cluster-api-provider-azure/azure/services/natgateways" "sigs.k8s.io/cluster-api-provider-azure/azure/services/routetables" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualnetworks" "sigs.k8s.io/cluster-api-provider-azure/azure/services/vnetpeerings" "sigs.k8s.io/cluster-api-provider-azure/util/futures" "sigs.k8s.io/cluster-api-provider-azure/util/tele" @@ -362,11 +363,14 @@ func (s *ClusterScope) VnetPeeringSpecs() []azure.ResourceSpecGetter { } // VNetSpec returns the virtual network spec. -func (s *ClusterScope) VNetSpec() azure.VNetSpec { - return azure.VNetSpec{ - ResourceGroup: s.Vnet().ResourceGroup, - Name: s.Vnet().Name, - CIDRs: s.Vnet().CIDRBlocks, +func (s *ClusterScope) VNetSpec() azure.ResourceSpecGetter { + return &virtualnetworks.VNetSpec{ + ResourceGroup: s.Vnet().ResourceGroup, + Name: s.Vnet().Name, + CIDRs: s.Vnet().CIDRBlocks, + Location: s.Location(), + ClusterName: s.ClusterName(), + AdditionalTags: s.AdditionalTags(), } } @@ -662,6 +666,7 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error { infrav1.NATGatewaysReadyCondition, infrav1.LoadBalancersReadyCondition, infrav1.BastionHostReadyCondition, + infrav1.VNetReadyCondition, ), ) @@ -678,6 +683,7 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error { infrav1.NATGatewaysReadyCondition, infrav1.LoadBalancersReadyCondition, infrav1.BastionHostReadyCondition, + infrav1.VNetReadyCondition, }}) } diff --git a/azure/scope/managedcontrolplane.go b/azure/scope/managedcontrolplane.go index 373eadf2d80..699ef8a4259 100644 --- a/azure/scope/managedcontrolplane.go +++ b/azure/scope/managedcontrolplane.go @@ -41,6 +41,7 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" "sigs.k8s.io/cluster-api-provider-azure/azure/services/groups" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualnetworks" infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/util/futures" "sigs.k8s.io/cluster-api-provider-azure/util/tele" @@ -226,11 +227,14 @@ func (s *ManagedControlPlaneScope) GroupSpec() azure.ResourceSpecGetter { } // VNetSpec returns the virtual network spec. -func (s *ManagedControlPlaneScope) VNetSpec() azure.VNetSpec { - return azure.VNetSpec{ - ResourceGroup: s.Vnet().ResourceGroup, - Name: s.Vnet().Name, - CIDRs: s.Vnet().CIDRBlocks, +func (s *ManagedControlPlaneScope) VNetSpec() azure.ResourceSpecGetter { + return &virtualnetworks.VNetSpec{ + ResourceGroup: s.Vnet().ResourceGroup, + Name: s.Vnet().Name, + CIDRs: s.Vnet().CIDRBlocks, + Location: s.Location(), + ClusterName: s.ClusterName(), + AdditionalTags: s.AdditionalTags(), } } diff --git a/azure/services/async/interfaces.go b/azure/services/async/interfaces.go index 2798ac44479..2970b469d3d 100644 --- a/azure/services/async/interfaces.go +++ b/azure/services/async/interfaces.go @@ -36,11 +36,16 @@ type FutureHandler interface { Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error) } +// Getter is an interface that can get a resource. +type Getter interface { + Get(ctx context.Context, spec azure.ResourceSpecGetter) (result interface{}, err error) +} + // Creator is a client that can create or update a resource asynchronously. type Creator interface { FutureHandler + Getter CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (result interface{}, future azureautorest.FutureAPI, err error) - Get(ctx context.Context, spec azure.ResourceSpecGetter) (result interface{}, err error) } // Deleter is a client that can delete a resource asynchronously. diff --git a/azure/services/async/mock_async/async_mock.go b/azure/services/async/mock_async/async_mock.go index fa98744eaab..13e5a33f5c3 100644 --- a/azure/services/async/mock_async/async_mock.go +++ b/azure/services/async/mock_async/async_mock.go @@ -181,6 +181,44 @@ func (mr *MockFutureHandlerMockRecorder) Result(ctx, future, futureType interfac return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Result", reflect.TypeOf((*MockFutureHandler)(nil).Result), ctx, future, futureType) } +// MockGetter is a mock of Getter interface. +type MockGetter struct { + ctrl *gomock.Controller + recorder *MockGetterMockRecorder +} + +// MockGetterMockRecorder is the mock recorder for MockGetter. +type MockGetterMockRecorder struct { + mock *MockGetter +} + +// NewMockGetter creates a new mock instance. +func NewMockGetter(ctrl *gomock.Controller) *MockGetter { + mock := &MockGetter{ctrl: ctrl} + mock.recorder = &MockGetterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockGetter) EXPECT() *MockGetterMockRecorder { + return m.recorder +} + +// Get mocks base method. +func (m *MockGetter) Get(ctx context.Context, spec azure0.ResourceSpecGetter) (interface{}, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Get", ctx, spec) + ret0, _ := ret[0].(interface{}) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Get indicates an expected call of Get. +func (mr *MockGetterMockRecorder) Get(ctx, spec interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockGetter)(nil).Get), ctx, spec) +} + // MockCreator is a mock of Creator interface. type MockCreator struct { ctrl *gomock.Controller diff --git a/azure/services/availabilitysets/availabilitysets.go b/azure/services/availabilitysets/availabilitysets.go index 4d96cb16316..dccde1b9b87 100644 --- a/azure/services/availabilitysets/availabilitysets.go +++ b/azure/services/availabilitysets/availabilitysets.go @@ -41,7 +41,7 @@ type AvailabilitySetScope interface { // Service provides operations on Azure resources. type Service struct { Scope AvailabilitySetScope - Client + async.Getter async.Reconciler resourceSKUCache *resourceskus.Cache } @@ -51,7 +51,7 @@ func New(scope AvailabilitySetScope, skuCache *resourceskus.Cache) *Service { client := NewClient(scope) return &Service{ Scope: scope, - Client: client, + Getter: client, resourceSKUCache: skuCache, Reconciler: async.New(scope, client, client), } @@ -88,7 +88,7 @@ func (s *Service) Delete(ctx context.Context) error { if setSpec := s.Scope.AvailabilitySetSpec(); setSpec == nil { log.V(2).Info("skip deletion when no availability set spec is found") } else { - existingSet, err := s.Client.Get(ctx, setSpec) + existingSet, err := s.Get(ctx, setSpec) if err != nil { if !azure.ResourceNotFound(err) { resultingErr = errors.Wrapf(err, "failed to get availability set %s in resource group %s", setSpec.ResourceName(), setSpec.ResourceGroupName()) diff --git a/azure/services/availabilitysets/availabilitysets_test.go b/azure/services/availabilitysets/availabilitysets_test.go index 7e87167e161..a9248495a96 100644 --- a/azure/services/availabilitysets/availabilitysets_test.go +++ b/azure/services/availabilitysets/availabilitysets_test.go @@ -149,12 +149,12 @@ func TestDeleteAvailabilitySets(t *testing.T) { testcases := []struct { name string expectedError string - expect func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) + expect func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_async.MockGetterMockRecorder, r *mock_async.MockReconcilerMockRecorder) }{ { name: "deletes availability set", expectedError: "", - expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_async.MockGetterMockRecorder, r *mock_async.MockReconcilerMockRecorder) { s.AvailabilitySetSpec().Return(&fakeSetSpec) gomock.InOrder( m.Get(gomockinternal.AContext(), &fakeSetSpec).Return(compute.AvailabilitySet{}, nil), @@ -166,7 +166,7 @@ func TestDeleteAvailabilitySets(t *testing.T) { { name: "noop if AvailabilitySetSpec returns nil", expectedError: "", - expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_async.MockGetterMockRecorder, r *mock_async.MockReconcilerMockRecorder) { s.AvailabilitySetSpec().Return(nil) s.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, nil) }, @@ -174,7 +174,7 @@ func TestDeleteAvailabilitySets(t *testing.T) { { name: "delete proceeds with missing required value in availability set spec", expectedError: "", - expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_async.MockGetterMockRecorder, r *mock_async.MockReconcilerMockRecorder) { s.AvailabilitySetSpec().Return(&fakeSetSpecMissing) gomock.InOrder( m.Get(gomockinternal.AContext(), &fakeSetSpecMissing).Return(compute.AvailabilitySet{}, nil), @@ -186,7 +186,7 @@ func TestDeleteAvailabilitySets(t *testing.T) { { name: "noop if availability set has vms", expectedError: "", - expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_async.MockGetterMockRecorder, r *mock_async.MockReconcilerMockRecorder) { s.AvailabilitySetSpec().Return(&fakeSetSpec) gomock.InOrder( m.Get(gomockinternal.AContext(), &fakeSetSpec).Return(fakeSetWithVMs, nil), @@ -197,7 +197,7 @@ func TestDeleteAvailabilitySets(t *testing.T) { { name: "availability set not found", expectedError: "", - expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_async.MockGetterMockRecorder, r *mock_async.MockReconcilerMockRecorder) { s.AvailabilitySetSpec().Return(&fakeSetSpec) gomock.InOrder( m.Get(gomockinternal.AContext(), &fakeSetSpec).Return(nil, notFoundError), @@ -208,7 +208,7 @@ func TestDeleteAvailabilitySets(t *testing.T) { { name: "error in getting availability set", expectedError: "failed to get availability set test-as in resource group test-rg: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_async.MockGetterMockRecorder, r *mock_async.MockReconcilerMockRecorder) { s.AvailabilitySetSpec().Return(&fakeSetSpec) gomock.InOrder( m.Get(gomockinternal.AContext(), &fakeSetSpec).Return(nil, internalError), @@ -219,7 +219,7 @@ func TestDeleteAvailabilitySets(t *testing.T) { { name: "availability set get result is not an availability set", expectedError: "string is not a compute.AvailabilitySet", - expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_async.MockGetterMockRecorder, r *mock_async.MockReconcilerMockRecorder) { s.AvailabilitySetSpec().Return(&fakeSetSpec) gomock.InOrder( m.Get(gomockinternal.AContext(), &fakeSetSpec).Return("not an availability set", nil), @@ -230,7 +230,7 @@ func TestDeleteAvailabilitySets(t *testing.T) { { name: "error in deleting availability set", expectedError: "#: Internal Server Error: StatusCode=500", - expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_async.MockGetterMockRecorder, r *mock_async.MockReconcilerMockRecorder) { s.AvailabilitySetSpec().Return(&fakeSetSpec) gomock.InOrder( m.Get(gomockinternal.AContext(), &fakeSetSpec).Return(compute.AvailabilitySet{}, nil), @@ -249,14 +249,14 @@ func TestDeleteAvailabilitySets(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() scopeMock := mock_availabilitysets.NewMockAvailabilitySetScope(mockCtrl) - clientMock := mock_availabilitysets.NewMockClient(mockCtrl) + getterMock := mock_async.NewMockGetter(mockCtrl) asyncMock := mock_async.NewMockReconciler(mockCtrl) - tc.expect(scopeMock.EXPECT(), clientMock.EXPECT(), asyncMock.EXPECT()) + tc.expect(scopeMock.EXPECT(), getterMock.EXPECT(), asyncMock.EXPECT()) s := &Service{ Scope: scopeMock, - Client: clientMock, + Getter: getterMock, Reconciler: asyncMock, } diff --git a/azure/services/availabilitysets/client.go b/azure/services/availabilitysets/client.go index 3479f43477b..6085708a195 100644 --- a/azure/services/availabilitysets/client.go +++ b/azure/services/availabilitysets/client.go @@ -28,22 +28,11 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) -// Client wraps go-sdk. -type Client interface { - Get(ctx context.Context, spec azure.ResourceSpecGetter) (result interface{}, err error) - CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (result interface{}, future azureautorest.FutureAPI, err error) - DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (future azureautorest.FutureAPI, err error) - IsDone(ctx context.Context, future azureautorest.FutureAPI) (isDone bool, err error) - Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error) -} - // AzureClient contains the Azure go-sdk Client. type AzureClient struct { availabilitySets compute.AvailabilitySetsClient } -var _ Client = (*AzureClient)(nil) - // NewClient creates a new Resource SKUs Client from subscription ID. func NewClient(auth azure.Authorizer) *AzureClient { return &AzureClient{ diff --git a/azure/services/availabilitysets/mock_availabilitysets/client_mock.go b/azure/services/availabilitysets/mock_availabilitysets/client_mock.go index 431c9b7f805..1cb8862f50f 100644 --- a/azure/services/availabilitysets/mock_availabilitysets/client_mock.go +++ b/azure/services/availabilitysets/mock_availabilitysets/client_mock.go @@ -19,111 +19,3 @@ limitations under the License. // Package mock_availabilitysets is a generated GoMock package. package mock_availabilitysets - -import ( - context "context" - reflect "reflect" - - azure "github.com/Azure/go-autorest/autorest/azure" - gomock "github.com/golang/mock/gomock" - azure0 "sigs.k8s.io/cluster-api-provider-azure/azure" -) - -// 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 -} - -// CreateOrUpdateAsync mocks base method. -func (m *MockClient) CreateOrUpdateAsync(ctx context.Context, spec azure0.ResourceSpecGetter, parameters interface{}) (interface{}, azure.FutureAPI, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateOrUpdateAsync", ctx, spec, parameters) - ret0, _ := ret[0].(interface{}) - ret1, _ := ret[1].(azure.FutureAPI) - ret2, _ := ret[2].(error) - return ret0, ret1, ret2 -} - -// CreateOrUpdateAsync indicates an expected call of CreateOrUpdateAsync. -func (mr *MockClientMockRecorder) CreateOrUpdateAsync(ctx, spec, parameters interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*MockClient)(nil).CreateOrUpdateAsync), ctx, spec, parameters) -} - -// DeleteAsync mocks base method. -func (m *MockClient) DeleteAsync(ctx context.Context, spec azure0.ResourceSpecGetter) (azure.FutureAPI, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteAsync", ctx, spec) - ret0, _ := ret[0].(azure.FutureAPI) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// DeleteAsync indicates an expected call of DeleteAsync. -func (mr *MockClientMockRecorder) DeleteAsync(ctx, spec interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAsync", reflect.TypeOf((*MockClient)(nil).DeleteAsync), ctx, spec) -} - -// Get mocks base method. -func (m *MockClient) Get(ctx context.Context, spec azure0.ResourceSpecGetter) (interface{}, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Get", ctx, spec) - ret0, _ := ret[0].(interface{}) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// Get indicates an expected call of Get. -func (mr *MockClientMockRecorder) Get(ctx, spec interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockClient)(nil).Get), ctx, spec) -} - -// IsDone mocks base method. -func (m *MockClient) IsDone(ctx context.Context, future azure.FutureAPI) (bool, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsDone", ctx, future) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// IsDone indicates an expected call of IsDone. -func (mr *MockClientMockRecorder) IsDone(ctx, future interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDone", reflect.TypeOf((*MockClient)(nil).IsDone), ctx, future) -} - -// Result mocks base method. -func (m *MockClient) Result(ctx context.Context, future azure.FutureAPI, futureType string) (interface{}, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Result", ctx, future, futureType) - ret0, _ := ret[0].(interface{}) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// Result indicates an expected call of Result. -func (mr *MockClientMockRecorder) Result(ctx, future, futureType interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Result", reflect.TypeOf((*MockClient)(nil).Result), ctx, future, futureType) -} diff --git a/azure/services/networkinterfaces/client.go b/azure/services/networkinterfaces/client.go index 0d4d5f565c2..99468eea994 100644 --- a/azure/services/networkinterfaces/client.go +++ b/azure/services/networkinterfaces/client.go @@ -31,23 +31,10 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) -type ( - // Client wraps go-sdk. - Client interface { - Get(context.Context, azure.ResourceSpecGetter) (result interface{}, err error) - CreateOrUpdateAsync(context.Context, azure.ResourceSpecGetter, interface{}) (result interface{}, future azureautorest.FutureAPI, err error) - DeleteAsync(context.Context, azure.ResourceSpecGetter) (future azureautorest.FutureAPI, err error) - IsDone(context.Context, azureautorest.FutureAPI) (isDone bool, err error) - Result(context.Context, azureautorest.FutureAPI, string) (result interface{}, err error) - } - - // AzureClient contains the Azure go-sdk Client. - AzureClient struct { - interfaces network.InterfacesClient - } -) - -var _ Client = &AzureClient{} +// AzureClient contains the Azure go-sdk Client. +type AzureClient struct { + interfaces network.InterfacesClient +} // NewClient creates a new VM client from subscription ID. func NewClient(auth azure.Authorizer) *AzureClient { diff --git a/azure/services/networkinterfaces/mock_networkinterfaces/client_mock.go b/azure/services/networkinterfaces/mock_networkinterfaces/client_mock.go index 0b57ab7f230..1f4b603aa67 100644 --- a/azure/services/networkinterfaces/mock_networkinterfaces/client_mock.go +++ b/azure/services/networkinterfaces/mock_networkinterfaces/client_mock.go @@ -19,111 +19,3 @@ limitations under the License. // Package mock_networkinterfaces is a generated GoMock package. package mock_networkinterfaces - -import ( - context "context" - reflect "reflect" - - azure "github.com/Azure/go-autorest/autorest/azure" - gomock "github.com/golang/mock/gomock" - azure0 "sigs.k8s.io/cluster-api-provider-azure/azure" -) - -// 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 -} - -// CreateOrUpdateAsync mocks base method. -func (m *MockClient) CreateOrUpdateAsync(arg0 context.Context, arg1 azure0.ResourceSpecGetter, arg2 interface{}) (interface{}, azure.FutureAPI, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateOrUpdateAsync", arg0, arg1, arg2) - ret0, _ := ret[0].(interface{}) - ret1, _ := ret[1].(azure.FutureAPI) - ret2, _ := ret[2].(error) - return ret0, ret1, ret2 -} - -// CreateOrUpdateAsync indicates an expected call of CreateOrUpdateAsync. -func (mr *MockClientMockRecorder) CreateOrUpdateAsync(arg0, arg1, arg2 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*MockClient)(nil).CreateOrUpdateAsync), arg0, arg1, arg2) -} - -// DeleteAsync mocks base method. -func (m *MockClient) DeleteAsync(arg0 context.Context, arg1 azure0.ResourceSpecGetter) (azure.FutureAPI, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteAsync", arg0, arg1) - ret0, _ := ret[0].(azure.FutureAPI) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// DeleteAsync indicates an expected call of DeleteAsync. -func (mr *MockClientMockRecorder) DeleteAsync(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAsync", reflect.TypeOf((*MockClient)(nil).DeleteAsync), arg0, arg1) -} - -// Get mocks base method. -func (m *MockClient) Get(arg0 context.Context, arg1 azure0.ResourceSpecGetter) (interface{}, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Get", arg0, arg1) - ret0, _ := ret[0].(interface{}) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// Get indicates an expected call of Get. -func (mr *MockClientMockRecorder) Get(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockClient)(nil).Get), arg0, arg1) -} - -// IsDone mocks base method. -func (m *MockClient) IsDone(arg0 context.Context, arg1 azure.FutureAPI) (bool, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsDone", arg0, arg1) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// IsDone indicates an expected call of IsDone. -func (mr *MockClientMockRecorder) IsDone(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDone", reflect.TypeOf((*MockClient)(nil).IsDone), arg0, arg1) -} - -// Result mocks base method. -func (m *MockClient) Result(arg0 context.Context, arg1 azure.FutureAPI, arg2 string) (interface{}, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Result", arg0, arg1, arg2) - ret0, _ := ret[0].(interface{}) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// Result indicates an expected call of Result. -func (mr *MockClientMockRecorder) Result(arg0, arg1, arg2 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Result", reflect.TypeOf((*MockClient)(nil).Result), arg0, arg1, arg2) -} diff --git a/azure/services/roleassignments/roleassignments.go b/azure/services/roleassignments/roleassignments.go index 2744b0fe452..95ed9dc18aa 100644 --- a/azure/services/roleassignments/roleassignments.go +++ b/azure/services/roleassignments/roleassignments.go @@ -26,6 +26,7 @@ import ( "github.com/pkg/errors" "sigs.k8s.io/cluster-api-provider-azure/azure" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/async" "sigs.k8s.io/cluster-api-provider-azure/azure/services/scalesets" "sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualmachines" "sigs.k8s.io/cluster-api-provider-azure/util/tele" @@ -43,7 +44,7 @@ type RoleAssignmentScope interface { type Service struct { Scope RoleAssignmentScope client - virtualMachinesClient virtualmachines.Client + virtualMachinesGetter async.Getter virtualMachineScaleSetClient scalesets.Client } @@ -52,7 +53,7 @@ func New(scope RoleAssignmentScope) *Service { return &Service{ Scope: scope, client: newClient(scope), - virtualMachinesClient: virtualmachines.NewClient(scope), + virtualMachinesGetter: virtualmachines.NewClient(scope), virtualMachineScaleSetClient: scalesets.NewClient(scope), } } @@ -85,7 +86,7 @@ func (s *Service) reconcileVM(ctx context.Context, roleSpec azure.RoleAssignment ResourceGroup: s.Scope.ResourceGroup(), } - resultVMIface, err := s.virtualMachinesClient.Get(ctx, spec) + resultVMIface, err := s.virtualMachinesGetter.Get(ctx, spec) if err != nil { return errors.Wrap(err, "cannot get VM to assign role to system assigned identity") } diff --git a/azure/services/roleassignments/roleassignments_test.go b/azure/services/roleassignments/roleassignments_test.go index 2b223ef9fee..dd5190df09c 100644 --- a/azure/services/roleassignments/roleassignments_test.go +++ b/azure/services/roleassignments/roleassignments_test.go @@ -28,10 +28,10 @@ import ( "github.com/golang/mock/gomock" . "github.com/onsi/gomega" "sigs.k8s.io/cluster-api-provider-azure/azure" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/async/mock_async" "sigs.k8s.io/cluster-api-provider-azure/azure/services/roleassignments/mock_roleassignments" "sigs.k8s.io/cluster-api-provider-azure/azure/services/scalesets/mock_scalesets" "sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualmachines" - "sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualmachines/mock_virtualmachines" gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" ) @@ -45,13 +45,13 @@ var ( func TestReconcileRoleAssignmentsVM(t *testing.T) { testcases := []struct { name string - expect func(s *mock_roleassignments.MockRoleAssignmentScopeMockRecorder, m *mock_roleassignments.MockclientMockRecorder, v *mock_virtualmachines.MockClientMockRecorder) + expect func(s *mock_roleassignments.MockRoleAssignmentScopeMockRecorder, m *mock_roleassignments.MockclientMockRecorder, v *mock_async.MockGetterMockRecorder) expectedError string }{ { name: "create a role assignment", expectedError: "", - expect: func(s *mock_roleassignments.MockRoleAssignmentScopeMockRecorder, m *mock_roleassignments.MockclientMockRecorder, v *mock_virtualmachines.MockClientMockRecorder) { + expect: func(s *mock_roleassignments.MockRoleAssignmentScopeMockRecorder, m *mock_roleassignments.MockclientMockRecorder, v *mock_async.MockGetterMockRecorder) { s.SubscriptionID().AnyTimes().Return("12345") s.ResourceGroup().Return("my-rg") s.RoleAssignmentSpecs().Return([]azure.RoleAssignmentSpec{ @@ -76,7 +76,7 @@ func TestReconcileRoleAssignmentsVM(t *testing.T) { { name: "error getting VM", expectedError: "cannot get VM to assign role to system assigned identity: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_roleassignments.MockRoleAssignmentScopeMockRecorder, m *mock_roleassignments.MockclientMockRecorder, v *mock_virtualmachines.MockClientMockRecorder) { + expect: func(s *mock_roleassignments.MockRoleAssignmentScopeMockRecorder, m *mock_roleassignments.MockclientMockRecorder, v *mock_async.MockGetterMockRecorder) { s.SubscriptionID().AnyTimes().Return("12345") s.ResourceGroup().Return("my-rg") s.RoleAssignmentSpecs().Return([]azure.RoleAssignmentSpec{ @@ -91,7 +91,7 @@ func TestReconcileRoleAssignmentsVM(t *testing.T) { { name: "return error when creating a role assignment", expectedError: "cannot assign role to VM system assigned identity: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_roleassignments.MockRoleAssignmentScopeMockRecorder, m *mock_roleassignments.MockclientMockRecorder, v *mock_virtualmachines.MockClientMockRecorder) { + expect: func(s *mock_roleassignments.MockRoleAssignmentScopeMockRecorder, m *mock_roleassignments.MockclientMockRecorder, v *mock_async.MockGetterMockRecorder) { s.SubscriptionID().AnyTimes().Return("12345") s.ResourceGroup().Return("my-rg") s.RoleAssignmentSpecs().Return([]azure.RoleAssignmentSpec{ @@ -119,14 +119,14 @@ func TestReconcileRoleAssignmentsVM(t *testing.T) { defer mockCtrl.Finish() scopeMock := mock_roleassignments.NewMockRoleAssignmentScope(mockCtrl) clientMock := mock_roleassignments.NewMockclient(mockCtrl) - vmMock := mock_virtualmachines.NewMockClient(mockCtrl) + vmGetterMock := mock_async.NewMockGetter(mockCtrl) - tc.expect(scopeMock.EXPECT(), clientMock.EXPECT(), vmMock.EXPECT()) + tc.expect(scopeMock.EXPECT(), clientMock.EXPECT(), vmGetterMock.EXPECT()) s := &Service{ Scope: scopeMock, client: clientMock, - virtualMachinesClient: vmMock, + virtualMachinesGetter: vmGetterMock, } err := s.Reconcile(context.TODO()) diff --git a/azure/services/virtualmachines/client.go b/azure/services/virtualmachines/client.go index ccf98eb27a2..1f8e4c5e971 100644 --- a/azure/services/virtualmachines/client.go +++ b/azure/services/virtualmachines/client.go @@ -32,23 +32,10 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) -type ( - // Client wraps go-sdk. - Client interface { - Get(context.Context, azure.ResourceSpecGetter) (interface{}, error) - CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (result interface{}, future azureautorest.FutureAPI, err error) - DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (future azureautorest.FutureAPI, err error) - IsDone(ctx context.Context, future azureautorest.FutureAPI) (isDone bool, err error) - Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error) - } - - // AzureClient contains the Azure go-sdk Client. - AzureClient struct { - virtualmachines compute.VirtualMachinesClient - } -) - -var _ Client = &AzureClient{} +// AzureClient contains the Azure go-sdk Client. +type AzureClient struct { + virtualmachines compute.VirtualMachinesClient +} // NewClient creates a new VM client from subscription ID. func NewClient(auth azure.Authorizer) *AzureClient { diff --git a/azure/services/virtualmachines/mock_virtualmachines/client_mock.go b/azure/services/virtualmachines/mock_virtualmachines/client_mock.go index 86be1de0949..62350954dba 100644 --- a/azure/services/virtualmachines/mock_virtualmachines/client_mock.go +++ b/azure/services/virtualmachines/mock_virtualmachines/client_mock.go @@ -19,111 +19,3 @@ limitations under the License. // Package mock_virtualmachines is a generated GoMock package. package mock_virtualmachines - -import ( - context "context" - reflect "reflect" - - azure "github.com/Azure/go-autorest/autorest/azure" - gomock "github.com/golang/mock/gomock" - azure0 "sigs.k8s.io/cluster-api-provider-azure/azure" -) - -// 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 -} - -// CreateOrUpdateAsync mocks base method. -func (m *MockClient) CreateOrUpdateAsync(ctx context.Context, spec azure0.ResourceSpecGetter, parameters interface{}) (interface{}, azure.FutureAPI, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateOrUpdateAsync", ctx, spec, parameters) - ret0, _ := ret[0].(interface{}) - ret1, _ := ret[1].(azure.FutureAPI) - ret2, _ := ret[2].(error) - return ret0, ret1, ret2 -} - -// CreateOrUpdateAsync indicates an expected call of CreateOrUpdateAsync. -func (mr *MockClientMockRecorder) CreateOrUpdateAsync(ctx, spec, parameters interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*MockClient)(nil).CreateOrUpdateAsync), ctx, spec, parameters) -} - -// DeleteAsync mocks base method. -func (m *MockClient) DeleteAsync(ctx context.Context, spec azure0.ResourceSpecGetter) (azure.FutureAPI, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteAsync", ctx, spec) - ret0, _ := ret[0].(azure.FutureAPI) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// DeleteAsync indicates an expected call of DeleteAsync. -func (mr *MockClientMockRecorder) DeleteAsync(ctx, spec interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAsync", reflect.TypeOf((*MockClient)(nil).DeleteAsync), ctx, spec) -} - -// Get mocks base method. -func (m *MockClient) Get(arg0 context.Context, arg1 azure0.ResourceSpecGetter) (interface{}, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Get", arg0, arg1) - ret0, _ := ret[0].(interface{}) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// Get indicates an expected call of Get. -func (mr *MockClientMockRecorder) Get(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockClient)(nil).Get), arg0, arg1) -} - -// IsDone mocks base method. -func (m *MockClient) IsDone(ctx context.Context, future azure.FutureAPI) (bool, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsDone", ctx, future) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// IsDone indicates an expected call of IsDone. -func (mr *MockClientMockRecorder) IsDone(ctx, future interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDone", reflect.TypeOf((*MockClient)(nil).IsDone), ctx, future) -} - -// Result mocks base method. -func (m *MockClient) Result(ctx context.Context, future azure.FutureAPI, futureType string) (interface{}, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Result", ctx, future, futureType) - ret0, _ := ret[0].(interface{}) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// Result indicates an expected call of Result. -func (mr *MockClientMockRecorder) Result(ctx, future, futureType interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Result", reflect.TypeOf((*MockClient)(nil).Result), ctx, future, futureType) -} diff --git a/azure/services/virtualmachines/virtualmachines.go b/azure/services/virtualmachines/virtualmachines.go index abe96b9f3c5..0c6a3a4afd5 100644 --- a/azure/services/virtualmachines/virtualmachines.go +++ b/azure/services/virtualmachines/virtualmachines.go @@ -30,7 +30,6 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/azure" "sigs.k8s.io/cluster-api-provider-azure/azure/converters" "sigs.k8s.io/cluster-api-provider-azure/azure/services/async" - "sigs.k8s.io/cluster-api-provider-azure/azure/services/availabilitysets" "sigs.k8s.io/cluster-api-provider-azure/azure/services/networkinterfaces" "sigs.k8s.io/cluster-api-provider-azure/azure/services/publicips" "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" @@ -54,20 +53,18 @@ type VMScope interface { type Service struct { Scope VMScope async.Reconciler - interfacesClient networkinterfaces.Client - publicIPsClient publicips.Client - availabilitySetsClient availabilitysets.Client + interfacesGetter async.Getter + publicIPsClient publicips.Client } // New creates a new service. func New(scope VMScope) *Service { Client := NewClient(scope) return &Service{ - Scope: scope, - interfacesClient: networkinterfaces.NewClient(scope), - publicIPsClient: publicips.NewClient(scope), - availabilitySetsClient: availabilitysets.NewClient(scope), - Reconciler: async.New(scope, Client, Client), + Scope: scope, + interfacesGetter: networkinterfaces.NewClient(scope), + publicIPsClient: publicips.NewClient(scope), + Reconciler: async.New(scope, Client, Client), } } @@ -146,7 +143,7 @@ func (s *Service) getAddresses(ctx context.Context, vm compute.VirtualMachine, r nicName := getResourceNameByID(to.String(nicRef.ID)) // Fetch nic and append its addresses - existingNic, err := s.interfacesClient.Get(ctx, &networkinterfaces.NICSpec{ + existingNic, err := s.interfacesGetter.Get(ctx, &networkinterfaces.NICSpec{ Name: nicName, ResourceGroup: rgName, }) diff --git a/azure/services/virtualmachines/virtualmachines_test.go b/azure/services/virtualmachines/virtualmachines_test.go index 8a63b629b24..5a39da4fbb4 100644 --- a/azure/services/virtualmachines/virtualmachines_test.go +++ b/azure/services/virtualmachines/virtualmachines_test.go @@ -30,9 +30,7 @@ import ( corev1 "k8s.io/api/core/v1" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure/services/async/mock_async" - "sigs.k8s.io/cluster-api-provider-azure/azure/services/availabilitysets/mock_availabilitysets" "sigs.k8s.io/cluster-api-provider-azure/azure/services/networkinterfaces" - "sigs.k8s.io/cluster-api-provider-azure/azure/services/networkinterfaces/mock_networkinterfaces" "sigs.k8s.io/cluster-api-provider-azure/azure/services/publicips/mock_publicips" "sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualmachines/mock_virtualmachines" gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" @@ -107,12 +105,12 @@ func TestReconcileVM(t *testing.T) { testcases := []struct { name string expectedError string - expect func(s *mock_virtualmachines.MockVMScopeMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) + expect func(s *mock_virtualmachines.MockVMScopeMockRecorder, mnic *mock_async.MockGetterMockRecorder, mpip *mock_publicips.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) }{ { name: "create vm succeeds", expectedError: "", - expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, mnic *mock_async.MockGetterMockRecorder, mpip *mock_publicips.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { s.VMSpec().Return(&fakeVMSpec) r.CreateResource(gomockinternal.AContext(), &fakeVMSpec, serviceName).Return(fakeExistingVM, nil) s.UpdatePutStatus(infrav1.VMRunningCondition, serviceName, nil) @@ -128,7 +126,7 @@ func TestReconcileVM(t *testing.T) { { name: "creating vm fails", expectedError: "#: Internal Server Error: StatusCode=500", - expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, mnic *mock_async.MockGetterMockRecorder, mpip *mock_publicips.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { s.VMSpec().Return(&fakeVMSpec) r.CreateResource(gomockinternal.AContext(), &fakeVMSpec, serviceName).Return(nil, internalError) s.UpdatePutStatus(infrav1.VMRunningCondition, serviceName, internalError) @@ -138,7 +136,7 @@ func TestReconcileVM(t *testing.T) { { name: "create vm succeeds but failed to get network interfaces", expectedError: "failed to fetch VM addresses: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, mnic *mock_async.MockGetterMockRecorder, mpip *mock_publicips.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { s.VMSpec().Return(&fakeVMSpec) r.CreateResource(gomockinternal.AContext(), &fakeVMSpec, serviceName).Return(fakeExistingVM, nil) s.UpdatePutStatus(infrav1.VMRunningCondition, serviceName, nil) @@ -151,7 +149,7 @@ func TestReconcileVM(t *testing.T) { { name: "create vm succeeds but failed to get public IPs", expectedError: "failed to fetch VM addresses: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, mnic *mock_async.MockGetterMockRecorder, mpip *mock_publicips.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { s.VMSpec().Return(&fakeVMSpec) r.CreateResource(gomockinternal.AContext(), &fakeVMSpec, serviceName).Return(fakeExistingVM, nil) s.UpdatePutStatus(infrav1.VMRunningCondition, serviceName, nil) @@ -173,19 +171,17 @@ func TestReconcileVM(t *testing.T) { defer mockCtrl.Finish() scopeMock := mock_virtualmachines.NewMockVMScope(mockCtrl) - interfaceMock := mock_networkinterfaces.NewMockClient(mockCtrl) + interfaceMock := mock_async.NewMockGetter(mockCtrl) publicIPMock := mock_publicips.NewMockClient(mockCtrl) - availabilitySetsMock := mock_availabilitysets.NewMockClient(mockCtrl) asyncMock := mock_async.NewMockReconciler(mockCtrl) tc.expect(scopeMock.EXPECT(), interfaceMock.EXPECT(), publicIPMock.EXPECT(), asyncMock.EXPECT()) s := &Service{ - Scope: scopeMock, - interfacesClient: interfaceMock, - publicIPsClient: publicIPMock, - availabilitySetsClient: availabilitySetsMock, - Reconciler: asyncMock, + Scope: scopeMock, + interfacesGetter: interfaceMock, + publicIPsClient: publicIPMock, + Reconciler: asyncMock, } err := s.Reconcile(context.TODO()) diff --git a/azure/services/virtualnetworks/client.go b/azure/services/virtualnetworks/client.go index be730ddda28..58700999c18 100644 --- a/azure/services/virtualnetworks/client.go +++ b/azure/services/virtualnetworks/client.go @@ -18,33 +18,28 @@ package virtualnetworks import ( "context" + "encoding/json" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" "github.com/Azure/go-autorest/autorest" + azureautorest "github.com/Azure/go-autorest/autorest/azure" + "github.com/pkg/errors" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" + "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) -// Client wraps go-sdk. -type Client interface { - Get(context.Context, string, string) (network.VirtualNetwork, error) - CreateOrUpdate(context.Context, string, string, network.VirtualNetwork) error - Delete(context.Context, string, string) error - CheckIPAddressAvailability(context.Context, string, string, string) (network.IPAddressAvailabilityResult, error) -} - -// AzureClient contains the Azure go-sdk Client. -type AzureClient struct { +// azureClient contains the Azure go-sdk Client. +type azureClient struct { virtualnetworks network.VirtualNetworksClient } -var _ Client = &AzureClient{} - -// 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 := newVirtualNetworksClient(auth.SubscriptionID(), auth.BaseURI(), auth.Authorizer()) - return &AzureClient{ + return &azureClient{ virtualnetworks: c, } } @@ -56,52 +51,113 @@ func newVirtualNetworksClient(subscriptionID string, baseURI string, authorizer return vnetsClient } -// Get gets the specified virtual network by resource group. -func (ac *AzureClient) Get(ctx context.Context, resourceGroupName, vnetName string) (network.VirtualNetwork, error) { - ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualnetworks.AzureClient.Get") +// Get gets the specified virtual network. +func (ac *azureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) (result interface{}, err error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualnetworks.azureClient.Get") defer done() - return ac.virtualnetworks.Get(ctx, resourceGroupName, vnetName, "") + return ac.virtualnetworks.Get(ctx, spec.ResourceGroupName(), spec.ResourceName(), "") } -// CreateOrUpdate creates or updates a virtual network in the specified resource group. -func (ac *AzureClient) CreateOrUpdate(ctx context.Context, resourceGroupName, vnetName string, vn network.VirtualNetwork) error { - ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualnetworks.AzureClient.CreateOrUpdate") +// CreateOrUpdateAsync creates or updates a virtual network in the specified resource group asynchronously. +// It sends a PUT request to Azure and if accepted without error, the func will return a Future which can be used to track the ongoing +// progress of the operation. +func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (result interface{}, future azureautorest.FutureAPI, err error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualnetworks.azureClient.CreateOrUpdateAsync") defer done() - future, err := ac.virtualnetworks.CreateOrUpdate(ctx, resourceGroupName, vnetName, vn) + vn, ok := parameters.(network.VirtualNetwork) + if !ok { + return nil, nil, errors.Errorf("%T is not a network.VirtualNetwork", parameters) + } + + createFuture, err := ac.virtualnetworks.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.ResourceName(), vn) if err != nil { - return err + return nil, nil, err } - err = future.WaitForCompletionRef(ctx, ac.virtualnetworks.Client) + + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + + err = createFuture.WaitForCompletionRef(ctx, ac.virtualnetworks.Client) if err != nil { - return err + // if an error occurs, return the future. + // this means the long-running operation didn't finish in the specified timeout. + return nil, &createFuture, err } - _, err = future.Result(ac.virtualnetworks) - return err + result, err = createFuture.Result(ac.virtualnetworks) + // if the operation completed, return a nil future. + return result, nil, err } -// Delete deletes the specified virtual network. -func (ac *AzureClient) Delete(ctx context.Context, resourceGroupName, vnetName string) error { - ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualnetworks.AzureClient.Delete") +// DeleteAsync deletes a virtual network asynchronously. DeleteAsync sends a DELETE +// request to Azure and if accepted without error, the func will return a Future which can be used to track the ongoing +// progress of the operation. +func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (future azureautorest.FutureAPI, err error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualnetworks.azureClient.DeleteAsync") defer done() - future, err := ac.virtualnetworks.Delete(ctx, resourceGroupName, vnetName) + deleteFuture, err := ac.virtualnetworks.Delete(ctx, spec.ResourceGroupName(), spec.ResourceName()) if err != nil { - return err + return nil, err } - err = future.WaitForCompletionRef(ctx, ac.virtualnetworks.Client) + + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + + err = deleteFuture.WaitForCompletionRef(ctx, ac.virtualnetworks.Client) if err != nil { - return err + // if an error occurs, return the future. + // this means the long-running operation didn't finish in the specified timeout. + return &deleteFuture, err } - _, err = future.Result(ac.virtualnetworks) - return err + _, err = deleteFuture.Result(ac.virtualnetworks) + // if the operation completed, return a nil future. + return nil, err } -// CheckIPAddressAvailability checks whether a private IP address is available for use. -func (ac *AzureClient) CheckIPAddressAvailability(ctx context.Context, resourceGroupName, vnetName, ip string) (network.IPAddressAvailabilityResult, error) { - ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualnetworks.AzureClient.CheckIPAddressAvailability") +// IsDone returns true if the long-running operation has completed. +func (ac *azureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (isDone bool, err error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualnetworks.azureClient.IsDone") defer done() - return ac.virtualnetworks.CheckIPAddressAvailability(ctx, resourceGroupName, vnetName, ip) + isDone, err = future.DoneWithContext(ctx, ac.virtualnetworks) + if err != nil { + return false, errors.Wrap(err, "failed checking if the operation was complete") + } + + return isDone, nil +} + +// Result fetches the result of a long-running operation future. +func (ac *azureClient) Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error) { + _, _, done := tele.StartSpanWithLogger(ctx, "virtualnetworks.azureClient.Result") + defer done() + + if future == nil { + return nil, errors.Errorf("cannot get result from nil future") + } + + switch futureType { + case infrav1.PutFuture: + // Marshal and Unmarshal the future to put it into the correct future type so we can access the Result function. + // Unfortunately the FutureAPI can't be casted directly to VirtualNetworksCreateOrUpdateFuture because it is a azureautorest.Future, which doesn't implement the Result function. See PR #1686 for discussion on alternatives. + // It was converted back to a generic azureautorest.Future from the CAPZ infrav1.Future type stored in Status: https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/main/azure/converters/futures.go#L49. + var createFuture *network.VirtualNetworksCreateOrUpdateFuture + jsonData, err := future.MarshalJSON() + if err != nil { + return nil, errors.Wrap(err, "failed to marshal future") + } + if err := json.Unmarshal(jsonData, &createFuture); err != nil { + return nil, errors.Wrap(err, "failed to unmarshal future data") + } + return (*createFuture).Result(ac.virtualnetworks) + + case infrav1.DeleteFuture: + // Delete does not return a result vnet. + return nil, nil + + default: + return nil, errors.Errorf("unknown future type %q", futureType) + } } diff --git a/azure/services/virtualnetworks/mock_virtualnetworks/client_mock.go b/azure/services/virtualnetworks/mock_virtualnetworks/client_mock.go index b2f48294cc6..e774e53779a 100644 --- a/azure/services/virtualnetworks/mock_virtualnetworks/client_mock.go +++ b/azure/services/virtualnetworks/mock_virtualnetworks/client_mock.go @@ -19,92 +19,3 @@ limitations under the License. // Package mock_virtualnetworks is a generated GoMock package. package mock_virtualnetworks - -import ( - context "context" - reflect "reflect" - - network "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" - gomock "github.com/golang/mock/gomock" -) - -// 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 -} - -// CheckIPAddressAvailability mocks base method. -func (m *MockClient) CheckIPAddressAvailability(arg0 context.Context, arg1, arg2, arg3 string) (network.IPAddressAvailabilityResult, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CheckIPAddressAvailability", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(network.IPAddressAvailabilityResult) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// CheckIPAddressAvailability indicates an expected call of CheckIPAddressAvailability. -func (mr *MockClientMockRecorder) CheckIPAddressAvailability(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckIPAddressAvailability", reflect.TypeOf((*MockClient)(nil).CheckIPAddressAvailability), arg0, arg1, arg2, arg3) -} - -// CreateOrUpdate mocks base method. -func (m *MockClient) CreateOrUpdate(arg0 context.Context, arg1, arg2 string, arg3 network.VirtualNetwork) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateOrUpdate", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(error) - return ret0 -} - -// CreateOrUpdate indicates an expected call of CreateOrUpdate. -func (mr *MockClientMockRecorder) CreateOrUpdate(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdate", reflect.TypeOf((*MockClient)(nil).CreateOrUpdate), arg0, arg1, arg2, arg3) -} - -// 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) -} - -// Get mocks base method. -func (m *MockClient) Get(arg0 context.Context, arg1, arg2 string) (network.VirtualNetwork, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Get", arg0, arg1, arg2) - ret0, _ := ret[0].(network.VirtualNetwork) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// Get indicates an expected call of Get. -func (mr *MockClientMockRecorder) Get(arg0, arg1, arg2 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockClient)(nil).Get), arg0, arg1, arg2) -} diff --git a/azure/services/virtualnetworks/mock_virtualnetworks/doc.go b/azure/services/virtualnetworks/mock_virtualnetworks/doc.go index f27db54b99a..10a929615aa 100644 --- a/azure/services/virtualnetworks/mock_virtualnetworks/doc.go +++ b/azure/services/virtualnetworks/mock_virtualnetworks/doc.go @@ -15,7 +15,7 @@ limitations under the License. */ // Run go generate to regenerate this mock. -//go:generate ../../../../hack/tools/bin/mockgen -destination client_mock.go -package mock_virtualnetworks -source ../client.go Client +//go:generate ../../../../hack/tools/bin/mockgen -destination client_mock.go -package mock_virtualnetworks -source ../client.go Getter //go:generate ../../../../hack/tools/bin/mockgen -destination virtualnetworks_mock.go -package mock_virtualnetworks -source ../virtualnetworks.go VNetScope //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 virtualnetworks_mock.go > _virtualnetworks_mock.go && mv _virtualnetworks_mock.go virtualnetworks_mock.go" diff --git a/azure/services/virtualnetworks/mock_virtualnetworks/virtualnetworks_mock.go b/azure/services/virtualnetworks/mock_virtualnetworks/virtualnetworks_mock.go index 2fbbf35b9b5..982acc86d73 100644 --- a/azure/services/virtualnetworks/mock_virtualnetworks/virtualnetworks_mock.go +++ b/azure/services/virtualnetworks/mock_virtualnetworks/virtualnetworks_mock.go @@ -27,6 +27,7 @@ import ( gomock "github.com/golang/mock/gomock" v1beta1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" azure "sigs.k8s.io/cluster-api-provider-azure/azure" + v1beta10 "sigs.k8s.io/cluster-api/api/v1beta1" ) // MockVNetScope is a mock of VNetScope interface. @@ -52,20 +53,6 @@ func (m *MockVNetScope) EXPECT() *MockVNetScopeMockRecorder { return m.recorder } -// AdditionalTags mocks base method. -func (m *MockVNetScope) AdditionalTags() v1beta1.Tags { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AdditionalTags") - ret0, _ := ret[0].(v1beta1.Tags) - return ret0 -} - -// AdditionalTags indicates an expected call of AdditionalTags. -func (mr *MockVNetScopeMockRecorder) AdditionalTags() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AdditionalTags", reflect.TypeOf((*MockVNetScope)(nil).AdditionalTags)) -} - // Authorizer mocks base method. func (m *MockVNetScope) Authorizer() autorest.Authorizer { m.ctrl.T.Helper() @@ -80,20 +67,6 @@ func (mr *MockVNetScopeMockRecorder) Authorizer() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Authorizer", reflect.TypeOf((*MockVNetScope)(nil).Authorizer)) } -// AvailabilitySetEnabled mocks base method. -func (m *MockVNetScope) AvailabilitySetEnabled() bool { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AvailabilitySetEnabled") - ret0, _ := ret[0].(bool) - return ret0 -} - -// AvailabilitySetEnabled indicates an expected call of AvailabilitySetEnabled. -func (mr *MockVNetScopeMockRecorder) AvailabilitySetEnabled() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AvailabilitySetEnabled", reflect.TypeOf((*MockVNetScope)(nil).AvailabilitySetEnabled)) -} - // BaseURI mocks base method. func (m *MockVNetScope) BaseURI() string { m.ctrl.T.Helper() @@ -150,20 +123,6 @@ func (mr *MockVNetScopeMockRecorder) CloudEnvironment() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CloudEnvironment", reflect.TypeOf((*MockVNetScope)(nil).CloudEnvironment)) } -// CloudProviderConfigOverrides mocks base method. -func (m *MockVNetScope) CloudProviderConfigOverrides() *v1beta1.CloudProviderConfigOverrides { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CloudProviderConfigOverrides") - ret0, _ := ret[0].(*v1beta1.CloudProviderConfigOverrides) - return ret0 -} - -// CloudProviderConfigOverrides indicates an expected call of CloudProviderConfigOverrides. -func (mr *MockVNetScopeMockRecorder) CloudProviderConfigOverrides() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CloudProviderConfigOverrides", reflect.TypeOf((*MockVNetScope)(nil).CloudProviderConfigOverrides)) -} - // ClusterName mocks base method. func (m *MockVNetScope) ClusterName() string { m.ctrl.T.Helper() @@ -178,60 +137,56 @@ func (mr *MockVNetScopeMockRecorder) ClusterName() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClusterName", reflect.TypeOf((*MockVNetScope)(nil).ClusterName)) } -// FailureDomains mocks base method. -func (m *MockVNetScope) FailureDomains() []string { +// DeleteLongRunningOperationState mocks base method. +func (m *MockVNetScope) DeleteLongRunningOperationState(arg0, arg1 string) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "FailureDomains") - ret0, _ := ret[0].([]string) - return ret0 + m.ctrl.Call(m, "DeleteLongRunningOperationState", arg0, arg1) } -// FailureDomains indicates an expected call of FailureDomains. -func (mr *MockVNetScopeMockRecorder) FailureDomains() *gomock.Call { +// DeleteLongRunningOperationState indicates an expected call of DeleteLongRunningOperationState. +func (mr *MockVNetScopeMockRecorder) DeleteLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FailureDomains", reflect.TypeOf((*MockVNetScope)(nil).FailureDomains)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteLongRunningOperationState", reflect.TypeOf((*MockVNetScope)(nil).DeleteLongRunningOperationState), arg0, arg1) } -// HashKey mocks base method. -func (m *MockVNetScope) HashKey() string { +// GetLongRunningOperationState mocks base method. +func (m *MockVNetScope) GetLongRunningOperationState(arg0, arg1 string) *v1beta1.Future { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "HashKey") - ret0, _ := ret[0].(string) + ret := m.ctrl.Call(m, "GetLongRunningOperationState", arg0, arg1) + ret0, _ := ret[0].(*v1beta1.Future) return ret0 } -// HashKey indicates an expected call of HashKey. -func (mr *MockVNetScopeMockRecorder) HashKey() *gomock.Call { +// GetLongRunningOperationState indicates an expected call of GetLongRunningOperationState. +func (mr *MockVNetScopeMockRecorder) GetLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HashKey", reflect.TypeOf((*MockVNetScope)(nil).HashKey)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLongRunningOperationState", reflect.TypeOf((*MockVNetScope)(nil).GetLongRunningOperationState), arg0, arg1) } -// Location mocks base method. -func (m *MockVNetScope) Location() string { +// HashKey mocks base method. +func (m *MockVNetScope) HashKey() string { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Location") + ret := m.ctrl.Call(m, "HashKey") ret0, _ := ret[0].(string) return ret0 } -// Location indicates an expected call of Location. -func (mr *MockVNetScopeMockRecorder) Location() *gomock.Call { +// HashKey indicates an expected call of HashKey. +func (mr *MockVNetScopeMockRecorder) HashKey() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Location", reflect.TypeOf((*MockVNetScope)(nil).Location)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HashKey", reflect.TypeOf((*MockVNetScope)(nil).HashKey)) } -// ResourceGroup mocks base method. -func (m *MockVNetScope) ResourceGroup() string { +// SetLongRunningOperationState mocks base method. +func (m *MockVNetScope) SetLongRunningOperationState(arg0 *v1beta1.Future) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ResourceGroup") - ret0, _ := ret[0].(string) - return ret0 + m.ctrl.Call(m, "SetLongRunningOperationState", arg0) } -// ResourceGroup indicates an expected call of ResourceGroup. -func (mr *MockVNetScopeMockRecorder) ResourceGroup() *gomock.Call { +// SetLongRunningOperationState indicates an expected call of SetLongRunningOperationState. +func (mr *MockVNetScopeMockRecorder) SetLongRunningOperationState(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResourceGroup", reflect.TypeOf((*MockVNetScope)(nil).ResourceGroup)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLongRunningOperationState", reflect.TypeOf((*MockVNetScope)(nil).SetLongRunningOperationState), arg0) } // SubscriptionID mocks base method. @@ -262,11 +217,47 @@ func (mr *MockVNetScopeMockRecorder) TenantID() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TenantID", reflect.TypeOf((*MockVNetScope)(nil).TenantID)) } +// UpdateDeleteStatus mocks base method. +func (m *MockVNetScope) UpdateDeleteStatus(arg0 v1beta10.ConditionType, arg1 string, arg2 error) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "UpdateDeleteStatus", arg0, arg1, arg2) +} + +// UpdateDeleteStatus indicates an expected call of UpdateDeleteStatus. +func (mr *MockVNetScopeMockRecorder) UpdateDeleteStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateDeleteStatus", reflect.TypeOf((*MockVNetScope)(nil).UpdateDeleteStatus), arg0, arg1, arg2) +} + +// UpdatePatchStatus mocks base method. +func (m *MockVNetScope) UpdatePatchStatus(arg0 v1beta10.ConditionType, arg1 string, arg2 error) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "UpdatePatchStatus", arg0, arg1, arg2) +} + +// UpdatePatchStatus indicates an expected call of UpdatePatchStatus. +func (mr *MockVNetScopeMockRecorder) UpdatePatchStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePatchStatus", reflect.TypeOf((*MockVNetScope)(nil).UpdatePatchStatus), arg0, arg1, arg2) +} + +// UpdatePutStatus mocks base method. +func (m *MockVNetScope) UpdatePutStatus(arg0 v1beta10.ConditionType, arg1 string, arg2 error) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "UpdatePutStatus", arg0, arg1, arg2) +} + +// UpdatePutStatus indicates an expected call of UpdatePutStatus. +func (mr *MockVNetScopeMockRecorder) UpdatePutStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePutStatus", reflect.TypeOf((*MockVNetScope)(nil).UpdatePutStatus), arg0, arg1, arg2) +} + // VNetSpec mocks base method. -func (m *MockVNetScope) VNetSpec() azure.VNetSpec { +func (m *MockVNetScope) VNetSpec() azure.ResourceSpecGetter { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "VNetSpec") - ret0, _ := ret[0].(azure.VNetSpec) + ret0, _ := ret[0].(azure.ResourceSpecGetter) return ret0 } diff --git a/azure/services/virtualnetworks/spec.go b/azure/services/virtualnetworks/spec.go new file mode 100644 index 00000000000..56d3ec53d0e --- /dev/null +++ b/azure/services/virtualnetworks/spec.go @@ -0,0 +1,72 @@ +/* +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 virtualnetworks + +import ( + "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" + "github.com/Azure/go-autorest/autorest/to" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-azure/azure/converters" +) + +// VNetSpec defines the specification for a Virtual Network. +type VNetSpec struct { + ResourceGroup string + Name string + CIDRs []string + Location string + ClusterName string + AdditionalTags infrav1.Tags +} + +// ResourceName returns the name of the vnet. +func (s *VNetSpec) ResourceName() string { + return s.Name +} + +// ResourceGroupName returns the name of the resource group. +func (s *VNetSpec) ResourceGroupName() string { + return s.ResourceGroup +} + +// OwnerResourceName is a no-op for vnets. +func (s *VNetSpec) OwnerResourceName() string { + return "" +} + +// Parameters returns the parameters for the vnet. +func (s *VNetSpec) Parameters(existing interface{}) (interface{}, error) { + if existing != nil { + // vnet already exists, nothing to update. + return nil, nil + } + return network.VirtualNetwork{ + Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{ + ClusterName: s.ClusterName, + Lifecycle: infrav1.ResourceLifecycleOwned, + Name: to.StringPtr(s.Name), + Role: to.StringPtr(infrav1.CommonRole), + Additional: s.AdditionalTags, + })), + Location: to.StringPtr(s.Location), + VirtualNetworkPropertiesFormat: &network.VirtualNetworkPropertiesFormat{ + AddressSpace: &network.AddressSpace{ + AddressPrefixes: &s.CIDRs, + }, + }, + }, nil +} diff --git a/azure/services/virtualnetworks/virtualnetworks.go b/azure/services/virtualnetworks/virtualnetworks.go index 9a6f42090d4..4be1cd731a5 100644 --- a/azure/services/virtualnetworks/virtualnetworks.go +++ b/azure/services/virtualnetworks/virtualnetworks.go @@ -18,7 +18,6 @@ package virtualnetworks import ( "context" - "fmt" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" "github.com/Azure/go-autorest/autorest/to" @@ -26,143 +25,117 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" "sigs.k8s.io/cluster-api-provider-azure/azure/converters" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/async" + "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) +const serviceName = "virtualnetworks" + // VNetScope defines the scope interface for a virtual network service. type VNetScope interface { - azure.ClusterDescriber + azure.Authorizer + azure.AsyncStatusUpdater Vnet() *infrav1.VnetSpec - VNetSpec() azure.VNetSpec + VNetSpec() azure.ResourceSpecGetter + ClusterName() string } // Service provides operations on Azure resources. type Service struct { Scope VNetScope - Client + async.Reconciler + async.Getter } // New creates a new service. func New(scope VNetScope) *Service { + client := newClient(scope) return &Service{ - Scope: scope, - Client: NewClient(scope), + Scope: scope, + Getter: client, + Reconciler: async.New(scope, client, client), } } -// Reconcile gets/creates/updates a virtual network. func (s *Service) Reconcile(ctx context.Context) error { - ctx, log, done := tele.StartSpanWithLogger(ctx, "virtualnetworks.Service.Reconcile") + ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualnetworks.Service.Reconcile") defer done() - // Following should be created upstream and provided as an input to NewService - // A VNet has following dependencies - // * VNet Cidr - // * Control Plane Subnet Cidr - // * Node Subnet Cidr - // * Control Plane NSG - // * Node NSG - // * Node Route Table - vnetSpec := s.Scope.VNetSpec() - existingVnet, err := s.getExisting(ctx, vnetSpec) + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() - switch { - case err != nil && !azure.ResourceNotFound(err): - return errors.Wrapf(err, "failed to get VNet %s", vnetSpec.Name) + vnetSpec := s.Scope.VNetSpec() - case err == nil: - // vnet already exists, cannot update since it's immutable - if !existingVnet.IsManaged(s.Scope.ClusterName()) { - log.V(2).Info("Working on custom VNet", "vnet-id", existingVnet.ID) + result, err := s.CreateResource(ctx, vnetSpec, serviceName) + s.Scope.UpdatePutStatus(infrav1.VNetReadyCondition, serviceName, err) + if err == nil && result != nil { + existingVnet, ok := result.(network.VirtualNetwork) + if !ok { + return errors.Errorf("%T is not a network.VirtualNetwork", result) } - existingVnet.DeepCopyInto(s.Scope.Vnet()) - - default: - log.V(2).Info("creating VNet", "VNet", vnetSpec.Name) - - vnetProperties := network.VirtualNetwork{ - Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{ - ClusterName: s.Scope.ClusterName(), - Lifecycle: infrav1.ResourceLifecycleOwned, - Name: to.StringPtr(vnetSpec.Name), - Role: to.StringPtr(infrav1.CommonRole), - Additional: s.Scope.AdditionalTags(), - })), - Location: to.StringPtr(s.Scope.Location()), - VirtualNetworkPropertiesFormat: &network.VirtualNetworkPropertiesFormat{ - AddressSpace: &network.AddressSpace{ - AddressPrefixes: &vnetSpec.CIDRs, - }, - }, - } - - err = s.Client.CreateOrUpdate(ctx, vnetSpec.ResourceGroup, vnetSpec.Name, vnetProperties) + vnet := s.Scope.Vnet() + vnet.ID = to.String(existingVnet.ID) + vnet.Tags = converters.MapToTags(existingVnet.Tags) - if err != nil { - return errors.Wrapf(err, "failed to create virtual network %s", vnetSpec.Name) + var prefixes []string + if existingVnet.VirtualNetworkPropertiesFormat != nil && existingVnet.VirtualNetworkPropertiesFormat.AddressSpace != nil { + prefixes = to.StringSlice(existingVnet.VirtualNetworkPropertiesFormat.AddressSpace.AddressPrefixes) } - log.V(2).Info("successfully created VNet", "VNet", vnetSpec.Name) + vnet.CIDRBlocks = prefixes } - - return nil + return err } -// Delete deletes the virtual network with the provided name. +// Delete deletes the virtual network if it is managed by capz. func (s *Service) Delete(ctx context.Context) error { ctx, log, done := tele.StartSpanWithLogger(ctx, "virtualnetworks.Service.Delete") defer done() - vnetSpec := s.Scope.VNetSpec() - existingVnet, err := s.getExisting(ctx, vnetSpec) - if azure.ResourceNotFound(err) { - // vnet does not exist, there is nothing to delete - return nil - } + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() - if !existingVnet.IsManaged(s.Scope.ClusterName()) { - log.V(4).Info("Skipping VNet deletion in custom vnet mode") - return nil - } + vnetSpec := s.Scope.VNetSpec() - log.V(2).Info("deleting VNet", "VNet", vnetSpec.Name) - err = s.Client.Delete(ctx, vnetSpec.ResourceGroup, vnetSpec.Name) + // Check that the vnet is not BYO. + managed, err := s.IsManaged(ctx, vnetSpec) if err != nil { - if azure.ResourceGroupNotFound(err) || azure.ResourceNotFound(err) { + if azure.ResourceNotFound(err) { + // already deleted or doesn't exist, cleanup status and return. + s.Scope.DeleteLongRunningOperationState(vnetSpec.ResourceName(), serviceName) + s.Scope.UpdateDeleteStatus(infrav1.VNetReadyCondition, serviceName, nil) return nil } + return errors.Wrap(err, "could not get VNet management state") } - if err != nil { - return errors.Wrapf(err, "failed to delete VNet %s in resource group %s", vnetSpec.Name, vnetSpec.ResourceGroup) + if !managed { + log.Info("Skipping VNet deletion in custom vnet mode") + return nil } - log.V(2).Info("successfully deleted VNet", "VNet", vnetSpec.Name) - return nil + err = s.DeleteResource(ctx, vnetSpec, serviceName) + s.Scope.UpdateDeleteStatus(infrav1.VNetReadyCondition, serviceName, err) + return err } -// getExisting provides information about an existing virtual network. -func (s *Service) getExisting(ctx context.Context, spec azure.VNetSpec) (*infrav1.VnetSpec, error) { - ctx, log, done := tele.StartSpanWithLogger(ctx, "virtualnetworks.Service.getExisting") +// IsManaged returns true if the virtual network has an owned tag with the cluster name as value, +// meaning that the vnet's lifecycle is managed. +func (s *Service) IsManaged(ctx context.Context, spec azure.ResourceSpecGetter) (bool, error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualnetworks.Service.IsManaged") defer done() - vnet, err := s.Client.Get(ctx, spec.ResourceGroup, spec.Name) + if spec == nil { + return false, errors.New("cannot get vnet to check if it is managed: spec is nil") + } + + vnetIface, err := s.Get(ctx, spec) if err != nil { - if azure.ResourceNotFound(err) { - log.V(2).Info(fmt.Sprintf("Resource not found for VNet %q from resource group %q", spec.Name, spec.ResourceGroup)) - return nil, err - } - return nil, errors.Wrapf(err, "failed to get VNet %s", spec.Name) + return false, err } - var prefixes []string - if vnet.VirtualNetworkPropertiesFormat != nil && vnet.VirtualNetworkPropertiesFormat.AddressSpace != nil { - prefixes = to.StringSlice(vnet.VirtualNetworkPropertiesFormat.AddressSpace.AddressPrefixes) + vnet, ok := vnetIface.(network.VirtualNetwork) + if !ok { + return false, errors.Errorf("%T is not a network.VirtualNetwork", vnetIface) } - - return &infrav1.VnetSpec{ - ResourceGroup: spec.ResourceGroup, - ID: to.String(vnet.ID), - Name: to.String(vnet.Name), - CIDRBlocks: prefixes, - Peerings: s.Scope.Vnet().Peerings, - Tags: converters.MapToTags(vnet.Tags), - }, nil + tags := converters.MapToTags(vnet.Tags) + return tags.HasOwned(s.Scope.ClusterName()), nil } diff --git a/azure/services/virtualnetworks/virtualnetworks_test.go b/azure/services/virtualnetworks/virtualnetworks_test.go index 55bc78f2c54..fcdb76a863f 100644 --- a/azure/services/virtualnetworks/virtualnetworks_test.go +++ b/azure/services/virtualnetworks/virtualnetworks_test.go @@ -28,209 +28,134 @@ import ( . "github.com/onsi/gomega" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/async/mock_async" "sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualnetworks/mock_virtualnetworks" gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" ) +var ( + fakeVNetSpec = VNetSpec{ + ResourceGroup: "test-group", + Name: "test-vnet", + CIDRs: []string{"10.0.0.0/8"}, + Location: "test-location", + ClusterName: "test-cluster", + AdditionalTags: map[string]string{"foo": "bar"}, + } + managedVnet = network.VirtualNetwork{ + ID: to.StringPtr("/subscriptions/subscription/resourceGroups/test-group/providers/Microsoft.Network/virtualNetworks/test-vnet"), + Name: to.StringPtr("test-vnet"), + Tags: map[string]*string{ + "foo": to.StringPtr("bar"), + "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": to.StringPtr("owned"), + }, + } + customVnet = network.VirtualNetwork{ + ID: to.StringPtr("/subscriptions/subscription/resourceGroups/test-group/providers/Microsoft.Network/virtualNetworks/test-vnet"), + Name: to.StringPtr("test-vnet"), + Tags: map[string]*string{ + "foo": to.StringPtr("bar"), + "something": to.StringPtr("else"), + }, + } + internalError = autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error") +) + func TestReconcileVnet(t *testing.T) { testcases := []struct { name string expectedError string - expect func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_virtualnetworks.MockClientMockRecorder) + expect func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_async.MockGetterMockRecorder, r *mock_async.MockReconcilerMockRecorder) }{ { - name: "managed vnet exists", + name: "create vnet succeeds, should not return an error", expectedError: "", - expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_virtualnetworks.MockClientMockRecorder) { - s.ClusterName().AnyTimes().Return("fake-cluster") - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "vnet-exists"}) - s.VNetSpec().Return(azure.VNetSpec{ - ResourceGroup: "my-rg", - Name: "vnet-exists", - CIDRs: []string{"10.0.0.0/8"}, - }) - m.Get(gomockinternal.AContext(), "my-rg", "vnet-exists"). - Return(network.VirtualNetwork{ - ID: to.StringPtr("azure/fake/id"), - Name: to.StringPtr("vnet-exists"), - VirtualNetworkPropertiesFormat: &network.VirtualNetworkPropertiesFormat{ - AddressSpace: &network.AddressSpace{ - AddressPrefixes: to.StringSlicePtr([]string{"10.0.0.0/8"}), - }, - }, - Tags: map[string]*string{ - "Name": to.StringPtr("vnet-exists"), - "sigs.k8s.io_cluster-api-provider-azure_cluster_fake-cluster": to.StringPtr("owned"), - "sigs.k8s.io_cluster-api-provider-azure_role": to.StringPtr("common"), - }, - }, nil) + expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_async.MockGetterMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.VNetSpec().Return(&fakeVNetSpec) + r.CreateResource(gomockinternal.AContext(), &fakeVNetSpec, serviceName).Return(nil, nil) + s.UpdatePutStatus(infrav1.VNetReadyCondition, serviceName, nil) }, }, { - name: "managed ipv6 vnet exists", - expectedError: "", - expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_virtualnetworks.MockClientMockRecorder) { - s.ClusterName().AnyTimes().Return("fake-cluster") - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "vnet-exists"}) - s.VNetSpec().Return(azure.VNetSpec{ - ResourceGroup: "my-rg", - Name: "ipv6-vnet-exists", - CIDRs: []string{"10.0.0.0/8", "2001:1234:5678:9a00::/56"}, - }) - m.Get(gomockinternal.AContext(), "my-rg", "ipv6-vnet-exists"). - Return(network.VirtualNetwork{ - ID: to.StringPtr("azure/fake/id"), - Name: to.StringPtr("ipv6-vnet-exists"), - VirtualNetworkPropertiesFormat: &network.VirtualNetworkPropertiesFormat{ - AddressSpace: &network.AddressSpace{ - AddressPrefixes: to.StringSlicePtr([]string{ - "10.0.0.0/8", - "2001:1234:5678:9a00::/56", - }), - }, - }, - Tags: map[string]*string{ - "Name": to.StringPtr("ipv6-vnet-exists"), - "sigs.k8s.io_cluster-api-provider-azure_cluster_fake-cluster": to.StringPtr("owned"), - "sigs.k8s.io_cluster-api-provider-azure_role": to.StringPtr("common"), - }, - }, nil) + name: "create vnet fails, should return an error", + expectedError: internalError.Error(), + expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_async.MockGetterMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.VNetSpec().Return(&fakeVNetSpec) + r.CreateResource(gomockinternal.AContext(), &fakeVNetSpec, serviceName).Return(nil, internalError) + s.UpdatePutStatus(infrav1.VNetReadyCondition, serviceName, internalError) }, }, - { - name: "vnet created successufuly", - expectedError: "", - expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_virtualnetworks.MockClientMockRecorder) { - s.ClusterName().AnyTimes().Return("fake-cluster") - s.Location().AnyTimes().Return("fake-location") - s.AdditionalTags().AnyTimes().Return(infrav1.Tags{}) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "vnet-new"}) - s.VNetSpec().Return(azure.VNetSpec{ - ResourceGroup: "my-rg", - Name: "vnet-new", - CIDRs: []string{"10.0.0.0/8"}, - }) - m.Get(gomockinternal.AContext(), "my-rg", "vnet-new"). - Return(network.VirtualNetwork{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) + } - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "vnet-new", gomock.AssignableToTypeOf(network.VirtualNetwork{})) - }, - }, - { - name: "ipv6 vnet created successufuly", - expectedError: "", - expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_virtualnetworks.MockClientMockRecorder) { - s.ClusterName().AnyTimes().Return("fake-cluster") - s.Location().AnyTimes().Return("fake-location") - s.AdditionalTags().AnyTimes().Return(infrav1.Tags{}) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "vnet-new"}) - s.VNetSpec().Return(azure.VNetSpec{ - ResourceGroup: "my-rg", - Name: "vnet-ipv6-new", - CIDRs: []string{"10.0.0.0/8", "2001:1234:5678:9a00::/56"}, - }) - m.Get(gomockinternal.AContext(), "my-rg", "vnet-ipv6-new"). - Return(network.VirtualNetwork{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) + for _, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "vnet-ipv6-new", gomockinternal.DiffEq(network.VirtualNetwork{ - Tags: map[string]*string{ - "Name": to.StringPtr("vnet-ipv6-new"), - "sigs.k8s.io_cluster-api-provider-azure_cluster_fake-cluster": to.StringPtr(string(infrav1.ResourceLifecycleOwned)), - "sigs.k8s.io_cluster-api-provider-azure_role": to.StringPtr(infrav1.CommonRole), - }, - Location: to.StringPtr("fake-location"), - VirtualNetworkPropertiesFormat: &network.VirtualNetworkPropertiesFormat{ - AddressSpace: &network.AddressSpace{ - AddressPrefixes: to.StringSlicePtr([]string{ - "10.0.0.0/8", - "2001:1234:5678:9a00::/56", - }), - }, - }, - })) - }, - }, - { - name: "unmanaged vnet exists", - expectedError: "", - expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_virtualnetworks.MockClientMockRecorder) { - s.ClusterName().AnyTimes().Return("fake-cluster") - s.Location().AnyTimes().Return("fake-location") - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "custom-vnet"}) - s.VNetSpec().Return(azure.VNetSpec{ - ResourceGroup: "custom-vnet-rg", - Name: "custom-vnet", - CIDRs: []string{"10.0.0.0/16"}, - }) - m.Get(gomockinternal.AContext(), "custom-vnet-rg", "custom-vnet"). - Return(network.VirtualNetwork{ - ID: to.StringPtr("azure/custom-vnet/id"), - Name: to.StringPtr("custom-vnet"), - VirtualNetworkPropertiesFormat: &network.VirtualNetworkPropertiesFormat{ - AddressSpace: &network.AddressSpace{ - AddressPrefixes: to.StringSlicePtr([]string{"10.0.0.0/16"}), - }, - }, - Tags: map[string]*string{ - "Name": to.StringPtr("my-custom-vnet"), - }, - }, nil) - }, - }, + t.Parallel() + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + scopeMock := mock_virtualnetworks.NewMockVNetScope(mockCtrl) + getterMock := mock_async.NewMockGetter(mockCtrl) + reconcilerMock := mock_async.NewMockReconciler(mockCtrl) + + tc.expect(scopeMock.EXPECT(), getterMock.EXPECT(), reconcilerMock.EXPECT()) + + s := &Service{ + Scope: scopeMock, + Getter: getterMock, + Reconciler: reconcilerMock, + } + + err := s.Reconcile(context.TODO()) + if tc.expectedError != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tc.expectedError)) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + }) + } +} + +func TestDeleteVnet(t *testing.T) { + testcases := []struct { + name string + expectedError string + expect func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_async.MockGetterMockRecorder, r *mock_async.MockReconcilerMockRecorder) + }{ { - name: "custom vnet not found", + name: "delete vnet succeeds, should not return an error", expectedError: "", - expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_virtualnetworks.MockClientMockRecorder) { - s.ClusterName().AnyTimes().Return("fake-cluster") - s.Location().AnyTimes().Return("fake-location") - s.AdditionalTags().AnyTimes().Return(infrav1.Tags{}) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "custom-vnet"}) - s.VNetSpec().Return(azure.VNetSpec{ - ResourceGroup: "custom-vnet-rg", - Name: "custom-vnet", - CIDRs: []string{"10.0.0.0/16"}, - }) - m.Get(gomockinternal.AContext(), "custom-vnet-rg", "custom-vnet"). - Return(network.VirtualNetwork{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - - m.CreateOrUpdate(gomockinternal.AContext(), "custom-vnet-rg", "custom-vnet", gomock.AssignableToTypeOf(network.VirtualNetwork{})) + expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_async.MockGetterMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.VNetSpec().Return(&fakeVNetSpec) + m.Get(gomockinternal.AContext(), &fakeVNetSpec).Return(managedVnet, nil) + s.ClusterName().Return("test-cluster") + r.DeleteResource(gomockinternal.AContext(), &fakeVNetSpec, serviceName).Return(nil) + s.UpdateDeleteStatus(infrav1.VNetReadyCondition, serviceName, nil) }, }, { - name: "failed to fetch vnet", - expectedError: "failed to get VNet custom-vnet: failed to get VNet custom-vnet: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_virtualnetworks.MockClientMockRecorder) { - s.ClusterName().AnyTimes().Return("fake-cluster") - s.VNetSpec().Return(azure.VNetSpec{ - ResourceGroup: "custom-vnet-rg", - Name: "custom-vnet", - CIDRs: []string{"10.0.0.0/16"}, - }) - m.Get(gomockinternal.AContext(), "custom-vnet-rg", "custom-vnet"). - Return(network.VirtualNetwork{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) + name: "delete vnet fails, should return an error", + expectedError: internalError.Error(), + expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_async.MockGetterMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.VNetSpec().Return(&fakeVNetSpec) + m.Get(gomockinternal.AContext(), &fakeVNetSpec).Return(managedVnet, nil) + s.ClusterName().Return("test-cluster") + r.DeleteResource(gomockinternal.AContext(), &fakeVNetSpec, serviceName).Return(internalError) + s.UpdateDeleteStatus(infrav1.VNetReadyCondition, serviceName, internalError) }, }, { - name: "fail to create vnet", - expectedError: "failed to create virtual network custom-vnet: #: Internal Server Honk: StatusCode=500", - expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_virtualnetworks.MockClientMockRecorder) { - s.ClusterName().AnyTimes().Return("fake-cluster") - s.Location().AnyTimes().Return("fake-location") - s.AdditionalTags().AnyTimes().Return(infrav1.Tags{}) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "custom-vnet"}) - s.VNetSpec().Return(azure.VNetSpec{ - ResourceGroup: "custom-vnet-rg", - Name: "custom-vnet", - CIDRs: []string{"10.0.0.0/16"}, - }) - m.Get(gomockinternal.AContext(), "custom-vnet-rg", "custom-vnet"). - Return(network.VirtualNetwork{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - - m.CreateOrUpdate(gomockinternal.AContext(), "custom-vnet-rg", "custom-vnet", gomock.AssignableToTypeOf(network.VirtualNetwork{})).Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Honk")) + name: "vnet is not managed, do nothing", + expectedError: "", + expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_async.MockGetterMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.VNetSpec().Return(&fakeVNetSpec) + m.Get(gomockinternal.AContext(), &fakeVNetSpec).Return(customVnet, nil) + s.ClusterName().Return("test-cluster") }, }, } - for _, tc := range testcases { tc := tc t.Run(tc.name, func(t *testing.T) { @@ -240,16 +165,18 @@ func TestReconcileVnet(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() scopeMock := mock_virtualnetworks.NewMockVNetScope(mockCtrl) - clientMock := mock_virtualnetworks.NewMockClient(mockCtrl) + getterMock := mock_async.NewMockGetter(mockCtrl) + reconcilerMock := mock_async.NewMockReconciler(mockCtrl) - tc.expect(scopeMock.EXPECT(), clientMock.EXPECT()) + tc.expect(scopeMock.EXPECT(), getterMock.EXPECT(), reconcilerMock.EXPECT()) s := &Service{ - Scope: scopeMock, - Client: clientMock, + Scope: scopeMock, + Getter: getterMock, + Reconciler: reconcilerMock, } - err := s.Reconcile(context.TODO()) + err := s.Delete(context.TODO()) if tc.expectedError != "" { g.Expect(err).To(HaveOccurred()) g.Expect(err).To(MatchError(tc.expectedError)) @@ -260,130 +187,48 @@ func TestReconcileVnet(t *testing.T) { } } -func TestDeleteVnet(t *testing.T) { +func TestIsVnetManaged(t *testing.T) { testcases := []struct { name string + vnetSpec azure.ResourceSpecGetter expectedError string - expect func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_virtualnetworks.MockClientMockRecorder) + result bool + expect func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_async.MockGetterMockRecorder) }{ { - name: "managed vnet exists", - expectedError: "", - expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_virtualnetworks.MockClientMockRecorder) { - s.ClusterName().AnyTimes().Return("fake-cluster") - s.Location().AnyTimes().Return("fake-location") - s.AdditionalTags().AnyTimes().Return(infrav1.Tags{ - "Name": "vnet-exists", - "sigs.k8s.io_cluster-api-provider-azure_cluster_fake-cluster": "owned", - "sigs.k8s.io_cluster-api-provider-azure_role": "common", - }) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "vnet-exists"}) - s.VNetSpec().Return(azure.VNetSpec{ - ResourceGroup: "my-rg", - Name: "vnet-exists", - CIDRs: []string{"10.0.0.0/16"}, - }) - m.Get(gomockinternal.AContext(), "my-rg", "vnet-exists"). - Return(network.VirtualNetwork{ - ID: to.StringPtr("azure/fake/id"), - Name: to.StringPtr("vnet-exists"), - VirtualNetworkPropertiesFormat: &network.VirtualNetworkPropertiesFormat{ - AddressSpace: &network.AddressSpace{ - AddressPrefixes: to.StringSlicePtr([]string{"10.0.0.0/8"}), - }, - }, - Tags: map[string]*string{ - "Name": to.StringPtr("vnet-exists"), - "sigs.k8s.io_cluster-api-provider-azure_cluster_fake-cluster": to.StringPtr("owned"), - "sigs.k8s.io_cluster-api-provider-azure_role": to.StringPtr("common"), - }, - }, nil) - m.Delete(gomockinternal.AContext(), "my-rg", "vnet-exists") + name: "spec is nil", + vnetSpec: nil, + result: false, + expectedError: "cannot get vnet to check if it is managed: spec is nil", + expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_async.MockGetterMockRecorder) { }, }, { - name: "managed vnet already deleted", + name: "managed vnet returns true", + vnetSpec: &fakeVNetSpec, + result: true, expectedError: "", - expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_virtualnetworks.MockClientMockRecorder) { - s.ClusterName().AnyTimes().Return("fake-cluster") - s.Location().AnyTimes().Return("fake-location") - s.AdditionalTags().AnyTimes().Return(infrav1.Tags{ - "Name": "vnet-exists", - "sigs.k8s.io_cluster-api-provider-azure_cluster_fake-cluster": "owned", - "sigs.k8s.io_cluster-api-provider-azure_role": "common", - }) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "vnet-exists"}) - s.VNetSpec().Return(azure.VNetSpec{ - ResourceGroup: "my-rg", - Name: "vnet-exists", - CIDRs: []string{"10.0.0.0/16"}, - }) - m.Get(gomockinternal.AContext(), "my-rg", "vnet-exists"). - Return(network.VirtualNetwork{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) + expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_async.MockGetterMockRecorder) { + m.Get(gomockinternal.AContext(), &fakeVNetSpec).Return(managedVnet, nil) + s.ClusterName().Return("test-cluster") }, }, { - name: "unmanaged vnet", + name: "custom vnet returns false", + vnetSpec: &fakeVNetSpec, + result: false, expectedError: "", - expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_virtualnetworks.MockClientMockRecorder) { - s.ClusterName().AnyTimes().Return("fake-cluster") - s.Location().AnyTimes().Return("fake-location") - s.AdditionalTags().AnyTimes().Return(infrav1.Tags{}) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{ResourceGroup: "my-rg", Name: "my-vnet", ID: "azure/custom-vnet/id"}) - s.VNetSpec().Return(azure.VNetSpec{ - ResourceGroup: "my-rg", - Name: "my-vnet", - CIDRs: []string{"10.0.0.0/16"}, - }) - m.Get(gomockinternal.AContext(), "my-rg", "my-vnet"). - Return(network.VirtualNetwork{ - ID: to.StringPtr("azure/custom-vnet/id"), - Name: to.StringPtr("my-vnet"), - VirtualNetworkPropertiesFormat: &network.VirtualNetworkPropertiesFormat{ - AddressSpace: &network.AddressSpace{ - AddressPrefixes: to.StringSlicePtr([]string{"10.0.0.0/16"}), - }, - }, - Tags: map[string]*string{ - "Name": to.StringPtr("my-vnet"), - }, - }, nil) + expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_async.MockGetterMockRecorder) { + m.Get(gomockinternal.AContext(), &fakeVNetSpec).Return(customVnet, nil) + s.ClusterName().Return("test-cluster") }, }, { - name: "fail to delete vnet", - expectedError: "failed to delete VNet vnet-exists in resource group my-rg: #: Internal Honk Server: StatusCode=500", - expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_virtualnetworks.MockClientMockRecorder) { - s.ClusterName().AnyTimes().Return("fake-cluster") - s.Location().AnyTimes().Return("fake-location") - s.AdditionalTags().AnyTimes().Return(infrav1.Tags{ - "Name": "vnet-exists", - "sigs.k8s.io_cluster-api-provider-azure_cluster_fake-cluster": "owned", - "sigs.k8s.io_cluster-api-provider-azure_role": "common", - }) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "vnet-exists"}) - s.VNetSpec().Return(azure.VNetSpec{ - ResourceGroup: "my-rg", - Name: "vnet-exists", - CIDRs: []string{"10.0.0.0/16"}, - }) - m.Get(gomockinternal.AContext(), "my-rg", "vnet-exists"). - Return(network.VirtualNetwork{ - ID: to.StringPtr("azure/fake/id"), - Name: to.StringPtr("vnet-exists"), - VirtualNetworkPropertiesFormat: &network.VirtualNetworkPropertiesFormat{ - AddressSpace: &network.AddressSpace{ - AddressPrefixes: to.StringSlicePtr([]string{"10.0.0.0/8"}), - }, - }, - Tags: map[string]*string{ - "Name": to.StringPtr("vnet-exists"), - "sigs.k8s.io_cluster-api-provider-azure_cluster_fake-cluster": to.StringPtr("owned"), - "sigs.k8s.io_cluster-api-provider-azure_role": to.StringPtr("common"), - }, - }, nil) - m.Delete(gomockinternal.AContext(), "my-rg", "vnet-exists"). - Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Honk Server")) + name: "GET fails returns an error", + vnetSpec: &fakeVNetSpec, + expectedError: internalError.Error(), + expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_async.MockGetterMockRecorder) { + m.Get(gomockinternal.AContext(), &fakeVNetSpec).Return(network.VirtualNetwork{}, internalError) }, }, } @@ -396,21 +241,22 @@ func TestDeleteVnet(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() scopeMock := mock_virtualnetworks.NewMockVNetScope(mockCtrl) - clientMock := mock_virtualnetworks.NewMockClient(mockCtrl) + getterMock := mock_async.NewMockGetter(mockCtrl) - tc.expect(scopeMock.EXPECT(), clientMock.EXPECT()) + tc.expect(scopeMock.EXPECT(), getterMock.EXPECT()) s := &Service{ Scope: scopeMock, - Client: clientMock, + Getter: getterMock, } - err := s.Delete(context.TODO()) + result, err := s.IsManaged(context.TODO(), tc.vnetSpec) if tc.expectedError != "" { g.Expect(err).To(HaveOccurred()) g.Expect(err).To(MatchError(tc.expectedError)) } else { g.Expect(err).NotTo(HaveOccurred()) + g.Expect(result).To(Equal(tc.result)) } }) } diff --git a/azure/types.go b/azure/types.go index 0b9db5e6081..1c94b271d79 100644 --- a/azure/types.go +++ b/azure/types.go @@ -42,14 +42,6 @@ type SubnetSpec struct { NatGatewayName string } -// VNetSpec defines the specification for a Virtual Network. -type VNetSpec struct { - ResourceGroup string - Name string - CIDRs []string - Peerings []infrav1.VnetPeeringSpec -} - // RoleAssignmentSpec defines the specification for a Role Assignment. type RoleAssignmentSpec struct { MachineName string