Don't exit on container destroy events #8859

sdt · 2021-10-31T04:22:06Z

When watchContainers receives a container destroy event, calling ContainerInspect returns an error, because the container no longer exists. This causes both docker-compose up and docker-compose logs -f to exit with Error: No such container: <id> when removing a stopped container.

This patch checks for the destroy event, and just does an early-out. The die event will have already been processed, so the internal cleanup for the destroyed container will have already happened.

Fixes #8747

Signed-off-by: Stephen Thirlwall [email protected]

Fixes docker#8747 When the event is a container destroy, calling ContainerInspect returns an error, because the container no longer exists. This causes both `docker-compose up` and `docker-compose logs -f` to exit when removing a stopped container. This container has already emitted its die event, and has already been cleaned up. I believe all that needs doing in this case is to early-out. Signed-off-by: Stephen Thirlwall <[email protected]>

Knew I'd forget something. Signed-off-by: Stephen Thirlwall <[email protected]>

mat007

Makes sense!
It would be great if we could add a test somehow 🤔

mat007 · 2021-11-02T09:06:12Z

pkg/compose/start.go

@@ -79,6 +79,12 @@ func (s *composeService) watchContainers(ctx context.Context, projectName string
 	err := s.Events(ctx, projectName, api.EventsOptions{
 		Services: services,
 		Consumer: func(event api.Event) error {
+			if event.Status == "destroy" {
+				// This container can't be inspected, because it's gone.
+				// Its already been removed from the watched map.


Suggested change

// Its already been removed from the watched map.

// It's already been removed from the watched map.

Signed-off-by: Stephen Thirlwall <[email protected]>

thaJeztah · 2021-11-02T22:41:08Z

pkg/compose/start.go

 			inspected, err := s.apiClient.ContainerInspect(ctx, event.Container)
 			if err != nil {
 				return err


Looking at the code below, it looks like only 2 event-types are currently handled (die and start); is there a reason to perform ContainerInspect for every event?

Also note that the event stream itself already has some attributes/metadata that is being obtained from the inspect. For example, the following docker run;

docker network create foobar docker volume create bla docker run --rm --label com.docker.compose.service=service1 --label com.docker.compose.project=bla --name service1 busybox docker run --rm --label com.docker.compose.service=service2 --label com.docker.compose.project=bla --name service2 busybox docker run --rm --label com.docker.compose.service=service3 --label com.docker.compose.project=bla --name service3 busybox docker network rm foobar docker volume rm bla

Returns these details in the events stream (so already includes container name, ID, and labels);

{"Type":"network","Action":"create","Actor":{"ID":"41ae0bfcff9464ce75638facafbb1314287cbed7373ae36e58a20b8b8e59df5e","Attributes":{"name":"foobar","type":"bridge"}},"scope":"local","time":1635892339,"timeNano":1635892339122996703} {"Type":"volume","Action":"create","Actor":{"ID":"bla","Attributes":{"driver":"local"}},"scope":"local","time":1635892339,"timeNano":1635892339325053299} {"status":"create","id":"61f972180486444944e999ec49c298fd8290a2373abf4727ac99d11455461868","from":"busybox","Type":"container","Action":"create","Actor":{"ID":"61f972180486444944e999ec49c298fd8290a2373abf4727ac99d11455461868","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service1","image":"busybox","name":"service1"}},"scope":"local","time":1635892339,"timeNano":1635892339569732117} {"status":"attach","id":"61f972180486444944e999ec49c298fd8290a2373abf4727ac99d11455461868","from":"busybox","Type":"container","Action":"attach","Actor":{"ID":"61f972180486444944e999ec49c298fd8290a2373abf4727ac99d11455461868","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service1","image":"busybox","name":"service1"}},"scope":"local","time":1635892339,"timeNano":1635892339574172088} {"Type":"network","Action":"connect","Actor":{"ID":"37a8a35f8d843207e72d4283cc777bdbbdc2b6d72f9631dcdf3e185a4ce3fc40","Attributes":{"container":"61f972180486444944e999ec49c298fd8290a2373abf4727ac99d11455461868","name":"bridge","type":"bridge"}},"scope":"local","time":1635892339,"timeNano":1635892339616669421} {"status":"start","id":"61f972180486444944e999ec49c298fd8290a2373abf4727ac99d11455461868","from":"busybox","Type":"container","Action":"start","Actor":{"ID":"61f972180486444944e999ec49c298fd8290a2373abf4727ac99d11455461868","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service1","image":"busybox","name":"service1"}},"scope":"local","time":1635892339,"timeNano":1635892339872730006} {"status":"die","id":"61f972180486444944e999ec49c298fd8290a2373abf4727ac99d11455461868","from":"busybox","Type":"container","Action":"die","Actor":{"ID":"61f972180486444944e999ec49c298fd8290a2373abf4727ac99d11455461868","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service1","exitCode":"0","image":"busybox","name":"service1"}},"scope":"local","time":1635892339,"timeNano":1635892339897176538} {"Type":"network","Action":"disconnect","Actor":{"ID":"37a8a35f8d843207e72d4283cc777bdbbdc2b6d72f9631dcdf3e185a4ce3fc40","Attributes":{"container":"61f972180486444944e999ec49c298fd8290a2373abf4727ac99d11455461868","name":"bridge","type":"bridge"}},"scope":"local","time":1635892339,"timeNano":1635892339947263591} {"status":"destroy","id":"61f972180486444944e999ec49c298fd8290a2373abf4727ac99d11455461868","from":"busybox","Type":"container","Action":"destroy","Actor":{"ID":"61f972180486444944e999ec49c298fd8290a2373abf4727ac99d11455461868","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service1","image":"busybox","name":"service1"}},"scope":"local","time":1635892339,"timeNano":1635892339974551091} {"status":"create","id":"2c3f35b3bd53e2c8b5677bc57143b35bb43e9b097f91cb75e0c3308a1be0595f","from":"busybox","Type":"container","Action":"create","Actor":{"ID":"2c3f35b3bd53e2c8b5677bc57143b35bb43e9b097f91cb75e0c3308a1be0595f","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service2","image":"busybox","name":"service1"}},"scope":"local","time":1635892340,"timeNano":1635892340216345875} {"status":"attach","id":"2c3f35b3bd53e2c8b5677bc57143b35bb43e9b097f91cb75e0c3308a1be0595f","from":"busybox","Type":"container","Action":"attach","Actor":{"ID":"2c3f35b3bd53e2c8b5677bc57143b35bb43e9b097f91cb75e0c3308a1be0595f","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service2","image":"busybox","name":"service1"}},"scope":"local","time":1635892340,"timeNano":1635892340219356281} {"Type":"network","Action":"connect","Actor":{"ID":"37a8a35f8d843207e72d4283cc777bdbbdc2b6d72f9631dcdf3e185a4ce3fc40","Attributes":{"container":"2c3f35b3bd53e2c8b5677bc57143b35bb43e9b097f91cb75e0c3308a1be0595f","name":"bridge","type":"bridge"}},"scope":"local","time":1635892340,"timeNano":1635892340257223076} {"status":"start","id":"2c3f35b3bd53e2c8b5677bc57143b35bb43e9b097f91cb75e0c3308a1be0595f","from":"busybox","Type":"container","Action":"start","Actor":{"ID":"2c3f35b3bd53e2c8b5677bc57143b35bb43e9b097f91cb75e0c3308a1be0595f","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service2","image":"busybox","name":"service1"}},"scope":"local","time":1635892340,"timeNano":1635892340508218250} {"status":"die","id":"2c3f35b3bd53e2c8b5677bc57143b35bb43e9b097f91cb75e0c3308a1be0595f","from":"busybox","Type":"container","Action":"die","Actor":{"ID":"2c3f35b3bd53e2c8b5677bc57143b35bb43e9b097f91cb75e0c3308a1be0595f","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service2","exitCode":"0","image":"busybox","name":"service1"}},"scope":"local","time":1635892340,"timeNano":1635892340527895321} {"Type":"network","Action":"disconnect","Actor":{"ID":"37a8a35f8d843207e72d4283cc777bdbbdc2b6d72f9631dcdf3e185a4ce3fc40","Attributes":{"container":"2c3f35b3bd53e2c8b5677bc57143b35bb43e9b097f91cb75e0c3308a1be0595f","name":"bridge","type":"bridge"}},"scope":"local","time":1635892340,"timeNano":1635892340570591353} {"status":"destroy","id":"2c3f35b3bd53e2c8b5677bc57143b35bb43e9b097f91cb75e0c3308a1be0595f","from":"busybox","Type":"container","Action":"destroy","Actor":{"ID":"2c3f35b3bd53e2c8b5677bc57143b35bb43e9b097f91cb75e0c3308a1be0595f","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service2","image":"busybox","name":"service1"}},"scope":"local","time":1635892340,"timeNano":1635892340595382775} {"status":"create","id":"833f9b0b118c8aff4953b0a09d28f757a20740a82263ca575ef06c8213bad30c","from":"busybox","Type":"container","Action":"create","Actor":{"ID":"833f9b0b118c8aff4953b0a09d28f757a20740a82263ca575ef06c8213bad30c","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service3","image":"busybox","name":"service1"}},"scope":"local","time":1635892340,"timeNano":1635892340850484951} {"status":"attach","id":"833f9b0b118c8aff4953b0a09d28f757a20740a82263ca575ef06c8213bad30c","from":"busybox","Type":"container","Action":"attach","Actor":{"ID":"833f9b0b118c8aff4953b0a09d28f757a20740a82263ca575ef06c8213bad30c","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service3","image":"busybox","name":"service1"}},"scope":"local","time":1635892340,"timeNano":1635892340853675299} {"Type":"network","Action":"connect","Actor":{"ID":"37a8a35f8d843207e72d4283cc777bdbbdc2b6d72f9631dcdf3e185a4ce3fc40","Attributes":{"container":"833f9b0b118c8aff4953b0a09d28f757a20740a82263ca575ef06c8213bad30c","name":"bridge","type":"bridge"}},"scope":"local","time":1635892340,"timeNano":1635892340899506704} {"status":"start","id":"833f9b0b118c8aff4953b0a09d28f757a20740a82263ca575ef06c8213bad30c","from":"busybox","Type":"container","Action":"start","Actor":{"ID":"833f9b0b118c8aff4953b0a09d28f757a20740a82263ca575ef06c8213bad30c","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service3","image":"busybox","name":"service1"}},"scope":"local","time":1635892341,"timeNano":1635892341160493528} {"status":"die","id":"833f9b0b118c8aff4953b0a09d28f757a20740a82263ca575ef06c8213bad30c","from":"busybox","Type":"container","Action":"die","Actor":{"ID":"833f9b0b118c8aff4953b0a09d28f757a20740a82263ca575ef06c8213bad30c","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service3","exitCode":"0","image":"busybox","name":"service1"}},"scope":"local","time":1635892341,"timeNano":1635892341180656407} {"Type":"network","Action":"disconnect","Actor":{"ID":"37a8a35f8d843207e72d4283cc777bdbbdc2b6d72f9631dcdf3e185a4ce3fc40","Attributes":{"container":"833f9b0b118c8aff4953b0a09d28f757a20740a82263ca575ef06c8213bad30c","name":"bridge","type":"bridge"}},"scope":"local","time":1635892341,"timeNano":1635892341227847223} {"status":"destroy","id":"833f9b0b118c8aff4953b0a09d28f757a20740a82263ca575ef06c8213bad30c","from":"busybox","Type":"container","Action":"destroy","Actor":{"ID":"833f9b0b118c8aff4953b0a09d28f757a20740a82263ca575ef06c8213bad30c","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service3","image":"busybox","name":"service1"}},"scope":"local","time":1635892341,"timeNano":1635892341249803569} {"Type":"network","Action":"destroy","Actor":{"ID":"41ae0bfcff9464ce75638facafbb1314287cbed7373ae36e58a20b8b8e59df5e","Attributes":{"name":"foobar","type":"bridge"}},"scope":"local","time":1635892341,"timeNano":1635892341501624688} {"Type":"volume","Action":"destroy","Actor":{"ID":"bla","Attributes":{"driver":"local"}},"scope":"local","time":1635892341,"timeNano":1635892341694193941}

