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

cli: Add KV export and import #2633

Merged
merged 1 commit into from
Jan 5, 2017
Merged

cli: Add KV export and import #2633

merged 1 commit into from
Jan 5, 2017

Conversation

jen20
Copy link
Contributor

@jen20 jen20 commented Jan 5, 2017

This commit adds two new commands to the Consul KV CLI, which export and import a JSON formatted representation of the Consul KV tree. It is useful to migrate parts of the KV tree between unrelated Consul clusters, and could also be used for initial data population of the KV store.

@jen20 jen20 added the type/enhancement Proposed improvement or new feature label Jan 5, 2017
@jen20 jen20 force-pushed the kv-export-import branch from b1ef59b to 44bba65 Compare January 5, 2017 00:53
Copy link
Contributor

@slackpad slackpad left a comment

Choose a reason for hiding this comment

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

Noted a few minor things, otherwise LGTM.


` + apiOptsText + `

KV Get Options:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should say KV Export Options:

@@ -81,7 +81,7 @@ KV Get Options:

func (c *KVGetCommand) Run(args []string) int {
cmdFlags := flag.NewFlagSet("get", flag.ContinueOnError)
cmdFlags.Usage = func() { c.Ui.Output(c.Help()) }
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like an unrelated change, but if this is implied by not setting .Usage then makes sense and is fine.


` + apiOptsText + `

KV Get Options:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is stale, too.


Or it can be read from stdin using the "-" symbol:

$ cat filename.json | consul kv put config/program/license -
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs an update.


data := args[0]

switch data[0] {
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this crash if you pass an empty quoted string in as the first arg?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Likely yes - this is copied from kv put so that probably has the same behaviour. I'll update.

if strings.TrimSpace(string(pair.Value)) != "bar" {
t.Fatalf("bad: expected: bar, got %s", pair.Value)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Might as well check foo/a as well.

@@ -81,7 +81,7 @@ KV Get Options:

func (c *KVGetCommand) Run(args []string) int {
cmdFlags := flag.NewFlagSet("get", flag.ContinueOnError)
cmdFlags.Usage = func() { c.Ui.Output(c.Help()) }

Choose a reason for hiding this comment

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

Is this intended? Maybe I missed it but I didn't see this move elsewhere.

@jen20 jen20 force-pushed the kv-export-import branch 4 times, most recently from 69fe814 to f26d387 Compare January 5, 2017 01:09
@jen20
Copy link
Contributor Author

jen20 commented Jan 5, 2017

I've fixed up most of the review comments, probably also need to look at kv put for the data[0] issue.

@slackpad
Copy link
Contributor

slackpad commented Jan 5, 2017

Latest changes look good to merge. I think it's fine if you want to fix the empty arg issue here for put as well.

This commit adds two new commands to the Consul KV CLI, which export and
import a JSON formatted representation of the Consul KV tree. It is
useful to migrate parts of the KV tree between unrelated Consul
clusters, and could also be used for initial data population of the KV
store.
@jen20 jen20 force-pushed the kv-export-import branch from f26d387 to d4e8c8a Compare January 5, 2017 13:57
@jen20
Copy link
Contributor Author

jen20 commented Jan 5, 2017

The panic in kv put is fixed in #2635, the same bug in kv import is fixed here. I'm currently waiting for Travis confirmation then I'll merge this one and leave #2635 for review of the new behaviour.

@jen20 jen20 merged commit 0e6e315 into master Jan 5, 2017
@jen20 jen20 deleted the kv-export-import branch January 5, 2017 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Proposed improvement or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants