Skip to content

Make subnets reconcile/delete async #1914

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 1 commit into from
Feb 9, 2022
Merged

Make subnets reconcile/delete async #1914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 31 additions & 5 deletions azure/scope/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"sigs.k8s.io/cluster-api-provider-azure/azure/services/loadbalancers"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/natgateways"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/routetables"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/subnets"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualnetworks"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/vnetpeerings"
"sigs.k8s.io/cluster-api-provider-azure/util/futures"
Expand Down Expand Up @@ -289,20 +290,25 @@ func (s *ClusterScope) NSGSpecs() []azure.NSGSpec {
}

// SubnetSpecs returns the subnets specs.
func (s *ClusterScope) SubnetSpecs() []azure.SubnetSpec {
func (s *ClusterScope) SubnetSpecs() []azure.ResourceSpecGetter {
numberOfSubnets := len(s.AzureCluster.Spec.NetworkSpec.Subnets)
if s.IsAzureBastionEnabled() {
numberOfSubnets++
}

subnetSpecs := make([]azure.SubnetSpec, 0, numberOfSubnets)
subnetSpecs := make([]azure.ResourceSpecGetter, 0, numberOfSubnets)

for _, subnet := range s.AzureCluster.Spec.NetworkSpec.Subnets {
subnetSpec := azure.SubnetSpec{
subnetSpec := &subnets.SubnetSpec{
Name: subnet.Name,
ResourceGroup: s.ResourceGroup(),
Comment thread
CecileRobertMichon marked this conversation as resolved.
SubscriptionID: s.SubscriptionID(),
CIDRs: subnet.CIDRBlocks,
VNetName: s.Vnet().Name,
SecurityGroupName: subnet.SecurityGroup.Name,
VNetResourceGroup: s.Vnet().ResourceGroup,
IsVNetManaged: s.IsVnetManaged(),
RouteTableName: subnet.RouteTable.Name,
SecurityGroupName: subnet.SecurityGroup.Name,
Role: subnet.Role,
NatGatewayName: subnet.NatGateway.Name,
}
Expand All @@ -311,10 +317,14 @@ func (s *ClusterScope) SubnetSpecs() []azure.SubnetSpec {

if s.IsAzureBastionEnabled() {
azureBastionSubnet := s.AzureCluster.Spec.BastionSpec.AzureBastion.Subnet
subnetSpecs = append(subnetSpecs, azure.SubnetSpec{
subnetSpecs = append(subnetSpecs, &subnets.SubnetSpec{
Name: azureBastionSubnet.Name,
ResourceGroup: s.ResourceGroup(),
Copy link
Copy Markdown
Contributor

@shysank shysank Jan 26, 2022

Choose a reason for hiding this comment

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

same comment as above for vnet resource rg.

SubscriptionID: s.SubscriptionID(),
CIDRs: azureBastionSubnet.CIDRBlocks,
VNetName: s.Vnet().Name,
VNetResourceGroup: s.Vnet().ResourceGroup,
IsVNetManaged: s.IsVnetManaged(),
SecurityGroupName: azureBastionSubnet.SecurityGroup.Name,
RouteTableName: azureBastionSubnet.RouteTable.Name,
Role: azureBastionSubnet.Role,
Expand Down Expand Up @@ -507,6 +517,20 @@ func (s *ClusterScope) SetNatGatewayIDInSubnets(name string, id string) {
}
}

// UpdateSubnetCIDRs updates the subnet CIDRs for the subnet with the same name.
func (s *ClusterScope) UpdateSubnetCIDRs(name string, cidrBlocks []string) {
Comment thread
Jont828 marked this conversation as resolved.
subnetSpecInfra := s.Subnet(name)
subnetSpecInfra.CIDRBlocks = cidrBlocks
s.SetSubnet(subnetSpecInfra)
}

// UpdateSubnetIDs updates the subnet IDs for the subnet with the same name.
func (s *ClusterScope) UpdateSubnetID(name string, id string) {
subnetSpecInfra := s.Subnet(name)
subnetSpecInfra.ID = id
s.SetSubnet(subnetSpecInfra)
}

// ControlPlaneRouteTable returns the cluster controlplane routetable.
func (s *ClusterScope) ControlPlaneRouteTable() infrav1.RouteTable {
subnet, _ := s.AzureCluster.Spec.NetworkSpec.GetControlPlaneSubnet()
Expand Down Expand Up @@ -666,6 +690,7 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error {
infrav1.LoadBalancersReadyCondition,
infrav1.BastionHostReadyCondition,
infrav1.VNetReadyCondition,
infrav1.SubnetsReadyCondition,
),
)

Expand All @@ -683,6 +708,7 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error {
infrav1.LoadBalancersReadyCondition,
infrav1.BastionHostReadyCondition,
infrav1.VNetReadyCondition,
infrav1.SubnetsReadyCondition,
}})
}

Expand Down
30 changes: 24 additions & 6 deletions azure/scope/managedcontrolplane.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-azure/azure"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/groups"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/subnets"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualnetworks"
infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-azure/util/futures"
Expand Down Expand Up @@ -255,12 +256,17 @@ func (s *ManagedControlPlaneScope) NodeNatGateway() infrav1.NatGateway {
}

// SubnetSpecs returns the subnets specs.
func (s *ManagedControlPlaneScope) SubnetSpecs() []azure.SubnetSpec {
return []azure.SubnetSpec{
{
Name: s.NodeSubnet().Name,
CIDRs: s.NodeSubnet().CIDRBlocks,
VNetName: s.Vnet().Name,
func (s *ManagedControlPlaneScope) SubnetSpecs() []azure.ResourceSpecGetter {
return []azure.ResourceSpecGetter{
&subnets.SubnetSpec{
Name: s.NodeSubnet().Name,
ResourceGroup: s.ResourceGroup(),
SubscriptionID: s.SubscriptionID(),
CIDRs: s.NodeSubnet().CIDRBlocks,
VNetName: s.Vnet().Name,
VNetResourceGroup: s.Vnet().ResourceGroup,
IsVNetManaged: s.IsVnetManaged(),
Role: infrav1.SubnetNode,
},
}
}
Expand All @@ -286,6 +292,18 @@ func (s *ManagedControlPlaneScope) SetSubnet(_ infrav1.SubnetSpec) {
// no-op
}

// UpdateSubnetCIDRs updates the subnet CIDRs for the subnet with the same name.
// This is not used when using a managed control plane.
func (s *ManagedControlPlaneScope) UpdateSubnetCIDRs(_ string, _ []string) {
Comment thread
Jont828 marked this conversation as resolved.
// no-op
}

// UpdateSubnetIDs updates the subnet IDs for the subnet with the same name.
// This is not used when using a managed control plane.
func (s *ManagedControlPlaneScope) UpdateSubnetID(_ string, _ string) {
// no-op
}

// ControlPlaneSubnet returns the cluster control plane subnet.
func (s *ManagedControlPlaneScope) ControlPlaneSubnet() infrav1.SubnetSpec {
return infrav1.SubnetSpec{}
Expand Down
124 changes: 95 additions & 29 deletions azure/services/subnets/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,23 @@ package subnets

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, string) (network.Subnet, error)
CreateOrUpdate(context.Context, string, string, string, network.Subnet) error
Delete(context.Context, string, string, string) error
}

// AzureClient contains the Azure go-sdk Client.
type AzureClient struct {
subnets network.SubnetsClient
}

var _ Client = &AzureClient{}

// NewClient creates a new subnets client from subscription ID.
func NewClient(auth azure.Authorizer) *AzureClient {
c := newSubnetsClient(auth.SubscriptionID(), auth.BaseURI(), auth.Authorizer())
Expand All @@ -53,43 +49,113 @@ func newSubnetsClient(subscriptionID string, baseURI string, authorizer autorest
}

// Get gets the specified subnet by virtual network and resource group.
func (ac *AzureClient) Get(ctx context.Context, resourceGroupName, vnetName, snName string) (network.Subnet, error) {
func (ac *AzureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) (result interface{}, err error) {
ctx, _, done := tele.StartSpanWithLogger(ctx, "subnets.AzureClient.Get")
defer done()

return ac.subnets.Get(ctx, resourceGroupName, vnetName, snName, "")
return ac.subnets.Get(ctx, spec.ResourceGroupName(), spec.OwnerResourceName(), spec.ResourceName(), "")
Comment thread
CecileRobertMichon marked this conversation as resolved.
}

// CreateOrUpdate creates or updates a subnet in the specified virtual network.
func (ac *AzureClient) CreateOrUpdate(ctx context.Context, resourceGroupName, vnetName, snName string, sn network.Subnet) error {
ctx, _, done := tele.StartSpanWithLogger(ctx, "subnets.AzureClient.CreateOrUpdate")
// CreateOrUpdateAsync creates or updates a subnet 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, "subnets.AzureClient.CreateOrUpdateAsync")
defer done()

future, err := ac.subnets.CreateOrUpdate(ctx, resourceGroupName, vnetName, snName, sn)
subnet, ok := parameters.(network.Subnet)
if !ok {
return nil, nil, errors.Errorf("%T is not a network.Subnet", parameters)
}

createFuture, err := ac.subnets.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.OwnerResourceName(), spec.ResourceName(), subnet)
Comment thread
CecileRobertMichon marked this conversation as resolved.
if err != nil {
return err
return nil, nil, err
}
err = future.WaitForCompletionRef(ctx, ac.subnets.Client)

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

err = createFuture.WaitForCompletionRef(ctx, ac.subnets.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.subnets)
return err

result, err = createFuture.Result(ac.subnets)
// if the operation completed, return a nil future
return result, nil, err
}

// Delete deletes the specified subnet.
func (ac *AzureClient) Delete(ctx context.Context, resourceGroupName, vnetName, snName string) error {
ctx, _, done := tele.StartSpanWithLogger(ctx, "subnets.AzureClient.Delete")
// DeleteAsync deletes a subnet 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, "subnets.AzureClient.DeleteAsync")
defer done()

future, err := ac.subnets.Delete(ctx, resourceGroupName, vnetName, snName)
deleteFuture, err := ac.subnets.Delete(ctx, spec.ResourceGroupName(), spec.OwnerResourceName(), spec.ResourceName())
if err != nil {
return nil, err
}

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

err = deleteFuture.WaitForCompletionRef(ctx, ac.subnets.Client)
if err != nil {
return err
// if an error occurs, return the future.
// this means the long-running operation didn't finish in the specified timeout.
return &deleteFuture, err
}
err = future.WaitForCompletionRef(ctx, ac.subnets.Client)
_, err = deleteFuture.Result(ac.subnets)
// 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, _, done := tele.StartSpanWithLogger(ctx, "subnets.AzureClient.IsDone")
defer done()

isDone, err := future.DoneWithContext(ctx, ac.subnets)
if err != nil {
return err
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, "subnets.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 SubnetsCreateOrUpdateFuture 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.SubnetsCreateOrUpdateFuture
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.subnets)

case infrav1.DeleteFuture:
// Delete does not return a result subnet
return nil, nil

default:
return nil, errors.Errorf("unknown future type %q", futureType)
}
_, err = future.Result(ac.subnets)
return err
}
Loading