Slackware service state initial support by piterpunk · Pull Request #58207 · saltstack/salt

piterpunk · 2020-08-14T22:50:10Z

What does this PR do?

Adds support to manage services in Slackware Linux.

Without this module even a simple operation as service.start doesn't works, that makes impossible to use any service state.

Now, with this module, all service module operations works fine (provided the managed rc-scripts provides the required operation) and the service states are available.

What issues does this PR fix or reference?

Fixes: #58206

Previous Behavior

Calling a service.start:

# salt-call service.start ntp
Error running 'service.start': Unable to run command '['/etc/init.d/ntp', 'start']' with the context '{'cwd': '/root', 'shell': False, 'env': {'SHELL': '/bin/bash', 'LESS': '-M', 'PKG_CONFIG_PATH': '/usr/local/lib/pkgconfig:/usr/local/share/pkgconfig:/usr/lib/pkgconfig:/usr/share/pkgconfig', 'GNOME_KEYRING_CONTROL': '/root/.cache/keyring-WUK8O0', 'G_BROKEN_FILENAMES': '1', 'HOSTNAME': 'marvin.mylab', 'MINICOM': '-c on', 'QT4DIR': '/usr/lib/qt', 'PWD': '/srv/salt/ntp', 'LOGNAME': 'root', 'LS_OPTIONS': '-F -b -T 0 --color=auto', 'HOME': '/root', 'LANG': 'en_US.UTF-8', 'LS_COLORS': 'no=00:fi=00:di=01;34:ln=01;36:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.bat=01;32:*.btm=01;32:*.cmd=01;32:*.com=01;32:*.dll=01;32:*.exe=01;32:*.7z=01;31:*.ace=01;31:*.arj=01;31:*.bz2=01;31:*.cpio=01;31:*.deb=01;31:*.dz=01;31:*.gz=01;31:*.jar=01;31:*.lha=01;31:*.lz=01;31:*.lzh=01;31:*.lzma=01;31:*.rar=01;31:*.rpm=01;31:*.rz=01;31:*.tar=01;31:*.taz=01;31:*.tb2=01;31:*.tbz2=01;31:*.tbz=01;31:*.tgz=01;31:*.tlz=01;31:*.trz=01;31:*.txz=01;31:*.tz=01;31:*.tz2=01;31:*.xz=01;31:*.z=01;31:*.zip=01;31:*.zoo=01;31:*.aac=01;35:*.anx=01;35:*.asf=01;35:*.au=01;35:*.axa=01;35:*.axv=01;35:*.avi=01;35:*.bmp=01;35:*.divx=01;35:*.flac=01;35:*.gif=01;35:*.ico=01;35:*.jpg=01;35:*.jpeg=01;35:*.m2a=01;35:*.m2v=01;35:*.m4a=01;35:*.m4p=01;35:*.m4v=01;35:*.mid=01;35:*.midi=01;35:*.mka=01;35:*.mkv=01;35:*.mov=01;35:*.mp3=01;35:*.mp4=01;35:*.mp4v=01;35:*.mpc=01;35:*.mpeg=01;35:*.mpg=01;35:*.nuv=01;35:*.oga=01;35:*.ogv=01;35:*.ogx=01;35:*.ogg=01;35:*.opus=01;35:*.pbm=01;35:*.pgm=01;35:*.png=01;35:*.ppm=01;35:*.qt=01;35:*.ra=01;35:*.ram=01;35:*.rm=01;35:*.spx=01;35:*.svg=01;35:*.svgz=01;35:*.tga=01;35:*.tif=01;35:*.tiff=01;35:*.vob=01;35:*.wav=01;35:*.wma=01;35:*.wmv=01;35:*.xbm=01;35:*.xcf=01;35:*.xpm=01;35:*.xspf=01;35:*.xwd=01;35:*.xvid=01;35:', 'SSH_CONNECTION': '192.168.0.19 45190 192.168.0.42 22', 'TERM': 'xterm-256color', 'G_FILENAME_ENCODING': '@locale', 'CPLUS_INCLUDE_PATH': '/usr/lib/qt/include', 'LESSOPEN': '|lesspipe.sh %s', 'USER': 'root', 'T1LIB_CONFIG': '/usr/share/t1lib/t1lib.config', 'GDK_USE_XFT': '1', 'SHLVL': '1', 'INPUTRC': '/etc/inputrc', 'PS2': '> ', 'PS1': '\\u@\\h:\\w\\$ ', 'SSH_CLIENT': '192.168.0.19 45190 22', 'QT5DIR': '/usr/lib/qt5', 'LC_COLLATE': 'C', 'VDPAU_LOG': '0', 'PATH': '/usr/local/sbin:/usr/sbin:/sbin:/usr/local/bin:/usr/bin:/bin:/usr/games:/usr/lib/qt/bin:/usr/lib/qt5/bin', 'SSH_TTY': '/dev/pts/1', '_': '/usr/bin/salt-call', 'OLDPWD': '/srv/salt', 'LC_CTYPE': 'C', 'LC_NUMERIC': 'C', 'LC_TIME': 'C', 'LC_MONETARY': 'C', 'LC_MESSAGES': 'C', 'LC_PAPER': 'C', 'LC_NAME': 'C', 'LC_ADDRESS': 'C', 'LC_TELEPHONE': 'C', 'LC_MEASUREMENT': 'C', 'LC_IDENTIFICATION': 'C', 'LANGUAGE': 'C'}, 'stdin': None, 'stdout': -1, 'stderr': -2, 'with_communicate': True, 'timeout': None, 'bg': False, 'close_fds': True}', reason: [Errno 2] No such file or directory: '/etc/init.d/ntp'

