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

Fixes issue with stats on start event #20792

Merged
merged 1 commit into from
Mar 1, 2016

Conversation

cpuguy83
Copy link
Member

In situations where a client is called like docker stats with no
arguments or flags, if a container which was already created but not
started yet is then subsequently started it will not be added to the
stats list as expected.

Also splits some of the stats helpers to a separate file from the stats
CLI which is already quite long.

@calavera
Copy link
Contributor

LGTM. I really like the new event handlers.

@icecrime
Copy link
Contributor

Well, that's new (win2lin):

21:03:21 FAIL: docker_cli_events_test.go:544: DockerSuite.TestEventsFilterType
21:03:21 
21:03:21 docker_cli_events_test.go:585:
21:03:21     // Events generated by the container that builds the image
21:03:21     c.Assert(events, checker.HasLen, 3, check.Commentf("Events == %s", events))
21:03:21 ... obtained []string = []string{"2016-02-29T21:03:18.174819695Z container create 6607da1f2b310022f9aa9465b7e214686087d53f1988e2614fbbbadcf8fbe62d (image=sha256:d9551b4026f0e2950ddb557cc640871710bf88476ca938b71499305647231b82, io.docker.testing=image, name=high_easley)", "2016-02-29T21:03:18.272585767Z container commit 6607da1f2b310022f9aa9465b7e214686087d53f1988e2614fbbbadcf8fbe62d (comment=, image=sha256:d9551b4026f0e2950ddb557cc640871710bf88476ca938b71499305647231b82, io.docker.testing=image, name=high_easley)", "2016-02-29T21:03:18.299829424Z container destroy 6607da1f2b310022f9aa9465b7e214686087d53f1988e2614fbbbadcf8fbe62d (image=sha256:d9551b4026f0e2950ddb557cc640871710bf88476ca938b71499305647231b82, io.docker.testing=image, name=high_easley)", "\r", "This application has requested the Runtime to terminate it in an unusual way.", "Please contact the application's support team for more information.\r", "runtime: failed to create new OS thread (13)"}
21:03:21 ... n int = 3
21:03:21 ... Events == [2016-02-29T21:03:18.174819695Z container create 6607da1f2b310022f9aa9465b7e214686087d53f1988e2614fbbbadcf8fbe62d (image=sha256:d9551b4026f0e2950ddb557cc640871710bf88476ca938b71499305647231b82, io.docker.testing=image, name=high_easley) 2016-02-29T21:03:18.272585767Z container commit 6607da1f2b310022f9aa9465b7e214686087d53f1988e2614fbbbadcf8fbe62d (comment=, image=sha256:d9551b4026f0e2950ddb557cc640871710bf88476ca938b71499305647231b82, io.docker.testing=image, name=high_easley) 2016-02-29T21:03:18.299829424Z container destroy 6607da1f2b310022f9aa9465b7e214686087d53f1988e2614fbbbadcf8fbe62d (image=sha256:d9551b4026f0e2950ddb557cc640871710bf88476ca938b71499305647231b82, io.docker.testing=image, name=high_easley) 
 This application has requested the Runtime to terminate it in an unusual way. Please contact the application's support team for more information.
 runtime: failed to create new OS thread (13)]

@cpuguy83
Copy link
Member Author

lol, StatsAll has passed twice.

In situations where a client is called like `docker stats` with no
arguments or flags, if a container which was already created but not
started yet is then subsequently started it will not be added to the
stats list as expected.

Also splits some of the stats helpers to a separate file from the stats
CLI which is already quite long.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@cpuguy83
Copy link
Member Author

One little change to remove the not-neccessary "close" function on the event handler.

@tiborvass
Copy link
Contributor

LGTM

icecrime pushed a commit that referenced this pull request Mar 1, 2016
Fixes issue with stats on start event
@icecrime icecrime merged commit 187a2fb into moby:master Mar 1, 2016
@cpuguy83 cpuguy83 deleted the more_stats_client_cleanup branch March 1, 2016 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants