job_manager: Add htcondorVC3 job manager #251

CodyKank · 2020-04-27T15:54:33Z

Integrating the VC3 job manager class for 0.6 branch.

Added htcondor submission for workflow steps. No monitoring of any file transfers is implemented yet.

Add Cody changes for condor submission

…ller Conflicts: reana_job_controller/rest.py Merged upstream changes. Commit condor changes. Updated to most recent rjc. Added delete job for condor.

Update RJC

Still a lot of room for improvement.

…ller into cody_dev Conflicts: reana_job_controller/rest.py setup.py Merge khurtado's RJC with reanahub's current master. Conflicts involved changing the hardcoded paths within rjc/rest.py. Setup.py didn't like the added 'htcondor' requirement.

Create first instance of HTCondor's job manager There are a few VC3 specifics within. Testing is still needed.

Added ability to call htcondor_job_manager over the already defined kubernetes_job_manager.

Cody dev

- Implement watch jobs method for condor.

Changed how the htcondor job wrapper searches for a container. Instead of assuming singularity now searches through a list to be expanded later. Singularity is still hardcoded with arguments.

Erroneously left out shifter in module search list, fixed incorrect usage of cntr_path in job_wrapper.

To account for environments with bash versions > 4.0 associative arrays in job_wrapper for htcondor were removed and replaced with simple arrays. Adjusted call to modules within job_wrapper due to failure on Blue Waters.

First implementation to search for shifter

…ller into reanahub-master Conflicts: setup.py

Should only apply to tag 0.6.0

Removed an older method for passing the htcondor schedd address to the container, instead of at build time we moved it long ago to within the yaml configuration file for reana-cluster

Bump HTCondor installed through pip from 8.9.1 to 8.9.6

condor.py was used in the past before the creation of job_manager classes.

tiborsimko · 2020-04-28T07:28:10Z

reana_job_controller/variables.py

+
+import os
+
+MAX_JOB_RESTARTS = 3


If these variables are used only in VC3 context, it would be good to name the file more specifically. Examples:

Do we need to have them as separate file? If not, consider moving into htcondorvc3 class.

Shall they be reused for other VC3 backends than just HTCondor? If yes, leave them in separate file named like vc3_config.py or vc3_variables.py.

We can always use vc3_... name prefix technique for other possible VC3-specific files such as job wrapper. Currently it lives in files/job_wrapper.sh, it may be better to use something like scripts/vc3/job_wrapper.sh.

@khurtado any thoughts on if we'd like to move these or prefix the naming?

Looking into it, it seems we do not use these 2 anymore:

reana-job-controller/reana_job_controller/variables.py

Lines 16 to 17 in 0e76609

SHARED_VOLUME_PATH = os.getenv('SHARED_VOLUME_PATH', '/var/reana')

SHARED_VOLUME_PATH_ROOT = os.getenv('SHARED_VOLUME_PATH_ROOT', SHARED_VOLUME_PATH)

So, I think we can simply remove variables.py, get rid of this line and set MAX_JOB_RESTARTS = 3 below the import calls in htcondorvc3_job_manager.py

tiborsimko · 2020-04-28T07:30:35Z

reana_job_controller/variables.py

+# -*- coding: utf-8 -*-
+#
+# This file is part of REANA.
+# Copyright (C) 2017, 2018 CERN.


Great to see your contribution!

Please amend copyright years.

Please add yourself and @khurtado to the author list in AUTHORS.rst.

Regarding merging, we usually merge feature branches with linear history. There are basically two options: (1) Shall we rebase and keep the history? (2) Shall we squash everything under one commit? What would you prefer?

I'll add us to AUTHORS.rst!

Regarding merging, we are fine with whatever would be easier for the REANA team. If squashing everything under one commit is easier for you then we are fine with that, the history of our commits is not as important to us as integrating them.

tiborsimko · 2020-04-28T07:49:31Z

files/job_wrapper.sh

+#!/bin/bash
+
+# Replicate input files directory structure
+# @TODO: This could be executed 


We have:

etc/job_wrapper.sh files/job_wrapper.sh

It is not clear which files are for what.

What about using:

etc/htcondorcern/job_wrapper.sh etc/htcondorvc3/job_wrapper.sh

so that we can easily support several backends and distinguish between them?

This would mean that we'd have to move quite a few CERN-specific files from etc/* to /etc/htcondorcern/*` ourselves... but merging this branch will be quite a lot of work anyway, so could perhaps do it alongside?!

(An alternative would be to use the same directory but strictly "cern" and "vc" naming prefix everywhere.)

I like creating different subdirectories in etc:

etc/htcondorvc3/job_wrapper.sh etc/htcondorcern/job_wrapper.sh

As you said, handling several different backends would be easier to manage and visualize than prefixes or suffixes (job_wrapper_cern.sh / cern_job_wrapper.sh).

If you're OK with the subdirectories in etc/, I'd be happy to help move CERN specific files as well as the VC3 files.

tiborsimko · 2020-04-28T07:50:33Z

Dockerfile

