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.

Sign up for GitHub

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

Jump to bottom

htcondor: migrate to myschedd #260

Merged
merged 1 commit into from
Jul 9, 2020
Merged

htcondor: migrate to myschedd #260

merged 1 commit into from
Jul 9, 2020

Conversation

alintulu
Copy link
Member

@alintulu alintulu commented Jul 7, 2020 •
edited
Loading

Migrate from using cernbatchsubmit to myschedd for submitting job to CERN HTCondor.

In r-j-controller if you tried to import htcondor before having a valid kerberos ticket you got

ERROR: Unauthorized 401 - do you have authentication tokens?
Error "/usr/bin/myschedd.sh |": command terminated with exit code 256
Configuration Error Line 0 while reading config source /usr/bin/myschedd.sh |

Hence changed the order and added the import into the method, is this okey to do?

initialize_krb5_token(workflow_uuid=self.workflow_uuid)
globals()["htcondor"] = __import__("htcondor")

closes #256

tiborsimko
tiborsimko reviewed Jul 7, 2020
View reviewed changes
reana_job_controller/htcondorcern_job_manager.py Outdated
@@ -95,6 +94,8 @@ def execute(self):
"""Execute / submit a job with HTCondor."""
os.chdir(self.workflow_workspace)
initialize_krb5_token(workflow_uuid=self.workflow_uuid)
globals()["htcondor"] = __import__("htcondor")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Perhaps move the import magic to the constructor, if it works OK there, because most people would expect this to happen at the class constructor stage.

  • Please leave a comment why we do this, so that we recall for later. Something like "# We need to import htcondor package later during runtime after the Kerberos environment is fully initialised, otherwise..." (The behaviour of htcondor package is not exactly Python-idiomatic...)

@alintulu alintulu force-pushed the htcondor branch 2 times, most recently from 6c5940d to c37f778 Compare July 7, 2020 15:34
@alintulu alintulu marked this pull request as ready for review July 8, 2020 10:44
tiborsimko
tiborsimko reviewed Jul 9, 2020
View reviewed changes
Dockerfile Outdated
@@ -42,7 +42,9 @@ RUN if echo "$COMPUTE_BACKENDS" | grep -q "slurmcern"; then \
--no-install-recommends; \
fi

ADD etc/cernsubmit.yaml /etc/condor/
RUN mkdir -p /etc/myschedd
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please delete trailing whitespace after this line.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

tiborsimko
tiborsimko reviewed Jul 9, 2020
View reviewed changes
etc/myschedd.yaml Outdated
tzero:
standard:
- tbird02.cern.ch

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please delete final trailing blank line.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

tiborsimko
tiborsimko reviewed Jul 9, 2020
View reviewed changes
reana_job_controller/htcondorcern_job_manager.py
@@ -90,11 +89,15 @@ def __init__(
self.workflow = self._get_workflow()
self.unpacked_img = unpacked_img

# We need to import the htcondor package later during runtime after the Kerberos environment is fully initialised.
# Without a valid Kerberos ticket, importing will exit with "ERROR: Unauthorized 401 - do you have authentication tokens? Error "/usr/bin/myschedd.sh |"
initialize_krb5_token(workflow_uuid=self.workflow_uuid)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Can you please add yourself to the AUTHORS.rst file?

  • Can you add a note to the CHANGES.rst file about this change? For example: "Amends CERN HTCondor compute backend to use the new myschedd connection library"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@alintulu alintulu force-pushed the htcondor branch 2 times, most recently from d555017 to a177a9f Compare July 9, 2020 14:48
tiborsimko
tiborsimko approved these changes Jul 9, 2020
View reviewed changes
Copy link
Member

@tiborsimko tiborsimko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on HTC, working fine.

The certificate update may be better done by installing upstream CERN certificate package, instead of maintaining our own copy here, but let's make another issue about that.

@tiborsimko tiborsimko merged commit 83465dc into reanahub:master Jul 9, 2020
@tiborsimko tiborsimko mentioned this pull request Jul 9, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tiborsimko tiborsimko tiborsimko approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

htcondorcern: migrate to myschedd
2 participants
@alintulu @tiborsimko