Trying to apply service.running state:

          ID: ntp_service-running-ntpd
    Function: service.running
        Name: ntpd
      Result: False
     Comment: Unable to run command '['/etc/init.d/ntpd', 'restart']' with the context '{'cwd': '/root', 'shell': False, 'env': {'SHELL': '/bin/bash', 'LESS': '-M', 'PKG_CONFIG_PATH': '/usr/local/lib/pkgconfig:/usr/local/share/pkgconfig:/usr/lib/pkgconfig:/usr/share/pkgconfig', 'GNOME_KEYRING_CONTROL': '/root/.cache/keyring-WUK8O0', 'G_BROKEN_FILENAMES': '1', 'HOSTNAME': 'marvin.mylab', 'MINICOM': '-c on', 'QT4DIR': '/usr/lib/qt', 'PWD': '/srv/salt/ntp', 'LOGNAME': 'root', 'LS_OPTIONS': '-F -b -T 0 --color=auto', 'HOME': '/root', 'LANG': 'en_US.UTF-8', 'LS_COLORS': 'no=00:fi=00:di=01;34:ln=01;36:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.bat=01;32:*.btm=01;32:*.cmd=01;32:*.com=01;32:*.dll=01;32:*.exe=01;32:*.7z=01;31:*.ace=01;31:*.arj=01;31:*.bz2=01;31:*.cpio=01;31:*.deb=01;31:*.dz=01;31:*.gz=01;31:*.jar=01;31:*.lha=01;31:*.lz=01;31:*.lzh=01;31:*.lzma=01;31:*.rar=01;31:*.rpm=01;31:*.rz=01;31:*.tar=01;31:*.taz=01;31:*.tb2=01;31:*.tbz2=01;31:*.tbz=01;31:*.tgz=01;31:*.tlz=01;31:*.trz=01;31:*.txz=01;31:*.tz=01;31:*.tz2=01;31:*.xz=01;31:*.z=01;31:*.zip=01;31:*.zoo=01;31:*.aac=01;35:*.anx=01;35:*.asf=01;35:*.au=01;35:*.axa=01;35:*.axv=01;35:*.avi=01;35:*.bmp=01;35:*.divx=01;35:*.flac=01;35:*.gif=01;35:*.ico=01;35:*.jpg=01;35:*.jpeg=01;35:*.m2a=01;35:*.m2v=01;35:*.m4a=01;35:*.m4p=01;35:*.m4v=01;35:*.mid=01;35:*.midi=01;35:*.mka=01;35:*.mkv=01;35:*.mov=01;35:*.mp3=01;35:*.mp4=01;35:*.mp4v=01;35:*.mpc=01;35:*.mpeg=01;35:*.mpg=01;35:*.nuv=01;35:*.oga=01;35:*.ogv=01;35:*.ogx=01;35:*.ogg=01;35:*.opus=01;35:*.pbm=01;35:*.pgm=01;35:*.png=01;35:*.ppm=01;35:*.qt=01;35:*.ra=01;35:*.ram=01;35:*.rm=01;35:*.spx=01;35:*.svg=01;35:*.svgz=01;35:*.tga=01;35:*.tif=01;35:*.tiff=01;35:*.vob=01;35:*.wav=01;35:*.wma=01;35:*.wmv=01;35:*.xbm=01;35:*.xcf=01;35:*.xpm=01;35:*.xspf=01;35:*.xwd=01;35:*.xvid=01;35:', 'SSH_CONNECTION': '192.168.0.19 45190 192.168.0.42 22', 'TERM': 'xterm-256color', 'G_FILENAME_ENCODING': '@locale', 'CPLUS_INCLUDE_PATH': '/usr/lib/qt/include', 'LESSOPEN': '|lesspipe.sh %s', 'USER': 'root', 'T1LIB_CONFIG': '/usr/share/t1lib/t1lib.config', 'GDK_USE_XFT': '1', 'SHLVL': '1', 'INPUTRC': '/etc/inputrc', 'PS2': '> ', 'PS1': '\\u@\\h:\\w\\$ ', 'SSH_CLIENT': '192.168.0.19 45190 22', 'QT5DIR': '/usr/lib/qt5', 'LC_COLLATE': 'C', 'VDPAU_LOG': '0', 'PATH': '/usr/local/sbin:/usr/sbin:/sbin:/usr/local/bin:/usr/bin:/bin:/usr/games:/usr/lib/qt/bin:/usr/lib/qt5/bin', 'SSH_TTY': '/dev/pts/1', '_': '/usr/bin/salt-call', 'OLDPWD': '/srv/salt', 'LC_CTYPE': 'C', 'LC_NUMERIC': 'C', 'LC_TIME': 'C', 'LC_MONETARY': 'C', 'LC_MESSAGES': 'C', 'LC_PAPER': 'C', 'LC_NAME': 'C', 'LC_ADDRESS': 'C', 'LC_TELEPHONE': 'C', 'LC_MEASUREMENT': 'C', 'LC_IDENTIFICATION': 'C', 'LANGUAGE': 'C'}, 'stdin': None, 'stdout': -1, 'stderr': -2, 'with_communicate': True, 'timeout': None, 'bg': False, 'close_fds': True}', reason: [Errno 2] No such file or directory: '/etc/init.d/ntpd'
     Started: 19:41:35.631266
    Duration: 21163.166 ms
     Changes:

