Conversation
a2a05ca to
48e7cce
Compare
48e7cce to
855e529
Compare
|
/cc @jmhbnz |
|
Hey @wzshiming - I'll review this one next, can you please rebase to fix conflicts, thanks! 🙏🏻 |
855e529 to
bfce023
Compare
|
Done, thank you for reviewing it 😄 |
| clientv3 "go.etcd.io/etcd/client/v3" | ||
| ) | ||
|
|
||
| func (c *client) Watch(ctx context.Context, prefix string, opOpts ...OpOption) error { |
There was a problem hiding this comment.
Hey @wzshiming - General question, have you put any thought into potential approaches for writing tests for augerctl? To add new features it gets easier over time if we have tests to verify existing features don't break.
There was a problem hiding this comment.
Sure, my original plan was to add the e2e test after the subcommand get.
bfce023 to
1a58621
Compare
|
@wzshiming |
1a58621 to
53fbbfe
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wzshiming The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Signed-off-by: Shiming Zhang <wzshiming@hotmail.com>
53fbbfe to
0c7870f
Compare
There was a problem hiding this comment.
Pull request overview
Adds watch support to augerctl get, enabling streaming updates from etcd (similar to kubectl get -w) as part of the broader effort to provide kubectl-like direct-etcd operations.
Changes:
- Extend the etcd client abstraction with
Watch, plus aWithRevisionoption andPrevValuesupport inKeyValue. - Add
--watch/-wand--watch-onlyflags toaugerctl get, wiring them to the new watch implementation. - Update YAML/JSON printers to fall back to printing
PrevValuewhen the current value is absent (e.g., delete events), and document watch usage.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/client/client.go | Adds Watch to the interface, introduces WithRevision, and extends KeyValue with PrevValue. |
| pkg/client/client_watch.go | New etcd watch implementation that streams events into the existing response callback. |
| augerctl/command/get_command.go | Adds CLI flags and control flow to list-then-watch or watch-only. |
| augerctl/command/printer_yaml.go | Prints PrevValue when Value is missing during watch output. |
| augerctl/command/printer_json.go | Prints PrevValue when Value is missing during watch output. |
| augerctl/README.md | Documents augerctl get ... -w usage and kubectl equivalence guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| cmd.Flags().BoolVarP(&flags.Watch, "watch", "w", false, "after listing/getting the requested object, watch for changes") | ||
| cmd.Flags().BoolVar(&flags.WatchOnly, "watch-only", false, "watch for changes to the requested object(s), without listing/getting first") |
There was a problem hiding this comment.
--watch-only currently requires --watch (and otherwise errors), which differs from kubectl semantics and makes --watch-only awkward to use. Consider making --watch-only imply watch mode (i.e., treat --watch-only as equivalent to --watch --watch-only) and enforce mutual exclusion in flag validation instead of requiring both flags.
part of #3
Test
Preparation cluster
Create a cluster and expose etcd port, to facilitate the creation of a cluster using kwokctl, this can be any other k8s cluster
# brew install kwok kwokctl create cluster --etcd-port 2379Watch leases changes