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.

Sign up for GitHub

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

Jump to bottom

Include event type in kubectl get -w output #72416

Merged
merged 2 commits into from
Jul 9, 2019
Merged

Include event type in kubectl get -w output #72416

merged 2 commits into from
Jul 9, 2019

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented Dec 28, 2018 •
edited
Loading

What type of PR is this?
/kind feature

What this PR does / why we need it:
Allows the type of event (ADDED, MODIFIED, DELETED) to be seen when watching objects with kubectl get -w --output-watch-events

Before:

kubectl get ns -w
NAME              STATUS   AGE
default           Active   9h
kube-node-lease   Active   9h
kube-public       Active   9h
kube-system       Active   9h
ns1   Active   0s
ns1   Terminating   8s
ns1   Terminating   13s
ns1   Terminating   13s

After:

kubectl get ns -w --output-watch-events
EVENT      NAME              STATUS   AGE
ADDED      default           Active   84m
ADDED      kube-node-lease   Active   84m
ADDED      kube-public       Active   84m
ADDED      kube-system       Active   84m
ADDED      ns1               Active   0s
MODIFIED   ns1               Terminating   5s
MODIFIED   ns1               Terminating   10s
DELETED    ns1               Terminating   10s

Which issue(s) this PR fixes:
Fixes kubernetes/kubectl#185

Based on #79530

Does this PR introduce a user-facing change?:

`kubectl get -w` now takes an `--output-watch-events` flag to indicate the event type (ADDED, MODIFIED, DELETED)

/cc @smarterclayton @seans3
/sig cli

@k8s-ci-robot k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Dec 28, 2018
@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. sig/cli Categorizes an issue or PR as relevant to SIG CLI. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. area/kubectl approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Dec 28, 2018
@liggitt
Copy link
Member Author

liggitt commented Dec 28, 2018

/retest

This was referenced Dec 29, 2018
Merged
Closed
liggitt
liggitt commented Jan 2, 2019
View reviewed changes
pkg/printers/humanreadable.go Outdated Show resolved Hide resolved
liggitt
liggitt commented Jan 2, 2019
View reviewed changes
pkg/printers/humanreadable.go Outdated Show resolved Hide resolved
@liggitt liggitt force-pushed the watch-event-type branch 2 times, most recently from 57b7fcd to 8b0da1e Compare January 3, 2019 05:06
@liggitt liggitt changed the title Include event type in human readable kubectl get -w output WIP - Include event type in human readable kubectl get -w output Jan 3, 2019
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 3, 2019
@liggitt
Copy link
Member Author

liggitt commented Jan 5, 2019

/retest

smarterclayton
smarterclayton reviewed Feb 19, 2019
View reviewed changes
staging/src/k8s.io/cli-runtime/pkg/genericclioptions/printers/name.go Outdated
@@ -42,6 +43,11 @@ type NamePrinter struct {
// PrintObj is an implementation of ResourcePrinter.PrintObj which decodes the object
// and print "resource/name" pair. If the object is a List, print all items in it.
func (p *NamePrinter) PrintObj(obj runtime.Object, w io.Writer) error {
switch castObj := obj.(type) {
case *metav1.WatchEvent:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish we'd done a watch event interface. I can take that as a future cleanup.

@liggitt liggitt added this to the v1.15 milestone Mar 7, 2019
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 9, 2019
@liggitt liggitt removed this from the v1.15 milestone May 29, 2019
@liggitt liggitt force-pushed the watch-event-type branch from 4ecb0b1 to 313ffbd Compare June 24, 2019 01:13
@liggitt liggitt changed the title WIP - Include event type in kubectl get -w output Include event type in kubectl get -w output Jun 24, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 24, 2019
@liggitt
Copy link
Member Author

liggitt commented Jun 24, 2019

/priority important-longterm

@k8s-ci-robot k8s-ci-robot added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jun 24, 2019
@liggitt liggitt force-pushed the watch-event-type branch from 313ffbd to e3139c4 Compare June 28, 2019 17:19
@k8s-ci-robot 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 Jun 28, 2019
@liggitt
Copy link
Member Author

liggitt commented Jun 28, 2019

addressed comments, rebased on #79530

@liggitt liggitt force-pushed the watch-event-type branch from e3139c4 to fc6730a Compare June 28, 2019 21:16
@k8s-ci-robot k8s-ci-robot added area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Jun 28, 2019
@liggitt liggitt force-pushed the watch-event-type branch from fc6730a to 1327346 Compare June 28, 2019 23:25
@k8s-ci-robot k8s-ci-robot added 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 Jun 28, 2019
smarterclayton
smarterclayton reviewed Jul 4, 2019
View reviewed changes
pkg/kubectl/cmd/get/get.go
@@ -171,6 +173,7 @@ func NewCmdGet(parent string, f cmdutil.Factory, streams genericclioptions.IOStr
cmd.Flags().StringVar(&o.Raw, "raw", o.Raw, "Raw URI to request from the server. Uses the transport specified by the kubeconfig file.")
cmd.Flags().BoolVarP(&o.Watch, "watch", "w", o.Watch, "After listing/getting the requested object, watch for changes. Uninitialized objects are excluded if no object name is provided.")
cmd.Flags().BoolVar(&o.WatchOnly, "watch-only", o.WatchOnly, "Watch for changes to the requested object(s), without listing/getting first.")
cmd.Flags().BoolVar(&o.OutputWatchEvents, "output-watch-events", o.OutputWatchEvents, "Output watch event objects when --watch or --watch-only is used. Existing objects are output as initial ADDED events.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should be --watch-events or --include-events (although I could maybe see that being confused with Events). We're not super strict about prefixing flags though.

Copy link
Member Author

@liggitt liggitt Jul 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like --output-... as a prefix, and since it's only allowed with --watch (or --watch-only), I thought qualifying it would be clearer

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--include-events sounds like what describe does, separately fetching/printing related events

@smarterclayton
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 9, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@k8s-ci-robot k8s-ci-robot merged commit 5625ead into kubernetes:master Jul 9, 2019
@liggitt liggitt deleted the watch-event-type branch July 11, 2019 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smarterclayton smarterclayton smarterclayton left review comments

@seans3 seans3 Awaiting requested review from seans3

Assignees

@smarterclayton smarterclayton

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubectl area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.16
Development

Successfully merging this pull request may close these issues.

Watch flag should indicate when a resource is deleted
3 participants
@liggitt @smarterclayton @k8s-ci-robot