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

Fix draining of slots having a startd name #123

Merged
merged 1 commit into from
Jan 10, 2020

Conversation

giffels
Copy link
Member

@giffels giffels commented Jan 10, 2020

TARDIS supports running more than one Drone on the same host. To differentiate the drones, the tardis-uuid has been introduced and will be set as STARTD_NAME on the corresponding sites. The HTCondor Name in this case is

slot1@tardis-uuid@hostname

Due to backward compatibility for sites running only one Drone per host, TARDIS supports also drones without having a STARTD_NAME. Leading to

slot1@hostname

as HTCondor Name.

Currently, TARDIS drains drones by using just its hostname (HTCondor Machine)

condor_drain -graceful hostname

which does not work in case multiple drones running on the same host. This pull request fixes this bug by using the HTCondor Name instead.

condor_drain -graceful slot1@tardis-uuid@hostname

@giffels giffels added the bug Something isn't working label Jan 10, 2020
@giffels giffels requested a review from a team January 10, 2020 10:29
@codecov-io
Copy link

codecov-io commented Jan 10, 2020

Codecov Report

Merging #123 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #123      +/-   ##
=========================================
+ Coverage   97.68%   97.7%   +0.01%     
=========================================
  Files          39      39              
  Lines        1424    1435      +11     
=========================================
+ Hits         1391    1402      +11     
  Misses         33      33
Impacted Files Coverage Δ
tardis/adapters/batchsystems/htcondor.py 100% <100%> (ø) ⬆️
tardis/adapters/sites/openstack.py 100% <0%> (ø) ⬆️
tardis/adapters/sites/htcondor.py 100% <0%> (ø) ⬆️
tardis/adapters/sites/moab.py 100% <0%> (ø) ⬆️
tardis/utilities/pipeline.py 100% <0%> (ø) ⬆️
tardis/interfaces/siteadapter.py 100% <0%> (ø) ⬆️
tardis/utilities/asynccachemap.py 100% <0%> (ø) ⬆️
tardis/adapters/sites/cloudstack.py 100% <0%> (ø) ⬆️
tardis/adapters/sites/slurm.py 100% <0%> (ø) ⬆️
tardis/interfaces/executor.py 80% <0%> (+5%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 546a3b5...312bd61. Read the comment docs.

@ghost ghost requested review from eileen-kuehn and maxfischer2781 and removed request for a team January 10, 2020 10:38
Copy link
Member

@eileen-kuehn eileen-kuehn left a comment

Choose a reason for hiding this comment

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

I think it looks good. I added one comment as I don't know if both Machine and Name should be given.

@giffels giffels requested a review from eileen-kuehn January 10, 2020 11:46
Copy link
Member

@maxfischer2781 maxfischer2781 left a comment

Choose a reason for hiding this comment

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

No objections. Good to see Tardis supports this now!

Copy link
Member

@eileen-kuehn eileen-kuehn left a comment

Choose a reason for hiding this comment

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

Go for it 👍

@giffels giffels merged commit 0227a7c into MatterMiners:master Jan 10, 2020
@giffels giffels deleted the fix-draining-startd-name branch January 10, 2020 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants