[HTCondor-devel] Inclusion of additional arguments to docker run

[Matthew Hinton <matth@xxxxxxxxxxxxxx>]

Hi,Â

I am working with the docker universe at the moment, and moving a lot of our current processes over to using this code.Â

However, as part of this process, we required volume sharing with the container, since the input / output files used can be several GB, and therefore we don't want to be doing file copies.Â

I have therefore patched the source in the following, fairly hacky way (basically copying the code for docker_image). (I've attached a file, let me know if that doesn't work here). This source is now building and working as expected, but obviously there is no check performed or ability to add multiple volumes etc... for now.Â

In future it would be useful to be able to specify quite a few of the docker run arguments in the condor submit file. I therefore wanted to check if work along these lines is being done already, and therefore might be released soon, before I continue work along this branch to fulfil our requirements.Â

Thanks in advance,

Matt Hinton

diff --git a/src/condor_includes/condor_attributes.h b/src/condor_includes/condor_attributes.h
index 221d7fc..067074e 100644
--- a/src/condor_includes/condor_attributes.h
+++ b/src/condor_includes/condor_attributes.h
@@ -172,6 +172,7 @@
 #define ATTR_DISK  "Disk"
 #define ATTR_DISK_USAGE  "DiskUsage"
 #define ATTR_DOCKER_IMAGE "DockerImage"
+#define ATTR_DOCKER_VOLUME "DockerVolume"
 #define ATTR_DOCKER_VERSION  "DockerVersion"
 #define ATTR_EMAIL_ATTRIBUTES  "EmailAttributes"
 #define ATTR_ENTERED_CURRENT_ACTIVITY  "EnteredCurrentActivity"
diff --git a/src/condor_starter.V6.1/docker-api.cpp b/src/condor_starter.V6.1/docker-api.cpp
index bc11d68..a0a695d 100644
--- a/src/condor_starter.V6.1/docker-api.cpp
+++ b/src/condor_starter.V6.1/docker-api.cpp
@@ -11,8 +11,9 @@
 #include <algorithm>

 static bool add_env_to_args_for_docker(ArgList &runArgs, const Env &env);
+static bool add_volume_to_args_for_docker(ArgList &runArgs, const std::string &vol);
 static bool add_docker_arg(ArgList &runArgs);
-static int run_simple_docker_command(  const std::string &command,
+static int run_simple_docker_command(const std::string &command,
                                        const std::string &container,
                                        CondorError &e);

@@ -28,6 +29,7 @@ int DockerAPI::run(
        const std::string & command,
        const ArgList & args,
        const Env & env,
+       const std::string & volume,
        const std::string & sandboxPath,
        int & pid,
        int * childFDs,
@@ -42,7 +44,9 @@ int DockerAPI::run(
        if ( ! add_docker_arg(runArgs))
                return -1;
        runArgs.AppendArg( "run" );
-       runArgs.AppendArg( "--tty" );
+
+       //switched off to test problem with unix line endings.
+       // runArgs.AppendArg( "--tty" );

        // Write out a file with the container ID.
        // FIXME: The startd can check this to clean up after us.
@@ -88,9 +92,20 @@ int DockerAPI::run(
                return -8;
        }

+       //add the volume argument to the docker args
+
+
+       if ( ! add_volume_to_args_for_docker(runArgs, volume)) {
+               dprintf( D_ALWAYS | D_FAILURE, "Failed to pass volume to docker.\n" );
+               return -10;
+       }
+
+
        // Map the external sanbox to the internal sandbox.
        runArgs.AppendArg( "--volume" );
        runArgs.AppendArg( sandboxPath + ":" + sandboxPath );
+
+

        // Start in the sandbox.
        runArgs.AppendArg( "--workdir" );
@@ -106,10 +121,13 @@ int DockerAPI::run(
        uid = get_user_uid();
 #endif

-       if (uid == 0) {
-               dprintf(D_ALWAYS|D_FAILURE, "Failed to get userid to run docker job\n");
-               return -9;
+       if (uid <0 || uid >2147483647) {
+               uid = 0;
        }
+       //if(uid==0){
+       //      dprintf(D_ALWAYS|D_FAILURE, "Failed to get userid to run docker job.\n");
+       //      return -9;
+       // }
        runArgs.AppendArg("--user");
        runArgs.AppendArg(uid);

@@ -123,7 +141,7 @@ int DockerAPI::run(
        }

        runArgs.AppendArgsFromArgList( args );
-
+
        MyString displayString;
        runArgs.GetArgsStringForLogging( & displayString );
        dprintf( D_FULLDEBUG, "Attempting to run: %s\n", displayString.c_str() );
@@ -444,6 +462,15 @@ bool add_env_to_args_for_docker(ArgList &runArgs, const Env &env)
        return true;
 }

+bool add_volume_to_args_for_docker(ArgList &runArgs, const std::string &vol )
+{
+       if (vol.length()>0){
+               runArgs.AppendArg("--volume");
+               runArgs.AppendArg(vol+ ":" +vol);
+       }
+       return true;
+}
+
 int
 run_simple_docker_command(const std::string &command, const std::string &container,
                        CondorError &)
diff --git a/src/condor_starter.V6.1/docker-api.h b/src/condor_starter.V6.1/docker-api.h
index 8104176..4ea5ce0 100644
--- a/src/condor_starter.V6.1/docker-api.h
+++ b/src/condor_starter.V6.1/docker-api.h
@@ -38,6 +38,7 @@ class DockerAPI {
                                                const std::string & command,
                                                const ArgList & arguments,
                                                const Env & environment,
+                                               const std::string & volume,
                                                const std::string & directory,
                                                int & pid,
                                                int * childFDs,
diff --git a/src/condor_starter.V6.1/docker_proc.cpp b/src/condor_starter.V6.1/docker_proc.cpp
index d5e1493..3b1fcd0 100644
--- a/src/condor_starter.V6.1/docker_proc.cpp
+++ b/src/condor_starter.V6.1/docker_proc.cpp
@@ -84,7 +84,11 @@ int DockerProc::StartJob() {
                dprintf( D_ALWAYS, "Aborting DockerProc::StartJob: %s\n", env_errors.Value());
                return 0;
        }
-
+
+       std::string volume;
+       JobAd->LookupString( ATTR_DOCKER_VOLUME, volume );
+
+
        // The GlobalJobID is unsuitable by virtue its octothorpes.  This
        // construction is informative, but could be made even less likely
        // to collide if it had a timestamp.
@@ -130,8 +134,8 @@ int DockerProc::StartJob() {
        // will trigger the reaper(s) when the container terminates.

        ClassAd *machineAd = Starter->jic->machClassAd();
-
-       int rv = DockerAPI::run( *machineAd, containerName, imageID, command, args, job_env, sandboxPath, JobPid, childFDs, err );
+
+       int rv = DockerAPI::run( *machineAd, containerName, imageID, command, args, job_env, volume, sandboxPath, JobPid, childFDs, err );
        if( rv < 0 ) {
                dprintf( D_ALWAYS | D_FAILURE, "DockerAPI::run( %s, %s, ... ) failed with return value %d\n", imageID.c_str(), command.c_str(), rv );
                return FALSE;
diff --git a/src/condor_submit.V6/submit.cpp b/src/condor_submit.V6/submit.cpp
index e7ded38..f9d3721 100644
--- a/src/condor_submit.V6/submit.cpp
+++ b/src/condor_submit.V6/submit.cpp
@@ -523,6 +523,7 @@ const char* AcctGroupUser = "accounting_group_user";
 // docker "universe" Parameters
 //
 const char    *DockerImage="docker_image";
+const char       *DockerVolume="docker_volume";

 //
 // VM universe Parameters
@@ -2127,7 +2128,7 @@ SetExecutable()
                        exit(1);
                }
                char * image = check_docker_image(docker_image);
-               if ( ! image || ! image[0]) {
+               if ( ! image || ! image[0]){
                        fprintf(stderr, "\nERROR: '%s' is not a valid docker_image\n", docker_image);
                        DoCleanup(0,0,NULL);
                        exit(1);
@@ -2136,6 +2137,15 @@ SetExecutable()
                InsertJobExpr(buffer);
                free(docker_image);
                ignore_it = true; // we don't require an executable if we have a docker image.
+
+               //sort out the volume stuff
+               char * docker_volume = condor_param(DockerVolume, ATTR_DOCKER_VOLUME);
+               if (docker_volume){
+                       buffer.formatstr("%s = \"%s\"", ATTR_DOCKER_VOLUME, docker_volume);
+                       InsertJobExpr(buffer);
+               }
+               free(docker_volume);
+
        }

        ename = condor_param( Executable, ATTR_JOB_CMD );