Skip to content

Add a service_provider fact #506

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 24, 2015
Merged

Add a service_provider fact #506

merged 1 commit into from
Sep 24, 2015

Conversation

binford2k
Copy link
Contributor

This returns the default provider Puppet will choose to manage services
on this system by instantiating a dummy service resource type and
returning the provider chosen.

The original suggestion came from @asasfu but I'm not sure how to credit him.

@nibalizer
Copy link
Contributor

I really want this feature.

I tried once: #473

This doesn't work on my Ubuntu 15.04 system, which on it's own isn't a big deal but I think that reveals the problem with this solution. The solution (inspecting puppet's defaults) is neat and I like it. But since it's asking Puppet core for the answer, it can't be updated and changed quickly or easily. A change informing Puppet core which systems have moved to systemd will need to be proposed, landed, and then deployed by the operator.

I hate it, but I think the best thing to do is just keep a big nested case statement based on osfamily, operatingsystem, lsbrelease, etc; inside stdlib.

@nibalizer
Copy link
Contributor

As for attributing another, you can put

Co-Authored-By: John Derp <derp@derpstack.org>

In the commit message

@binford2k
Copy link
Contributor Author

That would violate the principle of least surprise.

For example, you'd expect this to work, and it likely wouldn't because your case statement would come up with one provider and Puppet itself would come up with another.

$service_provider = { gnarly logic that returns expected provider }

case $service_provider {
    'systemd': {
        file { '/etc/systemd/system/mything.service':
            ensure => file,
            source => 'puppet:///modules/mything/mything.service',
        }
    'redhat': {
        file { '/etc/init.d/mything':
            ensure => file,
            source => 'puppet:///modules/mything/mything.init',
        }
    }
}
service { 'mything':
    enable => true,
    running => true,
}

Another edge case in which that would fail would be if the user has done something like update a CentOS 6 system to use systemd.

I suspect that what you want is a variable to override Puppet's default provider choice automatically and I don't think that's a good idea. The Ubuntu bit needs to be fixed in core Puppet, but a short term mitigation could be to publish a community module with a systemd provider for Ubuntu.

If @asasfu comments with his email or updates his github profile, I'll add that.

@asasfu
Copy link
Contributor

asasfu commented Aug 19, 2015

Thanks for working on this @binford2k . I've updated my profile to include my email and org.

This returns the default provider Puppet will choose to manage services
on this system by instantiating a dummy service resource type and
returning the provider chosen.

Co-Authored-By: Simon Fraser University <asa188@sfu.ca>
@nibalizer
Copy link
Contributor

stdlib can move (somewhat) faster than core, meaning it's a good place to move the decision about what is and is not systemd. We can put warnings inside the documentation that clarify that we do not use the same decision tree as Puppet core. We can even socialize the following snippet:

service { 'derp':
   provider => service_provider(),
}

@DavidS
Copy link
Contributor

DavidS commented Aug 20, 2015

This all looks like (workable, but) hacky workarounds of underlying issues with working with multiple init systems. Long-term I'd much rather have a solution that either derives info from intrinsic system information ("Debian 6 can only ever have sysvinit, Debian 7 can have either sysvinit or old systemd, Debian 8 can have both, but defaults to systemd") or applies the desired state to a system in a way that others can piggy back off ("Use systemd on this node").

I'm collecting stuff like this at https://tickets.puppetlabs.com/browse/FM-3350

@asasfu
Copy link
Contributor

asasfu commented Aug 20, 2015

I don't have access to view that ticket @DavidS . the point of this addition was to use the provider which puppet already decides based on confines. If puppet currently supports let's say Debian 7 or 8 and supports them whether they picked sysvinit or systemd as their provider via grub, as in puppet would still work on them either way when running service { 'apache': ensure => running }, then this fact would work exactly as we wanted it to. It would allow us the ability to actually know which provider the user picked and use that for deciding which codepath in our puppet module to take for things such as deploying a service file.
I do know that it doesn't allow for things like Service { provider => systemd } overrides, but there are ways to do that as well.
The benefit is that as long as puppet maintains supporting OSes, as well as their freedom of some to pick their init style, then this fact can provide some use to module developers in reducing their code requirement and constant maintenance with new releases of OS.
While I do like this fact, if we can't get closer to guaranteeing puppet maintaining provider confines for new OS's even in slightly older puppet releases such as puppet 3.x then we should consider supporting overrides for this as well such as I stated above with Service {}. Thoughts?

@nibalizer
Copy link
Contributor

I see it as a 'wheels within wheels' problem. Puppet core is the innermost wheel, and it rotates the slowest. Also telling a user to upgrade to puppet latest to fix a bug like this isn't a great solution.

https://github.com/puppetlabs/puppetlabs-postgresql/blob/master/manifests/params.pp#L152

Is a use case where this fact could be used that doesn't directly involve overriding provider.

@asasfu
Copy link
Contributor

asasfu commented Aug 20, 2015

@nibalizer but in that case aren't you putting an override into the module to support whether or not they're systemd or upstart or whatever... when in this case nothing else in their environment will work for that Debian 8 system unless it also had the same logic that this module does?
The point of this fact is to allow us to ask puppet what provider they would pick, and in the case where they support the OS it will work. Yes it's true, if they do a patch release to support an OS and the user does not want to upgrade to that patch release then technically the user should be doing something in site.pp to override the provider based on the logic that they think is relevant to decide if their Debian 8 system is running upstart or systemd...
If they did do the override in site.pp though we would need to use my extra function that goes along with this fact to say, if Service override is done, take the override, else, take service_provider fact.
TLDR:
If users don't want to patch their puppet to the patch release that supports their OS, then we should introduce a function which merges their override with service_provider fact and accepts override as definitive.

In my utopia we would add service_provider fact but not use it directly, we would use a function such as service_provider_func which contains:

override = self.lookupdefaults('Service')[:provider].value
override = lookupvar('service_provider') if override.nil? || override.empty?

return override

@nibalizer
Copy link
Contributor

@asasfu I think you're correct now. I'm realizing that 'bubbling up the service provider' and 'overriding the service provider because puppet is slow to change' are different problems. As written, the function provides the information needed in that postgres code section. Overriding defaults as a way to get around patching puppet can be left for a different day, and yes your override system is on the right path there.

hunner added a commit that referenced this pull request Sep 24, 2015
@hunner hunner merged commit 4d1bca3 into puppetlabs:master Sep 24, 2015
@igalic
Copy link
Contributor

igalic commented Dec 12, 2015

on Ubuntu 15.10, i'm getting debian which seems quite wrong, given that the init there has been moved to systemd… but then that's a problem in puppet/type/service (or rather, the providers), not so much in this fact.

@asasfu
Copy link
Contributor

asasfu commented Dec 12, 2015

That's because Ubuntu 15.10 is not supported in your puppet release most likely.

@igalic
Copy link
Contributor

igalic commented Dec 12, 2015

puppet --version
4.3.1

@igalic
Copy link
Contributor

igalic commented Dec 14, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants