feat(slurm_ops): set correct permissions on files owned by Slurm #40
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
Adds logic to create slurmrestd user at install time. slurmrestd cannot be the SlurmUser or SlurmdUser, so we must create a dedicated user similar to how the slurmrestd charm currently creates the user + group. Signed-off-by: Jason C. Nucciarone <[email protected]>
pyfakefs seemingly doesn't work for JWTKeyManager, so we need to manually mock the user and group for `shutil`. Just sets the user and group to the current user and group running the unit tests. Signed-off-by: Jason C. Nucciarone <[email protected]>
Apply formatting to the TODO messages so that they render nicely within tool windows such as PyCharm's TODO widget. Easy to quickly browse through outstanding items we want to tackle. Signed-off-by: Jason C. Nucciarone <[email protected]>
NucciTheBoss
force-pushed
the
feat-slurm-users
branch
from
e7273ad to
ae0ee9e
Compare
|
Unit tests are expected to fail because setting config file permissions with |
Signed-off-by: Jason C. Nucciarone <[email protected]>
…g files Changes: - Adds onto `test_config` tests to check the file mode, user, and group of file. - Fake the user and group by reassigning the `_user` and `_group` to the uid and gid of the user running the tests. - Redefines constants at the top of the test module so they can be easily found. Signed-off-by: Jason C. Nucciarone <[email protected]>
|
I know the problem here. Slurm snap doesn't create a |
The Slurm snap currently does not create a Slurm system user, so the integration tests needs an additional step to add both the slurm group and user. This is something that should be patched in the Slurm snap when the team revisits the snap package. Signed-off-by: Jason C. Nucciarone <[email protected]>
|
@jedel1043 R4R 🔍 |
jedel1043
reviewed
Oct 8, 2024
|
@jedel1043 please sir, might I have another review 🥺👉👈 |
jedel1043
approved these changes
Oct 8, 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 enables the
slurm_opslib to correctly set the user, group, and mode on files owned by Slurm. Originally, we had everything owned byrootwhich isn't bad, but it's encourage that files like slurm.conf and jwt_hs256.key are owned by the dedicated system userslurminstead.The PR adds public
userandgroupproperties that (a) be used to retrieve theuserandgroupfor configuration options such asSlurmUserand (b) pushes down to the configuration managers so thatslurmutilscan assign the proper access permissions to the configuration files.Currently a draft because this PR needs PR charmed-hpc/slurmutils#24 to land since it introduces the mode, user, and group setting features to slurmutils.