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

ctlv3: support ETCDCTL_WATCH_KEY, ETCDCTL_WATCH_RANGE_END #9142

Merged
merged 9 commits into from
Jan 17, 2018

Conversation

gyuho
Copy link
Contributor

@gyuho gyuho commented Jan 14, 2018

Parsing logic became more complicated, but added test cases should cover all edge cases.

Can you give this a try @debovema @vikin91 @wwyiwzhang @pldmgg ?

ETCDCTL_API=3 ETCDCTL_WATCH_KEY=foo ./bin/etcdctl watch
ETCDCTL_API=3 ETCDCTL_WATCH_KEY=foo ./bin/etcdctl watch --rev 5
ETCDCTL_API=3 ETCDCTL_WATCH_KEY=foo ./bin/etcdctl watch -i

ref. #8919 (comment)
#8814

Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@7a8c192). Click here to learn what that means.
The diff coverage is 73.33%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #9142   +/-   ##
=========================================
  Coverage          ?   76.01%           
=========================================
  Files             ?      359           
  Lines             ?    30038           
  Branches          ?        0           
=========================================
  Hits              ?    22833           
  Misses            ?     5618           
  Partials          ?     1587
Impacted Files Coverage Δ
etcdctl/ctlv3/command/global.go 60.6% <60%> (ø)
etcdctl/ctlv3/command/watch_command.go 50.34% <75%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a8c192...388b7fe. Read the comment docs.

@vikin91
Copy link

vikin91 commented Jan 16, 2018

I will give it a try in the next days. However, in the comments #8919 (comment) and #8919 (comment) me and @debovema meant different set of variables. The variables (ETCD_WATCH_KEY, ETCD_WATCH_VALUE, ...) shall be set by the etcdctl watch command. In this PR, the env variables are the inputs to the etcdctl watch command, which is only a nice addition, but not the feature that is missing in APIv3.

In this comment, you referenced the proper values that should be set before the execution of the command.

Consider this trivial scenario:

etcdctl watch foo -- echo "key: $ETCD_WATCH_KEY value: $ETCD_WATCH_VALUE"
# The output shall be non-empty and contain the key (prefix) and value that have changed in etcd

@gyuho gyuho added the WIP label Jan 16, 2018
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
@gyuho gyuho removed the WIP label Jan 16, 2018
@gyuho
Copy link
Contributor Author

gyuho commented Jan 16, 2018

@vikin91 Just added

./etcdctl watch foo -- sh -c "env | grep ETCD_WATCH_"

# PUT
# foo
# bar
# ETCD_WATCH_REVISION=11
# ETCD_WATCH_KEY="foo"
# ETCD_WATCH_EVENT_TYPE="PUT"
# ETCD_WATCH_VALUE="bar"

It will be set for each event. Think of ETCD_WATCH_REVISION as v2 exec-watch's ETCD_WATCH_MODIFIED_INDEX. It is the latest revision at the time of watch response from server.

@vikin91
Copy link

vikin91 commented Jan 16, 2018

Fantastic, thanks! I will get back to you as soon as I test it. Hopefully in the next 48hrs.

Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Copy link
Contributor

@jpbetz jpbetz left a comment

Choose a reason for hiding this comment

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

This looks right to me. Docs and tests look comprehensive.

lgtm

@gyuho gyuho merged commit 3051186 into etcd-io:master Jan 17, 2018
@gyuho gyuho deleted the watch-env branch January 17, 2018 20:39
@vikin91
Copy link

vikin91 commented Jan 26, 2018

Sorry for reporting back so late - so far it works for me as expected. Many thanks for this PR!

@rajnishahuja
Copy link

rajnishahuja commented May 11, 2023

  1. Is below not supposed to work ?
    etcdctl watch foo -- echo "key: $ETCD_WATCH_KEY value: $ETCD_WATCH_VALUE"
    PUT
    foo
    bar
    key: value:

  2. I want to take action only for PUT entries for a specific prefix and ignore the DELETEs . While setting the key via env variable works, it does NOT work for WATCH_EVENT_TYPE

Below works and reports only events on foo (and ignore foo1)
ETCDCTL_WATCH_KEY=foo etcdctl watch -- sh -c 'env |grep ETCD_WATCH' , (and it monitors only the foo key and ignores foo1)

This DOES NOT work and reports all types of events (PUT OR DELETE).

ETCD_WATCH_EVENT_TYPE=PUT etcdctl watch foo -- sh -c 'env |grep ETCD_WATCH'
PUT
foo
bar
ETCD_WATCH_KEY="foo"
ETCD_WATCH_REVISION=630
ETCD_WATCH_VALUE="bar"
ETCD_WATCH_EVENT_TYPE="PUT"
DELETE
foo

ETCD_WATCH_KEY="foo"
ETCD_WATCH_REVISION=631
ETCD_WATCH_VALUE=""
ETCD_WATCH_EVENT_TYPE="DELETE"

  1. While ETCDCTL_WATCH_KEY works ; ETCDCTL_WATCH_EVENT_TYPE throws an error ;

2023-05-11 16:22:39.793490 W | pkg/flags: unrecognized environment variable ETCDCTL_WATCH_EVENT_TYPE=PUT

Any way of making this work ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants