Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTCondor Docker Integration #2278

Merged
merged 7 commits into from May 17, 2016

Conversation

Projects
None yet
5 participants
@bgruening
Copy link
Member

commented Apr 30, 2016

The latest release of HTCondor supports the docker universe to encapsulate jobs into a containers.

This PR enables Galaxy to submit containers via the HTCondor job runner to your cluster.
An alternative approach would be kubernetes which @pcm32 is working on in #1902.

@abdulrahmanazab: this needs proper testing. Would be great if you can look at it.

@@ -1,9 +1,9 @@
<tool id="catdc" name="Concatenate datasets (in docker)">
<description>tail-to-head</description>
<requirements>
<container type="docker">busybox:ubuntu-14.04</container>
<container type="docker">bgruening/busybox-bash:0.1</container>

This comment has been minimized.

Copy link
@bgruening

bgruening Apr 30, 2016

Author Member

Since we depended on bash we need a new default container.
The source can be found here: https://github.com/bgruening/docker-busybox-bash

This comment has been minimized.

Copy link
@bgruening

bgruening Apr 30, 2016

Author Member

Happy to move all repos to the Galaxy project.

This comment has been minimized.

Copy link
@jmchilton

jmchilton May 2, 2016

Member

Let's revert the bash related changes in lieu of solving the problem of containers not having bash with #2282?

@galaxybot galaxybot added the triage label Apr 30, 2016

@galaxybot galaxybot added this to the 16.07 milestone Apr 30, 2016

@erasche erasche added kind/feature area/jobs and removed triage labels Apr 30, 2016

@bgruening bgruening referenced this pull request May 1, 2016

Merged

Add HTCondor support #65

jmchilton added a commit to jmchilton/galaxy that referenced this pull request May 2, 2016

Switch Dockerized commands to use sh instead of bash.
We switched to bash for the conda and that doesn't get sourced in containers - so we should be a bit pragmatic about this and switch back to /bin/sh for containers which is more common than /bin/bash.

xref galaxyproject#2278
@bgruening

This comment has been minimized.

Copy link
Member Author

commented May 2, 2016

@jmchilton reverted this particular commit. Thanks for looking at it!

@jmchilton

This comment has been minimized.

Copy link
Member

commented May 2, 2016

@bgruening this is obviously very exciting - thanks a bunch. How does condor know what volumes to mount in the Docker container? Is that configured globally - each container mounts Galaxy's datasets and index data? Is there an option to configure that on a per-job basis?

jmchilton added a commit to jmchilton/galaxy that referenced this pull request May 2, 2016

Switch Dockerized commands to use sh instead of bash.
We switched to bash for the conda and that doesn't get sourced in containers - so we should be a bit pragmatic about this and switch back to /bin/sh for containers which is more common than /bin/bash.

xref galaxyproject#2278

Update galaxy.ini.sample to mention this as suggested by @bgruening in galaxyproject#2282 (comment).
@bgruening

This comment has been minimized.

Copy link
Member Author

commented May 2, 2016

Sorry, I don't know much about the details of how this works. I implemented this as a request from our Norwegian friends (@abdulrahmanazab ...). For me this magically works and I know that condor is managing all input and outputs on their own if you specify something like should_transfer_files = YES. This can be set globally with this implementation.

A few more informations are here: https://research.cs.wisc.edu/htcondor/manual/v8.5/2_12Docker_Universe.html

Changing it on a per-job basis is supported via the original condor job runner. For example you could create a destination with:

<destination id="encrypt_condor" runner="condor">
      <param id="transfer_input_files">file1,dir1,file2</param>
      <param id="encrypt_input_files">file1</param>
</destination>

But as I'm said I'm not an expert in this and @erasche probably knows more about it. I hope @abdulrahmanazab can test all the glory details in his setup at Tjenester for Sensitive Data (TSD).

@abdulrahmanazab

This comment has been minimized.

Copy link
Contributor

commented May 14, 2016

@jmchilton The typical setup for docker support in HTCondor is to mount specific volumes (with the same path) globally, e.g. ..../database, then everything under ....database will be visible to the containers with the same path as is in Galaxy

@abdulrahmanazab

This comment has been minimized.

Copy link
Contributor

commented May 14, 2016

@bgruening Sorry about the delay which is my fault. But we had to prepare another PRACE deliverable for creating a docker job runner for slurm. I called it socker, using which slurm can run and control containers with cgroups. We may create another pull request soon ;) I'm flying to Colombia now for CCGrid2016. Will start running galaxy on HTCondor once I arrive.

@jmchilton

This comment has been minimized.

Copy link
Member

commented May 14, 2016

Thanks for the update - this clarifies a lot. I'm eager to merge this - I have a couple small implementation improvements I'd like to make first - if I haven't opened a PR against this branch by Monday afternoon with those though I'll just merge this as is.

Revise HTCondor Docker image resolution.
Bring inline with the rest of the job runner's Docker image resolution stuff. Order of reosolution is:

 - Use destination specified override if found: docker_container_id_override
 - Use the tool specified container if not found and no override: see tool
 - Else - use docker_default_container_id.

This adjusts the documentation to reflect this and make the the parameters more consistent throughout. I've modified this so docker_image (the old name introduced by the Docker Condor PR) still works - but I have left it undocumented intentionally for consistency.
@jmchilton

This comment has been minimized.

Copy link
Member

commented May 16, 2016

So my comments (implemented as a PR bgruening#10) are pretty much the same as my comments on the Kubernates PR (#2314) - I'm excited for the change I just want to remain consistent in how we resolve docker identifiers throughout tools and jobs in Galaxy.

This way - the work done to decompose the docker options as part of #2314 for instance will be instantly usable by this condor runtime mode.

@jmchilton jmchilton referenced this pull request May 16, 2016

Merged

Kubernetes job runner #2314

Merge pull request #10 from jmchilton/htcondor_docker
Revise HTCondor Docker image resolution.
@bgruening

This comment has been minimized.

Copy link
Member Author

commented May 17, 2016

@jmchilton thanks a lot for looking at it! If this all works out like planned .. we have an amazing new feature to play with :)
You rock!

@jmchilton jmchilton merged commit c2ea4ee into galaxyproject:dev May 17, 2016

4 checks passed

api test Build finished. 213 tests run, 0 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 105 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 581 tests run, 0 skipped, 0 failed.
Details
@bgruening

This comment has been minimized.

Copy link
Member Author

commented May 17, 2016

Thanks for merging @jmchilton!

@bgruening bgruening deleted the bgruening:htcondor_docker branch May 17, 2016

@nsoranzo nsoranzo referenced this pull request Jan 23, 2017

Merged

fix condor containers #3462

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.