diff --git a/azure/interfaces.go b/azure/interfaces.go index de5ecf0d5b3..c089e96cd36 100644 --- a/azure/interfaces.go +++ b/azure/interfaces.go @@ -110,5 +110,5 @@ type ResourceSpecGetter interface { // 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) + Parameters(existing interface{}) (params interface{}, err error) } diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go index 89d1de14625..bf77c213e6e 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -37,6 +37,7 @@ import ( "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/natgateways" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/routetables" "sigs.k8s.io/cluster-api-provider-azure/azure/services/vnetpeerings" "sigs.k8s.io/cluster-api-provider-azure/util/futures" "sigs.k8s.io/cluster-api-provider-azure/util/tele" @@ -209,16 +210,20 @@ func (s *ClusterScope) LBSpecs() []azure.LBSpec { return specs } -// RouteTableSpecs returns the node route table. -func (s *ClusterScope) RouteTableSpecs() []azure.RouteTableSpec { - var routetables []azure.RouteTableSpec +// RouteTableSpecs returns the subnet route tables. +func (s *ClusterScope) RouteTableSpecs() []azure.ResourceSpecGetter { + var specs []azure.ResourceSpecGetter for _, subnet := range s.AzureCluster.Spec.NetworkSpec.Subnets { if subnet.RouteTable.Name != "" { - routetables = append(routetables, azure.RouteTableSpec{Name: subnet.RouteTable.Name, Subnet: subnet}) + specs = append(specs, &routetables.RouteTableSpec{ + Name: subnet.RouteTable.Name, + Location: s.Location(), + ResourceGroup: s.ResourceGroup(), + }) } } - return routetables + return specs } // NatGatewaySpecs returns the node NAT gateway. @@ -613,6 +618,7 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error { conditions.SetSummary(s.AzureCluster, conditions.WithConditions( infrav1.ResourceGroupReadyCondition, + infrav1.RouteTablesReadyCondition, infrav1.NetworkInfrastructureReadyCondition, infrav1.VnetPeeringReadyCondition, infrav1.DisksReadyCondition, @@ -626,6 +632,7 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error { patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{ clusterv1.ReadyCondition, infrav1.ResourceGroupReadyCondition, + infrav1.RouteTablesReadyCondition, infrav1.NetworkInfrastructureReadyCondition, infrav1.VnetPeeringReadyCondition, infrav1.DisksReadyCondition, diff --git a/azure/services/async/async.go b/azure/services/async/async.go index 1314f5f0ac7..4f44039f008 100644 --- a/azure/services/async/async.go +++ b/azure/services/async/async.go @@ -48,7 +48,7 @@ func New(scope FutureScope, createClient Creator, deleteClient Deleter) *Service // 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) (interface{}, error) { +func processOngoingOperation(ctx context.Context, scope FutureScope, client FutureHandler, resourceName string, serviceName string) (result interface{}, err error) { ctx, log, done := tele.StartSpanWithLogger(ctx, "async.Service.processOngoingOperation") defer done() @@ -79,7 +79,7 @@ func processOngoingOperation(ctx context.Context, scope FutureScope, client Futu // Resource has been created/deleted/updated. log.V(2).Info("long running operation has completed", "service", serviceName, "resource", resourceName) - result, err := client.Result(ctx, sdkFuture, future.Type) + result, err = client.Result(ctx, sdkFuture, future.Type) if err == nil { scope.DeleteLongRunningOperationState(resourceName, serviceName) } @@ -87,7 +87,7 @@ func processOngoingOperation(ctx context.Context, scope FutureScope, client Futu } // CreateResource implements the logic for creating a resource Asynchronously. -func (s *Service) CreateResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) (interface{}, error) { +func (s *Service) CreateResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) (result interface{}, err error) { ctx, log, done := tele.StartSpanWithLogger(ctx, "async.Service.CreateResource") defer done() @@ -138,7 +138,7 @@ func (s *Service) CreateResource(ctx context.Context, spec azure.ResourceSpecGet } // DeleteResource implements the logic for deleting a resource Asynchronously. -func (s *Service) DeleteResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) error { +func (s *Service) DeleteResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) (err error) { ctx, log, done := tele.StartSpanWithLogger(ctx, "async.Service.DeleteResource") defer done() diff --git a/azure/services/async/interfaces.go b/azure/services/async/interfaces.go index c66ec3a0d57..2798ac44479 100644 --- a/azure/services/async/interfaces.go +++ b/azure/services/async/interfaces.go @@ -31,26 +31,26 @@ type FutureScope interface { // FutureHandler is a client that can check on the progress of a future. type FutureHandler interface { // IsDone returns true if the operation is complete. - IsDone(ctx context.Context, future azureautorest.FutureAPI) (bool, error) + IsDone(ctx context.Context, future azureautorest.FutureAPI) (isDone bool, err error) // Result returns the result of the operation. - Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (interface{}, error) + Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error) } // Creator is a client that can create or update a resource asynchronously. type Creator interface { FutureHandler - CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, existingResource interface{}) (interface{}, azureautorest.FutureAPI, error) - Get(ctx context.Context, spec azure.ResourceSpecGetter) (interface{}, error) + CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (result interface{}, future azureautorest.FutureAPI, err error) + Get(ctx context.Context, spec azure.ResourceSpecGetter) (result interface{}, err error) } // Deleter is a client that can delete a resource asynchronously. type Deleter interface { FutureHandler - DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) + DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (future azureautorest.FutureAPI, err error) } // Reconciler is a generic interface used to perform asynchronous reconciliation of Azure resources. type Reconciler interface { - CreateResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) (interface{}, error) - DeleteResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) error + CreateResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) (result interface{}, err error) + DeleteResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) (err error) } diff --git a/azure/services/async/mock_async/async_mock.go b/azure/services/async/mock_async/async_mock.go index 4c4c1da09a1..fa98744eaab 100644 --- a/azure/services/async/mock_async/async_mock.go +++ b/azure/services/async/mock_async/async_mock.go @@ -205,9 +205,9 @@ func (m *MockCreator) EXPECT() *MockCreatorMockRecorder { } // CreateOrUpdateAsync mocks base method. -func (m *MockCreator) CreateOrUpdateAsync(ctx context.Context, spec azure0.ResourceSpecGetter, existingResource interface{}) (interface{}, azure.FutureAPI, error) { +func (m *MockCreator) CreateOrUpdateAsync(ctx context.Context, spec azure0.ResourceSpecGetter, parameters interface{}) (interface{}, azure.FutureAPI, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateOrUpdateAsync", ctx, spec, existingResource) + ret := m.ctrl.Call(m, "CreateOrUpdateAsync", ctx, spec, parameters) ret0, _ := ret[0].(interface{}) ret1, _ := ret[1].(azure.FutureAPI) ret2, _ := ret[2].(error) @@ -215,9 +215,9 @@ func (m *MockCreator) CreateOrUpdateAsync(ctx context.Context, spec azure0.Resou } // CreateOrUpdateAsync indicates an expected call of CreateOrUpdateAsync. -func (mr *MockCreatorMockRecorder) CreateOrUpdateAsync(ctx, spec, existingResource interface{}) *gomock.Call { +func (mr *MockCreatorMockRecorder) CreateOrUpdateAsync(ctx, spec, parameters interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*MockCreator)(nil).CreateOrUpdateAsync), ctx, spec, existingResource) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*MockCreator)(nil).CreateOrUpdateAsync), ctx, spec, parameters) } // Get mocks base method. diff --git a/azure/services/availabilitysets/client.go b/azure/services/availabilitysets/client.go index 389de4393e4..3479f43477b 100644 --- a/azure/services/availabilitysets/client.go +++ b/azure/services/availabilitysets/client.go @@ -30,11 +30,11 @@ import ( // Client wraps go-sdk. type Client interface { - Get(ctx context.Context, spec azure.ResourceSpecGetter) (interface{}, error) - CreateOrUpdateAsync(context.Context, azure.ResourceSpecGetter, interface{}) (interface{}, azureautorest.FutureAPI, error) - DeleteAsync(context.Context, azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) - IsDone(context.Context, azureautorest.FutureAPI) (bool, error) - Result(context.Context, azureautorest.FutureAPI, string) (interface{}, error) + Get(ctx context.Context, spec azure.ResourceSpecGetter) (result interface{}, err error) + CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (result interface{}, future azureautorest.FutureAPI, err error) + DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (future azureautorest.FutureAPI, err error) + IsDone(ctx context.Context, future azureautorest.FutureAPI) (isDone bool, err error) + Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error) } // AzureClient contains the Azure go-sdk Client. @@ -59,7 +59,7 @@ func newAvailabilitySetsClient(subscriptionID string, baseURI string, authorizer } // Get gets an availability set. -func (ac *AzureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) (interface{}, error) { +func (ac *AzureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) (result interface{}, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "availabilitysets.AzureClient.Get") defer done() @@ -69,7 +69,7 @@ func (ac *AzureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) ( // CreateOrUpdateAsync creates or updates a availability set asynchronously. // It sends a PUT request to Azure and if accepted without error, the func will return a Future which can be used to track the ongoing // progress of the operation. -func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (interface{}, azureautorest.FutureAPI, error) { +func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (result interface{}, future azureautorest.FutureAPI, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "availabilitySets.AzureClient.CreateOrUpdateAsync") defer done() @@ -78,18 +78,18 @@ func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.Resou return nil, nil, errors.Errorf("%T is not a compute.AvailabilitySet", parameters) } - result, err := ac.availabilitySets.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.ResourceName(), availabilitySet) + result, err = ac.availabilitySets.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.ResourceName(), availabilitySet) return result, nil, err } // DeleteAsync deletes a availability set 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) { +func (ac *AzureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (future azureautorest.FutureAPI, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "availabilitysets.AzureClient.Delete") defer done() - _, err := ac.availabilitySets.Delete(ctx, spec.ResourceGroupName(), spec.ResourceName()) + _, err = ac.availabilitySets.Delete(ctx, spec.ResourceGroupName(), spec.ResourceName()) if err != nil { return nil, err @@ -99,20 +99,20 @@ func (ac *AzureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecG } // Result fetches the result of a long-running operation future. -func (ac *AzureClient) Result(ctx context.Context, futureData azureautorest.FutureAPI, futureType string) (interface{}, error) { +func (ac *AzureClient) Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error) { // Result is a no-op for resource groups as only Delete operations return a future. return nil, nil } // 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, "availabilitysets.AzureClient.IsDone") - defer span.End() +func (ac *AzureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (isDone bool, err error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "availabilitysets.AzureClient.IsDone") + defer done() - done, err := future.DoneWithContext(ctx, ac.availabilitySets) + isDone, err = future.DoneWithContext(ctx, ac.availabilitySets) if err != nil { return false, errors.Wrap(err, "failed checking if the operation was complete") } - return done, nil + return isDone, nil } diff --git a/azure/services/availabilitysets/mock_availabilitysets/client_mock.go b/azure/services/availabilitysets/mock_availabilitysets/client_mock.go index 8ad89c67c76..431c9b7f805 100644 --- a/azure/services/availabilitysets/mock_availabilitysets/client_mock.go +++ b/azure/services/availabilitysets/mock_availabilitysets/client_mock.go @@ -53,9 +53,9 @@ func (m *MockClient) EXPECT() *MockClientMockRecorder { } // CreateOrUpdateAsync mocks base method. -func (m *MockClient) CreateOrUpdateAsync(arg0 context.Context, arg1 azure0.ResourceSpecGetter, arg2 interface{}) (interface{}, azure.FutureAPI, error) { +func (m *MockClient) CreateOrUpdateAsync(ctx context.Context, spec azure0.ResourceSpecGetter, parameters interface{}) (interface{}, azure.FutureAPI, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateOrUpdateAsync", arg0, arg1, arg2) + ret := m.ctrl.Call(m, "CreateOrUpdateAsync", ctx, spec, parameters) ret0, _ := ret[0].(interface{}) ret1, _ := ret[1].(azure.FutureAPI) ret2, _ := ret[2].(error) @@ -63,24 +63,24 @@ func (m *MockClient) CreateOrUpdateAsync(arg0 context.Context, arg1 azure0.Resou } // CreateOrUpdateAsync indicates an expected call of CreateOrUpdateAsync. -func (mr *MockClientMockRecorder) CreateOrUpdateAsync(arg0, arg1, arg2 interface{}) *gomock.Call { +func (mr *MockClientMockRecorder) CreateOrUpdateAsync(ctx, spec, parameters interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*MockClient)(nil).CreateOrUpdateAsync), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*MockClient)(nil).CreateOrUpdateAsync), ctx, spec, parameters) } // DeleteAsync mocks base method. -func (m *MockClient) DeleteAsync(arg0 context.Context, arg1 azure0.ResourceSpecGetter) (azure.FutureAPI, error) { +func (m *MockClient) DeleteAsync(ctx context.Context, spec azure0.ResourceSpecGetter) (azure.FutureAPI, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteAsync", arg0, arg1) + ret := m.ctrl.Call(m, "DeleteAsync", ctx, spec) ret0, _ := ret[0].(azure.FutureAPI) ret1, _ := ret[1].(error) return ret0, ret1 } // DeleteAsync indicates an expected call of DeleteAsync. -func (mr *MockClientMockRecorder) DeleteAsync(arg0, arg1 interface{}) *gomock.Call { +func (mr *MockClientMockRecorder) DeleteAsync(ctx, spec interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAsync", reflect.TypeOf((*MockClient)(nil).DeleteAsync), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAsync", reflect.TypeOf((*MockClient)(nil).DeleteAsync), ctx, spec) } // Get mocks base method. @@ -99,31 +99,31 @@ func (mr *MockClientMockRecorder) Get(ctx, spec interface{}) *gomock.Call { } // IsDone mocks base method. -func (m *MockClient) IsDone(arg0 context.Context, arg1 azure.FutureAPI) (bool, error) { +func (m *MockClient) IsDone(ctx context.Context, future azure.FutureAPI) (bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsDone", arg0, arg1) + ret := m.ctrl.Call(m, "IsDone", ctx, future) ret0, _ := ret[0].(bool) ret1, _ := ret[1].(error) return ret0, ret1 } // IsDone indicates an expected call of IsDone. -func (mr *MockClientMockRecorder) IsDone(arg0, arg1 interface{}) *gomock.Call { +func (mr *MockClientMockRecorder) IsDone(ctx, future interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDone", reflect.TypeOf((*MockClient)(nil).IsDone), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDone", reflect.TypeOf((*MockClient)(nil).IsDone), ctx, future) } // Result mocks base method. -func (m *MockClient) Result(arg0 context.Context, arg1 azure.FutureAPI, arg2 string) (interface{}, error) { +func (m *MockClient) Result(ctx context.Context, future azure.FutureAPI, futureType string) (interface{}, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Result", arg0, arg1, arg2) + ret := m.ctrl.Call(m, "Result", ctx, future, futureType) ret0, _ := ret[0].(interface{}) ret1, _ := ret[1].(error) return ret0, ret1 } // Result indicates an expected call of Result. -func (mr *MockClientMockRecorder) Result(arg0, arg1, arg2 interface{}) *gomock.Call { +func (mr *MockClientMockRecorder) Result(ctx, future, futureType interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Result", reflect.TypeOf((*MockClient)(nil).Result), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Result", reflect.TypeOf((*MockClient)(nil).Result), ctx, future, futureType) } diff --git a/azure/services/availabilitysets/spec.go b/azure/services/availabilitysets/spec.go index 1f884500897..a2131991101 100644 --- a/azure/services/availabilitysets/spec.go +++ b/azure/services/availabilitysets/spec.go @@ -53,7 +53,7 @@ func (s *AvailabilitySetSpec) OwnerResourceName() string { } // Parameters returns the parameters for the availability set. -func (s *AvailabilitySetSpec) Parameters(existing interface{}) (interface{}, error) { +func (s *AvailabilitySetSpec) Parameters(existing interface{}) (params interface{}, err error) { if existing != nil { if _, ok := existing.(compute.AvailabilitySet); !ok { return nil, errors.Errorf("%T is not a compute.AvailabilitySet", existing) diff --git a/azure/services/disks/client.go b/azure/services/disks/client.go index 26623eb96f3..487348c0543 100644 --- a/azure/services/disks/client.go +++ b/azure/services/disks/client.go @@ -50,11 +50,11 @@ func NewDisksClient(subscriptionID string, baseURI string, authorizer autorest.A // DeleteAsync deletes a route table 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) { +func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (future azureautorest.FutureAPI, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "disks.azureClient.DeleteAsync") defer done() - future, err := ac.disks.Delete(ctx, spec.ResourceGroupName(), spec.ResourceName()) + deleteFuture, err := ac.disks.Delete(ctx, spec.ResourceGroupName(), spec.ResourceName()) if err != nil { return nil, err } @@ -62,28 +62,28 @@ func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecG ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) defer cancel() - err = future.WaitForCompletionRef(ctx, ac.disks.Client) + err = deleteFuture.WaitForCompletionRef(ctx, ac.disks.Client) if err != nil { // if an error occurs, return the future. // this means the long-running operation didn't finish in the specified timeout. - return &future, err + return &deleteFuture, err } - _, err = future.Result(ac.disks) + _, err = deleteFuture.Result(ac.disks) // if the operation completed, return a nil future. return nil, err } // Result fetches the result of a long-running operation future. -func (ac *azureClient) Result(ctx context.Context, futureData azureautorest.FutureAPI, futureType string) (interface{}, error) { +func (ac *azureClient) Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error) { return nil, nil } // IsDone returns true if the long-running operation has completed. -func (ac *azureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (bool, error) { +func (ac *azureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (isDone bool, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "disks.azureClient.IsDone") defer done() - isDone, err := future.DoneWithContext(ctx, ac.disks) + isDone, err = future.DoneWithContext(ctx, ac.disks) if err != nil { return false, errors.Wrap(err, "failed checking if the operation was complete") } diff --git a/azure/services/disks/disks.go b/azure/services/disks/disks.go index 516f8030c79..f150d46ce26 100644 --- a/azure/services/disks/disks.go +++ b/azure/services/disks/disks.go @@ -70,8 +70,8 @@ func (s *Service) Delete(ctx context.Context) error { var result error // We go through the list of DiskSpecs to delete each one, independently of the result of the previous one. - // If multiple errors occur, we return the most pressing one - // order of precedence is: error deleting -> deleting in progress -> deleted (no error) + // If multiple errors occur, we return the most pressing one. + // Order of precedence (highest -> lowest) is: error that is not an operationNotDoneError (ie. error creating) -> operationNotDoneError (ie. creating in progress) -> no error (ie. created) for _, diskSpec := range s.Scope.DiskSpecs() { if err := s.DeleteResource(ctx, diskSpec, serviceName); err != nil { if !azure.IsOperationNotDoneError(err) || result == nil { diff --git a/azure/services/disks/spec.go b/azure/services/disks/spec.go index a4b81d704d1..6dd9cd8ee46 100644 --- a/azure/services/disks/spec.go +++ b/azure/services/disks/spec.go @@ -38,6 +38,6 @@ func (s *DiskSpec) OwnerResourceName() string { } // Parameters is a no-op for disks. -func (s *DiskSpec) Parameters(existing interface{}) (interface{}, error) { +func (s *DiskSpec) Parameters(existing interface{}) (params interface{}, err error) { return nil, nil } diff --git a/azure/services/groups/client.go b/azure/services/groups/client.go index 447537776da..896edd9926b 100644 --- a/azure/services/groups/client.go +++ b/azure/services/groups/client.go @@ -32,10 +32,10 @@ import ( // client wraps go-sdk. type client interface { Get(context.Context, azure.ResourceSpecGetter) (interface{}, error) - CreateOrUpdateAsync(context.Context, azure.ResourceSpecGetter, interface{}) (interface{}, azureautorest.FutureAPI, error) - DeleteAsync(context.Context, azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) - IsDone(context.Context, azureautorest.FutureAPI) (bool, error) - Result(context.Context, azureautorest.FutureAPI, string) (interface{}, error) + CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (result interface{}, future azureautorest.FutureAPI, err error) + DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (future azureautorest.FutureAPI, err error) + IsDone(ctx context.Context, future azureautorest.FutureAPI) (isDone bool, err error) + Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error) } // azureClient contains the Azure go-sdk Client. @@ -61,7 +61,7 @@ func newGroupsClient(subscriptionID string, baseURI string, authorizer autorest. } // Get gets a resource group. -func (ac *azureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) (interface{}, error) { +func (ac *azureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) (result interface{}, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "groups.AzureClient.Get") defer done() @@ -70,7 +70,7 @@ func (ac *azureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) ( // 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, parameters interface{}) (interface{}, azureautorest.FutureAPI, error) { +func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (result interface{}, future azureautorest.FutureAPI, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "groups.AzureClient.CreateOrUpdate") defer done() @@ -79,7 +79,7 @@ func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.Resou return nil, nil, errors.Errorf("%T is not a resources.Group", parameters) } - result, err := ac.groups.CreateOrUpdate(ctx, spec.ResourceName(), group) + result, err = ac.groups.CreateOrUpdate(ctx, spec.ResourceName(), group) return result, nil, err } @@ -88,11 +88,11 @@ func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.Resou // 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) { +func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (future azureautorest.FutureAPI, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "groups.AzureClient.Delete") defer done() - future, err := ac.groups.Delete(ctx, spec.ResourceName()) + deleteFuture, err := ac.groups.Delete(ctx, spec.ResourceName()) if err != nil { return nil, err } @@ -100,23 +100,23 @@ func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecG ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) defer cancel() - err = future.WaitForCompletionRef(ctx, ac.groups.Client) + err = deleteFuture.WaitForCompletionRef(ctx, ac.groups.Client) if err != nil { // if an error occurs, return the future. // this means the long-running operation didn't finish in the specified timeout. - return &future, err + return &deleteFuture, err } - _, err = future.Result(ac.groups) + _, err = deleteFuture.Result(ac.groups) // 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) { +func (ac *azureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (isDone bool, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "groups.AzureClient.IsDone") defer done() - isDone, err := future.DoneWithContext(ctx, ac.groups) + isDone, err = future.DoneWithContext(ctx, ac.groups) if err != nil { return false, errors.Wrap(err, "failed checking if the operation was complete") } @@ -125,7 +125,7 @@ func (ac *azureClient) IsDone(ctx context.Context, future azureautorest.FutureAP } // Result fetches the result of a long-running operation future. -func (ac *azureClient) Result(ctx context.Context, futureData azureautorest.FutureAPI, futureType string) (interface{}, error) { +func (ac *azureClient) Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error) { // Result is a no-op for resource groups as only Delete operations return a future. return nil, nil } diff --git a/azure/services/groups/mock_groups/client_mock.go b/azure/services/groups/mock_groups/client_mock.go index 77b530db5e3..21d71b7382d 100644 --- a/azure/services/groups/mock_groups/client_mock.go +++ b/azure/services/groups/mock_groups/client_mock.go @@ -53,9 +53,9 @@ func (m *Mockclient) EXPECT() *MockclientMockRecorder { } // CreateOrUpdateAsync mocks base method. -func (m *Mockclient) CreateOrUpdateAsync(arg0 context.Context, arg1 azure0.ResourceSpecGetter, arg2 interface{}) (interface{}, azure.FutureAPI, error) { +func (m *Mockclient) CreateOrUpdateAsync(ctx context.Context, spec azure0.ResourceSpecGetter, parameters interface{}) (interface{}, azure.FutureAPI, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateOrUpdateAsync", arg0, arg1, arg2) + ret := m.ctrl.Call(m, "CreateOrUpdateAsync", ctx, spec, parameters) ret0, _ := ret[0].(interface{}) ret1, _ := ret[1].(azure.FutureAPI) ret2, _ := ret[2].(error) @@ -63,24 +63,24 @@ func (m *Mockclient) CreateOrUpdateAsync(arg0 context.Context, arg1 azure0.Resou } // CreateOrUpdateAsync indicates an expected call of CreateOrUpdateAsync. -func (mr *MockclientMockRecorder) CreateOrUpdateAsync(arg0, arg1, arg2 interface{}) *gomock.Call { +func (mr *MockclientMockRecorder) CreateOrUpdateAsync(ctx, spec, parameters interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*Mockclient)(nil).CreateOrUpdateAsync), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*Mockclient)(nil).CreateOrUpdateAsync), ctx, spec, parameters) } // DeleteAsync mocks base method. -func (m *Mockclient) DeleteAsync(arg0 context.Context, arg1 azure0.ResourceSpecGetter) (azure.FutureAPI, error) { +func (m *Mockclient) DeleteAsync(ctx context.Context, spec azure0.ResourceSpecGetter) (azure.FutureAPI, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteAsync", arg0, arg1) + ret := m.ctrl.Call(m, "DeleteAsync", ctx, spec) ret0, _ := ret[0].(azure.FutureAPI) ret1, _ := ret[1].(error) return ret0, ret1 } // DeleteAsync indicates an expected call of DeleteAsync. -func (mr *MockclientMockRecorder) DeleteAsync(arg0, arg1 interface{}) *gomock.Call { +func (mr *MockclientMockRecorder) DeleteAsync(ctx, spec interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAsync", reflect.TypeOf((*Mockclient)(nil).DeleteAsync), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAsync", reflect.TypeOf((*Mockclient)(nil).DeleteAsync), ctx, spec) } // Get mocks base method. @@ -99,31 +99,31 @@ func (mr *MockclientMockRecorder) Get(arg0, arg1 interface{}) *gomock.Call { } // IsDone mocks base method. -func (m *Mockclient) IsDone(arg0 context.Context, arg1 azure.FutureAPI) (bool, error) { +func (m *Mockclient) IsDone(ctx context.Context, future azure.FutureAPI) (bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsDone", arg0, arg1) + ret := m.ctrl.Call(m, "IsDone", ctx, future) ret0, _ := ret[0].(bool) ret1, _ := ret[1].(error) return ret0, ret1 } // IsDone indicates an expected call of IsDone. -func (mr *MockclientMockRecorder) IsDone(arg0, arg1 interface{}) *gomock.Call { +func (mr *MockclientMockRecorder) IsDone(ctx, future interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDone", reflect.TypeOf((*Mockclient)(nil).IsDone), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDone", reflect.TypeOf((*Mockclient)(nil).IsDone), ctx, future) } // Result mocks base method. -func (m *Mockclient) Result(arg0 context.Context, arg1 azure.FutureAPI, arg2 string) (interface{}, error) { +func (m *Mockclient) Result(ctx context.Context, future azure.FutureAPI, futureType string) (interface{}, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Result", arg0, arg1, arg2) + ret := m.ctrl.Call(m, "Result", ctx, future, futureType) ret0, _ := ret[0].(interface{}) ret1, _ := ret[1].(error) return ret0, ret1 } // Result indicates an expected call of Result. -func (mr *MockclientMockRecorder) Result(arg0, arg1, arg2 interface{}) *gomock.Call { +func (mr *MockclientMockRecorder) Result(ctx, future, futureType interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Result", reflect.TypeOf((*Mockclient)(nil).Result), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Result", reflect.TypeOf((*Mockclient)(nil).Result), ctx, future, futureType) } diff --git a/azure/services/groups/spec.go b/azure/services/groups/spec.go index cd4a407600b..bc8bcc686d4 100644 --- a/azure/services/groups/spec.go +++ b/azure/services/groups/spec.go @@ -48,7 +48,7 @@ func (s *GroupSpec) OwnerResourceName() string { } // Parameters returns the parameters for the group. -func (s *GroupSpec) Parameters(existing interface{}) (interface{}, error) { +func (s *GroupSpec) Parameters(existing interface{}) (params interface{}, err error) { if existing != nil { // rg already exists, nothing to update. // Note that rg tags are updated separately using tags service. diff --git a/azure/services/natgateways/client.go b/azure/services/natgateways/client.go index 1490bba326d..2685be00726 100644 --- a/azure/services/natgateways/client.go +++ b/azure/services/natgateways/client.go @@ -50,7 +50,7 @@ func netNatGatewaysClient(subscriptionID string, baseURI string, authorizer auto } // Get gets the specified nat gateway. -func (ac *azureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) (interface{}, error) { +func (ac *azureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) (result interface{}, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "natgateways.azureClient.Get") defer done() @@ -60,7 +60,7 @@ func (ac *azureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) ( // CreateOrUpdateAsync creates or updates a Nat Gateway asynchronously. // It sends a PUT request to Azure and if accepted without error, the func will return a Future which can be used to track the ongoing // progress of the operation. -func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (interface{}, azureautorest.FutureAPI, error) { +func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (result interface{}, future azureautorest.FutureAPI, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "natgateways.azureClient.CreateOrUpdateAsync") defer done() @@ -69,7 +69,7 @@ func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.Resou return nil, nil, errors.Errorf("%T is not a network.NatGateway", parameters) } - future, err := ac.natgateways.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.ResourceName(), natGateway) + createFuture, err := ac.natgateways.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.ResourceName(), natGateway) if err != nil { return nil, nil, err } @@ -77,14 +77,14 @@ func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.Resou ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) defer cancel() - err = future.WaitForCompletionRef(ctx, ac.natgateways.Client) + err = createFuture.WaitForCompletionRef(ctx, ac.natgateways.Client) if err != nil { // if an error occurs, return the future. // this means the long-running operation didn't finish in the specified timeout. - return nil, &future, err + return nil, &createFuture, err } - result, err := future.Result(ac.natgateways) + result, err = createFuture.Result(ac.natgateways) // if the operation completed, return a nil future return result, nil, err } @@ -92,11 +92,11 @@ func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.Resou // DeleteAsync deletes a Nat Gateway 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) { +func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (future azureautorest.FutureAPI, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "natgateways.azureClient.DeleteAsync") defer done() - future, err := ac.natgateways.Delete(ctx, spec.ResourceGroupName(), spec.ResourceName()) + deleteFuture, err := ac.natgateways.Delete(ctx, spec.ResourceGroupName(), spec.ResourceName()) if err != nil { return nil, err } @@ -104,23 +104,23 @@ func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecG ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) defer cancel() - err = future.WaitForCompletionRef(ctx, ac.natgateways.Client) + err = deleteFuture.WaitForCompletionRef(ctx, ac.natgateways.Client) if err != nil { // if an error occurs, return the future. // this means the long-running operation didn't finish in the specified timeout. - return &future, err + return &deleteFuture, err } - _, err = future.Result(ac.natgateways) + _, err = deleteFuture.Result(ac.natgateways) // 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) { +func (ac *azureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (isDone bool, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "natgateways.azureClient.IsDone") defer done() - isDone, err := future.DoneWithContext(ctx, ac.natgateways) + isDone, err = future.DoneWithContext(ctx, ac.natgateways) if err != nil { return false, errors.Wrap(err, "failed checking if the operation was complete") } @@ -129,26 +129,28 @@ func (ac *azureClient) IsDone(ctx context.Context, future azureautorest.FutureAP } // Result fetches the result of a long-running operation future. -func (ac *azureClient) Result(ctx context.Context, futureData azureautorest.FutureAPI, futureType string) (interface{}, error) { +func (ac *azureClient) Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error) { _, _, done := tele.StartSpanWithLogger(ctx, "natgateways.azureClient.Result") defer done() - if futureData == nil { + if future == nil { return nil, errors.Errorf("cannot get result from nil future") } - var result func(client network.NatGatewaysClient) (natGateway network.NatGateway, err error) switch futureType { case infrav1.PutFuture: - var future *network.NatGatewaysCreateOrUpdateFuture - jsonData, err := futureData.MarshalJSON() + // Marshal and Unmarshal the future to put it into the correct future type so we can access the Result function. + // Unfortunately the FutureAPI can't be casted directly to NatGatewaysCreateOrUpdateFuture because it is a azureautorest.Future, which doesn't implement the Result function. See PR #1686 for discussion on alternatives. + // It was converted back to a generic azureautorest.Future from the CAPZ infrav1.Future type stored in Status: https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/main/azure/converters/futures.go#L49. + var createFuture *network.NatGatewaysCreateOrUpdateFuture + jsonData, err := future.MarshalJSON() if err != nil { return nil, errors.Wrap(err, "failed to marshal future") } - if err := json.Unmarshal(jsonData, &future); err != nil { + if err := json.Unmarshal(jsonData, &createFuture); err != nil { return nil, errors.Wrap(err, "failed to unmarshal future data") } - result = (*future).Result + return (*createFuture).Result(ac.natgateways) case infrav1.DeleteFuture: // Delete does not return a result NAT gateway @@ -157,6 +159,4 @@ func (ac *azureClient) Result(ctx context.Context, futureData azureautorest.Futu default: return nil, errors.Errorf("unknown future type %q", futureType) } - - return result(ac.natgateways) } diff --git a/azure/services/natgateways/natgateways.go b/azure/services/natgateways/natgateways.go index 7e2b1b30a64..9db4332c538 100644 --- a/azure/services/natgateways/natgateways.go +++ b/azure/services/natgateways/natgateways.go @@ -70,8 +70,8 @@ func (s *Service) Reconcile(ctx context.Context) error { } // We go through the list of NatGatewaySpecs to reconcile each one, independently of the resultingErr of the previous one. - // If multiple errors occur, we return the most pressing one - // order of precedence is: error creating -> creating in progress -> created (no error) + // If multiple errors occur, we return the most pressing one. + // Order of precedence (highest -> lowest) is: error that is not an operationNotDoneError (ie. error creating) -> operationNotDoneError (ie. creating in progress) -> no error (ie. created) var resultingErr error for _, natGatewaySpec := range s.Scope.NatGatewaySpecs() { result, err := s.CreateResource(ctx, natGatewaySpec, serviceName) @@ -115,8 +115,8 @@ func (s *Service) Delete(ctx context.Context) error { var resultingErr error // We go through the list of NatGatewaySpecs to delete each one, independently of the resultingErr of the previous one. - // If multiple errors occur, we return the most pressing one - // order of precedence is: error deleting -> deleting in progress -> deleted (no error) + // If multiple errors occur, we return the most pressing one. + // Order of precedence (highest -> lowest) is: error that is not an operationNotDoneError (ie. error creating) -> operationNotDoneError (ie. creating in progress) -> no error (ie. created) for _, natGatewaySpec := range s.Scope.NatGatewaySpecs() { if err := s.DeleteResource(ctx, natGatewaySpec, serviceName); err != nil { if !azure.IsOperationNotDoneError(err) || resultingErr == nil { diff --git a/azure/services/natgateways/spec.go b/azure/services/natgateways/spec.go index 6999d48b4bb..4cf0ee384fd 100644 --- a/azure/services/natgateways/spec.go +++ b/azure/services/natgateways/spec.go @@ -50,7 +50,7 @@ func (s *NatGatewaySpec) OwnerResourceName() string { } // Parameters returns the parameters for the NAT gateway. -func (s *NatGatewaySpec) Parameters(existing interface{}) (interface{}, error) { +func (s *NatGatewaySpec) Parameters(existing interface{}) (params interface{}, err error) { if existing != nil { existingNatGateway, ok := existing.(network.NatGateway) if !ok { diff --git a/azure/services/routetables/client.go b/azure/services/routetables/client.go index 91bb7dbc742..45510ffe1f0 100644 --- a/azure/services/routetables/client.go +++ b/azure/services/routetables/client.go @@ -18,29 +18,25 @@ package routetables import ( "context" + "encoding/json" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" "github.com/Azure/go-autorest/autorest" + azureautorest "github.com/Azure/go-autorest/autorest/azure" + "github.com/pkg/errors" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" + "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) -// client wraps go-sdk. -type client interface { - Get(context.Context, string, string) (network.RouteTable, error) - CreateOrUpdate(context.Context, string, string, network.RouteTable) error - Delete(context.Context, string, string) error -} - // azureClient contains the Azure go-sdk Client. type azureClient struct { routetables network.RouteTablesClient } -var _ client = (*azureClient)(nil) - -// newClient creates a new VM client from subscription ID. +// newClient creates a new route tables client from subscription ID. func newClient(auth azure.Authorizer) *azureClient { c := newRouteTablesClient(auth.SubscriptionID(), auth.BaseURI(), auth.Authorizer()) return &azureClient{c} @@ -54,43 +50,112 @@ func newRouteTablesClient(subscriptionID string, baseURI string, authorizer auto } // Get gets the specified route table. -func (ac *azureClient) Get(ctx context.Context, resourceGroupName, rtName string) (network.RouteTable, error) { - ctx, _, done := tele.StartSpanWithLogger(ctx, "routetables.AzureClient.Get") +func (ac *azureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) (result interface{}, err error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "routetables.azureClient.Get") defer done() - return ac.routetables.Get(ctx, resourceGroupName, rtName, "") + return ac.routetables.Get(ctx, spec.ResourceGroupName(), spec.ResourceName(), "") } -// CreateOrUpdate create or updates a route table in a specified resource group. -func (ac *azureClient) CreateOrUpdate(ctx context.Context, resourceGroupName string, rtName string, rt network.RouteTable) error { - ctx, _, done := tele.StartSpanWithLogger(ctx, "routetables.AzureClient.CreateOrUpdate") +// CreateOrUpdateAsync creates or updates a route table asynchronously. +// It sends a PUT request to Azure and if accepted without error, the func will return a Future which can be used to track the ongoing +// progress of the operation. +func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (result interface{}, future azureautorest.FutureAPI, err error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "routetables.azureClient.CreateOrUpdateAsync") defer done() - future, err := ac.routetables.CreateOrUpdate(ctx, resourceGroupName, rtName, rt) + rt, ok := parameters.(network.RouteTable) + if !ok { + return nil, nil, errors.Errorf("%T is not a network.RouteTable", parameters) + } + + createFuture, err := ac.routetables.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.ResourceName(), rt) if err != nil { - return err + return nil, nil, err } - err = future.WaitForCompletionRef(ctx, ac.routetables.Client) + + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + + err = createFuture.WaitForCompletionRef(ctx, ac.routetables.Client) if err != nil { - return err + // if an error occurs, return the future. + // this means the long-running operation didn't finish in the specified timeout. + return nil, &createFuture, err } - _, err = future.Result(ac.routetables) - return err + result, err = createFuture.Result(ac.routetables) + // if the operation completed, return a nil future + return result, nil, err } -// Delete deletes the specified route table. -func (ac *azureClient) Delete(ctx context.Context, resourceGroupName, rtName string) error { - ctx, _, done := tele.StartSpanWithLogger(ctx, "routetables.AzureClient.Delete") +// DeleteAsync deletes a route table asynchronously. DeleteAsync sends a DELETE +// request to Azure and if accepted without error, the func will return a Future which can be used to track the ongoing +// progress of the operation. +func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (future azureautorest.FutureAPI, err error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "routetables.azureClient.DeleteAsync") defer done() - future, err := ac.routetables.Delete(ctx, resourceGroupName, rtName) + deleteFuture, err := ac.routetables.Delete(ctx, spec.ResourceGroupName(), spec.ResourceName()) if err != nil { - return err + return nil, err } - err = future.WaitForCompletionRef(ctx, ac.routetables.Client) + + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + + err = deleteFuture.WaitForCompletionRef(ctx, ac.routetables.Client) if err != nil { - return err + // if an error occurs, return the future. + // this means the long-running operation didn't finish in the specified timeout. + return &deleteFuture, err + } + _, err = deleteFuture.Result(ac.routetables) + // 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) (isDone bool, err error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "routetables.azureClient.IsDone") + defer done() + + isDone, err = future.DoneWithContext(ctx, ac.routetables) + if err != nil { + return false, errors.Wrap(err, "failed checking if the operation was complete") + } + + return isDone, nil +} + +// Result fetches the result of a long-running operation future. +func (ac *azureClient) Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error) { + _, _, done := tele.StartSpanWithLogger(ctx, "routetables.azureClient.Result") + defer done() + + if future == nil { + return nil, errors.Errorf("cannot get result from nil future") + } + + switch futureType { + case infrav1.PutFuture: + // Marshal and Unmarshal the future to put it into the correct future type so we can access the Result function. + // Unfortunately the FutureAPI can't be casted directly to RouteTablesCreateOrUpdateFuture because it is a azureautorest.Future, which doesn't implement the Result function. See PR #1686 for discussion on alternatives. + // It was converted back to a generic azureautorest.Future from the CAPZ infrav1.Future type stored in Status: https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/main/azure/converters/futures.go#L49. + var createFuture *network.RouteTablesCreateOrUpdateFuture + jsonData, err := future.MarshalJSON() + if err != nil { + return nil, errors.Wrap(err, "failed to marshal future") + } + if err := json.Unmarshal(jsonData, &createFuture); err != nil { + return nil, errors.Wrap(err, "failed to unmarshal future data") + } + return (*createFuture).Result(ac.routetables) + + case infrav1.DeleteFuture: + // Delete does not return a result route table. + return nil, nil + + default: + return nil, errors.Errorf("unknown future type %q", futureType) } - _, err = future.Result(ac.routetables) - return err } diff --git a/azure/services/routetables/mock_routetables/client_mock.go b/azure/services/routetables/mock_routetables/client_mock.go deleted file mode 100644 index c8d9e464f63..00000000000 --- a/azure/services/routetables/mock_routetables/client_mock.go +++ /dev/null @@ -1,95 +0,0 @@ -/* -Copyright The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// Code generated by MockGen. DO NOT EDIT. -// Source: ../client.go - -// Package mock_routetables is a generated GoMock package. -package mock_routetables - -import ( - context "context" - reflect "reflect" - - network "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" - gomock "github.com/golang/mock/gomock" -) - -// Mockclient is a mock of client interface. -type Mockclient struct { - ctrl *gomock.Controller - recorder *MockclientMockRecorder -} - -// MockclientMockRecorder is the mock recorder for Mockclient. -type MockclientMockRecorder struct { - mock *Mockclient -} - -// NewMockclient creates a new mock instance. -func NewMockclient(ctrl *gomock.Controller) *Mockclient { - mock := &Mockclient{ctrl: ctrl} - mock.recorder = &MockclientMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *Mockclient) EXPECT() *MockclientMockRecorder { - return m.recorder -} - -// CreateOrUpdate mocks base method. -func (m *Mockclient) CreateOrUpdate(arg0 context.Context, arg1, arg2 string, arg3 network.RouteTable) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateOrUpdate", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(error) - return ret0 -} - -// CreateOrUpdate indicates an expected call of CreateOrUpdate. -func (mr *MockclientMockRecorder) CreateOrUpdate(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdate", reflect.TypeOf((*Mockclient)(nil).CreateOrUpdate), arg0, arg1, arg2, arg3) -} - -// Delete mocks base method. -func (m *Mockclient) Delete(arg0 context.Context, arg1, arg2 string) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Delete", arg0, arg1, arg2) - ret0, _ := ret[0].(error) - return ret0 -} - -// Delete indicates an expected call of Delete. -func (mr *MockclientMockRecorder) Delete(arg0, arg1, arg2 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*Mockclient)(nil).Delete), arg0, arg1, arg2) -} - -// Get mocks base method. -func (m *Mockclient) Get(arg0 context.Context, arg1, arg2 string) (network.RouteTable, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Get", arg0, arg1, arg2) - ret0, _ := ret[0].(network.RouteTable) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// Get indicates an expected call of Get. -func (mr *MockclientMockRecorder) Get(arg0, arg1, arg2 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*Mockclient)(nil).Get), arg0, arg1, arg2) -} diff --git a/azure/services/routetables/mock_routetables/doc.go b/azure/services/routetables/mock_routetables/doc.go index ac09cdfadb2..d7763103ffb 100644 --- a/azure/services/routetables/mock_routetables/doc.go +++ b/azure/services/routetables/mock_routetables/doc.go @@ -15,8 +15,6 @@ limitations under the License. */ // Run go generate to regenerate this mock. -//go:generate ../../../../hack/tools/bin/mockgen -destination client_mock.go -package mock_routetables -source ../client.go Client //go:generate ../../../../hack/tools/bin/mockgen -destination routetables_mock.go -package mock_routetables -source ../routetables.go RouteTableScope -//go:generate /usr/bin/env bash -c "cat ../../../../hack/boilerplate/boilerplate.generatego.txt client_mock.go > _client_mock.go && mv _client_mock.go client_mock.go" //go:generate /usr/bin/env bash -c "cat ../../../../hack/boilerplate/boilerplate.generatego.txt routetables_mock.go > _routetables_mock.go && mv _routetables_mock.go routetables_mock.go" package mock_routetables //nolint diff --git a/azure/services/routetables/mock_routetables/routetables_mock.go b/azure/services/routetables/mock_routetables/routetables_mock.go index c6649694910..b0a5b9813bb 100644 --- a/azure/services/routetables/mock_routetables/routetables_mock.go +++ b/azure/services/routetables/mock_routetables/routetables_mock.go @@ -27,6 +27,7 @@ import ( gomock "github.com/golang/mock/gomock" v1beta1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" azure "sigs.k8s.io/cluster-api-provider-azure/azure" + v1beta10 "sigs.k8s.io/cluster-api/api/v1beta1" ) // MockRouteTableScope is a mock of RouteTableScope interface. @@ -52,48 +53,6 @@ func (m *MockRouteTableScope) EXPECT() *MockRouteTableScopeMockRecorder { return m.recorder } -// APIServerLBName mocks base method. -func (m *MockRouteTableScope) 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 *MockRouteTableScopeMockRecorder) APIServerLBName() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "APIServerLBName", reflect.TypeOf((*MockRouteTableScope)(nil).APIServerLBName)) -} - -// APIServerLBPoolName mocks base method. -func (m *MockRouteTableScope) 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 *MockRouteTableScopeMockRecorder) APIServerLBPoolName(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "APIServerLBPoolName", reflect.TypeOf((*MockRouteTableScope)(nil).APIServerLBPoolName), arg0) -} - -// AdditionalTags mocks base method. -func (m *MockRouteTableScope) AdditionalTags() v1beta1.Tags { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AdditionalTags") - ret0, _ := ret[0].(v1beta1.Tags) - return ret0 -} - -// AdditionalTags indicates an expected call of AdditionalTags. -func (mr *MockRouteTableScopeMockRecorder) AdditionalTags() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AdditionalTags", reflect.TypeOf((*MockRouteTableScope)(nil).AdditionalTags)) -} - // Authorizer mocks base method. func (m *MockRouteTableScope) Authorizer() autorest.Authorizer { m.ctrl.T.Helper() @@ -108,20 +67,6 @@ func (mr *MockRouteTableScopeMockRecorder) Authorizer() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Authorizer", reflect.TypeOf((*MockRouteTableScope)(nil).Authorizer)) } -// AvailabilitySetEnabled mocks base method. -func (m *MockRouteTableScope) 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 *MockRouteTableScopeMockRecorder) AvailabilitySetEnabled() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AvailabilitySetEnabled", reflect.TypeOf((*MockRouteTableScope)(nil).AvailabilitySetEnabled)) -} - // BaseURI mocks base method. func (m *MockRouteTableScope) BaseURI() string { m.ctrl.T.Helper() @@ -178,88 +123,30 @@ func (mr *MockRouteTableScopeMockRecorder) CloudEnvironment() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CloudEnvironment", reflect.TypeOf((*MockRouteTableScope)(nil).CloudEnvironment)) } -// CloudProviderConfigOverrides mocks base method. -func (m *MockRouteTableScope) CloudProviderConfigOverrides() *v1beta1.CloudProviderConfigOverrides { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CloudProviderConfigOverrides") - ret0, _ := ret[0].(*v1beta1.CloudProviderConfigOverrides) - return ret0 -} - -// CloudProviderConfigOverrides indicates an expected call of CloudProviderConfigOverrides. -func (mr *MockRouteTableScopeMockRecorder) CloudProviderConfigOverrides() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CloudProviderConfigOverrides", reflect.TypeOf((*MockRouteTableScope)(nil).CloudProviderConfigOverrides)) -} - -// ClusterName mocks base method. -func (m *MockRouteTableScope) 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 *MockRouteTableScopeMockRecorder) ClusterName() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClusterName", reflect.TypeOf((*MockRouteTableScope)(nil).ClusterName)) -} - -// ControlPlaneRouteTable mocks base method. -func (m *MockRouteTableScope) ControlPlaneRouteTable() v1beta1.RouteTable { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ControlPlaneRouteTable") - ret0, _ := ret[0].(v1beta1.RouteTable) - return ret0 -} - -// ControlPlaneRouteTable indicates an expected call of ControlPlaneRouteTable. -func (mr *MockRouteTableScopeMockRecorder) ControlPlaneRouteTable() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ControlPlaneRouteTable", reflect.TypeOf((*MockRouteTableScope)(nil).ControlPlaneRouteTable)) -} - -// ControlPlaneSubnet mocks base method. -func (m *MockRouteTableScope) ControlPlaneSubnet() v1beta1.SubnetSpec { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ControlPlaneSubnet") - ret0, _ := ret[0].(v1beta1.SubnetSpec) - return ret0 -} - -// ControlPlaneSubnet indicates an expected call of ControlPlaneSubnet. -func (mr *MockRouteTableScopeMockRecorder) ControlPlaneSubnet() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ControlPlaneSubnet", reflect.TypeOf((*MockRouteTableScope)(nil).ControlPlaneSubnet)) -} - -// FailureDomains mocks base method. -func (m *MockRouteTableScope) FailureDomains() []string { +// DeleteLongRunningOperationState mocks base method. +func (m *MockRouteTableScope) DeleteLongRunningOperationState(arg0, arg1 string) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "FailureDomains") - ret0, _ := ret[0].([]string) - return ret0 + m.ctrl.Call(m, "DeleteLongRunningOperationState", arg0, arg1) } -// FailureDomains indicates an expected call of FailureDomains. -func (mr *MockRouteTableScopeMockRecorder) FailureDomains() *gomock.Call { +// DeleteLongRunningOperationState indicates an expected call of DeleteLongRunningOperationState. +func (mr *MockRouteTableScopeMockRecorder) DeleteLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FailureDomains", reflect.TypeOf((*MockRouteTableScope)(nil).FailureDomains)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteLongRunningOperationState", reflect.TypeOf((*MockRouteTableScope)(nil).DeleteLongRunningOperationState), arg0, arg1) } -// GetPrivateDNSZoneName mocks base method. -func (m *MockRouteTableScope) GetPrivateDNSZoneName() string { +// GetLongRunningOperationState mocks base method. +func (m *MockRouteTableScope) GetLongRunningOperationState(arg0, arg1 string) *v1beta1.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].(*v1beta1.Future) return ret0 } -// GetPrivateDNSZoneName indicates an expected call of GetPrivateDNSZoneName. -func (mr *MockRouteTableScopeMockRecorder) GetPrivateDNSZoneName() *gomock.Call { +// GetLongRunningOperationState indicates an expected call of GetLongRunningOperationState. +func (mr *MockRouteTableScopeMockRecorder) GetLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPrivateDNSZoneName", reflect.TypeOf((*MockRouteTableScope)(nil).GetPrivateDNSZoneName)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLongRunningOperationState", reflect.TypeOf((*MockRouteTableScope)(nil).GetLongRunningOperationState), arg0, arg1) } // HashKey mocks base method. @@ -276,34 +163,6 @@ func (mr *MockRouteTableScopeMockRecorder) HashKey() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HashKey", reflect.TypeOf((*MockRouteTableScope)(nil).HashKey)) } -// IsAPIServerPrivate mocks base method. -func (m *MockRouteTableScope) 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 *MockRouteTableScopeMockRecorder) IsAPIServerPrivate() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsAPIServerPrivate", reflect.TypeOf((*MockRouteTableScope)(nil).IsAPIServerPrivate)) -} - -// IsIPv6Enabled mocks base method. -func (m *MockRouteTableScope) 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 *MockRouteTableScopeMockRecorder) IsIPv6Enabled() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsIPv6Enabled", reflect.TypeOf((*MockRouteTableScope)(nil).IsIPv6Enabled)) -} - // IsVnetManaged mocks base method. func (m *MockRouteTableScope) IsVnetManaged() bool { m.ctrl.T.Helper() @@ -318,81 +177,11 @@ func (mr *MockRouteTableScopeMockRecorder) IsVnetManaged() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsVnetManaged", reflect.TypeOf((*MockRouteTableScope)(nil).IsVnetManaged)) } -// Location mocks base method. -func (m *MockRouteTableScope) 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 *MockRouteTableScopeMockRecorder) Location() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Location", reflect.TypeOf((*MockRouteTableScope)(nil).Location)) -} - -// NodeSubnets mocks base method. -func (m *MockRouteTableScope) NodeSubnets() []v1beta1.SubnetSpec { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "NodeSubnets") - ret0, _ := ret[0].([]v1beta1.SubnetSpec) - return ret0 -} - -// NodeSubnets indicates an expected call of NodeSubnets. -func (mr *MockRouteTableScopeMockRecorder) NodeSubnets() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NodeSubnets", reflect.TypeOf((*MockRouteTableScope)(nil).NodeSubnets)) -} - -// OutboundLBName mocks base method. -func (m *MockRouteTableScope) 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 *MockRouteTableScopeMockRecorder) OutboundLBName(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "OutboundLBName", reflect.TypeOf((*MockRouteTableScope)(nil).OutboundLBName), arg0) -} - -// OutboundPoolName mocks base method. -func (m *MockRouteTableScope) OutboundPoolName(arg0 string) string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "OutboundPoolName", arg0) - ret0, _ := ret[0].(string) - return ret0 -} - -// OutboundPoolName indicates an expected call of OutboundPoolName. -func (mr *MockRouteTableScopeMockRecorder) OutboundPoolName(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "OutboundPoolName", reflect.TypeOf((*MockRouteTableScope)(nil).OutboundPoolName), arg0) -} - -// ResourceGroup mocks base method. -func (m *MockRouteTableScope) ResourceGroup() string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ResourceGroup") - ret0, _ := ret[0].(string) - return ret0 -} - -// ResourceGroup indicates an expected call of ResourceGroup. -func (mr *MockRouteTableScopeMockRecorder) ResourceGroup() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResourceGroup", reflect.TypeOf((*MockRouteTableScope)(nil).ResourceGroup)) -} - // RouteTableSpecs mocks base method. -func (m *MockRouteTableScope) RouteTableSpecs() []azure.RouteTableSpec { +func (m *MockRouteTableScope) RouteTableSpecs() []azure.ResourceSpecGetter { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "RouteTableSpecs") - ret0, _ := ret[0].([]azure.RouteTableSpec) + ret0, _ := ret[0].([]azure.ResourceSpecGetter) return ret0 } @@ -402,44 +191,16 @@ func (mr *MockRouteTableScopeMockRecorder) RouteTableSpecs() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RouteTableSpecs", reflect.TypeOf((*MockRouteTableScope)(nil).RouteTableSpecs)) } -// SetSubnet mocks base method. -func (m *MockRouteTableScope) SetSubnet(arg0 v1beta1.SubnetSpec) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "SetSubnet", arg0) -} - -// SetSubnet indicates an expected call of SetSubnet. -func (mr *MockRouteTableScopeMockRecorder) SetSubnet(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetSubnet", reflect.TypeOf((*MockRouteTableScope)(nil).SetSubnet), arg0) -} - -// Subnet mocks base method. -func (m *MockRouteTableScope) Subnet(arg0 string) v1beta1.SubnetSpec { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Subnet", arg0) - ret0, _ := ret[0].(v1beta1.SubnetSpec) - return ret0 -} - -// Subnet indicates an expected call of Subnet. -func (mr *MockRouteTableScopeMockRecorder) Subnet(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Subnet", reflect.TypeOf((*MockRouteTableScope)(nil).Subnet), arg0) -} - -// Subnets mocks base method. -func (m *MockRouteTableScope) Subnets() v1beta1.Subnets { +// SetLongRunningOperationState mocks base method. +func (m *MockRouteTableScope) SetLongRunningOperationState(arg0 *v1beta1.Future) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Subnets") - ret0, _ := ret[0].(v1beta1.Subnets) - return ret0 + m.ctrl.Call(m, "SetLongRunningOperationState", arg0) } -// Subnets indicates an expected call of Subnets. -func (mr *MockRouteTableScopeMockRecorder) Subnets() *gomock.Call { +// SetLongRunningOperationState indicates an expected call of SetLongRunningOperationState. +func (mr *MockRouteTableScopeMockRecorder) SetLongRunningOperationState(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Subnets", reflect.TypeOf((*MockRouteTableScope)(nil).Subnets)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLongRunningOperationState", reflect.TypeOf((*MockRouteTableScope)(nil).SetLongRunningOperationState), arg0) } // SubscriptionID mocks base method. @@ -470,16 +231,38 @@ func (mr *MockRouteTableScopeMockRecorder) TenantID() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TenantID", reflect.TypeOf((*MockRouteTableScope)(nil).TenantID)) } -// Vnet mocks base method. -func (m *MockRouteTableScope) Vnet() *v1beta1.VnetSpec { +// UpdateDeleteStatus mocks base method. +func (m *MockRouteTableScope) UpdateDeleteStatus(arg0 v1beta10.ConditionType, arg1 string, arg2 error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Vnet") - ret0, _ := ret[0].(*v1beta1.VnetSpec) - return ret0 + m.ctrl.Call(m, "UpdateDeleteStatus", arg0, arg1, arg2) +} + +// UpdateDeleteStatus indicates an expected call of UpdateDeleteStatus. +func (mr *MockRouteTableScopeMockRecorder) UpdateDeleteStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateDeleteStatus", reflect.TypeOf((*MockRouteTableScope)(nil).UpdateDeleteStatus), arg0, arg1, arg2) +} + +// UpdatePatchStatus mocks base method. +func (m *MockRouteTableScope) UpdatePatchStatus(arg0 v1beta10.ConditionType, arg1 string, arg2 error) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "UpdatePatchStatus", arg0, arg1, arg2) +} + +// UpdatePatchStatus indicates an expected call of UpdatePatchStatus. +func (mr *MockRouteTableScopeMockRecorder) UpdatePatchStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePatchStatus", reflect.TypeOf((*MockRouteTableScope)(nil).UpdatePatchStatus), arg0, arg1, arg2) +} + +// UpdatePutStatus mocks base method. +func (m *MockRouteTableScope) UpdatePutStatus(arg0 v1beta10.ConditionType, arg1 string, arg2 error) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "UpdatePutStatus", arg0, arg1, arg2) } -// Vnet indicates an expected call of Vnet. -func (mr *MockRouteTableScopeMockRecorder) Vnet() *gomock.Call { +// UpdatePutStatus indicates an expected call of UpdatePutStatus. +func (mr *MockRouteTableScopeMockRecorder) UpdatePutStatus(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Vnet", reflect.TypeOf((*MockRouteTableScope)(nil).Vnet)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePutStatus", reflect.TypeOf((*MockRouteTableScope)(nil).UpdatePutStatus), arg0, arg1, arg2) } diff --git a/azure/services/routetables/routetables.go b/azure/services/routetables/routetables.go index 426bcf3ab4d..0a769052906 100644 --- a/azure/services/routetables/routetables.go +++ b/azure/services/routetables/routetables.go @@ -19,101 +19,93 @@ package routetables 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/pkg/errors" - + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" - "sigs.k8s.io/cluster-api-provider-azure/azure/scope" + "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 = "routetables" + // RouteTableScope defines the scope interface for route table service. type RouteTableScope interface { - azure.ClusterDescriber - azure.NetworkDescriber - RouteTableSpecs() []azure.RouteTableSpec + azure.Authorizer + azure.AsyncStatusUpdater + RouteTableSpecs() []azure.ResourceSpecGetter + IsVnetManaged() bool } // Service provides operations on azure resources. type Service struct { Scope RouteTableScope - client + async.Reconciler } // New creates a new service. -func New(scope *scope.ClusterScope) *Service { +func New(scope RouteTableScope) *Service { + client := newClient(scope) return &Service{ - Scope: scope, - client: newClient(scope), + Scope: scope, + Reconciler: async.New(scope, client, client), } } -// Reconcile gets/creates/updates a route table. +// Reconcile gets/creates/updates route tables. func (s *Service) Reconcile(ctx context.Context) error { ctx, log, done := tele.StartSpanWithLogger(ctx, "routetables.Service.Reconcile") defer done() - if !s.Scope.Vnet().IsManaged(s.Scope.ClusterName()) { - log.V(4).Info("Skipping route tables reconcile in custom vnet mode") - return nil - } + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() - for _, routeTableSpec := range s.Scope.RouteTableSpecs() { - existingRouteTable, err := s.Get(ctx, s.Scope.ResourceGroup(), routeTableSpec.Name) - if !azure.ResourceNotFound(err) { - if err != nil { - return errors.Wrapf(err, "failed to get route table %s in %s", routeTableSpec.Name, s.Scope.ResourceGroup()) - } - - // route table already exists - // currently don't support specifying your own routes via spec - routeTableSpec.Subnet.RouteTable.Name = to.String(existingRouteTable.Name) - routeTableSpec.Subnet.RouteTable.ID = to.String(existingRouteTable.ID) - s.Scope.SetSubnet(routeTableSpec.Subnet) - - continue - } + var resErr error - log.V(2).Info("creating Route Table", "route table", routeTableSpec.Name) - err = s.client.CreateOrUpdate( - ctx, - s.Scope.ResourceGroup(), - routeTableSpec.Name, - network.RouteTable{ - Location: to.StringPtr(s.Scope.Location()), - RouteTablePropertiesFormat: &network.RouteTablePropertiesFormat{}, - }, - ) - if err != nil { - return errors.Wrapf(err, "failed to create route table %s in resource group %s", routeTableSpec.Name, s.Scope.ResourceGroup()) + if !s.Scope.IsVnetManaged() { + log.V(4).Info("Skipping route tables reconcile in custom vnet mode") + } else { + // We go through the list of route tables to reconcile each one, independently of the result of the previous one. + // If multiple errors occur, we return the most pressing one. + // Order of precedence (highest -> lowest) is: error that is not an operationNotDoneError (ie. error creating) -> operationNotDoneError (ie. creating in progress) -> no error (ie. created) + for _, rtSpec := range s.Scope.RouteTableSpecs() { + if _, err := s.CreateResource(ctx, rtSpec, serviceName); err != nil { + if !azure.IsOperationNotDoneError(err) || resErr == nil { + resErr = err + } + } } - log.V(2).Info("successfully created route table", "route table", routeTableSpec.Name) } - return nil + s.Scope.UpdatePutStatus(infrav1.RouteTablesReadyCondition, serviceName, resErr) + return resErr } -// Delete deletes the route table with the provided name. +// Delete deletes route tables. func (s *Service) Delete(ctx context.Context) error { ctx, log, done := tele.StartSpanWithLogger(ctx, "routetables.Service.Delete") defer done() - if !s.Scope.Vnet().IsManaged(s.Scope.ClusterName()) { + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() + + // Only delete the route tables if their lifecycle is managed by this controller. + // route tables are managed if and only if the vnet is managed. + if !s.Scope.IsVnetManaged() { log.V(4).Info("Skipping route table deletion in custom vnet mode") return nil } - for _, routeTableSpec := range s.Scope.RouteTableSpecs() { - log.V(2).Info("deleting route table", "route table", routeTableSpec.Name) - err := s.client.Delete(ctx, s.Scope.ResourceGroup(), routeTableSpec.Name) - if err != nil && azure.ResourceNotFound(err) { - // already deleted - continue - } - if err != nil { - return errors.Wrapf(err, "failed to delete route table %s in resource group %s", routeTableSpec.Name, s.Scope.ResourceGroup()) - } - log.V(2).Info("successfully deleted route table", "route table", routeTableSpec.Name) + var result error + + // We go through the list of RouteTableSpecs to delete each one, independently of the result of the previous one. + // If multiple erros occur, we return the most pressing one + // order of precedence is: error deleting -> deleting in progress -> deleted (no error) + for _, rtSpec := range s.Scope.RouteTableSpecs() { + if err := s.DeleteResource(ctx, rtSpec, serviceName); err != nil { + if !azure.IsOperationNotDoneError(err) || result == nil { + result = err + } + } } - return nil + s.Scope.UpdateDeleteStatus(infrav1.RouteTablesReadyCondition, serviceName, result) + return result } diff --git a/azure/services/routetables/routetables_test.go b/azure/services/routetables/routetables_test.go index 32a41ae8b43..efeb403c269 100644 --- a/azure/services/routetables/routetables_test.go +++ b/azure/services/routetables/routetables_test.go @@ -18,200 +18,80 @@ package routetables import ( "context" - "net/http" + "errors" "testing" - "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" - "github.com/Azure/go-autorest/autorest" - "github.com/Azure/go-autorest/autorest/to" "github.com/golang/mock/gomock" . "github.com/onsi/gomega" - "k8s.io/client-go/kubernetes/scheme" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/async/mock_async" "sigs.k8s.io/cluster-api-provider-azure/azure/services/routetables/mock_routetables" gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" - clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" ) -func init() { - _ = clusterv1.AddToScheme(scheme.Scheme) -} +var ( + fakeRT = RouteTableSpec{ + Name: "test-rt-1", + ResourceGroup: "test-rg", + Location: "fake-location", + } + fakeRT2 = RouteTableSpec{ + Name: "test-rt-2", + ResourceGroup: "test-rg", + Location: "fake-location", + } + errFake = errors.New("this is an error") + notDoneError = azure.NewOperationNotDoneError(&infrav1.Future{}) +) func TestReconcileRouteTables(t *testing.T) { testcases := []struct { name string tags infrav1.Tags expectedError string - expect func(s *mock_routetables.MockRouteTableScopeMockRecorder, m *mock_routetables.MockclientMockRecorder) + expect func(s *mock_routetables.MockRouteTableScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) }{ { - name: "route tables in custom vnet mode", - tags: infrav1.Tags{ - "Name": "my-vnet", - "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "shared", - "sigs.k8s.io_cluster-api-provider-azure_role": "common", - }, + name: "create multiple route tables succeeds", expectedError: "", - expect: func(s *mock_routetables.MockRouteTableScopeMockRecorder, m *mock_routetables.MockclientMockRecorder) { - s.Vnet().Return(&infrav1.VnetSpec{ - ID: "1234", - Name: "my-vnet", - }) - s.ClusterName() + expect: func(s *mock_routetables.MockRouteTableScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.IsVnetManaged().Return(true) + s.RouteTableSpecs().Return([]azure.ResourceSpecGetter{&fakeRT, &fakeRT2}) + r.CreateResource(gomockinternal.AContext(), &fakeRT, serviceName).Return(nil, nil) + r.CreateResource(gomockinternal.AContext(), &fakeRT2, serviceName).Return(nil, nil) + s.UpdatePutStatus(infrav1.RouteTablesReadyCondition, serviceName, nil) }, }, { - name: "route table create successfully", - tags: infrav1.Tags{ - "Name": "my-vnet", - "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "owned", - "sigs.k8s.io_cluster-api-provider-azure_role": "common", - }, - expectedError: "", - expect: func(s *mock_routetables.MockRouteTableScopeMockRecorder, m *mock_routetables.MockclientMockRecorder) { - s.Vnet().Return(&infrav1.VnetSpec{ - Name: "my-vnet", - }) - s.ClusterName() - s.RouteTableSpecs().Return([]azure.RouteTableSpec{ - { - Name: "my-cp-routetable", - Subnet: infrav1.SubnetSpec{ - Name: "control-plane-subnet", - Role: infrav1.SubnetControlPlane, - }, - }, - { - Name: "my-node-routetable", - Subnet: infrav1.SubnetSpec{ - Name: "node-subnet", - Role: infrav1.SubnetNode, - }, - }, - }) - s.ControlPlaneRouteTable().AnyTimes().Return(infrav1.RouteTable{Name: "my-cp-routetable"}) - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Get(gomockinternal.AContext(), "my-rg", "my-cp-routetable").Return(network.RouteTable{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - s.Location().Return("westus") - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-cp-routetable", gomock.AssignableToTypeOf(network.RouteTable{})) - m.Get(gomockinternal.AContext(), "my-rg", "my-node-routetable").Return(network.RouteTable{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - s.Location().Return("westus") - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-node-routetable", gomock.AssignableToTypeOf(network.RouteTable{})) + name: "first route table create fails", + expectedError: errFake.Error(), + expect: func(s *mock_routetables.MockRouteTableScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.IsVnetManaged().Return(true) + s.RouteTableSpecs().Return([]azure.ResourceSpecGetter{&fakeRT, &fakeRT2}) + r.CreateResource(gomockinternal.AContext(), &fakeRT, serviceName).Return(nil, errFake) + r.CreateResource(gomockinternal.AContext(), &fakeRT2, serviceName).Return(nil, nil) + s.UpdatePutStatus(infrav1.RouteTablesReadyCondition, serviceName, errFake) }, }, { - name: "do not create route table if already exists", - tags: infrav1.Tags{ - "Name": "my-vnet", - "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "owned", - "sigs.k8s.io_cluster-api-provider-azure_role": "common", - }, - expectedError: "", - expect: func(s *mock_routetables.MockRouteTableScopeMockRecorder, m *mock_routetables.MockclientMockRecorder) { - s.Vnet().Return(&infrav1.VnetSpec{ - Name: "my-vnet", - }) - s.ClusterName() - s.RouteTableSpecs().AnyTimes().Return([]azure.RouteTableSpec{ - { - Name: "my-cp-routetable", - Subnet: infrav1.SubnetSpec{ - Name: "control-plane-subnet", - Role: infrav1.SubnetControlPlane, - }, - }, - { - Name: "my-node-routetable", - Subnet: infrav1.SubnetSpec{ - Name: "node-subnet", - Role: infrav1.SubnetNode, - }, - }, - }) - s.ControlPlaneSubnet().AnyTimes().Return(infrav1.SubnetSpec{Name: "control-plane-subnet", Role: infrav1.SubnetControlPlane}) - s.ControlPlaneRouteTable().AnyTimes().Return(infrav1.RouteTable{Name: "my-cp-routetable"}) - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Get(gomockinternal.AContext(), "my-rg", "my-cp-routetable").Return(network.RouteTable{ - Name: to.StringPtr("my-cp-routetable"), - ID: to.StringPtr("1"), - }, nil) - s.SetSubnet(infrav1.SubnetSpec{ - Name: "control-plane-subnet", - Role: infrav1.SubnetControlPlane, - RouteTable: infrav1.RouteTable{ - ID: "1", - Name: "my-cp-routetable", - }, - }).Times(1) - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Get(gomockinternal.AContext(), "my-rg", "my-node-routetable").Return(network.RouteTable{ - Name: to.StringPtr("my-node-routetable"), - ID: to.StringPtr("2"), - }, nil) - s.SetSubnet(infrav1.SubnetSpec{ - Name: "node-subnet", - Role: infrav1.SubnetNode, - RouteTable: infrav1.RouteTable{ - ID: "2", - Name: "my-node-routetable", - }, - }).Times(1) + name: "second route table create not done", + expectedError: errFake.Error(), + expect: func(s *mock_routetables.MockRouteTableScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.IsVnetManaged().Return(true) + s.RouteTableSpecs().Return([]azure.ResourceSpecGetter{&fakeRT, &fakeRT2}) + r.CreateResource(gomockinternal.AContext(), &fakeRT, serviceName).Return(nil, errFake) + r.CreateResource(gomockinternal.AContext(), &fakeRT2, serviceName).Return(nil, notDoneError) + s.UpdatePutStatus(infrav1.RouteTablesReadyCondition, serviceName, errFake) }, }, { - name: "fail when getting existing route table", - tags: infrav1.Tags{ - "Name": "my-vnet", - "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "owned", - "sigs.k8s.io_cluster-api-provider-azure_role": "common", - }, - expectedError: "failed to get route table my-cp-routetable in my-rg: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_routetables.MockRouteTableScopeMockRecorder, m *mock_routetables.MockclientMockRecorder) { - s.Vnet().Return(&infrav1.VnetSpec{ - Name: "my-vnet", - }) - s.ClusterName() - s.RouteTableSpecs().Return([]azure.RouteTableSpec{{ - Name: "my-cp-routetable", - Subnet: infrav1.SubnetSpec{ - Name: "control-plane-subnet", - Role: infrav1.SubnetControlPlane, - }, - }}) - s.ControlPlaneSubnet().AnyTimes().Return(infrav1.SubnetSpec{}) - s.ControlPlaneRouteTable().AnyTimes().Return(infrav1.RouteTable{Name: "my-routetable"}) - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Get(gomockinternal.AContext(), "my-rg", "my-cp-routetable").Return(network.RouteTable{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) - m.CreateOrUpdate(gomockinternal.AContext(), gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(network.RouteTable{})).Times(0) - }, - }, - { - name: "fail to create a route table", - tags: infrav1.Tags{ - "Name": "my-vnet", - "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "owned", - "sigs.k8s.io_cluster-api-provider-azure_role": "common", - }, - expectedError: "failed to create route table my-cp-routetable in resource group my-rg: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_routetables.MockRouteTableScopeMockRecorder, m *mock_routetables.MockclientMockRecorder) { - s.Vnet().Return(&infrav1.VnetSpec{ - Name: "my-vnet", - }) - s.ClusterName() - s.RouteTableSpecs().Return([]azure.RouteTableSpec{{ - Name: "my-cp-routetable", - Subnet: infrav1.SubnetSpec{ - Name: "control-plane-subnet", - Role: infrav1.SubnetControlPlane, - }, - }}) - s.ControlPlaneSubnet().AnyTimes().Return(infrav1.SubnetSpec{}) - s.ControlPlaneRouteTable().AnyTimes().Return(infrav1.RouteTable{Name: "my-cp-routetable"}) - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Get(gomockinternal.AContext(), "my-rg", "my-cp-routetable").Return(network.RouteTable{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - s.Location().Return("westus") - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-cp-routetable", gomock.AssignableToTypeOf(network.RouteTable{})).Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) + name: "vnet is not managed", + expectedError: "", + expect: func(s *mock_routetables.MockRouteTableScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.IsVnetManaged().Return(false) + s.UpdatePutStatus(infrav1.RouteTablesReadyCondition, serviceName, nil) }, }, } @@ -224,13 +104,13 @@ func TestReconcileRouteTables(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() scopeMock := mock_routetables.NewMockRouteTableScope(mockCtrl) - clientMock := mock_routetables.NewMockclient(mockCtrl) + reconcilerMock := mock_async.NewMockReconciler(mockCtrl) - tc.expect(scopeMock.EXPECT(), clientMock.EXPECT()) + tc.expect(scopeMock.EXPECT(), reconcilerMock.EXPECT()) s := &Service{ - Scope: scopeMock, - client: clientMock, + Scope: scopeMock, + Reconciler: reconcilerMock, } err := s.Reconcile(context.TODO()) @@ -249,119 +129,46 @@ func TestDeleteRouteTable(t *testing.T) { name string tags infrav1.Tags expectedError string - expect func(s *mock_routetables.MockRouteTableScopeMockRecorder, m *mock_routetables.MockclientMockRecorder) + expect func(s *mock_routetables.MockRouteTableScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) }{ { - name: "route tables in custom vnet mode", - tags: infrav1.Tags{ - "Name": "my-vnet", - "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "shared", - "sigs.k8s.io_cluster-api-provider-azure_role": "common", - }, + name: "delete multiple route tables succeeds", expectedError: "", - expect: func(s *mock_routetables.MockRouteTableScopeMockRecorder, m *mock_routetables.MockclientMockRecorder) { - s.Vnet().Return(&infrav1.VnetSpec{ - ID: "1234", - Name: "my-vnet", - }) - s.ClusterName() + expect: func(s *mock_routetables.MockRouteTableScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.IsVnetManaged().Return(true) + s.RouteTableSpecs().Return([]azure.ResourceSpecGetter{&fakeRT, &fakeRT2}) + r.DeleteResource(gomockinternal.AContext(), &fakeRT, serviceName).Return(nil) + r.DeleteResource(gomockinternal.AContext(), &fakeRT2, serviceName).Return(nil) + s.UpdateDeleteStatus(infrav1.RouteTablesReadyCondition, serviceName, nil) }, }, { - name: "route table deleted successfully", - tags: infrav1.Tags{ - "Name": "my-vnet", - "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "owned", - "sigs.k8s.io_cluster-api-provider-azure_role": "common", - }, - expectedError: "", - expect: func(s *mock_routetables.MockRouteTableScopeMockRecorder, m *mock_routetables.MockclientMockRecorder) { - s.Vnet().Return(&infrav1.VnetSpec{ - Name: "my-vnet", - }) - s.ClusterName() - s.RouteTableSpecs().Return([]azure.RouteTableSpec{ - { - Name: "my-cp-routetable", - Subnet: infrav1.SubnetSpec{ - Name: "control-plane-subnet", - Role: infrav1.SubnetControlPlane, - }, - }, - { - Name: "my-node-routetable", - Subnet: infrav1.SubnetSpec{ - Name: "node-subnet", - Role: infrav1.SubnetNode, - }, - }, - }) - s.ControlPlaneRouteTable().AnyTimes().Return(infrav1.RouteTable{Name: "my-cp-routetable"}) - s.ResourceGroup().Return("my-rg") - m.Delete(gomockinternal.AContext(), "my-rg", "my-cp-routetable") - s.ResourceGroup().Return("my-rg") - m.Delete(gomockinternal.AContext(), "my-rg", "my-node-routetable") + name: "first route table delete fails", + expectedError: errFake.Error(), + expect: func(s *mock_routetables.MockRouteTableScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.IsVnetManaged().Return(true) + s.RouteTableSpecs().Return([]azure.ResourceSpecGetter{&fakeRT, &fakeRT2}) + r.DeleteResource(gomockinternal.AContext(), &fakeRT, serviceName).Return(errFake) + r.DeleteResource(gomockinternal.AContext(), &fakeRT2, serviceName).Return(nil) + s.UpdateDeleteStatus(infrav1.RouteTablesReadyCondition, serviceName, errFake) }, }, { - name: "route table already deleted", - tags: infrav1.Tags{ - "Name": "my-vnet", - "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "owned", - "sigs.k8s.io_cluster-api-provider-azure_role": "common", - }, - expectedError: "", - expect: func(s *mock_routetables.MockRouteTableScopeMockRecorder, m *mock_routetables.MockclientMockRecorder) { - s.Vnet().Return(&infrav1.VnetSpec{ - Name: "my-vnet", - }) - s.ClusterName() - s.RouteTableSpecs().Return([]azure.RouteTableSpec{ - { - Name: "my-cp-routetable", - Subnet: infrav1.SubnetSpec{ - Name: "control-plane-subnet", - Role: infrav1.SubnetControlPlane, - }, - }, - { - Name: "my-node-routetable", - Subnet: infrav1.SubnetSpec{ - Name: "node-subnet", - Role: infrav1.SubnetNode, - }, - }, - }) - s.ControlPlaneRouteTable().AnyTimes().Return(infrav1.RouteTable{Name: "my-cp-routetable"}) - s.ResourceGroup().Return("my-rg") - m.Delete(gomockinternal.AContext(), "my-rg", "my-cp-routetable").Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not Found")) - s.ResourceGroup().Return("my-rg") - m.Delete(gomockinternal.AContext(), "my-rg", "my-node-routetable").Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not Found")) + name: "second route table delete not done", + expectedError: errFake.Error(), + expect: func(s *mock_routetables.MockRouteTableScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.IsVnetManaged().Return(true) + s.RouteTableSpecs().Return([]azure.ResourceSpecGetter{&fakeRT, &fakeRT2}) + r.DeleteResource(gomockinternal.AContext(), &fakeRT, serviceName).Return(errFake) + r.DeleteResource(gomockinternal.AContext(), &fakeRT2, serviceName).Return(notDoneError) + s.UpdateDeleteStatus(infrav1.RouteTablesReadyCondition, serviceName, errFake) }, }, { - name: "route table deletion fails", - tags: infrav1.Tags{ - "Name": "my-vnet", - "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "owned", - "sigs.k8s.io_cluster-api-provider-azure_role": "common", - }, - expectedError: "failed to delete route table my-cp-routetable in resource group my-rg: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_routetables.MockRouteTableScopeMockRecorder, m *mock_routetables.MockclientMockRecorder) { - s.Vnet().Return(&infrav1.VnetSpec{ - Name: "my-vnet", - }) - s.ClusterName() - s.RouteTableSpecs().Return([]azure.RouteTableSpec{{ - Name: "my-cp-routetable", - Subnet: infrav1.SubnetSpec{ - Name: "control-plane-subnet", - Role: infrav1.SubnetControlPlane, - }, - }}) - s.ControlPlaneRouteTable().AnyTimes().Return(infrav1.RouteTable{Name: "my-cp-routetable"}) - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Delete(gomockinternal.AContext(), "my-rg", "my-cp-routetable").Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) + name: "vnet is not managed", + expectedError: "", + expect: func(s *mock_routetables.MockRouteTableScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.IsVnetManaged().Return(false) }, }, } @@ -374,13 +181,13 @@ func TestDeleteRouteTable(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() scopeMock := mock_routetables.NewMockRouteTableScope(mockCtrl) - clientMock := mock_routetables.NewMockclient(mockCtrl) + reconcilerMock := mock_async.NewMockReconciler(mockCtrl) - tc.expect(scopeMock.EXPECT(), clientMock.EXPECT()) + tc.expect(scopeMock.EXPECT(), reconcilerMock.EXPECT()) s := &Service{ - Scope: scopeMock, - client: clientMock, + Scope: scopeMock, + Reconciler: reconcilerMock, } err := s.Delete(context.TODO()) diff --git a/azure/services/routetables/spec.go b/azure/services/routetables/spec.go new file mode 100644 index 00000000000..1a96099e026 --- /dev/null +++ b/azure/services/routetables/spec.go @@ -0,0 +1,61 @@ +/* +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 routetables + +import ( + "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" +) + +// RouteTableSpec defines the specification for a route table. +type RouteTableSpec struct { + Name string + ResourceGroup string + Location string +} + +// ResourceName returns the name of the route table. +func (s *RouteTableSpec) ResourceName() string { + return s.Name +} + +// ResourceGroupName returns the name of the resource group. +func (s *RouteTableSpec) ResourceGroupName() string { + return s.ResourceGroup +} + +// OwnerResourceName is a no-op for route tables. +func (s *RouteTableSpec) OwnerResourceName() string { + return "" +} + +// Parameters returns the parameters for the route table. +func (s *RouteTableSpec) Parameters(existing interface{}) (params interface{}, err error) { + if existing != nil { + if _, ok := existing.(network.RouteTable); !ok { + return nil, errors.Errorf("%T is not a network.RouteTable", existing) + } + // route table already exists + // currently don't support specifying your own routes via spec. + return nil, nil + } + return network.RouteTable{ + Location: to.StringPtr(s.Location), + RouteTablePropertiesFormat: &network.RouteTablePropertiesFormat{}, + }, nil +} diff --git a/azure/services/virtualmachines/client.go b/azure/services/virtualmachines/client.go index f4f77618c0f..a7ba8c7f8ec 100644 --- a/azure/services/virtualmachines/client.go +++ b/azure/services/virtualmachines/client.go @@ -36,10 +36,10 @@ import ( type ( Client interface { Get(context.Context, azure.ResourceSpecGetter) (interface{}, error) - CreateOrUpdateAsync(context.Context, azure.ResourceSpecGetter, interface{}) (interface{}, azureautorest.FutureAPI, error) - DeleteAsync(context.Context, azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) - IsDone(context.Context, azureautorest.FutureAPI) (bool, error) - Result(context.Context, azureautorest.FutureAPI, string) (interface{}, error) + CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (result interface{}, future azureautorest.FutureAPI, err error) + DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (future azureautorest.FutureAPI, err error) + IsDone(ctx context.Context, future azureautorest.FutureAPI) (isDone bool, err error) + Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error) } // AzureClient contains the Azure go-sdk Client. @@ -64,7 +64,7 @@ func newVirtualMachinesClient(subscriptionID string, baseURI string, authorizer } // Get retrieves information about the model view or the instance view of a virtual machine. -func (ac *AzureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) (interface{}, error) { +func (ac *AzureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) (result interface{}, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualmachines.AzureClient.Get") defer done() @@ -74,7 +74,7 @@ func (ac *AzureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) ( // CreateOrUpdateAsync creates or updates a virtual machine asynchronously. // It sends a PUT request to Azure and if accepted without error, the func will return a Future which can be used to track the ongoing // progress of the operation. -func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (interface{}, azureautorest.FutureAPI, error) { +func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (result interface{}, future azureautorest.FutureAPI, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualmachines.AzureClient.CreateOrUpdate") defer done() @@ -83,7 +83,7 @@ func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.Resou return nil, nil, errors.Errorf("%T is not a compute.VirtualMachine", parameters) } - future, err := ac.virtualmachines.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.ResourceName(), vm) + createFuture, err := ac.virtualmachines.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.ResourceName(), vm) if err != nil { return nil, nil, err } @@ -91,13 +91,13 @@ func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.Resou ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) defer cancel() - err = future.WaitForCompletionRef(ctx, ac.virtualmachines.Client) + err = createFuture.WaitForCompletionRef(ctx, ac.virtualmachines.Client) if err != nil { // if an error occurs, return the future. // this means the long-running operation didn't finish in the specified timeout. - return nil, &future, err + return nil, &createFuture, err } - result, err := future.Result(ac.virtualmachines) + result, err = createFuture.Result(ac.virtualmachines) // if the operation completed, return a nil future return result, nil, err } @@ -105,13 +105,13 @@ func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.Resou // DeleteAsync deletes a virtual machine 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) { +func (ac *AzureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (future azureautorest.FutureAPI, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualmachines.AzureClient.Delete") defer done() // TODO: pass variable to force the deletion or not // now we are not forcing. - future, err := ac.virtualmachines.Delete(ctx, spec.ResourceGroupName(), spec.ResourceName(), to.BoolPtr(false)) + deleteFuture, err := ac.virtualmachines.Delete(ctx, spec.ResourceGroupName(), spec.ResourceName(), to.BoolPtr(false)) if err != nil { return nil, err } @@ -119,23 +119,23 @@ func (ac *AzureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecG ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) defer cancel() - err = future.WaitForCompletionRef(ctx, ac.virtualmachines.Client) + err = deleteFuture.WaitForCompletionRef(ctx, ac.virtualmachines.Client) if err != nil { // if an error occurs, return the future. // this means the long-running operation didn't finish in the specified timeout. - return &future, err + return &deleteFuture, err } - _, err = future.Result(ac.virtualmachines) + _, err = deleteFuture.Result(ac.virtualmachines) // 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) { +func (ac *AzureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (isDone bool, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualmachines.AzureClient.IsDone") defer done() - isDone, err := future.DoneWithContext(ctx, ac.virtualmachines) + isDone, err = future.DoneWithContext(ctx, ac.virtualmachines) if err != nil { return false, errors.Wrap(err, "failed checking if the operation was complete") } @@ -144,37 +144,42 @@ func (ac *AzureClient) IsDone(ctx context.Context, future azureautorest.FutureAP } // Result fetches the result of a long-running operation future. -func (ac *AzureClient) Result(ctx context.Context, futureData azureautorest.FutureAPI, futureType string) (interface{}, error) { +func (ac *AzureClient) Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error) { _, _, done := tele.StartSpanWithLogger(ctx, "virtualmachines.AzureClient.Result") defer done() - if futureData == nil { + if future == nil { return nil, errors.Errorf("cannot get result from nil future") } - var result func(client compute.VirtualMachinesClient) (VM compute.VirtualMachine, err error) switch futureType { case infrav1.PatchFuture: - var future *compute.VirtualMachinesUpdateFuture - jsonData, err := futureData.MarshalJSON() + // Marshal and Unmarshal the future to put it into the correct future type so we can access the Result function. + // Unfortunately the FutureAPI can't be casted directly to VirtualMachinesUpdateFuture because it is a azureautorest.Future, which doesn't implement the Result function. See PR #1686 for discussion on alternatives. + // It was converted back to a generic azureautorest.Future from the CAPZ infrav1.Future type stored in Status: https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/main/azure/converters/futures.go#L49. + var updateFuture *compute.VirtualMachinesUpdateFuture + jsonData, err := future.MarshalJSON() if err != nil { return nil, errors.Wrap(err, "failed to marshal future") } - if err := json.Unmarshal(jsonData, &future); err != nil { + if err := json.Unmarshal(jsonData, &updateFuture); err != nil { return nil, errors.Wrap(err, "failed to unmarshal future data") } - result = (*future).Result + return (*updateFuture).Result(ac.virtualmachines) case infrav1.PutFuture: - var future *compute.VirtualMachinesCreateOrUpdateFuture - jsonData, err := futureData.MarshalJSON() + // Marshal and Unmarshal the future to put it into the correct future type so we can access the Result function. + // Unfortunately the FutureAPI can't be casted directly to VirtualMachinesCreateOrUpdateFuture because it is a azureautorest.Future, which doesn't implement the Result function. See PR #1686 for discussion on alternatives. + // It was converted back to a generic azureautorest.Future from the CAPZ infrav1.Future type stored in Status: https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/main/azure/converters/futures.go#L49. + var createFuture *compute.VirtualMachinesCreateOrUpdateFuture + jsonData, err := future.MarshalJSON() if err != nil { return nil, errors.Wrap(err, "failed to marshal future") } - if err := json.Unmarshal(jsonData, &future); err != nil { + if err := json.Unmarshal(jsonData, &createFuture); err != nil { return nil, errors.Wrap(err, "failed to unmarshal future data") } - result = (*future).Result + return (*createFuture).Result(ac.virtualmachines) case infrav1.DeleteFuture: // Delete does not return a result VM. @@ -183,6 +188,4 @@ func (ac *AzureClient) Result(ctx context.Context, futureData azureautorest.Futu default: return nil, errors.Errorf("unknown future type %q", futureType) } - - return result(ac.virtualmachines) } diff --git a/azure/services/virtualmachines/mock_virtualmachines/client_mock.go b/azure/services/virtualmachines/mock_virtualmachines/client_mock.go index 8236de64ea2..86be1de0949 100644 --- a/azure/services/virtualmachines/mock_virtualmachines/client_mock.go +++ b/azure/services/virtualmachines/mock_virtualmachines/client_mock.go @@ -53,9 +53,9 @@ func (m *MockClient) EXPECT() *MockClientMockRecorder { } // CreateOrUpdateAsync mocks base method. -func (m *MockClient) CreateOrUpdateAsync(arg0 context.Context, arg1 azure0.ResourceSpecGetter, arg2 interface{}) (interface{}, azure.FutureAPI, error) { +func (m *MockClient) CreateOrUpdateAsync(ctx context.Context, spec azure0.ResourceSpecGetter, parameters interface{}) (interface{}, azure.FutureAPI, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateOrUpdateAsync", arg0, arg1, arg2) + ret := m.ctrl.Call(m, "CreateOrUpdateAsync", ctx, spec, parameters) ret0, _ := ret[0].(interface{}) ret1, _ := ret[1].(azure.FutureAPI) ret2, _ := ret[2].(error) @@ -63,24 +63,24 @@ func (m *MockClient) CreateOrUpdateAsync(arg0 context.Context, arg1 azure0.Resou } // CreateOrUpdateAsync indicates an expected call of CreateOrUpdateAsync. -func (mr *MockClientMockRecorder) CreateOrUpdateAsync(arg0, arg1, arg2 interface{}) *gomock.Call { +func (mr *MockClientMockRecorder) CreateOrUpdateAsync(ctx, spec, parameters interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*MockClient)(nil).CreateOrUpdateAsync), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*MockClient)(nil).CreateOrUpdateAsync), ctx, spec, parameters) } // DeleteAsync mocks base method. -func (m *MockClient) DeleteAsync(arg0 context.Context, arg1 azure0.ResourceSpecGetter) (azure.FutureAPI, error) { +func (m *MockClient) DeleteAsync(ctx context.Context, spec azure0.ResourceSpecGetter) (azure.FutureAPI, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteAsync", arg0, arg1) + ret := m.ctrl.Call(m, "DeleteAsync", ctx, spec) ret0, _ := ret[0].(azure.FutureAPI) ret1, _ := ret[1].(error) return ret0, ret1 } // DeleteAsync indicates an expected call of DeleteAsync. -func (mr *MockClientMockRecorder) DeleteAsync(arg0, arg1 interface{}) *gomock.Call { +func (mr *MockClientMockRecorder) DeleteAsync(ctx, spec interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAsync", reflect.TypeOf((*MockClient)(nil).DeleteAsync), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAsync", reflect.TypeOf((*MockClient)(nil).DeleteAsync), ctx, spec) } // Get mocks base method. @@ -99,31 +99,31 @@ func (mr *MockClientMockRecorder) Get(arg0, arg1 interface{}) *gomock.Call { } // IsDone mocks base method. -func (m *MockClient) IsDone(arg0 context.Context, arg1 azure.FutureAPI) (bool, error) { +func (m *MockClient) IsDone(ctx context.Context, future azure.FutureAPI) (bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsDone", arg0, arg1) + ret := m.ctrl.Call(m, "IsDone", ctx, future) ret0, _ := ret[0].(bool) ret1, _ := ret[1].(error) return ret0, ret1 } // IsDone indicates an expected call of IsDone. -func (mr *MockClientMockRecorder) IsDone(arg0, arg1 interface{}) *gomock.Call { +func (mr *MockClientMockRecorder) IsDone(ctx, future interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDone", reflect.TypeOf((*MockClient)(nil).IsDone), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDone", reflect.TypeOf((*MockClient)(nil).IsDone), ctx, future) } // Result mocks base method. -func (m *MockClient) Result(arg0 context.Context, arg1 azure.FutureAPI, arg2 string) (interface{}, error) { +func (m *MockClient) Result(ctx context.Context, future azure.FutureAPI, futureType string) (interface{}, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Result", arg0, arg1, arg2) + ret := m.ctrl.Call(m, "Result", ctx, future, futureType) ret0, _ := ret[0].(interface{}) ret1, _ := ret[1].(error) return ret0, ret1 } // Result indicates an expected call of Result. -func (mr *MockClientMockRecorder) Result(arg0, arg1, arg2 interface{}) *gomock.Call { +func (mr *MockClientMockRecorder) Result(ctx, future, futureType interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Result", reflect.TypeOf((*MockClient)(nil).Result), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Result", reflect.TypeOf((*MockClient)(nil).Result), ctx, future, futureType) } diff --git a/azure/services/virtualmachines/spec.go b/azure/services/virtualmachines/spec.go index 11d523245d8..d191d196d1b 100644 --- a/azure/services/virtualmachines/spec.go +++ b/azure/services/virtualmachines/spec.go @@ -73,7 +73,7 @@ func (s *VMSpec) OwnerResourceName() string { } // Parameters returns the parameters for the virtual machine. -func (s *VMSpec) Parameters(existing interface{}) (interface{}, error) { +func (s *VMSpec) Parameters(existing interface{}) (params interface{}, err error) { if existing != nil { if _, ok := existing.(compute.VirtualMachine); !ok { return nil, errors.Errorf("%T is not a compute.VirtualMachine", existing) diff --git a/azure/services/vnetpeerings/client.go b/azure/services/vnetpeerings/client.go index b16e89faea9..fc55d11f7f8 100644 --- a/azure/services/vnetpeerings/client.go +++ b/azure/services/vnetpeerings/client.go @@ -50,7 +50,7 @@ func newPeeringsClient(subscriptionID string, baseURI string, authorizer autores } // Get gets the specified virtual network peering by the peering name, virtual network, and resource group. -func (ac *AzureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) (interface{}, error) { +func (ac *AzureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) (result interface{}, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "vnetpeerings.AzureClient.Get") defer done() @@ -60,7 +60,7 @@ func (ac *AzureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) ( // CreateOrUpdateAsync creates or updates a virtual network peering asynchronously. // It sends a PUT request to Azure and if accepted without error, the func will return a Future which can be used to track the ongoing // progress of the operation. -func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (interface{}, azureautorest.FutureAPI, error) { +func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (result interface{}, future azureautorest.FutureAPI, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "vnetpeerings.AzureClient.CreateOrUpdateAsync") defer done() @@ -69,7 +69,7 @@ func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.Resou return nil, nil, errors.Errorf("%T is not a network.VirtualNetworkPeering", parameters) } - future, err := ac.peerings.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.OwnerResourceName(), spec.ResourceName(), peering, network.SyncRemoteAddressSpaceTrue) + createFuture, err := ac.peerings.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.OwnerResourceName(), spec.ResourceName(), peering, network.SyncRemoteAddressSpaceTrue) if err != nil { return nil, nil, err } @@ -77,14 +77,14 @@ func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.Resou ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) defer cancel() - err = future.WaitForCompletionRef(ctx, ac.peerings.Client) + err = createFuture.WaitForCompletionRef(ctx, ac.peerings.Client) if err != nil { // if an error occurs, return the future. // this means the long-running operation didn't finish in the specified timeout. - return nil, &future, err + return nil, &createFuture, err } - result, err := future.Result(ac.peerings) + result, err = createFuture.Result(ac.peerings) // if the operation completed, return a nil future return result, nil, err } @@ -92,11 +92,11 @@ func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.Resou // DeleteAsync deletes a virtual network peering 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) { +func (ac *AzureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (future azureautorest.FutureAPI, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "vnetpeerings.AzureClient.Delete") defer done() - future, err := ac.peerings.Delete(ctx, spec.ResourceGroupName(), spec.OwnerResourceName(), spec.ResourceName()) + deleteFuture, err := ac.peerings.Delete(ctx, spec.ResourceGroupName(), spec.OwnerResourceName(), spec.ResourceName()) if err != nil { return nil, err } @@ -104,23 +104,23 @@ func (ac *AzureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecG ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) defer cancel() - err = future.WaitForCompletionRef(ctx, ac.peerings.Client) + err = deleteFuture.WaitForCompletionRef(ctx, ac.peerings.Client) if err != nil { // if an error occurs, return the future. // this means the long-running operation didn't finish in the specified timeout. - return &future, err + return &deleteFuture, err } - _, err = future.Result(ac.peerings) + _, err = deleteFuture.Result(ac.peerings) // 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) { +func (ac *AzureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (isDone bool, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "vnetpeerings.AzureClient.IsDone") defer done() - isDone, err := future.DoneWithContext(ctx, ac.peerings) + isDone, err = future.DoneWithContext(ctx, ac.peerings) if err != nil { return false, errors.Wrap(err, "failed checking if the operation was complete") } @@ -129,26 +129,28 @@ func (ac *AzureClient) IsDone(ctx context.Context, future azureautorest.FutureAP } // Result fetches the result of a long-running operation future. -func (ac *AzureClient) Result(ctx context.Context, futureData azureautorest.FutureAPI, futureType string) (interface{}, error) { +func (ac *AzureClient) Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error) { _, _, done := tele.StartSpanWithLogger(ctx, "vnetpeerings.AzureClient.Result") defer done() - if futureData == nil { + if future == nil { return nil, errors.Errorf("cannot get result from nil future") } - var result func(client network.VirtualNetworkPeeringsClient) (peering network.VirtualNetworkPeering, err error) switch futureType { case infrav1.PutFuture: - var future *network.VirtualNetworkPeeringsCreateOrUpdateFuture - jsonData, err := futureData.MarshalJSON() + // Marshal and Unmarshal the future to put it into the correct future type so we can access the Result function. + // Unfortunately the FutureAPI can't be casted directly to VirtualNetworkPeeringsCreateOrUpdateFuture because it is a azureautorest.Future, which doesn't implement the Result function. See PR #1686 for discussion on alternatives. + // It was converted back to a generic azureautorest.Future from the CAPZ infrav1.Future type stored in Status: https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/main/azure/converters/futures.go#L49. + var createFuture *network.VirtualNetworkPeeringsCreateOrUpdateFuture + jsonData, err := future.MarshalJSON() if err != nil { return nil, errors.Wrap(err, "failed to marshal future") } - if err := json.Unmarshal(jsonData, &future); err != nil { + if err := json.Unmarshal(jsonData, &createFuture); err != nil { return nil, errors.Wrap(err, "failed to unmarshal future data") } - result = (*future).Result + return (*createFuture).Result(ac.peerings) case infrav1.DeleteFuture: // Delete does not return a result virtual network peering @@ -157,6 +159,4 @@ func (ac *AzureClient) Result(ctx context.Context, futureData azureautorest.Futu default: return nil, errors.Errorf("unknown future type %q", futureType) } - - return result(ac.peerings) } diff --git a/azure/services/vnetpeerings/spec.go b/azure/services/vnetpeerings/spec.go index 2fcc0af3f51..2c3fb8f3f89 100644 --- a/azure/services/vnetpeerings/spec.go +++ b/azure/services/vnetpeerings/spec.go @@ -49,7 +49,7 @@ func (s *VnetPeeringSpec) OwnerResourceName() string { } // Parameters returns the parameters for the virtual network peering. -func (s *VnetPeeringSpec) Parameters(existing interface{}) (interface{}, error) { +func (s *VnetPeeringSpec) Parameters(existing interface{}) (params interface{}, err error) { if existing != nil { if _, ok := existing.(network.VirtualNetworkPeering); !ok { return nil, errors.Errorf("%T is not a network.VnetPeering", existing) diff --git a/azure/services/vnetpeerings/vnetpeerings.go b/azure/services/vnetpeerings/vnetpeerings.go index 6ff3bd95c56..1012097d45d 100644 --- a/azure/services/vnetpeerings/vnetpeerings.go +++ b/azure/services/vnetpeerings/vnetpeerings.go @@ -59,8 +59,8 @@ func (s *Service) Reconcile(ctx context.Context) error { defer cancel() // We go through the list of VnetPeeringSpecs to reconcile each one, independently of the result of the previous one. - // If multiple errors occur, we return the most pressing one - // order of precedence is: error creating -> creating in progress -> created (no error) + // If multiple errors occur, we return the most pressing one. + // Order of precedence (highest -> lowest) is: error that is not an operationNotDoneError (i.e. error creating) -> operationNotDoneError (i.e. creating in progress) -> no error (i.e. created) var result error for _, peeringSpec := range s.Scope.VnetPeeringSpecs() { if _, err := s.CreateResource(ctx, peeringSpec, serviceName); err != nil { @@ -85,8 +85,8 @@ func (s *Service) Delete(ctx context.Context) error { var result error // We go through the list of VnetPeeringSpecs to delete each one, independently of the result of the previous one. - // If multiple errors occur, we return the most pressing one - // order of precedence is: error deleting -> deleting in progress -> deleted (no error) + // If multiple errors occur, we return the most pressing one. + // Order of precedence (highest -> lowest) is: error that is not an operationNotDoneError (i.e. error deleting) -> operationNotDoneError (i.e. deleting in progress) -> no error (i.e. deleted) for _, peeringSpec := range s.Scope.VnetPeeringSpecs() { if err := s.DeleteResource(ctx, peeringSpec, serviceName); err != nil { if !azure.IsOperationNotDoneError(err) || result == nil { diff --git a/azure/types.go b/azure/types.go index 351ffc24841..b62a6db74f3 100644 --- a/azure/types.go +++ b/azure/types.go @@ -64,15 +64,6 @@ type LBSpec struct { IdleTimeoutInMinutes *int32 } -// RouteTableRole defines the unique role of a route table. -type RouteTableRole string - -// RouteTableSpec defines the specification for a Route Table. -type RouteTableSpec struct { - Name string - Subnet infrav1.SubnetSpec -} - // InboundNatSpec defines the specification for an inbound NAT rule. type InboundNatSpec struct { Name string