Bug 1126874

will be reproducible by autotest docker test restart.

git clone –recursive git://github.com/autotest/autotest.git
cd autotest/client/
./autotest-local run docker --args="docker_daemon/restart"

This pull req countaine docker_daemon/restart tests
https://github.com/autotest/autotest-docker/pull/255

Could you recheck with docker-1.2

moving docker bugs off alexl

This is waaaaay too docker specific for me to make any sense of how this is a problem with DM thinp (kernel) code.

The comment#0 is very difficult for me to understand.  Not understanding why vbatts cannot own fixing this problem but consult me as needed.

Reassigning to Dan Walsh... ;)

Ok, So there seems to be two problems here.

- Should an init inside the container still be ignore SIGTERM if it is sent from host or any of the parent pid namespaces. I am not sure about the answer to that question but I kind of feel that it sounds wrong. How is one supposed to stop a container otherwise.

- There seem to be a problem on docker side too. If device is busy devmapper backend will return error to docker  and "docker rm" operation should fail. For some reason it looks like docker might be ignoring that code and still continue ahead with deletion of some metadata associated with the container. And when docker later starts, it probably is loading all the metadata and it finds the files associated with container in devmapper backend. As it has lost the original name of container, it generates one.

I think second behavior should get in docker for sure. Fail "docker rm" if backend can not finish the operation and returns error.

Ok, w.r.t first problem (init in pid namespace ignores SIGTERM), there has been some discussion.

https://lists.openvz.org/pipermail/devel/2010-May/024843.html
http://lwn.net/Articles/532748/

Bottom line seems to be that init needs to establish a SIGTERM handler. I am not sure sure if SIGTERM will be blocked from processes in same container or child/peer container or not. I hope it will be.

If that's the case, can we modify docker init to establish SIGTERM handler and
hopefully that will take care of the problem.

Irrespective of that, docker still needs to be fixed to handle error from devmapper backend though.

I guess following code is the problem.

Destroy() {
        // Deregister the container before removing its directory, to avoid race conditions
        daemon.idIndex.Delete(container.ID)
        daemon.containers.Delete(container.ID)
        container.derefVolumes()
        if _, err := daemon.containerGraph.Purge(container.ID); err != nil {
                log.Debugf("Unable to remove container from link graph: %s", err)
        }

        if err := daemon.driver.Remove(container.ID); err != nil {
                return fmt.Errorf("Driver %s failed to remove root filesystem %s: %s", daemon.driver, container.ID, err)
        }

}

we seem to delete container first from containerGraph and cleanup some
other metadata before we actually call into driver to Remove().

I would think that if we call into driver first to Remove() and cleanup
container metadata only after a successful call this problem should be fixed.

Not sure if there  are any other implications of changing this order.

Second part of the problem should be fixed by this commit. If device removal failed from device mapper, then docker will report it instead of silently removing container data from its metadata.

https://github.com/docker/docker/pull/11137

This is being tracked in following bz.

https://bugzilla.redhat.com/show_bug.cgi?id=1194909

Ok, this bug is combination of 3 issues.

- Restarting docker service did not kill the running container.
- As container was not killed, underlying device was still busy, hence docker rm operation failed but container was deleted from the view and it should not have been.
- When docker service was restarted, deleted container reappeared with a new regenerated name.

First problem seems to have been fixed upstream. I am testing latest docker bits and now container gets killed. I am not sure which set of patches fixed it.

Second problem should be fixed in 1.6.0 where my patches are there to not remove container from active view.

I think third problem is still there. I see that container reappears with new name. I will have to dig deeper into it.

Vivek any update on this bug?