Revised changes for mode 3 cvmfsexec support #372
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
namrathaurs
force-pushed
the
branch_v3_11_revised
branch
from
04605e1
to
d21211c
Compare
namrathaurs
force-pushed
the
branch_v3_11_revised
branch
2 times, most recently
from
92f0336
to
c39a07e
Compare
namrathaurs
force-pushed
the
branch_v3_11_revised
branch
from
8bead3b
to
f60d23d
Compare
mambelli
requested changes
Apr 17, 2024
namrathaurs
force-pushed
the
branch_v3_11_revised
branch
from
f60d23d
to
fa5aa1e
Compare
- also updated syntax for the condition in the helper script
- complete refactoring of code based on improved logic and better flow handling in exception cases - changed cvmfs-related helper scripts to always be downloaded regardless of whether CVMFS will be mounted or not - added documentation for the helper methods used towards the goal of mounting CVMFS on demand - updated the unmount logic based on the refactored code
for more information, see https://pre-commit.ci
- minor changes (formatting, indentation, improved user facing messages etc.) - improvements to the existing unmounting process now that mode 3 will also be supported - code refactoring in cvmfs_setup script for better flow and readability - improvements in cvmfs_helper_funcs to allow for different behaviors as a result of using cvmfsexec modes
namrathaurs
force-pushed
the
branch_v3_11_revised
branch
from
6a97279
to
d50f10b
Compare
- fixed a runtime error when concatenating a set with only one element - added a hyperlink reference to the existing section that outlines the variables related to the on-demand CVMFS provisioning feature - updated name of the variable (GLIDEIN_WORK_DIR) to specify its usage being internal to GWMS - added `export` command to explicitly export the value of PATH when a glidein is reinvoked. This was done to resolve the `pivot_root: command not found` error
namrathaurs
force-pushed
the
branch_v3_11_revised
branch
from
d50f10b
to
bd2fbdf
Compare
creation/lib/cgWParamDict.py
Outdated
| @@ -219,15 +221,26 @@ def populate(self, other=None): | |||
| self.dicts["params"].add("GLIDEIN_Factory_Collector", str(factory_monitoring_collector)) | |||
| populate_gridmap(self.conf, self.dicts["gridmap"]) | |||
|
|
|||
| # the following list will be a megalist containing all the scripts; used for duplication check logic subsequently | |||
| all_scripts = list() | |||
There was a problem hiding this comment.
I suggest using all_scripts = [] for style consistency.
| @@ -397,7 +408,7 @@ def populate(self, other=None): | |||
| # TODO: This check could be done in the XML, checking if the entries are consistent in the current version | |||
| # fetch the on-demand cvmfs provisioning feature setting | |||
| # if on-demand CVMFS not used at the global level; ignore and continue | |||
| ondemand_cvmfs = self.dicts["attrs"].get("GLIDEIN_USE_CVMFSEXEC", 0) | |||
| ondemand_cvmfs = self.dicts["attrs"].get("GLIDEIN_USE_CVMFS", 0) | |||
There was a problem hiding this comment.
Should we also check for GLIDEIN_USE_CVMFSEXEC for backward compatibility?
There was a problem hiding this comment.
The configuration knob GLIDEIN_USE_CVMFSEXEC will be changed to GLIDEIN_USE_CVMFS, but I do not know for sure if we need to ensure backward compatibility in this case, if this change will eventually be propagated to production.
@mambelli would like to know your thoughts on this!
| @@ -716,7 +733,7 @@ def populate(self, entry, schedd, main_dicts): | |||
| # TODO: This check could be done in the XML, checking if the entries are consistent in the current version | |||
| # fetch the on-demand cvmfs provisioning feature setting | |||
| # if on-demand CVMFS not used by entry, ignore and continue | |||
| ondemand_cvmfs = self.dicts["attrs"].get("GLIDEIN_USE_CVMFSEXEC", 0) | |||
| ondemand_cvmfs = self.dicts["attrs"].get("GLIDEIN_USE_CVMFS", 0) | |||
There was a problem hiding this comment.
Should we also check for GLIDEIN_USE_CVMFSEXEC for backward compatibility?
Comment on lines
33
to
+34
| GWMS_STARTUP_SCRIPT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/$(basename "${BASH_SOURCE[0]}")" | ||
| export GWMS_STARTUP_SCRIPT=${GWMS_STARTUP_SCRIPT} # for glidein reinvocation |
There was a problem hiding this comment.
Couldn't we declare and export GWMS_STARTUP_SCRIPT in one line?
There was a problem hiding this comment.
That is a good suggestion. The export statement was something I added as part of my changes and it did not occur to me that declaring and exporting could be done in one line.
BrunoCoimbra
previously approved these changes
Aug 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR brings in changes corresponding to the comments received on PR #245. Since there was a significant number of comments on that PR, it would have been difficult to retain the comments while making the necessary changes. So the suggestion received was to create a new PR that contains the suggested changes (in accordance with the comments received) while the comments are retained for looking back into during the process of editing the files.
NOTE: This PR supersedes #245.
Sub-tasks that close this PR: