-
Notifications
You must be signed in to change notification settings - Fork 108
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
Update deployment script to accept python3 WMAgent service #10302
Conversation
Jenkins results:
|
c09de30
to
18edc37
Compare
Jenkins results:
|
Jenkins results:
|
With the proposed changes, WMAgent and all its dependencies can be installed. Unfortunately things fail to get initialized and the final agent deployment setup cannot be completed. This is the error message from the logs:
and we are ending up with a mix of python2 and python3 libraries (we haven't yet migrated away of some python2-only libraries), examples are:
I will make some new issues soon. |
In terms of deployment changes, I think this covers what is needed. The whole wmagentpy3 deployment is not fully working yet because there are many other issues with RPMs and their dependencies. |
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.
@amaltaro I just made two comments inline. I hope you have already tested the current deployment script with the python3 version of the rpm package and it is working, but those lines that I am pointing to seem confusing. Could you please take a look.
deploy/deploy-wmagent.sh
Outdated
@@ -285,12 +293,15 @@ echo -e "\n*** Applying agent patches ***" | |||
if [ "x$PATCHES" != "x" ]; then | |||
cd $CURRENT_DIR | |||
for pr in $PATCHES; do | |||
wget -nv https://github.com/dmwm/WMCore/pull/$pr.patch -O - | patch -d apps/wmagent/lib/python2*/site-packages/ -p 3 | |||
wget -nv https://github.com/dmwm/WMCore/pull/$pr.patch -O - | patch -d apps/${RPM_NAME}/lib/python2*/site-packages/ -p 3 |
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.
I am pretty sure you have already tested it, but just my curiosity: Why hard coded path leading to python2
libraries even though here we provide the option to deploy a python3 version of the rpm package
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.
Good catch! This needs to be fixed.
done | ||
set +e | ||
|
||
echo -e "\n*** Creating wmagent symlink ***" | ||
cd $CURRENT_DIR | ||
ln -s ../sw/${WMA_ARCH}/cms/${RPM_NAME}/${WMA_TAG} apps/wmagent |
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.
Here you seem to create the link name always as apps/wmagent
, which to me is logical, but down bellow you refer to it as apps/${RPM_NAME}
(line 296 in the current commit)
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.
I had to create this symlink because some of those bin/* scripts look for files under apps/wmagent
, which does not exist when we deploy wmagentpy3
.
The Deploy script automatically creates a symlink according to the package name. So, for python2, it won't make any difference since the symlink will be already there. For python3, we will have both symlinks pointing to the same directory structure.
In short, I could simply use apps/wmagent
below as well, but I tried to keep it as consistent as possible.
update MANAGE_DIR create wmagent symlink fix reference to python directory
Thanks for the review, Todor. I fixed one issue that you spotted. |
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.
looks good to me
Thanks @amaltaro. the temporary name sounds logical to me too. |
Thanks Todor. |
Fixes #10230
Status
tested
Description
Add a new argument to the deploy-wmagent script such that we can specify which RPM we would like to deploy:
NOTE: when deploying a python3 agent, we need to update the
env.sh
script to point to:instead of:
Is it backward compatible (if not, which system it affects?)
yes
Related PRs
none
External dependencies / deployment changes
Deployment changes: dmwm/deployment#1034