Perhaps the events filter should also be tweaked to only include

container events

even-types die and start (as those are the only ones we're interested in?

filter by and the expected services

I think all of those should be supported by the API (see https://docs.docker.com/engine/api/v1.41/#operation/SystemEvents) which means that the client has a lot less to deal with, and there'd be no need to perform the filtering client side;

compose/pkg/compose/events.go

Lines 39 to 52 in 1ae9b3c

// TODO: support other event types

if event.Type != "container" {

continue

}

oneOff := event.Actor.Attributes[api.OneoffLabel]

if oneOff == "True" {

// ignore

continue

}

service := event.Actor.Attributes[api.ServiceLabel]

if len(options.Services) > 0 && !utils.StringContains(options.Services, service) {

continue

}

Running the above again, this time with some filters, shows that only the start and die events for the service2 and service3 containers are returned;

docker events \ --format='{{ json .}}' \ --filter type=container \ --filter event=die \ --filter event=start \ --filter label=com.docker.compose.project=bla \ --filter container=service2 \ --filter container=service3

{"status":"start","id":"ba946a5b4eeb566c919d536e725a80f0731e4edb5591c7db0fcc51143570b712","from":"busybox","Type":"container","Action":"start","Actor":{"ID":"ba946a5b4eeb566c919d536e725a80f0731e4edb5591c7db0fcc51143570b712","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service2","image":"busybox","name":"service2"}},"scope":"local","time":1635892805,"timeNano":1635892805107760894} {"status":"die","id":"ba946a5b4eeb566c919d536e725a80f0731e4edb5591c7db0fcc51143570b712","from":"busybox","Type":"container","Action":"die","Actor":{"ID":"ba946a5b4eeb566c919d536e725a80f0731e4edb5591c7db0fcc51143570b712","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service2","exitCode":"0","image":"busybox","name":"service2"}},"scope":"local","time":1635892805,"timeNano":1635892805129638234} {"status":"start","id":"5fc9d3ac8ac19fc04abc5c3208e97d4e0a221f55cf8e43ce2b52fad37305026c","from":"busybox","Type":"container","Action":"start","Actor":{"ID":"5fc9d3ac8ac19fc04abc5c3208e97d4e0a221f55cf8e43ce2b52fad37305026c","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service3","image":"busybox","name":"service3"}},"scope":"local","time":1635892805,"timeNano":1635892805721668621} {"status":"die","id":"5fc9d3ac8ac19fc04abc5c3208e97d4e0a221f55cf8e43ce2b52fad37305026c","from":"busybox","Type":"container","Action":"die","Actor":{"ID":"5fc9d3ac8ac19fc04abc5c3208e97d4e0a221f55cf8e43ce2b52fad37305026c","Attributes":{"com.docker.compose.project":"bla","com.docker.compose.service":"service3","exitCode":"0","image":"busybox","name":"service3"}},"scope":"local","time":1635892805,"timeNano":1635892805742243259}

