diff --git a/cloud/scope/common_test.go b/cloud/scope/common_test.go index e888c359b1..47cefcd4ac 100644 --- a/cloud/scope/common_test.go +++ b/cloud/scope/common_test.go @@ -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 ( @@ -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()) - } -} diff --git a/cloud/scope/machine_test.go b/cloud/scope/machine_test.go index e5c9876fd6..caa6052dfa 100644 --- a/cloud/scope/machine_test.go +++ b/cloud/scope/machine_test.go @@ -17,6 +17,7 @@ limitations under the License. package scope import ( + "context" "errors" "testing" @@ -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, } @@ -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) diff --git a/cloud/scope/powervs_cluster.go b/cloud/scope/powervs_cluster.go index af619fd0f9..f9507940e0 100644 --- a/cloud/scope/powervs_cluster.go +++ b/cloud/scope/powervs_cluster.go @@ -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" ) @@ -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 @@ -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 diff --git a/cloud/scope/powervs_image.go b/cloud/scope/powervs_image.go index d90bfdf98b..3af66c15c2 100644 --- a/cloud/scope/powervs_image.go +++ b/cloud/scope/powervs_image.go @@ -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" ) @@ -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 @@ -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, } @@ -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 diff --git a/cloud/scope/powervs_machine.go b/cloud/scope/powervs_machine.go index 689f2374c6..de86885d4b 100644 --- a/cloud/scope/powervs_machine.go +++ b/cloud/scope/powervs_machine.go @@ -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" @@ -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" @@ -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 @@ -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, } diff --git a/cloud/scope/powervs_machine_test.go b/cloud/scope/powervs_machine_test.go index b52d9f6dea..fc36d9b67f 100644 --- a/cloud/scope/powervs_machine_test.go +++ b/cloud/scope/powervs_machine_test.go @@ -17,6 +17,7 @@ limitations under the License. package scope import ( + "context" "errors" "fmt" "testing" @@ -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) @@ -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" diff --git a/go.mod b/go.mod index ec84a65536..e477287474 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/go.sum b/go.sum index 9001d7039b..ac2dd5fca9 100644 --- a/go.sum +++ b/go.sum @@ -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= diff --git a/pkg/cloud/services/powervs/service.go b/pkg/cloud/services/powervs/service.go index 78fe3a3dd2..96d2d607ef 100644 --- a/pkg/cloud/services/powervs/service.go +++ b/pkg/cloud/services/powervs/service.go @@ -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{} @@ -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 diff --git a/pkg/endpoints/endpoints.go b/pkg/endpoints/endpoints.go index ef01d834f8..875ad3b59e 100644 --- a/pkg/endpoints/endpoints.go +++ b/pkg/endpoints/endpoints.go @@ -19,6 +19,7 @@ package endpoints import ( "errors" "net/url" + "regexp" "strings" ) @@ -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, "") +} diff --git a/pkg/endpoints/endpoints_test.go b/pkg/endpoints/endpoints_test.go index fb433e3f6d..50c2146d24 100644 --- a/pkg/endpoints/endpoints_test.go +++ b/pkg/endpoints/endpoints_test.go @@ -153,3 +153,136 @@ func TestParseFlags(t *testing.T) { }) } } + +func TestFetchVPCEndpoint(t *testing.T) { + testCases := []struct { + name string + region string + serviceEndpoint []ServiceEndpoint + expectedOutput string + }{ + { + name: "Return constructed endpoint", + region: "us-south", + serviceEndpoint: []ServiceEndpoint{}, + expectedOutput: "https://us-south.iaas.cloud.ibm.com/v1", + }, + { + name: "Return fetched endpoint", + region: "us-south", + serviceEndpoint: []ServiceEndpoint{ + { + ID: "vpc", + URL: "https://vpchost:8080", + Region: "us-south", + }, + }, + expectedOutput: "https://vpchost:8080", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + out := FetchVPCEndpoint(tc.region, tc.serviceEndpoint) + require.Equal(t, tc.expectedOutput, out) + }) + } +} + +func TestFetchPVSEndpoint(t *testing.T) { + testCases := []struct { + name string + region string + serviceEndpoint []ServiceEndpoint + expectedOutput string + }{ + { + name: "Return empty endpoint", + region: "us-south", + serviceEndpoint: []ServiceEndpoint{}, + expectedOutput: "", + }, + { + name: "Return fetched endpoint", + region: "us-south", + serviceEndpoint: []ServiceEndpoint{ + { + ID: "powervs", + URL: "https://powervshost:8080", + Region: "us-south", + }, + }, + expectedOutput: "https://powervshost:8080", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + out := FetchPVSEndpoint(tc.region, tc.serviceEndpoint) + require.Equal(t, tc.expectedOutput, out) + }) + } +} + +func TestFetchRCEndpoint(t *testing.T) { + testCases := []struct { + name string + serviceEndpoint []ServiceEndpoint + expectedOutput string + }{ + { + name: "Return empty endpoint", + serviceEndpoint: []ServiceEndpoint{}, + expectedOutput: "", + }, + { + name: "Return fetched endpoint", + serviceEndpoint: []ServiceEndpoint{ + { + ID: "rc", + URL: "https://rchost:8080", + Region: "us-south", + }, + }, + expectedOutput: "https://rchost:8080", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + out := FetchRCEndpoint(tc.serviceEndpoint) + require.Equal(t, tc.expectedOutput, out) + }) + } +} + +func TestCostructRegionFromZone(t *testing.T) { + testCases := []struct { + name string + zone string + expectedRegion string + }{ + { + name: "Return region us-south", + zone: "us-south", + expectedRegion: "us-south", + }, + { + name: "Return region dal", + zone: "dal12", + expectedRegion: "dal", + }, + { + name: "Return region eu-de", + zone: "eu-de-1", + expectedRegion: "eu-de", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + out := CostructRegionFromZone(tc.zone) + require.Equal(t, tc.expectedRegion, out) + }) + } +}