Support api chunking in kubectl get #53768

smarterclayton · 2017-10-12T04:09:02Z

This enables chunking in the resource builder to make it easy to
retrieve resources in pages and visit partial result sets. This adds
--chunk-size to kubectl get only so that users can get comfortable
with the use of chunking in beta. Future changes will enable chunking
for all CLI commands so that bulk actions can be performed more
efficiently.

$ kubectl get pods --all-namespaces
... print batch of 500 pods ...
... print second batch of 500 pods ...
...

@kubernetes/sig-cli-pr-reviews @kubernetes/sig-api-machinery-pr-reviews

`kubectl get` will by default fetch large lists of resources in chunks of up to 500 items rather than requesting all resources up front from the server. This reduces the perceived latency of managing large clusters since the server returns the first set of results to the client much more quickly.  A new flag `--chunk-size=SIZE` may be used to alter the number of items or disable this feature when `0` is passed.  This is a beta feature.

k8s-ci-robot · 2017-10-12T04:09:04Z

@smarterclayton: Adding do-not-merge/release-note-label-needed because the release note process has not been followed.

One of the following labels is required "release-note", "release-note-action-required", or "release-note-none".
Please see: https://github.com/kubernetes/community/blob/master/contributors/devel/pull-requests.md#write-release-notes-if-needed.

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.

liggitt · 2017-10-12T04:15:58Z

hack/make-rules/test-cmd-util.sh

+  output_message=$(kubectl --v=6 get clusterrole --chunk-size=10 2>&1 "${kube_flags[@]}")
+  # Post-condition: Check if we get a limit and continue
+  kube::test::if_has_string "${output_message}" "/apis/rbac.authorization.k8s.io/v1/clusterroles?limit=10 200 OK"
+  kube::test::if_has_string "${output_message}" "/apis/rbac.authorization.k8s.io/v1/clusterroles?continue="


omit /apis/rbac.authorization.k8s.io/v1... we don't care what version, just the query params

liggitt · 2017-10-12T04:17:54Z

pkg/kubectl/cmd/get.go

