From e8a3e483431c2ebc071fb9c26992afd3814762be Mon Sep 17 00:00:00 2001 From: Karthik K N Date: Tue, 4 Jun 2024 12:04:50 +0530 Subject: [PATCH] Refactor PowerVSClusterscope function to faciliate UT integration --- cloud/scope/powervs_cluster.go | 213 +++++++----- cloud/scope/powervs_cluster_test.go | 109 +++++- controllers/ibmpowervscluster_controller.go | 3 + .../ibmpowervscluster_controller_test.go | 325 +++++++++++++----- pkg/cloud/services/powervs/service.go | 25 +- .../services/resourcecontroller/service.go | 2 +- 6 files changed, 488 insertions(+), 189 deletions(-) diff --git a/cloud/scope/powervs_cluster.go b/cloud/scope/powervs_cluster.go index c8b749b81..bbec94f2f 100644 --- a/cloud/scope/powervs_cluster.go +++ b/cloud/scope/powervs_cluster.go @@ -83,6 +83,19 @@ type PowerVSClusterScopeParams struct { Cluster *capiv1beta1.Cluster IBMPowerVSCluster *infrav1beta2.IBMPowerVSCluster ServiceEndpoint []endpoints.ServiceEndpoint + + // ClientFactory contains collection of functions to override actual client, which helps in testing. + ClientFactory +} + +// ClientFactory is collection of function used for overriding actual clients to help in testing. +type ClientFactory struct { + AuthenticatorFactory func() (core.Authenticator, error) + PowerVSClientFactory func() (powervs.PowerVS, error) + VPCClientFactory func() (vpc.Vpc, error) + TransitGatewayFactory func() (transitgateway.TransitGateway, error) + ResourceControllerFactory func() (resourcecontroller.ResourceController, error) + ResourceManagerFactory func() (resourcemanager.ResourceManager, error) } // PowerVSClusterScope defines a scope defined around a Power VS Cluster. @@ -90,7 +103,6 @@ type PowerVSClusterScope struct { logr.Logger Client client.Client patchHelper *patch.Helper - session *ibmpisession.IBMPISession IBMPowerVSClient powervs.PowerVS IBMVPCClient vpc.Vpc @@ -105,7 +117,7 @@ type PowerVSClusterScope struct { } // NewPowerVSClusterScope creates a new PowerVSClusterScope from the supplied parameters. -func NewPowerVSClusterScope(params PowerVSClusterScopeParams) (*PowerVSClusterScope, error) { //nolint:gocyclo +func NewPowerVSClusterScope(params PowerVSClusterScopeParams) (*PowerVSClusterScope, error) { if params.Client == nil { err := errors.New("failed to generate new scope as client is nil") return nil, err @@ -128,7 +140,20 @@ func NewPowerVSClusterScope(params PowerVSClusterScopeParams) (*PowerVSClusterSc return nil, err } - options := powervs.ServiceOptions{ + // if powervs.cluster.x-k8s.io/create-infra=true annotation is not set, create only powerVSClient. + if !CheckCreateInfraAnnotation(*params.IBMPowerVSCluster) { + return &PowerVSClusterScope{ + Logger: params.Logger, + Client: params.Client, + patchHelper: helper, + Cluster: params.Cluster, + IBMPowerVSCluster: params.IBMPowerVSCluster, + ServiceEndpoint: params.ServiceEndpoint, + }, nil + } + + // if powervs.cluster.x-k8s.io/create-infra=true annotation is set, create necessary clients. + piOptions := powervs.ServiceOptions{ IBMPIOptions: &ibmpisession.IBMPIOptions{ Debug: params.Logger.V(DEBUGLEVEL).Enabled(), }, @@ -164,76 +189,27 @@ func NewPowerVSClusterScope(params PowerVSClusterScopeParams) (*PowerVSClusterSc if err != nil { return nil, fmt.Errorf("failed to get resource instance: %w", err) } - options.Zone = *res.RegionID - options.CloudInstanceID = params.IBMPowerVSCluster.Spec.ServiceInstanceID + piOptions.Zone = *res.RegionID + piOptions.CloudInstanceID = params.IBMPowerVSCluster.Spec.ServiceInstanceID } else { - options.Zone = *params.IBMPowerVSCluster.Spec.Zone + piOptions.Zone = *params.IBMPowerVSCluster.Spec.Zone } - // Fetch the PowerVS service endpoint. - powerVSServiceEndpoint := endpoints.FetchEndpoints(string(endpoints.PowerVS), params.ServiceEndpoint) - if powerVSServiceEndpoint != "" { - params.Logger.V(3).Info("Overriding the default PowerVS endpoint", "powerVSEndpoint", powerVSServiceEndpoint) - options.IBMPIOptions.URL = powerVSServiceEndpoint - } - - // TODO(karhtik-k-n): may be optimize NewService to use the session created here - powerVSClient, err := powervs.NewService(options) - if err != nil { - return nil, fmt.Errorf("failed to create PowerVS client %w", err) - } - - auth, err := authenticator.GetAuthenticator() + // Get the authenticator. + auth, err := params.getAuthenticator() if err != nil { return nil, fmt.Errorf("failed to create authenticator %w", err) } - account, err := utils.GetAccount(auth) - if err != nil { - return nil, fmt.Errorf("failed to get account details %w", err) - } + piOptions.Authenticator = auth - sessionOptions := &ibmpisession.IBMPIOptions{ - Authenticator: auth, - UserAccount: account, - Zone: options.Zone, - Debug: params.Logger.V(DEBUGLEVEL).Enabled(), - } - if powerVSServiceEndpoint != "" { - sessionOptions.URL = powerVSServiceEndpoint - } - session, err := ibmpisession.NewIBMPISession(sessionOptions) + // Create PowerVS client. + powerVSClient, err := params.getPowerVSClient(piOptions) if err != nil { - return nil, fmt.Errorf("failed to get PowerVS session %w", err) - } - - // if powervs.cluster.x-k8s.io/create-infra=true annotation is not set, create only powerVSClient. - if !CheckCreateInfraAnnotation(*params.IBMPowerVSCluster) { - return &PowerVSClusterScope{ - session: session, - Logger: params.Logger, - Client: params.Client, - patchHelper: helper, - Cluster: params.Cluster, - IBMPowerVSCluster: params.IBMPowerVSCluster, - ServiceEndpoint: params.ServiceEndpoint, - IBMPowerVSClient: powerVSClient, - }, nil - } - - // if powervs.cluster.x-k8s.io/create-infra=true annotation is set, create necessary clients. - if params.IBMPowerVSCluster.Spec.VPC == nil || params.IBMPowerVSCluster.Spec.VPC.Region == nil { - return nil, fmt.Errorf("failed to create VPC client as VPC info is nil") - } - - if params.Logger.V(DEBUGLEVEL).Enabled() { - core.SetLoggingLevel(core.LevelDebug) + return nil, fmt.Errorf("failed to create PowerVS client %w", err) } - // Fetch the VPC service endpoint. - svcEndpoint := endpoints.FetchVPCEndpoint(*params.IBMPowerVSCluster.Spec.VPC.Region, params.ServiceEndpoint) - // Create VPC client. - vpcClient, err := vpc.NewService(svcEndpoint) + vpcClient, err := params.getVPCClient() if err != nil { return nil, fmt.Errorf("failed to create VPC client: %w", err) } @@ -242,14 +218,8 @@ func NewPowerVSClusterScope(params PowerVSClusterScopeParams) (*PowerVSClusterSc tgOptions := &tgapiv1.TransitGatewayApisV1Options{ Authenticator: auth, } - // Fetch the TransitGateway service endpoint. - tgServiceEndpoint := endpoints.FetchEndpoints(string(endpoints.TransitGateway), params.ServiceEndpoint) - if tgServiceEndpoint != "" { - params.Logger.V(3).Info("Overriding the default TransitGateway endpoint", "transitGatewayEndpoint", tgServiceEndpoint) - tgOptions.URL = tgServiceEndpoint - } - tgClient, err := transitgateway.NewService(tgOptions) + tgClient, err := params.getTransitGatewayClient(tgOptions) if err != nil { return nil, fmt.Errorf("failed to create tranist gateway client: %w", err) } @@ -260,13 +230,8 @@ func NewPowerVSClusterScope(params PowerVSClusterScopeParams) (*PowerVSClusterSc Authenticator: auth, }, } - // Fetch the resource controller endpoint. - rcEndpoint := endpoints.FetchEndpoints(string(endpoints.RC), params.ServiceEndpoint) - if rcEndpoint != "" { - serviceOption.URL = rcEndpoint - params.Logger.V(3).Info("Overriding the default resource controller endpoint", "ResourceControllerEndpoint", rcEndpoint) - } - resourceClient, err := resourcecontroller.NewService(serviceOption) + + resourceClient, err := params.getResourceControllerClient(serviceOption) if err != nil { return nil, fmt.Errorf("failed to create resource controller client: %w", err) } @@ -276,19 +241,12 @@ func NewPowerVSClusterScope(params PowerVSClusterScopeParams) (*PowerVSClusterSc Authenticator: auth, } - rmEndpoint := endpoints.FetchEndpoints(string(endpoints.RM), params.ServiceEndpoint) - if rmEndpoint != "" { - rcManagerOptions.URL = rmEndpoint - params.Logger.V(3).Info("Overriding the default resource manager endpoint", "ResourceManagerEndpoint", rmEndpoint) - } - - rmClient, err := resourcemanager.NewService(rcManagerOptions) + rmClient, err := params.getResourceManagerClient(rcManagerOptions) if err != nil { return nil, fmt.Errorf("failed to create resource manager client: %w", err) } clusterScope := &PowerVSClusterScope{ - session: session, Logger: params.Logger, Client: params.Client, patchHelper: helper, @@ -304,6 +262,81 @@ func NewPowerVSClusterScope(params PowerVSClusterScopeParams) (*PowerVSClusterSc return clusterScope, nil } +func (params PowerVSClusterScopeParams) getAuthenticator() (core.Authenticator, error) { + if params.AuthenticatorFactory != nil { + return params.AuthenticatorFactory() + } + return authenticator.GetAuthenticator() +} + +func (params PowerVSClusterScopeParams) getPowerVSClient(options powervs.ServiceOptions) (powervs.PowerVS, error) { + if params.PowerVSClientFactory != nil { + return params.PowerVSClientFactory() + } + + // Fetch the PowerVS service endpoint. + powerVSServiceEndpoint := endpoints.FetchEndpoints(string(endpoints.PowerVS), params.ServiceEndpoint) + if powerVSServiceEndpoint != "" { + params.Logger.V(3).Info("Overriding the default PowerVS endpoint", "powerVSEndpoint", powerVSServiceEndpoint) + options.URL = powerVSServiceEndpoint + } + return powervs.NewService(options) +} + +func (params PowerVSClusterScopeParams) getVPCClient() (vpc.Vpc, error) { + if params.Logger.V(DEBUGLEVEL).Enabled() { + core.SetLoggingLevel(core.LevelDebug) + } + if params.VPCClientFactory != nil { + return params.VPCClientFactory() + } + if params.IBMPowerVSCluster.Spec.VPC == nil || params.IBMPowerVSCluster.Spec.VPC.Region == nil { + return nil, fmt.Errorf("failed to create VPC client as VPC info is nil") + } + // Fetch the VPC service endpoint. + svcEndpoint := endpoints.FetchVPCEndpoint(*params.IBMPowerVSCluster.Spec.VPC.Region, params.ServiceEndpoint) + return vpc.NewService(svcEndpoint) +} + +func (params PowerVSClusterScopeParams) getTransitGatewayClient(options *tgapiv1.TransitGatewayApisV1Options) (transitgateway.TransitGateway, error) { + if params.TransitGatewayFactory != nil { + return params.TransitGatewayFactory() + } + // Fetch the TransitGateway service endpoint. + tgServiceEndpoint := endpoints.FetchEndpoints(string(endpoints.TransitGateway), params.ServiceEndpoint) + if tgServiceEndpoint != "" { + params.Logger.V(3).Info("Overriding the default TransitGateway endpoint", "transitGatewayEndpoint", tgServiceEndpoint) + options.URL = tgServiceEndpoint + } + return transitgateway.NewService(options) +} + +func (params PowerVSClusterScopeParams) getResourceControllerClient(options resourcecontroller.ServiceOptions) (resourcecontroller.ResourceController, error) { + if params.ResourceControllerFactory != nil { + return params.ResourceControllerFactory() + } + // Fetch the resource controller endpoint. + rcEndpoint := endpoints.FetchEndpoints(string(endpoints.RC), params.ServiceEndpoint) + if rcEndpoint != "" { + options.URL = rcEndpoint + params.Logger.V(3).Info("Overriding the default resource controller endpoint", "ResourceControllerEndpoint", rcEndpoint) + } + return resourcecontroller.NewService(options) +} + +func (params PowerVSClusterScopeParams) getResourceManagerClient(options *resourcemanagerv2.ResourceManagerV2Options) (resourcemanager.ResourceManager, error) { + if params.ResourceManagerFactory != nil { + return params.ResourceManagerFactory() + } + // Fetch the resource manager endpoint. + rmEndpoint := endpoints.FetchEndpoints(string(endpoints.RM), params.ServiceEndpoint) + if rmEndpoint != "" { + options.URL = rmEndpoint + params.Logger.V(3).Info("Overriding the default resource manager endpoint", "ResourceManagerEndpoint", rmEndpoint) + } + return resourcemanager.NewService(options) +} + // PatchObject persists the cluster configuration and status. func (s *PowerVSClusterScope) PatchObject() error { return s.patchHelper.Patch(context.TODO(), s.IBMPowerVSCluster) @@ -2183,8 +2216,18 @@ func (s *PowerVSClusterScope) fetchResourceGroupID() (string, error) { return "", fmt.Errorf("resource group name is not set") } + auth, err := authenticator.GetAuthenticator() + if err != nil { + return "", err + } + + account, err := utils.GetAccount(auth) + if err != nil { + return "", err + } + resourceGroup := s.ResourceGroup().Name - rmv2ListResourceGroupOpt := resourcemanagerv2.ListResourceGroupsOptions{Name: resourceGroup, AccountID: &s.session.Options.UserAccount} + rmv2ListResourceGroupOpt := resourcemanagerv2.ListResourceGroupsOptions{Name: resourceGroup, AccountID: &account} resourceGroupListResult, _, err := s.ResourceManagerClient.ListResourceGroups(&rmv2ListResourceGroupOpt) if err != nil { return "", err diff --git a/cloud/scope/powervs_cluster_test.go b/cloud/scope/powervs_cluster_test.go index 51636da2c..715fb1c65 100644 --- a/cloud/scope/powervs_cluster_test.go +++ b/cloud/scope/powervs_cluster_test.go @@ -19,19 +19,35 @@ package scope import ( "testing" + "github.com/IBM/go-sdk-core/v5/core" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + + infrav1beta2 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta2" + capiv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1" + + "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/powervs" + "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/resourcecontroller" + "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/resourcemanager" + "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/transitgateway" + "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/vpc" + . "github.com/onsi/gomega" ) func TestNewPowerVSClusterScope(t *testing.T) { testCases := []struct { - name string - params PowerVSClusterScopeParams + name string + params PowerVSClusterScopeParams + expectError bool }{ { name: "Error when Client in nil", params: PowerVSClusterScopeParams{ Client: nil, }, + expectError: true, }, { name: "Error when Cluster in nil", @@ -39,6 +55,7 @@ func TestNewPowerVSClusterScope(t *testing.T) { Client: testEnv.Client, Cluster: nil, }, + expectError: true, }, { name: "Error when IBMPowerVSCluster is nil", @@ -47,16 +64,80 @@ func TestNewPowerVSClusterScope(t *testing.T) { Cluster: newCluster(clusterName), IBMPowerVSCluster: nil, }, + expectError: true, + }, + { + name: "Successfully create cluster scope when create infra annotation is not set", + params: PowerVSClusterScopeParams{ + Client: testEnv.Client, + Cluster: newCluster(clusterName), + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "powervs-test-", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: capiv1beta1.GroupVersion.String(), + Kind: "Cluster", + Name: "capi-test", + UID: "1", + }}}, + Spec: infrav1beta2.IBMPowerVSClusterSpec{Zone: ptr.To("zone")}, + }, + ClientFactory: ClientFactory{ + AuthenticatorFactory: func() (core.Authenticator, error) { + return nil, nil + }, + PowerVSClientFactory: func() (powervs.PowerVS, error) { + return nil, nil + }, + }, + }, + expectError: false, + }, + { + name: "Successfully create cluster scope when create infra annotation is set", + params: PowerVSClusterScopeParams{ + Client: testEnv.Client, + Cluster: newCluster(clusterName), + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"powervs.cluster.x-k8s.io/create-infra": "true"}, + GenerateName: "powervs-test-", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: capiv1beta1.GroupVersion.String(), + Kind: "Cluster", + Name: "capi-test", + UID: "1", + }}}, + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + Zone: ptr.To("zone"), + VPC: &infrav1beta2.VPCResourceReference{Region: ptr.To("eu-gb")}, + }, + }, + ClientFactory: ClientFactory{ + AuthenticatorFactory: func() (core.Authenticator, error) { + return nil, nil + }, + PowerVSClientFactory: func() (powervs.PowerVS, error) { + return nil, nil + }, + VPCClientFactory: func() (vpc.Vpc, error) { + return nil, nil + }, + TransitGatewayFactory: func() (transitgateway.TransitGateway, error) { + return nil, nil + }, + ResourceControllerFactory: func() (resourcecontroller.ResourceController, error) { + return nil, nil + }, + ResourceManagerFactory: func() (resourcemanager.ResourceManager, error) { + return nil, nil + }, + }, + }, + expectError: false, }, - //TODO: Fix and add more tests - //{ - // name: "Failed to get authenticator", - // params: PowerVSClusterScopeParams{ - // Client: testEnv.Client, - // Cluster: newCluster(clusterName), - // IBMPowerVSCluster: newPowerVSCluster(clusterName), - // }, - // }, } for _, tc := range testCases { g := NewWithT(t) @@ -64,7 +145,11 @@ func TestNewPowerVSClusterScope(t *testing.T) { _, err := NewPowerVSClusterScope(tc.params) // Note: only error/failure cases covered // TO-DO: cover success cases - g.Expect(err).To(Not(BeNil())) + if tc.expectError { + g.Expect(err).To(Not(BeNil())) + } else { + g.Expect(err).To(BeNil()) + } }) } } diff --git a/controllers/ibmpowervscluster_controller.go b/controllers/ibmpowervscluster_controller.go index 5c612e60a..de90429ef 100644 --- a/controllers/ibmpowervscluster_controller.go +++ b/controllers/ibmpowervscluster_controller.go @@ -53,6 +53,8 @@ type IBMPowerVSClusterReconciler struct { Recorder record.EventRecorder ServiceEndpoint []endpoints.ServiceEndpoint Scheme *runtime.Scheme + + ClientFactory scope.ClientFactory } // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=ibmpowervsclusters,verbs=get;list;watch;create;update;patch;delete @@ -90,6 +92,7 @@ func (r *IBMPowerVSClusterReconciler) Reconcile(ctx context.Context, req ctrl.Re Cluster: cluster, IBMPowerVSCluster: ibmCluster, ServiceEndpoint: r.ServiceEndpoint, + ClientFactory: r.ClientFactory, }) if err != nil { diff --git a/controllers/ibmpowervscluster_controller_test.go b/controllers/ibmpowervscluster_controller_test.go index 67dde4d61..efc13ae78 100644 --- a/controllers/ibmpowervscluster_controller_test.go +++ b/controllers/ibmpowervscluster_controller_test.go @@ -21,111 +21,274 @@ import ( "testing" "time" + "github.com/IBM/go-sdk-core/v5/core" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" "k8s.io/klog/v2" "k8s.io/utils/ptr" - capiv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/cluster-api/util" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + capiv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/util" + infrav1beta2 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta2" "sigs.k8s.io/cluster-api-provider-ibmcloud/cloud/scope" + "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/powervs" . "github.com/onsi/gomega" ) func TestIBMPowerVSClusterReconciler_Reconcile(t *testing.T) { - testCases := []struct { - name string - powervsCluster *infrav1beta2.IBMPowerVSCluster - ownerCluster *capiv1beta1.Cluster - expectError bool - }{ - { - name: "Should fail Reconcile if owner cluster not found", - powervsCluster: &infrav1beta2.IBMPowerVSCluster{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "powervs-test-", - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: capiv1beta1.GroupVersion.String(), - Kind: "Cluster", - Name: "capi-test", - UID: "1", - }}}, - Spec: infrav1beta2.IBMPowerVSClusterSpec{ServiceInstanceID: "foo"}}, - expectError: true, - }, - { - name: "Should not reconcile if owner reference is not set", - powervsCluster: &infrav1beta2.IBMPowerVSCluster{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "powervs-test-"}, - Spec: infrav1beta2.IBMPowerVSClusterSpec{ - ServiceInstanceID: "foo"}}, - expectError: false, - }, - { - name: "Should Reconcile successfully if no IBMPowerVSCluster found", - expectError: false, - }, - } + t.Run("Should fail Reconcile if owner cluster not found", func(t *testing.T) { + g := NewWithT(t) - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - g := NewWithT(t) - reconciler := &IBMPowerVSClusterReconciler{ - Client: testEnv.Client, - } + ns, err := testEnv.CreateNamespace(ctx, fmt.Sprintf("namespace-%s", util.RandomString(5))) + g.Expect(err).To(BeNil()) - ns, err := testEnv.CreateNamespace(ctx, fmt.Sprintf("namespace-%s", util.RandomString(5))) - g.Expect(err).To(BeNil()) + powerVSCluster := &infrav1beta2.IBMPowerVSCluster{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "powervs-test-", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: capiv1beta1.GroupVersion.String(), + Kind: "Cluster", + Name: "capi-test", + UID: "1", + }}}, + Spec: infrav1beta2.IBMPowerVSClusterSpec{ServiceInstanceID: "foo"}, + } + + createCluster(g, powerVSCluster, ns.Name) + defer cleanupCluster(g, powerVSCluster, ns) + + reconciler := &IBMPowerVSClusterReconciler{ + Client: testEnv.Client, + } + _, err = reconciler.Reconcile(ctx, ctrl.Request{ + NamespacedName: client.ObjectKey{ + Namespace: powerVSCluster.Namespace, + Name: powerVSCluster.Name, + }, + }) + + g.Expect(err).ToNot(BeNil()) + }) + + t.Run("Should not reconcile if owner reference is not set", func(t *testing.T) { + g := NewWithT(t) + + ns, err := testEnv.CreateNamespace(ctx, fmt.Sprintf("namespace-%s", util.RandomString(5))) + g.Expect(err).To(BeNil()) + + powerVSCluster := &infrav1beta2.IBMPowerVSCluster{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "powervs-test-"}, + Spec: infrav1beta2.IBMPowerVSClusterSpec{ServiceInstanceID: "foo"}, + } + + createCluster(g, powerVSCluster, ns.Name) + defer cleanupCluster(g, powerVSCluster, ns) + + reconciler := &IBMPowerVSClusterReconciler{ + Client: testEnv.Client, + } + _, err = reconciler.Reconcile(ctx, ctrl.Request{ + NamespacedName: client.ObjectKey{ + Namespace: powerVSCluster.Namespace, + Name: powerVSCluster.Name, + }, + }) + + g.Expect(err).To(BeNil()) + }) + + t.Run("Should Reconcile successfully if no IBMPowerVSCluster found", func(t *testing.T) { + g := NewWithT(t) + + ns, err := testEnv.CreateNamespace(ctx, fmt.Sprintf("namespace-%s", util.RandomString(5))) + g.Expect(err).To(BeNil()) + + reconciler := &IBMPowerVSClusterReconciler{ + Client: testEnv.Client, + } + _, err = reconciler.Reconcile(ctx, ctrl.Request{ + NamespacedName: client.ObjectKey{ + Namespace: ns.Name, + Name: "test-cluster", + }, + }) + + g.Expect(err).To(BeNil()) + }) - if tc.ownerCluster != nil { - tc.ownerCluster.Namespace = ns.Name - g.Expect(testEnv.Create(ctx, tc.ownerCluster)).To(Succeed()) - defer func(do ...client.Object) { - g.Expect(testEnv.Cleanup(ctx, do...)).To(Succeed()) - }(tc.ownerCluster) - tc.powervsCluster.OwnerReferences = []metav1.OwnerReference{ + t.Run("Error creating cluster scope", func(t *testing.T) { + g := NewWithT(t) + + ns, err := testEnv.CreateNamespace(ctx, fmt.Sprintf("namespace-%s", util.RandomString(5))) + g.Expect(err).To(BeNil()) + + powerVSCluster := &infrav1beta2.IBMPowerVSCluster{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "powervs-test-", + OwnerReferences: []metav1.OwnerReference{ { APIVersion: capiv1beta1.GroupVersion.String(), Kind: "Cluster", - Name: tc.ownerCluster.Name, + Name: "capi-test", UID: "1", - }, - } - } - createCluster(g, tc.powervsCluster, ns.Name) - defer cleanupCluster(g, tc.powervsCluster, ns) - - if tc.powervsCluster != nil { - _, err := reconciler.Reconcile(ctx, ctrl.Request{ - NamespacedName: client.ObjectKey{ - Namespace: tc.powervsCluster.Namespace, - Name: tc.powervsCluster.Name, - }, - }) - if tc.expectError { - g.Expect(err).ToNot(BeNil()) - } else { - g.Expect(err).To(BeNil()) - } - } else { - _, err := reconciler.Reconcile(ctx, ctrl.Request{ - NamespacedName: client.ObjectKey{ - Namespace: ns.Name, - Name: "test", - }, - }) - g.Expect(err).To(BeNil()) - } + }}}, + Spec: infrav1beta2.IBMPowerVSClusterSpec{ServiceInstanceID: "foo"}, + } + + createCluster(g, powerVSCluster, ns.Name) + defer cleanupCluster(g, powerVSCluster, ns) + + reconciler := &IBMPowerVSClusterReconciler{ + Client: testEnv.Client, + } + _, err = reconciler.Reconcile(ctx, ctrl.Request{ + NamespacedName: client.ObjectKey{ + Namespace: powerVSCluster.Namespace, + Name: powerVSCluster.Name, + }, }) - } + g.Expect(err).ToNot(BeNil()) + }) + + t.Run("Successfully reconcile", func(t *testing.T) { + g := NewWithT(t) + + ns, err := testEnv.CreateNamespace(ctx, fmt.Sprintf("namespace-%s", util.RandomString(5))) + g.Expect(err).To(BeNil()) + + powerVSCluster := &infrav1beta2.IBMPowerVSCluster{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "powervs-test-", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: capiv1beta1.GroupVersion.String(), + Kind: "Cluster", + Name: "capi-test", + UID: "1", + }}}, + Spec: infrav1beta2.IBMPowerVSClusterSpec{Zone: ptr.To("zone")}, + } + + ownerCluster := &capiv1beta1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "capi-test", + Namespace: ns.Name, + }, + } + + g.Expect(testEnv.Create(ctx, ownerCluster)).To(Succeed()) + defer func(obj ...client.Object) { + g.Expect(testEnv.Cleanup(ctx, obj...)).To(Succeed()) + }(ownerCluster) + + createCluster(g, powerVSCluster, ns.Name) + defer cleanupCluster(g, powerVSCluster, ns) + + reconciler := &IBMPowerVSClusterReconciler{ + Client: testEnv.Client, + ClientFactory: scope.ClientFactory{ + PowerVSClientFactory: func() (powervs.PowerVS, error) { + return nil, nil + }, + AuthenticatorFactory: func() (core.Authenticator, error) { + return nil, nil + }, + }, + } + _, err = reconciler.Reconcile(ctx, ctrl.Request{ + NamespacedName: client.ObjectKey{ + Namespace: powerVSCluster.Namespace, + Name: powerVSCluster.Name, + }, + }) + g.Expect(err).To(BeNil()) + + ibmPowerVSCluster := &infrav1beta2.IBMPowerVSCluster{} + g.Eventually(func(gomega Gomega) { + gomega.Expect(testEnv.Client.Get(ctx, client.ObjectKey{ + Name: powerVSCluster.GetName(), + Namespace: powerVSCluster.GetNamespace(), + }, ibmPowerVSCluster)).To(Succeed()) + gomega.Expect(len(ibmPowerVSCluster.Finalizers)).To(Equal(1)) + }).Should(Succeed()) + }) + + t.Run("Successfully call reconcile delete", func(t *testing.T) { + g := NewWithT(t) + + ns, err := testEnv.CreateNamespace(ctx, fmt.Sprintf("namespace-%s", util.RandomString(5))) + g.Expect(err).To(BeNil()) + + powerVSCluster := &infrav1beta2.IBMPowerVSCluster{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "powervs-test-", + Finalizers: []string{"ibmpowervscluster.infrastructure.cluster.x-k8s.io"}, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: capiv1beta1.GroupVersion.String(), + Kind: "Cluster", + Name: "capi-test", + UID: "1", + }}}, + Spec: infrav1beta2.IBMPowerVSClusterSpec{Zone: ptr.To("zone")}, + } + + ownerCluster := &capiv1beta1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "capi-test", + Namespace: ns.Name, + }, + } + + g.Expect(testEnv.Create(ctx, ownerCluster)).To(Succeed()) + defer func(obj ...client.Object) { + g.Expect(testEnv.Cleanup(ctx, obj...)).To(Succeed()) + }(ownerCluster) + + createCluster(g, powerVSCluster, ns.Name) + defer cleanupCluster(g, powerVSCluster, ns) + + g.Expect(testEnv.Delete(ctx, powerVSCluster)).To(Succeed()) + ibmPowerVSCluster := &infrav1beta2.IBMPowerVSCluster{} + + g.Eventually(func() bool { + err := testEnv.Client.Get(ctx, client.ObjectKey{ + Name: powerVSCluster.GetName(), + Namespace: powerVSCluster.GetNamespace(), + }, ibmPowerVSCluster) + g.Expect(err).To(BeNil()) + return ibmPowerVSCluster.DeletionTimestamp.IsZero() == false + }, 10*time.Second).Should(BeTrue(), "Eventually failed while checking delete timestamp") + + reconciler := &IBMPowerVSClusterReconciler{ + Client: testEnv.Client, + ClientFactory: scope.ClientFactory{ + PowerVSClientFactory: func() (powervs.PowerVS, error) { + return nil, nil + }, + AuthenticatorFactory: func() (core.Authenticator, error) { + return nil, nil + }, + }, + } + _, err = reconciler.Reconcile(ctx, ctrl.Request{ + NamespacedName: client.ObjectKey{ + Namespace: powerVSCluster.Namespace, + Name: powerVSCluster.Name, + }, + }) + g.Expect(err).To(BeNil()) + }) } func TestIBMPowerVSClusterReconciler_reconcile(t *testing.T) { diff --git a/pkg/cloud/services/powervs/service.go b/pkg/cloud/services/powervs/service.go index 08f14928f..bda8b48de 100644 --- a/pkg/cloud/services/powervs/service.go +++ b/pkg/cloud/services/powervs/service.go @@ -45,22 +45,27 @@ type Service struct { // ServiceOptions holds the PowerVS Service Options specific information. type ServiceOptions struct { *ibmpisession.IBMPIOptions - CloudInstanceID string } -// NewService returns a new service for the Power VS api client. +// NewService returns a new service for the PowerVS api client. +// This will create only PowerVS session and actual clients can be created later by calling WithClients method. func NewService(options ServiceOptions) (PowerVS, error) { - auth, err := authenticator.GetAuthenticator() - if err != nil { - return nil, err + if options.Authenticator == nil { + auth, err := authenticator.GetAuthenticator() + if err != nil { + return nil, err + } + options.Authenticator = auth } - options.Authenticator = auth - account, err := utils.GetAccount(auth) - if err != nil { - return nil, err + if options.UserAccount == "" { + account, err := utils.GetAccount(options.Authenticator) + if err != nil { + return nil, err + } + options.IBMPIOptions.UserAccount = account } - options.IBMPIOptions.UserAccount = account + session, err := ibmpisession.NewIBMPISession(options.IBMPIOptions) if err != nil { return nil, err diff --git a/pkg/cloud/services/resourcecontroller/service.go b/pkg/cloud/services/resourcecontroller/service.go index df67ac7c0..a3028ea3d 100644 --- a/pkg/cloud/services/resourcecontroller/service.go +++ b/pkg/cloud/services/resourcecontroller/service.go @@ -197,7 +197,7 @@ func (s *Service) CreateResourceKey(options *resourcecontrollerv2.CreateResource } // NewService returns a new service for the IBM Cloud Resource Controller api client. -func NewService(options ServiceOptions) (*Service, error) { +func NewService(options ServiceOptions) (ResourceController, error) { if options.ResourceControllerV2Options == nil { options.ResourceControllerV2Options = &resourcecontrollerv2.ResourceControllerV2Options{} }