Revisiting the defaults for `atomic run` with and without --spc about atomic

[rhatdan]

@sctweedie What do you think?

I am not crazy about breaking current behaviour, but I would like a way to differentiate between a RUN Once container, and a run some container multiple times.

atomic run currently handles the rhel-tools container fairly well, although users get confused on the first run. But people building containers to run once and specify a command can get confused.

[rhatdan]

@riekrh Comment?

I think the original idea was to RUN the initial command for the rhel-tools image and the leave the container around for quick execution of subsequent RUNs (vs. recreating multiple instances of the container - which is not intended - or using --rm - which is slow).

I agree that consistency for --spc is desirable and differentiating between CREATE and RUN is a good idea. But perhaps we need an explicit flag for the re-use existing vs create new instance point?

[bexelbie]

Maybe the default case should be the simplest one and we should allow containers like rhel-tools to have sufficient freedom to define the create/exec functionality?

I agree - at the end it should be defined in the container, so the end-user
is not exposed to decisions.

On Thu, Jul 9, 2015 at 10:13 AM, Brian Exelbierd [email protected]
wrote:

Maybe the default case should be the simplest one and we should allow
containers like rhel-tools to have sufficient freedom to define the
create/exec functionality?

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

[bexelbie]

@riekrh would you support the proposal above, with the addition that CREATE is not executed if the container is already running?

I think there should be a 'reuse_existing_container' LABEL controlling whether run (and start) will re-create the container.

[rhatdan]

Can we get a little less awful label.

LABEL shared_container

But the problem is how do we handle existing containers. I guess we could continue to look to see if a container exists, but if the container does not exist and the LABEL shared_container does not exist it just does the run command not the create

On Thu, Jul 9, 2015 at 12:13 PM, Daniel J Walsh [email protected]
wrote:

Can we get a little less awful label.

LABEL shared_container

But the problem is how do we handle existing containers. I guess we could
continue to look to see if a container exists, but if the container does
not exist and the LABEL shared_container does not exist it just does the
run command not the create

That should work for the transition. Can drop it later.

[bexelbie]

I am not 100% sure we need a label for a shared container. Wouldn't it be enough for the RUN to not include --rm? This assumes that the default isn't trying to support multiple style use cases. Perhaps we could move to this default:

atomic run NAME COMMAND

if container exists
   verify it is the same version as the image of the same name, if not warning message
if container is STOPPED or CREATED
  if START label
    run START label and continue
  else
    run `docker start NAME` and continue
if container is RUNNING
   if EXEC label
     EXECUTE EXEC label and return
   else
     EXECUTE `docker exec -it NAME [COMMAND] and return
if RUN label
  EXECUTE RUN label and return
else
  cmd = `docker run -i -t --rm --name NAME NAME COMMAND`
  if SPC
    cmd also gets `--privileged -v /:/host -v /run:/run -v /etc/loaltime:/etc/loaltime --net=host --ipc=h
ost --pid=host -e HOST=/host -e NAME=NAME -e IMAGE=IMAGE`
  if COMMAND
    cmd += COMMAND
  EXECUTE cmd and return

If you need a container that persists you will need to include a custom RUN that either daemonizes the container or doesn't clean it up. rhel-tools does this. We could also just state SPC loses the --rm if we wanted to in the default.

Maybe that is is sufficient. Can someone play through the scenarios?

On Wed, Jul 15, 2015 at 6:43 AM, Brian Exelbierd [email protected]
wrote:

I am not 100% sure we need a label for a shared container. Wouldn't it be
enough for the RUN to not include --rm? This assumes that the default isn't
trying to support multiple style use cases. Perhaps we could move to this
default:

atomic run NAME COMMAND

if container exists
verify it is the same version as the image of the same name, if not warning message

if container is STOPPED or CREATED
if START label
run START label and continue
else
run docker start NAME and continue

if container is RUNNING
if EXEC label
EXECUTE EXEC label and return
else
EXECUTE `docker exec -it NAME [COMMAND] and return

if RUN label
EXECUTE RUN label and return
else
cmd = docker run -i -t --rm --name NAME NAME COMMAND
if SPC
cmd also gets --privileged -v /:/host -v /run:/run -v /etc/loaltime:/etc/loaltime --net=host --ipc=h ost --pid=host -e HOST=/host -e NAME=NAME -e IMAGE=IMAGE
if COMMAND
cmd += COMMAND
EXECUTE cmd and return

If you need a container that persists you will need to include a custom
RUN that either daemonizes the container or doesn't clean it up. rhel-tools
does this. We could also just state SPC loses the --rm if we wanted to in
the default.

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

[rhatdan]

That seems to work for me. We need @sctweedie to comment on this. When is he back from PTO?