Support OPT1, OPT2 in install/run command

[sub-mod]

@rhatdan

The Labels look like below.
LABEL RUN docker run -it --rm ${OPT1} --privileged -v pwd:/atomicapp -v /run:/run -v /:/host --net=host --name ${NAME} -e NAME=${NAME} -e IMAGE=${IMAGE} ${IMAGE} -v ${OPT2} run ${OPT3} /atomicapp

LABEL STOP docker run -it --rm ${OPT1} --privileged -v pwd:/atomicapp -v /run:/run -v /:/host --net=host --name ${NAME} -e NAME=${NAME} -e IMAGE=${IMAGE} ${IMAGE} -v ${OPT2} stop ${OPT3} /atomicapp

LABEL INSTALL docker run -it --rm ${OPT1} --privileged -v pwd:/atomicapp -v /run:/run --name ${NAME} -e NAME=${NAME} -e IMAGE=${IMAGE} ${IMAGE} -v ${OPT2} install ${OPT3} --destination /atomicapp /application-entity

Usage:
atomic install --opt1="-p :5000" --opt2="-d" submod/guestbookphp-app
used in projectatomic/atomicapp#152
cockpit-project/cockpit#2474

[goern]

can we provide a default to RUNARGS via a LABEL in a Dockerfile? Or should such a default be moved to the INSTALL/RUN LABEL?

[sub-mod]

@goern can you give an example
i dont think we can avoid putting RUNARGS in install/run labels

[vpavlin]

LABEL RUN docker run ... IMAGE RUNARGS /project
LABEL RUNARGS -v

atomic run $image

if not args.runargs:
  runargs = getLabel("RUNARGS")

Basically baing able to provide a default RUNARGS in LABELS so that we have a default behaviour specified and user can override it easily

[rhatdan]

I have been asked to put in multiple of these.

For example some people want to put args before the image and some after. Should it just be OPTION1, OPTION2, OPTION3?

atomic run --opt1 "-b foobridge" --opt2 "--ti"

LABEL RUN "docker ${opt1} run ${opt2} -n ${NAME} ${IMAGE}

[rhatdan]

This would allow users to specify multiple options where the command could take options.

[rhatdan]

I have a nice patch that I am about to accept that would just set environment variables and would eliminate the need to do replacements.

[sub-mod]

@rhatdan can you please share the patch

[rhatdan]

@sub-mod The patch has been merged into docker.

Now we need a patch to allow --opt1, --opt2, --opt3 patch.

[sub-mod]

@rhatdan I am working on it now.
Can you please give me the docker patch PR

[bexelbie]

@sub-mod I believe that @rhatdan may have been referencing #66

[sub-mod]

@bexelbie thanks, i tried that .It doesnt work for me. Has anyone tried that ? @rhatdan

[bexelbie]

@sub-mod can you be more specific about what isn't working? The substitution seems to be working fine.

[sub-mod]

@rhatdan kindly review.
This works for me,

[rhatdan]

Merge the two pull requests together, and I think we should only set the environment variable if the options are used. Perhaps we should have three also.

opt1, opt2, opt3.

We should not be mentioning nulecule in the man pages. I think these options should be available for run, install and uninstall, Can be used to pass arguments to the LABEL RUN|INSTALL|UNINSTALL as ${opt1}. Then we should show an example for each man page.

LABEL RUN docker run -d ${opt1} -t ${NAME} ${IMAGE} ${opt2} --foobar ${opt3}

[purpleidea]

Without understanding all the specifics of this patch (since I didn't need it) it seems this might be similar to the feature needed for a spc puppet apply container for example.

[sub-mod]

@rhatdan kindly review

[rhatdan]

How about we change

 +    installp.add_argument("--opt3", dest="opt3", help="additional arguments to be passed for install Sub Command.") 

to

 +    installp.add_argument("--opt3", dest="opt3", help="additional arguments to be passed as ${OPT3} for install Sub Command.") 

For each --opt command.

[sub-mod]

@rhatdan kindly review

[rhatdan]

I think --opt1, --opt2 should be options of atomic run, atomic install, atomic uninstall.

Not just atomic.

It looks like you made --opt3 specific to each subcommand. These make no sense to separate out.

atomic run --opt1 "-d"
is better then

atomic --opt1 "-d" run

[sub-mod]

here is an example
atomic --opt1="-p :5000" --opt2="-d" install --opt3="--update" submod/guestbookphp-app

internally it calls
atomicapp -d install --update submod/guestbookphp-app
opt1 is for docker
opt2 is for atomicapp
opt3 is for sub commands of atomicapp..like install run uninstall
I underdstand this doesn't look pretty

[rhatdan]

You are substituting for the LABEL command, has nothing to do with docker, All we really are saying is substitute OPT1 with the contents of --opt1. We don't care where $OPT1 is with the LABEL. IE Whether it is early in the docker line or late.

LABEL RUN "docker ${OPT1} run ${IMAGE} ..."
LABEL RUN "docker run ${OPT1} ${IMAGE} ..."
LABEL RUN "docker run ${IMAGE} ${OPT1} ..."
Are all valid. If I don't have three substitutions in my LABEL, I would only use --opt1.

[sub-mod]

ahh ok makes sense
so is this OK ?
atomic install --opt1="-p :5000" --opt2="-d" --opt3="--update" submod/guestbookphp-app ?

[sub-mod]

With the way LABELs are written and used by atomic ,Suppose i want to expose a port in the atomic docker container and if it(-p option ) is not already mentioned in the LABELS , there no way other than using --opt1
So i was wondering should we FIX the location of OPT1 for docker only ?

[rhatdan]

No I don't think we should fix the location of --opt1/${OPT1} Leave it fully up the the image developer to specify how he wants the options to be used.

[rhatdan]

Remove "Sub"

...uninstall Sub Command.

Should be
...uninstall command.

[sub-mod]

ok done.

[rhatdan]

@sub-mod Sorry to be a pain, but lowercase Command and then I will merge.

Other then this, it LGTM

[sub-mod]

np