Skip to content

Commit

Permalink
Cleanup code
Browse files Browse the repository at this point in the history
  • Loading branch information
kishen-v committed Jun 14, 2024
1 parent 68fc5fe commit 108d7b3
Show file tree
Hide file tree
Showing 34 changed files with 139 additions and 172 deletions.
4 changes: 1 addition & 3 deletions cmd/create/port/port.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
)

var (
network, ipaddress, description string
network, ipaddress, description, netID string
)

var Cmd = &cobra.Command{
Expand Down Expand Up @@ -66,8 +66,6 @@ var Cmd = &cobra.Command{
networkNames = append(networkNames, *net.Name)
}

var netID string

if utils.Contains(networkIDs, network) {
netID = network
} else if utils.Contains(networkNames, network) {
Expand Down
6 changes: 3 additions & 3 deletions cmd/dhcp-sync/dhcp-sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ var Cmd = &cobra.Command{

watcher, err := fsnotify.NewWatcher()
if err != nil {
klog.Fatal(err)
klog.Fatalf("cannot create a new fsNotify watcher: %v", err)
}
defer watcher.Close()

Expand All @@ -184,7 +184,7 @@ var Cmd = &cobra.Command{
if !ok {
return
}
klog.V(2).Infof("event: %v", event)
klog.V(2).Infof("Filesystem event: %v", event)
if event.Op&fsnotify.Write == fsnotify.Write {
klog.V(2).Infof("%s has been modified, proceeding to restart dhcpd service", event.Name)
exitcode, out, err := utils.RunCMD("systemctl", "restart", "dhcpd")
Expand All @@ -203,7 +203,7 @@ var Cmd = &cobra.Command{

err = watcher.Add(file)
if err != nil {
klog.Fatal(err)
klog.Fatalf("cannot sync DHCP server: %+v", err)
}
<-done
return nil
Expand Down
2 changes: 1 addition & 1 deletion cmd/get/peravailability/peravailability.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ var Cmd = &cobra.Command{
if err != nil {
return err
}
supportsPER := false
var supportsPER bool
for _, datacenter := range ret.Datacenters {
if datacenter.Capabilities[powerEdgeRouter] {
perEnabledRegions = append(perEnabledRegions, *datacenter.Location.Region)
Expand Down
8 changes: 3 additions & 5 deletions cmd/image/import/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pvsadm image import --help for information
# Set the API key or feed the --api-key commandline argument
export IBMCLOUD_API_KEY=<IBM_CLOUD_API_KEY>
# To Import the imge across the two different IBM account use accesskey and secretkey options
# To Import the image across the two different IBM account use accesskey and secretkey options
# To Import the image from public bucket use public-bucket option
Expand All @@ -97,10 +97,8 @@ pvsadm image import -n upstream-core-lon04 -b <BUCKETNAME> --object rhel-83-1003
return fmt.Errorf("--workspace-name or --workspace-id required")
}

case1 := pkg.ImageCMDOptions.AccessKey == "" && pkg.ImageCMDOptions.SecretKey != ""
case2 := pkg.ImageCMDOptions.AccessKey != "" && pkg.ImageCMDOptions.SecretKey == ""

if case1 || case2 {
// ensure that both, the AccessKey and SecretKey are either both set or unset
if (len(pkg.ImageCMDOptions.AccessKey) > 0) != (len(pkg.ImageCMDOptions.SecretKey) > 0) {
return fmt.Errorf("required both --accesskey and --secretkey values")
}
return nil
Expand Down
12 changes: 6 additions & 6 deletions cmd/image/sync/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ type InstanceItem struct {

// sync constants
const (
serviceType = "cloud-object-storage"
noOfcopyWorkers = 20
serviceType = "cloud-object-storage"
maxWorkers = 20
)

// Worker method to copy object from source bucket to target bucket
Expand Down Expand Up @@ -102,8 +102,8 @@ func calculateChannels(spec []pkg.Spec, instanceList []InstanceItem) (int, error
return 0, err
}

noOfTargets := len(item.Target)
totalChannelsForSrc := noOfTargets * len(selectedObjects)
numTargets := len(item.Target)
totalChannelsForSrc := numTargets * len(selectedObjects)
totalChannels = totalChannels + totalChannelsForSrc
}
return totalChannels, nil
Expand Down Expand Up @@ -186,8 +186,8 @@ func syncObjects(spec []pkg.Spec, instanceList []InstanceItem) error {
// Creating workers and channels
copyJobs := make(chan copyWorkload, totalChannels)
results := make(chan bool, totalChannels)
for w := 1; w <= noOfcopyWorkers; w++ {
go copyWorker(copyJobs, results, w)
for worker := 1; worker <= maxWorkers; worker++ {
go copyWorker(copyJobs, results, worker)
}

// Copy objects
Expand Down
44 changes: 22 additions & 22 deletions cmd/image/sync/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ import (

// Test case constants
const (
noOfSources = 3
noOfTargetsPerSource = 3
noOfObjects = 200
numSources = 3
numTargetsPerSource = 3
numObjects = 200
)

func TestCalculateChannels(t *testing.T) {
Expand All @@ -47,8 +47,8 @@ func TestCalculateChannels(t *testing.T) {
mockSyncClient := mocksync.NewMockSyncClient(mockCtrl)

// test case setup
mockSetBucketLocationConstraint(mockSyncClient, noOfSources, true, "")
mockSetSelectedObjects(mockSyncClient, noOfObjects, noOfSources)
mockSetBucketLocationConstraint(mockSyncClient, numSources, true, "")
mockSetSelectedObjects(mockSyncClient, numObjects, numSources)

// generating spec slice
spec := mockCreateSpec()
Expand All @@ -59,7 +59,7 @@ func TestCalculateChannels(t *testing.T) {
// test case verification section
totalChannels, err := calculateChannels(spec, instanceList)
require.NoError(t, err, "Error calculating channels")
assert.Equal(t, noOfObjects*noOfSources*noOfTargetsPerSource, totalChannels)
assert.Equal(t, numObjects*numSources*numTargetsPerSource, totalChannels)
})
}

Expand Down Expand Up @@ -129,11 +129,11 @@ func TestSync(t *testing.T) {
return mockCreateSpec()
},
setup: func(mockSyncClient *mocksync.MockSyncClient) {
mockSetBucketLocationConstraint(mockSyncClient, noOfSources, true, "")
mockSetSelectedObjects(mockSyncClient, noOfObjects, noOfSources)
mockSetSelectedObjects(mockSyncClient, noOfObjects, noOfSources)
mockSetBucketLocationConstraint(mockSyncClient, noOfSources*noOfTargetsPerSource, true, "")
mockSetCopyObjectToBucket(mockSyncClient, noOfObjects*noOfSources*noOfTargetsPerSource, "")
mockSetBucketLocationConstraint(mockSyncClient, numSources, true, "")
mockSetSelectedObjects(mockSyncClient, numObjects, numSources)
mockSetSelectedObjects(mockSyncClient, numObjects, numSources)
mockSetBucketLocationConstraint(mockSyncClient, numSources*numTargetsPerSource, true, "")
mockSetCopyObjectToBucket(mockSyncClient, numObjects*numSources*numTargetsPerSource, "")

},
expectedError: "",
Expand All @@ -152,10 +152,10 @@ func TestSync(t *testing.T) {
return mockCreateSpec()
},
setup: func(mockSyncClient *mocksync.MockSyncClient) {
mockSetBucketLocationConstraint(mockSyncClient, noOfSources, true, "")
mockSetSelectedObjects(mockSyncClient, 0, noOfSources)
mockSetSelectedObjects(mockSyncClient, 0, noOfSources)
mockSetBucketLocationConstraint(mockSyncClient, noOfSources*noOfTargetsPerSource, true, "")
mockSetBucketLocationConstraint(mockSyncClient, numSources, true, "")
mockSetSelectedObjects(mockSyncClient, 0, numSources)
mockSetSelectedObjects(mockSyncClient, 0, numSources)
mockSetBucketLocationConstraint(mockSyncClient, numSources*numTargetsPerSource, true, "")
mockSetCopyObjectToBucket(mockSyncClient, 0, "")

},
Expand Down Expand Up @@ -193,9 +193,9 @@ func TestSync(t *testing.T) {
return mockCreateSpec()
},
setup: func(mockSyncClient *mocksync.MockSyncClient) {
mockSetBucketLocationConstraint(mockSyncClient, noOfSources, true, "")
mockSetSelectedObjects(mockSyncClient, noOfObjects, noOfSources)
mockSetSelectedObjects(mockSyncClient, noOfObjects, 1)
mockSetBucketLocationConstraint(mockSyncClient, numSources, true, "")
mockSetSelectedObjects(mockSyncClient, numObjects, numSources)
mockSetSelectedObjects(mockSyncClient, numObjects, 1)
mockSetBucketLocationConstraint(mockSyncClient, 1, false, "Failed to verify bucket location constriant")
},
expectedError: "bucket location constraint verification failed",
Expand Down Expand Up @@ -234,11 +234,11 @@ func TestSync(t *testing.T) {

func mockCreateInstances(mockSyncClient *mocksync.MockSyncClient) []InstanceItem {
var instanceList []InstanceItem
for i := 0; i < noOfSources; i++ {
for i := 0; i < numSources; i++ {
instance := InstanceItem{}
instance.Source = mockSyncClient

for j := 0; j < noOfTargetsPerSource; j++ {
for j := 0; j < numTargetsPerSource; j++ {
instance.Target = append(instance.Target, mockSyncClient)
}
instanceList = append(instanceList, instance)
Expand All @@ -249,8 +249,8 @@ func mockCreateInstances(mockSyncClient *mocksync.MockSyncClient) []InstanceItem
func mockCreateSpec() []pkg.Spec {
klog.V(1).Info("STEP: Generating Spec")
specSlice := make([]pkg.Spec, 0)
for i := 0; i < noOfSources; i++ {
specSlice = append(specSlice, utils.GenerateSpec(noOfTargetsPerSource))
for i := 0; i < numSources; i++ {
specSlice = append(specSlice, utils.GenerateSpec(numTargetsPerSource))
}
return specSlice
}
Expand Down
14 changes: 6 additions & 8 deletions cmd/image/upload/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ import (

const (
ServiceType = "cloud-object-storage"
UseExistingPromptMessage = "Would You Like to use Available COS Instance for creating bucket?"
CreatePromptMessage = "Would you like to create new COS Instance?"
UseExistingPromptMessage = "Would you like to use an existing COS Instance for creating bucket?"
CreatePromptMessage = "Would you like to create a new COS Instance?"
ResourceGroupAPIRegion = "global"
)

Expand Down Expand Up @@ -68,18 +68,16 @@ pvsadm image upload --bucket bucket1320 -f centos-8-latest.ova.gz --bucket-regio
return fmt.Errorf("image upload in test/staging env storage bucket is not supported")
}

case1 := pkg.ImageCMDOptions.AccessKey == "" && pkg.ImageCMDOptions.SecretKey != ""
case2 := pkg.ImageCMDOptions.AccessKey != "" && pkg.ImageCMDOptions.SecretKey == ""

if case1 || case2 {
// ensure that both, the AccessKey and SecretKey are set.
if (len(pkg.ImageCMDOptions.AccessKey) > 0) != (len(pkg.ImageCMDOptions.SecretKey) > 0) {
return fmt.Errorf("required both --accesskey and --secretkey values")
}

return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
var s3Cli *client.S3Client
var bucketExists bool = false
var bucketExists bool
var apikey string = pkg.Options.APIKey
opt := pkg.ImageCMDOptions

Expand All @@ -88,7 +86,7 @@ pvsadm image upload --bucket bucket1320 -f centos-8-latest.ova.gz --bucket-regio
}

if pkg.ImageCMDOptions.AccessKey != "" && pkg.ImageCMDOptions.SecretKey != "" {
s3Cli, err := client.NewS3Clientwithkeys(pkg.ImageCMDOptions.AccessKey, pkg.ImageCMDOptions.SecretKey, opt.Region)
s3Cli, err := client.NewS3ClientWithKeys(pkg.ImageCMDOptions.AccessKey, pkg.ImageCMDOptions.SecretKey, opt.Region)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/purge/images/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ const deletePromptMessage = "Deleting all the above images, images can't be clai

var Cmd = &cobra.Command{
Use: "images",
Short: "Purge the powervs images",
Long: `Purge the powervs images!
Short: "Purge the PowerVS images",
Long: `Purge the PowerVS images!
pvsadm purge --help for information
`,
RunE: func(cmd *cobra.Command, args []string) error {
Expand Down
4 changes: 2 additions & 2 deletions cmd/purge/networks/networks.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ var (

var Cmd = &cobra.Command{
Use: "networks",
Short: "Purge the powervs networks",
Long: `Purge the powervs networks!
Short: "Purge the PowerVS networks",
Long: `Purge the PowerVS networks!
pvsadm purge --help for information
`,
RunE: func(cmd *cobra.Command, args []string) error {
Expand Down
4 changes: 2 additions & 2 deletions cmd/purge/purge.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ import (

var Cmd = &cobra.Command{
Use: "purge",
Short: "Purge the powervs resources",
Long: `Purge the powervs resources
Short: "Purge the PowerVS resources",
Long: `Purge the PowerVS resources
# Set the API key or feed the --api-key commandline argument
export IBMCLOUD_API_KEY=<IBM_CLOUD_API_KEY>
Expand Down
4 changes: 2 additions & 2 deletions cmd/purge/vms/vms.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ const deletePromptMessage = "Deleting all the above instances, instances can't b

var Cmd = &cobra.Command{
Use: "vms",
Short: "Purge the powervs vms",
Long: `Purge the powervs vms!
Short: "Purge the PowerVS vms",
Long: `Purge the PowerVS vms!
pvsadm purge --help for information
`,
RunE: func(cmd *cobra.Command, args []string) error {
Expand Down
4 changes: 2 additions & 2 deletions cmd/purge/volumes/volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ const deletePromptMessage = "Deleting all the volumes in available state, volume

var Cmd = &cobra.Command{
Use: "volumes",
Short: "Purge the powervs volumes",
Long: `Deletes all the volumes for the powervs instance which are in available state(not attached to any instances)
Short: "Purge the PowerVS volumes",
Long: `Deletes all the volumes for the PowerVS instance which are in available state(not attached to any instances)
pvsadm purge --help for information
`,
RunE: func(cmd *cobra.Command, args []string) error {
Expand Down
2 changes: 1 addition & 1 deletion cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import (

var rootCmd = &cobra.Command{
Use: "pvsadm",
Short: "pvsadm is a command for managing powervs infra",
Short: "pvsadm is a command line tool for managing IBM Cloud PowerVS infrastructure",
Long: `Power Systems Virtual Server projects deliver flexible compute capacity for Power Systems workloads.
Integrated with the IBM Cloud platform for on-demand provisioning.
Expand Down
4 changes: 2 additions & 2 deletions docs/How to Import Image to PowerVS Instance.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# Overview
This guide talks about how to import image to PowerVs workspace using pvsadm.
This guide talks about how to import image to PowerVS workspace using pvsadm.

# Prerequisite
- pvsadm tool
- IBMCLOUD_API_KEY. [How to create api key](https://cloud.ibm.com/docs/account?topic=account-userapikey#create_user_key)
- IBMCLOUD_API_KEY. [How to create API key](https://cloud.ibm.com/docs/account?topic=account-userapikey#create_user_key)
- S3 BucketName, Bucket Region, ObjectName
- PowerVS Workspace Name/PowerVS Workspace ID.

Expand Down
8 changes: 4 additions & 4 deletions pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func (c *Client) ListServiceInstances(serviceType string) (map[string]string, er
return instances, nil
}

// func ListWorkspaceInstances is used to retrieve serviceInstances aloong with their regions.
// func ListWorkspaceInstances is used to retrieve serviceInstances along with their regions.
func (c *Client) ListWorkspaceInstances() (*resourcecontrollerv2.ResourceInstancesList, error) {
auth, err := core.GetAuthenticatorFromEnvironment(serviceIBMCloud)

Expand All @@ -213,7 +213,7 @@ func (c *Client) ListWorkspaceInstances() (*resourcecontrollerv2.ResourceInstanc
}
workspaces, _, err := c.ResouceControllerClient.ListResourceInstances(listServiceInstanceOptions)
if err != nil {
klog.Errorf("error while listing Resource Instances: %+v", err)
klog.Errorf("error while listing resource instances: %+v", err)
return nil, err
}
return workspaces, nil
Expand Down Expand Up @@ -292,8 +292,8 @@ func (c *Client) CreateServiceInstance(instanceName, serviceName, servicePlan, r
return "", err
}

klog.Infof("Resource service Instance Details :%v", serviceInstance)
klog.Infof("Resource service InstanceID :%v", serviceInstance.Crn.ServiceInstance)
klog.Infof("Resource service instance details :%v", serviceInstance)
klog.Infof("Resource service instanceID :%v", serviceInstance.Crn.ServiceInstance)

return serviceInstance.Crn.ServiceInstance, nil
}
Expand Down
5 changes: 2 additions & 3 deletions pkg/client/cloudconnection/cloudconnection.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,10 @@ type Client struct {
}

func NewClient(sess *ibmpisession.IBMPISession, powerinstanceid string) *Client {
c := &Client{
return &Client{
instanceID: powerinstanceid,
client: instance.NewIBMPICloudConnectionClient(context.Background(), sess, powerinstanceid),
}
c.client = instance.NewIBMPICloudConnectionClient(context.Background(), sess, powerinstanceid)
return c
}

func (c *Client) Get(id string) (*models.CloudConnection, error) {
Expand Down
5 changes: 2 additions & 3 deletions pkg/client/datacenter/datacenter.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,10 @@ type Client struct {
}

func NewClient(sess *ibmpisession.IBMPISession, powerinstanceid string) *Client {
c := &Client{
return &Client{
instanceID: powerinstanceid,
client: instance.NewIBMPIDatacenterClient(context.Background(), sess, powerinstanceid),
}
c.client = instance.NewIBMPIDatacenterClient(context.Background(), sess, powerinstanceid)
return c
}

func (c *Client) Get(id string) (*models.Datacenter, error) {
Expand Down
Loading

0 comments on commit 108d7b3

Please sign in to comment.