@@ -77,5 +80,6 @@ EXPOSE 5000

 ENV COMPUTE_BACKENDS $COMPUTE_BACKENDS
 ENV FLASK_APP reana_job_controller/app.py
+ENV REANA_LOG_LEVEL DEBUG


It should not be necessary to hard-code REANA_LOG_LEVEL in Dockerfile. One could pass it as environment variable...

Just to add a bit more information on this: the way of passing this configuration is a bit cumbersome right now as one would have to: go inside RWC, read the config value and pass it as an environment variable to RJC. In reanahub/reana#277 there is a description of the current process and possible solutions we could take to centralise these configuration variables.

Yes this should have been taken out, I missed this when cleaning things up.

tiborsimko · 2020-04-28T07:54:51Z

reana_job_controller/htcondorvc3_job_manager.py

+    def __init__(self, docker_img=None, cmd=None, prettified_cmd=None,
+                 env_vars=None, workflow_uuid=None, workflow_workspace=None,
+                 cvmfs_mounts='false', shared_file_system=False,
+                 job_name=None, kerberos=False, kubernetes_uid=None):


We have added in the meantime support for running unpacked Singularity images from CVMFS. (See unpacked_img in master branch.) Is this something that would interest you for VC3 integration as well? If yes, this will be something to look into. If not, we can either raise NotImplemented or pass while merging. Asking just for the future in order to know how to propagate v0.6.0 -> master changes to the HTCondorJobManagerVC3 class the best.

In the current moment we haven't explored CVMFS very much as we are targeting mainly National Science Foundation HPC sites here in the US which will not support it, so for now when merging a raise NotImplemented may be best.

diegodelemos · 2020-04-28T08:11:56Z

Dockerfile

+    pip install --upgrade pip && \
+    pip install htcondor==8.9.6 retrying


Do we need to have these libraries installed at this specific point in the Dockerfile, maybe because of the deb packages? Otherwise, we could just continue setting them in setup.py so they will be installed at this point, and later available in the code .

You're right, these should be handled in setup.py, I'll clean these up.

diegodelemos · 2020-04-28T08:50:07Z

reana_job_controller/config.py

+# How is this set in the environment?
+# It is hardcoded in Dockerfile and there is no code
+# in workflow-controller to override that for the job-controller.
+#SUPPORTED_COMPUTE_BACKENDS = os.getenv('COMPUTE_BACKENDS',
+#                                       DEFAULT_COMPUTE_BACKEND).split(",")


Very good point for improvement 👍. The way this is set is a bit shady right now as one could pass it in REANA-Workflow-Controller with the process described in https://github.com/reanahub/reana-job-controller/pull/251/files#r416428666, but actually this is set at build time because that knowledge is used to selectively install the correct packages.

So when we build images for reana.cern.ch we use the Makefile like:

BUILD_ARGUMENTS="COMPUTE_BACKENDS=htcondorcern,slurmcern,kubernetes" BUILD_TYPE=release make build

What is happening behind the scenes is that we pass these build args to REANA-Job-Controller docker build, which results in the final image having this environment variable.

Side note, as it might be of interest here: Then we take this image and we name it reanahub/reana-job-controller-htcondorcern-slurmcern, not very pretty but this is what we have for now :) (see previous discussions here).

Thanks for the note Diego, it's very helpful! We'll be sure to follow that from now on. Perhaps we've been 'cheating' while building our images, we have simply been building them straight from the dockerfile within the RJC repo rather than through the reanahub/reana repo. I suppose this is a bad habit of ours.

Rename MAX_JOB_RESTARTS to MAX_NUM_RETRIES to make it consistent with the HTCondor CERN implementation.

To better organize job wrappers, files/job_wrapper has been moved to etc/htcondorvc3/job_wrapper.sh

…end.

Increase number of default retries.

Fix issue with krb5 dialog

Increase parrot timeout

htcondor\_submit.py no longer used

Remove old settings

Update default backend

…pying whole workflow dir

Change s3 repository to get parrot

khurtado and others added 30 commits February 19, 2019 14:54

Add htcondor python bindings

362d8ed

Update Dockerfile

5530ddb

Update Dockerfile

37f3cb9

Update Dockerfile

0385162

Test htcondor python bindings

7fc09fe

Add htcondor

50d4c8f

Set /tmp as the initialdir

9844e83

Initial changes for HTcondor job backend.

8e363be

Added htcondor submission for workflow steps. No monitoring of any file transfers is implemented yet.

Merge pull request #1 from CodyKank/master

94c61e9

Add Cody changes for condor submission

Merge branch 'master' of https://github.com/reanahub/reana-job-contro…

9243471

…ller Conflicts: reana_job_controller/rest.py Merged upstream changes. Commit condor changes. Updated to most recent rjc. Added delete job for condor.

Merge pull request #2 from CodyKank/master

031417c

Update RJC

Execute condor submit as a user.

af37025

Add first version of a singularity job wrapper.

21ba4c3

Job wrapper changes.

e1646e9

Job wrapper improvements.

889c952

Add limited support for transferring output files via condor_chirp.

66da263

Still a lot of room for improvement.

Add htcondor_job_manger.py

8238cab

