Add an explicit noop option for minimal containers that do not have echo

[vizowl]

In order to support data only volumes using minimal images #514 I have added an option that allows a no-op command to be explicitly specified - currently it is assumed to be echo which isn't necessarily available on a minimal image.

I have added a test and updated the documentation.

I am concerned that this approach essentially exposes an internal feature ...

[kojiromike]

👍 Thanks for tackling this issue!

[dnephin]

It looks like noop is just being used as the entrypoint. Why not use entrypoint ?

This only changes an intermediate container, which I don't think is ever started, so I'm not sure this is doing what you expect. If you want to have a data volumes container, you can set entrypoint on it to something that exists.

[kojiromike]

@dnephin fig up recreates existing containers, and in doing so creates intermediate ones for dangling volumes-from. The intermediate must not do anything with its entry point, and it's not safe to assume that the original container's entry point does nothing. I agree that this exposes an internal feature. What I don't understand is why fig up recreates containers every time. In other words, why isn't --no-recreate the default behavior for fig up, with an optional --recreate flag?

[dnephin]

I was also not sure why recreate existed. I believe it is so that the new container has the same name, while still supporting volumes-from the original container. Otherwise you wouldn't be able to do both (preserve the name, and copy the volumes).

What I'm saying is that I believe if your service has an entrypoint you won't need to provide this noop, you can just do (using your test case):

datavol:
  image: hello-world
  entrypoint: /hello
  volume:
    - /mydata

Which I believe is the same as what you're proposing, just without having to make any code changes. Does this not work? I would think that since the intermediate container never runs, it doesn't really matter what's in the entrypoint?

Maybe you'd still have to make a smaller code change? (use self.options.get('entrypoint', ['echo']) instead ? (although i'm not sure I understand why this would be necessary).

[kojiromike]

@dnephin yes, setting entrypoint in yaml does not solve the problem. The intermediate container always has echo as its entrypoint, regardless of the entrypoint in yaml. That's what #514 is about.

[dnephin]

Ok, right. But you can probably still fix this with the one line change right?

(on the intermediate container)

entrypoing=self.options.get('entrypoint', ['echo']),

That way setting the entrypoint will be preserved from the yaml.

[kojiromike]

@dnephin this is where noop comes in. It's not safe to assume or require that the entrypoint actually be a noop, but by explicitly calling it that, it should be clear to users that that command must do nothing.

[dnephin]

I don't see how that's an assumption. What you're actually doing is setting an entrypoint. Calling it noop is misleading. noop is not a docker thing at all. It's introducing some new idiom that is really just a duplicate of an existing one.

[vizowl]

@dnephin what you are proposing was my initial solution. However I then realised that that would break the recreate behaviour when using volumes-from on a container with a real entry-point - e.g.

db:
  image: postgres
backup:
  image: debian
  volumes_from: 
    - db
  command: my-backup-script

The standard library postgres image defines an entrypoint which starts postgres which should not be run during the recreate phase when a user just wants access to the volumes.

[dnephin]

I see what I was missing. intermediate_container is actually started. I wonder if that is still necessary.

[nathanleclaire]

👍 for this. We just gotten bitten by it.

[jpetazzo]

👍 .

/cc @tianon

[bfirsh]

I wonder if starting the container is still necessary too. I wonder if we could also use a small image with a known executable in it.

/cc @aanand

[dnephin]

I tried the test suite without the start and wait. There was a volume on the recreated container, but it had a different id. I haven't tried a volume with files to see if the files match.

I like the idea of a small image with known executable. I was also thinking that if docker supported removing the name from a container you would be able to do this recreate without the need for an intermediate container.

[aanand]

The current behaviour is hacky as hell, and while I appreciate the need for a solution to the no-op problem, I'm nervous about adding one more hack to the tower.

Could we dispose with volumes_from entirely? What if we instead look at the volumes configuration of the existing container and copy it wholesale? That way we don't need an intermediate container - we can destroy the existing one and then start a new one with the same name and config.

We'd need to decide what to do when users make changes to a service's volumes config, but we already have that problem - we just don't solve it (the new container has the new volumes config + volumes_from the intermediate container, which causes weird behaviour when they change a host path).

[bfirsh]

Looks like volumes_from moved from create to start in 0.10.0: 80991f1

