Make group, vnet, and nsg services async

api/v1alpha3/azurecluster_conversion.go

@@ -95,6 +95,8 @@ func (src *AzureCluster) ConvertTo(dstRaw conversion.Hub) error { // nolint
 		}
 	}
 
+	dst.Status.LongRunningOperationStates = restored.Status.LongRunningOperationStates
+
 	return nil
 }
 

api/v1alpha3/azuremachine_conversion.go

@@ -53,6 +53,8 @@ func (src *AzureMachine) ConvertTo(dstRaw conversion.Hub) error { // nolint
 
 	dst.Spec.SubnetName = restored.Spec.SubnetName
 
+	dst.Status.LongRunningOperationStates = restored.Status.LongRunningOperationStates
+
 	return nil
 }
 

api/v1alpha4/azurecluster_types.go

@@ -105,7 +105,9 @@ type AzureClusterStatus struct {
 
 // +kubebuilder:object:root=true
 // +kubebuilder:printcolumn:name="Cluster",type="string",JSONPath=".metadata.labels.cluster\\.x-k8s\\.io/cluster-name",description="Cluster to which this AzureCluster belongs"
-// +kubebuilder:printcolumn:name="Ready",type="boolean",JSONPath=".status.ready"
+// +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status"
+// +kubebuilder:printcolumn:name="Reason",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].reason"
+// +kubebuilder:printcolumn:name="Message",type="string",priority=1,JSONPath=".status.conditions[?(@.type=='Ready')].message"
 // +kubebuilder:printcolumn:name="Resource Group",type="string",priority=1,JSONPath=".spec.resourceGroup"
 // +kubebuilder:printcolumn:name="SubscriptionID",type="string",priority=1,JSONPath=".spec.subscriptionID"
 // +kubebuilder:printcolumn:name="Location",type="string",priority=1,JSONPath=".spec.location"

api/v1alpha4/conditions_consts.go

@@ -20,8 +20,6 @@ import clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4"
 
 // AzureCluster Conditions and Reasons.
 const (
-	// NetworkInfrastructureReadyCondition reports of current status of cluster infrastructure.
-	NetworkInfrastructureReadyCondition clusterv1.ConditionType = "NetworkInfrastructureReady"
 	// NamespaceNotAllowedByIdentity used to indicate cluster in a namespace not allowed by identity.
 	NamespaceNotAllowedByIdentity = "NamespaceNotAllowedByIdentity"
 )
@@ -115,4 +113,6 @@ const (
 	DeletedReason = "Deleted"
 	// DeletionFailedReason means the resource failed to be deleted.
 	DeletionFailedReason = "DeletionFailed"
+	// UpdatingReason means the resource is being updated.
+	UpdatingReason = "Updating"
 )

api/v1alpha4/types.go

@@ -31,6 +31,15 @@ const (
 
 type Futures []Future
 
+const (
+	// PatchFuture is a future that was derived from a PATCH request.
+	PatchFuture string = "PATCH"
+	// PutFuture is a future that was derived from a PUT request.
+	PutFuture string = "PUT"
+	// DeleteFuture is a future that was derived from a DELETE request.
+	DeleteFuture string = "DELETE"
+)
+
 // Future contains the data needed for an Azure long-running operation to continue across reconcile loops.
 type Future struct {
 	// Type describes the type of future, such as update, create, delete, etc.

azure/converters/futures.go

@@ -18,22 +18,12 @@ package converters
 
 import (
 	"encoding/base64"
-	"encoding/json"
 
 	azureautorest "github.com/Azure/go-autorest/autorest/azure"
 	"github.com/pkg/errors"
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4"
 )
 
-const (
-	// PatchFuture is a future that was derived from a PATCH request.
-	PatchFuture string = "PATCH"
-	// PutFuture is a future that was derived from a PUT request.
-	PutFuture string = "PUT"
-	// DeleteFuture is a future that was derived from a DELETE request.
-	DeleteFuture string = "DELETE"
-)
-
 func SDKToFuture(future azureautorest.FutureAPI, futureType, service, resourceName, rgName string) (*infrav1.Future, error) {
 	jsonData, err := future.MarshalJSON()
 	if err != nil {
@@ -49,14 +39,14 @@ func SDKToFuture(future azureautorest.FutureAPI, futureType, service, resourceNa
 	}, nil
 }
 
-func FutureToSDK(future *infrav1.Future) (azureautorest.FutureAPI, error) {
+func FutureToSDK(future infrav1.Future) (azureautorest.FutureAPI, error) {
 	futureData, err := base64.URLEncoding.DecodeString(future.Data)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to base64 decode future data")
 	}
-	var genericFuture azureautorest.FutureAPI
-	if err := json.Unmarshal(futureData, &genericFuture); err != nil {
+	var genericFuture azureautorest.Future
+	if err := genericFuture.UnmarshalJSON(futureData); err != nil {
 		return nil, errors.Wrap(err, "failed to unmarshal future data")
 	}
-	return genericFuture, nil
+	return &genericFuture, nil
 }

azure/converters/futures_test.go

@@ -0,0 +1,108 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package converters
+
+import (
+	"testing"
+
+	azureautorest "github.com/Azure/go-autorest/autorest/azure"
+	"github.com/onsi/gomega"
+
+	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4"
+)
+
+var (
+	validFuture = infrav1.Future{
+		Type:          infrav1.DeleteFuture,
+		ServiceName:   "test-service",
+		Name:          "test-group",
+		ResourceGroup: "test-group",
+		Data:          "eyJtZXRob2QiOiJERUxFVEUiLCJwb2xsaW5nTWV0aG9kIjoiTG9jYXRpb24iLCJscm9TdGF0ZSI6IkluUHJvZ3Jlc3MifQ==",
+	}
+
+	emptyDataFuture = infrav1.Future{
+		Type:          infrav1.DeleteFuture,
+		ServiceName:   "test-service",
+		Name:          "test-group",
+		ResourceGroup: "test-group",
+		Data:          "",
+	}
+
+	decodedDataFuture = infrav1.Future{
+		Type:          infrav1.DeleteFuture,
+		ServiceName:   "test-service",
+		Name:          "test-group",
+		ResourceGroup: "test-group",
+		Data:          "this is not b64 encoded",
+	}
+
+	invalidFuture = infrav1.Future{
+		Type:          infrav1.DeleteFuture,
+		ServiceName:   "test-service",
+		Name:          "test-group",
+		ResourceGroup: "test-group",
+		Data:          "ZmFrZSBiNjQgZnV0dXJlIGRhdGEK",
+	}
+)
+
+func Test_FutureToSDK(t *testing.T) {
+	cases := []struct {
+		name   string
+		future infrav1.Future
+		expect func(*gomega.GomegaWithT, azureautorest.FutureAPI, error)
+	}{
+		{
+			name:   "data is empty",
+			future: emptyDataFuture,
+			expect: func(g *gomega.GomegaWithT, f azureautorest.FutureAPI, err error) {
+				g.Expect(err.Error()).Should(gomega.ContainSubstring("failed to unmarshal future data"))
+			},
+		},
+		{
+			name:   "data is not base64 encoded",
+			future: decodedDataFuture,
+			expect: func(g *gomega.GomegaWithT, f azureautorest.FutureAPI, err error) {
+				g.Expect(err.Error()).Should(gomega.ContainSubstring("failed to base64 decode future data"))
+			},
+		},
+		{
+			name:   "base64 data is not a valid future",
+			future: invalidFuture,
+			expect: func(g *gomega.GomegaWithT, f azureautorest.FutureAPI, err error) {
+				g.Expect(err.Error()).Should(gomega.ContainSubstring("failed to unmarshal future data"))
+			},
+		},
+		{
+			name:   "valid future data",
+			future: validFuture,
+			expect: func(g *gomega.GomegaWithT, f azureautorest.FutureAPI, err error) {
+				g.Expect(err).Should(gomega.BeNil())
+				g.Expect(f).Should(gomega.BeAssignableToTypeOf(&azureautorest.Future{}))
+			},
+		},
+	}
+
+	for _, c := range cases {
+		c := c
+		t.Run(c.name, func(t *testing.T) {
+			t.Parallel()
+			g := gomega.NewGomegaWithT(t)
+			result, err := FutureToSDK(c.future)
+			c.expect(g, result, err)
+		})
+	}
+}

azure/errors.go

@@ -107,8 +107,8 @@ func (t ReconcileError) IsTerminal() bool {
 	return t.errorType == TerminalErrorType
 }
 
-// Is returns true if the target is a ReconcileError.
-func (t ReconcileError) Is(target error) bool {
+// IsReconcileError returns true if the target is a ReconcileError.
+func IsReconcileError(target error) bool {
 	return errors.As(target, &ReconcileError{})
 }
 
@@ -133,8 +133,8 @@ type OperationNotDoneError struct {
 }
 
 // NewOperationNotDoneError returns a new OperationNotDoneError wrapping a Future.
-func NewOperationNotDoneError(future *infrav1.Future) *OperationNotDoneError {
-	return &OperationNotDoneError{
+func NewOperationNotDoneError(future *infrav1.Future) OperationNotDoneError {
+	return OperationNotDoneError{
 		Future: future,
 	}
 }
@@ -144,7 +144,11 @@ func (onde OperationNotDoneError) Error() string {
 	return fmt.Sprintf("operation type %s on Azure resource %s/%s is not done", onde.Future.Type, onde.Future.ResourceGroup, onde.Future.Name)
 }
 
-// Is returns true if the target is an OperationNotDoneError.
-func (onde OperationNotDoneError) Is(target error) bool {
+// IsOperationNotDoneError returns true if the target is an OperationNotDoneError.
+func IsOperationNotDoneError(target error) bool {
+	reconcileErr := &ReconcileError{}
+	if errors.As(target, reconcileErr) {
+		return IsOperationNotDoneError(reconcileErr.error)
+	}
 	return errors.As(target, &OperationNotDoneError{})
 }

azure/interfaces.go

@@ -54,7 +54,7 @@ type Authorizer interface {
 // NetworkDescriber is an interface which can get common Azure Cluster Networking information.
 type NetworkDescriber interface {
 	Vnet() *infrav1.VnetSpec
-	IsVnetManaged() bool
+	IsVnetManaged(context.Context) (bool, error)
 	ControlPlaneSubnet() infrav1.SubnetSpec
 	Subnets() infrav1.Subnets
 	Subnet(string) infrav1.SubnetSpec
@@ -81,16 +81,33 @@ type ClusterDescriber interface {
 	CloudProviderConfigOverrides() *infrav1.CloudProviderConfigOverrides
 }
 
+// AsyncStatusUpdater is an interface used to keep track of long running operations in Status that has Conditions and Futures.
 type AsyncStatusUpdater interface {
 	SetLongRunningOperationState(*infrav1.Future)
 	GetLongRunningOperationState(string, string) *infrav1.Future
 	DeleteLongRunningOperationState(string, string)
-	SetConditionTrue(clusterv1.ConditionType)
-	SetConditionFalse(clusterv1.ConditionType, string, clusterv1.ConditionSeverity)
+	UpdatePutStatus(clusterv1.ConditionType, string, error)
+	UpdateDeleteStatus(clusterv1.ConditionType, string, error)
+	UpdatePatchStatus(clusterv1.ConditionType, string, error)
 }
 
 // ClusterScoper combines the ClusterDescriber and NetworkDescriber interfaces.
 type ClusterScoper interface {
 	ClusterDescriber
 	NetworkDescriber
 }
+
+// ResourceSpecGetter is an interface for getting all the required information to create/update/delete an Azure resource.
+type ResourceSpecGetter interface {
+	// ResourceName returns the name of the resource.
+	ResourceName() string
+	// OwnerResourceName returns the name of the resource that owns the resource
+	// in the case that the resource is an Azure subresource.
+	OwnerResourceName() string
+	// ResourceGroupName returns the name of the resource group the resource is in.
+	ResourceGroupName() string
+	// Parameters takes the existing resource and returns the desired parameters of the resource.
+	// If the resource does not exist, or we do not care about existing parameters to update the resource, existing should be nil.
+	// If no update is needed on the resource, Parameters should return nil.
+	Parameters(existing interface{}) (interface{}, error)
+}