Create first instance of HTCondor's job manager There are a few VC3 specifics within. Testing is still needed.

Adjust config and rest for htcondor

53d4af7

Added ability to call htcondor_job_manager over the already defined kubernetes_job_manager.

Merge pull request #3 from CodyKank/cody_dev

7019dbb

Cody dev

Some fixes for the condor job manager.

448b2a4

- Transfer input/output files via parrot

38a5b65

- Implement watch jobs method for condor.

First implementation to search for shifter

40bd56e

Changed how the htcondor job wrapper searches for a container. Instead of assuming singularity now searches through a list to be expanded later. Singularity is still hardcoded with arguments.

Add shifter to module search list in job_wrapper

e4991c7

Erroneously left out shifter in module search list, fixed incorrect usage of cntr_path in job_wrapper.

Remove bash associative arrays.

3951c04

To account for environments with bash versions > 4.0 associative arrays in job_wrapper for htcondor were removed and replaced with simple arrays. Adjusted call to modules within job_wrapper due to failure on Blue Waters.

Merge pull request #4 from CodyKank/job_manager

8ffa28c

First implementation to search for shifter

Updates on finding module utility

b6af97d

Merge branch 'master' of https://github.com/reanahub/reana-job-contro…

9bc053f

…ller into reanahub-master Conflicts: setup.py

Merge branch 'reanahub-master' into job_manager

70cd180

Fix schedd dependency.

d783329

khurtado and others added 6 commits April 6, 2020 18:20

Working around kubernetes version issues

da97df0

Should only apply to tag 0.6.0

Remove HTCONDOR address line from dockerfile

70bfbbc

Removed an older method for passing the htcondor schedd address to the container, instead of at build time we moved it long ago to within the yaml configuration file for reana-cluster

Change HTCondor version to 8.9.6

5afab13

Bump HTCondor installed through pip from 8.9.1 to 8.9.6

Bump htcondor in setup.py

0549154

Remove outdated files.

189814b

condor.py was used in the past before the creation of job_manager classes.

Correct function documentation from run_tests.sh

49cb367

tiborsimko reviewed Apr 28, 2020

View reviewed changes

diegodelemos reviewed Apr 28, 2020

View reviewed changes

CodyKank and others added 12 commits May 11, 2020 14:23

Add Kenyi and Cody to AUTHORS.rst.

0e76609

Clean up old variables no longer needed for VC3 implementation.

004d064

Rename MAX_JOB_RESTARTS to MAX_NUM_RETRIES to make it consistent with the HTCondor CERN implementation.

Move htcondorvc3's job_wrapper.

c5ef635

To better organize job wrappers, files/job_wrapper has been moved to etc/htcondorvc3/job_wrapper.sh

Remove extra htcondor installation in dockerfile.

48896bc

Remove comments from config.py. Reinstate k8s as default compute back…

6c41e90

…end.

Update htcondorvc3_job_manager.py

643d027

Increase number of default retries.

Update Dockerfile

1eed46c

Fix issue with krb5 dialog

Update job_wrapper.sh

c10886f

Increase parrot timeout

Fix typo in Dockerfile

a17a4b3

Update Dockerfile

3c23c31

htcondor\_submit.py no longer used

Update config.py

a651465

Remove old settings

Update config.py using vc3 backend as default

9f524b4

Update default backend

khurtado force-pushed the vc3v2p2 branch from 508e6b6 to 9f524b4 Compare May 14, 2021 20:49

khurtado and others added 4 commits February 8, 2022 02:41

Workaround to define input files on madminer workflow, rather than co…

b8684b5

…pying whole workflow dir

Fix bug while parsing command arguments

37b865b

Strip double quotes when parsing arguments

324425b

Update job_wrapper.sh

1010e51

Change s3 repository to get parrot

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

job_manager: Add htcondorVC3 job manager #251

job_manager: Add htcondorVC3 job manager #251

CodyKank commented Apr 27, 2020

tiborsimko Apr 28, 2020

CodyKank May 11, 2020

khurtado May 11, 2020 •

edited

Loading

tiborsimko Apr 28, 2020

CodyKank May 11, 2020

tiborsimko Apr 28, 2020

CodyKank May 11, 2020

tiborsimko Apr 28, 2020

diegodelemos Apr 28, 2020

CodyKank May 11, 2020

tiborsimko Apr 28, 2020

CodyKank May 11, 2020

diegodelemos Apr 28, 2020

CodyKank May 11, 2020

diegodelemos Apr 28, 2020

CodyKank May 26, 2020

	SHARED_VOLUME_PATH = os.getenv('SHARED_VOLUME_PATH', '/var/reana')
	SHARED_VOLUME_PATH_ROOT = os.getenv('SHARED_VOLUME_PATH_ROOT', SHARED_VOLUME_PATH)

		pip install --upgrade pip && \
		pip install htcondor==8.9.6 retrying

job_manager: Add htcondorVC3 job manager #251

Are you sure you want to change the base?

job_manager: Add htcondorVC3 job manager #251

Conversation

CodyKank commented Apr 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

khurtado May 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

khurtado May 11, 2020 •

edited

Loading