[kueuectl] Added kueuectl list pods command #2280

Kavinraja-G · 2024-05-25T19:05:54Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Adds kueuectl list pods --for job/job-name. This will help users to list all the pods controlled by a specific Job in a namespace.

Which issue(s) this PR fixes:

Fixes #2204

Special notes for your reviewer:

Nil

Does this PR introduce a user-facing change?

Adds kueuectl list pods --for job/job-name

k8s-ci-robot · 2024-05-25T19:06:03Z

Welcome @Kavinraja-G!

It looks like this is your first PR to kubernetes-sigs/kueue 🎉. 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/kueue 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 · 2024-05-25T19:06:04Z

Hi @Kavinraja-G. 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-sigs/prow repository.

netlify · 2024-05-25T19:06:10Z

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	`ded5836`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/66937530993b7c0008cad536

cmd/kueuectl/app/util/client_getter.go

mbobrovskyi

Don't forget to add docs here.

cmd/kueuectl/app/list/helpers.go

cmd/kueuectl/app/list/list_pods.go

cmd/kueuectl/app/list/list_pods_test.go

cmd/kueuectl/app/list/list_pods.go

mbobrovskyi · 2024-05-27T09:13:26Z

/assign

mbobrovskyi · 2024-05-27T09:29:50Z

/ok-to-test

cmd/kueuectl/app/list/list_pods.go

cmd/kueuectl/app/list/list_pods_test.go

mbobrovskyi · 2024-05-28T06:32:49Z

/test pull-kueue-test-e2e-main-1-27

cmd/kueuectl/app/testing/fake.go

cmd/kueuectl/app/list/list_pods.go

site/content/en/docs/reference/kubectl-kueue/commands/list.md

cmd/kueuectl/app/list/list_pods_test.go

mbobrovskyi · 2024-07-14T12:40:24Z

cmd/kueuectl/app/testing/fake.go

@@ -41,6 +41,11 @@ func NewTestClientGetter() *TestClientGetter {
 	}
 }