New Behavior

Calling a service.start

# salt-call service.start ntpd
local:
    True

Applying a service.running state:

          ID: ntp_service-running-ntpd
    Function: service.running
        Name: ntpd
      Result: True
     Comment: The service ntpd is already running
     Started: 19:48:23.647516
    Duration: 21.559 ms
     Changes:

Merge requirements satisfied?

Docs
Changelog - https://docs.saltstack.com/en/master/topics/development/changelog.html
Tests written/updated

Commits signed with GPG?

No

piterpunk · 2020-08-18T13:27:47Z

@dwoz , can you help on this? All the builds failed without actually do any build:

09:53:01  Running on EC2 (saltstack-ci) - kitchen (i-075b7ee69471a539c) in /var/jenkins/workspace/pr-fedora32-py3/PR-58207
09:53:01  [Pipeline] {
09:53:01  [Pipeline] timeout
09:53:01  Timeout set to expire in 7 hr 0 min
09:53:01  [Pipeline] {
09:53:01  [Pipeline] withEnv
09:53:01  [Pipeline] {
09:53:01  [Pipeline] stage
09:53:01  [Pipeline] { (Clone)
09:53:01  [Pipeline] cleanWs
09:53:01  [Pipeline] }
09:53:02  [Pipeline] // stage
09:53:02  [Pipeline] }
09:53:02  [Pipeline] // withEnv
09:53:02  [Pipeline] cleanWs
09:53:02  [Pipeline] }
09:53:02  [Pipeline] // timeout
09:53:02  [Pipeline] }
09:53:02  [Pipeline] // node
09:53:02  [Pipeline] echo
09:53:02  Setting currentBuild.result to FAILURE

I don't know what happened :(

Ch3LL · 2020-08-27T10:30:18Z

Can you add .. versionadded:: Magnesium to all of these new methods.

OK, I will do that tonight

Ch3LL · 2020-08-27T10:34:53Z

Is this always the path? This would prevent a user from setting their own path where they would prefer to manage these files. Might be better to set this as a kwarg in the methods that use this path?

