-
Notifications
You must be signed in to change notification settings - Fork 33
Add documentation for new Spoolman Rock-on #554
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
|
@MikeMcPherson Just had a quick look at this locally, and as per our GitHub Action report re:
It looks to be failing to build re: Do you fancy having a go at these? Don't worry about our linkcheck fail here as we are always having links fail - unless they are within your additions of course. |
|
@MikeMcPherson Incidentally my apologies again for getting ahead of these instructions when looking at the associated Rock-on. |
| The Spoolman docker app does not run as root, so you must change the | ||
| ownership of the share so that the user is the same as the user you | ||
| created when installing Rockstor (the "admin" user) and the group is | ||
| "users". Give group full permissions, as well. |
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.
It might be worthwhile to mention that the admin:users should have the corresponding UID:GID of 1000:1000 in case that mapping changes in the future. What do you think @phillxnet?
| The Spoolman docker app does not run as root, so you must change the | |
| ownership of the share so that the user is the same as the user you | |
| created when installing Rockstor (the "admin" user) and the group is | |
| "users". Give group full permissions, as well. | |
| The Spoolman docker app does not run as `root`, so you must change the ownership of the share to have the User ID (UID) as well as the group ID (GID) of `1000`. | |
| Typically those values are assigned to the the user and group that were created when installing Rockstor (the `admin` user and its associated `users` group). | |
| Give the group full permissions, as well. |
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 like your wording better.
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.
@Hooverdan96 re:
What do you think @phillxnet?
Agreed. The admin user is likely to be 1000, but not guaranteed; plus explicit is best and it is the UID that is canonical for docker, not the user. We also have a partial unknown: group 1000 is from CentOS et-al (users), I don't think our "Built on openSUSE" base even have one out of the box (users group is 100 here). It may be that the user is all that matters here. Otherwise folks have to create a place-holder group of 1000 as we can only assign named groups to Shares. But if all works OK with Share owner as user 1000 only, we are good and can also use the -1 uid Rock-on element.
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.
[EDIT] just check on a buildbot worker we use to prove stuff and it has the first use as buildbot, akin to a normal install where the first created non-system user is uid 1000:
id
uid=1000(buildbot) gid=100(users) groups=100(users)
So in that case the Web-UI admin user would then be 'other' than 1000, in this case I see:
id
uid=1026(radmin) gid=100(users) groups=100(users)
But a regular install on our new openSUSE base results in the following for the Web-UI setup created user:
id
uid=1000(radmin) gid=100(users) groups=100(users)
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.
We can drop the group permissions instruction; it was really "belt and suspenders". I just tested and it works fine leaving GID and group permissions at the default.
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'm leaning back to my original wording for the share owner. It seems to me that the one thing the admin does control is the username they picked when they install Rockstor. What do you think?
No worries! |
Hmmm... Missed those. I'll take a look. I did check to make sure the link errors aren't in my document. |
|
I have to go to my class, so further progress will happen later. |
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.
as mentioned in the rst below this screenshot won't be necessary anymore, since you adapted the Rockon definition to not ask for a timezone anymore, but automatically will use the system timezone.
|
I just saw, we were all commenting/suggesting at the same time. Hope, I did not overstep with my additional comments above. @phillxnet, see whether you agree with my interpretation of the logical line breaks vs. character count driven. |
|
@Hooverdan96 When you mentioned it I looked up to found the issue we have outstanding on that for some time. We should get to that sooner rather than later really. I didn't realise we wern't up-front about that yet. Just needs a little note in the docs really; along with the associated link to explain it more in the technical document context. |
On the last error during the build, I believe you need to insert your document as a link to this section: rockstor-doc/interface/overview.rst Lines 236 to 250 in 62aa475
|
…ce to unnecessary environment variable Co-authored-by: Hooverdan96 <[email protected]>
…eaks. Co-authored-by: Hooverdan96 <[email protected]>
…eaks. Co-authored-by: Hooverdan96 <[email protected]>
…eaks. Co-authored-by: Hooverdan96 <[email protected]>
…erline error Co-authored-by: Hooverdan96 <[email protected]>
…erline error Co-authored-by: Hooverdan96 <[email protected]>
…erline error Co-authored-by: Hooverdan96 <[email protected]>
…erline error Co-authored-by: Hooverdan96 <[email protected]>
I've added Spoolman to overview.rst, but I'm still failing the build_docs test. I'm afraid I don't know where to go with this next. I don't have much Sphinx experience. I think the linkcheck error is coming from somewhere other than Spoolman. I could use some advice, please. Thanks. Mike |
|
@MikeMcPherson Nearly there. Re:
I think I see what's happened here. The remaining error is the outstanding toctree (Table Of Contents tree) one.
The addition you made to overview.rst file was to the section under the title It can also often help to look at past Closed (and merged) Pull Requests here on GitHub doing the same thing. In this case adding a Rock-on write-up. The last example was the following:
Take a look at that past PR, and the section within it highlighted by @Hooverdan96 in his review here, and you should see what has happened here. For the write-ups, which are in their own files, we create a toc tree via the following Sphinx facility: Which, once compiled by Sphinx, procudes the list you see here: Where-as the more basic link facility is what we use for the list you have inadvertently added you overview.rst changes to: The fix is to undo your Hope that helps. Incidentally our following doc section: https://rockstor.com/docs/contribute/contribute_documentation.html#building-html-files-with-sphinx has some pointers on how to test locally. But pushing here works too. |
I think it's fixed. I'm sorry, it's the beginning of the school year and I'm burning the candle at both ends and in the middle. |
phillxnet
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.
@MikeMcPherson Thanks for persevearing with this PR. I know it's been a lot of back-and-forth with some hickups and cross-talk. But this is all good in the end and we, @Hooverdan96 mainly, has has now improved our contirbutor docs section to assist with some of the elements that were not up-front initially.
As we have an outstanding Rock-on pull-request that depends on this doc entry, I'm moving to getting this merged and published as-is. There are some outstanding anomalies re users group requirement etc but it is often easier to correct/improve something that already exists; so lets publish this and follow-up with corrections improvements there-after. There is also some indication that the upstream docker author is also making changes. So we may have some related changes to the Rock-ons and docs in the near future as a result of those changes also.
@Hooverdan96 I know there are uncommitted changes of your's still in play here, and I'm in agreement with them. However we have also had some moving goal-posts on this one so I'm keen to enable @MikeMcPherson to proceed with adding a link to this doc section so the pending Rock-on can get published.
I also think that many of the pics could be more aggressively cropped. But again this is not a show stopper and we can re-address as resources (human time mainly) allows.
|
Sounds good. When things calm down here I'll revisit the documentation in light of the new guidance. Thanks for all your help! |
|
PRODUCTION published |
Fixes #553
Add documentation for new Spoolman Rock-on
Checklist