+func (f *TestClientGetter) WithRestMapper(mapper meta.RESTMapper) *TestClientGetter {


We already have WithRESTMapper.

mbobrovskyi · 2024-07-14T12:49:16Z

cmd/kueuectl/app/list/list_pods.go

+	if len(o.LabelSelector) != 0 {
+		o.PodLabelSelector = "," + o.PodLabelSelector
+	}


Suggested change

if len(o.LabelSelector) != 0 {

o.PodLabelSelector = "," + o.PodLabelSelector

}

podLabelSelector := o.PodLabelSelector

if len(o.LabelSelector) != 0 {

podLabelSelector = "," + o.PodLabelSelector

}

mbobrovskyi · 2024-07-14T12:57:03Z

cmd/kueuectl/app/list/list_pods.go

+		FieldSelectorParam(o.FieldSelector).
+		LabelSelectorParam(o.LabelSelector+o.PodLabelSelector).
+		ResourceTypeOrNameArgs(true, "pods").
+		ContinueOnError().


Suggested change

ContinueOnError().

ContinueOnError().

RequestChunksOf(o.Limit).

mbobrovskyi · 2024-07-14T12:59:40Z

cmd/kueuectl/app/list/list_pods.go

+func (o *PodOptions) transformRequests(req *rest.Request) {
+	req.SetHeader("Accept", strings.Join([]string{
+		fmt.Sprintf("application/json;as=Table;v=%s;g=%s", metav1.SchemeGroupVersion.Version, metav1.GroupName),
+		fmt.Sprintf("application/json;as=Table;v=%s;g=%s", metav1beta1.SchemeGroupVersion.Version, metav1beta1.GroupName),


Suggested change

fmt.Sprintf("application/json;as=Table;v=%s;g=%s", metav1beta1.SchemeGroupVersion.Version, metav1beta1.GroupName),

Do we need it?

mbobrovskyi · 2024-07-14T13:10:13Z

cmd/kueuectl/app/list/list_pods.go

+		Run: func(cmd *cobra.Command, args []string) {
+			cobra.CheckErr(o.Complete(clientGetter))
+			if o.ForObject != nil {
+				cobra.CheckErr(o.Run(cmd.Context(), clientGetter))
+			}
+		},


Suggested change

Run: func(cmd *cobra.Command, args []string) {

cobra.CheckErr(o.Complete(clientGetter))

if o.ForObject != nil {

cobra.CheckErr(o.Run(cmd.Context(), clientGetter))

}

},

RunE: func(cmd *cobra.Command, args []string) error {

cmd.SilenceUsage = true

err := o.Complete(clientGetter)

if err != nil {

return err

}

if o.ForObject == nil {

return nil

}

return o.Run(clientGetter)

},

We already fixed another commands #2578.

mbobrovskyi · 2024-07-14T13:53:54Z

cmd/kueuectl/app/list/list_pods.go

+	belonging to the specified namespace, matching
+	the label selector or the field selector.`


Suggested change

belonging to the specified namespace, matching

the label selector or the field selector.`

belonging to the specified namespace, matching

the label selector or the field selector.`

To do not add tabs on --help.

mbobrovskyi · 2024-07-14T14:12:57Z

cmd/kueuectl/app/list/list_pods.go

+}
+
+// fetchDynamicResourceInfos builds and executes a dynamic client query for a resource specified in --for
+func (o *PodOptions) fetchDynamicResourceInfos(clientGetter util.ClientGetter) ([]*resource.Info, error) {


Maybe...

Suggested change

func (o *PodOptions) fetchDynamicResourceInfos(clientGetter util.ClientGetter) ([]*resource.Info, error) {

func (o *PodOptions) getForObjectInfos(clientGetter util.ClientGetter) ([]*resource.Info, error) {

mbobrovskyi · 2024-07-14T14:13:59Z

cmd/kueuectl/app/list/list_pods.go

+}
+
+// getPodsUsingAPI gets the pods raw infos directly from the API server
+func (o *PodOptions) getPodsUsingAPI(clientGetter util.ClientGetter) ([]*resource.Info, error) {


And...

Suggested change

func (o *PodOptions) getPodsUsingAPI(clientGetter util.ClientGetter) ([]*resource.Info, error) {

func (o *PodOptions) getPodsInfos(clientGetter util.ClientGetter) ([]*resource.Info, error) {

mbobrovskyi · 2024-07-14T14:22:27Z

cmd/kueuectl/app/list/list_pods_test.go

+			clientset := k8sfake.NewSimpleClientset()
+			tf.K8sClientset = clientset
+			tf.K8sClientset.Discovery().(*fakediscovery.FakeDiscovery).Resources = tc.apiResourceLists


Suggested change

clientset := k8sfake.NewSimpleClientset()

tf.K8sClientset = clientset

tf.K8sClientset.Discovery().(*fakediscovery.FakeDiscovery).Resources = tc.apiResourceLists

Looks like we don't need it more?

mbobrovskyi · 2024-07-14T14:31:23Z

cmd/kueuectl/app/list/table_printer.go

Could you also add test case for pod groups?

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels May 25, 2024

k8s-ci-robot requested review from denkensk and trasc May 25, 2024 19:05

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 25, 2024

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 May 25, 2024

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label May 25, 2024

Kavinraja-G commented May 25, 2024

View reviewed changes

cmd/kueuectl/app/util/client_getter.go Outdated Show resolved Hide resolved

mbobrovskyi reviewed May 27, 2024

View reviewed changes

cmd/kueuectl/app/list/list_pods.go Outdated Show resolved Hide resolved

k8s-ci-robot assigned mbobrovskyi May 27, 2024

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 27, 2024

mbobrovskyi reviewed May 27, 2024

View reviewed changes

cmd/kueuectl/app/list/list_pods.go Outdated Show resolved Hide resolved

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 28, 2024

Kavinraja-G force-pushed the feat/kueuectl-list-pods branch from b83d793 to eb80584 Compare May 28, 2024 05:50

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

mbobrovskyi reviewed May 28, 2024

View reviewed changes

cmd/kueuectl/app/list/list_pods.go Outdated Show resolved Hide resolved

cmd/kueuectl/app/list/list_pods.go Outdated Show resolved Hide resolved

cmd/kueuectl/app/list/list_pods.go Outdated Show resolved Hide resolved

cmd/kueuectl/app/list/list_pods_test.go Outdated Show resolved Hide resolved

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 28, 2024

mbobrovskyi reviewed May 28, 2024

View reviewed changes

cmd/kueuectl/app/list/list_pods_test.go Outdated Show resolved Hide resolved

Kavinraja-G added 24 commits July 10, 2024 21:39

chore: remove ctx args in tt

11909ee

refactor get pods logic for multiple types

56ce351

test: Add more job type tests

b958af2

refactor: based on comments for listpods & helpers

6c92e1f

remove fmt

6536029

refactor unit tests

068445b

docs: Update kueuectl list pod

ee8fb4d

refactor based on commments (1)

18b2add

refactor based on commments (2)

96c1a5a

refactor: handle if no resources found

6669f00

Added seperate printer for list pods

84150f6

rename list_pods_printer

27bb499

Refactor printers logic and comments

3a7a0d6

fix lint

d1346ee

Refactor ForObject parsing and comments

fbc3c9e

Refactor ListPods logic to use common podLabelSelector

9e2ad1a

Refactor jobLabels as per comment

3146af1

breakdown Complete() by adding functions

22f3d2c

merge listPods() to Run()

ff4d26f

go mod vendor

42fa626

more refactor for podLabelSelection

8c71e33

remove auth imports

80ef005

support more columns for list pod

ae3bda1

more refactor based on PR comments (1)

824da48

Kavinraja-G force-pushed the feat/kueuectl-list-pods branch from 029344f to 824da48 Compare July 10, 2024 16:12

Refactor get pods logic to user API directly

ded5836

mbobrovskyi reviewed Jul 14, 2024

View reviewed changes

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[kueuectl] Added kueuectl list pods command #2280

[kueuectl] Added kueuectl list pods command #2280

Kavinraja-G commented May 25, 2024 •

edited

Loading

k8s-ci-robot commented May 25, 2024

k8s-ci-robot commented May 25, 2024

netlify bot commented May 25, 2024 •

edited

Loading

mbobrovskyi left a comment

mbobrovskyi commented May 27, 2024

mbobrovskyi commented May 27, 2024

mbobrovskyi commented May 28, 2024

mbobrovskyi Jul 14, 2024

mbobrovskyi Jul 14, 2024

mbobrovskyi Jul 14, 2024

mbobrovskyi Jul 14, 2024

mbobrovskyi Jul 14, 2024

mbobrovskyi Jul 14, 2024

mbobrovskyi Jul 14, 2024

mbobrovskyi Jul 14, 2024

mbobrovskyi Jul 14, 2024

mbobrovskyi Jul 14, 2024

	ContinueOnError().
	ContinueOnError().
	RequestChunksOf(o.Limit).

-		Run: func(cmd *cobra.Command, args []string) {
-			cobra.CheckErr(o.Complete(clientGetter))
-			if o.ForObject != nil {
-				cobra.CheckErr(o.Run(cmd.Context(), clientGetter))
-			}
-		},
+RunE: func(cmd *cobra.Command, args []string) error {
+			cmd.SilenceUsage = true
+			err := o.Complete(clientGetter)
+			if err != nil {
+				return err
+			}
+			if o.ForObject == nil {
+				return nil
+			}
+			return o.Run(clientGetter)
+		},

		belonging to the specified namespace, matching
		the label selector or the field selector.`

	func (o PodOptions) fetchDynamicResourceInfos(clientGetter util.ClientGetter) ([]resource.Info, error) {
	func (o PodOptions) getForObjectInfos(clientGetter util.ClientGetter) ([]resource.Info, error) {

	func (o PodOptions) getPodsUsingAPI(clientGetter util.ClientGetter) ([]resource.Info, error) {
	func (o PodOptions) getPodsInfos(clientGetter util.ClientGetter) ([]resource.Info, error) {

	clientset := k8sfake.NewSimpleClientset()
	tf.K8sClientset = clientset
	tf.K8sClientset.Discovery().(*fakediscovery.FakeDiscovery).Resources = tc.apiResourceLists

[kueuectl] Added kueuectl list pods command #2280

Are you sure you want to change the base?

[kueuectl] Added kueuectl list pods command #2280

Conversation

Kavinraja-G commented May 25, 2024 • edited Loading

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

k8s-ci-robot commented May 25, 2024

k8s-ci-robot commented May 25, 2024

netlify bot commented May 25, 2024 • edited Loading

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

mbobrovskyi left a comment

Choose a reason for hiding this comment

mbobrovskyi commented May 27, 2024

mbobrovskyi commented May 27, 2024

mbobrovskyi commented May 28, 2024

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

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

Kavinraja-G commented May 25, 2024 •

edited

Loading

netlify bot commented May 25, 2024 •

edited

Loading