Ports list command implementation - PowerVS #1084

Niharika0306 · 2023-02-08T09:27:19Z

What this PR does / why we need it:
This PR is for listing ports from PowerVC

$ ./capibmadm powervs port list --service-instance-id "b9db6009-30fc-4718-98cb-16ba6fd37b82"  --zone mon01 --network rdr-acm-spoke2-411-mon01-pub-net
Listing PowerVS ports service-instance-id="b9db6009-30fc-4718-98cb-16ba6fd37b82" network="rdr-acm-spoke2-411-mon01-pub-net"
DESCRIPTION   EXTERNALIP      IPADDRESS        MACADDRESS          PORTID                                 STATUS
              169.54.110.91   192.168.164.91   fa:d4:c3:31:24:20   19712105-d5be-4e97-9fc1-d9f49300ddf9   ACTIVE

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

/area provider/ibmcloud

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:

linux-foundation-easycla · 2023-02-08T09:27:22Z

The committers listed above are authorized under a signed CLA.

✅ login: Niharika0306 / name: Niharika Gurram (630d931)

netlify · 2023-02-08T09:27:23Z

✅ Deploy Preview for kubernetes-sigs-cluster-api-ibmcloud ready!

Name	Link
🔨 Latest commit	`a3969a2`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-cluster-api-ibmcloud/deploys/63e9f7d25ca506000808e0c1
😎 Deploy Preview	https://deploy-preview-1084--kubernetes-sigs-cluster-api-ibmcloud.netlify.app/print
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

k8s-ci-robot · 2023-02-08T09:27:27Z

Welcome @Niharika0306!

It looks like this is your first PR to kubernetes-sigs/cluster-api-provider-ibmcloud 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api-provider-ibmcloud has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2023-02-08T09:27:28Z

Hi @Niharika0306. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

cmd/capibmadm/cmd/powervs/port/list.go

Prajyot-Parab · 2023-02-09T06:29:33Z

/ok-to-test

cmd/capibmadm/cmd/powervs/port/type.go

dharaneeshvrd · 2023-02-09T07:18:34Z

cmd/capibmadm/cmd/powervs/port/list.go

+	log.Info("Listing PowerVS ports", "service-instance-id", options.GlobalOptions.ServiceInstanceID, "network", network)
+
+	auth := iam.GetIAMAuth()
+	accountID, _ := utils.GetAccountID(ctx, auth)


can we handle the error here?

cmd/capibmadm/cmd/powervs/port/list.go

docs/book/src/topics/capibmadm/powervs/port.md

cmd/capibmadm/cmd/powervs/port/list.go

Karthik-K-N · 2023-02-10T07:27:36Z

/lgtm

Prajyot-Parab · 2023-02-10T08:05:15Z

cmd/capibmadm/cmd/powervs/port/list.go

+		Example: `
+ # List ports in PowerVS Network
+ export IBMCLOUD_API_KEY=<api-key>
+ capibmadm powervs port list --service-instance-id <service-instance-id> --zone <zone> --network <network ID or name>`,


Suggested change

capibmadm powervs port list --service-instance-id <service-instance-id> --zone <zone> --network <network ID or name>`,

capibmadm powervs port list --service-instance-id <service-instance-id> --zone <zone> --network <network-name/network-id>`,

Prajyot-Parab · 2023-02-10T08:05:40Z

docs/book/src/topics/capibmadm/powervs/port.md

