Adding support for Ubuntu 15.04

[asasfu]

Added fact 'is_systemd' to detect if Ubuntu 15.04 is running in upstart or systemd mode, systemd is default but still optional.
Added support in params to detect Ubuntu 15.04.
Added support for Ubuntu 15.04 path to systemctl by changing how the path ENV is handled in config.pp (Ubuntu 15.04 uses /bin/systemctl)

[aj-jester]

can we condense this into:

$init_style = str2bool($::is_systemd) ? {
  true    => 'systemd',
  default => 'upstart',
}

[asasfu]

Sure can. I'll do that in a few minutes. I'm working through your code at the moment as well to patch it up as It doesn't update consul version if it's already installed. This is due to the 'creates' section in staging and how this module is using it. I have a plan of attack to make use of versioning and am about half way through patching it... well fully done with consul, investigating web_ui at the moment.

[aj-jester]

Oh nice! This has been a LONG standing issue with the url install method. I myself have been wanting to write one just hadn't had the time.

This is as close as we've gotten #118

[asasfu]

Ya, I saw that. I'm pretty sure mine will work, theoretically it does :) But I may call it a weekend before I get any thorough testing done on it and return to fixing it on Monday. I'm hoping to deploy consul using this to our University soon enough so I want to make sure everything is up to snuff. Since there is no yum packages or apt that I know of for consul in the distros that we're using.

[aj-jester]

No rush dude! heh. Yah, unfortunately there are no packages. When I have some time I'll build a package and try to deploy it to a public apt repo.

Let me know how the consul deploy goes!

[asasfu]

Hey AJ.
You've been really helpful so far with the responses to my PR. You mind just peeking at this commit with the working consul upgrade/downgrade (web_ui) not done yet but part way in.
sfu-rcg@8fbbf44
It tests fine for me. The Exec { provider => shell } is because staging module doesn't allow us to push path to the exec and I can't tie the test, grep, and rm down to specific directories in the 'onlyif' statement as they are located in different locations in Ubuntu vs CentOS. provider => shell fixes that up :)

I'm not sure how constrained the Exec { } is when called there. Does it only affect local items to the manifest plus things it calls or?
As I could be more specific and do Exec["extract ${consul::real_download_file}"] { provider => shell }

[aj-jester]

Can we either use $::lsbdistrelease here or replace the above with $::operatingsystemrelease for consistency sake? I'm in favor of the latter because $::operatingsystemrelease is used everywhere else.

[asasfu]

I can't be authoritative on that comment. I don't know Ubuntu that far back to know if puppet supported $::operatingsystemrelease fact back then, I've heard that there was no $::operatingsystemmajrelease in older versions of some linux distros. So I figured i'd let it stay, just in case as i'm not the pro on old linux systems.

[aj-jester]

I wasn't aware. Then lets change this to $::lsbdistrelease as well since we know it exists even on the latest versions. My concern is we don't want to troubleshoot a bug where these two facts have different values.

[asasfu]

I started programming puppet after version 3 and I've been a fan of less dependencies. So I was always trying to convert our old stuff to operatingsystemrelease and operatingsystemmajrelease.
lsbdistrelease is relying on redhat-lsb and whatever the package is in Ubuntu and other distros. I believe the operatingsystem values are coming straight from facter Facter::Operatingsystem.implementation
Though i'm not sure where it gets that from. I'm pretty sure the standing thought in puppet programming was to phase lsb out but that might be a personal preference of people... as I said, I haven't been doing this long enough. :)

[aj-jester]

According to https://launchpad.net/ubuntu/vivid/+package/lsb-release

"The Linux Standard Base (http://www.linuxbase.org/) is a standard
core system that third-party applications written for Linux can
depend upon"

So I think we are safe 😄 my vote is for consistency over deprecation/dependencies, in this specific case. But I agree with you in general, we should try to move away from legacy stuff as much as possible.

[aj-jester]

Also, if you are curious, this is how $::operatingsystemrelease gets its fact: https://docs.puppetlabs.com/facter/1.6/core_facts.html#operatingsystemrelease
https://docs.puppetlabs.com/facter/2.3/core_facts.html#operatingsystemrelease

As you can see its a mess lol

[aj-jester]

@asasfu this looks great! Can you please write a unit test for this? After that this should be good to merge!

[asasfu]

I'm not sure what happened on that Travis CI build but it kinda went tits up... I have never done a unit test before... :(

I think I got the unit test wrote up. Just realized that test box I used to add those last few commits doesn't have a working ntp on it and has an old date. Will that fuck up the commit history?

[asasfu]

Also, i'm curious, how do you guys handle the 'consul reload' .... if the ENV variable(s) are not set such as CONSUL_RPC_ADDR=facter ipaddress:8400 then consul reload fails during the puppet run...

[aj-jester]

All params are set in config.json, nothing is via ENV.

In this case, if you want a specific rpc address just pass the rpc key to $consul::config_hash. This will basically convert the hash into a json object and dumps it in config.json. So you can pass any configuration option supported by consul.

If you dont set rpc key or if ENV var CONSUL_RPC_ADDR is not set, it defaults to client_addr https://www.consul.io/docs/agent/options.html#rpc

[asasfu]

Ah you're right, it doesn't affect the puppet run's consul reload, just the user if he wants to type consul members or something needs to set his ENV or set client_addr to 0.0.0.0 or use reverse_proxy... because if client_addr is set to the specific IP it seems that when you run consul members from the command line it tries to contact 127.0.0.1:8400

[asasfu]

Ok, my commit history time issue was corrected and I fixed up the spec tests. This should be ready for production.

