-
Notifications
You must be signed in to change notification settings - Fork 107
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
Discover the singularity container OS at runtime. #11896
Discover the singularity container OS at runtime. #11896
Conversation
Jenkins results:
|
299dc93
to
8337c22
Compare
Jenkins results:
|
8337c22
to
2d700fe
Compare
Jenkins results:
|
Jenkins results:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@todor-ivanov please find a couple of comments inline.
In addition, can you please amend the commit message and fix that typo? Thanks
2d700fe
to
cf2a22e
Compare
Jenkins results:
|
Add a check for the value obtained for the WMA_CURRENT_OS Fix regular expression Fix regular expression Add support for rhel6
cf2a22e
to
f7dda5a
Compare
Jenkins results:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes are looking good to me. Thanks for updating the commit message as well, Todor.
Can you please take care of the backporting and patching as well? We need to backport this PR to the branch As previously discussed, we also need to patch all the agents connected to the production system. Note that this only affects the job wrapper, so there is no need to restart any service/components in the agent. @germanfgv @LinaresToine could you please take care of the T0 agents? If you need any guidance, please let us know. |
@amaltaro All has been backported and new release created: https://github.com/dmwm/WMCore/commits/2.3.0_wmagent/ |
Thank you, Todor. We have usually made the backport through another PR targetting the specific branch, instead of pushing the commit directly upstream. I don't think there is any strong reason for doing it that way or this way, but if you feel like adding a short section with a procedure for backporting hot-fixes, I would suggest this wiki: https://github.com/dmwm/WMCore/wiki/TaggingAndReleasing#new-tagging-convention (well, the relevant in gitlab). |
Hi @amaltaro I know the general procedure we usually follow. But this time, I decided to directly cherry-pick just the commit from the master branch to the one to be back ported. I think this keeps the repository history more clean. And avoids any eventual differences between the master branch and the branch to which the commit is backported. Also in the case of eventual conflicts (which may appear if in the meantime some code changes have accumulated on the same source files as the current patch), they will be resolved in the process of cherry-picking, which makes it local to the working tree of the person doing the operation, instead of the upstream repository during the merge of the additional PR. This again - helps keeping the history clean. Of course, we lack the additional PR, but that's fine with me - we can create it if we want of course. I have no strong opinion here either. |
I like having the reference to the PR and the details that come with it, I guess that's why I always went with an extra PR against the particular branch. In addition, note that we can also cherry-pick - actually, this is the recommended way to keep meta-data intact - the master commit and provide it in a development branch against the particular wmagent/cmsweb branch. So, conflict resolution is the same between pushing upstream or pushing through a development branch. |
This is the patch being applied: dmwm/WMCore#11896
Fixes #11893
Status
ready
Description
With the current PR, we try to estimate the OS on the singularity container, where the job has landed at runtime.
Is it backward compatible (if not, which system it affects?)
YES
Related PRs
None
External dependencies / deployment changes
None