Skip to content
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.

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

Make group delete async #1667

Merged
merged 2 commits into from
Sep 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 0 deletions api/v1alpha4/conditions_consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4"

// AzureCluster Conditions and Reasons.
const (
// NetworkInfrastructureReadyCondition reports of current status of cluster infrastructure.
NetworkInfrastructureReadyCondition clusterv1.ConditionType = "NetworkInfrastructureReady"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

re-added so that the cluster doesn't show as Ready until all services are done reconciling (might remove once each service has its own condition in place to use as summary)

// NamespaceNotAllowedByIdentity used to indicate cluster in a namespace not allowed by identity.
NamespaceNotAllowedByIdentity = "NamespaceNotAllowedByIdentity"
)
Expand Down
20 changes: 19 additions & 1 deletion azure/scope/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (

infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4"
"sigs.k8s.io/cluster-api-provider-azure/azure"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/groups"
"sigs.k8s.io/cluster-api-provider-azure/util/futures"
)

Expand Down Expand Up @@ -286,6 +287,16 @@ func (s *ClusterScope) SubnetSpecs() []azure.SubnetSpec {
return subnetSpecs
}

// GroupSpec returns the resource group spec.
func (s *ClusterScope) GroupSpec() azure.ResourceSpecGetter {
return &groups.GroupSpec{
Name: s.ResourceGroup(),
Location: s.Location(),
ClusterName: s.ClusterName(),
AdditionalTags: s.AdditionalTags(),
}
}

