service resource: attempt a SysV fallback if SystemD unit file is not found #2473
Conversation
This modifies the `enabled?` check to fallback to `sysv_service` in the event that a `.service` file cannot be found. For example: On Debian 8.7 the stock apache2 package does not deploy a `.service` file but deploys a SysV style service. This causes `systemctl is-enabled` to fail when the service is in fact enabled. Signed-off-by: Jerry Aldrich <jerryaldrichiii@gmail.com>
adamleff
changed the title
There was a problem hiding this comment.
Functionally looks good, @jerryaldrichiii. I think we can cleanup the test helper before merging though.
test/helper.rb
Outdated
| @@ -195,6 +195,11 @@ def md.directory? | |||
| mock.mock_command('', '', '', 0) | |||
| } | |||
|
|
|||
| cmd_stderr = lambda { |x| | |||
| stderr = ::File.read(::File.join(scriptpath, '/unit/mock/cmd/'+x)) | |||
There was a problem hiding this comment.
We don't need to do string concatenation or interpolation when we're already in a File.join call. It's also customary to remove the leading and trailing slashes and let File.join do its thing:
stderr = ::File.read(::File.join(scriptpath, 'unit/mock/cmd', x))I also question whether we really need another helper here, especially when this is also doing an exit 1 like cmd_exit_1 is already doing. Should we just enhance cmd_exit_1 to accept an optional path to stderr output?
There was a problem hiding this comment.
I agree with all those points! Update incoming.
Thanks for your always stellar reviews!
Signed-off-by: Jerry Aldrich <jerryaldrichiii@gmail.com>
test/helper.rb
Outdated
| stderr = ::File.read(::File.join(scriptpath, '/unit/mock/cmd/'+x)) | ||
| mock.mock_command('', '', stderr, 1) | ||
| cmd_exit_1 = lambda { |x = nil| | ||
| stderr = ::File.read(::File.join(scriptpath, 'unit/mock/cmd', x)) if x |
There was a problem hiding this comment.
You might think I'm nitpicking here, but I think we can eliminate the conditional in the following line and just let stderr always be the right thing (rather than mock_command having another in-line conditional):
stderr = x.nil? ? '' : ::File.read(::File.join(scriptpath, 'unit/mock/cmd', x))
mock.mock_command('', '', stderr, 1)That's a bit more direct and clear, would you agree?
There was a problem hiding this comment.
Yup, I agree, elegant use of the ternary. Not nitpicky at all! 😄
Signed-off-by: Jerry Aldrich <jerryaldrichiii@gmail.com>
There was a problem hiding this comment.
Great handling of a corner-case, love the fallback @jerryaldrichiii thank you!
|
@jerryaldrichiii You've solved my #1 inspec bug. Thanks |
This modifies the
enabled?check to fallback tosysv_servicein the event that a.servicefile cannot be found.For example: On Debian 8.7 the stock apache2 package does not deploy a
.servicefile but deploys a SysV style service. This causessystemctl is-enabledto fail when the service is in fact enabled.This closes #1708