Yes, this is always the path for rc scripts in Slackware. It's like /etc/init.d on SysV init.

Ch3LL · 2020-08-27T10:37:59Z

This needs to be changed to Magnesium instead

Hahahaha... sorry, this what happens when copying and pasting stuff, I will fix that tonight.

Ch3LL · 2020-08-27T12:12:38Z

we are still in the process of migrating entirely to pytest, but since this is a new test file i would love to see it using pytest instead in tests/pytests/unit/modules so we dont have to migrate it later if you're willing to do so.

Oh, ok. I will change it, as it's a new feature it's better to born in the right way.

Ok, I guess it's done.

Ch3LL · 2020-08-31T21:29:45Z

thanks for all these updates, really appreciate them. I have just one more thing. Can we add these variables to a pytest fixture? This actually helps speed up things when pytest is collecting tests and prevents any issues we have ran into the past where someone edited a list/dict at the global module level that affects other tests.

It took a while, but following your advice and with the explanations of the @waynew from the Saltstack Test Clinic, I removed all the global variables and took the opportunity to make a big overhaul of test names, descriptions, comments and reorganized some them. I hope this time everything is fine :)

- Added slackware_service.py module to handle the Slackware rc-scripts - Added Slackware in the list to not use the linux_service.py

- Added tests/init/modules/test_slackware_service.py - Fixed the permission mask on slackware_service.py - Added slackware_service in index.rst

- Call to os.stat wasn't properly mocked and the tests fails on all non-Slackware platforms. - Fixed the affected tests and changed the test argument to a non-existent file, so the wrong mock can be detected in Slackware.

- Removed global variables and put inside a reusable fixture - Changed the test names to be more descriptive - Also added better explanations and comments to the mocked data and the tests themselves - Changed the enabled/disabled tests to be more useful

Ch3LL

thanks for all your work on this one, much appreciated

piterpunk · 2020-09-23T13:36:21Z

@dwoz why the "Needs Testcase" label?

piterpunk requested a review from a team as a code owner August 14, 2020 22:50

ghost requested review from dwoz and removed request for a team August 14, 2020 22:50

dwoz added the has-failing-test label Aug 15, 2020

Ch3LL suggested changes Aug 27, 2020

View reviewed changes

sagetherage added the Magnesium Mg release after Na prior to Al label Aug 27, 2020

piterpunk force-pushed the slackware_service_state_initial_support branch from 4c1ca6c to 2fd336a Compare August 31, 2020 03:14

piterpunk requested a review from Ch3LL August 31, 2020 04:44

Ch3LL suggested changes Aug 31, 2020

View reviewed changes

piterpunk added 7 commits September 21, 2020 21:04

Initial support to manage services in Slackware

325262f

- Added slackware_service.py module to handle the Slackware rc-scripts - Added Slackware in the list to not use the linux_service.py

Added unit tests to slackware_service.py module

aa878aa

- Added tests/init/modules/test_slackware_service.py - Fixed the permission mask on slackware_service.py - Added slackware_service in index.rst

Added Changelog entry for saltstack#58206

83f63ab

Fixed slackware_service's enable and disable tests

bd906a4

- Call to os.stat wasn't properly mocked and the tests fails on all non-Slackware platforms. - Fixed the affected tests and changed the test argument to a non-existent file, so the wrong mock can be detected in Slackware.

Fixed underlines in RST file

00e88b9

Migrated tests to PyTest and added release version

559c402

piterpunk force-pushed the slackware_service_state_initial_support branch from 2fd336a to ed2cb39 Compare September 22, 2020 00:05

piterpunk requested a review from Ch3LL September 22, 2020 12:15

dwoz added the needs-testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases label Sep 22, 2020

Ch3LL approved these changes Sep 23, 2020

View reviewed changes

dwoz merged commit e941aa8 into saltstack:master Sep 23, 2020

piterpunk deleted the slackware_service_state_initial_support branch September 29, 2020 21:15

Conversation

piterpunk commented Aug 14, 2020

What does this PR do?

What issues does this PR fix or reference?

Previous Behavior

New Behavior

Merge requirements satisfied?

Commits signed with GPG?

Uh oh!

piterpunk commented Aug 18, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ch3LL left a comment

Choose a reason for hiding this comment

Uh oh!

piterpunk commented Sep 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants