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

[8.0] Pilot stamps available in the pilot execution environment #6405

Merged
merged 14 commits into from
Feb 15, 2023

Conversation

atsareg
Copy link
Contributor

@atsareg atsareg commented Oct 11, 2022

This PR is for passing pilot stamps to the pilot execution environment. This should be done differently for different ComputingElements.

  • HTCondorCE
  • ARC/ARC6
  • AREX
  • SSH
  • Cloud

BEGINRELEASENOTES

*Resources
NEW: [ALL]ComputingElements - pass pilot stamp to the pilot environment

ENDRELEASENOTES

@atsareg atsareg requested a review from fstagni as a code owner October 11, 2022 11:35
@DIRACGridBot DIRACGridBot added the alsoTargeting:integration Cherry pick this PR to integration after merge label Oct 11, 2022
@atsareg atsareg marked this pull request as draft October 11, 2022 11:39
@sfayer
Copy link
Member

sfayer commented Oct 11, 2022

Hi,

I'm not sure the cloud changes will work quite correctly: The pilot/inst UUID is just a random value generated by the cloud provider used as the pilotReference, it doesn't connect back to the instRandom which is only currently included in the instance name. (The return statement from the end of submitPilot has also gone missing).

It is possible to get the name from inside the VM, but I think it's probably safer (more portable to different cloud providers) to substitute it directly into the metadata, something like this:
77ae2f0

Regards,
Simon

@atsareg
Copy link
Contributor Author

atsareg commented Oct 11, 2022

Thanks Simon, I have included your proposed changes

@martynia
Copy link
Contributor

Hi,
In the meantime I tested the stamps for HT Condor, and I can confirm that the pilot stamps are correctly passed to the pilot.

@@ -256,6 +256,7 @@ def _writeXRSL(self, executableFile, inputs=None, outputs=None, executables=None
(inputFiles=({executable} "{executableFile}") {xrslInputAdditions})
(stdout="{diracStamp}.out")
(stderr="{diracStamp}.err")
(environment="(DIRAC_PILOT_STAMP {diracStamp}"))
Copy link
Member

Choose a reason for hiding this comment

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

Janusz has been testing this and it seems this line is causing submission errors; should it be:
(environment=("DIRAC_PILOT_STAMP {diracStamp}")) (e.g. with the bracket outside of the string after the equals)?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, my previous suggestion isn't quite right... The key and value should also be quoted independently:
(environment=("DIRAC_PILOT_STAMP" "{diracStamp}"))

Copy link
Contributor

Choose a reason for hiding this comment

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

I can confirm that the submission to ARC CE works now when setting:
(environment=("DIRAC_PILOT_STAMP" "{diracStamp}"))

@atsareg
Copy link
Contributor Author

atsareg commented Feb 13, 2023

No extra intervention needed for AREXCE. Please, review

@atsareg atsareg marked this pull request as ready for review February 13, 2023 15:53
Copy link
Contributor

@fstagni fstagni left a comment

Choose a reason for hiding this comment

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

Review OK for me, but other opinions are more than welcome!

I will merge it before the Thursday hackathon so it can be tried out in DIRAC certification. Some specific test/checks to make?

@aldbr
Copy link
Contributor

aldbr commented Feb 14, 2023

I just reviewed the PR: it looks OK to me, I have nothing to report.

@fstagni fstagni merged commit a452b31 into DIRACGrid:rel-v8r0 Feb 15, 2023
@DIRACGridBot DIRACGridBot added the sweep:done All sweeping actions have been done for this PR label Feb 15, 2023
DIRACGridBot pushed a commit to DIRACGridBot/DIRAC that referenced this pull request Feb 15, 2023
@DIRACGridBot
Copy link

Sweep summary

Sweep ran in https://github.com/DIRACGrid/DIRAC/actions/runs/4185234036

Successful:

  • integration

simon-mazenoux pushed a commit to simon-mazenoux/DIRAC that referenced this pull request May 24, 2023
simon-mazenoux pushed a commit to simon-mazenoux/DIRAC that referenced this pull request May 24, 2023
fstagni pushed a commit to fstagni/DIRAC that referenced this pull request May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alsoTargeting:integration Cherry pick this PR to integration after merge sweep:done All sweeping actions have been done for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PilotLogger - How to identify pilots in an early submission state
7 participants