@@ -223,6 +225,7 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [
 			FilenameParam(enforceNamespace, &options.FilenameOptions).
 			SelectorParam(selector).
 			ExportParam(export).
+			RequestChunksOf(options.ChunkSize).


why not Limit() or LimitParam()?

I didn't want to expose that yet, Limit itself is only of partial utility. I was focused on "i want to do what i did before, but in chunks".

liggitt · 2017-10-12T04:23:14Z

that was shockingly small

smarterclayton · 2017-10-12T17:35:22Z

I realized why this was so easy. It's because get isn't using the visitor pattern anymore. That needs to be fixed. So this will get larger because get is a monstrosity now.

mml · 2017-10-12T19:37:13Z

/assign @jpbetz
cc me too

smarterclayton · 2017-10-23T17:46:47Z

I'm going to do the get changes in a follow up (to show partial results on the client) - no reason to block here and this clean up sets the stage for it to be useful.

smarterclayton · 2017-10-23T23:54:05Z

/retest

smarterclayton · 2017-10-24T14:44:19Z

/retest

liggitt · 2017-10-24T14:49:25Z

staging/src/k8s.io/apimachinery/pkg/api/meta/interfaces.go

@@ -75,6 +75,9 @@ type MetadataAccessor interface {
 	Annotations(obj runtime.Object) (map[string]string, error)
 	SetAnnotations(obj runtime.Object, annotations map[string]string) error

+	Continue(obj runtime.Object) (string, error)
+	SetContinue(obj runtime.Object, c string) error


doesn't this belong in ListMetaAccessor, not MetadataAccessor?

also, surprised this didn't complain that this wasn't implemented in *ObjectMeta, genericAccessor, etc

There are three accessors. A generic one (that has all methods and returns an error if it doesn't support it), and then one specific one for lists and one for metadata.

smarterclayton · 2017-10-25T01:14:45Z

Fixed a bug in the get loop where an error wouldn't exit correctly.

smarterclayton · 2017-10-26T17:42:18Z

Any other comments?

liggitt · 2017-10-27T17:55:42Z

pkg/kubectl/cmd/get.go

@@ -137,6 +138,7 @@ func NewCmdGet(f cmdutil.Factory, out io.Writer, errOut io.Writer) *cobra.Comman
 	cmd.Flags().Bool("show-kind", false, "If present, list the resource type for the requested object(s).")
 	cmd.Flags().Bool("all-namespaces", false, "If present, list the requested object(s) across all namespaces. Namespace in current context is ignored even if specified with --namespace.")
 	cmd.Flags().BoolVar(&options.IgnoreNotFound, "ignore-not-found", false, "Treat \"resource not found\" as a successful retrieval.")
+	cmd.Flags().Int64Var(&options.ChunkSize, "experimental-chunk-size", 500, "Return large lists in chunks rather than all at once. Pass 0 to disable.")


drop experimental from the flag and explicitly document it as beta? (per https://kubernetes.io/docs/reference/deprecation-policy/)

Updated, we don't have a lot of other flags with wording like this yet so I tried to stay generic.

liggitt · 2017-10-27T17:56:10Z

nit on flag, LGTM otherwise

This enables chunking in the resource builder to make it easy to retrieve resources in pages and visit partial result sets. This adds `--chunk-size` to `kubectl get` only so that users can get comfortable with the use of chunking in beta. Future changes will enable chunking for all CLI commands so that bulk actions can be performed more efficiently.

liggitt · 2017-10-28T16:33:46Z

/lgtm

k8s-github-robot · 2017-10-28T16:34:35Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: liggitt, smarterclayton

No associated issue. Update pull-request body to add a reference to an issue, or get approval with /approve no-issue

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

Needs approval from an approver in each of these OWNERS Files:

~~hack/OWNERS~~ [smarterclayton]
~~pkg/kubectl/OWNERS~~ [liggitt,smarterclayton]
~~staging/src/k8s.io/apimachinery/pkg/OWNERS~~ [liggitt,smarterclayton]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-ci-robot · 2017-10-29T22:00:37Z

@smarterclayton: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-e2e-gce-bazel	`da1065f`	link	`/test pull-kubernetes-e2e-gce-bazel`
pull-kubernetes-e2e-gce-etcd3	`da1065f`	link	`/test pull-kubernetes-e2e-gce-etcd3`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

smarterclayton · 2017-10-29T22:16:36Z

/retest

k8s-github-robot · 2017-10-29T22:59:54Z

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

smarterclayton added this to the v1.9 milestone Oct 12, 2017

smarterclayton added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Oct 12, 2017

k8s-github-robot assigned derekwaynecarr and mfojtik Oct 12, 2017

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Oct 12, 2017

liggitt reviewed Oct 12, 2017

View reviewed changes

smarterclayton force-pushed the chunking_cli branch 2 times, most recently from 52760bb to da1065f Compare October 12, 2017 16:11

k8s-ci-robot assigned jpbetz Oct 12, 2017

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 18, 2017

smarterclayton force-pushed the chunking_cli branch from da1065f to 4d40086 Compare October 23, 2017 17:53

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 23, 2017

liggitt reviewed Oct 24, 2017

View reviewed changes

smarterclayton force-pushed the chunking_cli branch from 4d40086 to d1b20e4 Compare October 24, 2017 21:41

liggitt reviewed Oct 27, 2017

View reviewed changes

smarterclayton force-pushed the chunking_cli branch from d1b20e4 to 4780ad0 Compare October 27, 2017 21:23

k8s-ci-robot assigned liggitt Oct 28, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 28, 2017

smarterclayton added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 29, 2017

k8s-github-robot merged commit 12e5db5 into kubernetes:master Oct 29, 2017

KnVerey mentioned this pull request Nov 30, 2018

Request json in one batch Shopify/krane#394

Merged

pacoxu mentioned this pull request Feb 24, 2021

APIListChunking - feature gate to GA? #96497

Closed

KnVerey mentioned this pull request Mar 11, 2021

Support --chunk-size on kubectl drain and kubectl describe kubernetes/kubectl#1028

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support api chunking in kubectl get #53768

Support api chunking in kubectl get #53768

smarterclayton commented Oct 12, 2017 •

edited

Loading

k8s-ci-robot commented Oct 12, 2017

liggitt Oct 12, 2017

liggitt Oct 12, 2017 •

edited

Loading

smarterclayton Oct 12, 2017

liggitt commented Oct 12, 2017

smarterclayton commented Oct 12, 2017

mml commented Oct 12, 2017

smarterclayton commented Oct 23, 2017

smarterclayton commented Oct 23, 2017

smarterclayton commented Oct 24, 2017

liggitt Oct 24, 2017

liggitt Oct 24, 2017

smarterclayton Oct 24, 2017

smarterclayton commented Oct 25, 2017

smarterclayton commented Oct 26, 2017

liggitt Oct 27, 2017

smarterclayton Oct 27, 2017

liggitt commented Oct 27, 2017

liggitt commented Oct 28, 2017

k8s-github-robot commented Oct 28, 2017

k8s-ci-robot commented Oct 29, 2017 •

edited

Loading

smarterclayton commented Oct 29, 2017

k8s-github-robot commented Oct 29, 2017

Support api chunking in kubectl get #53768

Support api chunking in kubectl get #53768

Conversation

smarterclayton commented Oct 12, 2017 • edited Loading

k8s-ci-robot commented Oct 12, 2017

liggitt Oct 12, 2017

Choose a reason for hiding this comment

liggitt Oct 12, 2017 • edited Loading

Choose a reason for hiding this comment

smarterclayton Oct 12, 2017

Choose a reason for hiding this comment

liggitt commented Oct 12, 2017

smarterclayton commented Oct 12, 2017

mml commented Oct 12, 2017

smarterclayton commented Oct 23, 2017

smarterclayton commented Oct 23, 2017

smarterclayton commented Oct 24, 2017

liggitt Oct 24, 2017

Choose a reason for hiding this comment

liggitt Oct 24, 2017

Choose a reason for hiding this comment

smarterclayton Oct 24, 2017

Choose a reason for hiding this comment

smarterclayton commented Oct 25, 2017

smarterclayton commented Oct 26, 2017

liggitt Oct 27, 2017

Choose a reason for hiding this comment

smarterclayton Oct 27, 2017

Choose a reason for hiding this comment

liggitt commented Oct 27, 2017

liggitt commented Oct 28, 2017

k8s-github-robot commented Oct 28, 2017

k8s-ci-robot commented Oct 29, 2017 • edited Loading

smarterclayton commented Oct 29, 2017

k8s-github-robot commented Oct 29, 2017

smarterclayton commented Oct 12, 2017 •

edited

Loading

liggitt Oct 12, 2017 •

edited

Loading

k8s-ci-robot commented Oct 29, 2017 •

edited

Loading