Skip to content
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.

Sign up for GitHub

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

Adding support for Ubuntu 15.04 #163

Merged
merged 8 commits into from
Aug 10, 2015
Merged

Adding support for Ubuntu 15.04 #163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c813053
Adding support for Ubuntu 15.04
asasfu Aug 8, 2015
c411f8d
Travis CI fixes
asasfu Aug 8, 2015
76885dc
More Travis CI fixes
asasfu Aug 8, 2015
e075d30
Puppetlabs documentation tells me to quote the str2bool, turns out th…
asasfu Aug 8, 2015
9354524
Changed to reduced lines of code in is_systemd test as per aj-jester
asasfu Aug 8, 2015
1675a87
Removed is_systemd, going with sane default
asasfu Aug 8, 2015
883cc57
Adding spec test for Ubuntu 15.04 and fixing Ubuntu 8 to operatingsys…
asasfu Aug 8, 2015
67dcbb5
Fixed init_spec.rb default away from lsb
asasfu Aug 8, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion manifests/config.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@
content => template('consul/consul.systemd.erb'),
}~>
exec { 'consul-systemd-reload':
command => '/usr/bin/systemctl daemon-reload',
command => 'systemctl daemon-reload',
path => [ '/usr/bin', '/bin', '/usr/sbin' ],
refreshonly => true,
}
}
Expand Down
6 changes: 4 additions & 2 deletions manifests/params.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@
$os = downcase($::kernel)

if $::operatingsystem == 'Ubuntu' {
if versioncmp($::lsbdistrelease, '8.04') < 1 {
if versioncmp($::operatingsystemrelease, '8.04') < 1 {
$init_style = 'debian'
} else {
} elsif versioncmp($::operatingsystemrelease, '15.04') < 0 {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. :)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

$init_style = 'upstart'
} else {
$init_style = 'systemd'
}
} elsif $::operatingsystem =~ /Scientific|CentOS|RedHat|OracleLinux/ {
if versioncmp($::operatingsystemrelease, '7.0') < 0 {
Expand Down
3 changes: 2 additions & 1 deletion metadata.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@
"operatingsystemrelease": [
"12.04",
"10.04",
"14.04"
"14.04",
"15.04"
]
},
{
Expand Down
22 changes: 16 additions & 6 deletions spec/classes/init_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@

RSpec.configure do |c|
c.default_facts = {
:architecture => 'x86_64',
:operatingsystem => 'Ubuntu',
:osfamily => 'Debian',
:lsbdistrelease => '10.04',
:kernel => 'Linux',
:architecture => 'x86_64',
:operatingsystem => 'Ubuntu',
:osfamily => 'Debian',
:operatingsystemrelease => '10.04',
:kernel => 'Linux',
}
end
# Installation Stuff
Expand Down Expand Up @@ -374,7 +374,7 @@
context "On hardy" do
let(:facts) {{
:operatingsystem => 'Ubuntu',
:lsbdistrelease => '8.04',
:operatingsystemrelease => '8.04',
}}

it { should contain_class('consul').with_init_style('debian') }
Expand All @@ -386,6 +386,16 @@
}
end

context "On a Ubuntu Vivid 15.04 based OS" do
let(:facts) {{
:operatingsystem => 'Ubuntu',
:operatingsystemrelease => '15.04'
}}

it { should contain_class('consul').with_init_style('systemd') }
it { should contain_file('/lib/systemd/system/consul.service').with_content(/consul agent/) }
end

context "When asked not to manage the init_style" do
let(:params) {{ :init_style => false }}
it { should contain_class('consul').with_init_style(false) }
Expand Down