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

Enhance the output of consul snapshot inspect #8787

Merged
merged 32 commits into from
Oct 9, 2020
Merged

Conversation

schristoff
Copy link
Contributor

@schristoff schristoff commented Sep 30, 2020

Adding @banks Consul Snapshot tool which enhances the output of the consul snapshot inspect command to look something like this:

consul snapshot inspect -enhance backup.snap

 Type                        Count      Size
 ----                        ----       ----
 Register                    3          1.7KB
 ConnectCA                   1          1.2KB
 ConnectCAProviderState      1          1.1KB
 Index                       12         344B
 AutopilotRequest            1          199B
 ConnectCAConfig             1          197B
 FederationState             1          139B
 SystemMetadata              1          68B
 ChunkingState               1          12B
 ----                        ----       ----
 Total                                   5KB

On the downside, each time a new type is added it will need to be added in two places as RB as shown below.

agent/structs/structs.go Outdated Show resolved Hide resolved
@schristoff schristoff changed the title [WIP] Enhance the output of consul snapshot inspect Enhance the output of consul snapshot inspect Oct 8, 2020
Copy link
Contributor

@freddygv freddygv left a comment

Choose a reason for hiding this comment

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

This is looking good. Great to see it's being released soon!

Added some mostly stylistic suggestions.

command/snapshot/inspect/snapshot_inspect.go Outdated Show resolved Hide resolved
command/snapshot/inspect/snapshot_inspect.go Outdated Show resolved Hide resolved
agent/structs/structs.go Show resolved Hide resolved
agent/structs/structs.go Outdated Show resolved Hide resolved
command/snapshot/inspect/snapshot_inspect.go Outdated Show resolved Hide resolved
agent/structs/structs.go Outdated Show resolved Hide resolved
agent/structs/structs.go Show resolved Hide resolved
}
stats, offset, err := enhance(file)
if err != nil {
c.UI.Error(fmt.Sprintf("Error verifying snapshot: %s", err))
Copy link
Contributor

Choose a reason for hiding this comment

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

This error message could be updated, it's from the old call to snapshot.Verify()

s-christoff and others added 4 commits October 8, 2020 16:21
Co-authored-by: Freddy <freddygv@users.noreply.github.com>
Co-authored-by: Freddy <freddygv@users.noreply.github.com>
Co-authored-by: Freddy <freddygv@users.noreply.github.com>
Co-authored-by: Freddy <freddygv@users.noreply.github.com>
s-christoff and others added 2 commits October 8, 2020 22:56
agent/structs/structs.go Outdated Show resolved Hide resolved
agent/structs/structs.go Show resolved Hide resolved
command/snapshot/inspect/formatter.go Outdated Show resolved Hide resolved
command/snapshot/inspect/formatter.go Outdated Show resolved Hide resolved
command/snapshot/inspect/snapshot_inspect.go Outdated Show resolved Hide resolved
website/pages/commands/snapshot/inspect.mdx Outdated Show resolved Hide resolved
agent/consul/fsm/fsm.go Outdated Show resolved Hide resolved
Copy link
Member

@mkeeler mkeeler left a comment

Choose a reason for hiding this comment

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

Just the one change necessary to the changelog entry but otherwise 👍

@@ -0,0 +1,3 @@
```release-note:feature
agent: update `consul snapshot inspect` command to provide more detailed snapshot data
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
agent: update `consul snapshot inspect` command to provide more detailed snapshot data
snapshot: update `consul snapshot inspect` command to provide more detailed snapshot data

agent/structs/structs.go Outdated Show resolved Hide resolved
Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

a few minor things, but I think the defer to remove the temp file would be good to handle

command/snapshot/inspect/snapshot_inspect.go Outdated Show resolved Hide resolved
command/snapshot/inspect/snapshot_inspect.go Outdated Show resolved Hide resolved
command/snapshot/inspect/snapshot_inspect.go Outdated Show resolved Hide resolved
command/snapshot/inspect/snapshot_inspect.go Outdated Show resolved Hide resolved
command/snapshot/inspect/snapshot_inspect_test.go Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants