Merge pull request #354 from CecileRobertMichon/services-tests

Refactor services and Azure clients to make services unit testable

cloud/scope/clients.go

@@ -17,11 +17,45 @@ limitations under the License.
 package scope
 
 import (
+	"os"
+
 	"github.com/Azure/go-autorest/autorest"
+	"github.com/Azure/go-autorest/autorest/azure/auth"
+	"github.com/pkg/errors"
 )
 
 // AzureClients contains all the Azure clients used by the scopes.
 type AzureClients struct {
 	SubscriptionID string
 	Authorizer     autorest.Authorizer
 }
+
+func (c *AzureClients) setCredentials() error {
+	if c.SubscriptionID == "" {
+		subID, err := getSubscriptionID()
+		if err != nil {
+			return err
+		}
+		c.SubscriptionID = subID
+	}
+	if c.Authorizer == nil {
+		auth, err := getAuthorizer()
+		if err != nil {
+			return err
+		}
+		c.Authorizer = auth
+	}
+	return nil
+}
+
+func getSubscriptionID() (string, error) {
+	subscriptionID := os.Getenv("AZURE_SUBSCRIPTION_ID")
+	if subscriptionID == "" {
+		return "", errors.New("error creating azure services. Environment variable AZURE_SUBSCRIPTION_ID is not set")
+	}
+	return subscriptionID, nil
+}
+
+func getAuthorizer() (autorest.Authorizer, error) {
+	return auth.NewAuthorizerFromEnvironment()
+}

cloud/scope/cluster.go

@@ -18,9 +18,7 @@ package scope
 
 import (
 	"context"
-	"os"
 
-	"github.com/Azure/go-autorest/autorest/azure/auth"
 	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
 	"k8s.io/klog/klogr"
@@ -54,17 +52,10 @@ func NewClusterScope(params ClusterScopeParams) (*ClusterScope, error) {
 		params.Logger = klogr.New()
 	}
 
-	authorizer, err := auth.NewAuthorizerFromEnvironment()
+	err := params.AzureClients.setCredentials()
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to create azure session")
+		return nil, errors.Wrap(err, "failed to create Azure session")
 	}
