Skip to content

Commit

Permalink
Minor UT coverage improvements
Browse files Browse the repository at this point in the history
Signed-off-by: Prajyot-Parab <[email protected]>
  • Loading branch information
Prajyot-Parab committed Jul 14, 2022
1 parent 28d2a96 commit fe5d220
Show file tree
Hide file tree
Showing 11 changed files with 204 additions and 122 deletions.
16 changes: 0 additions & 16 deletions cloud/scope/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,8 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
capiv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/controller-runtime/pkg/client"

infrav1beta1 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta1"

. "github.com/onsi/gomega"
)

const (
Expand Down Expand Up @@ -91,16 +88,3 @@ func newBootstrapSecret(clusterName, machineName string) *corev1.Secret {
},
}
}

func createObject(g *WithT, obj client.Object, namespace string) {
if obj.DeepCopyObject() != nil {
obj.SetNamespace(namespace)
g.Expect(testEnv.Create(ctx, obj)).To(Succeed())
}
}

func cleanupObject(g *WithT, obj client.Object) {
if obj.DeepCopyObject() != nil {
g.Expect(testEnv.Cleanup(ctx, obj)).To(Succeed())
}
}
34 changes: 20 additions & 14 deletions cloud/scope/machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package scope

import (
"context"
"errors"
"testing"

Expand Down Expand Up @@ -145,20 +146,6 @@ func TestCreateMachine(t *testing.T) {
t.Run("Should create Machine", func(t *testing.T) {
scope := setupMachineScope(clusterName, machineName, mockvpc)
scope.IBMVPCMachine.Spec = vpcMachine.Spec
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
capiv1beta1.ClusterLabelName: scope.Cluster.Name,
},
Name: scope.Machine.Name,
Namespace: "default",
},
Data: map[string][]byte{
"value": []byte("user data"),
},
}
createObject(g, secret, "default")
defer cleanupObject(g, secret)
instance := &vpcv1.Instance{
Name: &scope.Machine.Name,
}
Expand Down Expand Up @@ -203,6 +190,25 @@ func TestCreateMachine(t *testing.T) {
g.Expect(err).To(Not(BeNil()))
})

t.Run("Failed to retrieve bootstrap data, secret value key is missing", func(t *testing.T) {
scope := setupMachineScope(clusterName, machineName, mockvpc)
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
capiv1beta1.ClusterLabelName: clusterName,
},
Name: machineName,
Namespace: "default",
},
Data: map[string][]byte{
"val": []byte("user data"),
}}
g.Expect(scope.Client.Update(context.Background(), secret)).To(Succeed())
mockvpc.EXPECT().ListInstances(gomock.AssignableToTypeOf(listInstancesOptions)).Return(instanceCollection, detailedResponse, nil)
_, err := scope.CreateMachine()
g.Expect(err).To(Not(BeNil()))
})