I did wonder if this approach might be possible/preferable, subscribing only to the events of interest.

Big disclaimer that while I'm a maintainer for the docker engine, I'm not very familiar with the compose code in this repository, so mainly keeping 'half an eye' out of interest 😅

we indeed could filter by events we manage, to avoid unnecessary computation.
IIRC ContainerInspect is only required for die event to get retryCount

That approach works. Here's a quick proof-of-concept implementation: sdt/compose@72e4519...sdt:8747-add-event-filters

It adds an array of event names to the EventOptions struct, and creates an event filter for each name. The watchContainers function passes in the events it's interested in: currently just die and start.

I don't have any tests for this as yet - please let me know if you'd like me to proceed down either of these paths.

sdt added 2 commits October 31, 2021 15:18

Fix formatting with gofmt -s

80a3cf9

Knew I'd forget something. Signed-off-by: Stephen Thirlwall <[email protected]>

sdt force-pushed the 8747-dont-exit-on-container-destroy branch from cdc067b to 80a3cf9 Compare November 1, 2021 22:39

mat007 reviewed Nov 2, 2021

View reviewed changes

Fix typo in destroy event comment

4d0518e

Signed-off-by: Stephen Thirlwall <[email protected]>

thaJeztah reviewed Nov 2, 2021

View reviewed changes

