Valid ensure file resource value.

[schkovich]

In case that supervisor::service::ensure is set to present and supervisor::service::enable is set to true, value of $config_ensure will be set to undef. Possible values of file resource ensure property are absent, present, file, directory, and link.

Due to passing undef instead of file when creating _"${supervisor::conf_dir}/${name}${supervisor::confext}" file resource creation of supervisor's configuration file will fail.

[andyshinn]

I am actually confused at the difference between supervisor::service::ensure and supervisor::service::enable. What would happen in the case on supervisor::service::ensure = 'present' and supervisor::service::enable = false? Isn't supervisor::service::enable redundant in this case?

It seems like the $ensure case should just have $config_ensure = 'file' on present and $config_ensure = 'absent' on absent?

[schkovich]

Well, that would be a conceptual problem. I was after a bug. :) However, I do agree with you @andyshinn: supervisor::service::enable is kind of redundant. I could not come with a meaningful and valid use case to support supervisor::service::enable property.

[andyshinn]

I am actually working on another branch that changes the meaning of supervisor::service::enable to disable the management of the service resource (so that supervisor::service can be used to just add the INI file only). So, I wonder if you can solve the bug by just removing the conditional altogether and setting supervisor::service::config_ensure to file, like:

case $ensure {
    absent: {
      $autostart = false
      $dir_ensure = 'absent'
      $dir_recurse = true
      $dir_force = true
      $service_ensure = 'stopped'
      $config_ensure = 'absent'
    }
    present: {
      $autostart = true
      $dir_ensure = 'directory'
      $dir_recurse = false
      $dir_force = false
      $service_ensure = 'running'
      $config_ensure = 'file'
    }
    default: {
      fail("ensure must be 'present' or 'absent', not ${ensure}")
    }
  }

My reasoning being that the file will need to be there if the service ensure is set to running. There wouldn't be a way to have the service set to running and the file absent... Thoughts?

[schkovich]

I agree. As said, supervisor::service::enable is adding complexity and giving (almost) nothing in return.

[skrings]

:+1:

[andyshinn]

:ship:it

[plathrop]

To give a little color behind the existence of both enable and ensure, this is to model the interface of puppet's native service type. You might want to have a service enabled at boot, but not have puppet manage whether it is running or not post-boot (perhaps you want your engineers to be able to start/stop the service as needed).

Anyway, that's the original design thinking.

[andyshinn]

@plathrop Thanks! I've got another PR coming (depending on the service provider library) to make enable act just like that (at the moment, it didn't actually control the service at all).