t.Run("Failed to create instance", func(t *testing.T) {
scope := setupMachineScope(clusterName, machineName, mockvpc)
mockvpc.EXPECT().ListInstances(gomock.AssignableToTypeOf(listInstancesOptions)).Return(instanceCollection, detailedResponse, nil)
Expand Down
28 changes: 3 additions & 25 deletions cloud/scope/powervs_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,14 @@ import (
"github.com/IBM/platform-services-go-sdk/resourcecontrollerv2"
"github.com/go-logr/logr"
"github.com/pkg/errors"
utils "github.com/ppc64le-cloud/powervs-utils"
"k8s.io/klog/v2/klogr"
capiv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/controller-runtime/pkg/client"

infrav1beta1 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/authenticator"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/powervs"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/resourcecontroller"
servicesutils "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/utils"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/endpoints"
)

Expand Down Expand Up @@ -101,18 +98,6 @@ func NewPowerVSClusterScope(params PowerVSClusterScopeParams) (scope *PowerVSClu

spec := params.IBMPowerVSCluster.Spec

auth, err := authenticator.GetAuthenticator()
if err != nil {
err = errors.Wrap(err, "failed to get authenticator")
return
}

account, err := servicesutils.GetAccount(auth)
if err != nil {
err = errors.Wrap(err, "failed to get account")
return
}

rc, err := resourcecontroller.NewService(resourcecontroller.ServiceOptions{})
if err != nil {
return
Expand All @@ -135,22 +120,15 @@ func NewPowerVSClusterScope(params PowerVSClusterScopeParams) (scope *PowerVSClu
return
}

region, err := utils.GetRegion(*res.RegionID)
if err != nil {
err = errors.Wrap(err, "failed to get region")
return
}

options := powervs.ServiceOptions{
IBMPIOptions: &ibmpisession.IBMPIOptions{
Debug: params.Logger.V(DEBUGLEVEL).Enabled(),
UserAccount: account,
Region: region,
Zone: *res.RegionID,
Debug: params.Logger.V(DEBUGLEVEL).Enabled(),
Zone: *res.RegionID,
},
CloudInstanceID: spec.ServiceInstanceID,
}

region := endpoints.CostructRegionFromZone(*res.RegionID)
// Fetch the service endpoint.
if svcEndpoint := endpoints.FetchPVSEndpoint(region, params.ServiceEndpoint); svcEndpoint != "" {
options.IBMPIOptions.URL = svcEndpoint
Expand Down
28 changes: 3 additions & 25 deletions cloud/scope/powervs_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,13 @@ import (
"github.com/IBM/platform-services-go-sdk/resourcecontrollerv2"
"github.com/go-logr/logr"
"github.com/pkg/errors"
utils "github.com/ppc64le-cloud/powervs-utils"
"k8s.io/klog/v2/klogr"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/controller-runtime/pkg/client"

infrav1beta1 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/authenticator"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/powervs"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/resourcecontroller"
servicesutils "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/utils"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/endpoints"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/record"
)
Expand Down Expand Up @@ -94,18 +91,6 @@ func NewPowerVSImageScope(params PowerVSImageScopeParams) (scope *PowerVSImageSc

spec := params.IBMPowerVSImage.Spec

auth, err := authenticator.GetAuthenticator()
if err != nil {
err = errors.Wrap(err, "failed to get authenticator")
return
}

account, err := servicesutils.GetAccount(auth)
if err != nil {
err = errors.Wrap(err, "failed to get account")
return
}

rc, err := resourcecontroller.NewService(resourcecontroller.ServiceOptions{})
if err != nil {
return
Expand All @@ -128,18 +113,10 @@ func NewPowerVSImageScope(params PowerVSImageScopeParams) (scope *PowerVSImageSc
return
}

region, err := utils.GetRegion(*res.RegionID)
if err != nil {
err = errors.Wrap(err, "failed to get region")
return
}

options := powervs.ServiceOptions{
IBMPIOptions: &ibmpisession.IBMPIOptions{
Debug: params.Logger.V(DEBUGLEVEL).Enabled(),
UserAccount: account,
Region: region,
Zone: *res.RegionID,
Debug: params.Logger.V(DEBUGLEVEL).Enabled(),
Zone: *res.RegionID,
},
CloudInstanceID: spec.ServiceInstanceID,
}
Expand All @@ -150,6 +127,7 @@ func NewPowerVSImageScope(params PowerVSImageScopeParams) (scope *PowerVSImageSc
}
scope.IBMPowerVSCluster = params.IBMPowerVSCluster

region := endpoints.CostructRegionFromZone(*res.RegionID)
// Fetch the service endpoint.
if svcEndpoint := endpoints.FetchPVSEndpoint(region, params.ServiceEndpoint); svcEndpoint != "" {
options.IBMPIOptions.URL = svcEndpoint
Expand Down
27 changes: 3 additions & 24 deletions cloud/scope/powervs_machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"github.com/IBM/platform-services-go-sdk/resourcecontrollerv2"
"github.com/go-logr/logr"
"github.com/pkg/errors"
powerVSUtils "github.com/ppc64le-cloud/powervs-utils"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/cache"
Expand All @@ -42,10 +41,8 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

infrav1beta1 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/authenticator"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/powervs"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/resourcecontroller"
servicesutils "sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/utils"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/endpoints"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/options"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/record"
Expand Down Expand Up @@ -124,18 +121,6 @@ func NewPowerVSMachineScope(params PowerVSMachineScopeParams) (scope *PowerVSMac

m := params.IBMPowerVSMachine

auth, err := authenticator.GetAuthenticator()
if err != nil {
err = errors.Wrap(err, "failed to get authenticator")
return
}

account, err := servicesutils.GetAccount(auth)
if err != nil {
err = errors.Wrap(err, "failed to get account")
return
}

rc, err := resourcecontroller.NewService(resourcecontroller.ServiceOptions{})
if err != nil {
return
Expand All @@ -158,20 +143,14 @@ func NewPowerVSMachineScope(params PowerVSMachineScopeParams) (scope *PowerVSMac
return
}

region, err := powerVSUtils.GetRegion(*res.RegionID)
if err != nil {
err = errors.Wrap(err, "failed to get region")
return
}
region := endpoints.CostructRegionFromZone(*res.RegionID)
scope.SetRegion(region)
scope.SetZone(*res.RegionID)

serviceOptions := powervs.ServiceOptions{
IBMPIOptions: &ibmpisession.IBMPIOptions{
Debug: params.Logger.V(DEBUGLEVEL).Enabled(),
UserAccount: account,
Region: region,
Zone: *res.RegionID,
Debug: params.Logger.V(DEBUGLEVEL).Enabled(),
Zone: *res.RegionID,
},
CloudInstanceID: m.Spec.ServiceInstanceID,
}
Expand Down
35 changes: 20 additions & 15 deletions cloud/scope/powervs_machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package scope

import (
"context"
"errors"
"fmt"
"testing"
Expand Down Expand Up @@ -223,21 +224,6 @@ func TestCreateMachinePVS(t *testing.T) {

t.Run("Should create Machine", func(t *testing.T) {
scope := setupPowerVSMachineScope(clusterName, machineName, core.StringPtr(pvsImage), core.StringPtr(pvsNetwork), true, mockpowervs)
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
capiv1beta1.ClusterLabelName: scope.Cluster.Name,
},
Name: scope.Machine.Name,
Namespace: "default",
},
Data: map[string][]byte{
"value": []byte("user data"),
},
}
createObject(g, secret, "default")
defer cleanupObject(g, secret)

mockpowervs.EXPECT().GetAllInstance().Return(pvmInstances, nil)
mockpowervs.EXPECT().GetAllImage().Return(images, nil)
mockpowervs.EXPECT().GetAllNetwork().Return(networks, nil)
Expand Down Expand Up @@ -281,6 +267,25 @@ func TestCreateMachinePVS(t *testing.T) {
g.Expect(err).To(Not(BeNil()))
})

t.Run("Failed to retrieve bootstrap data, secret value key is missing", func(t *testing.T) {
scope := setupPowerVSMachineScope(clusterName, machineName, core.StringPtr(pvsImage), core.StringPtr(pvsNetwork), true, mockpowervs)
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
capiv1beta1.ClusterLabelName: clusterName,
},
Name: machineName,
Namespace: "default",
},
Data: map[string][]byte{
"val": []byte("user data"),
}}
g.Expect(scope.Client.Update(context.Background(), secret)).To(Succeed())
mockpowervs.EXPECT().GetAllInstance().Return(pvmInstances, nil)
_, err := scope.CreateMachine()
g.Expect(err).To(Not(BeNil()))
})

t.Run("Invalid memory value", func(t *testing.T) {
scope := setupPowerVSMachineScope(clusterName, machineName, core.StringPtr(pvsImage), core.StringPtr(pvsNetwork), true, mockpowervs)
scope.IBMPowerVSMachine.Spec.Memory = "illegal"
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ require (
github.com/onsi/ginkgo v1.16.5
github.com/onsi/gomega v1.19.0
github.com/pkg/errors v0.9.1
github.com/ppc64le-cloud/powervs-utils v0.0.0-20220607051746-a593c549605b
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.8.0
golang.org/x/text v0.3.7
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -780,8 +780,6 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/posener/complete v1.1.1/go.mod h1:em0nMJCgc9GFtwrmVmEMR/ZL6WyhyjMBndrE9hABlRI=
github.com/posener/complete v1.2.3/go.mod h1:WZIdtGGp+qx0sLrYKtIRAruyNpv6hFCicSgv7Sy7s/s=
github.com/ppc64le-cloud/powervs-utils v0.0.0-20220607051746-a593c549605b h1:OljQqJCHS68jU3iKjhpGsD85OKOLCkApmAU3jLYMwIA=
github.com/ppc64le-cloud/powervs-utils v0.0.0-20220607051746-a593c549605b/go.mod h1:KImYgHmvBVtAczNhyDBDSN54PGIdz0+QiPVQMmObEQY=
github.com/pquerna/cachecontrol v0.0.0-20171018203845-0dec1b30a021/go.mod h1:prYjPmNq4d1NPVmpShWobRqXY3q7Vp+80DqgxxUrUIA=
github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw=
github.com/prometheus/client_golang v0.9.3/go.mod h1:/TN21ttK/J9q6uSwhBd54HahCDft0ttaMvbicHlPoso=
Expand Down
6 changes: 6 additions & 0 deletions pkg/cloud/services/powervs/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/IBM-Cloud/power-go-client/power/models"

"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/authenticator"
"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/utils"
)

var _ PowerVS = &Service{}
Expand Down Expand Up @@ -128,6 +129,11 @@ func NewService(options ServiceOptions) (PowerVS, error) {
return nil, err
}
options.Authenticator = auth
account, err := utils.GetAccount(auth)
if err != nil {
return nil, err
}
options.IBMPIOptions.UserAccount = account
session, err := ibmpisession.NewIBMPISession(options.IBMPIOptions)
if err != nil {
return nil, err
Expand Down
16 changes: 16 additions & 0 deletions pkg/endpoints/endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package endpoints
import (
"errors"
"net/url"
"regexp"
"strings"
)

Expand Down Expand Up @@ -142,3 +143,18 @@ func FetchRCEndpoint(serviceEndpoint []ServiceEndpoint) string {
}
return ""
}

// CostructRegionFromZone Calculate region based on location/zone.
func CostructRegionFromZone(zone string) string {
var regex string
if strings.Contains(zone, "-") {
// it's a region or AZ
regex = "-[0-9]+$"
} else {
// it's a datacenter
regex = "[0-9]+$"
}

reg, _ := regexp.Compile(regex)
return reg.ReplaceAllString(zone, "")
}
Loading

0 comments on commit fe5d220

Please sign in to comment.