Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use server-side printing in `kubectl get -w` #76161

Merged
merged 7 commits into from Apr 8, 2019

Conversation

@liggitt
Copy link
Member

commented Apr 4, 2019

What type of PR is this?
/kind feature

What this PR does / why we need it:
Requests and handles server-side print events for kubectl get -w

Which issue(s) this PR fixes:
Fixes #66538

Does this PR introduce a user-facing change?:

`kubectl get -w` now prints custom resource definitions with custom print columns

/sig cli
/cc @smarterclayton @seans3

@liggitt liggitt force-pushed the liggitt:kubectl-watch-table branch from 6b08f38 to 53e55d3 Apr 5, 2019

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt

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:

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

@liggitt

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2019

/retest

@liggitt liggitt changed the title WIP - use server-side printing in `kubectl get -w` use server-side printing in `kubectl get -w` Apr 5, 2019

@fejta-bot

This comment has been minimized.

Copy link

commented Apr 5, 2019

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

if o.IsHumanReadablePrinter {
// printing always takes the internal version, but the watch event uses externals
if o.IsHumanReadablePrinter && objToPrint.GetObjectKind().GroupVersionKind().GroupKind() != tableGK {
// printing anything other than tables always takes the internal version, but the watch event uses externals

This comment has been minimized.

Copy link
@smarterclayton

smarterclayton Apr 5, 2019

Contributor

reminds me for v1 we'll need two versions of this, beta1 and v1

This comment has been minimized.

Copy link
@liggitt

liggitt Apr 5, 2019

Author Member

this one in particular just checks group/kind, so it's version neutral... places where we check for *metav1beta1.Table will definitely need to add a switch

@smarterclayton

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2019

looks reasonable, didn't see anything objectionable so far

@smarterclayton

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2019

/assign @smarterclayton

@smarterclayton

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Apr 8, 2019

@k8s-ci-robot k8s-ci-robot merged commit c082ace into kubernetes:master Apr 8, 2019

17 checks passed

cla/linuxfoundation liggitt authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-godeps Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details

@juanvallejo juanvallejo referenced this pull request Apr 8, 2019

Closed

Server-side printing continued #60712

11 of 12 tasks complete
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

@liggitt: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-kubemark-e2e-gce-big 53e55d3 link /test pull-kubernetes-kubemark-e2e-gce-big

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.

@liggitt liggitt deleted the liggitt:kubectl-watch-table branch Apr 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.