-
Notifications
You must be signed in to change notification settings - Fork 146
Add CTest for XRootd Remote support #4199
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
Conversation
|
@eisenhauer I added tmp images to check that xrootd is installed correctly |
|
Cool. Lets see how this goes... |
ae5b4b1 to
bf213a0
Compare
|
@eisenhauer find the temp images with xrootd. |
bf213a0 to
44792b3
Compare
|
@vicentebolea Hey Vicente! So current issue is that the XRootD server really doesn't want to run as root. It can be started as root and you can use the -R option to tell it another user or UID to run as, but "-R root" or "-R 0" are rejected. Are there github action options that we might use to help get around this? I.E. adding a user to the docker that we only use to run the Xrootd server? |
eisenhauer
left a comment
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.
Digging into the XRootD code here, this error means that getpwnam("daemon") didn't return anything... If you pass a numeric value it passes that to getpwuid().
3d64d01 to
3d7cab9
Compare
5ce0a68 to
dde8830
Compare
|
@eisenhauer the tests finally passes |
|
Nice! |
dde8830 to
ae43432
Compare
|
@vicentebolea Time to revive this PR and prepare to merge. I tried (badly) to update the CI to reflect changes since it was first done, but it likely needs you attention. |
|
do you need to re-do the docker images? |
|
I'm actually not sure what you did before to get these to run. Temporary docker images? There were conflicts with master in some of the setup files that I tried to resolve, but perhaps incorrectly. |
|
So I guess the goal is this: The non-CI stuff in here mostly just enables testing of the XRootD plugin. We'd like to test on one or more platform in CI, so we'd need xrootd installed. Doesn't have to be everywhere, but one or two places would be good. |
55ccd71 to
735e964
Compare
144fc9c to
1c1f8a3
Compare
|
@eisenhauer the CI is ready, the 20.04 images now have xrootd. |
|
Minor tweak to fix server args. |
vicentebolea
left a comment
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.
Added a few comments, looks good otherwise. After addressing the comemnts that makes sense to you I can remove the 'tmp' and we can merge this.
|
@eisenhauer we are still having errors, let me know when the CI is green |
4d0450f to
438e4d3
Compare
|
@vicentebolea It looks like we're back to the user problem. In the manipulations, did we lose the bit that made this work? |
e0e68b2 to
813311d
Compare
|
@eisenhauer lets squash-merge to get rid of all this fixup commits |
08daa54 to
853d00e
Compare
|
@eisenhauer images are pushed (without the tmp) |
No description provided.