moby / moby

The Moby Project - a collaborative project for the container ecosystem to assemble container-based systems
https://mobyproject.org/
Apache License 2.0
68.86k stars 18.67k forks source link

Event filtering on "exec_start", "exec_create", and "health_status" is broken #25798

Closed thaJeztah closed 8 years ago

thaJeztah commented 8 years ago

Output of docker version:

Client:
 Version:      1.12.1-rc1
 API version:  1.24
 Go version:   go1.6.3
 Git commit:   7889dc7
 Built:        Fri Aug 12 18:35:53 2016
 OS/Arch:      darwin/amd64
 Experimental: true

Server:
 Version:      1.12.1-rc1
 API version:  1.24
 Go version:   go1.6.3
 Git commit:   7889dc7
 Built:        Fri Aug 12 18:35:53 2016
 OS/Arch:      linux/amd64
 Experimental: true

Steps to reproduce the issue:

In one terminal:

docker events --filter event=health_status

In another terminal;

docker run --name=test -d --health-cmd='stat /etc/passwd || exit 1' --health-interval=2s   busybox sleep 1d

Describe the results you received:

$ docker events --filter event=health_status
<no output>

$ docker events --filter event=exec_create
<no output>

$ docker events --filter event=exec_start
<no output>

Describe the results you expected:

$ docker events --filter event=health_status

2016-08-17T12:10:34.337301145Z container health_status: healthy da1bab7bc91a424cd8e0571808c1b4638a903a8508eb6612e9cdfc49f9dde555 (image=busybox, name=test)

$ docker events --filter event=exec_create

2016-08-17T12:10:34.198347963Z container exec_create: /bin/sh -c stat /etc/passwd || exit 1 da1bab7bc91a424cd8e0571808c1b4638a903a8508eb6612e9cdfc49f9dde555 (image=busybox, name=test)
2016-08-17T12:10:36.338156681Z container exec_create: /bin/sh -c stat /etc/passwd || exit 1 da1bab7bc91a424cd8e0571808c1b4638a903a8508eb6612e9cdfc49f9dde555 (image=busybox, name=test)

$ docker events --filter event=exec_start

2016-08-17T12:10:34.198549073Z container exec_start: /bin/sh -c stat /etc/passwd || exit 1 da1bab7bc91a424cd8e0571808c1b4638a903a8508eb6612e9cdfc49f9dde555 (image=busybox, name=test)
2016-08-17T12:10:36.338214034Z container exec_start: /bin/sh -c stat /etc/passwd || exit 1 da1bab7bc91a424cd8e0571808c1b4638a903a8508eb6612e9cdfc49f9dde555 (image=busybox, name=test)

Additional information you deem important (e.g. issue happens only occasionally):

I tried to reproduce on older versions of docker, and this seems to work on Docker 1.8, but fails on Docker 1.9

I debugged this issue, and as far as I can see, the cause of this is that, unlike other events, the event.Action for exec_create, exec_start, and health_status contains additional output. This is the content of event.Action of events fired by the example container;

create
connect
start
exec_create: /bin/sh -c stat /etc/passwd || exit 1
exec_start: /bin/sh -c stat /etc/passwd || exit 1
health_status: healthy

The filter executes an exact match on event.Action (ef.filter.ExactMatch("event", ev.Action)), and fails to recognize the events due to this extra output;

See daemon/events/filter.go#L21), and vendor/src/github.com/docker/engine-api/types/filters/parse.go#L209-L219

Possibly we can change the ef.filter.ExactMatch() with ef.filter.FuzzyMatch(), however that would result in --filter event=exec to match both exec_start and exec_create (not sure we want that).

If we want this filter to be an "exact" match, we could filter on;

  1. Exact match (for, e.g. start)
  2. A prefix match for exec_start: (the event name provided, but with a colon (:) after

This would prevent exec to match exec_start.

jhorwit2 commented 8 years ago

Isn't this expected for health_status since the action is health_status: healthy and health_status: unhealthy? You can see here that's how the tests look for those events. That was one reason I made https://github.com/docker/docker/issues/24720.

thaJeztah commented 8 years ago

hm, perhaps, weird thing is though that for exec_start it also fails, and I doubt people will filter for --filter "event=exec_create: /bin/sh -c stat /etc/passwd || exit 1"

So, hm, yes, challenge (I see it possibly being useful to filter for only "healthy" events, or the opposite

thaJeztah commented 8 years ago

Perhaps --filter event=health_status --filter event-message=healthy (or similar)

thaJeztah commented 8 years ago

@jhorwit2 what do you think would be best? would you expect to always be filtering on "event + status"?

vdemeester commented 8 years ago

@jhorwit2 the action should be something like an enum, and thus to real status should be somewhere in the attributes (a status attribute for example) — i.e the event action is a health_status, and the value (or one of the value) is healthy or something else.

We could add another filter for events attributes but this should be definitely fixed by used constant event action. I'm on it :angel:

jhorwit2 commented 8 years ago

@thaJeztah Albeit cumbersome / hard to remember, filtering on health_status: healthy and health_status: unhealthy has been exactly what I wanted when I was monitoring for those.

I like health action with a state attribute of healthy or unhealthy though. Personally i'm starting to find status overloaded/confusing. The container struct has an attribute State which is a combination of health state + container state + uptime. Then in docker ps that's under the STATUS header. It's also Status in the container json obj returned from the daemon. https://github.com/docker/docker/blob/master/daemon/list.go#L467

edit: after thinking more, if we are going with health_status then an attribute of status makes more sense.

thaJeztah commented 8 years ago

@jhorwit2 would "health-status" (instead of "status") be better? Naming is hard 😅

thaJeztah commented 8 years ago

oh darn, so hard, that of course the event already is named like that, haha

don't know what would be good for filtering the health-status result.

jhorwit2 commented 8 years ago

Naming is hardest problem in CS 👼