-	params.AzureClients.Authorizer = authorizer
-
-	subscriptionID := os.Getenv("AZURE_SUBSCRIPTION_ID")
-	if subscriptionID == "" {
-		return nil, errors.New("error creating azure services. Environment variable AZURE_SUBSCRIPTION_ID is not set")
-	}
-	params.AzureClients.SubscriptionID = subscriptionID
 
 	helper, err := patch.NewHelper(params.AzureCluster, params.Client)
 	if err != nil {

cloud/services/availabilityzones/client.go

@@ -0,0 +1,56 @@
+/*
+Copyright 2019 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 availabilityzones
+
+import (
+	"context"
+
+	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-07-01/compute"
+	"github.com/Azure/go-autorest/autorest"
+	azure "sigs.k8s.io/cluster-api-provider-azure/cloud"
+)
+
+// Client wraps go-sdk
+type Client interface {
+	ListComplete(context.Context) (compute.ResourceSkusResultIterator, error)
+}
+
+// AzureClient contains the Azure go-sdk Client
+type AzureClient struct {
+	resourceSkus compute.ResourceSkusClient
+}
+
+var _ Client = &AzureClient{}
+
+// NewClient creates a new VM client from subscription ID.
+func NewClient(subscriptionID string, authorizer autorest.Authorizer) *AzureClient {
+	c := newResourceSkusClient(subscriptionID, authorizer)
+	return &AzureClient{c}
+}
+
+// getResourceSkusClient creates a new availability zones client from subscription ID.
+func newResourceSkusClient(subscriptionID string, authorizer autorest.Authorizer) compute.ResourceSkusClient {
+	skusClient := compute.NewResourceSkusClient(subscriptionID)
+	skusClient.Authorizer = authorizer
+	skusClient.AddToUserAgent(azure.UserAgent)
+	return skusClient
+}
+
+// ListComplete enumerates all values, automatically crossing page boundaries as required.
+func (ac *AzureClient) ListComplete(ctx context.Context) (compute.ResourceSkusResultIterator, error) {
+	return ac.resourceSkus.ListComplete(ctx)
+}

cloud/services/availabilityzones/mock_availabilityzones/doc.go

@@ -0,0 +1,21 @@
+/*
+Copyright 2019 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.
+*/
+
+// Run go generate to regenerate this mock.
+//go:generate ../../../../hack/tools/bin/mockgen -destination availabilityzones_mock.go -package mock_availabilityzones -source ../client.go Client
+//go:generate /usr/bin/env bash -c "cat ../../../../hack/boilerplate/boilerplate.generatego.txt availabilityzones_mock.go > _availabilityzones_mock.go && mv _availabilityzones_mock.go availabilityzones_mock.go"
+
+package mock_availabilityzones //nolint

cloud/services/availabilityzones/service.go

@@ -17,32 +17,19 @@ limitations under the License.
 package availabilityzones
 
 import (
-	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-07-01/compute"
-	"github.com/Azure/go-autorest/autorest"
-	azure "sigs.k8s.io/cluster-api-provider-azure/cloud"
 	"sigs.k8s.io/cluster-api-provider-azure/cloud/scope"
 )
 
-var _ azure.Service = (*Service)(nil)
-
-// Service provides operations on availability zones
+// Service provides operations on azure resources
 type Service struct {
-	Client compute.ResourceSkusClient
-	Scope  *scope.ClusterScope
-}
-
-// getResourceSkusClient creates a new availability zones client from subscriptionid.
-func getResourceSkusClient(subscriptionID string, authorizer autorest.Authorizer) compute.ResourceSkusClient {
-	skusClient := compute.NewResourceSkusClient(subscriptionID)
-	skusClient.Authorizer = authorizer
-	skusClient.AddToUserAgent(azure.UserAgent)
-	return skusClient
+	Scope *scope.ClusterScope
+	Client
 }
 
-// NewService creates a new availability zones service.
+// NewService creates a new service.
 func NewService(scope *scope.ClusterScope) *Service {
 	return &Service{
-		Client: getResourceSkusClient(scope.SubscriptionID, scope.Authorizer),
 		Scope:  scope,
+		Client: NewClient(scope.SubscriptionID, scope.Authorizer),
 	}
 }

cloud/services/disks/client.go

@@ -0,0 +1,64 @@
+/*
+Copyright 2019 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 disks
+
+import (
+	"context"
+
+	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-07-01/compute"
+	"github.com/Azure/go-autorest/autorest"
+	azure "sigs.k8s.io/cluster-api-provider-azure/cloud"
+)
+
+// Client wraps go-sdk
+type Client interface {
+	Delete(context.Context, string, string) error
+}
+
+// AzureClient contains the Azure go-sdk Client
+type AzureClient struct {
+	disks compute.DisksClient
+}
+
+var _ Client = &AzureClient{}
+
+// NewClient creates a new VM client from subscription ID.
+func NewClient(subscriptionID string, authorizer autorest.Authorizer) *AzureClient {
+	c := newDisksClient(subscriptionID, authorizer)
+	return &AzureClient{c}
+}
+
+// newDisksClient creates a new disks client from subscription ID.
+func newDisksClient(subscriptionID string, authorizer autorest.Authorizer) compute.DisksClient {
+	disksClient := compute.NewDisksClient(subscriptionID)
+	disksClient.Authorizer = authorizer
+	disksClient.AddToUserAgent(azure.UserAgent)
+	return disksClient
+}
+
+func (ac *AzureClient) Delete(ctx context.Context, resourceGroupName, name string) error {
+	future, err := ac.disks.Delete(ctx, resourceGroupName, name)
+	if err != nil {
+		return err
+	}
+	err = future.WaitForCompletionRef(ctx, ac.disks.Client)
+	if err != nil {
+		return err
+	}
+	_, err = future.Result(ac.disks)
+	return err
+}

cloud/services/disks/disks.go

@@ -46,7 +46,7 @@ func (s *Service) Delete(ctx context.Context, spec interface{}) error {
 		return errors.New("Invalid disk specification")
 	}
 	klog.V(2).Infof("deleting disk %s", diskSpec.Name)
-	future, err := s.Client.Delete(ctx, s.Scope.AzureCluster.Spec.ResourceGroup, diskSpec.Name)
+	err := s.Client.Delete(ctx, s.Scope.AzureCluster.Spec.ResourceGroup, diskSpec.Name)
 	if err != nil && azure.ResourceNotFound(err) {
 		// already deleted
 		return nil
@@ -55,15 +55,6 @@ func (s *Service) Delete(ctx context.Context, spec interface{}) error {
 		return errors.Wrapf(err, "failed to delete disk %s in resource group %s", diskSpec.Name, s.Scope.AzureCluster.Spec.ResourceGroup)
 	}
 
-	err = future.WaitForCompletionRef(ctx, s.Client.Client)
-	if err != nil {
-		return errors.Wrap(err, "cannot create, future response")
-	}
-
-	_, err = future.Result(s.Client)
-	if err != nil {
-		return errors.Wrap(err, "result error")
-	}
 	klog.V(2).Infof("successfully deleted disk %s", diskSpec.Name)
-	return err
+	return nil
 }