ndeloof approved these changes Nov 4, 2021

View reviewed changes

ndeloof merged commit 95f0431 into docker:v2 Nov 4, 2021

sdt mentioned this pull request Nov 5, 2021

Attaching to a new container, docker compose up misses the initial logs #8884

Closed

sdt deleted the 8747-dont-exit-on-container-destroy branch December 12, 2021 20:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't exit on container destroy events #8859

Don't exit on container destroy events #8859

sdt commented Oct 31, 2021

mat007 left a comment

mat007 Nov 2, 2021

thaJeztah Nov 2, 2021

thaJeztah Nov 2, 2021

sdt Nov 2, 2021

thaJeztah Nov 2, 2021

ndeloof Nov 3, 2021

sdt Nov 4, 2021

	// Its already been removed from the watched map.
	// It's already been removed from the watched map.

	// TODO: support other event types
	if event.Type != "container" {
	continue
	}

	oneOff := event.Actor.Attributes[api.OneoffLabel]
	if oneOff == "True" {
	// ignore
	continue
	}
	service := event.Actor.Attributes[api.ServiceLabel]
	if len(options.Services) > 0 && !utils.StringContains(options.Services, service) {
	continue
	}

Don't exit on container destroy events #8859

Don't exit on container destroy events #8859

Conversation

sdt commented Oct 31, 2021

mat007 left a comment

Choose a reason for hiding this comment

mat007 Nov 2, 2021

Choose a reason for hiding this comment

thaJeztah Nov 2, 2021

Choose a reason for hiding this comment

thaJeztah Nov 2, 2021

Choose a reason for hiding this comment

sdt Nov 2, 2021

Choose a reason for hiding this comment

thaJeztah Nov 2, 2021

Choose a reason for hiding this comment

ndeloof Nov 3, 2021

Choose a reason for hiding this comment

sdt Nov 4, 2021

Choose a reason for hiding this comment