-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix docker info, docker version --format=json not outputting json format #4168
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
Changes from all commits
1aaa179
46234b8
80640bc
23bd746
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,7 +12,9 @@ import ( | |
| pluginmanager "github.com/docker/cli/cli-plugins/manager" | ||
| "github.com/docker/cli/cli/command" | ||
| "github.com/docker/cli/cli/command/completion" | ||
| "github.com/docker/cli/cli/command/formatter" | ||
| "github.com/docker/cli/cli/debug" | ||
| flagsHelper "github.com/docker/cli/cli/flags" | ||
| "github.com/docker/cli/templates" | ||
| "github.com/docker/docker/api/types" | ||
| "github.com/docker/docker/api/types/swarm" | ||
|
|
@@ -62,10 +64,7 @@ func NewInfoCommand(dockerCli command.Cli) *cobra.Command { | |
| ValidArgsFunction: completion.NoComplete, | ||
| } | ||
|
|
||
| flags := cmd.Flags() | ||
|
|
||
| flags.StringVarP(&opts.format, "format", "f", "", "Format the output using the given Go template") | ||
|
|
||
| cmd.Flags().StringVarP(&opts.format, "format", "f", "", flagsHelper.InspectFormatHelp) | ||
| return cmd | ||
| } | ||
|
|
||
|
|
@@ -507,6 +506,10 @@ func printServerWarningsLegacy(dockerCli command.Cli, info types.Info) { | |
| } | ||
|
|
||
| func formatInfo(dockerCli command.Cli, info info, format string) error { | ||
| if format == formatter.JSONFormatKey { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is unfortunate, changing the parameter on fly is not great
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, the |
||
| format = formatter.JSONFormat | ||
| } | ||
|
|
||
| // Ensure slice/array fields render as `[]` not `null` | ||
| if info.ClientInfo != nil && info.ClientInfo.Plugins == nil { | ||
| info.ClientInfo.Plugins = make([]pluginmanager.Plugin, 0) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| {"Client":{"Platform":{"Name":""},"Version":"18.99.5-ce","ApiVersion":"1.38","DefaultAPIVersion":"1.38","GitCommit":"deadbeef","GoVersion":"go1.10.2","Os":"linux","Arch":"amd64","BuildTime":"Wed May 30 22:21:05 2018","Context":"my-context"},"Server":{"Platform":{"Name":"Docker Enterprise Edition (EE) 2.0"},"Components":[{"Name":"Engine","Version":"17.06.2-ee-15","Details":{"ApiVersion":"1.30","Arch":"amd64","BuildTime":"Mon Jul 9 23:38:38 2018","Experimental":"false","GitCommit":"64ddfa6","GoVersion":"go1.8.7","MinAPIVersion":"1.12","Os":"linux"}},{"Name":"Universal Control Plane","Version":"17.06.2-ee-15","Details":{"ApiVersion":"1.30","Arch":"amd64","BuildTime":"Mon Jul 2 21:24:07 UTC 2018","GitCommit":"4513922","GoVersion":"go1.9.4","MinApiVersion":"1.20","Os":"linux","Version":"3.0.3-tp2"}},{"Name":"Kubernetes","Version":"1.8+","Details":{"buildDate":"2018-04-26T16:51:21Z","compiler":"gc","gitCommit":"8d637aedf46b9c21dde723e29c645b9f27106fa5","gitTreeState":"clean","gitVersion":"v1.8.11-docker-8d637ae","goVersion":"go1.8.3","major":"1","minor":"8+","platform":"linux/amd64"}},{"Name":"Calico","Version":"v3.0.8","Details":{"cni":"v2.0.6","kube-controllers":"v2.0.5","node":"v3.0.8"}}],"Version":"","ApiVersion":"","GitCommit":"","GoVersion":"","Os":"","Arch":""}} |
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sneaky little commit that's unrelated :)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a assert.Check(t, golden.String(cli.OutBuffer().String(), "docker-client-version.json.golden"))
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😂 so I thought you meant that JSON file, but it was referring to the "make it a const" commit. I had that in another PR, but doing it later would mean we had more code-churn, as the lines using it are touched by this PR, so I moved it first to prevent that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we want the system info formatting to go to the formatting package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, we'd have to look. I think there's been some back-and-forth in the past; a single "formatting" package would also have to import all the types used by all objects, so I think it originally was a single package, then got split to each object type. to allow for specific logic for each where needed.
Definitely something to look at what the current state is, and if things are all in the right place.