OleHolmNielsen / Slurm_tools

My tools for the Slurm HPC workload manager
GNU General Public License v3.0
444 stars 96 forks source link

slurmusersetting: suppression of users with non-login shell #20

Closed tardigradus closed 2 years ago

tardigradus commented 2 years ago

Would it be possible to add a flag to allow the suppression of the output regarding users with a non-login shell?

Our use-case is that, as part of our user life-cycle management, we set the login shells of users who haven't logged on for a certain period or whom we have not been able to contact to /sbin/nologin. We would like to keep them in the SlurmDB but just prevent them from logging in. This potentially affects quite a large number of users, so it would be nice to be able to suppress the corresponding output.

WDYT?

OleHolmNielsen commented 2 years ago

Thanks for the suggestion!

In the script there is a check for non-login shells:

        if (SHELL == "/sbin/nologin" || SHELL == "/bin/false") {                # Skip non-login users
                userinformation[u] = "User " u " UNIX account has non-login shell " SHELL
                next
        }

I've tested this on a user and getting this output:

### Slurm account xxx error: User xxx UNIX account has non-login shell /sbin/nologin
/usr/bin/sacctmgr -i delete user xxx

It's only the second line that you would like to suppress?

Would it be useful to have a new option for slurmusersettings (say, -n) meaning "Ignore non-login users"? I could rather easily make this generate this output:

### IGNORE this username: xxx
tardigradus commented 2 years ago

At the moment I would quite like to have no output produced for non-login shells, so that I can concentrate on all the other output. How about a separate flag which toggles whether the comment lines are printed? Would that be useful?

OleHolmNielsen commented 2 years ago

But the non-login users would still be printed as:

/usr/bin/sacctmgr -i delete user xxx

Suppressing the ### comment lines I think is a separate request. That could be solved with a -c option somehow?

tardigradus commented 2 years ago

Yes, you're right - suppressing the comments would be a separate feature to suppressing the non-login users.

OleHolmNielsen commented 2 years ago

OK, so would you like to suggest both of these new options be added to slurmusersettings?

tardigradus commented 2 years ago

Yes, please!

OleHolmNielsen commented 2 years ago

I've now added 3 new flags, could you test them:

$ slurmusersettings -h
Usage: ./slurmusersettings [-u username | -d | -n | -c | -h ]
where:
    -u username: Select user <username> (Default is all users)
    -d Print debugging output
    -n Omit users with non-login shells (-d overrides this)
    -c Do not print explanatory comments (-d overrides this)
    -h Print help information
tardigradus commented 2 years ago

Thanks for making the changes. However, the -u options does not seem to work. The test for the existence of the UID is OK, but the variable $username isn't used after that.

Unfortunately I confused slurmusersettings with slurmaccount and assumed the former also just just prints stuff without changing anything :-/ So now I have some tidying up to do, before I can do any more testing. Maybe you could also add a dry run option for slurmusersettings?

OleHolmNielsen commented 2 years ago

Thanks for the testing. The slurmusersettings definitely doesn't change anything, it just prints text to stdout! It's a dry-run as designed, and you may choose to run the commands if you want.

I will check the -u username option.

tardigradus commented 2 years ago

Ah, sorry, I was doubly confused in mistaking the output with -i to mean that the commands were indeed carried out without confirmation. I was wondering why I couldn't find anything to tidy up :-)

OleHolmNielsen commented 2 years ago

Regarding the the -u username option, it seems to me that it should work correctly. In the awk script this code checks for username:

        # Process only the selected username:
        if (username != "") {
                if (u!= username) next
tardigradus commented 2 years ago

Maybe I am misunderstanding what -u should do. Isn't there more code after the code you mention which applies to all users? If I use the option, I get less output, but still more than I would expect:

$ slurmusersettings -c | wc -l
203
$ slurmusersettings -c -u loris | wc -l
81

In the latter case there are lines such as

/usr/bin/sacctmgr -i add user joedoe account=staff

Is this intended?

OleHolmNielsen commented 2 years ago

I have suppressed additional comment lines now. I've also moved the username check up earlier in the code. Could you please check it the new code looks OK in your environment? Thanks, Ole

tardigradus commented 2 years ago

Looks good! Thanks!

OleHolmNielsen commented 2 years ago

Great, thanks a lot for your input and suggestions! Best regards, Ole