+#### Example:
+```shell
+export IBMCLOUD_API_KEY=<api-key>
+capibmadm powervs port list --service-instance-id <service-instance-id> --zone <zone> --network <network>


Suggested change

capibmadm powervs port list --service-instance-id <service-instance-id> --zone <zone> --network <network>

capibmadm powervs port list --service-instance-id <service-instance-id> --zone <zone> --network <network-name/network-id>

Prajyot-Parab · 2023-02-10T08:28:51Z

/lgtm
PTAL @Karthik-K-N @mkumatag

yussufsh · 2023-02-10T09:08:26Z

cmd/capibmadm/cmd/powervs/port/list.go

+	networks, err := networkClient.GetAll()
+	if err != nil {
+		return fmt.Errorf("failed to get the networks, err: %v", err)
+	}
+
+	var netID string
+
+	for _, net := range networks.Networks {
+		if *net.Name == network || *net.NetworkID == network {
+			netID = *net.NetworkID
+			break
+		}
+	}


Why do we need to perform this heavy operation? If I am not wrong just Get() on the network would return you 404 if does not exist.
@Prajyot-Parab

The Get call works even with name

yussufsh · 2023-02-10T09:14:06Z

cmd/capibmadm/cmd/powervs/port/list.go

+
+	var network string
+
+	cmd.Flags().StringVar(&network, "network", "", "Network ID or Name(preference will be given to the ID over Name)")


IMO there is no point of preference here if you can use the Get API to validate the network. Even the port list works with id or name.

yussufsh · 2023-02-10T09:14:50Z

cmd/capibmadm/cmd/powervs/port/list.go

+		return fmt.Errorf("not able to find network: \"%s\" by ID or name in the network list", network)
+	}
+
+	portListResp, err := networkClient.GetAllPorts(netID)


Works with both id and name, hence suggest you to use network directly here.

Here is a snippet of GetAllPorts func, seems it expects WithNetworkID(id) @yussufsh am i missing anything here?

// Get All Ports on a Network func (f *IBMPINetworkClient) GetAllPorts(id string) (*models.NetworkPorts, error) { params := p_cloud_networks.NewPcloudNetworksPortsGetallParams(). WithContext(f.ctx).WithTimeout(helpers.PIGetTimeOut). WithCloudInstanceID(f.cloudInstanceID).WithNetworkID(id)

I have tried running the curl command with network name as well. It works for network port object.

# network_id=ocp-net # curl -X GET https://syd.power-iaas.cloud.ibm.com/pcloud/v1/cloud-instances/${CLOUD_INSTANCE_ID}/networks/${network_id}/ports -H 'Authorization: Bearer <token>' -H 'CRN:<crn>' -H 'Content-Type: application/json' {"ports":[{"description":"","href":"/pcloud/v1/cloud-instances/<snipped>/networks/2c24ec46-96d2-4dd9-a3f0-f1f96c7e4342/ports/644863e1-2f97-4339-8ec1-3378490a0cf1","ipAddress":"192.168.200.156","macAddress":"fa:19:9e:27:fb:20","portID":"644863e1-2f97-4339-8ec1-3378490a0cf1","pvmInstance":{"href":"/pcloud/v1/cloud-instances/<snipped>/pvm-instances/b687cd71-313d-41df-bfa5-b5f04c5e4575","pvmInstanceID":"b687cd71-313d-41df-bfa5-b5f04c5e4575"},"status":"DOWN"}]}

@yussufsh this ain't working via code, am I on right track here?

prajyotparab@Prajyots-MacBook-Pro capibmadm % ./capibmadm powervs port list --service-instance-id 3229a94c-af54-4212-bf60-6202b6fd0a07 --zone osa21 --network capi-test --debug Listing PowerVS ports service-instance-id="3229a94c-af54-4212-bf60-6202b6fd0a07" network="capi-test" GET /pcloud/v1/cloud-instances/3229a94c-af54-4212-bf60-6202b6fd0a07/networks/capi-test/ports HTTP/1.1 Host: osa.power-iaas.cloud.ibm.com User-Agent: Go-http-client/1.1 Accept: application/json Authorization: [PRIVATE DATA HIDDEN] Crn: crn:v1:bluemix:public:power-iaas:osa21:a/c265c8cefda241ca9c107adcbbacaa84:3229a94c-af54-4212-bf60-6202b6fd0a07:: Accept-Encoding: gzip HTTP/2.0 500 Internal Server Error Content-Length: 448 Cf-Cache-Status: DYNAMIC Cf-Ray: 797483d70c1c855b-BOM Content-Type: application/json Date: Fri, 10 Feb 2023 11:30:51 GMT Server: cloudflare Strict-Transport-Security: max-age=15724800; includeSubDomains {"description":"unable to get network 9906818c-4ad1-4cd9-b6ca-bba776f2a22d: unable to get network for 9906818c-4ad1-4cd9-b6ca-bba776f2a22d: Resource not found: [GET https://192.168.40.140:9696/v2.0/networks/9906818c-4ad1-4cd9-b6ca-bba776f2a22d], error message: {\"NeutronError\": {\"type\": \"NetworkNotFound\", \"message\": \"Network 9906818c-4ad1-4cd9-b6ca-bba776f2a22d could not be found.\", \"detail\": \"\"}}","error":"internal server error"} Error: failed to Get all Network Ports for Network capi-test: [GET /pcloud/v1/cloud-instances/{cloud_instance_id}/networks/{network_id}/ports][500] pcloudNetworksPortsGetallInternalServerError &{Code:0 Description:unable to get network 9906818c-4ad1-4cd9-b6ca-bba776f2a22d: unable to get network for 9906818c-4ad1-4cd9-b6ca-bba776f2a22d: Resource not found: [GET https://192.168.40.140:9696/v2.0/networks/9906818c-4ad1-4cd9-b6ca-bba776f2a22d], error message: {"NeutronError": {"type": "NetworkNotFound", "message": "Network 9906818c-4ad1-4cd9-b6ca-bba776f2a22d could not be found.", "detail": ""}} Error:internal server error Message:}

As per @yussufsh's comments, the GetAllPorts command works with network name too. Hence removed the GetAllNetworks call ( and passing network argument directly to portList call) and just running the get network with id/name to check if network exists and capturing the error.

$ ./capibmadm powervs port list --service-instance-id "b9db6009-30fc-4718-98cb-16ba6fd37b82" --zone mon01 --network rdr-acm-spoke2-411-mon01-pub-net Listing PowerVS ports service-instance-id="b9db6009-30fc-4718-98cb-16ba6fd37b82" network="rdr-acm-spoke2-411-mon01-pub-net" DESCRIPTION EXTERNAL IP IP ADDRESS MAC ADDRESS PORT ID STATUS 169.54.110.91 192.168.164.91 fa:d4:c3:31:24:20 19712105-d5be-4e97-9fc1-d9f49300ddf9 ACTIVE

$ ./capibmadm powervs port list --service-instance-id "b9db6009-30fc-4718-98cb-16ba6fd37b82" --zone mon01 --network Temp Listing PowerVS ports service-instance-id="b9db6009-30fc-4718-98cb-16ba6fd37b82" network="Temp" Error: failed to perform Get Network Operation for Network id Temp with error [GET /pcloud/v1/cloud-instances/{cloud_instance_id}/networks/{network_id}][404] pcloudNetworksGetNotFound &{Code:0 Description:item not found: unable to get network, subnet, and possibly public network cidr for network Temp on cloud instance 914b05caac05408694e562586a65478b Error:item not found Message:}

Correct. @Prajyot-Parab makes sense?

Prajyot-Parab

/lgtm

Prajyot-Parab · 2023-02-13T08:10:16Z

PTAL @Karthik-K-N

Prajyot-Parab

/lgtm

mkumatag

/lgtm

k8s-ci-robot · 2023-02-14T06:21:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mkumatag, Niharika0306, Prajyot-Parab

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [mkumatag]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added the area/provider/ibmcloud Issues or PRs related to ibmcloud provider label Feb 8, 2023

k8s-ci-robot requested review from jichenjc and wentao-zh February 8, 2023 09:27

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 8, 2023

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 8, 2023

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 8, 2023

Niharika0306 force-pushed the ports_list branch from fe6ae4f to 4dbc352 Compare February 8, 2023 09:32

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 8, 2023

dharaneeshvrd reviewed Feb 8, 2023

View reviewed changes

Niharika0306 force-pushed the ports_list branch from 4dbc352 to aadb731 Compare February 8, 2023 11:37

yussufsh reviewed Feb 9, 2023

View reviewed changes

cmd/capibmadm/cmd/powervs/port/list.go Outdated Show resolved Hide resolved

Niharika0306 force-pushed the ports_list branch from aadb731 to 630d931 Compare February 9, 2023 05:47

dharaneeshvrd reviewed Feb 9, 2023

View reviewed changes

dharaneeshvrd mentioned this pull request Feb 9, 2023

Add support for port create command #1090

Merged

Niharika0306 force-pushed the ports_list branch 2 times, most recently from e64c85c to 692fa89 Compare February 9, 2023 09:32

kishen-v reviewed Feb 9, 2023

View reviewed changes

docs/book/src/topics/capibmadm/powervs/port.md Outdated Show resolved Hide resolved

Karthik-K-N reviewed Feb 9, 2023

View reviewed changes

cmd/capibmadm/cmd/powervs/port/list.go Outdated Show resolved Hide resolved

Niharika0306 force-pushed the ports_list branch 2 times, most recently from b879221 to 2aa3995 Compare February 9, 2023 10:14

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 10, 2023

Prajyot-Parab suggested changes Feb 10, 2023

View reviewed changes

Niharika0306 force-pushed the ports_list branch from 16d598b to 8fdebd3 Compare February 10, 2023 08:13

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 10, 2023

Niharika0306 force-pushed the ports_list branch from 8fdebd3 to 39d0ea3 Compare February 10, 2023 08:21

k8s-ci-robot assigned Prajyot-Parab Feb 10, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 10, 2023

yussufsh reviewed Feb 10, 2023

View reviewed changes

yussufsh suggested changes Feb 10, 2023

View reviewed changes

Niharika0306 force-pushed the ports_list branch from 39d0ea3 to eecccd3 Compare February 10, 2023 12:51

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 10, 2023

Niharika0306 force-pushed the ports_list branch from eecccd3 to 0f787eb Compare February 10, 2023 13:14

Prajyot-Parab approved these changes Feb 10, 2023

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 10, 2023

Prajyot-Parab mentioned this pull request Feb 13, 2023

Added Port Delete command to capibmadm #1086

Merged

Ports list command implementation - PowerVS

a3969a2

Niharika0306 force-pushed the ports_list branch from 0f787eb to a3969a2 Compare February 13, 2023 08:41

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 13, 2023

Prajyot-Parab approved these changes Feb 13, 2023

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 13, 2023

mkumatag approved these changes Feb 14, 2023

View reviewed changes

k8s-ci-robot assigned mkumatag Feb 14, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 14, 2023

k8s-ci-robot merged commit dc92a2a into kubernetes-sigs:main Feb 14, 2023

Karthik-K-N mentioned this pull request Mar 10, 2023

CLI tool for Cluster API Provider IBM Cloud #1052

Closed

16 tasks

Niharika0306 deleted the ports_list branch November 26, 2023 18:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ports list command implementation - PowerVS #1084

Ports list command implementation - PowerVS #1084

Niharika0306 commented Feb 8, 2023

linux-foundation-easycla bot commented Feb 8, 2023 •

edited

Loading

netlify bot commented Feb 8, 2023 •

edited

Loading

k8s-ci-robot commented Feb 8, 2023

k8s-ci-robot commented Feb 8, 2023

Prajyot-Parab commented Feb 9, 2023

dharaneeshvrd Feb 9, 2023

Niharika0306 Feb 9, 2023

Karthik-K-N commented Feb 10, 2023

Prajyot-Parab Feb 10, 2023

Niharika0306 Feb 10, 2023

Prajyot-Parab Feb 10, 2023

Niharika0306 Feb 10, 2023

Prajyot-Parab commented Feb 10, 2023

yussufsh Feb 10, 2023

yussufsh Feb 10, 2023

yussufsh Feb 10, 2023

yussufsh Feb 10, 2023

Prajyot-Parab Feb 10, 2023 •

edited

Loading

yussufsh Feb 10, 2023

yussufsh Feb 10, 2023

Prajyot-Parab Feb 10, 2023

Niharika0306 Feb 10, 2023

yussufsh Feb 10, 2023

Prajyot-Parab left a comment

Prajyot-Parab commented Feb 13, 2023

Prajyot-Parab left a comment

mkumatag left a comment

k8s-ci-robot commented Feb 14, 2023

	capibmadm powervs port list --service-instance-id <service-instance-id> --zone <zone> --network <network ID or name>`,
	capibmadm powervs port list --service-instance-id <service-instance-id> --zone <zone> --network <network-name/network-id>`,


		var network string

		cmd.Flags().StringVar(&network, "network", "", "Network ID or Name(preference will be given to the ID over Name)")

Ports list command implementation - PowerVS #1084

Ports list command implementation - PowerVS #1084

Conversation

Niharika0306 commented Feb 8, 2023

linux-foundation-easycla bot commented Feb 8, 2023 • edited Loading

netlify bot commented Feb 8, 2023 • edited Loading

✅ Deploy Preview for kubernetes-sigs-cluster-api-ibmcloud ready!

k8s-ci-robot commented Feb 8, 2023

k8s-ci-robot commented Feb 8, 2023

Prajyot-Parab commented Feb 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Karthik-K-N commented Feb 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Prajyot-Parab commented Feb 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Prajyot-Parab Feb 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Prajyot-Parab left a comment

Choose a reason for hiding this comment

Prajyot-Parab commented Feb 13, 2023

Prajyot-Parab left a comment

Choose a reason for hiding this comment

mkumatag left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Feb 14, 2023

linux-foundation-easycla bot commented Feb 8, 2023 •

edited

Loading

netlify bot commented Feb 8, 2023 •

edited

Loading

Prajyot-Parab Feb 10, 2023 •

edited

Loading