Systemd-docker client detaches if -d option is given by user

[apatil]

Hi @ibuildthecloud,

My use case for wanting -d is running containers in ExecStartPre. I still want systemd-docker to do its job, but I need the client to exit afterward so that the other ExecStartPre's and ExecStart can run.

Thanks!

[ibuildthecloud]

This slightly changes the behaviour. If you didn't have a -d in your docker run command, systemd-docker would automatically add it because it is required for it to work. So if you have -d or not in the run command, systemd-docker will behave the same. With this PR it changes slightly. I think it would be better to add -d to the options of systemd-docker itself. So the command would become systemd-docker -d run .... Then it becomes clear that -d is changing the behaviour of systemd-docker itself and not the arguements of docker run.

[LadyNamedLaura]

Hi @apatil,

Have you tested this usecase? afaik systemd kills (or at least should kill, if it doesn't that is a bug, and I will try to fix it) anything left over by ExecStartPre, so not only systemd-docker, but your whole container (since systemd-docker moves it into the service cgroup) will be killed after systemd-docker detaches.

I am not against adding a -d option for debugging, or for using Type=forking, but the ExecStartPre usecase can or at least should not work.

Greetings Simon

[apatil]

@ibuildthecloud: OK, I understand. I can change it accordingly.

@SimonPe: Ouch, I hadn't thought of that, and hadn't tested from an ExecStartPre, just the CLI. I'll have to move my ExecStartPre stuff into ExecStart. This feature is still a win for me in that case, if a slightly smaller win.

[apatil]

@ibuildthecloud systemd-docker has its own detach argument now.

[ibuildthecloud]

@apatil Hmm, so I just merged this PR, but then it dawned on me that systemd-docker may already do what you want. If you run systemd-docker --logs=false run ... it doesn't block but just spawns the containers and continues. Effectively doing the same thing as this PR.

[apatil]

@ibuildthecloud Come to think of it the code also looks to me like it would do that, but I've tested and it doesn't work that way for me; --logs=false without -d stays attached. This is both with my pr's version and the previous version, faf1c44d3ad550b4536cea7013ad5d112d96bf9f.

[ibuildthecloud]

@apatil If you use --rm or --logs=true it will stay attached. Are you using --rm? If you are, with your PR, the --rm is being stripped and not sent to docker so it might not be doing what you expect.

[apatil]

@ibuildthecloud Ah ok, I didn't test without --rm with the previous version.

With the current options, it looks like there's no way to have --rm=false and --logs=false and stay attached. -d=false could force attachment in that case, although it doesn't currently. I'm not sure whether that's enough of a win to justify a new option.

I don't have a strong opinion about whether to keep -d or add something to the readme explaining how to get the client to detach. What do you think?

[ibuildthecloud]

@apatil So I think I'm in favor of rolling back this PR. The problem I see is that -d and --rm are conflicting options. You can't do both of those in docker and the same applies to systemd-docker. So the only way to properly run with systemd-docker -d run ... is to have --rm=false. (Right now if you do --rm=true, systemd-docker is switching it to --rm-false, which is odd for the user.) From that perspective -d essentially the same as --logs=false. It would be nice to support systemd-docker -d run --rm ... but I think the only feasible way to actually do that is for systemd-docker to background itself which is a much bigger change.

[ibuildthecloud]

So I think a documentation change properly explaining how to get systemd-docker to detach would be good.

[apatil]

Added the README section in https://github.com/ibuildthecloud/systemd-docker/pull/8 .