// VNetSpec returns the virtual network spec.
func (s *ClusterScope) VNetSpec() azure.VNetSpec {
return azure.VNetSpec{
Expand Down Expand Up @@ -539,13 +550,20 @@ func (s *ClusterScope) ListOptionsLabelSelector() client.ListOption {

// PatchObject persists the cluster configuration and status.
func (s *ClusterScope) PatchObject(ctx context.Context) error {
conditions.SetSummary(s.AzureCluster)
conditions.SetSummary(s.AzureCluster,
conditions.WithConditions(
infrav1.ResourceGroupReadyCondition,
infrav1.NetworkInfrastructureReadyCondition,
),
)

return s.patchHelper.Patch(
ctx,
s.AzureCluster,
patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{
clusterv1.ReadyCondition,
infrav1.ResourceGroupReadyCondition,
infrav1.NetworkInfrastructureReadyCondition,
}})
}

Expand Down
11 changes: 11 additions & 0 deletions azure/scope/managedcontrolplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (

infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4"
"sigs.k8s.io/cluster-api-provider-azure/azure"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/groups"
infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1alpha4"
"sigs.k8s.io/cluster-api-provider-azure/util/futures"
)
Expand Down Expand Up @@ -197,6 +198,16 @@ func (s *ManagedControlPlaneScope) Vnet() *infrav1.VnetSpec {
}
}

// GroupSpec returns the resource group spec.
func (s *ManagedControlPlaneScope) GroupSpec() azure.ResourceSpecGetter {
return &groups.GroupSpec{
Name: s.ResourceGroup(),
Location: s.Location(),
ClusterName: s.ClusterName(),
AdditionalTags: s.AdditionalTags(),
}
}

// VNetSpec returns the virtual network spec.
func (s *ManagedControlPlaneScope) VNetSpec() azure.VNetSpec {
return azure.VNetSpec{
Expand Down
98 changes: 86 additions & 12 deletions azure/services/groups/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,20 @@ import (

"github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-05-01/resources"
"github.com/Azure/go-autorest/autorest"
azureautorest "github.com/Azure/go-autorest/autorest/azure"
"github.com/pkg/errors"

"sigs.k8s.io/cluster-api-provider-azure/azure"
"sigs.k8s.io/cluster-api-provider-azure/util/reconciler"
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
)

// client wraps go-sdk.
type client interface {
Get(context.Context, string) (resources.Group, error)
CreateOrUpdate(context.Context, string, resources.Group) (resources.Group, error)
Delete(context.Context, string) error
CreateOrUpdateAsync(context.Context, azure.ResourceSpecGetter) (azureautorest.FutureAPI, error)
DeleteAsync(context.Context, azure.ResourceSpecGetter) (azureautorest.FutureAPI, error)
IsDone(context.Context, azureautorest.FutureAPI) (bool, error)
}

// azureClient contains the Azure go-sdk Client.
Expand Down Expand Up @@ -63,27 +67,97 @@ func (ac *azureClient) Get(ctx context.Context, name string) (resources.Group, e
return ac.groups.Get(ctx, name)
}

// CreateOrUpdate creates or updates a resource group.
func (ac *azureClient) CreateOrUpdate(ctx context.Context, name string, group resources.Group) (resources.Group, error) {
// CreateOrUpdateAsync creates or updates a resource group.
// Creating a resource group is not a long running operation, so we don't ever return a future.
func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) {
ctx, span := tele.Tracer().Start(ctx, "groups.AzureClient.CreateOrUpdate")
defer span.End()

return ac.groups.CreateOrUpdate(ctx, name, group)
group, err := ac.resourceGroupParams(ctx, spec)
if err != nil {
return nil, errors.Wrapf(err, "failed to get desired parameters for group %s", spec.ResourceName())
} else if group == nil {
// nothing to do here
return nil, nil
}

_, err = ac.groups.CreateOrUpdate(ctx, spec.ResourceName(), *group)
return nil, err
}

// Delete deletes a resource group. When you delete a resource group, all of its resources are also deleted.
func (ac *azureClient) Delete(ctx context.Context, name string) error {
ctx, span := tele.Tracer().Start(ctx, "groups.AzureClient.Delete")
// DeleteAsync deletes a resource group asynchronously. DeleteAsync sends a DELETE
// request to Azure and if accepted without error, the func will return a Future which can be used to track the ongoing
// progress of the operation.
//
// NOTE: When you delete a resource group, all of its resources are also deleted.
func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) {
ctx, span := tele.Tracer().Start(ctx, "groups.AzureClient.DeleteAsync")
defer span.End()

future, err := ac.groups.Delete(ctx, name)
future, err := ac.groups.Delete(ctx, spec.ResourceName())
if err != nil {
return err
return nil, err
}

ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout)
defer cancel()

err = future.WaitForCompletionRef(ctx, ac.groups.Client)
if err != nil {
return err
// if an error occurs, return the future.
// this means the long-running operation didn't finish in the specified timeout.
return &future, err
}
_, err = future.Result(ac.groups)
return err
// if the operation completed, return a nil future.
return nil, err
}

// IsDone returns true if the long-running operation has completed.
func (ac *azureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (bool, error) {
ctx, span := tele.Tracer().Start(ctx, "groups.AzureClient.IsDone")
defer span.End()

done, err := future.DoneWithContext(ctx, ac.groups)
if err != nil {
return false, errors.Wrap(err, "failed checking if the operation was complete")
}

return done, nil
}

// resourceGroupParams returns the desired resource group parameters from the given spec.
func (ac *azureClient) resourceGroupParams(ctx context.Context, spec azure.ResourceSpecGetter) (*resources.Group, error) {
ctx, span := tele.Tracer().Start(ctx, "groups.AzureClient.resourceGroupParams")
defer span.End()

var params interface{}
CecileRobertMichon marked this conversation as resolved.
Show resolved Hide resolved

existingRG, err := ac.Get(ctx, spec.ResourceName())
if azure.ResourceNotFound(err) {
// rg doesn't exist, create it from scratch.
params, err = spec.Parameters(nil)
if err != nil {
return nil, err
}
} else if err != nil {
return nil, errors.Wrapf(err, "failed to get RG %s", spec.ResourceName())
} else {
// rg already exists
params, err = spec.Parameters(existingRG)
if err != nil {
return nil, err
}
}

rg, ok := params.(resources.Group)
if !ok {
if params == nil {
// nothing to do here.
return nil, nil
}
return nil, errors.Errorf("%T is not a resources.Group", params)
}

return &rg, nil
}
78 changes: 32 additions & 46 deletions azure/services/groups/groups.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,19 @@ package groups
import (
"context"

"github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-05-01/resources"
"github.com/Azure/go-autorest/autorest/to"
"github.com/go-logr/logr"
"github.com/pkg/errors"

infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4"
"sigs.k8s.io/cluster-api-provider-azure/azure"
"sigs.k8s.io/cluster-api-provider-azure/azure/converters"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/async"
"sigs.k8s.io/cluster-api-provider-azure/util/reconciler"
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
)

const serviceName = "group"

// Service provides operations on Azure resources.
type Service struct {
Scope GroupScope
Expand All @@ -39,7 +41,10 @@ type Service struct {
// GroupScope defines the scope interface for a group service.
type GroupScope interface {
logr.Logger
azure.ClusterDescriber
azure.Authorizer
azure.AsyncStatusUpdater
GroupSpec() azure.ResourceSpecGetter
ClusterName() string
}

// New creates a new service.
Expand All @@ -55,65 +60,45 @@ func (s *Service) Reconcile(ctx context.Context) error {
ctx, span := tele.Tracer().Start(ctx, "groups.Service.Reconcile")
defer span.End()

if _, err := s.client.Get(ctx, s.Scope.ResourceGroup()); err == nil {
// resource group already exists, skip creation
return nil
} else if !azure.ResourceNotFound(err) {
return errors.Wrapf(err, "failed to get resource group %s", s.Scope.ResourceGroup())
}
ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout)
defer cancel()

s.Scope.V(2).Info("creating resource group", "resource group", s.Scope.ResourceGroup())
group := resources.Group{
Location: to.StringPtr(s.Scope.Location()),
Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{
ClusterName: s.Scope.ClusterName(),
Lifecycle: infrav1.ResourceLifecycleOwned,
Name: to.StringPtr(s.Scope.ResourceGroup()),
Role: to.StringPtr(infrav1.CommonRole),
Additional: s.Scope.AdditionalTags(),
})),
}
groupSpec := s.Scope.GroupSpec()

_, err := s.client.CreateOrUpdate(ctx, s.Scope.ResourceGroup(), group)
if err != nil {
return errors.Wrapf(err, "failed to create resource group %s", s.Scope.ResourceGroup())
}

s.Scope.V(2).Info("successfully created resource group", "resource group", s.Scope.ResourceGroup())
return nil
err := async.CreateResource(ctx, s.Scope, s.client, groupSpec, serviceName)
s.Scope.UpdatePutStatus(infrav1.ResourceGroupReadyCondition, serviceName, err)
return err
}

// Delete deletes the resource group with the provided name.
// Delete deletes the resource group if it is managed by capz.
func (s *Service) Delete(ctx context.Context) error {
ctx, span := tele.Tracer().Start(ctx, "groups.Service.Delete")
defer span.End()

ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout)
CecileRobertMichon marked this conversation as resolved.
Show resolved Hide resolved
defer cancel()

groupSpec := s.Scope.GroupSpec()

// check that the resource group is not BYO.
managed, err := s.IsGroupManaged(ctx)
if err != nil && azure.ResourceNotFound(err) {
// already deleted or doesn't exist
return nil
}
if err != nil {
if azure.ResourceNotFound(err) {
// already deleted or doesn't exist, cleanup status and return.
s.Scope.DeleteLongRunningOperationState(groupSpec.ResourceName(), serviceName)
s.Scope.UpdateDeleteStatus(infrav1.ResourceGroupReadyCondition, serviceName, nil)
return nil
}
return errors.Wrap(err, "could not get resource group management state")
}

if !managed {
s.Scope.V(2).Info("Should not delete resource group in unmanaged mode")
return azure.ErrNotOwned
}

s.Scope.V(2).Info("deleting resource group", "resource group", s.Scope.ResourceGroup())
err = s.client.Delete(ctx, s.Scope.ResourceGroup())
if err != nil && azure.ResourceNotFound(err) {
// already deleted
return nil
}
if err != nil {
return errors.Wrapf(err, "failed to delete resource group %s", s.Scope.ResourceGroup())
}

s.Scope.V(2).Info("successfully deleted resource group", "resource group", s.Scope.ResourceGroup())
return nil
err = async.DeleteResource(ctx, s.Scope, s.client, groupSpec, serviceName)
s.Scope.UpdateDeleteStatus(infrav1.ResourceGroupReadyCondition, serviceName, err)
return err
}

// IsGroupManaged returns true if the resource group has an owned tag with the cluster name as value,
Expand All @@ -122,7 +107,8 @@ func (s *Service) IsGroupManaged(ctx context.Context) (bool, error) {
ctx, span := tele.Tracer().Start(ctx, "groups.Service.IsGroupManaged")
defer span.End()

group, err := s.client.Get(ctx, s.Scope.ResourceGroup())
groupSpec := s.Scope.GroupSpec()
group, err := s.client.Get(ctx, groupSpec.ResourceName())
if err != nil {
return false, err
}
Expand Down
Loading