diff --git a/api/v1alpha3/azurecluster_conversion.go b/api/v1alpha3/azurecluster_conversion.go index 60103e5c3f9..93c4f64e2b9 100644 --- a/api/v1alpha3/azurecluster_conversion.go +++ b/api/v1alpha3/azurecluster_conversion.go @@ -95,6 +95,8 @@ func (src *AzureCluster) ConvertTo(dstRaw conversion.Hub) error { // nolint } } + dst.Status.LongRunningOperationStates = restored.Status.LongRunningOperationStates + return nil } diff --git a/api/v1alpha3/azuremachine_conversion.go b/api/v1alpha3/azuremachine_conversion.go index c42e8ea6587..2119172629d 100644 --- a/api/v1alpha3/azuremachine_conversion.go +++ b/api/v1alpha3/azuremachine_conversion.go @@ -53,6 +53,8 @@ func (src *AzureMachine) ConvertTo(dstRaw conversion.Hub) error { // nolint dst.Spec.SubnetName = restored.Spec.SubnetName + dst.Status.LongRunningOperationStates = restored.Status.LongRunningOperationStates + return nil } diff --git a/api/v1alpha3/zz_generated.conversion.go b/api/v1alpha3/zz_generated.conversion.go index 650c437ba9c..904a328d52e 100644 --- a/api/v1alpha3/zz_generated.conversion.go +++ b/api/v1alpha3/zz_generated.conversion.go @@ -225,16 +225,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*Future)(nil), (*v1alpha4.Future)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha3_Future_To_v1alpha4_Future(a.(*Future), b.(*v1alpha4.Future), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1alpha4.Future)(nil), (*Future)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha4_Future_To_v1alpha3_Future(a.(*v1alpha4.Future), b.(*Future), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*Image)(nil), (*v1alpha4.Image)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha3_Image_To_v1alpha4_Image(a.(*Image), b.(*v1alpha4.Image), scope) }); err != nil { @@ -340,6 +330,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*Future)(nil), (*v1alpha4.Future)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha3_Future_To_v1alpha4_Future(a.(*Future), b.(*v1alpha4.Future), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*IngressRule)(nil), (*v1alpha4.SecurityRule)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha3_IngressRule_To_v1alpha4_SecurityRule(a.(*IngressRule), b.(*v1alpha4.SecurityRule), scope) }); err != nil { @@ -410,6 +405,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1alpha4.Future)(nil), (*Future)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha4_Future_To_v1alpha3_Future(a.(*v1alpha4.Future), b.(*Future), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1alpha4.LoadBalancerSpec)(nil), (*LoadBalancerSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha4_LoadBalancerSpec_To_v1alpha3_LoadBalancerSpec(a.(*v1alpha4.LoadBalancerSpec), b.(*LoadBalancerSpec), scope) }); err != nil { diff --git a/api/v1alpha4/azurecluster_types.go b/api/v1alpha4/azurecluster_types.go index 1f11aacf031..ee7c42b71d4 100644 --- a/api/v1alpha4/azurecluster_types.go +++ b/api/v1alpha4/azurecluster_types.go @@ -105,7 +105,9 @@ type AzureClusterStatus struct { // +kubebuilder:object:root=true // +kubebuilder:printcolumn:name="Cluster",type="string",JSONPath=".metadata.labels.cluster\\.x-k8s\\.io/cluster-name",description="Cluster to which this AzureCluster belongs" -// +kubebuilder:printcolumn:name="Ready",type="boolean",JSONPath=".status.ready" +// +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status" +// +kubebuilder:printcolumn:name="Reason",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].reason" +// +kubebuilder:printcolumn:name="Message",type="string",priority=1,JSONPath=".status.conditions[?(@.type=='Ready')].message" // +kubebuilder:printcolumn:name="Resource Group",type="string",priority=1,JSONPath=".spec.resourceGroup" // +kubebuilder:printcolumn:name="SubscriptionID",type="string",priority=1,JSONPath=".spec.subscriptionID" // +kubebuilder:printcolumn:name="Location",type="string",priority=1,JSONPath=".spec.location" diff --git a/api/v1alpha4/conditions_consts.go b/api/v1alpha4/conditions_consts.go index 90045df2a41..bdcdf1eecc7 100644 --- a/api/v1alpha4/conditions_consts.go +++ b/api/v1alpha4/conditions_consts.go @@ -20,8 +20,6 @@ import clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" // AzureCluster Conditions and Reasons. const ( - // NetworkInfrastructureReadyCondition reports of current status of cluster infrastructure. - NetworkInfrastructureReadyCondition clusterv1.ConditionType = "NetworkInfrastructureReady" // NamespaceNotAllowedByIdentity used to indicate cluster in a namespace not allowed by identity. NamespaceNotAllowedByIdentity = "NamespaceNotAllowedByIdentity" ) @@ -115,4 +113,6 @@ const ( DeletedReason = "Deleted" // DeletionFailedReason means the resource failed to be deleted. DeletionFailedReason = "DeletionFailed" + // UpdatingReason means the resource is being updated. + UpdatingReason = "Updating" ) diff --git a/api/v1alpha4/types.go b/api/v1alpha4/types.go index 58c607a721d..0636e4989ec 100644 --- a/api/v1alpha4/types.go +++ b/api/v1alpha4/types.go @@ -31,6 +31,15 @@ const ( type Futures []Future +const ( + // PatchFuture is a future that was derived from a PATCH request. + PatchFuture string = "PATCH" + // PutFuture is a future that was derived from a PUT request. + PutFuture string = "PUT" + // DeleteFuture is a future that was derived from a DELETE request. + DeleteFuture string = "DELETE" +) + // Future contains the data needed for an Azure long-running operation to continue across reconcile loops. type Future struct { // Type describes the type of future, such update, create, delete, etc. diff --git a/azure/converters/futures.go b/azure/converters/futures.go index 29e87cbe84b..afafa12f2f6 100644 --- a/azure/converters/futures.go +++ b/azure/converters/futures.go @@ -18,22 +18,12 @@ package converters import ( "encoding/base64" - "encoding/json" azureautorest "github.com/Azure/go-autorest/autorest/azure" "github.com/pkg/errors" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" ) -const ( - // PatchFuture is a future that was derived from a PATCH request. - PatchFuture string = "PATCH" - // PutFuture is a future that was derived from a PUT request. - PutFuture string = "PUT" - // DeleteFuture is a future that was derived from a DELETE request. - DeleteFuture string = "DELETE" -) - func SDKToFuture(future azureautorest.FutureAPI, futureType, service, resourceName, rgName string) (*infrav1.Future, error) { jsonData, err := future.MarshalJSON() if err != nil { @@ -49,14 +39,14 @@ func SDKToFuture(future azureautorest.FutureAPI, futureType, service, resourceNa }, nil } -func FutureToSDK(future *infrav1.Future) (azureautorest.FutureAPI, error) { +func FutureToSDK(future infrav1.Future) (azureautorest.FutureAPI, error) { futureData, err := base64.URLEncoding.DecodeString(future.Data) if err != nil { return nil, errors.Wrap(err, "failed to base64 decode future data") } - var genericFuture azureautorest.FutureAPI - if err := json.Unmarshal(futureData, &genericFuture); err != nil { + var genericFuture azureautorest.Future + if err := genericFuture.UnmarshalJSON(futureData); err != nil { return nil, errors.Wrap(err, "failed to unmarshal future data") } - return genericFuture, nil + return &genericFuture, nil } diff --git a/azure/converters/futures_test.go b/azure/converters/futures_test.go new file mode 100644 index 00000000000..8058be7b215 --- /dev/null +++ b/azure/converters/futures_test.go @@ -0,0 +1,108 @@ +/* +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 converters + +import ( + "testing" + + azureautorest "github.com/Azure/go-autorest/autorest/azure" + "github.com/onsi/gomega" + + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" +) + +var ( + validFuture = infrav1.Future{ + Type: infrav1.DeleteFuture, + ServiceName: "test-service", + Name: "test-group", + ResourceGroup: "test-group", + Data: "eyJtZXRob2QiOiJERUxFVEUiLCJwb2xsaW5nTWV0aG9kIjoiTG9jYXRpb24iLCJscm9TdGF0ZSI6IkluUHJvZ3Jlc3MifQ==", + } + + emptyDataFuture = infrav1.Future{ + Type: infrav1.DeleteFuture, + ServiceName: "test-service", + Name: "test-group", + ResourceGroup: "test-group", + Data: "", + } + + decodedDataFuture = infrav1.Future{ + Type: infrav1.DeleteFuture, + ServiceName: "test-service", + Name: "test-group", + ResourceGroup: "test-group", + Data: "this is not b64 encoded", + } + + invalidFuture = infrav1.Future{ + Type: infrav1.DeleteFuture, + ServiceName: "test-service", + Name: "test-group", + ResourceGroup: "test-group", + Data: "ZmFrZSBiNjQgZnV0dXJlIGRhdGEK", + } +) + +func Test_FutureToSDK(t *testing.T) { + cases := []struct { + name string + future infrav1.Future + expect func(*gomega.GomegaWithT, azureautorest.FutureAPI, error) + }{ + { + name: "data is empty", + future: emptyDataFuture, + expect: func(g *gomega.GomegaWithT, f azureautorest.FutureAPI, err error) { + g.Expect(err.Error()).Should(gomega.ContainSubstring("failed to unmarshal future data")) + }, + }, + { + name: "data is not base64 encoded", + future: decodedDataFuture, + expect: func(g *gomega.GomegaWithT, f azureautorest.FutureAPI, err error) { + g.Expect(err.Error()).Should(gomega.ContainSubstring("failed to base64 decode future data")) + }, + }, + { + name: "base64 data is not a valid future", + future: invalidFuture, + expect: func(g *gomega.GomegaWithT, f azureautorest.FutureAPI, err error) { + g.Expect(err.Error()).Should(gomega.ContainSubstring("failed to unmarshal future data")) + }, + }, + { + name: "valid future data", + future: validFuture, + expect: func(g *gomega.GomegaWithT, f azureautorest.FutureAPI, err error) { + g.Expect(err).Should(gomega.BeNil()) + g.Expect(f).Should(gomega.BeAssignableToTypeOf(&azureautorest.Future{})) + }, + }, + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + t.Parallel() + g := gomega.NewGomegaWithT(t) + result, err := FutureToSDK(c.future) + c.expect(g, result, err) + }) + } +} diff --git a/azure/errors.go b/azure/errors.go index 56360487e01..f5474d393f6 100644 --- a/azure/errors.go +++ b/azure/errors.go @@ -107,8 +107,8 @@ func (t ReconcileError) IsTerminal() bool { return t.errorType == TerminalErrorType } -// Is returns true if the target is a ReconcileError. -func (t ReconcileError) Is(target error) bool { +// IsReconcileError returns true if the target is a ReconcileError. +func IsReconcileError(target error) bool { return errors.As(target, &ReconcileError{}) } @@ -133,8 +133,8 @@ type OperationNotDoneError struct { } // NewOperationNotDoneError returns a new OperationNotDoneError wrapping a Future. -func NewOperationNotDoneError(future *infrav1.Future) *OperationNotDoneError { - return &OperationNotDoneError{ +func NewOperationNotDoneError(future *infrav1.Future) OperationNotDoneError { + return OperationNotDoneError{ Future: future, } } @@ -144,7 +144,11 @@ func (onde OperationNotDoneError) Error() string { return fmt.Sprintf("operation type %s on Azure resource %s/%s is not done", onde.Future.Type, onde.Future.ResourceGroup, onde.Future.Name) } -// Is returns true if the target is an OperationNotDoneError. -func (onde OperationNotDoneError) Is(target error) bool { +// IsOperationNotDoneError returns true if the target is an OperationNotDoneError. +func IsOperationNotDoneError(target error) bool { + reconcileErr := &ReconcileError{} + if errors.As(target, reconcileErr) { + return IsOperationNotDoneError(reconcileErr.error) + } return errors.As(target, &OperationNotDoneError{}) } diff --git a/azure/interfaces.go b/azure/interfaces.go index 12b03e25e6a..e8e0f2fad54 100644 --- a/azure/interfaces.go +++ b/azure/interfaces.go @@ -54,7 +54,7 @@ type Authorizer interface { // NetworkDescriber is an interface which can get common Azure Cluster Networking information. type NetworkDescriber interface { Vnet() *infrav1.VnetSpec - IsVnetManaged() bool + IsVnetManaged(context.Context) (bool, error) ControlPlaneSubnet() infrav1.SubnetSpec Subnets() infrav1.Subnets Subnet(string) infrav1.SubnetSpec @@ -81,12 +81,14 @@ type ClusterDescriber interface { CloudProviderConfigOverrides() *infrav1.CloudProviderConfigOverrides } +// AsyncStatusUpdater is an interface used to keep track of long running operations in Status that has Conditions and Futures. type AsyncStatusUpdater interface { SetLongRunningOperationState(*infrav1.Future) GetLongRunningOperationState(string, string) *infrav1.Future DeleteLongRunningOperationState(string, string) - SetConditionTrue(clusterv1.ConditionType) - SetConditionFalse(clusterv1.ConditionType, string, clusterv1.ConditionSeverity) + UpdatePutStatus(clusterv1.ConditionType, string, error) + UpdateDeleteStatus(clusterv1.ConditionType, string, error) + UpdatePatchStatus(clusterv1.ConditionType, string, error) } // ClusterScoper combines the ClusterDescriber and NetworkDescriber interfaces. @@ -94,3 +96,18 @@ type ClusterScoper interface { ClusterDescriber NetworkDescriber } + +// ResourceSpecGetter is an interface for getting all the required information to create/update/delete an Azure resource. +type ResourceSpecGetter interface { + // ResourceName returns the name of the resource. + ResourceName() string + // OwnerResourceName returns the name of the resource that owns the resource + // in the case that the resource is an Azure subresource. + OwnerResourceName() string + // ResourceGroupName returns the name of the resource group the resource is in. + ResourceGroupName() string + // Parameters takes the existing resource and returns the desired parameters of the resource. + // If the resource does not exist, or we do not care about existing parameters to update the resource, existing should be nil. + // If no update is needed on the resource, Parameters should return nil. + Parameters(existing interface{}) (interface{}, error) +} diff --git a/azure/mocks/service_mock.go b/azure/mock_azure/azure_mock.go similarity index 89% rename from azure/mocks/service_mock.go rename to azure/mock_azure/azure_mock.go index 1e5e90abf8d..6c6222d6059 100644 --- a/azure/mocks/service_mock.go +++ b/azure/mock_azure/azure_mock.go @@ -17,8 +17,8 @@ limitations under the License. // Code generated by MockGen. DO NOT EDIT. // Source: ../interfaces.go -// Package mocks is a generated GoMock package. -package mocks +// Package mock_azure is a generated GoMock package. +package mock_azure import ( context "context" @@ -404,17 +404,18 @@ func (mr *MockNetworkDescriberMockRecorder) IsIPv6Enabled() *gomock.Call { } // IsVnetManaged mocks base method. -func (m *MockNetworkDescriber) IsVnetManaged() bool { +func (m *MockNetworkDescriber) IsVnetManaged(arg0 context.Context) (bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsVnetManaged") + ret := m.ctrl.Call(m, "IsVnetManaged", arg0) ret0, _ := ret[0].(bool) - return ret0 + ret1, _ := ret[1].(error) + return ret0, ret1 } // IsVnetManaged indicates an expected call of IsVnetManaged. -func (mr *MockNetworkDescriberMockRecorder) IsVnetManaged() *gomock.Call { +func (mr *MockNetworkDescriberMockRecorder) IsVnetManaged(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsVnetManaged", reflect.TypeOf((*MockNetworkDescriber)(nil).IsVnetManaged)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsVnetManaged", reflect.TypeOf((*MockNetworkDescriber)(nil).IsVnetManaged), arg0) } // NodeSubnets mocks base method. @@ -781,40 +782,52 @@ func (mr *MockAsyncStatusUpdaterMockRecorder) GetLongRunningOperationState(arg0, return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLongRunningOperationState", reflect.TypeOf((*MockAsyncStatusUpdater)(nil).GetLongRunningOperationState), arg0, arg1) } -// SetConditionFalse mocks base method. -func (m *MockAsyncStatusUpdater) SetConditionFalse(arg0 v1alpha40.ConditionType, arg1 string, arg2 v1alpha40.ConditionSeverity) { +// SetLongRunningOperationState mocks base method. +func (m *MockAsyncStatusUpdater) SetLongRunningOperationState(arg0 *v1alpha4.Future) { m.ctrl.T.Helper() - m.ctrl.Call(m, "SetConditionFalse", arg0, arg1, arg2) + m.ctrl.Call(m, "SetLongRunningOperationState", arg0) } -// SetConditionFalse indicates an expected call of SetConditionFalse. -func (mr *MockAsyncStatusUpdaterMockRecorder) SetConditionFalse(arg0, arg1, arg2 interface{}) *gomock.Call { +// SetLongRunningOperationState indicates an expected call of SetLongRunningOperationState. +func (mr *MockAsyncStatusUpdaterMockRecorder) SetLongRunningOperationState(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetConditionFalse", reflect.TypeOf((*MockAsyncStatusUpdater)(nil).SetConditionFalse), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLongRunningOperationState", reflect.TypeOf((*MockAsyncStatusUpdater)(nil).SetLongRunningOperationState), arg0) } -// SetConditionTrue mocks base method. -func (m *MockAsyncStatusUpdater) SetConditionTrue(arg0 v1alpha40.ConditionType) { +// UpdateDeleteStatus mocks base method. +func (m *MockAsyncStatusUpdater) UpdateDeleteStatus(arg0 v1alpha40.ConditionType, arg1 string, arg2 error) { m.ctrl.T.Helper() - m.ctrl.Call(m, "SetConditionTrue", arg0) + m.ctrl.Call(m, "UpdateDeleteStatus", arg0, arg1, arg2) } -// SetConditionTrue indicates an expected call of SetConditionTrue. -func (mr *MockAsyncStatusUpdaterMockRecorder) SetConditionTrue(arg0 interface{}) *gomock.Call { +// UpdateDeleteStatus indicates an expected call of UpdateDeleteStatus. +func (mr *MockAsyncStatusUpdaterMockRecorder) UpdateDeleteStatus(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetConditionTrue", reflect.TypeOf((*MockAsyncStatusUpdater)(nil).SetConditionTrue), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateDeleteStatus", reflect.TypeOf((*MockAsyncStatusUpdater)(nil).UpdateDeleteStatus), arg0, arg1, arg2) } -// SetLongRunningOperationState mocks base method. -func (m *MockAsyncStatusUpdater) SetLongRunningOperationState(arg0 *v1alpha4.Future) { +// UpdatePatchStatus mocks base method. +func (m *MockAsyncStatusUpdater) UpdatePatchStatus(arg0 v1alpha40.ConditionType, arg1 string, arg2 error) { m.ctrl.T.Helper() - m.ctrl.Call(m, "SetLongRunningOperationState", arg0) + m.ctrl.Call(m, "UpdatePatchStatus", arg0, arg1, arg2) } -// SetLongRunningOperationState indicates an expected call of SetLongRunningOperationState. -func (mr *MockAsyncStatusUpdaterMockRecorder) SetLongRunningOperationState(arg0 interface{}) *gomock.Call { +// UpdatePatchStatus indicates an expected call of UpdatePatchStatus. +func (mr *MockAsyncStatusUpdaterMockRecorder) UpdatePatchStatus(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLongRunningOperationState", reflect.TypeOf((*MockAsyncStatusUpdater)(nil).SetLongRunningOperationState), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePatchStatus", reflect.TypeOf((*MockAsyncStatusUpdater)(nil).UpdatePatchStatus), arg0, arg1, arg2) +} + +// UpdatePutStatus mocks base method. +func (m *MockAsyncStatusUpdater) UpdatePutStatus(arg0 v1alpha40.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 *MockAsyncStatusUpdaterMockRecorder) UpdatePutStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePutStatus", reflect.TypeOf((*MockAsyncStatusUpdater)(nil).UpdatePutStatus), arg0, arg1, arg2) } // MockClusterScoper is a mock of ClusterScoper interface. @@ -1079,17 +1092,18 @@ func (mr *MockClusterScoperMockRecorder) IsIPv6Enabled() *gomock.Call { } // IsVnetManaged mocks base method. -func (m *MockClusterScoper) IsVnetManaged() bool { +func (m *MockClusterScoper) IsVnetManaged(arg0 context.Context) (bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsVnetManaged") + ret := m.ctrl.Call(m, "IsVnetManaged", arg0) ret0, _ := ret[0].(bool) - return ret0 + ret1, _ := ret[1].(error) + return ret0, ret1 } // IsVnetManaged indicates an expected call of IsVnetManaged. -func (mr *MockClusterScoperMockRecorder) IsVnetManaged() *gomock.Call { +func (mr *MockClusterScoperMockRecorder) IsVnetManaged(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsVnetManaged", reflect.TypeOf((*MockClusterScoper)(nil).IsVnetManaged)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsVnetManaged", reflect.TypeOf((*MockClusterScoper)(nil).IsVnetManaged), arg0) } // Location mocks base method. @@ -1243,3 +1257,83 @@ func (mr *MockClusterScoperMockRecorder) Vnet() *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Vnet", reflect.TypeOf((*MockClusterScoper)(nil).Vnet)) } + +// MockResourceSpecGetter is a mock of ResourceSpecGetter interface. +type MockResourceSpecGetter struct { + ctrl *gomock.Controller + recorder *MockResourceSpecGetterMockRecorder +} + +// MockResourceSpecGetterMockRecorder is the mock recorder for MockResourceSpecGetter. +type MockResourceSpecGetterMockRecorder struct { + mock *MockResourceSpecGetter +} + +// NewMockResourceSpecGetter creates a new mock instance. +func NewMockResourceSpecGetter(ctrl *gomock.Controller) *MockResourceSpecGetter { + mock := &MockResourceSpecGetter{ctrl: ctrl} + mock.recorder = &MockResourceSpecGetterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockResourceSpecGetter) EXPECT() *MockResourceSpecGetterMockRecorder { + return m.recorder +} + +// OwnerResourceName mocks base method. +func (m *MockResourceSpecGetter) OwnerResourceName() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "OwnerResourceName") + ret0, _ := ret[0].(string) + return ret0 +} + +// OwnerResourceName indicates an expected call of OwnerResourceName. +func (mr *MockResourceSpecGetterMockRecorder) OwnerResourceName() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "OwnerResourceName", reflect.TypeOf((*MockResourceSpecGetter)(nil).OwnerResourceName)) +} + +// Parameters mocks base method. +func (m *MockResourceSpecGetter) Parameters(existing interface{}) (interface{}, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Parameters", existing) + ret0, _ := ret[0].(interface{}) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Parameters indicates an expected call of Parameters. +func (mr *MockResourceSpecGetterMockRecorder) Parameters(existing interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Parameters", reflect.TypeOf((*MockResourceSpecGetter)(nil).Parameters), existing) +} + +// ResourceGroupName mocks base method. +func (m *MockResourceSpecGetter) ResourceGroupName() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ResourceGroupName") + ret0, _ := ret[0].(string) + return ret0 +} + +// ResourceGroupName indicates an expected call of ResourceGroupName. +func (mr *MockResourceSpecGetterMockRecorder) ResourceGroupName() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResourceGroupName", reflect.TypeOf((*MockResourceSpecGetter)(nil).ResourceGroupName)) +} + +// ResourceName mocks base method. +func (m *MockResourceSpecGetter) ResourceName() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ResourceName") + ret0, _ := ret[0].(string) + return ret0 +} + +// ResourceName indicates an expected call of ResourceName. +func (mr *MockResourceSpecGetterMockRecorder) ResourceName() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResourceName", reflect.TypeOf((*MockResourceSpecGetter)(nil).ResourceName)) +} diff --git a/azure/mocks/doc.go b/azure/mock_azure/doc.go similarity index 73% rename from azure/mocks/doc.go rename to azure/mock_azure/doc.go index 92f6e009db0..baf8d4d9ac4 100644 --- a/azure/mocks/doc.go +++ b/azure/mock_azure/doc.go @@ -15,6 +15,6 @@ limitations under the License. */ // Run go generate to regenerate this mock. -//go:generate ../../hack/tools/bin/mockgen -destination service_mock.go -package mocks -source ../interfaces.go Service -//go:generate /usr/bin/env bash -c "cat ../../hack/boilerplate/boilerplate.generatego.txt service_mock.go > _service_mock.go && mv _service_mock.go service_mock.go" -package mocks //nolint +//go:generate ../../hack/tools/bin/mockgen -destination azure_mock.go -package mock_azure -source ../interfaces.go +//go:generate /usr/bin/env bash -c "cat ../../hack/boilerplate/boilerplate.generatego.txt azure_mock.go > _azure_mock.go && mv _azure_mock.go azure_mock.go" +package mock_azure //nolint diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go index 75e5cb6e720..531677c6cdf 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -36,6 +36,9 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" "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/securitygroups" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualnetworks" "sigs.k8s.io/cluster-api-provider-azure/util/futures" ) @@ -90,6 +93,7 @@ func NewClusterScope(ctx context.Context, params ClusterScopeParams) (*ClusterSc Cluster: params.Cluster, AzureCluster: params.AzureCluster, patchHelper: helper, + cache: &ClusterCache{}, }, nil } @@ -102,6 +106,12 @@ type ClusterScope struct { AzureClients Cluster *clusterv1.Cluster AzureCluster *infrav1.AzureCluster + cache *ClusterCache +} + +// ClusterCache stores common cluster information so we don't have to hit the API multiple times within the same reconcile loop. +type ClusterCache struct { + IsVnetManaged *bool } // BaseURI returns the Azure ResourceManagerEndpoint. @@ -243,12 +253,14 @@ func (s *ClusterScope) NatGatewaySpecs() []azure.NatGatewaySpec { } // NSGSpecs returns the security group specs. -func (s *ClusterScope) NSGSpecs() []azure.NSGSpec { - nsgspecs := []azure.NSGSpec{} +func (s *ClusterScope) NSGSpecs() []azure.ResourceSpecGetter { + var nsgspecs []azure.ResourceSpecGetter for _, subnet := range s.AzureCluster.Spec.NetworkSpec.Subnets { - nsgspecs = append(nsgspecs, azure.NSGSpec{ + nsgspecs = append(nsgspecs, &securitygroups.NSGSpec{ Name: subnet.SecurityGroup.Name, SecurityRules: subnet.SecurityGroup.SecurityRules, + ResourceGroup: s.ResourceGroup(), + Location: s.Location(), }) } @@ -286,12 +298,25 @@ func (s *ClusterScope) SubnetSpecs() []azure.SubnetSpec { return subnetSpecs } +// GroupSpec returns the resource group spec. +func (s *ClusterScope) GroupSpec() azure.ResourceSpecGetter { + return &groups.GroupSpec{ + Name: s.ResourceGroup(), + Location: s.Location(), + ClusterName: s.ClusterName(), + AdditionalTags: s.AdditionalTags(), + } +} + // 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(), } } @@ -336,8 +361,16 @@ func (s *ClusterScope) Vnet() *infrav1.VnetSpec { } // IsVnetManaged returns true if the vnet is managed. -func (s *ClusterScope) IsVnetManaged() bool { - return s.Vnet().ID == "" || s.Vnet().Tags.HasOwned(s.ClusterName()) +func (s *ClusterScope) IsVnetManaged(ctx context.Context) (bool, error) { + if s.cache.IsVnetManaged == nil { + vnetSvc := virtualnetworks.New(s) + if managed, err := vnetSvc.IsVnetManaged(ctx); err != nil { + return false, err + } else { + s.cache.IsVnetManaged = &managed + } + } + return to.Bool(s.cache.IsVnetManaged), nil } // IsIPv6Enabled returns true if IPv6 is enabled. @@ -541,20 +574,21 @@ func (s *ClusterScope) ListOptionsLabelSelector() client.ListOption { func (s *ClusterScope) PatchObject(ctx context.Context) error { conditions.SetSummary(s.AzureCluster, conditions.WithConditions( - infrav1.NetworkInfrastructureReadyCondition, - ), - conditions.WithStepCounterIfOnly( - infrav1.NetworkInfrastructureReadyCondition, + infrav1.ResourceGroupReadyCondition, + infrav1.VNetReadyCondition, + infrav1.SecurityGroupsReadyCondition, ), + conditions.WithStepCounterIf(s.AzureCluster.ObjectMeta.DeletionTimestamp.IsZero()), ) - // TODO (cecile): add new conditions to owned conditions return s.patchHelper.Patch( ctx, s.AzureCluster, patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{ clusterv1.ReadyCondition, - infrav1.NetworkInfrastructureReadyCondition, + infrav1.ResourceGroupReadyCondition, + infrav1.VNetReadyCondition, + infrav1.SecurityGroupsReadyCondition, }}) } @@ -697,12 +731,44 @@ func (s *ClusterScope) DeleteLongRunningOperationState(name, service string) { futures.Delete(s.AzureCluster, name, service) } -// SetConditionTrue will set a condition to true on the AzureCluster status. -func (s *ClusterScope) SetConditionTrue(condition clusterv1.ConditionType) { - conditions.MarkTrue(s.AzureCluster, condition) +// UpdateDeleteStatus updates a condition on the AzureCluster status after a DELETE operation. +func (s *ClusterScope) UpdateDeleteStatus(condition clusterv1.ConditionType, service string, err error) { + switch { + case err == nil: + conditions.MarkFalse(s.AzureCluster, condition, infrav1.DeletedReason, clusterv1.ConditionSeverityInfo, "%s successfully deleted", service) + case errors.Is(err, azure.ErrNotOwned): + // do nothing + case azure.IsOperationNotDoneError(err): + conditions.MarkFalse(s.AzureCluster, condition, infrav1.DeletingReason, clusterv1.ConditionSeverityInfo, "%s deleting", service) + default: + conditions.MarkFalse(s.AzureCluster, condition, infrav1.DeletionFailedReason, clusterv1.ConditionSeverityError, "%s failed to delete. err: %s", service, err.Error()) + } +} + +// UpdatePutStatus updates a condition on the AzureCluster status after a PUT operation. +func (s *ClusterScope) UpdatePutStatus(condition clusterv1.ConditionType, service string, err error) { + switch { + case err == nil: + conditions.MarkTrue(s.AzureCluster, condition) + case errors.Is(err, azure.ErrNotOwned): + // do nothing + case azure.IsOperationNotDoneError(err): + conditions.MarkFalse(s.AzureCluster, condition, infrav1.CreatingReason, clusterv1.ConditionSeverityInfo, "%s creating or updating", service) + default: + conditions.MarkFalse(s.AzureCluster, condition, infrav1.FailedReason, clusterv1.ConditionSeverityError, "%s failed to create or update. err: %s", service, err.Error()) + } } -// SetConditionFalse will set a condition to false on the AzureCluster status. -func (s *ClusterScope) SetConditionFalse(condition clusterv1.ConditionType, reason string, severity clusterv1.ConditionSeverity) { - conditions.MarkFalse(s.AzureCluster, condition, reason, severity, "") // TODO: add message +// UpdatePatchStatus updates a condition on the AzureCluster status after a PATCH operation. +func (s *ClusterScope) UpdatePatchStatus(condition clusterv1.ConditionType, service string, err error) { + switch { + case err == nil: + conditions.MarkTrue(s.AzureCluster, condition) + case errors.Is(err, azure.ErrNotOwned): + // do nothing + case azure.IsOperationNotDoneError(err): + conditions.MarkFalse(s.AzureCluster, condition, infrav1.UpdatingReason, clusterv1.ConditionSeverityInfo, "%s updating", service) + default: + conditions.MarkFalse(s.AzureCluster, condition, infrav1.FailedReason, clusterv1.ConditionSeverityError, "%s failed to update. err: %s", service, err.Error()) + } } diff --git a/azure/scope/machine.go b/azure/scope/machine.go index 97037df1a18..7c1ae6f55c3 100644 --- a/azure/scope/machine.go +++ b/azure/scope/machine.go @@ -579,12 +579,44 @@ func (m *MachineScope) DeleteLongRunningOperationState(name, service string) { futures.Delete(m.AzureMachine, name, service) } -// SetConditionTrue will set a condition to true on the AzureMachine status. -func (m *MachineScope) SetConditionTrue(condition clusterv1.ConditionType) { - conditions.MarkTrue(m.AzureMachine, condition) +// UpdateDeleteStatus updates a condition on the AzureMachine status after a DELETE operation. +func (s *MachineScope) UpdateDeleteStatus(condition clusterv1.ConditionType, service string, err error) { + switch { + case err == nil: + conditions.MarkFalse(s.AzureMachine, condition, infrav1.DeletedReason, clusterv1.ConditionSeverityInfo, "%s successfully deleted", service) + case errors.Is(err, azure.ErrNotOwned): + // do nothing + case azure.IsOperationNotDoneError(err): + conditions.MarkFalse(s.AzureMachine, condition, infrav1.DeletingReason, clusterv1.ConditionSeverityInfo, "%s deleting", service) + default: + conditions.MarkFalse(s.AzureMachine, condition, infrav1.DeletionFailedReason, clusterv1.ConditionSeverityError, "%s failed to delete. err: %s", service, err.Error()) + } +} + +// UpdatePutStatus updates a condition on the AzureMachine status after a PUT operation. +func (s *MachineScope) UpdatePutStatus(condition clusterv1.ConditionType, service string, err error) { + switch { + case err == nil: + conditions.MarkTrue(s.AzureMachine, condition) + case errors.Is(err, azure.ErrNotOwned): + // do nothing + case azure.IsOperationNotDoneError(err): + conditions.MarkFalse(s.AzureMachine, condition, infrav1.CreatingReason, clusterv1.ConditionSeverityInfo, "%s creating or updating", service) + default: + conditions.MarkFalse(s.AzureMachine, condition, infrav1.FailedReason, clusterv1.ConditionSeverityError, "%s failed to create or update. err: %s", service, err.Error()) + } } -// SetConditionFalse will set a condition to false on the AzureMachine status. -func (m *MachineScope) SetConditionFalse(condition clusterv1.ConditionType, reason string, severity clusterv1.ConditionSeverity) { - conditions.MarkFalse(m.AzureMachine, condition, reason, severity, "") // TODO: add message +// UpdatePatchStatus updates a condition on the AzureMachine status after a PATCH operation. +func (s *MachineScope) UpdatePatchStatus(condition clusterv1.ConditionType, service string, err error) { + switch { + case err == nil: + conditions.MarkTrue(s.AzureMachine, condition) + case errors.Is(err, azure.ErrNotOwned): + // do nothing + case azure.IsOperationNotDoneError(err): + conditions.MarkFalse(s.AzureMachine, condition, infrav1.UpdatingReason, clusterv1.ConditionSeverityInfo, "%s updating", service) + default: + conditions.MarkFalse(s.AzureMachine, condition, infrav1.FailedReason, clusterv1.ConditionSeverityError, "%s failed to update. err: %s", service, err.Error()) + } } diff --git a/azure/scope/machinepool.go b/azure/scope/machinepool.go index 1192c53fe00..5cb1a1a146d 100644 --- a/azure/scope/machinepool.go +++ b/azure/scope/machinepool.go @@ -35,6 +35,7 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" "sigs.k8s.io/cluster-api-provider-azure/azure" machinepool "sigs.k8s.io/cluster-api-provider-azure/azure/scope/strategies/machinepool_deployments" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/scalesets" infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1alpha4" "sigs.k8s.io/cluster-api-provider-azure/util/tele" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" @@ -299,8 +300,7 @@ func (m *MachinePoolScope) applyAzureMachinePoolMachines(ctx context.Context) er return nil } - // TODO: use a const for service name - if futures.Has(m.AzureMachinePool, m.Name(), "scalesets") { + if futures.Has(m.AzureMachinePool, m.Name(), scalesets.ServiceName) { m.V(4).Info("exiting early due an in-progress long running operation on the ScaleSet") // exit early to be less greedy about delete return nil @@ -626,12 +626,44 @@ func (m *MachinePoolScope) SetSubnetName() error { return nil } -// SetConditionTrue will set a condition to true on the AzureMachinePool status. -func (m *MachinePoolScope) SetConditionTrue(condition clusterv1.ConditionType) { - conditions.MarkTrue(m.AzureMachinePool, condition) +// UpdateDeleteStatus updates a condition on the AzureMachinePool status after a DELETE operation. +func (s *MachinePoolScope) UpdateDeleteStatus(condition clusterv1.ConditionType, service string, err error) { + switch { + case err == nil: + conditions.MarkFalse(s.AzureMachinePool, condition, infrav1.DeletedReason, clusterv1.ConditionSeverityInfo, "%s successfully deleted", service) + case errors.Is(err, azure.ErrNotOwned): + // do nothing + case azure.IsOperationNotDoneError(err): + conditions.MarkFalse(s.AzureMachinePool, condition, infrav1.DeletingReason, clusterv1.ConditionSeverityInfo, "%s deleting", service) + default: + conditions.MarkFalse(s.AzureMachinePool, condition, infrav1.DeletionFailedReason, clusterv1.ConditionSeverityError, "%s failed to delete. err: %s", service, err.Error()) + } } -// SetConditionFalse will set a condition to false on the AzureMachinePool status. -func (m *MachinePoolScope) SetConditionFalse(condition clusterv1.ConditionType, reason string, severity clusterv1.ConditionSeverity) { - conditions.MarkFalse(m.AzureMachinePool, condition, reason, severity, "") // TODO: add message +// UpdatePutStatus updates a condition on the AzureMachinePool status after a PUT operation. +func (s *MachinePoolScope) UpdatePutStatus(condition clusterv1.ConditionType, service string, err error) { + switch { + case err == nil: + conditions.MarkTrue(s.AzureMachinePool, condition) + case errors.Is(err, azure.ErrNotOwned): + // do nothing + case azure.IsOperationNotDoneError(err): + conditions.MarkFalse(s.AzureMachinePool, condition, infrav1.CreatingReason, clusterv1.ConditionSeverityInfo, "%s creating or updating", service) + default: + conditions.MarkFalse(s.AzureMachinePool, condition, infrav1.FailedReason, clusterv1.ConditionSeverityError, "%s failed to create or update. err: %s", service, err.Error()) + } +} + +// UpdatePatchStatus updates a condition on the AzureMachinePool status after a PATCH operation. +func (s *MachinePoolScope) UpdatePatchStatus(condition clusterv1.ConditionType, service string, err error) { + switch { + case err == nil: + conditions.MarkTrue(s.AzureMachinePool, condition) + case errors.Is(err, azure.ErrNotOwned): + // do nothing + case azure.IsOperationNotDoneError(err): + conditions.MarkFalse(s.AzureMachinePool, condition, infrav1.UpdatingReason, clusterv1.ConditionSeverityInfo, "%s updating", service) + default: + conditions.MarkFalse(s.AzureMachinePool, condition, infrav1.FailedReason, clusterv1.ConditionSeverityError, "%s failed to update. err: %s", service, err.Error()) + } } diff --git a/azure/scope/machinepoolmachine.go b/azure/scope/machinepoolmachine.go index f40b68fa499..0a1cddd1035 100644 --- a/azure/scope/machinepoolmachine.go +++ b/azure/scope/machinepoolmachine.go @@ -190,14 +190,46 @@ func (s *MachinePoolMachineScope) DeleteLongRunningOperationState(name, service futures.Delete(s.AzureMachinePoolMachine, name, service) } -// SetConditionTrue will set a condition to true on the AzureMachinePoolMachine status. -func (s *MachinePoolMachineScope) SetConditionTrue(condition clusterv1.ConditionType) { - conditions.MarkTrue(s.AzureMachinePoolMachine, condition) +// UpdateDeleteStatus updates a condition on the AzureMachinePoolMachine status after a DELETE operation. +func (s *MachinePoolMachineScope) UpdateDeleteStatus(condition clusterv1.ConditionType, service string, err error) { + switch { + case err == nil: + conditions.MarkFalse(s.AzureMachinePoolMachine, condition, infrav1.DeletedReason, clusterv1.ConditionSeverityInfo, "%s successfully deleted", service) + case errors.Is(err, azure.ErrNotOwned): + // do nothing + case azure.IsOperationNotDoneError(err): + conditions.MarkFalse(s.AzureMachinePoolMachine, condition, infrav1.DeletingReason, clusterv1.ConditionSeverityInfo, "%s deleting", service) + default: + conditions.MarkFalse(s.AzureMachinePoolMachine, condition, infrav1.DeletionFailedReason, clusterv1.ConditionSeverityError, "%s failed to delete. err: %s", service, err.Error()) + } +} + +// UpdatePutStatus updates a condition on the AzureMachinePoolMachine status after a PUT operation. +func (s *MachinePoolMachineScope) UpdatePutStatus(condition clusterv1.ConditionType, service string, err error) { + switch { + case err == nil: + conditions.MarkTrue(s.AzureMachinePoolMachine, condition) + case errors.Is(err, azure.ErrNotOwned): + // do nothing + case azure.IsOperationNotDoneError(err): + conditions.MarkFalse(s.AzureMachinePoolMachine, condition, infrav1.CreatingReason, clusterv1.ConditionSeverityInfo, "%s creating or updating", service) + default: + conditions.MarkFalse(s.AzureMachinePoolMachine, condition, infrav1.FailedReason, clusterv1.ConditionSeverityError, "%s failed to create or update. err: %s", service, err.Error()) + } } -// SetConditionFalse will set a condition to false on the AzureMachinePoolMachine status. -func (s *MachinePoolMachineScope) SetConditionFalse(condition clusterv1.ConditionType, reason string, severity clusterv1.ConditionSeverity) { - conditions.MarkFalse(s.AzureMachinePoolMachine, condition, reason, severity, "") // TODO: add message +// UpdatePatchStatus updates a condition on the AzureMachinePoolMachine status after a PATCH operation. +func (s *MachinePoolMachineScope) UpdatePatchStatus(condition clusterv1.ConditionType, service string, err error) { + switch { + case err == nil: + conditions.MarkTrue(s.AzureMachinePoolMachine, condition) + case errors.Is(err, azure.ErrNotOwned): + // do nothing + case azure.IsOperationNotDoneError(err): + conditions.MarkFalse(s.AzureMachinePoolMachine, condition, infrav1.UpdatingReason, clusterv1.ConditionSeverityInfo, "%s updating", service) + default: + conditions.MarkFalse(s.AzureMachinePoolMachine, condition, infrav1.FailedReason, clusterv1.ConditionSeverityError, "%s failed to update. err: %s", service, err.Error()) + } } // SetVMSSVM update the scope with the current state of the VMSS VM. diff --git a/azure/scope/managedcontrolplane.go b/azure/scope/managedcontrolplane.go index 78f9945b005..ecf16f4f788 100644 --- a/azure/scope/managedcontrolplane.go +++ b/azure/scope/managedcontrolplane.go @@ -38,6 +38,8 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" "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/v1alpha4" "sigs.k8s.io/cluster-api-provider-azure/util/futures" ) @@ -198,11 +200,24 @@ func (s *ManagedControlPlaneScope) Vnet() *infrav1.VnetSpec { } // 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) GroupSpec() azure.ResourceSpecGetter { + return &groups.GroupSpec{ + Name: s.ResourceGroup(), + Location: s.Location(), + ClusterName: s.ClusterName(), + AdditionalTags: s.AdditionalTags(), + } +} + +// VNetSpec returns the virtual network spec. +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(), } } @@ -284,8 +299,9 @@ func (s *ManagedControlPlaneScope) IsIPv6Enabled() bool { } // IsVnetManaged returns true if the vnet is managed. -func (s *ManagedControlPlaneScope) IsVnetManaged() bool { - return true +func (s *ManagedControlPlaneScope) IsVnetManaged(ctx context.Context) (bool, error) { + vnetSvc := virtualnetworks.New(s) + return vnetSvc.IsVnetManaged(ctx) } // APIServerLBName returns the API Server LB name. @@ -562,12 +578,17 @@ func (s *ManagedControlPlaneScope) DeleteLongRunningOperationState(name, service futures.Delete(s.ControlPlane, name, service) } -// SetConditionTrue is a no-op for managed control planes. -func (s *ManagedControlPlaneScope) SetConditionTrue(condition clusterv1.ConditionType) { +// UpdateDeleteStatus updates a condition on the AzureManagedControlPlane status after a DELETE operation. +func (s *ManagedControlPlaneScope) UpdateDeleteStatus(condition clusterv1.ConditionType, service string, err error) { + // TODO: add condition to AzureManagedControlPlane status +} + +// UpdatePutStatus updates a condition on the AzureManagedControlPlane status after a PUT operation. +func (s *ManagedControlPlaneScope) UpdatePutStatus(condition clusterv1.ConditionType, service string, err error) { // TODO: add condition to AzureManagedControlPlane status } -// SetConditionFalse is a no-op for managed control planes. -func (s *ManagedControlPlaneScope) SetConditionFalse(condition clusterv1.ConditionType, reason string, severity clusterv1.ConditionSeverity) { +// UpdatePatchStatus updates a condition on the AzureManagedControlPlane status after a PATCH operation. +func (s *ManagedControlPlaneScope) UpdatePatchStatus(condition clusterv1.ConditionType, service string, err error) { // TODO: add condition to AzureManagedControlPlane status } diff --git a/azure/services/async/helper.go b/azure/services/async/helper.go new file mode 100644 index 00000000000..41a67ef70b0 --- /dev/null +++ b/azure/services/async/helper.go @@ -0,0 +1,123 @@ +/* +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 async + +import ( + "context" + + "github.com/pkg/errors" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" + "sigs.k8s.io/cluster-api-provider-azure/azure" + "sigs.k8s.io/cluster-api-provider-azure/azure/converters" + "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" +) + +// ProcessOngoingOperation is a helper function that will process an ongoing operation to check if it is done. +// If it is not done, it will return a transient error. +func ProcessOngoingOperation(ctx context.Context, scope FutureScope, client FutureHandler, resourceName string, serviceName string) error { + future := scope.GetLongRunningOperationState(resourceName, serviceName) + if future == nil { + scope.V(2).Info("no long running operation found", "service", serviceName, "resource", resourceName) + return nil + } + sdkFuture, err := converters.FutureToSDK(*future) + if err != nil { + // Reset the future data to avoid getting stuck in a bad loop. + scope.DeleteLongRunningOperationState(resourceName, serviceName) + return errors.Wrap(err, "could not decode future data, resetting long-running operation state") + } + done, err := client.IsDone(ctx, sdkFuture) + if err != nil { + return errors.Wrap(err, "failed checking if the operation was complete") + } + + if !done { + // Operation is still in progress, update conditions and requeue. + scope.V(2).Info("long running operation is still ongoing", "service", serviceName, "resource", resourceName) + return azure.WithTransientError(azure.NewOperationNotDoneError(future), reconciler.DefaultReconcilerRequeue) + } + + // Resource has been created/deleted/updated. + scope.V(2).Info("long running operation has completed", "service", serviceName, "resource", resourceName) + scope.DeleteLongRunningOperationState(resourceName, serviceName) + return nil +} + +// CreateResource implements the logic for creating a resource Asynchronously. +func CreateResource(ctx context.Context, scope FutureScope, client AsyncCreator, spec azure.ResourceSpecGetter, serviceName string) error { + resourceName := spec.ResourceName() + rgName := spec.ResourceGroupName() + + // Check if there is an ongoing long running operation. + future := scope.GetLongRunningOperationState(resourceName, serviceName) + if future != nil { + return ProcessOngoingOperation(ctx, scope, client, resourceName, serviceName) + } + + // No long running operation is active, so create the resource. + scope.V(2).Info("creating resource", "service", serviceName, "resource", resourceName, "resourceGroup", rgName) + sdkFuture, err := client.CreateOrUpdateAsync(ctx, spec) + if err != nil { + if sdkFuture != nil { + future, err := converters.SDKToFuture(sdkFuture, infrav1.PutFuture, serviceName, resourceName, rgName) + if err != nil { + return errors.Wrapf(err, "failed to create resource %s/%s (service: %s)", rgName, resourceName, serviceName) + } + scope.SetLongRunningOperationState(future) + return azure.WithTransientError(azure.NewOperationNotDoneError(future), reconciler.DefaultReconcilerRequeue) + } + + return errors.Wrapf(err, "failed to create resource %s/%s (service: %s)", rgName, resourceName, serviceName) + } + + scope.V(2).Info("successfully created resource", "service", serviceName, "resource", resourceName, "resourceGroup", rgName) + return nil +} + +// DeleteResource implements the logic for deleting a resource Asynchronously. +func DeleteResource(ctx context.Context, scope FutureScope, client AsyncDeleter, spec azure.ResourceSpecGetter, serviceName string) error { + resourceName := spec.ResourceName() + rgName := spec.ResourceGroupName() + + // Check if there is an ongoing long running operation. + future := scope.GetLongRunningOperationState(resourceName, serviceName) + if future != nil { + return ProcessOngoingOperation(ctx, scope, client, resourceName, serviceName) + } + + // No long running operation is active, so delete the resource. + scope.V(2).Info("deleting resource", "service", serviceName, "resource", resourceName, "resourceGroup", rgName) + sdkFuture, err := client.DeleteAsync(ctx, spec) + if err != nil { + if azure.ResourceNotFound(err) { + // already deleted + return nil + } else if sdkFuture != nil { + future, err := converters.SDKToFuture(sdkFuture, infrav1.DeleteFuture, serviceName, resourceName, rgName) + if err != nil { + return errors.Wrapf(err, "failed to delete resource %s/%s (service: %s)", rgName, resourceName, serviceName) + } + scope.SetLongRunningOperationState(future) + return azure.WithTransientError(azure.NewOperationNotDoneError(future), reconciler.DefaultReconcilerRequeue) + } + + return errors.Wrapf(err, "failed to delete resource %s/%s (service: %s)", rgName, resourceName, serviceName) + } + + scope.V(2).Info("successfully deleted resource", "service", serviceName, "resource", resourceName, "resourceGroup", rgName) + return nil +} diff --git a/azure/services/async/helper_test.go b/azure/services/async/helper_test.go new file mode 100644 index 00000000000..66395d875ce --- /dev/null +++ b/azure/services/async/helper_test.go @@ -0,0 +1,319 @@ +/* +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 async + +import ( + "context" + "errors" + "net/http" + "testing" + + "github.com/Azure/go-autorest/autorest" + azureautorest "github.com/Azure/go-autorest/autorest/azure" + "github.com/golang/mock/gomock" + . "github.com/onsi/gomega" + "k8s.io/klog/v2/klogr" + + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" + "sigs.k8s.io/cluster-api-provider-azure/azure/mock_azure" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/async/mock_async" + gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" +) + +var ( + validCreateFuture = infrav1.Future{ + Type: infrav1.PutFuture, + ServiceName: "test-service", + Name: "test-resource", + ResourceGroup: "test-group", + Data: "eyJtZXRob2QiOiJQVVQiLCJwb2xsaW5nTWV0aG9kIjoiTG9jYXRpb24iLCJscm9TdGF0ZSI6IkluUHJvZ3Jlc3MifQ==", + } + validDeleteFuture = infrav1.Future{ + Type: infrav1.DeleteFuture, + ServiceName: "test-service", + Name: "test-resource", + ResourceGroup: "test-group", + Data: "eyJtZXRob2QiOiJERUxFVEUiLCJwb2xsaW5nTWV0aG9kIjoiTG9jYXRpb24iLCJscm9TdGF0ZSI6IkluUHJvZ3Jlc3MifQ==", + } + invalidFuture = infrav1.Future{ + Type: infrav1.DeleteFuture, + ServiceName: "test-service", + Name: "test-resource", + ResourceGroup: "test-group", + Data: "ZmFrZSBiNjQgZnV0dXJlIGRhdGEK", + } + fakeError = autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error") + errCtxExceeded = errors.New("ctx exceeded") +) + +// TestProcessOngoingOperation tests the ProcessOngoingOperation function. +func TestProcessOngoingOperation(t *testing.T) { + testcases := []struct { + name string + resourceName string + serviceName string + expectedError string + expect func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockFutureHandlerMockRecorder) + }{ + { + name: "no future data stored in status", + expectedError: "", + resourceName: "test-resource", + serviceName: "test-service", + expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockFutureHandlerMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + s.GetLongRunningOperationState("test-resource", "test-service").Return(nil) + }, + }, + { + name: "future data is not valid", + expectedError: "could not decode future data, resetting long-running operation state", + resourceName: "test-resource", + serviceName: "test-service", + expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockFutureHandlerMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + s.GetLongRunningOperationState("test-resource", "test-service").Return(&invalidFuture) + s.DeleteLongRunningOperationState("test-resource", "test-service") + }, + }, + { + name: "fail to check if ongoing operation is done", + expectedError: "failed checking if the operation was complete", + resourceName: "test-resource", + serviceName: "test-service", + expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockFutureHandlerMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + s.GetLongRunningOperationState("test-resource", "test-service").Return(&validDeleteFuture) + c.IsDone(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{})).Return(false, fakeError) + }, + }, + { + name: "ongoing operation is not done", + expectedError: "operation type DELETE on Azure resource test-group/test-resource is not done", + resourceName: "test-resource", + serviceName: "test-service", + expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockFutureHandlerMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + s.GetLongRunningOperationState("test-resource", "test-service").Return(&validDeleteFuture) + c.IsDone(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{})).Return(false, nil) + }, + }, + { + name: "operation is done", + expectedError: "", + resourceName: "test-resource", + serviceName: "test-service", + expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockFutureHandlerMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + s.GetLongRunningOperationState("test-resource", "test-service").Return(&validDeleteFuture) + c.IsDone(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{})).Return(true, nil) + s.DeleteLongRunningOperationState("test-resource", "test-service") + }, + }, + } + + for _, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + t.Parallel() + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + scopeMock := mock_async.NewMockFutureScope(mockCtrl) + clientMock := mock_async.NewMockFutureHandler(mockCtrl) + + tc.expect(scopeMock.EXPECT(), clientMock.EXPECT()) + + err := ProcessOngoingOperation(context.TODO(), scopeMock, clientMock, tc.resourceName, tc.serviceName) + if tc.expectedError != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tc.expectedError)) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + }) + } +} + +// TestCreateResource tests the CreateResource function. +func TestCreateResource(t *testing.T) { + testcases := []struct { + name string + serviceName string + expectedError string + expect func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockAsyncCreatorMockRecorder, r *mock_azure.MockResourceSpecGetterMockRecorder) + }{ + { + name: "create operation is already in progress", + expectedError: "transient reconcile error occurred: operation type PUT on Azure resource test-group/test-resource is not done. Object will be requeued after 15s", + serviceName: "test-service", + expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockAsyncCreatorMockRecorder, r *mock_azure.MockResourceSpecGetterMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + r.ResourceName().Return("test-resource") + r.ResourceGroupName().Return("test-group") + s.GetLongRunningOperationState("test-resource", "test-service").Times(2).Return(&validCreateFuture) + c.IsDone(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{})).Return(false, nil) + }, + }, + { + name: "create async returns success", + expectedError: "", + serviceName: "test-service", + expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockAsyncCreatorMockRecorder, r *mock_azure.MockResourceSpecGetterMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + r.ResourceName().Return("test-resource") + r.ResourceGroupName().Return("test-group") + s.GetLongRunningOperationState("test-resource", "test-service").Return(nil) + c.CreateOrUpdateAsync(gomockinternal.AContext(), gomock.AssignableToTypeOf(&mock_azure.MockResourceSpecGetter{})).Return(nil, nil) + }, + }, + { + name: "error occurs while running async create", + expectedError: "failed to create resource test-group/test-resource (service: test-service)", + serviceName: "test-service", + expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockAsyncCreatorMockRecorder, r *mock_azure.MockResourceSpecGetterMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + r.ResourceName().Return("test-resource") + r.ResourceGroupName().Return("test-group") + s.GetLongRunningOperationState("test-resource", "test-service").Return(nil) + c.CreateOrUpdateAsync(gomockinternal.AContext(), gomock.AssignableToTypeOf(&mock_azure.MockResourceSpecGetter{})).Return(nil, fakeError) + }, + }, + { + name: "create async exits before completing", + expectedError: "transient reconcile error occurred: operation type PUT on Azure resource test-group/test-resource is not done. Object will be requeued after 15s", + serviceName: "test-service", + expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockAsyncCreatorMockRecorder, r *mock_azure.MockResourceSpecGetterMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + r.ResourceName().Return("test-resource") + r.ResourceGroupName().Return("test-group") + s.GetLongRunningOperationState("test-resource", "test-service").Return(nil) + c.CreateOrUpdateAsync(gomockinternal.AContext(), gomock.AssignableToTypeOf(&mock_azure.MockResourceSpecGetter{})).Return(&azureautorest.Future{}, errCtxExceeded) + s.SetLongRunningOperationState(gomock.AssignableToTypeOf(&infrav1.Future{})) + }, + }, + } + + for _, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + t.Parallel() + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + scopeMock := mock_async.NewMockFutureScope(mockCtrl) + clientMock := mock_async.NewMockAsyncCreator(mockCtrl) + specMock := mock_azure.NewMockResourceSpecGetter(mockCtrl) + + tc.expect(scopeMock.EXPECT(), clientMock.EXPECT(), specMock.EXPECT()) + + err := CreateResource(context.TODO(), scopeMock, clientMock, specMock, tc.serviceName) + if tc.expectedError != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tc.expectedError)) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + }) + } +} + +// TestDeleteResource tests the DeleteResource function. +func TestDeleteResource(t *testing.T) { + testcases := []struct { + name string + serviceName string + expectedError string + expect func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockAsyncDeleterMockRecorder, r *mock_azure.MockResourceSpecGetterMockRecorder) + }{ + { + name: "delete operation is already in progress", + expectedError: "transient reconcile error occurred: operation type DELETE on Azure resource test-group/test-resource is not done. Object will be requeued after 15s", + serviceName: "test-service", + expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockAsyncDeleterMockRecorder, r *mock_azure.MockResourceSpecGetterMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + r.ResourceName().Return("test-resource") + r.ResourceGroupName().Return("test-group") + s.GetLongRunningOperationState("test-resource", "test-service").Times(2).Return(&validDeleteFuture) + c.IsDone(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{})).Return(false, nil) + }, + }, + { + name: "delete async returns success", + expectedError: "", + serviceName: "test-service", + expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockAsyncDeleterMockRecorder, r *mock_azure.MockResourceSpecGetterMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + r.ResourceName().Return("test-resource") + r.ResourceGroupName().Return("test-group") + s.GetLongRunningOperationState("test-resource", "test-service").Return(nil) + c.DeleteAsync(gomockinternal.AContext(), gomock.AssignableToTypeOf(&mock_azure.MockResourceSpecGetter{})).Return(nil, nil) + }, + }, + { + name: "error occurs while running async delete", + expectedError: "failed to delete resource test-group/test-resource (service: test-service)", + serviceName: "test-service", + expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockAsyncDeleterMockRecorder, r *mock_azure.MockResourceSpecGetterMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + r.ResourceName().Return("test-resource") + r.ResourceGroupName().Return("test-group") + s.GetLongRunningOperationState("test-resource", "test-service").Return(nil) + c.DeleteAsync(gomockinternal.AContext(), gomock.AssignableToTypeOf(&mock_azure.MockResourceSpecGetter{})).Return(nil, fakeError) + }, + }, + { + name: "delete async exits before completing", + expectedError: "transient reconcile error occurred: operation type DELETE on Azure resource test-group/test-resource is not done. Object will be requeued after 15s", + serviceName: "test-service", + expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockAsyncDeleterMockRecorder, r *mock_azure.MockResourceSpecGetterMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + r.ResourceName().Return("test-resource") + r.ResourceGroupName().Return("test-group") + s.GetLongRunningOperationState("test-resource", "test-service").Return(nil) + c.DeleteAsync(gomockinternal.AContext(), gomock.AssignableToTypeOf(&mock_azure.MockResourceSpecGetter{})).Return(&azureautorest.Future{}, errCtxExceeded) + s.SetLongRunningOperationState(gomock.AssignableToTypeOf(&infrav1.Future{})) + }, + }, + } + + for _, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + t.Parallel() + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + scopeMock := mock_async.NewMockFutureScope(mockCtrl) + clientMock := mock_async.NewMockAsyncDeleter(mockCtrl) + specMock := mock_azure.NewMockResourceSpecGetter(mockCtrl) + + tc.expect(scopeMock.EXPECT(), clientMock.EXPECT(), specMock.EXPECT()) + + err := DeleteResource(context.TODO(), scopeMock, clientMock, specMock, tc.serviceName) + if tc.expectedError != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tc.expectedError)) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + }) + } +} diff --git a/azure/services/async/interfaces.go b/azure/services/async/interfaces.go new file mode 100644 index 00000000000..888c0630385 --- /dev/null +++ b/azure/services/async/interfaces.go @@ -0,0 +1,45 @@ +/* +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 async + +import ( + "context" + + azureautorest "github.com/Azure/go-autorest/autorest/azure" + "github.com/go-logr/logr" + "sigs.k8s.io/cluster-api-provider-azure/azure" +) + +type FutureScope interface { + logr.Logger + azure.AsyncStatusUpdater +} + +type FutureHandler interface { + // IsDone returns true if the operation is complete. + IsDone(ctx context.Context, future azureautorest.FutureAPI) (bool, error) +} + +type AsyncCreator interface { + FutureHandler + CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) +} + +type AsyncDeleter interface { + FutureHandler + DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) +} diff --git a/azure/services/async/mock_async/async_mock.go b/azure/services/async/mock_async/async_mock.go new file mode 100644 index 00000000000..8ac1e0be10d --- /dev/null +++ b/azure/services/async/mock_async/async_mock.go @@ -0,0 +1,368 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by MockGen. DO NOT EDIT. +// Source: ../interfaces.go + +// Package mock_async is a generated GoMock package. +package mock_async + +import ( + context "context" + reflect "reflect" + + azure "github.com/Azure/go-autorest/autorest/azure" + logr "github.com/go-logr/logr" + gomock "github.com/golang/mock/gomock" + v1alpha4 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" + azure0 "sigs.k8s.io/cluster-api-provider-azure/azure" + v1alpha40 "sigs.k8s.io/cluster-api/api/v1alpha4" +) + +// MockFutureScope is a mock of FutureScope interface. +type MockFutureScope struct { + ctrl *gomock.Controller + recorder *MockFutureScopeMockRecorder +} + +// MockFutureScopeMockRecorder is the mock recorder for MockFutureScope. +type MockFutureScopeMockRecorder struct { + mock *MockFutureScope +} + +// NewMockFutureScope creates a new mock instance. +func NewMockFutureScope(ctrl *gomock.Controller) *MockFutureScope { + mock := &MockFutureScope{ctrl: ctrl} + mock.recorder = &MockFutureScopeMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockFutureScope) EXPECT() *MockFutureScopeMockRecorder { + return m.recorder +} + +// DeleteLongRunningOperationState mocks base method. +func (m *MockFutureScope) DeleteLongRunningOperationState(arg0, arg1 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "DeleteLongRunningOperationState", arg0, arg1) +} + +// DeleteLongRunningOperationState indicates an expected call of DeleteLongRunningOperationState. +func (mr *MockFutureScopeMockRecorder) DeleteLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteLongRunningOperationState", reflect.TypeOf((*MockFutureScope)(nil).DeleteLongRunningOperationState), arg0, arg1) +} + +// Enabled mocks base method. +func (m *MockFutureScope) Enabled() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Enabled") + ret0, _ := ret[0].(bool) + return ret0 +} + +// Enabled indicates an expected call of Enabled. +func (mr *MockFutureScopeMockRecorder) Enabled() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Enabled", reflect.TypeOf((*MockFutureScope)(nil).Enabled)) +} + +// Error mocks base method. +func (m *MockFutureScope) Error(err error, msg string, keysAndValues ...interface{}) { + m.ctrl.T.Helper() + varargs := []interface{}{err, msg} + for _, a := range keysAndValues { + varargs = append(varargs, a) + } + m.ctrl.Call(m, "Error", varargs...) +} + +// Error indicates an expected call of Error. +func (mr *MockFutureScopeMockRecorder) Error(err, msg interface{}, keysAndValues ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{err, msg}, keysAndValues...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Error", reflect.TypeOf((*MockFutureScope)(nil).Error), varargs...) +} + +// GetLongRunningOperationState mocks base method. +func (m *MockFutureScope) GetLongRunningOperationState(arg0, arg1 string) *v1alpha4.Future { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetLongRunningOperationState", arg0, arg1) + ret0, _ := ret[0].(*v1alpha4.Future) + return ret0 +} + +// GetLongRunningOperationState indicates an expected call of GetLongRunningOperationState. +func (mr *MockFutureScopeMockRecorder) GetLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLongRunningOperationState", reflect.TypeOf((*MockFutureScope)(nil).GetLongRunningOperationState), arg0, arg1) +} + +// Info mocks base method. +func (m *MockFutureScope) Info(msg string, keysAndValues ...interface{}) { + m.ctrl.T.Helper() + varargs := []interface{}{msg} + for _, a := range keysAndValues { + varargs = append(varargs, a) + } + m.ctrl.Call(m, "Info", varargs...) +} + +// Info indicates an expected call of Info. +func (mr *MockFutureScopeMockRecorder) Info(msg interface{}, keysAndValues ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{msg}, keysAndValues...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Info", reflect.TypeOf((*MockFutureScope)(nil).Info), varargs...) +} + +// SetLongRunningOperationState mocks base method. +func (m *MockFutureScope) SetLongRunningOperationState(arg0 *v1alpha4.Future) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetLongRunningOperationState", arg0) +} + +// SetLongRunningOperationState indicates an expected call of SetLongRunningOperationState. +func (mr *MockFutureScopeMockRecorder) SetLongRunningOperationState(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLongRunningOperationState", reflect.TypeOf((*MockFutureScope)(nil).SetLongRunningOperationState), arg0) +} + +// UpdateDeleteStatus mocks base method. +func (m *MockFutureScope) UpdateDeleteStatus(arg0 v1alpha40.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 *MockFutureScopeMockRecorder) UpdateDeleteStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateDeleteStatus", reflect.TypeOf((*MockFutureScope)(nil).UpdateDeleteStatus), arg0, arg1, arg2) +} + +// UpdatePatchStatus mocks base method. +func (m *MockFutureScope) UpdatePatchStatus(arg0 v1alpha40.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 *MockFutureScopeMockRecorder) UpdatePatchStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePatchStatus", reflect.TypeOf((*MockFutureScope)(nil).UpdatePatchStatus), arg0, arg1, arg2) +} + +// UpdatePutStatus mocks base method. +func (m *MockFutureScope) UpdatePutStatus(arg0 v1alpha40.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 *MockFutureScopeMockRecorder) UpdatePutStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePutStatus", reflect.TypeOf((*MockFutureScope)(nil).UpdatePutStatus), arg0, arg1, arg2) +} + +// V mocks base method. +func (m *MockFutureScope) V(level int) logr.Logger { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "V", level) + ret0, _ := ret[0].(logr.Logger) + return ret0 +} + +// V indicates an expected call of V. +func (mr *MockFutureScopeMockRecorder) V(level interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "V", reflect.TypeOf((*MockFutureScope)(nil).V), level) +} + +// WithName mocks base method. +func (m *MockFutureScope) WithName(name string) logr.Logger { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "WithName", name) + ret0, _ := ret[0].(logr.Logger) + return ret0 +} + +// WithName indicates an expected call of WithName. +func (mr *MockFutureScopeMockRecorder) WithName(name interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WithName", reflect.TypeOf((*MockFutureScope)(nil).WithName), name) +} + +// WithValues mocks base method. +func (m *MockFutureScope) WithValues(keysAndValues ...interface{}) logr.Logger { + m.ctrl.T.Helper() + varargs := []interface{}{} + for _, a := range keysAndValues { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "WithValues", varargs...) + ret0, _ := ret[0].(logr.Logger) + return ret0 +} + +// WithValues indicates an expected call of WithValues. +func (mr *MockFutureScopeMockRecorder) WithValues(keysAndValues ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WithValues", reflect.TypeOf((*MockFutureScope)(nil).WithValues), keysAndValues...) +} + +// MockFutureHandler is a mock of FutureHandler interface. +type MockFutureHandler struct { + ctrl *gomock.Controller + recorder *MockFutureHandlerMockRecorder +} + +// MockFutureHandlerMockRecorder is the mock recorder for MockFutureHandler. +type MockFutureHandlerMockRecorder struct { + mock *MockFutureHandler +} + +// NewMockFutureHandler creates a new mock instance. +func NewMockFutureHandler(ctrl *gomock.Controller) *MockFutureHandler { + mock := &MockFutureHandler{ctrl: ctrl} + mock.recorder = &MockFutureHandlerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockFutureHandler) EXPECT() *MockFutureHandlerMockRecorder { + return m.recorder +} + +// IsDone mocks base method. +func (m *MockFutureHandler) 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 *MockFutureHandlerMockRecorder) IsDone(ctx, future interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDone", reflect.TypeOf((*MockFutureHandler)(nil).IsDone), ctx, future) +} + +// MockAsyncCreator is a mock of AsyncCreator interface. +type MockAsyncCreator struct { + ctrl *gomock.Controller + recorder *MockAsyncCreatorMockRecorder +} + +// MockAsyncCreatorMockRecorder is the mock recorder for MockAsyncCreator. +type MockAsyncCreatorMockRecorder struct { + mock *MockAsyncCreator +} + +// NewMockAsyncCreator creates a new mock instance. +func NewMockAsyncCreator(ctrl *gomock.Controller) *MockAsyncCreator { + mock := &MockAsyncCreator{ctrl: ctrl} + mock.recorder = &MockAsyncCreatorMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockAsyncCreator) EXPECT() *MockAsyncCreatorMockRecorder { + return m.recorder +} + +// CreateOrUpdateAsync mocks base method. +func (m *MockAsyncCreator) CreateOrUpdateAsync(ctx context.Context, spec azure0.ResourceSpecGetter) (azure.FutureAPI, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateOrUpdateAsync", ctx, spec) + ret0, _ := ret[0].(azure.FutureAPI) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CreateOrUpdateAsync indicates an expected call of CreateOrUpdateAsync. +func (mr *MockAsyncCreatorMockRecorder) CreateOrUpdateAsync(ctx, spec interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*MockAsyncCreator)(nil).CreateOrUpdateAsync), ctx, spec) +} + +// IsDone mocks base method. +func (m *MockAsyncCreator) 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 *MockAsyncCreatorMockRecorder) IsDone(ctx, future interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDone", reflect.TypeOf((*MockAsyncCreator)(nil).IsDone), ctx, future) +} + +// MockAsyncDeleter is a mock of AsyncDeleter interface. +type MockAsyncDeleter struct { + ctrl *gomock.Controller + recorder *MockAsyncDeleterMockRecorder +} + +// MockAsyncDeleterMockRecorder is the mock recorder for MockAsyncDeleter. +type MockAsyncDeleterMockRecorder struct { + mock *MockAsyncDeleter +} + +// NewMockAsyncDeleter creates a new mock instance. +func NewMockAsyncDeleter(ctrl *gomock.Controller) *MockAsyncDeleter { + mock := &MockAsyncDeleter{ctrl: ctrl} + mock.recorder = &MockAsyncDeleterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockAsyncDeleter) EXPECT() *MockAsyncDeleterMockRecorder { + return m.recorder +} + +// DeleteAsync mocks base method. +func (m *MockAsyncDeleter) 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 *MockAsyncDeleterMockRecorder) DeleteAsync(ctx, spec interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAsync", reflect.TypeOf((*MockAsyncDeleter)(nil).DeleteAsync), ctx, spec) +} + +// IsDone mocks base method. +func (m *MockAsyncDeleter) 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 *MockAsyncDeleterMockRecorder) IsDone(ctx, future interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDone", reflect.TypeOf((*MockAsyncDeleter)(nil).IsDone), ctx, future) +} diff --git a/azure/services/async/mock_async/doc.go b/azure/services/async/mock_async/doc.go new file mode 100644 index 00000000000..124cb54e0e2 --- /dev/null +++ b/azure/services/async/mock_async/doc.go @@ -0,0 +1,20 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Run go generate to regenerate this mock. +//go:generate ../../../../hack/tools/bin/mockgen -destination async_mock.go -package mock_async -source ../interfaces.go FutureHandler +//go:generate /usr/bin/env bash -c "cat ../../../../hack/boilerplate/boilerplate.generatego.txt async_mock.go > _async_mock.go && mv _async_mock.go async_mock.go" +package mock_async //nolint diff --git a/azure/services/bastionhosts/mocks_bastionhosts/bastionhosts_mock.go b/azure/services/bastionhosts/mocks_bastionhosts/bastionhosts_mock.go index 0683f524cfc..42d715c4a55 100644 --- a/azure/services/bastionhosts/mocks_bastionhosts/bastionhosts_mock.go +++ b/azure/services/bastionhosts/mocks_bastionhosts/bastionhosts_mock.go @@ -21,6 +21,7 @@ limitations under the License. package mock_bastionhosts import ( + context "context" reflect "reflect" autorest "github.com/Azure/go-autorest/autorest" @@ -354,17 +355,18 @@ func (mr *MockBastionScopeMockRecorder) IsIPv6Enabled() *gomock.Call { } // IsVnetManaged mocks base method. -func (m *MockBastionScope) IsVnetManaged() bool { +func (m *MockBastionScope) IsVnetManaged(arg0 context.Context) (bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsVnetManaged") + ret := m.ctrl.Call(m, "IsVnetManaged", arg0) ret0, _ := ret[0].(bool) - return ret0 + ret1, _ := ret[1].(error) + return ret0, ret1 } // IsVnetManaged indicates an expected call of IsVnetManaged. -func (mr *MockBastionScopeMockRecorder) IsVnetManaged() *gomock.Call { +func (mr *MockBastionScopeMockRecorder) IsVnetManaged(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsVnetManaged", reflect.TypeOf((*MockBastionScope)(nil).IsVnetManaged)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsVnetManaged", reflect.TypeOf((*MockBastionScope)(nil).IsVnetManaged), arg0) } // Location mocks base method. diff --git a/azure/services/futures/futures.go b/azure/services/futures/futures.go deleted file mode 100644 index 41dab338ce0..00000000000 --- a/azure/services/futures/futures.go +++ /dev/null @@ -1,41 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package futures - -import ( - "context" - "encoding/base64" - "encoding/json" - - "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/v1alpha4" -) - -func Done(ctx context.Context, future *infrav1.Future, client autorest.Sender) (bool, error) { - futureData, err := base64.URLEncoding.DecodeString(future.Data) - if err != nil { - return false, errors.Wrap(err, "failed to base64 decode future data") - } - var genericFuture azureautorest.FutureAPI - if err := json.Unmarshal(futureData, &genericFuture); err != nil { - return false, errors.Wrap(err, "failed to unmarshal future data") - } - - return genericFuture.DoneWithContext(ctx, client) -} diff --git a/azure/services/groups/client.go b/azure/services/groups/client.go index ab648f60e9c..e0d234ed136 100644 --- a/azure/services/groups/client.go +++ b/azure/services/groups/client.go @@ -21,16 +21,20 @@ import ( "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-05-01/resources" "github.com/Azure/go-autorest/autorest" + azureautorest "github.com/Azure/go-autorest/autorest/azure" + "github.com/pkg/errors" "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) (resources.Group, error) - CreateOrUpdate(context.Context, string, resources.Group) (resources.Group, error) - Delete(context.Context, string) error + CreateOrUpdateAsync(context.Context, azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) + DeleteAsync(context.Context, azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) + IsDone(context.Context, azureautorest.FutureAPI) (bool, error) } // azureClient contains the Azure go-sdk Client. @@ -63,27 +67,94 @@ func (ac *azureClient) Get(ctx context.Context, name string) (resources.Group, e return ac.groups.Get(ctx, name) } -// CreateOrUpdate creates or updates a resource group. -func (ac *azureClient) CreateOrUpdate(ctx context.Context, name string, group resources.Group) (resources.Group, error) { +// CreateOrUpdateAsync creates or updates a resource group. +// Creating a resource group is not a long running operation, so we don't ever return a future. +func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) { ctx, span := tele.Tracer().Start(ctx, "groups.AzureClient.CreateOrUpdate") defer span.End() - return ac.groups.CreateOrUpdate(ctx, name, group) + group, err := ac.resourceGroupParams(ctx, spec) + if err != nil { + return nil, errors.Wrapf(err, "failed to get desired parameters for group %s", spec.ResourceName()) + } else if group == nil { + // nothing to do here + return nil, nil + } + + _, err = ac.groups.CreateOrUpdate(ctx, spec.ResourceName(), *group) + return nil, err } -// Delete deletes a resource group. When you delete a resource group, all of its resources are also deleted. -func (ac *azureClient) Delete(ctx context.Context, name string) error { - ctx, span := tele.Tracer().Start(ctx, "groups.AzureClient.Delete") +// DeleteAsync deletes a resource group 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. +// +// NOTE: When you delete a resource group, all of its resources are also deleted. +func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) { + ctx, span := tele.Tracer().Start(ctx, "groups.AzureClient.DeleteAsync") defer span.End() - future, err := ac.groups.Delete(ctx, name) + future, err := ac.groups.Delete(ctx, spec.ResourceName()) if err != nil { - return err + return nil, err } + + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + err = future.WaitForCompletionRef(ctx, ac.groups.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 &future, err } _, err = future.Result(ac.groups) - return err + // if the operation completed, return a nil future. + return nil, err +} + +// IsDone returns true if the long-running operation has completed. +func (ac *azureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (bool, error) { + ctx, span := tele.Tracer().Start(ctx, "groups.AzureClient.IsDone") + defer span.End() + + done, err := future.DoneWithContext(ctx, ac.groups) + if err != nil { + return false, errors.Wrap(err, "failed checking if the operation was complete") + } + + return done, nil +} + +// resourceGroupParams returns the desired resource group parameters from the given spec. +func (ac *azureClient) resourceGroupParams(ctx context.Context, spec azure.ResourceSpecGetter) (*resources.Group, error) { + var params interface{} + + existingRG, err := ac.Get(ctx, spec.ResourceName()) + if azure.ResourceNotFound(err) { + // rg doesn't exist, create it from scratch. + params, err = spec.Parameters(nil) + if err != nil { + return nil, err + } + } else if err != nil { + return nil, errors.Wrapf(err, "failed to get RG %s", spec.ResourceName()) + } else { + // rg already exists + params, err = spec.Parameters(existingRG) + if err != nil { + return nil, err + } + } + + rg, ok := params.(resources.Group) + if !ok { + if params == nil { + // nothing to do here. + return nil, nil + } + return nil, errors.Errorf("%T is not a resources.Group", params) + } + + return &rg, nil } diff --git a/azure/services/groups/groups.go b/azure/services/groups/groups.go index 9386be22cda..efd5e514d09 100644 --- a/azure/services/groups/groups.go +++ b/azure/services/groups/groups.go @@ -19,17 +19,19 @@ package groups import ( "context" - "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-05-01/resources" - "github.com/Azure/go-autorest/autorest/to" "github.com/go-logr/logr" "github.com/pkg/errors" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" "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 = "groups" + // Service provides operations on Azure resources. type Service struct { Scope GroupScope @@ -39,7 +41,10 @@ type Service struct { // GroupScope defines the scope interface for a group service. type GroupScope interface { logr.Logger - azure.ClusterDescriber + azure.Authorizer + azure.AsyncStatusUpdater + GroupSpec() azure.ResourceSpecGetter + ClusterName() string } // New creates a new service. @@ -55,65 +60,43 @@ func (s *Service) Reconcile(ctx context.Context) error { ctx, span := tele.Tracer().Start(ctx, "groups.Service.Reconcile") defer span.End() - if _, err := s.client.Get(ctx, s.Scope.ResourceGroup()); err == nil { - // resource group already exists, skip creation - return nil - } else if !azure.ResourceNotFound(err) { - return errors.Wrapf(err, "failed to get resource group %s", s.Scope.ResourceGroup()) - } - - s.Scope.V(2).Info("creating resource group", "resource group", s.Scope.ResourceGroup()) - group := resources.Group{ - Location: to.StringPtr(s.Scope.Location()), - Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{ - ClusterName: s.Scope.ClusterName(), - Lifecycle: infrav1.ResourceLifecycleOwned, - Name: to.StringPtr(s.Scope.ResourceGroup()), - Role: to.StringPtr(infrav1.CommonRole), - Additional: s.Scope.AdditionalTags(), - })), - } + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() - _, err := s.client.CreateOrUpdate(ctx, s.Scope.ResourceGroup(), group) - if err != nil { - return errors.Wrapf(err, "failed to create resource group %s", s.Scope.ResourceGroup()) - } + groupSpec := s.Scope.GroupSpec() - s.Scope.V(2).Info("successfully created resource group", "resource group", s.Scope.ResourceGroup()) - return nil + err := async.CreateResource(ctx, s.Scope, s.client, groupSpec, serviceName) + s.Scope.UpdatePutStatus(infrav1.ResourceGroupReadyCondition, serviceName, err) + return err } -// Delete deletes the resource group with the provided name. +// Delete deletes the resource group if it is managed by us. func (s *Service) Delete(ctx context.Context) error { ctx, span := tele.Tracer().Start(ctx, "groups.Service.Delete") defer span.End() + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() + + // check that the resource group is not BYO. managed, err := s.IsGroupManaged(ctx) - if err != nil && azure.ResourceNotFound(err) { - // already deleted or doesn't exist - return nil - } if err != nil { + if azure.ResourceNotFound(err) { + // already deleted or doesn't exist + return nil + } return errors.Wrap(err, "could not get resource group management state") } - if !managed { s.Scope.V(2).Info("Should not delete resource group in unmanaged mode") return azure.ErrNotOwned } - s.Scope.V(2).Info("deleting resource group", "resource group", s.Scope.ResourceGroup()) - err = s.client.Delete(ctx, s.Scope.ResourceGroup()) - if err != nil && azure.ResourceNotFound(err) { - // already deleted - return nil - } - if err != nil { - return errors.Wrapf(err, "failed to delete resource group %s", s.Scope.ResourceGroup()) - } + groupSpec := s.Scope.GroupSpec() - s.Scope.V(2).Info("successfully deleted resource group", "resource group", s.Scope.ResourceGroup()) - return nil + err = async.DeleteResource(ctx, s.Scope, s.client, groupSpec, serviceName) + s.Scope.UpdateDeleteStatus(infrav1.ResourceGroupReadyCondition, serviceName, err) + return err } // IsGroupManaged returns true if the resource group has an owned tag with the cluster name as value, @@ -122,7 +105,8 @@ func (s *Service) IsGroupManaged(ctx context.Context) (bool, error) { ctx, span := tele.Tracer().Start(ctx, "groups.Service.IsGroupManaged") defer span.End() - group, err := s.client.Get(ctx, s.Scope.ResourceGroup()) + groupSpec := s.Scope.GroupSpec() + group, err := s.client.Get(ctx, groupSpec.ResourceName()) if err != nil { return false, err } diff --git a/azure/services/groups/groups_test.go b/azure/services/groups/groups_test.go index 4bddbc0743a..e1b5401fb68 100644 --- a/azure/services/groups/groups_test.go +++ b/azure/services/groups/groups_test.go @@ -18,22 +18,56 @@ package groups import ( "context" + "errors" + "fmt" "net/http" "testing" "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-05-01/resources" "github.com/Azure/go-autorest/autorest" + azureautorest "github.com/Azure/go-autorest/autorest/azure" + "github.com/Azure/go-autorest/autorest/to" "github.com/golang/mock/gomock" . "github.com/onsi/gomega" "k8s.io/klog/v2/klogr" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" "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/groups/mock_groups" gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" ) +var ( + fakeGroupSpec = GroupSpec{ + Name: "test-group", + Location: "test-location", + ClusterName: "test-cluster", + AdditionalTags: map[string]string{"foo": "bar"}, + } + internalError = autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error") + notFoundError = autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not Found") + ctxExceededError = errors.New("ctx exceeded") + fakeFuture = infrav1.Future{ + Type: infrav1.DeleteFuture, + ServiceName: serviceName, + Name: "test-group", + ResourceGroup: "test-group", + Data: "eyJtZXRob2QiOiJERUxFVEUiLCJwb2xsaW5nTWV0aG9kIjoiTG9jYXRpb24iLCJscm9TdGF0ZSI6IkluUHJvZ3Jlc3MifQ==", + } + sampleManagedGroup = resources.Group{ + Name: to.StringPtr("test-group"), + Location: to.StringPtr("test-location"), + Properties: &resources.GroupProperties{}, + Tags: map[string]*string{"sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": to.StringPtr("owned")}, + } + sampleBYOGroup = resources.Group{ + Name: to.StringPtr("test-group"), + Location: to.StringPtr("test-location"), + Properties: &resources.GroupProperties{}, + Tags: map[string]*string{"foo": to.StringPtr("bar")}, + } +) + func TestReconcileGroups(t *testing.T) { testcases := []struct { name string @@ -41,50 +75,23 @@ func TestReconcileGroups(t *testing.T) { expect func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder) }{ { - name: "resource group already exist", + name: "create group succeeds", expectedError: "", expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.ResourceGroup().Return("my-rg") - m.Get(gomockinternal.AContext(), "my-rg").Return(resources.Group{}, nil) - }, - }, - { - name: "return error when querying a resource group", - expectedError: "failed to get resource group my-rg: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder) { - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.ResourceGroup().AnyTimes().Return("my-rg") - s.Location().AnyTimes().Return("fake-location") - s.ClusterName().AnyTimes().Return("fake-cluster") - m.Get(gomockinternal.AContext(), "my-rg").Return(resources.Group{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) + s.GroupSpec().Return(&fakeGroupSpec) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakeGroupSpec).Return(nil, nil) + s.UpdatePutStatus(infrav1.ResourceGroupReadyCondition, serviceName, nil) }, }, - { - name: "create a resource group", - expectedError: "", + name: "create resource group fails", + expectedError: "failed to create resource test-group/test-group (service: groups): #: Internal Server Error: StatusCode=500", expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.ResourceGroup().AnyTimes().Return("my-rg") - s.Location().AnyTimes().Return("fake-location") - s.ClusterName().AnyTimes().Return("fake-cluster") - s.AdditionalTags().AnyTimes().Return(infrav1.Tags{}) - m.Get(gomockinternal.AContext(), "my-rg").Return(resources.Group{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", gomock.AssignableToTypeOf(resources.Group{})).Return(resources.Group{}, nil) - }, - }, - { - name: "return error when creating a resource group", - expectedError: "failed to create resource group my-rg: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder) { - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.ResourceGroup().AnyTimes().Return("my-rg") - s.Location().AnyTimes().Return("fake-location") - s.ClusterName().AnyTimes().Return("fake-cluster") - s.AdditionalTags().AnyTimes().Return(infrav1.Tags{}) - m.Get(gomockinternal.AContext(), "my-rg").Return(resources.Group{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", gomock.AssignableToTypeOf(resources.Group{})).Return(resources.Group{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) + s.GroupSpec().Return(&fakeGroupSpec) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakeGroupSpec).Return(nil, internalError) + s.UpdatePutStatus(infrav1.ResourceGroupReadyCondition, serviceName, gomockinternal.ErrStrEq(fmt.Sprintf("failed to create resource test-group/test-group (service: groups): %s", internalError.Error()))) }, }, } @@ -125,89 +132,97 @@ func TestDeleteGroups(t *testing.T) { expect func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder) }{ { - name: "error getting the resource group management state", - expectedError: "could not get resource group management state: #: Internal Server Error: StatusCode=500", + name: "long running delete operation is done", + expectedError: "", + expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + s.GroupSpec().AnyTimes().Return(&fakeGroupSpec) + s.GetLongRunningOperationState("test-group", serviceName).Times(2).Return(&fakeFuture) + m.IsDone(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{})).Return(true, nil) + s.DeleteLongRunningOperationState("test-group", serviceName) + s.UpdateDeleteStatus(infrav1.ResourceGroupReadyCondition, serviceName, nil) + }, + }, + { + name: "long running delete operation is not done", + expectedError: "transient reconcile error occurred: operation type DELETE on Azure resource test-group/test-group is not done. Object will be requeued after 15s", expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Get(gomockinternal.AContext(), "my-rg").Return(resources.Group{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) + s.GroupSpec().AnyTimes().Return(&fakeGroupSpec) + s.GetLongRunningOperationState("test-group", serviceName).Times(2).Return(&fakeFuture) + m.IsDone(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{})).Return(false, nil) + s.UpdateDeleteStatus(infrav1.ResourceGroupReadyCondition, serviceName, gomockinternal.ErrStrEq("transient reconcile error occurred: operation type DELETE on Azure resource test-group/test-group is not done. Object will be requeued after 15s")) }, }, { - name: "skip deletion in unmanaged mode", + name: "resource group is not managed by capz", expectedError: azure.ErrNotOwned.Error(), expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.ResourceGroup().AnyTimes().Return("my-rg") - s.ClusterName().AnyTimes().Return("fake-cluster") - m.Get(gomockinternal.AContext(), "my-rg").Return(resources.Group{}, nil) + s.GroupSpec().AnyTimes().Return(&fakeGroupSpec) + s.GetLongRunningOperationState("test-group", serviceName).Return(nil) + m.Get(gomockinternal.AContext(), "test-group").Return(sampleBYOGroup, nil) + s.ClusterName().Return("test-cluster") }, }, { - name: "resource group already deleted", - expectedError: "", + name: "fail to check if resource group is managed", + expectedError: "could not get resource group management state", expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.ResourceGroup().AnyTimes().Return("my-rg") - s.ClusterName().AnyTimes().Return("fake-cluster") - gomock.InOrder( - m.Get(gomockinternal.AContext(), "my-rg").Return(resources.Group{ - Tags: converters.TagsToMap(infrav1.Tags{ - "Name": "my-rg", - "sigs.k8s.io_cluster-api-provider-azure_cluster_fake-cluster": "owned", - "sigs.k8s.io_cluster-api-provider-azure_role": "common", - }), - }, nil), - m.Delete(gomockinternal.AContext(), "my-rg").Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not Found")), - ) + s.GroupSpec().AnyTimes().Return(&fakeGroupSpec) + s.GetLongRunningOperationState("test-group", serviceName).Return(nil) + m.Get(gomockinternal.AContext(), "test-group").Return(resources.Group{}, internalError) }, }, { - name: "resource group get returns error", + name: "resource group doesn't exist", expectedError: "", expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.ResourceGroup().AnyTimes().Return("my-rg") - s.ClusterName().AnyTimes().Return("fake-cluster") - m.Get(gomockinternal.AContext(), "my-rg").Return(resources.Group{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not Found")) + s.GroupSpec().AnyTimes().Return(&fakeGroupSpec) + s.GetLongRunningOperationState("test-group", serviceName).Return(nil) + m.Get(gomockinternal.AContext(), "test-group").Return(resources.Group{}, notFoundError) }, }, { - name: "resource group deletion fails", - expectedError: "failed to delete resource group my-rg: #: Internal Server Error: StatusCode=500", + name: "error occurs when deleting resource group", + expectedError: "failed to delete resource test-group/test-group (service: groups): #: Internal Server Error: StatusCode=500", expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.ResourceGroup().AnyTimes().Return("my-rg") - s.ClusterName().AnyTimes().Return("fake-cluster") - gomock.InOrder( - m.Get(gomockinternal.AContext(), "my-rg").Return(resources.Group{ - Tags: converters.TagsToMap(infrav1.Tags{ - "Name": "my-rg", - "sigs.k8s.io_cluster-api-provider-azure_cluster_fake-cluster": "owned", - "sigs.k8s.io_cluster-api-provider-azure_role": "common", - }), - }, nil), - m.Delete(gomockinternal.AContext(), "my-rg").Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")), - ) + s.GroupSpec().AnyTimes().Return(&fakeGroupSpec) + s.GetLongRunningOperationState("test-group", serviceName).Return(nil) + m.Get(gomockinternal.AContext(), "test-group").Return(sampleManagedGroup, nil) + s.ClusterName().Return("test-cluster") + m.DeleteAsync(gomockinternal.AContext(), &fakeGroupSpec).Return(nil, internalError) + s.UpdateDeleteStatus(infrav1.ResourceGroupReadyCondition, serviceName, gomockinternal.ErrStrEq("failed to delete resource test-group/test-group (service: groups): #: Internal Server Error: StatusCode=500")) }, }, { - name: "resource group deletion successfully", + name: "context deadline exceeded while deleting resource group", + expectedError: "transient reconcile error occurred: operation type DELETE on Azure resource test-group/test-group is not done. Object will be requeued after 15s", + expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder) { + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + s.GroupSpec().AnyTimes().Return(&fakeGroupSpec) + s.GetLongRunningOperationState("test-group", serviceName).Return(nil) + m.Get(gomockinternal.AContext(), "test-group").Return(sampleManagedGroup, nil) + s.ClusterName().Return("test-cluster") + m.DeleteAsync(gomockinternal.AContext(), &fakeGroupSpec).Return(&azureautorest.Future{}, ctxExceededError) + s.SetLongRunningOperationState(gomock.AssignableToTypeOf(&infrav1.Future{})) + s.UpdateDeleteStatus(infrav1.ResourceGroupReadyCondition, serviceName, gomockinternal.ErrStrEq("transient reconcile error occurred: operation type DELETE on Azure resource test-group/test-group is not done. Object will be requeued after 15s")) + }, + }, + { + name: "delete the resource group successfully", expectedError: "", expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.ResourceGroup().AnyTimes().Return("my-rg") - s.ClusterName().AnyTimes().Return("fake-cluster") - gomock.InOrder( - m.Get(gomockinternal.AContext(), "my-rg").Return(resources.Group{ - Tags: converters.TagsToMap(infrav1.Tags{ - "Name": "my-rg", - "sigs.k8s.io_cluster-api-provider-azure_cluster_fake-cluster": "owned", - "sigs.k8s.io_cluster-api-provider-azure_role": "common", - }), - }, nil), - m.Delete(gomockinternal.AContext(), "my-rg").Return(nil), - ) + s.GroupSpec().AnyTimes().Return(&fakeGroupSpec) + s.GetLongRunningOperationState("test-group", serviceName).Return(nil) + m.Get(gomockinternal.AContext(), "test-group").Return(sampleManagedGroup, nil) + s.ClusterName().Return("test-cluster") + m.DeleteAsync(gomockinternal.AContext(), &fakeGroupSpec).Return(nil, nil) + s.UpdateDeleteStatus(infrav1.ResourceGroupReadyCondition, serviceName, nil) }, }, } @@ -233,7 +248,7 @@ func TestDeleteGroups(t *testing.T) { err := s.Delete(context.TODO()) if tc.expectedError != "" { g.Expect(err).To(HaveOccurred()) - g.Expect(err).To(MatchError(tc.expectedError)) + g.Expect(err.Error()).To(ContainSubstring(tc.expectedError)) } else { g.Expect(err).NotTo(HaveOccurred()) } diff --git a/azure/services/groups/mock_groups/client_mock.go b/azure/services/groups/mock_groups/client_mock.go index b0ff7fe680c..e16d83a17df 100644 --- a/azure/services/groups/mock_groups/client_mock.go +++ b/azure/services/groups/mock_groups/client_mock.go @@ -25,7 +25,9 @@ import ( reflect "reflect" resources "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-05-01/resources" + 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. @@ -51,33 +53,34 @@ func (m *Mockclient) EXPECT() *MockclientMockRecorder { return m.recorder } -// CreateOrUpdate mocks base method. -func (m *Mockclient) CreateOrUpdate(arg0 context.Context, arg1 string, arg2 resources.Group) (resources.Group, error) { +// CreateOrUpdateAsync mocks base method. +func (m *Mockclient) CreateOrUpdateAsync(arg0 context.Context, arg1 azure0.ResourceSpecGetter) (azure.FutureAPI, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateOrUpdate", arg0, arg1, arg2) - ret0, _ := ret[0].(resources.Group) + ret := m.ctrl.Call(m, "CreateOrUpdateAsync", arg0, arg1) + ret0, _ := ret[0].(azure.FutureAPI) ret1, _ := ret[1].(error) return ret0, ret1 } -// CreateOrUpdate indicates an expected call of CreateOrUpdate. -func (mr *MockclientMockRecorder) CreateOrUpdate(arg0, arg1, arg2 interface{}) *gomock.Call { +// CreateOrUpdateAsync indicates an expected call of CreateOrUpdateAsync. +func (mr *MockclientMockRecorder) CreateOrUpdateAsync(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdate", reflect.TypeOf((*Mockclient)(nil).CreateOrUpdate), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*Mockclient)(nil).CreateOrUpdateAsync), arg0, arg1) } -// Delete mocks base method. -func (m *Mockclient) Delete(arg0 context.Context, arg1 string) error { +// 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, "Delete", arg0, arg1) - ret0, _ := ret[0].(error) - return ret0 + ret := m.ctrl.Call(m, "DeleteAsync", arg0, arg1) + ret0, _ := ret[0].(azure.FutureAPI) + ret1, _ := ret[1].(error) + return ret0, ret1 } -// Delete indicates an expected call of Delete. -func (mr *MockclientMockRecorder) Delete(arg0, arg1 interface{}) *gomock.Call { +// 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, "Delete", reflect.TypeOf((*Mockclient)(nil).Delete), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAsync", reflect.TypeOf((*Mockclient)(nil).DeleteAsync), arg0, arg1) } // Get mocks base method. @@ -94,3 +97,18 @@ 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) +} diff --git a/azure/services/groups/mock_groups/groups_mock.go b/azure/services/groups/mock_groups/groups_mock.go index 4879cd69dde..58229d9597f 100644 --- a/azure/services/groups/mock_groups/groups_mock.go +++ b/azure/services/groups/mock_groups/groups_mock.go @@ -27,6 +27,8 @@ import ( logr "github.com/go-logr/logr" gomock "github.com/golang/mock/gomock" v1alpha4 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" + azure "sigs.k8s.io/cluster-api-provider-azure/azure" + v1alpha40 "sigs.k8s.io/cluster-api/api/v1alpha4" ) // MockGroupScope is a mock of GroupScope interface. @@ -52,20 +54,6 @@ func (m *MockGroupScope) EXPECT() *MockGroupScopeMockRecorder { return m.recorder } -// AdditionalTags mocks base method. -func (m *MockGroupScope) AdditionalTags() v1alpha4.Tags { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AdditionalTags") - ret0, _ := ret[0].(v1alpha4.Tags) - return ret0 -} - -// AdditionalTags indicates an expected call of AdditionalTags. -func (mr *MockGroupScopeMockRecorder) AdditionalTags() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AdditionalTags", reflect.TypeOf((*MockGroupScope)(nil).AdditionalTags)) -} - // Authorizer mocks base method. func (m *MockGroupScope) Authorizer() autorest.Authorizer { m.ctrl.T.Helper() @@ -80,20 +68,6 @@ func (mr *MockGroupScopeMockRecorder) Authorizer() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Authorizer", reflect.TypeOf((*MockGroupScope)(nil).Authorizer)) } -// AvailabilitySetEnabled mocks base method. -func (m *MockGroupScope) 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 *MockGroupScopeMockRecorder) AvailabilitySetEnabled() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AvailabilitySetEnabled", reflect.TypeOf((*MockGroupScope)(nil).AvailabilitySetEnabled)) -} - // BaseURI mocks base method. func (m *MockGroupScope) BaseURI() string { m.ctrl.T.Helper() @@ -150,20 +124,6 @@ func (mr *MockGroupScopeMockRecorder) CloudEnvironment() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CloudEnvironment", reflect.TypeOf((*MockGroupScope)(nil).CloudEnvironment)) } -// CloudProviderConfigOverrides mocks base method. -func (m *MockGroupScope) CloudProviderConfigOverrides() *v1alpha4.CloudProviderConfigOverrides { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CloudProviderConfigOverrides") - ret0, _ := ret[0].(*v1alpha4.CloudProviderConfigOverrides) - return ret0 -} - -// CloudProviderConfigOverrides indicates an expected call of CloudProviderConfigOverrides. -func (mr *MockGroupScopeMockRecorder) CloudProviderConfigOverrides() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CloudProviderConfigOverrides", reflect.TypeOf((*MockGroupScope)(nil).CloudProviderConfigOverrides)) -} - // ClusterName mocks base method. func (m *MockGroupScope) ClusterName() string { m.ctrl.T.Helper() @@ -178,6 +138,18 @@ func (mr *MockGroupScopeMockRecorder) ClusterName() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClusterName", reflect.TypeOf((*MockGroupScope)(nil).ClusterName)) } +// DeleteLongRunningOperationState mocks base method. +func (m *MockGroupScope) DeleteLongRunningOperationState(arg0, arg1 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "DeleteLongRunningOperationState", arg0, arg1) +} + +// DeleteLongRunningOperationState indicates an expected call of DeleteLongRunningOperationState. +func (mr *MockGroupScopeMockRecorder) DeleteLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteLongRunningOperationState", reflect.TypeOf((*MockGroupScope)(nil).DeleteLongRunningOperationState), arg0, arg1) +} + // Enabled mocks base method. func (m *MockGroupScope) Enabled() bool { m.ctrl.T.Helper() @@ -209,6 +181,34 @@ func (mr *MockGroupScopeMockRecorder) Error(err, msg interface{}, keysAndValues return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Error", reflect.TypeOf((*MockGroupScope)(nil).Error), varargs...) } +// GetLongRunningOperationState mocks base method. +func (m *MockGroupScope) GetLongRunningOperationState(arg0, arg1 string) *v1alpha4.Future { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetLongRunningOperationState", arg0, arg1) + ret0, _ := ret[0].(*v1alpha4.Future) + return ret0 +} + +// GetLongRunningOperationState indicates an expected call of GetLongRunningOperationState. +func (mr *MockGroupScopeMockRecorder) GetLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLongRunningOperationState", reflect.TypeOf((*MockGroupScope)(nil).GetLongRunningOperationState), arg0, arg1) +} + +// GroupSpec mocks base method. +func (m *MockGroupScope) GroupSpec() azure.ResourceSpecGetter { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GroupSpec") + ret0, _ := ret[0].(azure.ResourceSpecGetter) + return ret0 +} + +// GroupSpec indicates an expected call of GroupSpec. +func (mr *MockGroupScopeMockRecorder) GroupSpec() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GroupSpec", reflect.TypeOf((*MockGroupScope)(nil).GroupSpec)) +} + // HashKey mocks base method. func (m *MockGroupScope) HashKey() string { m.ctrl.T.Helper() @@ -240,32 +240,16 @@ func (mr *MockGroupScopeMockRecorder) Info(msg interface{}, keysAndValues ...int return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Info", reflect.TypeOf((*MockGroupScope)(nil).Info), varargs...) } -// Location mocks base method. -func (m *MockGroupScope) Location() string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Location") - ret0, _ := ret[0].(string) - return ret0 -} - -// Location indicates an expected call of Location. -func (mr *MockGroupScopeMockRecorder) Location() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Location", reflect.TypeOf((*MockGroupScope)(nil).Location)) -} - -// ResourceGroup mocks base method. -func (m *MockGroupScope) ResourceGroup() string { +// SetLongRunningOperationState mocks base method. +func (m *MockGroupScope) SetLongRunningOperationState(arg0 *v1alpha4.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 *MockGroupScopeMockRecorder) ResourceGroup() *gomock.Call { +// SetLongRunningOperationState indicates an expected call of SetLongRunningOperationState. +func (mr *MockGroupScopeMockRecorder) SetLongRunningOperationState(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResourceGroup", reflect.TypeOf((*MockGroupScope)(nil).ResourceGroup)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLongRunningOperationState", reflect.TypeOf((*MockGroupScope)(nil).SetLongRunningOperationState), arg0) } // SubscriptionID mocks base method. @@ -296,6 +280,42 @@ func (mr *MockGroupScopeMockRecorder) TenantID() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TenantID", reflect.TypeOf((*MockGroupScope)(nil).TenantID)) } +// UpdateDeleteStatus mocks base method. +func (m *MockGroupScope) UpdateDeleteStatus(arg0 v1alpha40.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 *MockGroupScopeMockRecorder) UpdateDeleteStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateDeleteStatus", reflect.TypeOf((*MockGroupScope)(nil).UpdateDeleteStatus), arg0, arg1, arg2) +} + +// UpdatePatchStatus mocks base method. +func (m *MockGroupScope) UpdatePatchStatus(arg0 v1alpha40.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 *MockGroupScopeMockRecorder) UpdatePatchStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePatchStatus", reflect.TypeOf((*MockGroupScope)(nil).UpdatePatchStatus), arg0, arg1, arg2) +} + +// UpdatePutStatus mocks base method. +func (m *MockGroupScope) UpdatePutStatus(arg0 v1alpha40.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 *MockGroupScopeMockRecorder) UpdatePutStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePutStatus", reflect.TypeOf((*MockGroupScope)(nil).UpdatePutStatus), arg0, arg1, arg2) +} + // V mocks base method. func (m *MockGroupScope) V(level int) logr.Logger { m.ctrl.T.Helper() diff --git a/azure/services/groups/spec.go b/azure/services/groups/spec.go new file mode 100644 index 00000000000..ff04e6adaa2 --- /dev/null +++ b/azure/services/groups/spec.go @@ -0,0 +1,61 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package groups + +import ( + "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-05-01/resources" + "github.com/Azure/go-autorest/autorest/to" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" + "sigs.k8s.io/cluster-api-provider-azure/azure/converters" +) + +// GroupSpec defines the specification for a Resource Group. +type GroupSpec struct { + Name string + Location string + ClusterName string + AdditionalTags infrav1.Tags +} + +func (s *GroupSpec) ResourceName() string { + return s.Name +} + +func (s *GroupSpec) ResourceGroupName() string { + return s.Name +} + +func (s *GroupSpec) OwnerResourceName() string { + return "" // not applicable +} + +func (s *GroupSpec) Parameters(existing interface{}) (interface{}, error) { + if existing != nil { + // rg already exists, nothing to update. + return nil, nil + } + return resources.Group{ + Location: to.StringPtr(s.Location), + 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, + })), + }, nil +} diff --git a/azure/services/loadbalancers/mock_loadbalancers/loadbalancers_mock.go b/azure/services/loadbalancers/mock_loadbalancers/loadbalancers_mock.go index afa1ab5312d..9f378d8ac08 100644 --- a/azure/services/loadbalancers/mock_loadbalancers/loadbalancers_mock.go +++ b/azure/services/loadbalancers/mock_loadbalancers/loadbalancers_mock.go @@ -21,6 +21,7 @@ limitations under the License. package mock_loadbalancers import ( + context "context" reflect "reflect" autorest "github.com/Azure/go-autorest/autorest" @@ -340,17 +341,18 @@ func (mr *MockLBScopeMockRecorder) IsIPv6Enabled() *gomock.Call { } // IsVnetManaged mocks base method. -func (m *MockLBScope) IsVnetManaged() bool { +func (m *MockLBScope) IsVnetManaged(arg0 context.Context) (bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsVnetManaged") + ret := m.ctrl.Call(m, "IsVnetManaged", arg0) ret0, _ := ret[0].(bool) - return ret0 + ret1, _ := ret[1].(error) + return ret0, ret1 } // IsVnetManaged indicates an expected call of IsVnetManaged. -func (mr *MockLBScopeMockRecorder) IsVnetManaged() *gomock.Call { +func (mr *MockLBScopeMockRecorder) IsVnetManaged(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsVnetManaged", reflect.TypeOf((*MockLBScope)(nil).IsVnetManaged)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsVnetManaged", reflect.TypeOf((*MockLBScope)(nil).IsVnetManaged), arg0) } // LBSpecs mocks base method. diff --git a/azure/services/natgateways/mock_natgateways/natgateways_mock.go b/azure/services/natgateways/mock_natgateways/natgateways_mock.go index 41ca0506173..d912a869546 100644 --- a/azure/services/natgateways/mock_natgateways/natgateways_mock.go +++ b/azure/services/natgateways/mock_natgateways/natgateways_mock.go @@ -21,6 +21,7 @@ limitations under the License. package mock_natgateways import ( + context "context" reflect "reflect" autorest "github.com/Azure/go-autorest/autorest" @@ -340,17 +341,18 @@ func (mr *MockNatGatewayScopeMockRecorder) IsIPv6Enabled() *gomock.Call { } // IsVnetManaged mocks base method. -func (m *MockNatGatewayScope) IsVnetManaged() bool { +func (m *MockNatGatewayScope) IsVnetManaged(arg0 context.Context) (bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsVnetManaged") + ret := m.ctrl.Call(m, "IsVnetManaged", arg0) ret0, _ := ret[0].(bool) - return ret0 + ret1, _ := ret[1].(error) + return ret0, ret1 } // IsVnetManaged indicates an expected call of IsVnetManaged. -func (mr *MockNatGatewayScopeMockRecorder) IsVnetManaged() *gomock.Call { +func (mr *MockNatGatewayScopeMockRecorder) IsVnetManaged(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsVnetManaged", reflect.TypeOf((*MockNatGatewayScope)(nil).IsVnetManaged)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsVnetManaged", reflect.TypeOf((*MockNatGatewayScope)(nil).IsVnetManaged), arg0) } // Location mocks base method. diff --git a/azure/services/routetables/mock_routetables/routetables_mock.go b/azure/services/routetables/mock_routetables/routetables_mock.go index 4ebc9949f09..980d1b0082f 100644 --- a/azure/services/routetables/mock_routetables/routetables_mock.go +++ b/azure/services/routetables/mock_routetables/routetables_mock.go @@ -21,6 +21,7 @@ limitations under the License. package mock_routetables import ( + context "context" reflect "reflect" autorest "github.com/Azure/go-autorest/autorest" @@ -340,17 +341,18 @@ func (mr *MockRouteTableScopeMockRecorder) IsIPv6Enabled() *gomock.Call { } // IsVnetManaged mocks base method. -func (m *MockRouteTableScope) IsVnetManaged() bool { +func (m *MockRouteTableScope) IsVnetManaged(arg0 context.Context) (bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsVnetManaged") + ret := m.ctrl.Call(m, "IsVnetManaged", arg0) ret0, _ := ret[0].(bool) - return ret0 + ret1, _ := ret[1].(error) + return ret0, ret1 } // IsVnetManaged indicates an expected call of IsVnetManaged. -func (mr *MockRouteTableScopeMockRecorder) IsVnetManaged() *gomock.Call { +func (mr *MockRouteTableScopeMockRecorder) IsVnetManaged(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsVnetManaged", reflect.TypeOf((*MockRouteTableScope)(nil).IsVnetManaged)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsVnetManaged", reflect.TypeOf((*MockRouteTableScope)(nil).IsVnetManaged), arg0) } // Location mocks base method. diff --git a/azure/services/scalesets/client.go b/azure/services/scalesets/client.go index 5526813276c..c13d8fe58ce 100644 --- a/azure/services/scalesets/client.go +++ b/azure/services/scalesets/client.go @@ -31,7 +31,6 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" "sigs.k8s.io/cluster-api-provider-azure/azure" - "sigs.k8s.io/cluster-api-provider-azure/azure/converters" "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) @@ -213,7 +212,7 @@ func (ac *AzureClient) GetResultIfDone(ctx context.Context, future *infrav1.Futu } switch future.Type { - case converters.PatchFuture: + case infrav1.PatchFuture: var future compute.VirtualMachineScaleSetsUpdateFuture if err := json.Unmarshal(futureData, &future); err != nil { return compute.VirtualMachineScaleSet{}, errors.Wrap(err, "failed to unmarshal future data") @@ -223,7 +222,7 @@ func (ac *AzureClient) GetResultIfDone(ctx context.Context, future *infrav1.Futu FutureAPI: &future, result: future.Result, } - case converters.PutFuture: + case infrav1.PutFuture: var future compute.VirtualMachineScaleSetsCreateOrUpdateFuture if err := json.Unmarshal(futureData, &future); err != nil { return compute.VirtualMachineScaleSet{}, errors.Wrap(err, "failed to unmarshal future data") @@ -233,7 +232,7 @@ func (ac *AzureClient) GetResultIfDone(ctx context.Context, future *infrav1.Futu FutureAPI: &future, result: future.Result, } - case converters.DeleteFuture: + case infrav1.DeleteFuture: var future compute.VirtualMachineScaleSetsDeleteFuture if err := json.Unmarshal(futureData, &future); err != nil { return compute.VirtualMachineScaleSet{}, errors.Wrap(err, "failed to unmarshal future data") diff --git a/azure/services/scalesets/mock_scalesets/client_mock.go b/azure/services/scalesets/mock_scalesets/client_mock.go index 41efba51bb4..98ccfa8106b 100644 --- a/azure/services/scalesets/mock_scalesets/client_mock.go +++ b/azure/services/scalesets/mock_scalesets/client_mock.go @@ -25,7 +25,6 @@ import ( reflect "reflect" compute "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-04-01/compute" - network "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-11-01/network" autorest "github.com/Azure/go-autorest/autorest" gomock "github.com/golang/mock/gomock" v1alpha4 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" diff --git a/azure/services/scalesets/mock_scalesets/scalesets_mock.go b/azure/services/scalesets/mock_scalesets/scalesets_mock.go index eddc8adc9ba..af3c8a90a94 100644 --- a/azure/services/scalesets/mock_scalesets/scalesets_mock.go +++ b/azure/services/scalesets/mock_scalesets/scalesets_mock.go @@ -380,30 +380,6 @@ func (mr *MockScaleSetScopeMockRecorder) SetAnnotation(arg0, arg1 interface{}) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetAnnotation", reflect.TypeOf((*MockScaleSetScope)(nil).SetAnnotation), arg0, arg1) } -// SetConditionFalse mocks base method. -func (m *MockScaleSetScope) SetConditionFalse(arg0 v1alpha40.ConditionType, arg1 string, arg2 v1alpha40.ConditionSeverity) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "SetConditionFalse", arg0, arg1, arg2) -} - -// SetConditionFalse indicates an expected call of SetConditionFalse. -func (mr *MockScaleSetScopeMockRecorder) SetConditionFalse(arg0, arg1, arg2 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetConditionFalse", reflect.TypeOf((*MockScaleSetScope)(nil).SetConditionFalse), arg0, arg1, arg2) -} - -// SetConditionTrue mocks base method. -func (m *MockScaleSetScope) SetConditionTrue(arg0 v1alpha40.ConditionType) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "SetConditionTrue", arg0) -} - -// SetConditionTrue indicates an expected call of SetConditionTrue. -func (mr *MockScaleSetScopeMockRecorder) SetConditionTrue(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetConditionTrue", reflect.TypeOf((*MockScaleSetScope)(nil).SetConditionTrue), arg0) -} - // SetLongRunningOperationState mocks base method. func (m *MockScaleSetScope) SetLongRunningOperationState(arg0 *v1alpha4.Future) { m.ctrl.T.Helper() @@ -468,6 +444,42 @@ func (mr *MockScaleSetScopeMockRecorder) TenantID() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TenantID", reflect.TypeOf((*MockScaleSetScope)(nil).TenantID)) } +// UpdateDeleteStatus mocks base method. +func (m *MockScaleSetScope) UpdateDeleteStatus(arg0 v1alpha40.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 *MockScaleSetScopeMockRecorder) UpdateDeleteStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateDeleteStatus", reflect.TypeOf((*MockScaleSetScope)(nil).UpdateDeleteStatus), arg0, arg1, arg2) +} + +// UpdatePatchStatus mocks base method. +func (m *MockScaleSetScope) UpdatePatchStatus(arg0 v1alpha40.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 *MockScaleSetScopeMockRecorder) UpdatePatchStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePatchStatus", reflect.TypeOf((*MockScaleSetScope)(nil).UpdatePatchStatus), arg0, arg1, arg2) +} + +// UpdatePutStatus mocks base method. +func (m *MockScaleSetScope) UpdatePutStatus(arg0 v1alpha40.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 *MockScaleSetScopeMockRecorder) UpdatePutStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePutStatus", reflect.TypeOf((*MockScaleSetScope)(nil).UpdatePutStatus), arg0, arg1, arg2) +} + // V mocks base method. func (m *MockScaleSetScope) V(level int) logr.Logger { m.ctrl.T.Helper() diff --git a/azure/services/scalesets/scalesets.go b/azure/services/scalesets/scalesets.go index 39447a7ee8a..c8b46432e1b 100644 --- a/azure/services/scalesets/scalesets.go +++ b/azure/services/scalesets/scalesets.go @@ -37,7 +37,7 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) -const serviceName = "scalesets" +const ServiceName = "scalesets" type ( // ScaleSetScope defines the scope interface for a scale sets service. @@ -195,7 +195,7 @@ func (s *Service) Delete(ctx context.Context) error { return errors.Wrapf(err, "failed to delete VMSS %s in resource group %s", vmssSpec.Name, s.Scope.ResourceGroup()) } - future, err = converters.SDKToFuture(sdkFuture, converters.DeleteFuture, serviceName, vmssSpec.Name, s.Scope.ResourceGroup()) + future, err = converters.SDKToFuture(sdkFuture, infrav1.DeleteFuture, serviceName, vmssSpec.Name, s.Scope.ResourceGroup()) if err != nil { return errors.Wrapf(err, "failed to delete VMSS %s in resource group %s", vmssSpec.Name, s.Scope.ResourceGroup()) } @@ -229,7 +229,7 @@ func (s *Service) createVMSS(ctx context.Context) (*infrav1.Future, error) { return nil, errors.Wrap(err, "cannot create VMSS") } - future, err := converters.SDKToFuture(sdkFuture, converters.PutFuture, serviceName, spec.Name, s.Scope.ResourceGroup()) + future, err := converters.SDKToFuture(sdkFuture, infrav1.PutFuture, serviceName, spec.Name, s.Scope.ResourceGroup()) if err != nil { return future, errors.Wrapf(err, "failed to delete VMSS %s in resource group %s", spec.Name, s.Scope.ResourceGroup()) } @@ -284,7 +284,7 @@ func (s *Service) patchVMSSIfNeeded(ctx context.Context, infraVMSS *azure.VMSS) return nil, errors.Wrap(err, "failed updating VMSS") } - future, err := converters.SDKToFuture(sdkFuture, converters.PatchFuture, serviceName, spec.Name, s.Scope.ResourceGroup()) + future, err := converters.SDKToFuture(sdkFuture, infrav1.PatchFuture, serviceName, spec.Name, s.Scope.ResourceGroup()) if err != nil { return nil, errors.Wrapf(err, "failed to delete VMSS %s in resource group %s", spec.Name, s.Scope.ResourceGroup()) } @@ -348,7 +348,7 @@ func (s *Service) validateSpec(ctx context.Context) error { } for _, zone := range zones { - if disks.ManagedDisk != nil && disks.ManagedDisk.StorageAccountType == string(compute.UltraSSDLRS) && !sku.HasLocationCapability(resourceskus.UltraSSDAvailable, location, zone) { + if disks.ManagedDisk != nil && disks.ManagedDisk.StorageAccountType == string(compute.StorageAccountTypesUltraSSDLRS) && !sku.HasLocationCapability(resourceskus.UltraSSDAvailable, location, zone) { return azure.WithTerminalError(fmt.Errorf("vm size %s does not support ultra disks in location %s. select a different vm size or disable ultra disks", spec.Size, location)) } } @@ -493,7 +493,7 @@ func (s *Service) buildVMSSFromSpec(ctx context.Context, vmssSpec azure.ScaleSet } for _, dataDisk := range vmssSpec.DataDisks { - if dataDisk.ManagedDisk != nil && dataDisk.ManagedDisk.StorageAccountType == string(compute.UltraSSDLRS) { + if dataDisk.ManagedDisk != nil && dataDisk.ManagedDisk.StorageAccountType == string(compute.StorageAccountTypesUltraSSDLRS) { vmss.VirtualMachineScaleSetProperties.AdditionalCapabilities = &compute.AdditionalCapabilities{ UltraSSDEnabled: to.BoolPtr(true), } diff --git a/azure/services/scalesets/scalesets_test.go b/azure/services/scalesets/scalesets_test.go index 098d734412b..d888b4b56aa 100644 --- a/azure/services/scalesets/scalesets_test.go +++ b/azure/services/scalesets/scalesets_test.go @@ -225,13 +225,13 @@ func TestGetExistingVMSS(t *testing.T) { func TestReconcileVMSS(t *testing.T) { var ( putFuture = &infrav1.Future{ - Type: PutFuture, + Type: infrav1.PutFuture, ResourceGroup: defaultResourceGroup, Name: defaultVMSSName, } patchFuture = &infrav1.Future{ - Type: PatchFuture, + Type: infrav1.PatchFuture, ResourceGroup: defaultResourceGroup, Name: defaultVMSSName, } @@ -273,7 +273,7 @@ func TestReconcileVMSS(t *testing.T) { createdVMSS := newDefaultVMSS("VM_SIZE") instances := newDefaultInstances() _ = setupDefaultVMSSInProgressOperationDoneExpectations(s, m, createdVMSS, instances) - s.SetLongRunningOperationState(nil) + s.DeleteLongRunningOperationState(defaultSpec.Name, serviceName) }, }, { @@ -285,7 +285,7 @@ func TestReconcileVMSS(t *testing.T) { createdVMSS := newDefaultWindowsVMSS() instances := newDefaultInstances() _ = setupDefaultVMSSInProgressOperationDoneExpectations(s, m, createdVMSS, instances) - s.SetLongRunningOperationState(nil) + s.DeleteLongRunningOperationState(defaultSpec.Name, serviceName) }, }, { @@ -624,11 +624,11 @@ func TestDeleteVMSS(t *testing.T) { s.ResourceGroup().AnyTimes().Return("my-existing-rg") s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) future := &infrav1.Future{} - s.GetLongRunningOperationState().Return(future) + s.GetLongRunningOperationState("my-existing-vmss", serviceName).Return(future) m.GetResultIfDone(gomockinternal.AContext(), future).Return(compute.VirtualMachineScaleSet{}, nil) m.Get(gomockinternal.AContext(), "my-existing-rg", "my-existing-vmss"). Return(compute.VirtualMachineScaleSet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - s.SetLongRunningOperationState(nil) + s.DeleteLongRunningOperationState("my-existing-vmss", serviceName) }, }, { @@ -642,7 +642,7 @@ func TestDeleteVMSS(t *testing.T) { }).AnyTimes() s.ResourceGroup().AnyTimes().Return(resourceGroup) s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.GetLongRunningOperationState().Return(nil) + s.GetLongRunningOperationState(name, serviceName).Return(nil) m.DeleteAsync(gomockinternal.AContext(), resourceGroup, name). Return(nil, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) m.Get(gomockinternal.AContext(), resourceGroup, name). @@ -660,7 +660,7 @@ func TestDeleteVMSS(t *testing.T) { }).AnyTimes() s.ResourceGroup().AnyTimes().Return(resourceGroup) s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.GetLongRunningOperationState().Return(nil) + s.GetLongRunningOperationState(name, serviceName).Return(nil) m.DeleteAsync(gomockinternal.AContext(), resourceGroup, name). Return(nil, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) m.Get(gomockinternal.AContext(), resourceGroup, name). @@ -1203,12 +1203,12 @@ func setupDefaultVMSSInProgressOperationDoneExpectations(s *mock_scalesets.MockS createdVMSS.ProvisioningState = to.StringPtr(string(infrav1.Succeeded)) setupDefaultVMSSExpectations(s) future := &infrav1.Future{ - Type: PutFuture, + Type: infrav1.PutFuture, ResourceGroup: defaultResourceGroup, Name: defaultVMSSName, Data: "", } - s.GetLongRunningOperationState().Return(future) + s.GetLongRunningOperationState(defaultVMSSName, serviceName).Return(future) m.GetResultIfDone(gomockinternal.AContext(), future).Return(createdVMSS, nil).AnyTimes() m.ListInstances(gomockinternal.AContext(), defaultResourceGroup, defaultVMSSName).Return(instances, nil).AnyTimes() s.MaxSurge().Return(1, nil) @@ -1219,7 +1219,7 @@ func setupDefaultVMSSInProgressOperationDoneExpectations(s *mock_scalesets.MockS func setupDefaultVMSSStartCreatingExpectations(s *mock_scalesets.MockScaleSetScopeMockRecorder, m *mock_scalesets.MockClientMockRecorder) { setupDefaultVMSSExpectations(s) - s.GetLongRunningOperationState().Return(nil) + s.GetLongRunningOperationState(defaultVMSSName, serviceName).Return(nil) m.Get(gomockinternal.AContext(), defaultResourceGroup, defaultVMSSName). Return(compute.VirtualMachineScaleSet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) } @@ -1285,7 +1285,7 @@ func setupVMSSExpectationsWithoutVMImage(s *mock_scalesets.MockScaleSetScopeMock func setupDefaultVMSSUpdateExpectations(s *mock_scalesets.MockScaleSetScopeMockRecorder) { setupUpdateVMSSExpectations(s) s.SetProviderID(azure.ProviderIDPrefix + "vmss-id") - s.GetLongRunningOperationState().Return(nil) + s.GetLongRunningOperationState(defaultVMSSName, serviceName).Return(nil) s.MaxSurge().Return(1, nil) s.SetVMSSState(gomock.Any()) } diff --git a/azure/services/scalesetvms/client.go b/azure/services/scalesetvms/client.go index 870d3173e08..ad7b324c399 100644 --- a/azure/services/scalesetvms/client.go +++ b/azure/services/scalesetvms/client.go @@ -29,7 +29,6 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" "sigs.k8s.io/cluster-api-provider-azure/azure" - "sigs.k8s.io/cluster-api-provider-azure/azure/converters" "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) @@ -94,7 +93,7 @@ func (ac *azureClient) GetResultIfDone(ctx context.Context, future *infrav1.Futu } switch future.Type { - case converters.DeleteFuture: + case infrav1.DeleteFuture: var future compute.VirtualMachineScaleSetVMsDeleteFuture if err := json.Unmarshal(futureData, &future); err != nil { return compute.VirtualMachineScaleSetVM{}, errors.Wrap(err, "failed to unmarshal future data") @@ -147,7 +146,7 @@ func (ac *azureClient) DeleteAsync(ctx context.Context, resourceGroupName, vmssN } return &infrav1.Future{ - Type: converters.DeleteFuture, + Type: infrav1.DeleteFuture, ResourceGroup: resourceGroupName, Name: instanceID, Data: base64.URLEncoding.EncodeToString(jsonData), diff --git a/azure/services/scalesetvms/mock_scalesetvms/scalesetvms_mock.go b/azure/services/scalesetvms/mock_scalesetvms/scalesetvms_mock.go index aa26ab9c5b7..207151214e7 100644 --- a/azure/services/scalesetvms/mock_scalesetvms/scalesetvms_mock.go +++ b/azure/services/scalesetvms/mock_scalesetvms/scalesetvms_mock.go @@ -324,30 +324,6 @@ func (mr *MockScaleSetVMScopeMockRecorder) ScaleSetName() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ScaleSetName", reflect.TypeOf((*MockScaleSetVMScope)(nil).ScaleSetName)) } -// SetConditionFalse mocks base method. -func (m *MockScaleSetVMScope) SetConditionFalse(arg0 v1alpha40.ConditionType, arg1 string, arg2 v1alpha40.ConditionSeverity) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "SetConditionFalse", arg0, arg1, arg2) -} - -// SetConditionFalse indicates an expected call of SetConditionFalse. -func (mr *MockScaleSetVMScopeMockRecorder) SetConditionFalse(arg0, arg1, arg2 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetConditionFalse", reflect.TypeOf((*MockScaleSetVMScope)(nil).SetConditionFalse), arg0, arg1, arg2) -} - -// SetConditionTrue mocks base method. -func (m *MockScaleSetVMScope) SetConditionTrue(arg0 v1alpha40.ConditionType) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "SetConditionTrue", arg0) -} - -// SetConditionTrue indicates an expected call of SetConditionTrue. -func (mr *MockScaleSetVMScopeMockRecorder) SetConditionTrue(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetConditionTrue", reflect.TypeOf((*MockScaleSetVMScope)(nil).SetConditionTrue), arg0) -} - // SetLongRunningOperationState mocks base method. func (m *MockScaleSetVMScope) SetLongRunningOperationState(arg0 *v1alpha4.Future) { m.ctrl.T.Helper() @@ -400,6 +376,42 @@ func (mr *MockScaleSetVMScopeMockRecorder) TenantID() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TenantID", reflect.TypeOf((*MockScaleSetVMScope)(nil).TenantID)) } +// UpdateDeleteStatus mocks base method. +func (m *MockScaleSetVMScope) UpdateDeleteStatus(arg0 v1alpha40.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 *MockScaleSetVMScopeMockRecorder) UpdateDeleteStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateDeleteStatus", reflect.TypeOf((*MockScaleSetVMScope)(nil).UpdateDeleteStatus), arg0, arg1, arg2) +} + +// UpdatePatchStatus mocks base method. +func (m *MockScaleSetVMScope) UpdatePatchStatus(arg0 v1alpha40.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 *MockScaleSetVMScopeMockRecorder) UpdatePatchStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePatchStatus", reflect.TypeOf((*MockScaleSetVMScope)(nil).UpdatePatchStatus), arg0, arg1, arg2) +} + +// UpdatePutStatus mocks base method. +func (m *MockScaleSetVMScope) UpdatePutStatus(arg0 v1alpha40.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 *MockScaleSetVMScopeMockRecorder) UpdatePutStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePutStatus", reflect.TypeOf((*MockScaleSetVMScope)(nil).UpdatePutStatus), arg0, arg1, arg2) +} + // V mocks base method. func (m *MockScaleSetVMScope) V(level int) logr.Logger { m.ctrl.T.Helper() diff --git a/azure/services/scalesetvms/scalesetvms.go b/azure/services/scalesetvms/scalesetvms.go index fffd95c3137..737eaa36388 100644 --- a/azure/services/scalesetvms/scalesetvms.go +++ b/azure/services/scalesetvms/scalesetvms.go @@ -23,6 +23,7 @@ import ( "github.com/go-logr/logr" "github.com/pkg/errors" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" "sigs.k8s.io/cluster-api-provider-azure/azure" "sigs.k8s.io/cluster-api-provider-azure/azure/converters" "sigs.k8s.io/cluster-api-provider-azure/util/tele" @@ -103,7 +104,7 @@ func (s *Service) Delete(ctx context.Context) error { log.V(4).Info("entering delete") future := s.Scope.GetLongRunningOperationState(instanceID, serviceName) if future != nil { - if future.Type != converters.DeleteFuture { + if future.Type != infrav1.DeleteFuture { return azure.WithTransientError(errors.New("attempting to delete, non-delete operation in progress"), 30*time.Second) } diff --git a/azure/services/scalesetvms/scalesetvms_test.go b/azure/services/scalesetvms/scalesetvms_test.go index 40052d710d8..fcf7e78cf43 100644 --- a/azure/services/scalesetvms/scalesetvms_test.go +++ b/azure/services/scalesetvms/scalesetvms_test.go @@ -178,7 +178,7 @@ func TestService_Delete(t *testing.T) { s.ScaleSetName().Return("scaleset") s.GetLongRunningOperationState().Return(nil) future := &infrav1.Future{ - Type: DeleteFuture, + Type: infrav1.DeleteFuture, } m.DeleteAsync(gomock2.AContext(), "rg", "scaleset", "0").Return(future, nil) s.SetLongRunningOperationState(future) @@ -187,7 +187,7 @@ func TestService_Delete(t *testing.T) { }, CheckIsErr: true, Err: errors.Wrap(azure.WithTransientError(azure.NewOperationNotDoneError(&infrav1.Future{ - Type: DeleteFuture, + Type: infrav1.DeleteFuture, }), 15*time.Second), "failed to get result of long running operation"), }, { @@ -197,7 +197,7 @@ func TestService_Delete(t *testing.T) { s.InstanceID().Return("0") s.ScaleSetName().Return("scaleset") future := &infrav1.Future{ - Type: DeleteFuture, + Type: infrav1.DeleteFuture, } s.GetLongRunningOperationState().Return(future) m.GetResultIfDone(gomock2.AContext(), future).Return(compute.VirtualMachineScaleSetVM{}, nil) @@ -235,7 +235,7 @@ func TestService_Delete(t *testing.T) { s.InstanceID().Return("0") s.ScaleSetName().Return("scaleset") future := &infrav1.Future{ - Type: DeleteFuture, + Type: infrav1.DeleteFuture, } s.GetLongRunningOperationState().Return(future) m.GetResultIfDone(gomock2.AContext(), future).Return(compute.VirtualMachineScaleSetVM{}, errors.New("boom")) diff --git a/azure/services/securitygroups/client.go b/azure/services/securitygroups/client.go index e52d7447ac2..fe6ec5ce8a9 100644 --- a/azure/services/securitygroups/client.go +++ b/azure/services/securitygroups/client.go @@ -21,16 +21,20 @@ import ( "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" "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.SecurityGroup, error) - CreateOrUpdate(context.Context, string, string, network.SecurityGroup) error - Delete(context.Context, string, string) error + CreateOrUpdateAsync(context.Context, azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) + DeleteAsync(context.Context, azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) + IsDone(context.Context, azureautorest.FutureAPI) (bool, error) } // azureClient contains the Azure go-sdk Client. @@ -61,19 +65,29 @@ func (ac *azureClient) Get(ctx context.Context, resourceGroupName, sgName string return ac.securitygroups.Get(ctx, resourceGroupName, sgName, "") } -// CreateOrUpdate creates or updates a network security group in the specified resource group. -func (ac *azureClient) CreateOrUpdate(ctx context.Context, resourceGroupName string, sgName string, sg network.SecurityGroup) error { +// CreateOrUpdateAsync creates or updates a network security group in the specified resource group. +// 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) (azureautorest.FutureAPI, error) { ctx, span := tele.Tracer().Start(ctx, "securitygroups.AzureClient.CreateOrUpdate") defer span.End() + sg, err := ac.SecurityGroupParams(ctx, spec) + if err != nil { + return nil, err + } else if sg == nil { + // nothing to do here. + return nil, nil + } + var etag string if sg.Etag != nil { etag = *sg.Etag } - req, err := ac.securitygroups.CreateOrUpdatePreparer(ctx, resourceGroupName, sgName, sg) + req, err := ac.securitygroups.CreateOrUpdatePreparer(ctx, spec.ResourceGroupName(), spec.ResourceName(), *sg) if err != nil { err = autorest.NewErrorWithError(err, "network.SecurityGroupsClient", "CreateOrUpdate", nil, "Failure preparing request") - return err + return nil, err } if etag != "" { req.Header.Add("If-Match", etag) @@ -82,30 +96,90 @@ func (ac *azureClient) CreateOrUpdate(ctx context.Context, resourceGroupName str future, err := ac.securitygroups.CreateOrUpdateSender(req) if err != nil { err = autorest.NewErrorWithError(err, "network.SecurityGroupsClient", "CreateOrUpdate", future.Response(), "Failure sending request") - return err + return nil, err } + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + err = future.WaitForCompletionRef(ctx, ac.securitygroups.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 &future, err } _, err = future.Result(ac.securitygroups) - return err + // if the operation completed, return a nil future. + return nil, err } -// Delete deletes the specified network security group. -func (ac *azureClient) Delete(ctx context.Context, resourceGroupName, sgName string) error { - ctx, span := tele.Tracer().Start(ctx, "securitygroups.AzureClient.Delete") +// Delete deletes the specified network security group. 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) (azureautorest.FutureAPI, error) { + ctx, span := tele.Tracer().Start(ctx, "securitygroups.AzureClient.DeleteAsync") defer span.End() - future, err := ac.securitygroups.Delete(ctx, resourceGroupName, sgName) + future, err := ac.securitygroups.Delete(ctx, spec.ResourceGroupName(), spec.ResourceName()) if err != nil { - return err + return nil, err } + + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + err = future.WaitForCompletionRef(ctx, ac.securitygroups.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 &future, err } _, err = future.Result(ac.securitygroups) - return err + // if the operation completed, return a nil future. + return nil, err +} + +// IsDone returns true if the long-running operation has completed. +func (ac *azureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (bool, error) { + ctx, span := tele.Tracer().Start(ctx, "securitygroups.AzureClient.IsDone") + defer span.End() + + done, err := future.DoneWithContext(ctx, ac.securitygroups) + if err != nil { + return false, errors.Wrap(err, "failed checking if the operation was complete") + } + + return done, nil +} + +func (ac *azureClient) SecurityGroupParams(ctx context.Context, spec azure.ResourceSpecGetter) (*network.SecurityGroup, error) { + var params interface{} + + existingNSG, err := ac.Get(ctx, spec.ResourceGroupName(), spec.ResourceName()) + if azure.ResourceNotFound(err) { + // NSG doesn't exist, create it from scratch. + params, err = spec.Parameters(nil) + if err != nil { + return nil, errors.Wrapf(err, "failed to get desired parameters for nsg %s", spec.ResourceName()) + } + } else if err != nil { + return nil, errors.Wrapf(err, "failed to get NSG %s in %s", spec.ResourceName(), spec.ResourceGroupName()) + } else { + // NSG already exists + params, err = spec.Parameters(existingNSG) + if err != nil { + return nil, err + } + } + + sg, ok := params.(network.SecurityGroup) + if !ok { + if params == nil { + // nothing to do here. + return nil, nil + } + return nil, errors.Errorf("%T is not a network.SecurityGroup", params) + } + + return &sg, nil } diff --git a/azure/services/securitygroups/mock_securitygroups/client_mock.go b/azure/services/securitygroups/mock_securitygroups/client_mock.go index d3612742306..bcf0b788ed3 100644 --- a/azure/services/securitygroups/mock_securitygroups/client_mock.go +++ b/azure/services/securitygroups/mock_securitygroups/client_mock.go @@ -25,7 +25,9 @@ import ( reflect "reflect" network "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" + 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. @@ -51,32 +53,34 @@ func (m *Mockclient) EXPECT() *MockclientMockRecorder { return m.recorder } -// CreateOrUpdate mocks base method. -func (m *Mockclient) CreateOrUpdate(arg0 context.Context, arg1, arg2 string, arg3 network.SecurityGroup) error { +// CreateOrUpdateAsync mocks base method. +func (m *Mockclient) CreateOrUpdateAsync(arg0 context.Context, arg1 azure0.ResourceSpecGetter) (azure.FutureAPI, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateOrUpdate", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(error) - return ret0 + ret := m.ctrl.Call(m, "CreateOrUpdateAsync", arg0, arg1) + ret0, _ := ret[0].(azure.FutureAPI) + ret1, _ := ret[1].(error) + return ret0, ret1 } -// CreateOrUpdate indicates an expected call of CreateOrUpdate. -func (mr *MockclientMockRecorder) CreateOrUpdate(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { +// CreateOrUpdateAsync indicates an expected call of CreateOrUpdateAsync. +func (mr *MockclientMockRecorder) CreateOrUpdateAsync(arg0, arg1 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) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*Mockclient)(nil).CreateOrUpdateAsync), arg0, arg1) } -// Delete mocks base method. -func (m *Mockclient) Delete(arg0 context.Context, arg1, arg2 string) error { +// 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, "Delete", arg0, arg1, arg2) - ret0, _ := ret[0].(error) - return ret0 + ret := m.ctrl.Call(m, "DeleteAsync", arg0, arg1) + ret0, _ := ret[0].(azure.FutureAPI) + ret1, _ := ret[1].(error) + return ret0, ret1 } -// Delete indicates an expected call of Delete. -func (mr *MockclientMockRecorder) Delete(arg0, arg1, arg2 interface{}) *gomock.Call { +// 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, "Delete", reflect.TypeOf((*Mockclient)(nil).Delete), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAsync", reflect.TypeOf((*Mockclient)(nil).DeleteAsync), arg0, arg1) } // Get mocks base method. @@ -93,3 +97,18 @@ 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) } + +// 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) +} diff --git a/azure/services/securitygroups/mock_securitygroups/securitygroups_mock.go b/azure/services/securitygroups/mock_securitygroups/securitygroups_mock.go index 07c323e1b97..073434a7812 100644 --- a/azure/services/securitygroups/mock_securitygroups/securitygroups_mock.go +++ b/azure/services/securitygroups/mock_securitygroups/securitygroups_mock.go @@ -21,6 +21,7 @@ limitations under the License. package mock_securitygroups import ( + context "context" reflect "reflect" autorest "github.com/Azure/go-autorest/autorest" @@ -28,6 +29,7 @@ import ( gomock "github.com/golang/mock/gomock" v1alpha4 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" azure "sigs.k8s.io/cluster-api-provider-azure/azure" + v1alpha40 "sigs.k8s.io/cluster-api/api/v1alpha4" ) // MockNSGScope is a mock of NSGScope interface. @@ -53,48 +55,6 @@ func (m *MockNSGScope) EXPECT() *MockNSGScopeMockRecorder { return m.recorder } -// APIServerLBName mocks base method. -func (m *MockNSGScope) APIServerLBName() string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "APIServerLBName") - ret0, _ := ret[0].(string) - return ret0 -} - -// APIServerLBName indicates an expected call of APIServerLBName. -func (mr *MockNSGScopeMockRecorder) APIServerLBName() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "APIServerLBName", reflect.TypeOf((*MockNSGScope)(nil).APIServerLBName)) -} - -// APIServerLBPoolName mocks base method. -func (m *MockNSGScope) APIServerLBPoolName(arg0 string) string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "APIServerLBPoolName", arg0) - ret0, _ := ret[0].(string) - return ret0 -} - -// APIServerLBPoolName indicates an expected call of APIServerLBPoolName. -func (mr *MockNSGScopeMockRecorder) APIServerLBPoolName(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "APIServerLBPoolName", reflect.TypeOf((*MockNSGScope)(nil).APIServerLBPoolName), arg0) -} - -// AdditionalTags mocks base method. -func (m *MockNSGScope) AdditionalTags() v1alpha4.Tags { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AdditionalTags") - ret0, _ := ret[0].(v1alpha4.Tags) - return ret0 -} - -// AdditionalTags indicates an expected call of AdditionalTags. -func (mr *MockNSGScopeMockRecorder) AdditionalTags() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AdditionalTags", reflect.TypeOf((*MockNSGScope)(nil).AdditionalTags)) -} - // Authorizer mocks base method. func (m *MockNSGScope) Authorizer() autorest.Authorizer { m.ctrl.T.Helper() @@ -109,20 +69,6 @@ func (mr *MockNSGScopeMockRecorder) Authorizer() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Authorizer", reflect.TypeOf((*MockNSGScope)(nil).Authorizer)) } -// AvailabilitySetEnabled mocks base method. -func (m *MockNSGScope) 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 *MockNSGScopeMockRecorder) AvailabilitySetEnabled() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AvailabilitySetEnabled", reflect.TypeOf((*MockNSGScope)(nil).AvailabilitySetEnabled)) -} - // BaseURI mocks base method. func (m *MockNSGScope) BaseURI() string { m.ctrl.T.Helper() @@ -179,60 +125,16 @@ func (mr *MockNSGScopeMockRecorder) CloudEnvironment() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CloudEnvironment", reflect.TypeOf((*MockNSGScope)(nil).CloudEnvironment)) } -// CloudProviderConfigOverrides mocks base method. -func (m *MockNSGScope) CloudProviderConfigOverrides() *v1alpha4.CloudProviderConfigOverrides { +// DeleteLongRunningOperationState mocks base method. +func (m *MockNSGScope) DeleteLongRunningOperationState(arg0, arg1 string) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CloudProviderConfigOverrides") - ret0, _ := ret[0].(*v1alpha4.CloudProviderConfigOverrides) - return ret0 + m.ctrl.Call(m, "DeleteLongRunningOperationState", arg0, arg1) } -// CloudProviderConfigOverrides indicates an expected call of CloudProviderConfigOverrides. -func (mr *MockNSGScopeMockRecorder) CloudProviderConfigOverrides() *gomock.Call { +// DeleteLongRunningOperationState indicates an expected call of DeleteLongRunningOperationState. +func (mr *MockNSGScopeMockRecorder) DeleteLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CloudProviderConfigOverrides", reflect.TypeOf((*MockNSGScope)(nil).CloudProviderConfigOverrides)) -} - -// ClusterName mocks base method. -func (m *MockNSGScope) ClusterName() string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ClusterName") - ret0, _ := ret[0].(string) - return ret0 -} - -// ClusterName indicates an expected call of ClusterName. -func (mr *MockNSGScopeMockRecorder) ClusterName() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClusterName", reflect.TypeOf((*MockNSGScope)(nil).ClusterName)) -} - -// ControlPlaneRouteTable mocks base method. -func (m *MockNSGScope) ControlPlaneRouteTable() v1alpha4.RouteTable { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ControlPlaneRouteTable") - ret0, _ := ret[0].(v1alpha4.RouteTable) - return ret0 -} - -// ControlPlaneRouteTable indicates an expected call of ControlPlaneRouteTable. -func (mr *MockNSGScopeMockRecorder) ControlPlaneRouteTable() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ControlPlaneRouteTable", reflect.TypeOf((*MockNSGScope)(nil).ControlPlaneRouteTable)) -} - -// ControlPlaneSubnet mocks base method. -func (m *MockNSGScope) ControlPlaneSubnet() v1alpha4.SubnetSpec { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ControlPlaneSubnet") - ret0, _ := ret[0].(v1alpha4.SubnetSpec) - return ret0 -} - -// ControlPlaneSubnet indicates an expected call of ControlPlaneSubnet. -func (mr *MockNSGScopeMockRecorder) ControlPlaneSubnet() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ControlPlaneSubnet", reflect.TypeOf((*MockNSGScope)(nil).ControlPlaneSubnet)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteLongRunningOperationState", reflect.TypeOf((*MockNSGScope)(nil).DeleteLongRunningOperationState), arg0, arg1) } // Enabled mocks base method. @@ -266,18 +168,18 @@ func (mr *MockNSGScopeMockRecorder) Error(err, msg interface{}, keysAndValues .. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Error", reflect.TypeOf((*MockNSGScope)(nil).Error), varargs...) } -// GetPrivateDNSZoneName mocks base method. -func (m *MockNSGScope) GetPrivateDNSZoneName() string { +// GetLongRunningOperationState mocks base method. +func (m *MockNSGScope) GetLongRunningOperationState(arg0, arg1 string) *v1alpha4.Future { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetPrivateDNSZoneName") - ret0, _ := ret[0].(string) + ret := m.ctrl.Call(m, "GetLongRunningOperationState", arg0, arg1) + ret0, _ := ret[0].(*v1alpha4.Future) return ret0 } -// GetPrivateDNSZoneName indicates an expected call of GetPrivateDNSZoneName. -func (mr *MockNSGScopeMockRecorder) GetPrivateDNSZoneName() *gomock.Call { +// GetLongRunningOperationState indicates an expected call of GetLongRunningOperationState. +func (mr *MockNSGScopeMockRecorder) GetLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPrivateDNSZoneName", reflect.TypeOf((*MockNSGScope)(nil).GetPrivateDNSZoneName)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLongRunningOperationState", reflect.TypeOf((*MockNSGScope)(nil).GetLongRunningOperationState), arg0, arg1) } // HashKey mocks base method. @@ -311,67 +213,26 @@ func (mr *MockNSGScopeMockRecorder) Info(msg interface{}, keysAndValues ...inter return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Info", reflect.TypeOf((*MockNSGScope)(nil).Info), varargs...) } -// IsAPIServerPrivate mocks base method. -func (m *MockNSGScope) IsAPIServerPrivate() bool { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsAPIServerPrivate") - ret0, _ := ret[0].(bool) - return ret0 -} - -// IsAPIServerPrivate indicates an expected call of IsAPIServerPrivate. -func (mr *MockNSGScopeMockRecorder) IsAPIServerPrivate() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsAPIServerPrivate", reflect.TypeOf((*MockNSGScope)(nil).IsAPIServerPrivate)) -} - -// IsIPv6Enabled mocks base method. -func (m *MockNSGScope) IsIPv6Enabled() bool { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsIPv6Enabled") - ret0, _ := ret[0].(bool) - return ret0 -} - -// IsIPv6Enabled indicates an expected call of IsIPv6Enabled. -func (mr *MockNSGScopeMockRecorder) IsIPv6Enabled() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsIPv6Enabled", reflect.TypeOf((*MockNSGScope)(nil).IsIPv6Enabled)) -} - // IsVnetManaged mocks base method. -func (m *MockNSGScope) IsVnetManaged() bool { +func (m *MockNSGScope) IsVnetManaged(ctx context.Context) (bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsVnetManaged") + ret := m.ctrl.Call(m, "IsVnetManaged", ctx) ret0, _ := ret[0].(bool) - return ret0 + ret1, _ := ret[1].(error) + return ret0, ret1 } // IsVnetManaged indicates an expected call of IsVnetManaged. -func (mr *MockNSGScopeMockRecorder) IsVnetManaged() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsVnetManaged", reflect.TypeOf((*MockNSGScope)(nil).IsVnetManaged)) -} - -// Location mocks base method. -func (m *MockNSGScope) Location() string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Location") - ret0, _ := ret[0].(string) - return ret0 -} - -// Location indicates an expected call of Location. -func (mr *MockNSGScopeMockRecorder) Location() *gomock.Call { +func (mr *MockNSGScopeMockRecorder) IsVnetManaged(ctx interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Location", reflect.TypeOf((*MockNSGScope)(nil).Location)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsVnetManaged", reflect.TypeOf((*MockNSGScope)(nil).IsVnetManaged), ctx) } // NSGSpecs mocks base method. -func (m *MockNSGScope) NSGSpecs() []azure.NSGSpec { +func (m *MockNSGScope) NSGSpecs() []azure.ResourceSpecGetter { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "NSGSpecs") - ret0, _ := ret[0].([]azure.NSGSpec) + ret0, _ := ret[0].([]azure.ResourceSpecGetter) return ret0 } @@ -381,128 +242,80 @@ func (mr *MockNSGScopeMockRecorder) NSGSpecs() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NSGSpecs", reflect.TypeOf((*MockNSGScope)(nil).NSGSpecs)) } -// NodeSubnets mocks base method. -func (m *MockNSGScope) NodeSubnets() []v1alpha4.SubnetSpec { +// SetLongRunningOperationState mocks base method. +func (m *MockNSGScope) SetLongRunningOperationState(arg0 *v1alpha4.Future) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "NodeSubnets") - ret0, _ := ret[0].([]v1alpha4.SubnetSpec) - return ret0 + m.ctrl.Call(m, "SetLongRunningOperationState", arg0) } -// NodeSubnets indicates an expected call of NodeSubnets. -func (mr *MockNSGScopeMockRecorder) NodeSubnets() *gomock.Call { +// SetLongRunningOperationState indicates an expected call of SetLongRunningOperationState. +func (mr *MockNSGScopeMockRecorder) SetLongRunningOperationState(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NodeSubnets", reflect.TypeOf((*MockNSGScope)(nil).NodeSubnets)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLongRunningOperationState", reflect.TypeOf((*MockNSGScope)(nil).SetLongRunningOperationState), arg0) } -// OutboundLBName mocks base method. -func (m *MockNSGScope) OutboundLBName(arg0 string) string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "OutboundLBName", arg0) - ret0, _ := ret[0].(string) - return ret0 -} - -// OutboundLBName indicates an expected call of OutboundLBName. -func (mr *MockNSGScopeMockRecorder) OutboundLBName(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "OutboundLBName", reflect.TypeOf((*MockNSGScope)(nil).OutboundLBName), arg0) -} - -// OutboundPoolName mocks base method. -func (m *MockNSGScope) OutboundPoolName(arg0 string) string { +// SubscriptionID mocks base method. +func (m *MockNSGScope) SubscriptionID() string { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "OutboundPoolName", arg0) + ret := m.ctrl.Call(m, "SubscriptionID") ret0, _ := ret[0].(string) return ret0 } -// OutboundPoolName indicates an expected call of OutboundPoolName. -func (mr *MockNSGScopeMockRecorder) OutboundPoolName(arg0 interface{}) *gomock.Call { +// SubscriptionID indicates an expected call of SubscriptionID. +func (mr *MockNSGScopeMockRecorder) SubscriptionID() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "OutboundPoolName", reflect.TypeOf((*MockNSGScope)(nil).OutboundPoolName), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SubscriptionID", reflect.TypeOf((*MockNSGScope)(nil).SubscriptionID)) } -// ResourceGroup mocks base method. -func (m *MockNSGScope) ResourceGroup() string { +// TenantID mocks base method. +func (m *MockNSGScope) TenantID() string { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ResourceGroup") + ret := m.ctrl.Call(m, "TenantID") ret0, _ := ret[0].(string) return ret0 } -// ResourceGroup indicates an expected call of ResourceGroup. -func (mr *MockNSGScopeMockRecorder) ResourceGroup() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResourceGroup", reflect.TypeOf((*MockNSGScope)(nil).ResourceGroup)) -} - -// SetSubnet mocks base method. -func (m *MockNSGScope) SetSubnet(arg0 v1alpha4.SubnetSpec) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "SetSubnet", arg0) -} - -// SetSubnet indicates an expected call of SetSubnet. -func (mr *MockNSGScopeMockRecorder) SetSubnet(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetSubnet", reflect.TypeOf((*MockNSGScope)(nil).SetSubnet), arg0) -} - -// Subnet mocks base method. -func (m *MockNSGScope) Subnet(arg0 string) v1alpha4.SubnetSpec { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Subnet", arg0) - ret0, _ := ret[0].(v1alpha4.SubnetSpec) - return ret0 -} - -// Subnet indicates an expected call of Subnet. -func (mr *MockNSGScopeMockRecorder) Subnet(arg0 interface{}) *gomock.Call { +// TenantID indicates an expected call of TenantID. +func (mr *MockNSGScopeMockRecorder) TenantID() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Subnet", reflect.TypeOf((*MockNSGScope)(nil).Subnet), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TenantID", reflect.TypeOf((*MockNSGScope)(nil).TenantID)) } -// Subnets mocks base method. -func (m *MockNSGScope) Subnets() v1alpha4.Subnets { +// UpdateDeleteStatus mocks base method. +func (m *MockNSGScope) UpdateDeleteStatus(arg0 v1alpha40.ConditionType, arg1 string, arg2 error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Subnets") - ret0, _ := ret[0].(v1alpha4.Subnets) - return ret0 + m.ctrl.Call(m, "UpdateDeleteStatus", arg0, arg1, arg2) } -// Subnets indicates an expected call of Subnets. -func (mr *MockNSGScopeMockRecorder) Subnets() *gomock.Call { +// UpdateDeleteStatus indicates an expected call of UpdateDeleteStatus. +func (mr *MockNSGScopeMockRecorder) UpdateDeleteStatus(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Subnets", reflect.TypeOf((*MockNSGScope)(nil).Subnets)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateDeleteStatus", reflect.TypeOf((*MockNSGScope)(nil).UpdateDeleteStatus), arg0, arg1, arg2) } -// SubscriptionID mocks base method. -func (m *MockNSGScope) SubscriptionID() string { +// UpdatePatchStatus mocks base method. +func (m *MockNSGScope) UpdatePatchStatus(arg0 v1alpha40.ConditionType, arg1 string, arg2 error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SubscriptionID") - ret0, _ := ret[0].(string) - return ret0 + m.ctrl.Call(m, "UpdatePatchStatus", arg0, arg1, arg2) } -// SubscriptionID indicates an expected call of SubscriptionID. -func (mr *MockNSGScopeMockRecorder) SubscriptionID() *gomock.Call { +// UpdatePatchStatus indicates an expected call of UpdatePatchStatus. +func (mr *MockNSGScopeMockRecorder) UpdatePatchStatus(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SubscriptionID", reflect.TypeOf((*MockNSGScope)(nil).SubscriptionID)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePatchStatus", reflect.TypeOf((*MockNSGScope)(nil).UpdatePatchStatus), arg0, arg1, arg2) } -// TenantID mocks base method. -func (m *MockNSGScope) TenantID() string { +// UpdatePutStatus mocks base method. +func (m *MockNSGScope) UpdatePutStatus(arg0 v1alpha40.ConditionType, arg1 string, arg2 error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "TenantID") - ret0, _ := ret[0].(string) - return ret0 + m.ctrl.Call(m, "UpdatePutStatus", arg0, arg1, arg2) } -// TenantID indicates an expected call of TenantID. -func (mr *MockNSGScopeMockRecorder) TenantID() *gomock.Call { +// UpdatePutStatus indicates an expected call of UpdatePutStatus. +func (mr *MockNSGScopeMockRecorder) UpdatePutStatus(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TenantID", reflect.TypeOf((*MockNSGScope)(nil).TenantID)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePutStatus", reflect.TypeOf((*MockNSGScope)(nil).UpdatePutStatus), arg0, arg1, arg2) } // V mocks base method. @@ -519,20 +332,6 @@ func (mr *MockNSGScopeMockRecorder) V(level interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "V", reflect.TypeOf((*MockNSGScope)(nil).V), level) } -// Vnet mocks base method. -func (m *MockNSGScope) Vnet() *v1alpha4.VnetSpec { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Vnet") - ret0, _ := ret[0].(*v1alpha4.VnetSpec) - return ret0 -} - -// Vnet indicates an expected call of Vnet. -func (mr *MockNSGScopeMockRecorder) Vnet() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Vnet", reflect.TypeOf((*MockNSGScope)(nil).Vnet)) -} - // WithName mocks base method. func (m *MockNSGScope) WithName(name string) logr.Logger { m.ctrl.T.Helper() diff --git a/azure/services/securitygroups/securitygroups.go b/azure/services/securitygroups/securitygroups.go index 684e44b8e86..94178143199 100644 --- a/azure/services/securitygroups/securitygroups.go +++ b/azure/services/securitygroups/securitygroups.go @@ -18,24 +18,26 @@ package securitygroups import ( "context" - "strings" - "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" - "github.com/Azure/go-autorest/autorest/to" "github.com/go-logr/logr" "github.com/pkg/errors" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" "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 = "securitygroups" + // NSGScope defines the scope interface for a security groups service. type NSGScope interface { logr.Logger - azure.ClusterDescriber - azure.NetworkDescriber - NSGSpecs() []azure.NSGSpec + azure.Authorizer + azure.AsyncStatusUpdater + NSGSpecs() []azure.ResourceSpecGetter + IsVnetManaged(ctx context.Context) (bool, error) } // Service provides operations on Azure resources. @@ -52,111 +54,73 @@ func New(scope NSGScope) *Service { } } -// Reconcile gets/creates/updates a network security group. +// Reconcile gets/creates/updates network security groups. func (s *Service) Reconcile(ctx context.Context) error { ctx, span := tele.Tracer().Start(ctx, "securitygroups.Service.Reconcile") defer span.End() - if !s.Scope.IsVnetManaged() { - s.Scope.V(4).Info("Skipping network security group reconcile in custom VNet mode") + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() + + // Only create the NSGs if their lifecycle is managed by this controller. + // NSGs are managed if and only if the vnet is managed. + managed, err := s.Scope.IsVnetManaged(ctx) + if err != nil { + return errors.Wrap(err, "failed to determine if network security groups are managed") + + } else if !managed { + s.Scope.V(4).Info("Skipping network security groups reconcile in custom VNet mode") return nil } + // We go through the list of NSGSpecs to reconcile each one, independently of the result of the previous one. + // If multiple erros occur, we return the most pressing one + // order of precendence is: error creating -> creating in progress -> created (no error) + var result error + for _, nsgSpec := range s.Scope.NSGSpecs() { - securityRules := make([]network.SecurityRule, 0) - var etag *string - - existingNSG, err := s.client.Get(ctx, s.Scope.ResourceGroup(), nsgSpec.Name) - switch { - case err != nil && !azure.ResourceNotFound(err): - return errors.Wrapf(err, "failed to get NSG %s in %s", nsgSpec.Name, s.Scope.ResourceGroup()) - case err == nil: - // security group already exists - // We append the existing NSG etag to the header to ensure we only apply the updates if the NSG has not been modified. - etag = existingNSG.Etag - // Check if the expected rules are present - update := false - securityRules = *existingNSG.SecurityRules - for _, rule := range nsgSpec.SecurityRules { - sdkRule := converters.SecurityRuleToSDK(rule) - if !ruleExists(securityRules, sdkRule) { - update = true - securityRules = append(securityRules, sdkRule) - } - } - if !update { - // Skip update for NSG as the required default rules are present - s.Scope.V(2).Info("security group exists and no default rules are missing, skipping update", "security group", nsgSpec.Name) - continue - } - default: - s.Scope.V(2).Info("creating security group", "security group", nsgSpec.Name) - for _, rule := range nsgSpec.SecurityRules { - securityRules = append(securityRules, converters.SecurityRuleToSDK(rule)) + if err := async.CreateResource(ctx, s.Scope, s.client, nsgSpec, serviceName); err != nil { + if !azure.IsOperationNotDoneError(err) || result == nil { + result = err } } - sg := network.SecurityGroup{ - Location: to.StringPtr(s.Scope.Location()), - SecurityGroupPropertiesFormat: &network.SecurityGroupPropertiesFormat{ - SecurityRules: &securityRules, - }, - Etag: etag, - } - err = s.client.CreateOrUpdate(ctx, s.Scope.ResourceGroup(), nsgSpec.Name, sg) - if err != nil { - return errors.Wrapf(err, "failed to create or update security group %s in resource group %s", nsgSpec.Name, s.Scope.ResourceGroup()) - } - - s.Scope.V(2).Info("successfully created or updated security group", "security group", nsgSpec.Name) } - return nil -} -func ruleExists(rules []network.SecurityRule, rule network.SecurityRule) bool { - for _, existingRule := range rules { - if !strings.EqualFold(to.String(existingRule.Name), to.String(rule.Name)) { - continue - } - if !strings.EqualFold(to.String(existingRule.DestinationPortRange), to.String(rule.DestinationPortRange)) { - continue - } - if existingRule.Protocol != network.SecurityRuleProtocolTCP && - existingRule.Access != network.SecurityRuleAccessAllow && - existingRule.Direction != network.SecurityRuleDirectionInbound { - continue - } - if !strings.EqualFold(to.String(existingRule.SourcePortRange), "*") && - !strings.EqualFold(to.String(existingRule.SourceAddressPrefix), "*") && - !strings.EqualFold(to.String(existingRule.DestinationAddressPrefix), "*") { - continue - } - return true - } - return false + s.Scope.UpdatePutStatus(infrav1.SecurityGroupsReadyCondition, serviceName, result) + return result } -// Delete deletes the network security group with the provided name. +// Delete deletes network security groups. func (s *Service) Delete(ctx context.Context) error { ctx, span := tele.Tracer().Start(ctx, "securitygroups.Service.Delete") defer span.End() - if !s.Scope.IsVnetManaged() { - s.Scope.V(4).Info("Skipping network security group delete in custom VNet mode") + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() + + // Only delete the NSG if its lifecycle is managed by this controller. + // NSGs are managed if and only if the vnet is managed. + managed, err := s.Scope.IsVnetManaged(ctx) + if err != nil { + return errors.Wrap(err, "failed to determine if network security groups are managed") + } else if !managed { + s.Scope.V(4).Info("Skipping network security groups delete in custom VNet mode") return nil } + var result error + + // We go through the list of NSGSpecs to delete each one, independently of the result of the previous one. + // If multiple erros occur, we return the most pressing one + // order of precendence is: error deleting -> deleting in progress -> deleted (no error) for _, nsgSpec := range s.Scope.NSGSpecs() { - s.Scope.V(2).Info("deleting security group", "security group", nsgSpec.Name) - err := s.client.Delete(ctx, s.Scope.ResourceGroup(), nsgSpec.Name) - if err != nil && azure.ResourceNotFound(err) { - // already deleted - continue - } - if err != nil { - return errors.Wrapf(err, "failed to delete security group %s in resource group %s", nsgSpec.Name, s.Scope.ResourceGroup()) + if err := async.DeleteResource(ctx, s.Scope, s.client, nsgSpec, serviceName); err != nil { + if !azure.IsOperationNotDoneError(err) || result == nil { + result = err + } } - - s.Scope.V(2).Info("successfully deleted security group", "security group", nsgSpec.Name) } - return nil + + s.Scope.UpdateDeleteStatus(infrav1.SecurityGroupsReadyCondition, serviceName, result) + return result } diff --git a/azure/services/securitygroups/securitygroups_test.go b/azure/services/securitygroups/securitygroups_test.go index f3fdb8ce2a7..3e19cb98de9 100644 --- a/azure/services/securitygroups/securitygroups_test.go +++ b/azure/services/securitygroups/securitygroups_test.go @@ -75,7 +75,7 @@ func TestReconcileSecurityGroups(t *testing.T) { SecurityRules: infrav1.SecurityRules{}, }, }) - s.IsVnetManaged().Return(true) + s.IsVnetManaged(gomockinternal.AContext()).Return(true) s.ResourceGroup().AnyTimes().Return("my-rg") s.Location().AnyTimes().Return("test-location") s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) @@ -150,7 +150,7 @@ func TestReconcileSecurityGroups(t *testing.T) { SecurityRules: infrav1.SecurityRules{}, }, }) - s.IsVnetManaged().AnyTimes().Return(true) + s.IsVnetManaged(gomockinternal.AContext()).AnyTimes().Return(true) s.ResourceGroup().AnyTimes().Return("my-rg") s.Location().AnyTimes().Return("test-location") s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) @@ -227,7 +227,7 @@ func TestReconcileSecurityGroups(t *testing.T) { }, { name: "skipping network security group reconcile in custom VNet mode", expect: func(s *mock_securitygroups.MockNSGScopeMockRecorder, m *mock_securitygroups.MockclientMockRecorder) { - s.IsVnetManaged().Return(false) + s.IsVnetManaged(gomockinternal.AContext()).Return(false) s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) }, }, @@ -287,7 +287,7 @@ func TestDeleteSecurityGroups(t *testing.T) { }) s.ResourceGroup().AnyTimes().Return("my-rg") s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.IsVnetManaged().Return(true) + s.IsVnetManaged(gomockinternal.AContext()).Return(true) m.Delete(gomockinternal.AContext(), "my-rg", "nsg-one") m.Delete(gomockinternal.AContext(), "my-rg", "nsg-two") }, @@ -307,7 +307,7 @@ func TestDeleteSecurityGroups(t *testing.T) { }) s.ResourceGroup().AnyTimes().Return("my-rg") s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.IsVnetManaged().Return(true) + s.IsVnetManaged(gomockinternal.AContext()).Return(true) m.Delete(gomockinternal.AContext(), "my-rg", "nsg-one"). Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) m.Delete(gomockinternal.AContext(), "my-rg", "nsg-two") @@ -316,7 +316,7 @@ func TestDeleteSecurityGroups(t *testing.T) { { name: "skipping network security group delete in custom VNet mode", expect: func(s *mock_securitygroups.MockNSGScopeMockRecorder, m *mock_securitygroups.MockclientMockRecorder) { - s.IsVnetManaged().Return(false) + s.IsVnetManaged(gomockinternal.AContext()).Return(false) s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) }, }, diff --git a/azure/services/securitygroups/spec.go b/azure/services/securitygroups/spec.go new file mode 100644 index 00000000000..5f3b2ef28b6 --- /dev/null +++ b/azure/services/securitygroups/spec.go @@ -0,0 +1,112 @@ +/* +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 securitygroups + +import ( + "strings" + + "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" + "github.com/Azure/go-autorest/autorest/to" + "github.com/pkg/errors" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" + "sigs.k8s.io/cluster-api-provider-azure/azure/converters" +) + +// NSGSpec defines the specification for a Security Group. +type NSGSpec struct { + Name string + SecurityRules infrav1.SecurityRules + Location string + ResourceGroup string +} + +func (s *NSGSpec) ResourceName() string { + return s.Name +} + +func (s *NSGSpec) ResourceGroupName() string { + return s.ResourceGroup +} + +func (s *NSGSpec) OwnerResourceName() string { + return "" +} + +func (s *NSGSpec) Parameters(existing interface{}) (interface{}, error) { + securityRules := make([]network.SecurityRule, 0) + var etag *string + + if existing != nil { + existingNSG, ok := existing.(network.SecurityGroup) + if !ok { + return nil, errors.Errorf("%T is not a network.SecurityGroup", existing) + } + // security group already exists + // We append the existing NSG etag to the header to ensure we only apply the updates if the NSG has not been modified. + etag = existingNSG.Etag + // Check if the expected rules are present + update := false + securityRules = *existingNSG.SecurityRules + for _, rule := range s.SecurityRules { + sdkRule := converters.SecurityRuleToSDK(rule) + if !ruleExists(securityRules, sdkRule) { + update = true + securityRules = append(securityRules, sdkRule) + } + } + if !update { + // Skip update for NSG as the required default rules are present + return nil, nil + } + } else { + // new security group + for _, rule := range s.SecurityRules { + securityRules = append(securityRules, converters.SecurityRuleToSDK(rule)) + } + } + + return network.SecurityGroup{ + Location: to.StringPtr(s.Location), + SecurityGroupPropertiesFormat: &network.SecurityGroupPropertiesFormat{ + SecurityRules: &securityRules, + }, + Etag: etag, + }, nil +} + +func ruleExists(rules []network.SecurityRule, rule network.SecurityRule) bool { + for _, existingRule := range rules { + if !strings.EqualFold(to.String(existingRule.Name), to.String(rule.Name)) { + continue + } + if !strings.EqualFold(to.String(existingRule.DestinationPortRange), to.String(rule.DestinationPortRange)) { + continue + } + if existingRule.Protocol != network.SecurityRuleProtocolTCP && + existingRule.Access != network.SecurityRuleAccessAllow && + existingRule.Direction != network.SecurityRuleDirectionInbound { + continue + } + if !strings.EqualFold(to.String(existingRule.SourcePortRange), "*") && + !strings.EqualFold(to.String(existingRule.SourceAddressPrefix), "*") && + !strings.EqualFold(to.String(existingRule.DestinationAddressPrefix), "*") { + continue + } + return true + } + return false +} diff --git a/azure/services/subnets/mock_subnets/subnets_mock.go b/azure/services/subnets/mock_subnets/subnets_mock.go index 43f6e78565c..a3c6232c23f 100644 --- a/azure/services/subnets/mock_subnets/subnets_mock.go +++ b/azure/services/subnets/mock_subnets/subnets_mock.go @@ -21,6 +21,7 @@ limitations under the License. package mock_subnets import ( + context "context" reflect "reflect" autorest "github.com/Azure/go-autorest/autorest" @@ -340,17 +341,18 @@ func (mr *MockSubnetScopeMockRecorder) IsIPv6Enabled() *gomock.Call { } // IsVnetManaged mocks base method. -func (m *MockSubnetScope) IsVnetManaged() bool { +func (m *MockSubnetScope) IsVnetManaged(arg0 context.Context) (bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsVnetManaged") + ret := m.ctrl.Call(m, "IsVnetManaged", arg0) ret0, _ := ret[0].(bool) - return ret0 + ret1, _ := ret[1].(error) + return ret0, ret1 } // IsVnetManaged indicates an expected call of IsVnetManaged. -func (mr *MockSubnetScopeMockRecorder) IsVnetManaged() *gomock.Call { +func (mr *MockSubnetScopeMockRecorder) IsVnetManaged(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsVnetManaged", reflect.TypeOf((*MockSubnetScope)(nil).IsVnetManaged)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsVnetManaged", reflect.TypeOf((*MockSubnetScope)(nil).IsVnetManaged), arg0) } // Location mocks base method. diff --git a/azure/services/subnets/subnets.go b/azure/services/subnets/subnets.go index 001dfc9b892..c3dc095b8f4 100644 --- a/azure/services/subnets/subnets.go +++ b/azure/services/subnets/subnets.go @@ -57,7 +57,9 @@ func (s *Service) Reconcile(ctx context.Context) error { defer span.End() for _, subnetSpec := range s.Scope.SubnetSpecs() { + existingSubnet, err := s.getExisting(ctx, s.Scope.Vnet().ResourceGroup, subnetSpec) + switch { case err != nil && !azure.ResourceNotFound(err): return errors.Wrapf(err, "failed to get subnet %s", subnetSpec.Name) @@ -65,11 +67,13 @@ func (s *Service) Reconcile(ctx context.Context) error { // subnet already exists, update the spec and skip creation s.Scope.SetSubnet(*existingSubnet) continue - - case !s.Scope.IsVnetManaged(): - return fmt.Errorf("vnet was provided but subnet %s is missing", subnetSpec.Name) - default: + managed, err := s.Scope.IsVnetManaged(ctx) + if err != nil { + return errors.Wrap(err, "failed to check if vnet is managed") + } else if !managed { + return fmt.Errorf("vnet was provided but subnet %s is missing", subnetSpec.Name) + } subnetProperties := network.SubnetPropertiesFormat{ AddressPrefixes: &subnetSpec.CIDRs, diff --git a/azure/services/subnets/subnets_test.go b/azure/services/subnets/subnets_test.go index ec317fa2c7e..78056c31173 100644 --- a/azure/services/subnets/subnets_test.go +++ b/azure/services/subnets/subnets_test.go @@ -62,7 +62,7 @@ func TestReconcileSubnets(t *testing.T) { s.SubscriptionID().AnyTimes().Return("123") s.ResourceGroup().AnyTimes().Return("my-rg") s.IsIPv6Enabled().AnyTimes().Return(false) - s.IsVnetManaged().Return(true) + s.IsVnetManaged(gomockinternal.AContext()).Return(true) m.Get(gomockinternal.AContext(), "", "my-vnet", "my-subnet"). Return(network.Subnet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) m.CreateOrUpdate(gomockinternal.AContext(), "", "my-vnet", "my-subnet", gomockinternal.DiffEq(network.Subnet{ @@ -93,7 +93,7 @@ func TestReconcileSubnets(t *testing.T) { s.ClusterName().AnyTimes().Return("fake-cluster") s.ResourceGroup().AnyTimes().Return("my-rg") s.SubscriptionID().AnyTimes().Return("123") - s.IsVnetManaged().Return(true) + s.IsVnetManaged(gomockinternal.AContext()).Return(true) m.Get(gomockinternal.AContext(), "my-rg", "my-vnet", "my-ipv6-subnet"). Return(network.Subnet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-vnet", "my-ipv6-subnet", gomockinternal.DiffEq(network.Subnet{ @@ -128,7 +128,7 @@ func TestReconcileSubnets(t *testing.T) { s.SubscriptionID().AnyTimes().Return("123") s.ResourceGroup().AnyTimes().Return("my-rg") s.IsIPv6Enabled().AnyTimes().Return(false) - s.IsVnetManaged().Return(true) + s.IsVnetManaged(gomockinternal.AContext()).Return(true) m.Get(gomockinternal.AContext(), "", "my-vnet", "my-subnet"). Return(network.Subnet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) m.CreateOrUpdate(gomockinternal.AContext(), "", "my-vnet", "my-subnet", gomock.AssignableToTypeOf(network.Subnet{})).Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) @@ -180,7 +180,7 @@ func TestReconcileSubnets(t *testing.T) { s.ClusterName().AnyTimes().Return("fake-cluster") s.SubscriptionID().AnyTimes().Return("123") s.ResourceGroup().AnyTimes().Return("custom-vnet-rg") - s.IsVnetManaged().Return(false) + s.IsVnetManaged(gomockinternal.AContext()).Return(false) m.Get(gomockinternal.AContext(), "custom-vnet-rg", "custom-vnet", "my-subnet"). Return(network.Subnet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) }, diff --git a/azure/services/virtualmachines/virtualmachines.go b/azure/services/virtualmachines/virtualmachines.go index 560e1d850a6..e23c2ce1499 100644 --- a/azure/services/virtualmachines/virtualmachines.go +++ b/azure/services/virtualmachines/virtualmachines.go @@ -194,7 +194,7 @@ func (s *Service) Reconcile(ctx context.Context) error { } for _, dataDisk := range vmSpec.DataDisks { - if dataDisk.ManagedDisk != nil && dataDisk.ManagedDisk.StorageAccountType == string(compute.UltraSSDLRS) { + if dataDisk.ManagedDisk != nil && dataDisk.ManagedDisk.StorageAccountType == string(compute.StorageAccountTypesUltraSSDLRS) { virtualMachine.VirtualMachineProperties.AdditionalCapabilities = &compute.AdditionalCapabilities{ UltraSSDEnabled: to.BoolPtr(true), } @@ -431,7 +431,7 @@ func (s *Service) generateStorageProfile(ctx context.Context, vmSpec azure.VMSpe // check the support for ultra disks based on location and vm size location := s.Scope.Location() - if disk.ManagedDisk.StorageAccountType == string(compute.UltraSSDLRS) && !sku.HasLocationCapability(resourceskus.UltraSSDAvailable, location, vmSpec.Zone) { + if disk.ManagedDisk.StorageAccountType == string(compute.StorageAccountTypesUltraSSDLRS) && !sku.HasLocationCapability(resourceskus.UltraSSDAvailable, location, vmSpec.Zone) { return nil, azure.WithTerminalError(fmt.Errorf("vm size %s does not support ultra disks in location %s. select a different vm size or disable ultra disks", vmSpec.Size, location)) } } diff --git a/azure/services/virtualnetworks/client.go b/azure/services/virtualnetworks/client.go index 06e3a17df6b..6a9f1bd90a8 100644 --- a/azure/services/virtualnetworks/client.go +++ b/azure/services/virtualnetworks/client.go @@ -21,17 +21,21 @@ import ( "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" "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 + CreateOrUpdateAsync(context.Context, azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) + DeleteAsync(context.Context, azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) CheckIPAddressAvailability(context.Context, string, string, string) (network.IPAddressAvailabilityResult, error) + IsDone(context.Context, azureautorest.FutureAPI) (bool, error) } // AzureClient contains the Azure go-sdk Client. @@ -64,38 +68,64 @@ func (ac *AzureClient) Get(ctx context.Context, resourceGroupName, vnetName stri return ac.virtualnetworks.Get(ctx, resourceGroupName, vnetName, "") } -// 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, span := tele.Tracer().Start(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) (azureautorest.FutureAPI, error) { + ctx, span := tele.Tracer().Start(ctx, "virtualnetworks.AzureClient.CreateOrUpdateAsync") defer span.End() - future, err := ac.virtualnetworks.CreateOrUpdate(ctx, resourceGroupName, vnetName, vn) + vn, err := ac.VirtualNetworkParams(ctx, spec) if err != nil { - return err + return nil, errors.Wrapf(err, "failed to get desired parameters for vnet %s", spec.ResourceName()) + } else if vn == nil { + // nothing to do here + return nil, nil } + + future, err := ac.virtualnetworks.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.ResourceName(), *vn) + if err != nil { + return nil, err + } + + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + err = future.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 &future, err } _, err = future.Result(ac.virtualnetworks) - return err + // if the operation completed, return a nil future. + return nil, err } -// Delete deletes the specified virtual network. -func (ac *AzureClient) Delete(ctx context.Context, resourceGroupName, vnetName string) error { - ctx, span := tele.Tracer().Start(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) (azureautorest.FutureAPI, error) { + ctx, span := tele.Tracer().Start(ctx, "virtualnetworks.AzureClient.DeleteAsync") defer span.End() - future, err := ac.virtualnetworks.Delete(ctx, resourceGroupName, vnetName) + future, err := ac.virtualnetworks.Delete(ctx, spec.ResourceGroupName(), spec.ResourceName()) if err != nil { - return err + return nil, err } + + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + err = future.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 &future, err } _, err = future.Result(ac.virtualnetworks) - return err + // if the operation completed, return a nil future. + return nil, err } // CheckIPAddressAvailability checks whether a private IP address is available for use. @@ -105,3 +135,49 @@ func (ac *AzureClient) CheckIPAddressAvailability(ctx context.Context, resourceG return ac.virtualnetworks.CheckIPAddressAvailability(ctx, resourceGroupName, vnetName, ip) } + +// IsDone returns true if the long-running operation has completed. +func (ac *AzureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (bool, error) { + ctx, span := tele.Tracer().Start(ctx, "virtualnetworks.AzureClient.IsDone") + defer span.End() + + done, err := future.DoneWithContext(ctx, ac.virtualnetworks) + if err != nil { + return false, errors.Wrap(err, "failed checking if the operation was complete") + } + + return done, nil +} + +// VirtualNetworkParams creates a VirtualNetworkParams object from the given spec. +func (ac *AzureClient) VirtualNetworkParams(ctx context.Context, spec azure.ResourceSpecGetter) (*network.VirtualNetwork, error) { + var params interface{} + + existingVnet, err := ac.Get(ctx, spec.ResourceGroupName(), spec.ResourceName()) + if azure.ResourceNotFound(err) { + // vnet doesn't exist, create it from scratch. + params, err = spec.Parameters(nil) + if err != nil { + return nil, err + } + } else if err != nil { + return nil, errors.Wrapf(err, "failed to get VNet %s in %s", spec.ResourceName(), spec.ResourceGroupName()) + } else { + // vnet already exists + params, err = spec.Parameters(existingVnet) + if err != nil { + return nil, err + } + } + + vn, ok := params.(network.VirtualNetwork) + if !ok { + if params == nil { + // nothing to do here. + return nil, nil + } + return nil, errors.Errorf("%T is not a network.VirtualNetwork", params) + } + + return &vn, nil +} diff --git a/azure/services/virtualnetworks/mock_virtualnetworks/client_mock.go b/azure/services/virtualnetworks/mock_virtualnetworks/client_mock.go index b2f48294cc6..3ee6ee1f2d5 100644 --- a/azure/services/virtualnetworks/mock_virtualnetworks/client_mock.go +++ b/azure/services/virtualnetworks/mock_virtualnetworks/client_mock.go @@ -25,7 +25,9 @@ import ( reflect "reflect" network "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" + 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. @@ -66,32 +68,34 @@ func (mr *MockClientMockRecorder) CheckIPAddressAvailability(arg0, arg1, arg2, a 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 { +// CreateOrUpdateAsync mocks base method. +func (m *MockClient) CreateOrUpdateAsync(arg0 context.Context, arg1 azure0.ResourceSpecGetter) (azure.FutureAPI, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateOrUpdate", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(error) - return ret0 + ret := m.ctrl.Call(m, "CreateOrUpdateAsync", arg0, arg1) + ret0, _ := ret[0].(azure.FutureAPI) + ret1, _ := ret[1].(error) + return ret0, ret1 } -// CreateOrUpdate indicates an expected call of CreateOrUpdate. -func (mr *MockClientMockRecorder) CreateOrUpdate(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { +// CreateOrUpdateAsync indicates an expected call of CreateOrUpdateAsync. +func (mr *MockClientMockRecorder) CreateOrUpdateAsync(arg0, arg1 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) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*MockClient)(nil).CreateOrUpdateAsync), arg0, arg1) } -// Delete mocks base method. -func (m *MockClient) Delete(arg0 context.Context, arg1, arg2 string) error { +// 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, "Delete", arg0, arg1, arg2) - ret0, _ := ret[0].(error) - return ret0 + ret := m.ctrl.Call(m, "DeleteAsync", arg0, arg1) + ret0, _ := ret[0].(azure.FutureAPI) + ret1, _ := ret[1].(error) + return ret0, ret1 } -// Delete indicates an expected call of Delete. -func (mr *MockClientMockRecorder) Delete(arg0, arg1, arg2 interface{}) *gomock.Call { +// 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, "Delete", reflect.TypeOf((*MockClient)(nil).Delete), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAsync", reflect.TypeOf((*MockClient)(nil).DeleteAsync), arg0, arg1) } // Get mocks base method. @@ -108,3 +112,18 @@ 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) } + +// 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) +} diff --git a/azure/services/virtualnetworks/mock_virtualnetworks/virtualnetworks_mock.go b/azure/services/virtualnetworks/mock_virtualnetworks/virtualnetworks_mock.go index 325437d6749..ee6d64be6ae 100644 --- a/azure/services/virtualnetworks/mock_virtualnetworks/virtualnetworks_mock.go +++ b/azure/services/virtualnetworks/mock_virtualnetworks/virtualnetworks_mock.go @@ -21,6 +21,7 @@ limitations under the License. package mock_virtualnetworks import ( + context "context" reflect "reflect" autorest "github.com/Azure/go-autorest/autorest" @@ -28,6 +29,7 @@ import ( gomock "github.com/golang/mock/gomock" v1alpha4 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" azure "sigs.k8s.io/cluster-api-provider-azure/azure" + v1alpha40 "sigs.k8s.io/cluster-api/api/v1alpha4" ) // MockVNetScope is a mock of VNetScope interface. @@ -53,20 +55,6 @@ func (m *MockVNetScope) EXPECT() *MockVNetScopeMockRecorder { return m.recorder } -// AdditionalTags mocks base method. -func (m *MockVNetScope) AdditionalTags() v1alpha4.Tags { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AdditionalTags") - ret0, _ := ret[0].(v1alpha4.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() @@ -81,20 +69,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() @@ -151,20 +125,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() *v1alpha4.CloudProviderConfigOverrides { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CloudProviderConfigOverrides") - ret0, _ := ret[0].(*v1alpha4.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() @@ -179,6 +139,18 @@ func (mr *MockVNetScopeMockRecorder) ClusterName() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClusterName", reflect.TypeOf((*MockVNetScope)(nil).ClusterName)) } +// DeleteLongRunningOperationState mocks base method. +func (m *MockVNetScope) DeleteLongRunningOperationState(arg0, arg1 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "DeleteLongRunningOperationState", arg0, arg1) +} + +// 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, "DeleteLongRunningOperationState", reflect.TypeOf((*MockVNetScope)(nil).DeleteLongRunningOperationState), arg0, arg1) +} + // Enabled mocks base method. func (m *MockVNetScope) Enabled() bool { m.ctrl.T.Helper() @@ -210,6 +182,20 @@ func (mr *MockVNetScopeMockRecorder) Error(err, msg interface{}, keysAndValues . return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Error", reflect.TypeOf((*MockVNetScope)(nil).Error), varargs...) } +// GetLongRunningOperationState mocks base method. +func (m *MockVNetScope) GetLongRunningOperationState(arg0, arg1 string) *v1alpha4.Future { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetLongRunningOperationState", arg0, arg1) + ret0, _ := ret[0].(*v1alpha4.Future) + return ret0 +} + +// 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, "GetLongRunningOperationState", reflect.TypeOf((*MockVNetScope)(nil).GetLongRunningOperationState), arg0, arg1) +} + // HashKey mocks base method. func (m *MockVNetScope) HashKey() string { m.ctrl.T.Helper() @@ -241,32 +227,31 @@ func (mr *MockVNetScopeMockRecorder) Info(msg interface{}, keysAndValues ...inte return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Info", reflect.TypeOf((*MockVNetScope)(nil).Info), varargs...) } -// Location mocks base method. -func (m *MockVNetScope) Location() string { +// IsVnetManaged mocks base method. +func (m *MockVNetScope) IsVnetManaged(ctx context.Context) (bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Location") - ret0, _ := ret[0].(string) - return ret0 + ret := m.ctrl.Call(m, "IsVnetManaged", ctx) + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 } -// Location indicates an expected call of Location. -func (mr *MockVNetScopeMockRecorder) Location() *gomock.Call { +// IsVnetManaged indicates an expected call of IsVnetManaged. +func (mr *MockVNetScopeMockRecorder) IsVnetManaged(ctx interface{}) *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, "IsVnetManaged", reflect.TypeOf((*MockVNetScope)(nil).IsVnetManaged), ctx) } -// ResourceGroup mocks base method. -func (m *MockVNetScope) ResourceGroup() string { +// SetLongRunningOperationState mocks base method. +func (m *MockVNetScope) SetLongRunningOperationState(arg0 *v1alpha4.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. @@ -297,6 +282,42 @@ 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 v1alpha40.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 v1alpha40.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 v1alpha40.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) +} + // V mocks base method. func (m *MockVNetScope) V(level int) logr.Logger { m.ctrl.T.Helper() @@ -312,10 +333,10 @@ func (mr *MockVNetScopeMockRecorder) V(level interface{}) *gomock.Call { } // 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..c656bba0a0a --- /dev/null +++ b/azure/services/virtualnetworks/spec.go @@ -0,0 +1,68 @@ +/* +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/v1alpha4" + "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 +} + +func (s *VNetSpec) ResourceName() string { + return s.Name +} + +func (s *VNetSpec) ResourceGroupName() string { + return s.ResourceGroup +} + +func (s *VNetSpec) OwnerResourceName() string { + return "" +} + +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 55762a5acad..02e602f066c 100644 --- a/azure/services/virtualnetworks/virtualnetworks.go +++ b/azure/services/virtualnetworks/virtualnetworks.go @@ -19,23 +19,28 @@ package virtualnetworks import ( "context" - "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" - "github.com/Azure/go-autorest/autorest/to" "github.com/go-logr/logr" "github.com/pkg/errors" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" "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 { logr.Logger - azure.ClusterDescriber + azure.Authorizer + azure.AsyncStatusUpdater Vnet() *infrav1.VnetSpec - VNetSpec() azure.VNetSpec + VNetSpec() azure.ResourceSpecGetter + ClusterName() string + IsVnetManaged(ctx context.Context) (bool, error) } // Service provides operations on Azure resources. @@ -53,113 +58,60 @@ func New(scope VNetScope) *Service { } // Reconcile gets/creates/updates a virtual network. -func (s *Service) Reconcile(ctx context.Context) error { +func (s *Service) Reconcile(ctx context.Context) (err error) { ctx, span := tele.Tracer().Start(ctx, "virtualnetworks.Service.Reconcile") defer span.End() - // 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) - - switch { - case err != nil && !azure.ResourceNotFound(err): - return errors.Wrapf(err, "failed to get VNet %s", vnetSpec.Name) + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() - case err == nil: - // vnet already exists, cannot update since it's immutable - if !existingVnet.IsManaged(s.Scope.ClusterName()) { - s.Scope.V(2).Info("Working on custom VNet", "vnet-id", existingVnet.ID) - } - existingVnet.DeepCopyInto(s.Scope.Vnet()) - - default: - s.Scope.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) - if err != nil { - return errors.Wrapf(err, "failed to create virtual network %s", vnetSpec.Name) - } - s.Scope.V(2).Info("successfully created VNet", "VNet", vnetSpec.Name) - } + vnetSpec := s.Scope.VNetSpec() - return nil + err = async.CreateResource(ctx, s.Scope, s.Client, vnetSpec, serviceName) + s.Scope.UpdatePutStatus(infrav1.VNetReadyCondition, serviceName, err) + return err } -// Delete deletes the virtual network with the provided name. +// Delete deletes the virtual network if it is managed by us. func (s *Service) Delete(ctx context.Context) error { ctx, span := tele.Tracer().Start(ctx, "virtualnetworks.Service.Delete") defer span.End() - 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()) { - s.Scope.V(4).Info("Skipping VNet deletion in custom vnet mode") - return nil - } - - s.Scope.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.Scope.IsVnetManaged(ctx) if err != nil { - if azure.ResourceGroupNotFound(err) || azure.ResourceNotFound(err) { + if azure.ResourceNotFound(err) { + // already deleted or doesn't exist 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 { + s.Scope.V(4).Info("Skipping VNet deletion in custom vnet mode") + return nil } - s.Scope.V(2).Info("successfully deleted VNet", "VNet", vnetSpec.Name) - return nil + vnetSpec := s.Scope.VNetSpec() + + err = async.DeleteResource(ctx, s.Scope, s.Client, 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, span := tele.Tracer().Start(ctx, "virtualnetworks.Service.getExisting") +// IsVnetManaged 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) IsVnetManaged(ctx context.Context) (bool, error) { + ctx, span := tele.Tracer().Start(ctx, "virtualnetworks.Service.IsVnetManaged") defer span.End() - vnet, err := s.Client.Get(ctx, spec.ResourceGroup, spec.Name) + vnetSpec := s.Scope.VNetSpec() + vnet, err := s.Client.Get(ctx, vnetSpec.ResourceGroupName(), vnetSpec.ResourceName()) if err != nil { - if azure.ResourceNotFound(err) { - return nil, err - } - return nil, errors.Wrapf(err, "failed to get VNet %s", spec.Name) - } - var prefixes []string - if vnet.VirtualNetworkPropertiesFormat != nil && vnet.VirtualNetworkPropertiesFormat.AddressSpace != nil { - prefixes = to.StringSlice(vnet.VirtualNetworkPropertiesFormat.AddressSpace.AddressPrefixes) + return false, err } - return &infrav1.VnetSpec{ - ResourceGroup: spec.ResourceGroup, - ID: to.String(vnet.ID), - Name: to.String(vnet.Name), - CIDRBlocks: prefixes, - Tags: converters.MapToTags(vnet.Tags), - }, nil + tags := converters.MapToTags(vnet.Tags) + return tags.HasOwned(s.Scope.ClusterName()), nil } diff --git a/azure/types.go b/azure/types.go index 8d659eb85bf..8840bea3ae6 100644 --- a/azure/types.go +++ b/azure/types.go @@ -102,13 +102,6 @@ type SubnetSpec struct { NatGatewayName string } -// VNetSpec defines the specification for a Virtual Network. -type VNetSpec struct { - ResourceGroup string - Name string - CIDRs []string -} - // RoleAssignmentSpec defines the specification for a Role Assignment. type RoleAssignmentSpec struct { MachineName string @@ -129,12 +122,6 @@ const ( VirtualMachineScaleSet = "VirtualMachineScaleSet" ) -// NSGSpec defines the specification for a Security Group. -type NSGSpec struct { - Name string - SecurityRules infrav1.SecurityRules -} - // VMSpec defines the specification for a Virtual Machine. type VMSpec struct { Name string diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml index 43155fdfed2..6b80f6e88e2 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml @@ -413,9 +413,16 @@ spec: jsonPath: .metadata.labels.cluster\.x-k8s\.io/cluster-name name: Cluster type: string - - jsonPath: .status.ready + - jsonPath: .status.conditions[?(@.type=='Ready')].status name: Ready - type: boolean + type: string + - jsonPath: .status.conditions[?(@.type=='Ready')].reason + name: Reason + type: string + - jsonPath: .status.conditions[?(@.type=='Ready')].message + name: Message + priority: 1 + type: string - jsonPath: .spec.resourceGroup name: Resource Group priority: 1 @@ -1195,23 +1202,31 @@ spec: description: Future contains the data needed for an Azure long-running operation to continue across reconcile loops. properties: - futureData: - description: FutureData is the base64 url encoded json Azure - AutoRest Future + data: + description: Data is the base64 url encoded json Azure AutoRest + Future. type: string name: - description: Name is the name of the Azure resource + description: Name is the name of the Azure resource. Together + with the service name, this forms the unique identifier for + the future. type: string resourceGroup: description: ResourceGroup is the Azure resource group for the - resource + resource. + type: string + serviceName: + description: ServiceName is the name of the Azure service. Together + with the name of the resource, this forms the unique identifier + for the future. type: string type: - description: Type describes the type of future, update, create, - delete, etc + description: Type describes the type of future, such update, + create, delete, etc. type: string required: - name + - serviceName - type type: object type: array diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepoolmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepoolmachines.yaml index 454c62aa44f..30405697e13 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepoolmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepoolmachines.yaml @@ -152,30 +152,42 @@ spec: model, it means the instance may not be running the version of Kubernetes the Machine Pool has specified and needs to be updated. type: boolean - longRunningOperationState: - description: LongRunningOperationState saves the state for an Azure - long running operations so it can be continued on the next reconciliation + longRunningOperationStates: + description: LongRunningOperationStates saves the state for Azure + long running operations so they can be continued on the next reconciliation loop. - properties: - futureData: - description: FutureData is the base64 url encoded json Azure AutoRest - Future - type: string - name: - description: Name is the name of the Azure resource - type: string - resourceGroup: - description: ResourceGroup is the Azure resource group for the - resource - type: string - type: - description: Type describes the type of future, update, create, - delete, etc - type: string - required: - - name - - type - type: object + items: + description: Future contains the data needed for an Azure long-running + operation to continue across reconcile loops. + properties: + data: + description: Data is the base64 url encoded json Azure AutoRest + Future. + type: string + name: + description: Name is the name of the Azure resource. Together + with the service name, this forms the unique identifier for + the future. + type: string + resourceGroup: + description: ResourceGroup is the Azure resource group for the + resource. + type: string + serviceName: + description: ServiceName is the name of the Azure service. Together + with the name of the resource, this forms the unique identifier + for the future. + type: string + type: + description: Type describes the type of future, such update, + create, delete, etc. + type: string + required: + - name + - serviceName + - type + type: object + type: array nodeRef: description: NodeRef will point to the corresponding Node if it exists. properties: diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml index 529fea6385f..8e61dc00fd9 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml @@ -1245,23 +1245,31 @@ spec: description: Future contains the data needed for an Azure long-running operation to continue across reconcile loops. properties: - futureData: - description: FutureData is the base64 url encoded json Azure - AutoRest Future + data: + description: Data is the base64 url encoded json Azure AutoRest + Future. type: string name: - description: Name is the name of the Azure resource + description: Name is the name of the Azure resource. Together + with the service name, this forms the unique identifier for + the future. type: string resourceGroup: description: ResourceGroup is the Azure resource group for the - resource + resource. + type: string + serviceName: + description: ServiceName is the name of the Azure service. Together + with the name of the resource, this forms the unique identifier + for the future. type: string type: - description: Type describes the type of future, update, create, - delete, etc + description: Type describes the type of future, such update, + create, delete, etc. type: string required: - name + - serviceName - type type: object type: array diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml index 11adc20ff5b..ed2c0fdaeda 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml @@ -948,23 +948,31 @@ spec: description: Future contains the data needed for an Azure long-running operation to continue across reconcile loops. properties: - futureData: - description: FutureData is the base64 url encoded json Azure - AutoRest Future + data: + description: Data is the base64 url encoded json Azure AutoRest + Future. type: string name: - description: Name is the name of the Azure resource + description: Name is the name of the Azure resource. Together + with the service name, this forms the unique identifier for + the future. type: string resourceGroup: description: ResourceGroup is the Azure resource group for the - resource + resource. + type: string + serviceName: + description: ServiceName is the name of the Azure service. Together + with the name of the resource, this forms the unique identifier + for the future. type: string type: - description: Type describes the type of future, update, create, - delete, etc + description: Type describes the type of future, such update, + create, delete, etc. type: string required: - name + - serviceName - type type: object type: array diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml index 70bf2c67501..87d4b17478e 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml @@ -375,23 +375,31 @@ spec: description: Future contains the data needed for an Azure long-running operation to continue across reconcile loops. properties: - futureData: - description: FutureData is the base64 url encoded json Azure - AutoRest Future + data: + description: Data is the base64 url encoded json Azure AutoRest + Future. type: string name: - description: Name is the name of the Azure resource + description: Name is the name of the Azure resource. Together + with the service name, this forms the unique identifier for + the future. type: string resourceGroup: description: ResourceGroup is the Azure resource group for the - resource + resource. + type: string + serviceName: + description: ServiceName is the name of the Azure service. Together + with the name of the resource, this forms the unique identifier + for the future. type: string type: - description: Type describes the type of future, update, create, - delete, etc + description: Type describes the type of future, such update, + create, delete, etc. type: string required: - name + - serviceName - type type: object type: array diff --git a/controllers/azurecluster_controller.go b/controllers/azurecluster_controller.go index 7614ff2890f..0dbbc40f282 100644 --- a/controllers/azurecluster_controller.go +++ b/controllers/azurecluster_controller.go @@ -159,7 +159,7 @@ func (r *AzureClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request return reconcile.Result{}, err } if !scope.IsClusterNamespaceAllowed(ctx, r.Client, identity.Spec.AllowedNamespaces, azureCluster.Namespace) { - conditions.MarkFalse(azureCluster, infrav1.NetworkInfrastructureReadyCondition, infrav1.NamespaceNotAllowedByIdentity, clusterv1.ConditionSeverityError, "") + conditions.MarkFalse(azureCluster, clusterv1.ReadyCondition, infrav1.NamespaceNotAllowedByIdentity, clusterv1.ConditionSeverityError, "") return reconcile.Result{}, errors.New("AzureClusterIdentity list of allowed namespaces doesn't include current cluster namespace") } } else { @@ -229,7 +229,6 @@ func (r *AzureClusterReconciler) reconcileNormal(ctx context.Context, clusterSco // No errors, so mark us ready so the Cluster API Cluster Controller can pull it azureCluster.Status.Ready = true - conditions.MarkTrue(azureCluster, infrav1.NetworkInfrastructureReadyCondition) return reconcile.Result{}, nil } @@ -241,7 +240,7 @@ func (r *AzureClusterReconciler) reconcileDelete(ctx context.Context, clusterSco clusterScope.Info("Reconciling AzureCluster delete") azureCluster := clusterScope.AzureCluster - conditions.MarkFalse(azureCluster, infrav1.NetworkInfrastructureReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "") + conditions.MarkFalse(azureCluster, clusterv1.ReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "") if err := clusterScope.PatchObject(ctx); err != nil { return reconcile.Result{}, err } @@ -254,7 +253,7 @@ func (r *AzureClusterReconciler) reconcileDelete(ctx context.Context, clusterSco if err := acr.Delete(ctx); err != nil { wrappedErr := errors.Wrapf(err, "error deleting AzureCluster %s/%s", azureCluster.Namespace, azureCluster.Name) r.Recorder.Eventf(azureCluster, corev1.EventTypeWarning, "ClusterReconcilerDeleteFailed", wrappedErr.Error()) - conditions.MarkFalse(azureCluster, infrav1.NetworkInfrastructureReadyCondition, clusterv1.DeletionFailedReason, clusterv1.ConditionSeverityWarning, err.Error()) + conditions.MarkFalse(azureCluster, clusterv1.ReadyCondition, clusterv1.DeletionFailedReason, clusterv1.ConditionSeverityWarning, err.Error()) return reconcile.Result{}, wrappedErr } diff --git a/controllers/azurecluster_reconciler_test.go b/controllers/azurecluster_reconciler_test.go index c1a1c5c878c..7c9af2fd483 100644 --- a/controllers/azurecluster_reconciler_test.go +++ b/controllers/azurecluster_reconciler_test.go @@ -27,13 +27,13 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" "sigs.k8s.io/cluster-api-provider-azure/azure" - "sigs.k8s.io/cluster-api-provider-azure/azure/mocks" + "sigs.k8s.io/cluster-api-provider-azure/azure/mock_azure" "sigs.k8s.io/cluster-api-provider-azure/azure/scope" "sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus" gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" ) -type expect func(grp *mocks.MockReconcilerMockRecorder, vnet *mocks.MockReconcilerMockRecorder, sg *mocks.MockReconcilerMockRecorder, rt *mocks.MockReconcilerMockRecorder, sn *mocks.MockReconcilerMockRecorder, natg *mocks.MockReconcilerMockRecorder, pip *mocks.MockReconcilerMockRecorder, lb *mocks.MockReconcilerMockRecorder, dns *mocks.MockReconcilerMockRecorder, bastion *mocks.MockReconcilerMockRecorder) +type expect func(grp *mock_azure.MockReconcilerMockRecorder, vnet *mock_azure.MockReconcilerMockRecorder, sg *mock_azure.MockReconcilerMockRecorder, rt *mock_azure.MockReconcilerMockRecorder, sn *mock_azure.MockReconcilerMockRecorder, natg *mock_azure.MockReconcilerMockRecorder, pip *mock_azure.MockReconcilerMockRecorder, lb *mock_azure.MockReconcilerMockRecorder, dns *mock_azure.MockReconcilerMockRecorder, bastion *mock_azure.MockReconcilerMockRecorder) func TestAzureClusterReconcilerDelete(t *testing.T) { cases := map[string]struct { @@ -42,21 +42,21 @@ func TestAzureClusterReconcilerDelete(t *testing.T) { }{ "Resource Group is deleted successfully": { expectedError: "", - expect: func(grp *mocks.MockReconcilerMockRecorder, vnet *mocks.MockReconcilerMockRecorder, sg *mocks.MockReconcilerMockRecorder, rt *mocks.MockReconcilerMockRecorder, sn *mocks.MockReconcilerMockRecorder, natg *mocks.MockReconcilerMockRecorder, pip *mocks.MockReconcilerMockRecorder, lb *mocks.MockReconcilerMockRecorder, dns *mocks.MockReconcilerMockRecorder, bastion *mocks.MockReconcilerMockRecorder) { + expect: func(grp *mock_azure.MockReconcilerMockRecorder, vnet *mock_azure.MockReconcilerMockRecorder, sg *mock_azure.MockReconcilerMockRecorder, rt *mock_azure.MockReconcilerMockRecorder, sn *mock_azure.MockReconcilerMockRecorder, natg *mock_azure.MockReconcilerMockRecorder, pip *mock_azure.MockReconcilerMockRecorder, lb *mock_azure.MockReconcilerMockRecorder, dns *mock_azure.MockReconcilerMockRecorder, bastion *mock_azure.MockReconcilerMockRecorder) { gomock.InOrder( grp.Delete(gomockinternal.AContext()).Return(nil)) }, }, "Resource Group delete fails": { expectedError: "failed to delete resource group: internal error", - expect: func(grp *mocks.MockReconcilerMockRecorder, vnet *mocks.MockReconcilerMockRecorder, sg *mocks.MockReconcilerMockRecorder, rt *mocks.MockReconcilerMockRecorder, sn *mocks.MockReconcilerMockRecorder, natg *mocks.MockReconcilerMockRecorder, pip *mocks.MockReconcilerMockRecorder, lb *mocks.MockReconcilerMockRecorder, dns *mocks.MockReconcilerMockRecorder, bastion *mocks.MockReconcilerMockRecorder) { + expect: func(grp *mock_azure.MockReconcilerMockRecorder, vnet *mock_azure.MockReconcilerMockRecorder, sg *mock_azure.MockReconcilerMockRecorder, rt *mock_azure.MockReconcilerMockRecorder, sn *mock_azure.MockReconcilerMockRecorder, natg *mock_azure.MockReconcilerMockRecorder, pip *mock_azure.MockReconcilerMockRecorder, lb *mock_azure.MockReconcilerMockRecorder, dns *mock_azure.MockReconcilerMockRecorder, bastion *mock_azure.MockReconcilerMockRecorder) { gomock.InOrder( grp.Delete(gomockinternal.AContext()).Return(errors.New("internal error"))) }, }, "Resource Group not owned by cluster": { expectedError: "", - expect: func(grp *mocks.MockReconcilerMockRecorder, vnet *mocks.MockReconcilerMockRecorder, sg *mocks.MockReconcilerMockRecorder, rt *mocks.MockReconcilerMockRecorder, sn *mocks.MockReconcilerMockRecorder, natg *mocks.MockReconcilerMockRecorder, pip *mocks.MockReconcilerMockRecorder, lb *mocks.MockReconcilerMockRecorder, dns *mocks.MockReconcilerMockRecorder, bastion *mocks.MockReconcilerMockRecorder) { + expect: func(grp *mock_azure.MockReconcilerMockRecorder, vnet *mock_azure.MockReconcilerMockRecorder, sg *mock_azure.MockReconcilerMockRecorder, rt *mock_azure.MockReconcilerMockRecorder, sn *mock_azure.MockReconcilerMockRecorder, natg *mock_azure.MockReconcilerMockRecorder, pip *mock_azure.MockReconcilerMockRecorder, lb *mock_azure.MockReconcilerMockRecorder, dns *mock_azure.MockReconcilerMockRecorder, bastion *mock_azure.MockReconcilerMockRecorder) { gomock.InOrder( grp.Delete(gomockinternal.AContext()).Return(azure.ErrNotOwned), bastion.Delete(gomockinternal.AContext()), @@ -73,7 +73,7 @@ func TestAzureClusterReconcilerDelete(t *testing.T) { }, "Load Balancer delete fails": { expectedError: "failed to delete load balancer: some error happened", - expect: func(grp *mocks.MockReconcilerMockRecorder, vnet *mocks.MockReconcilerMockRecorder, sg *mocks.MockReconcilerMockRecorder, rt *mocks.MockReconcilerMockRecorder, sn *mocks.MockReconcilerMockRecorder, pip *mocks.MockReconcilerMockRecorder, natg *mocks.MockReconcilerMockRecorder, lb *mocks.MockReconcilerMockRecorder, dns *mocks.MockReconcilerMockRecorder, bastion *mocks.MockReconcilerMockRecorder) { + expect: func(grp *mock_azure.MockReconcilerMockRecorder, vnet *mock_azure.MockReconcilerMockRecorder, sg *mock_azure.MockReconcilerMockRecorder, rt *mock_azure.MockReconcilerMockRecorder, sn *mock_azure.MockReconcilerMockRecorder, pip *mock_azure.MockReconcilerMockRecorder, natg *mock_azure.MockReconcilerMockRecorder, lb *mock_azure.MockReconcilerMockRecorder, dns *mock_azure.MockReconcilerMockRecorder, bastion *mock_azure.MockReconcilerMockRecorder) { gomock.InOrder( grp.Delete(gomockinternal.AContext()).Return(azure.ErrNotOwned), bastion.Delete(gomockinternal.AContext()), @@ -84,7 +84,7 @@ func TestAzureClusterReconcilerDelete(t *testing.T) { }, "Route table delete fails": { expectedError: "failed to delete route table: some error happened", - expect: func(grp *mocks.MockReconcilerMockRecorder, vnet *mocks.MockReconcilerMockRecorder, sg *mocks.MockReconcilerMockRecorder, rt *mocks.MockReconcilerMockRecorder, sn *mocks.MockReconcilerMockRecorder, pip *mocks.MockReconcilerMockRecorder, natg *mocks.MockReconcilerMockRecorder, lb *mocks.MockReconcilerMockRecorder, dns *mocks.MockReconcilerMockRecorder, bastion *mocks.MockReconcilerMockRecorder) { + expect: func(grp *mock_azure.MockReconcilerMockRecorder, vnet *mock_azure.MockReconcilerMockRecorder, sg *mock_azure.MockReconcilerMockRecorder, rt *mock_azure.MockReconcilerMockRecorder, sn *mock_azure.MockReconcilerMockRecorder, pip *mock_azure.MockReconcilerMockRecorder, natg *mock_azure.MockReconcilerMockRecorder, lb *mock_azure.MockReconcilerMockRecorder, dns *mock_azure.MockReconcilerMockRecorder, bastion *mock_azure.MockReconcilerMockRecorder) { gomock.InOrder( grp.Delete(gomockinternal.AContext()).Return(azure.ErrNotOwned), bastion.Delete(gomockinternal.AContext()), @@ -107,16 +107,16 @@ func TestAzureClusterReconcilerDelete(t *testing.T) { t.Parallel() mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() - groupsMock := mocks.NewMockReconciler(mockCtrl) - vnetMock := mocks.NewMockReconciler(mockCtrl) - sgMock := mocks.NewMockReconciler(mockCtrl) - rtMock := mocks.NewMockReconciler(mockCtrl) - subnetsMock := mocks.NewMockReconciler(mockCtrl) - natGatewaysMock := mocks.NewMockReconciler(mockCtrl) - publicIPMock := mocks.NewMockReconciler(mockCtrl) - lbMock := mocks.NewMockReconciler(mockCtrl) - dnsMock := mocks.NewMockReconciler(mockCtrl) - bastionMock := mocks.NewMockReconciler(mockCtrl) + groupsMock := mock_azure.NewMockReconciler(mockCtrl) + vnetMock := mock_azure.NewMockReconciler(mockCtrl) + sgMock := mock_azure.NewMockReconciler(mockCtrl) + rtMock := mock_azure.NewMockReconciler(mockCtrl) + subnetsMock := mock_azure.NewMockReconciler(mockCtrl) + natGatewaysMock := mock_azure.NewMockReconciler(mockCtrl) + publicIPMock := mock_azure.NewMockReconciler(mockCtrl) + lbMock := mock_azure.NewMockReconciler(mockCtrl) + dnsMock := mock_azure.NewMockReconciler(mockCtrl) + bastionMock := mock_azure.NewMockReconciler(mockCtrl) tc.expect(groupsMock.EXPECT(), vnetMock.EXPECT(), sgMock.EXPECT(), rtMock.EXPECT(), subnetsMock.EXPECT(), natGatewaysMock.EXPECT(), publicIPMock.EXPECT(), lbMock.EXPECT(), dnsMock.EXPECT(), bastionMock.EXPECT()) diff --git a/exp/api/v1alpha3/azuremachinepool_conversion.go b/exp/api/v1alpha3/azuremachinepool_conversion.go index 15ebc4f8029..1f8899b452e 100644 --- a/exp/api/v1alpha3/azuremachinepool_conversion.go +++ b/exp/api/v1alpha3/azuremachinepool_conversion.go @@ -18,6 +18,7 @@ package v1alpha3 import ( convert "k8s.io/apimachinery/pkg/conversion" + infrav1alpha3 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha3" infrav1alpha4 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" expv1alpha4 "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1alpha4" utilconversion "sigs.k8s.io/cluster-api/util/conversion" @@ -76,6 +77,12 @@ func (src *AzureMachinePool) ConvertTo(dstRaw conversion.Hub) error { // nolint dst.Annotations = nil } + for i, r := range restored.Status.LongRunningOperationStates { + if r.Name == dst.Status.LongRunningOperationStates[i].Name { + dst.Status.LongRunningOperationStates[i].ServiceName = r.ServiceName + } + } + return nil } @@ -104,9 +111,24 @@ func Convert_v1alpha4_AzureMachinePoolSpec_To_v1alpha3_AzureMachinePoolSpec(in * } func Convert_v1alpha4_AzureMachinePoolStatus_To_v1alpha3_AzureMachinePoolStatus(in *expv1alpha4.AzureMachinePoolStatus, out *AzureMachinePoolStatus, s convert.Scope) error { + if len(in.LongRunningOperationStates) > 0 { + if out.LongRunningOperationState == nil { + out.LongRunningOperationState = &infrav1alpha3.Future{} + } + if err := infrav1alpha3.Convert_v1alpha4_Future_To_v1alpha3_Future(&in.LongRunningOperationStates[0], out.LongRunningOperationState, s); err != nil { + return err + } + } return autoConvert_v1alpha4_AzureMachinePoolStatus_To_v1alpha3_AzureMachinePoolStatus(in, out, s) } func Convert_v1alpha3_AzureMachinePoolStatus_To_v1alpha4_AzureMachinePoolStatus(in *AzureMachinePoolStatus, out *expv1alpha4.AzureMachinePoolStatus, s convert.Scope) error { + if in.LongRunningOperationState != nil { + f := infrav1alpha4.Future{} + if err := infrav1alpha3.Convert_v1alpha3_Future_To_v1alpha4_Future(in.LongRunningOperationState, &f, s); err != nil { + return err + } + out.LongRunningOperationStates = []infrav1alpha4.Future{f} + } return autoConvert_v1alpha3_AzureMachinePoolStatus_To_v1alpha4_AzureMachinePoolStatus(in, out, s) } diff --git a/exp/api/v1alpha3/azuremanagedcontrolplane_conversion.go b/exp/api/v1alpha3/azuremanagedcontrolplane_conversion.go index c49178686d7..616f147587c 100644 --- a/exp/api/v1alpha3/azuremanagedcontrolplane_conversion.go +++ b/exp/api/v1alpha3/azuremanagedcontrolplane_conversion.go @@ -39,6 +39,8 @@ func (src *AzureManagedControlPlane) ConvertTo(dstRaw conversion.Hub) error { // dst.Spec.IdentityRef = restored.Spec.IdentityRef + dst.Status.LongRunningOperationStates = restored.Status.LongRunningOperationStates + return nil } diff --git a/exp/controllers/azuremachinepool_controller_unit_test.go b/exp/controllers/azuremachinepool_controller_unit_test.go index 75e2144f249..081371b586a 100644 --- a/exp/controllers/azuremachinepool_controller_unit_test.go +++ b/exp/controllers/azuremachinepool_controller_unit_test.go @@ -29,7 +29,7 @@ import ( clusterv1exp "sigs.k8s.io/cluster-api/exp/api/v1alpha4" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" - "sigs.k8s.io/cluster-api-provider-azure/azure/mocks" + "sigs.k8s.io/cluster-api-provider-azure/azure/mock_azure" "sigs.k8s.io/cluster-api-provider-azure/azure/scope" infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1alpha4" ) @@ -47,7 +47,7 @@ func Test_newAzureMachinePoolService(t *testing.T) { Vnet: infrav1.VnetSpec{Name: "my-vnet", ResourceGroup: "my-rg"}, } - clusterMock := mocks.NewMockClusterScoper(mockCtrl) + clusterMock := mock_azure.NewMockClusterScoper(mockCtrl) clusterMock.EXPECT().SubscriptionID().AnyTimes() clusterMock.EXPECT().BaseURI().AnyTimes() clusterMock.EXPECT().Authorizer().AnyTimes() diff --git a/exp/controllers/azuremachinepoolmachine_controller_test.go b/exp/controllers/azuremachinepoolmachine_controller_test.go index a2765384003..39a1550c0ef 100644 --- a/exp/controllers/azuremachinepoolmachine_controller_test.go +++ b/exp/controllers/azuremachinepoolmachine_controller_test.go @@ -33,7 +33,7 @@ import ( "k8s.io/klog/v2/klogr" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" "sigs.k8s.io/cluster-api-provider-azure/azure" - "sigs.k8s.io/cluster-api-provider-azure/azure/mocks" + "sigs.k8s.io/cluster-api-provider-azure/azure/mock_azure" "sigs.k8s.io/cluster-api-provider-azure/azure/scope" infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1alpha4" gomock2 "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" @@ -46,12 +46,12 @@ import ( func TestAzureMachinePoolMachineReconciler_Reconcile(t *testing.T) { cases := []struct { Name string - Setup func(cb *fake.ClientBuilder, reconciler *mocks.MockReconcilerMockRecorder) + Setup func(cb *fake.ClientBuilder, reconciler *mock_azure.MockReconcilerMockRecorder) Verify func(g *WithT, result ctrl.Result, err error) }{ { Name: "should successfully reconcile", - Setup: func(cb *fake.ClientBuilder, reconciler *mocks.MockReconcilerMockRecorder) { + Setup: func(cb *fake.ClientBuilder, reconciler *mock_azure.MockReconcilerMockRecorder) { cluster, azCluster, mp, amp, ampm := getAReadyMachinePoolMachineCluster() reconciler.Reconcile(gomock2.AContext()).Return(nil) cb.WithObjects(cluster, azCluster, mp, amp, ampm) @@ -62,7 +62,7 @@ func TestAzureMachinePoolMachineReconciler_Reconcile(t *testing.T) { }, { Name: "should successfully delete", - Setup: func(cb *fake.ClientBuilder, reconciler *mocks.MockReconcilerMockRecorder) { + Setup: func(cb *fake.ClientBuilder, reconciler *mock_azure.MockReconcilerMockRecorder) { cluster, azCluster, mp, amp, ampm := getAReadyMachinePoolMachineCluster() ampm.DeletionTimestamp = &metav1.Time{ Time: time.Now(), @@ -85,7 +85,7 @@ func TestAzureMachinePoolMachineReconciler_Reconcile(t *testing.T) { var ( g = NewWithT(t) mockCtrl = gomock.NewController(t) - reconciler = mocks.NewMockReconciler(mockCtrl) + reconciler = mock_azure.NewMockReconciler(mockCtrl) scheme = func() *runtime.Scheme { s := runtime.NewScheme() for _, addTo := range []func(s *runtime.Scheme) error{ diff --git a/test/e2e/aks.go b/test/e2e/aks.go index 6e535650b7a..dfaef441729 100644 --- a/test/e2e/aks.go +++ b/test/e2e/aks.go @@ -23,7 +23,6 @@ import ( "errors" "fmt" - "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2020-06-30/compute" "github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2020-02-01/containerservice" "github.com/Azure/go-autorest/autorest/azure/auth" . "github.com/onsi/ginkgo" diff --git a/util/futures/getter_test.go b/util/futures/getter_test.go index 274d8ed6cd1..01b7419dff8 100644 --- a/util/futures/getter_test.go +++ b/util/futures/getter_test.go @@ -32,17 +32,17 @@ func TestGet(t *testing.T) { vnetName := "my-vnet" vm := "virtualmachines" vnet := "virtualnetworks" - vmFuture := fakeFuture(vmName) - vnetFuture := fakeFuture(vnetName) + vmFuture := fakeFuture(vmName, vm) + vnetFuture := fakeFuture(vnetName, vnet) g.Expect(Get(azurecluster, vmName, vm)).To(BeNil()) g.Expect(Get(azurecluster, vnetName, vnet)).To(BeNil()) azurecluster.SetFutures(infrav1.Futures{vmFuture, vnetFuture}) - g.Expect(Get(azurecluster, vmName, vm)).To(Equal(vmFuture)) + g.Expect(Get(azurecluster, vmName, vm)).To(Equal(&vmFuture)) g.Expect(Get(azurecluster, vmName, vnet)).To(BeNil()) - g.Expect(Get(azurecluster, vnetName, vnet)).To(Equal(vnetFuture)) + g.Expect(Get(azurecluster, vnetName, vnet)).To(Equal(&vnetFuture)) } func TestHas(t *testing.T) { @@ -53,7 +53,7 @@ func TestHas(t *testing.T) { vmName := "my-vm" vm := "virtualmachines" vnet := "virtualnetworks" - vmFuture := fakeFuture(vmName) + vmFuture := fakeFuture(vmName, vm) g.Expect(Has(azurecluster, vmName, vm)).To(BeFalse()) g.Expect(Has(azurecluster, "foo", vm)).To(BeFalse()) @@ -65,11 +65,12 @@ func TestHas(t *testing.T) { g.Expect(Has(azurecluster, vmName, vnet)).To(BeFalse()) } -func fakeFuture(name string) infrav1.Future { +func fakeFuture(name string, service string) infrav1.Future { return infrav1.Future{ Type: "PUT", Name: name, ResourceGroup: "test-rg", Data: "", + ServiceName: service, } } diff --git a/util/futures/setter_test.go b/util/futures/setter_test.go index 716667d89c1..824772457d8 100644 --- a/util/futures/setter_test.go +++ b/util/futures/setter_test.go @@ -24,8 +24,9 @@ import ( ) func TestSet(t *testing.T) { - a := fakeFuture("a") - b := fakeFuture("b") + testService := "test-service" + a := fakeFuture("a", testService) + b := fakeFuture("b", testService) newA := a newA.Data = "new" @@ -73,27 +74,28 @@ func TestSet(t *testing.T) { } func TestDelete(t *testing.T) { - a := fakeFuture("a") - b := fakeFuture("b") - c := fakeFuture("c") - d := fakeFuture("d") + testService := "test-service" + a := fakeFuture("a", testService) + b := fakeFuture("b", testService) + c := fakeFuture("c", testService) + d := fakeFuture("d", testService) tests := []struct { name string to Setter - future *infrav1.Future + future string want infrav1.Futures }{ { name: "Delete removes a future", to: setterWithFutures(infrav1.Futures{a, b, c, d}), - future: &b, + future: "b", want: infrav1.Futures{a, c, d}, }, { name: "Delete does nothing if the future does not exist", to: setterWithFutures(infrav1.Futures{a}), - future: &b, + future: "b", want: infrav1.Futures{a}, }, } @@ -102,7 +104,7 @@ func TestDelete(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - Set(tt.to, tt.future) + Delete(tt.to, tt.future, testService) g.Expect(tt.to.GetFutures()).To(Equal(tt.want)) }) diff --git a/util/reconciler/defaults.go b/util/reconciler/defaults.go index adabdb7c5bd..f75b020432d 100644 --- a/util/reconciler/defaults.go +++ b/util/reconciler/defaults.go @@ -29,6 +29,8 @@ const ( DefaultAzureServiceReconcileTimeout = 12 * time.Second // DefaultAzureCallTimeout is the default timeout for an Azure request after which an Azure operation is considered long running. DefaultAzureCallTimeout = 2 * time.Second + // DefaultReconcilerRequeue is the default value for the reconcile retry. + DefaultReconcilerRequeue = 15 * time.Second ) // DefaultedLoopTimeout will default the timeout if it is zero-valued.