On a side note: When Puppet loads all of its providers and checks its confines to decide which Service provider matches your system. Do you guys know if there's a way to get the provider that it picked via the confines? That'd be really nice. Since their confines do all the work and then it's just one place to track that information down from in order to decide what init_style puppet has decided to be correct for your system, based on confines that they maintain.

[aj-jester]

🚄 LGTM

Do you guys know if there's a way to get the provider that it picked via the confines?

I'm not exactly sure if thats possible 😕

[solarkennedy]

Thanks!

Yea you can't get a provider for something in puppet and have that data influence the catalog; the catalog is compile first and then applied to the host where providers are determined at the last moment.

[asasfu]

Hey hey, you wanna know something you can do?! While i'm preparing the ability for URL upgrade/downgrade I finally figured out how to stop the service before removing consul bin.

service_name = 'consul'
actionstop = 'start' # e.g., (start|stop|restart|status)
actionstart = 'start' # e.g., (start|stop|restart|status)

require 'puppet'
service = Puppet::Type.type(:service).newservice(:name => service_name)

service.provider.send(actionstop)

####### Complete replacement of version, including CONSUL BIN

service.provider.send(actionstart)

So I just need to create a function that does all the work and does those steps as well.

[asasfu]

@aj-jester and @solarkennedy you mind looking at this commit to see if you have any suggestions or gripes. I know it has some conditionals in it but it works for all cases that should be presented. It also leaves staging files behind per version rather than cleaning up, something I can add(cleanup) if wanted.
sfu-rcg@707af3f

Actually it's probably better to reference this comparison as well, since AJ and I have already talked on it:
master...sfu-rcg:fix_url_upgrade_and_downgrade

[asasfu]

My work is likely considered complete on that patch. Can you guys take a peek. I'll be submitting it as a pull request tomorrow. It has tested good on both my masterless and mastered puppet client. I'm not sure what or any tests you may want written for this prior to merging?

[asasfu]

Also now that I've done my testing, I've finally come across what I was looking for, that might be helpful to you guys when doing up params to decide init_style.
$prov = inline_template("<%= Puppet::Type.type(:service).newservice(:name => 'nonexistant').provider %>")
$provsub = split($prov, 'provider=([A-Za-z]+)')
notify { "Provider is: ${provsub[1]}": }

again though, this only supports providers already setup in confines by puppet, this doesn't support any site.pp overrides for provider or anything, so those slight edge cases like Ubuntu 15.04 and Fedora 22 in Puppet 3.x will still return the wrong provider as this is ASKING puppet what they think the provider should be. But still altogether, really cool.

[asasfu]

Ok part 2, even better. Provider with and without override provided by an upper class or site.pp via:
Service { provider => 'systemd' }

With override, what I'm doing here is calling to defaults and return provider:
$defaultprovscoperbe = inline_template("<% @def = scope.lookupdefaults('Service') %><%= @def[:provider] %>")

Without override, here I'm retrieving service provider with no allowance for anyone to have overridden it earlier and they'd have to use consul::init_style to actually override it then for example:
$prove = inline_template("<% @def2 = Puppet::Type.type(:service).newservice(:name => 'anyservice') %><%= @def2[:provider] %>")

Finally, no parsing needed this time, tested and showing proof:
notify { "Default Provider is: ${defaultprovscoperbe} and Provider is: ${prove}": }

[asasfu]

Last part, part 3.

This is the final piece of code as I noticed a parse needed to happen on the default provider on part2 and then this supports merging.

If you provide a Service { provider => systemd } in your site.pp or even parent class calling consul then you would override service { 'consul': } unless it was service { 'consul': provider => 'upstart' } as the more specific class wins if it defines that parameter or hash key/value.
I've provided the way to do this via ruby so that you can decide which one you would like, the one that is overridden by the user via Service { provider => systemd } or the one that doesn't care if you overrode it at all, it takes only what puppet's confines say the provider is.
I highly prefer the overridden one for services because you should probably always allow the user to have the final say on the provider if they needed to specify it in the first place then they've decided puppet isn't handling their provider via confines and needs fixing.

$merged_override_wins = inline_template("<% @puppet_decided = Puppet::Type.type(:service).newservice(:name => 'anyservice')[:provider].to_s %><% @OverRide = scope.lookupdefaults('Service')[:provider].instance_variable_get(:@value) %><% @OverRide = @puppet_decided if @override.nil? || @override.empty? %><%= @OverRide %>")

notify { "Merged provider with override being the winner: ${merged_override_wins}": }

$non_overridden = inline_template("<%= Puppet::Type::Service.new(:name => 'anyservice')[:provider] %>")

notify { "Provider is: ${non_overridden}": }

Might be a consideration to use this code to decide your init_style rather than what is currently in params. Then you would be modifying your conditional to match what they have in /usr/lib/ruby/vendor_ruby/puppet/provider/service/
/usr/lib/ruby/vendor_ruby/puppet/provider/service/
base.rb daemontools.rb freebsd.rb init.rb openbsd.rb openwrt.rb runit.rb smf.rb systemd.rb windows.rb
bsd.rb debian.rb gentoo.rb launchd.rb openrc.rb redhat.rb service.rb src.rb upstart.rb

Then you'd just have a selector stating what init script you're building based off that. No need to say, centos < 7 is redhat and centos >= 7 is systemd, ubuntu < 8 is debian, ubuntu > 8 is upstart, ubuntu > 15 is systemd. Kinda seems like a lot of decisions we're making that puppet or the user already does for us now that we know how to grab their decisions.

Thoughts?