[bfirsh]

@aanand that sounds like a good idea.

[bfirsh]

@dnephin @jpetazzo @nathanleclaire How major do you think this problem is? How did you get bitten by it? Do you think it's a blocker for 1.0.0?

[dnephin]

I have not had a problem with it. I build data volumes on busybox, which has echo.

Seems to me like the workaround is easy enough (use busybox instead of scratch). I believe the busybox image is only 2.4M, which seems like negligible overhead. I would say not a blocker personally (I just thought the issue was interesting!).

[tianon]

This would work for tianon/true, where /true could be the "noop" command, but for a container like tianon/sleeping-beauty, there's not something suitable unless it doesn't matter if the command never exits unless it gets a mean signal like SIGINT (a "kiss" from the prince, so to speak). I wasn't bitten by this, but I think @jpetazzo probably CC'd me because I'm one of those crazy people with images that are tiny enough to actually cause problems with assuming echo is available. 😄

[tianon]

(and I plan to make more of them in the future, and hopefully help make them easier to create so other people make more of them, too)

[jpetazzo]

I got bitten by it while using sleeping-beauty for an "environment
container".

IMHO, we could use busybox (or something like tianon/true) for the
"switchover" container, because it's better to rely on the existence of the
busybox container, than to rely on the existence of "echo" in the
container. But we're borderline religious here ;-)

On Thu, Oct 9, 2014 at 8:16 AM, Tianon Gravi notifications@github.com
wrote:

(and I plan to make more of them in the future, and hopefully help make
them easier to create so other people make more of them, too)

—
Reply to this email directly or view it on GitHub
#517 (comment).

@jpetazzo https://twitter.com/jpetazzo
Latest blog post:
http://jpetazzo.github.io/2014/06/23/docker-ssh-considered-evil/

[kojiromike]

If the builtin noop is assumed to be /bin/echo I can always COPY true-asm /bin/echo. The problem is that if the noop is echo, a minimalist container can't even find it. fig assumes the box has enough of a shell to do PATH lookups.

[dnephin]

That sounds like a good short-term solution to me (change to /bin/echo).

[bfirsh]

+1 #534

[bfirsh]

The problem with using tianon/true or busybox is we have to pull it somehow. Needs a bit more thought than this change, so let's get a proper fix for this in 1.0.1.

[jpetazzo]

Out of curiosity: what's wrong with pulling busybox?

On Fri, Oct 10, 2014 at 3:13 AM, Ben Firshman notifications@github.com
wrote:

The problem with using tianon/true or busybox is we have to pull it
somehow. Needs a bit more thought than this change, so let's get a proper
fix for this in 1.0.1.

—
Reply to this email directly or view it on GitHub
#517 (comment).

@jpetazzo https://twitter.com/jpetazzo
Latest blog post:
http://jpetazzo.github.io/2014/06/23/docker-ssh-considered-evil/

[bfirsh]

@jpetazzo Nothing – it sounds like a good plan. I just wanted to get the simplest possible fix in for 1.0.0, then we can do a proper fix for 1.0.1.

It's a little complex because we don't want to pull busybox on every recreate, so there has got to be some intelligence there. Perhaps just trying to create the container, then if busybox doesn't exist, pulling it. Still, much more complex than 4 extra characters. ;)

[tianon]

If you try to run it and it isn't there, won't the Daemon just auto-pull it?

[bfirsh]

@tianon Nope, that's done client-side!

[tianon]

Pretty sure I just threw up in my mouth, but I definitely believe you.

[hugoduncan]

Just hit this issue with an image that doesn't have /bin/echo. Using a known image for the intermediate container sounds like the best solution to me.

[rocketraman]

I just hit this issue as well -- my data container uses tianon/true, which doesn't have /bin/echo.

[tpires]

👍 Just got bitten by it.

Using busybox as @dnephin has proposed, and added entrypoint: /bin/echo .

[pwaller]

As I pointed out over here:

Is it possible that the no-op is no longer required since moby/moby#8942 was fixed in moby/moby#9089 ? That would mean that starting the intermediate container would be unnecessary.

[bfirsh]

@aanand @dnephin Is it correct that we no longer need to start the intermediate container? I think some combination of this and using renaming instead, I think we can safely close this...

[dnephin]

I agree