Skip to content

Make route tables reconcile/delete async #1686

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
k8s-ci-robot merged 2 commits into from
Jan 11, 2022
Merged

Make route tables reconcile/delete async #1686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a5c20a
Make route tables reconcile/delete async
Sep 14, 2021
229f425
Use named returns in async clients
Jan 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion azure/interfaces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Copy Markdown
Contributor

@devigned devigned Jan 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

func Parameters[T any](existing T) (params T, err error)

Will be kind of nice. No action to take from this, just commenting.

}
17 changes: 12 additions & 5 deletions azure/scope/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions azure/services/async/async.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -79,15 +79,15 @@ 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)
}
return result, err
}

// 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()

Expand Down Expand Up @@ -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()

Expand Down
14 changes: 7 additions & 7 deletions azure/services/async/interfaces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
8 changes: 4 additions & 4 deletions azure/services/async/mock_async/async_mock.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 16 additions & 16 deletions azure/services/availabilitysets/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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()

Expand All @@ -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()

Expand All @@ -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
Expand All @@ -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
}
32 changes: 16 additions & 16 deletions azure/services/availabilitysets/mock_availabilitysets/client_mock.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion azure/services/availabilitysets/spec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading