From 5258619380df768d6495cd37ac481e8b099c0cf8 Mon Sep 17 00:00:00 2001 From: Anshul Verma Date: Tue, 28 Feb 2023 21:27:15 +0530 Subject: [PATCH] used an interface in place of variables for tests --- pkg/portal/cluster/fetcher.go | 27 ++++++++++++++++++++--- pkg/portal/cluster/machines.go | 11 ++-------- pkg/portal/cluster/machines_test.go | 33 +++++++++++++++++------------ 3 files changed, 45 insertions(+), 26 deletions(-) diff --git a/pkg/portal/cluster/fetcher.go b/pkg/portal/cluster/fetcher.go index aebfb2f64..1a6824d71 100644 --- a/pkg/portal/cluster/fetcher.go +++ b/pkg/portal/cluster/fetcher.go @@ -6,6 +6,7 @@ package cluster import ( "context" + "github.com/Azure/go-autorest/autorest" configclient "github.com/openshift/client-go/config/clientset/versioned" machineclient "github.com/openshift/client-go/machine/clientset/versioned" "github.com/sirupsen/logrus" @@ -14,10 +15,18 @@ import ( "github.com/Azure/ARO-RP/pkg/api" "github.com/Azure/ARO-RP/pkg/env" "github.com/Azure/ARO-RP/pkg/proxy" + "github.com/Azure/ARO-RP/pkg/util/azureclient" + "github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/compute" + "github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/features" "github.com/Azure/ARO-RP/pkg/util/restconfig" "github.com/Azure/ARO-RP/pkg/util/stringutils" ) +type ResourceFactory interface { + NewResourcesClient(environment *azureclient.AROEnvironment, subscriptionID string, authorizer autorest.Authorizer) features.ResourcesClient + NewVirtualMachinesClient(environment *azureclient.AROEnvironment, subscriptionID string, authorizer autorest.Authorizer) compute.VirtualMachinesClient +} + // FetchClient is the interface that the Admin Portal Frontend uses to gather // information about clusters. It returns frontend-suitable data structures. type FetchClient interface { @@ -49,6 +58,7 @@ type realFetcher struct { kubernetesCli kubernetes.Interface machineClient machineclient.Interface azureSideFetcher azureSideFetcher + resourceFactory ResourceFactory } type azureSideFetcher struct { @@ -57,6 +67,16 @@ type azureSideFetcher struct { env env.Interface } +type resourceFactory struct{} + +func (rf resourceFactory) NewResourcesClient(environment *azureclient.AROEnvironment, subscriptionID string, authorizer autorest.Authorizer) features.ResourcesClient { + return features.NewResourcesClient(environment, subscriptionID, authorizer) +} + +func (rf resourceFactory) NewVirtualMachinesClient(environment *azureclient.AROEnvironment, subscriptionID string, authorizer autorest.Authorizer) compute.VirtualMachinesClient { + return compute.NewVirtualMachinesClient(environment, subscriptionID, authorizer) +} + func newAzureSideFetcher(resourceGroupName string, subscriptionDoc *api.SubscriptionDocument, env env.Interface) azureSideFetcher { return azureSideFetcher{ resourceGroupName: resourceGroupName, @@ -65,7 +85,7 @@ func newAzureSideFetcher(resourceGroupName string, subscriptionDoc *api.Subscrip } } -func newRealFetcher(log *logrus.Entry, dialer proxy.Dialer, doc *api.OpenShiftClusterDocument, azureSideFetcher azureSideFetcher) (*realFetcher, error) { +func newRealFetcher(log *logrus.Entry, dialer proxy.Dialer, doc *api.OpenShiftClusterDocument, azureSideFetcher azureSideFetcher, resourceFactory ResourceFactory) (*realFetcher, error) { restConfig, err := restconfig.RestConfig(dialer, doc.OpenShiftCluster) if err != nil { log.Error(err) @@ -95,14 +115,15 @@ func newRealFetcher(log *logrus.Entry, dialer proxy.Dialer, doc *api.OpenShiftCl kubernetesCli: kubernetesCli, machineClient: machineClient, azureSideFetcher: azureSideFetcher, + resourceFactory: resourceFactory, }, nil } func NewFetchClient(log *logrus.Entry, dialer proxy.Dialer, cluster *api.OpenShiftClusterDocument, subscriptionDoc *api.SubscriptionDocument, env env.Interface) (FetchClient, error) { resourceGroupName := stringutils.LastTokenByte(cluster.OpenShiftCluster.Properties.ClusterProfile.ResourceGroupID, '/') azureSideFetcher := newAzureSideFetcher(resourceGroupName, subscriptionDoc, env) - - fetcher, err := newRealFetcher(log, dialer, cluster, azureSideFetcher) + rf := resourceFactory{} + fetcher, err := newRealFetcher(log, dialer, cluster, azureSideFetcher, rf) if err != nil { return nil, err } diff --git a/pkg/portal/cluster/machines.go b/pkg/portal/cluster/machines.go index 28019257b..caf282358 100644 --- a/pkg/portal/cluster/machines.go +++ b/pkg/portal/cluster/machines.go @@ -14,13 +14,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/compute" - "github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/features" ) -// Creating local vars for these functions in order to make them swappable to make them testable. The function definition will is altered in the tests written. -var newResourceClientFunction = features.NewResourcesClient -var newVirtualMachineClientFunction = compute.NewVirtualMachinesClient - type MachinesInformation struct { Name string `json:"name"` CreatedTime string `json:"createdTime"` @@ -82,15 +77,13 @@ func (f *realFetcher) VMAllocationStatus(ctx context.Context) (map[string]string if err != nil { return nil, err } - // Getting Virtual Machine resources through the Cluster's Resource Group - computeResources, err := newResourceClientFunction(env.Environment(), subscriptionDoc.ID, fpAuth).ListByResourceGroup(ctx, clusterRGName, "resourceType eq 'Microsoft.Compute/virtualMachines'", "", nil) + computeResources, err := f.resourceFactory.NewResourcesClient(env.Environment(), subscriptionDoc.ID, fpAuth).ListByResourceGroup(ctx, clusterRGName, "resourceType eq 'Microsoft.Compute/virtualMachines'", "", nil) if err != nil { return nil, err } - vmAllocationStatus := make(map[string]string) - virtualMachineClient := newVirtualMachineClientFunction(env.Environment(), subscriptionDoc.ID, fpAuth) + virtualMachineClient := f.resourceFactory.NewVirtualMachinesClient(env.Environment(), subscriptionDoc.ID, fpAuth) for _, res := range computeResources { putAllocationStatusToMap(ctx, clusterRGName, vmAllocationStatus, res, virtualMachineClient, f.log) } diff --git a/pkg/portal/cluster/machines_test.go b/pkg/portal/cluster/machines_test.go index fe4360916..2143fe34b 100644 --- a/pkg/portal/cluster/machines_test.go +++ b/pkg/portal/cluster/machines_test.go @@ -112,25 +112,28 @@ func TestMachines(t *testing.T) { } } +type mockResourceFactory struct { + mockResourcesClient *mock_features.MockResourcesClient + mockVirtualMachinesClient *mock_compute.MockVirtualMachinesClient +} + +func newMockResourceFactory(mockResourcesClient *mock_features.MockResourcesClient, mockVirtualMachinesClient *mock_compute.MockVirtualMachinesClient) mockResourceFactory { + return mockResourceFactory{mockResourcesClient: mockResourcesClient, mockVirtualMachinesClient: mockVirtualMachinesClient} +} + +func (mrf mockResourceFactory) NewResourcesClient(environment *azureclient.AROEnvironment, subscriptionID string, authorizer autorest.Authorizer) features.ResourcesClient { + return mrf.mockResourcesClient +} + +func (mrf mockResourceFactory) NewVirtualMachinesClient(environment *azureclient.AROEnvironment, subscriptionID string, authorizer autorest.Authorizer) compute.VirtualMachinesClient { + return mrf.mockVirtualMachinesClient +} + func TestVMAllocationStatus(t *testing.T) { ctx := context.Background() controller := gomock.NewController(t) mockResourcesClient := mock_features.NewMockResourcesClient(controller) mockVirtualMachinesClient := mock_compute.NewMockVirtualMachinesClient(controller) - newResourceClientFunction = func(environment *azureclient.AROEnvironment, - subscriptionID string, - authorizer autorest.Authorizer) features.ResourcesClient { - return mockResourcesClient - } - newVirtualMachineClientFunction = func(environment *azureclient.AROEnvironment, subscriptionID string, authorizer autorest.Authorizer) compute.VirtualMachinesClient { - return mockVirtualMachinesClient - } - - defer func() { - newResourceClientFunction = features.NewResourcesClient - newVirtualMachineClientFunction = compute.NewVirtualMachinesClient - }() - type test struct { name string mocks func(*test, *mock_env.MockInterface, *mock_refreshable.MockAuthorizer, *mock_features.MockResourcesClient, *mock_compute.MockVirtualMachinesClient) @@ -245,9 +248,11 @@ func TestVMAllocationStatus(t *testing.T) { env: mockEnv, subscriptionDoc: subscriptionDoc, } + mrf := newMockResourceFactory(mockResourcesClient, mockVirtualMachinesClient) realFetcher := &realFetcher{ log: log, azureSideFetcher: azureSideFetcher, + resourceFactory: mrf, } client := &client{fetcher: realFetcher, log: log} _, err := client.VMAllocationStatus(ctx)