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

Fix panic of "docker stats --format {{.Name}} --all" #30776

Merged
merged 1 commit into from Feb 8, 2017

Conversation

Projects
None yet
5 participants
@WeiZhang555
Contributor

WeiZhang555 commented Feb 7, 2017

This commit fixes panic when execute stats command:

  • use --format {{.Name}} with --all when there're exited containers.
  • use --format {{.Name}} while stating exited container.

The root cause is when stating an exited container, the result from the
api didn't contain the Name and ID field, which will make format
process panic.

Panic log is like this:

panic: runtime error: slice bounds out of range [recovered]
	panic: runtime error: slice bounds out of range

goroutine 1 [running]:
panic(0xb20f80, 0xc420014110)
	/usr/local/go/src/runtime/panic.go:500 +0x1a1
text/template.errRecover(0xc4201773e8)
	/usr/local/go/src/text/template/exec.go:140 +0x2ad
panic(0xb20f80, 0xc420014110)
	/usr/local/go/src/runtime/panic.go:458 +0x243
github.com/docker/docker/cli/command/formatter.(*containerStatsContext).Name(0xc420430160,
0x0, 0x0)
	/go/src/github.com/docker/docker/cli/command/formatter/stats.go:148
+0x86
reflect.Value.call(0xb9a3a0, 0xc420430160, 0x2213, 0xbe3657, 0x4,
0x11bc9f8, 0x0, 0x0, 0x4d75b3, 0x1198940, ...)
	/usr/local/go/src/reflect/value.go:434 +0x5c8
reflect.Value.Call(0xb9a3a0, 0xc420430160, 0x2213, 0x11bc9f8, 0x0, 0x0,
0xc420424028, 0xb, 0xb)
	/usr/local/go/src/reflect/value.go:302 +0xa4
text/template.(*state).evalCall(0xc420177368, 0xb9a3a0, 0xc420430160,
0x16, 0xb9a3a0, 0xc420430160, 0x2213, 0x1178fa0, 0xc4203ea330,
0xc4203de283, ...)
	/usr/local/go/src/text/template/exec.go:658 +0x530

Signed-off-by: Zhang Wei zhangwei555@huawei.com

This is part of #29702, here is the client fix, daemon fix is still under discussion.

@AkihiroSuda

LGTM

@LK4D4

This comment has been minimized.

Show comment
Hide comment
@LK4D4

LK4D4 Feb 7, 2017

Contributor

@WeiZhang555 I believe there was some tests in old PR as well :)

Contributor

LK4D4 commented Feb 7, 2017

@WeiZhang555 I believe there was some tests in old PR as well :)

Fix panic of "docker stats --format {{.Name}} --all"
This commit fixes panic when execute stats command:

* use --format {{.Name}} with --all when there're exited containers.
* use --format {{.Name}} while stating exited container.

The root cause is when stating an exited container, the result from the
api didn't contain the Name and ID field, which will make format
process panic.

Panic log is like this:

```
panic: runtime error: slice bounds out of range [recovered]
	panic: runtime error: slice bounds out of range

goroutine 1 [running]:
panic(0xb20f80, 0xc420014110)
	/usr/local/go/src/runtime/panic.go:500 +0x1a1
text/template.errRecover(0xc4201773e8)
	/usr/local/go/src/text/template/exec.go:140 +0x2ad
panic(0xb20f80, 0xc420014110)
	/usr/local/go/src/runtime/panic.go:458 +0x243
github.com/docker/docker/cli/command/formatter.(*containerStatsContext).Name(0xc420430160,
0x0, 0x0)
	/go/src/github.com/docker/docker/cli/command/formatter/stats.go:148
+0x86
reflect.Value.call(0xb9a3a0, 0xc420430160, 0x2213, 0xbe3657, 0x4,
0x11bc9f8, 0x0, 0x0, 0x4d75b3, 0x1198940, ...)
	/usr/local/go/src/reflect/value.go:434 +0x5c8
reflect.Value.Call(0xb9a3a0, 0xc420430160, 0x2213, 0x11bc9f8, 0x0, 0x0,
0xc420424028, 0xb, 0xb)
	/usr/local/go/src/reflect/value.go:302 +0xa4
text/template.(*state).evalCall(0xc420177368, 0xb9a3a0, 0xc420430160,
0x16, 0xb9a3a0, 0xc420430160, 0x2213, 0x1178fa0, 0xc4203ea330,
0xc4203de283, ...)
	/usr/local/go/src/text/template/exec.go:658 +0x530
```

Signed-off-by: Zhang Wei <zhangwei555@huawei.com>
@WeiZhang555

This comment has been minimized.

Show comment
Hide comment
@WeiZhang555

WeiZhang555 Feb 8, 2017

Contributor

@LK4D4 adding an unit test, the other integration test depend on daemon side fix so I didn't move it here. :-)

Contributor

WeiZhang555 commented Feb 8, 2017

@LK4D4 adding an unit test, the other integration test depend on daemon side fix so I didn't move it here. :-)

@LK4D4

This comment has been minimized.

Show comment
Hide comment
@LK4D4

LK4D4 Feb 8, 2017

Contributor

@WeiZhang555 thanks!
LGTM

Contributor

LK4D4 commented Feb 8, 2017

@WeiZhang555 thanks!
LGTM

@LK4D4 LK4D4 merged commit f5116c6 into moby:master Feb 8, 2017

4 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 30526 has succeeded
Details
janky Jenkins build Docker-PRs 39111 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 10167 has succeeded
Details

@GordonTheTurtle GordonTheTurtle added this to the 1.14.0 milestone Feb 8, 2017

@WeiZhang555 WeiZhang555 deleted the WeiZhang555:stats-all-format-name-panic-cli branch Feb 9, 2017

@thaJeztah thaJeztah added this to PRs in 17.03.2-maybe Feb 9, 2017

thaJeztah added a commit to thaJeztah/docker that referenced this pull request Feb 18, 2017

Merge pull request moby#30776 from WeiZhang555/stats-all-format-name-…
…panic-cli

Fix panic of "docker stats --format {{.Name}} --all"
(cherry picked from commit f5116c6)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

@thaJeztah thaJeztah referenced this pull request Feb 18, 2017

Merged

17.03 cherry picks #31140

@thaJeztah thaJeztah moved this from PRs to picked in 17.03.2-maybe Feb 18, 2017

@thaJeztah thaJeztah modified the milestones: 17.03.0, 17.04.0 Feb 18, 2017

thaJeztah added a commit to thaJeztah/docker that referenced this pull request Feb 18, 2017

Merge pull request moby#30776 from WeiZhang555/stats-all-format-name-…
…panic-cli

Fix panic of "docker stats --format {{.Name}} --all"
(cherry picked from commit f5116c6)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

@thaJeztah thaJeztah removed this from picked in 17.03.2-maybe Feb 22, 2017

dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017

Merge pull request moby#30776 from WeiZhang555/stats-all-format-name-…
…panic-cli

Fix panic of "docker stats --format {{.Name}} --all"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment