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

Jump to bottom

Refactor running with a service to Foreman 1.22 #723

Merged
merged 1 commit into from
Apr 18, 2019
Merged

Refactor running with a service to Foreman 1.22 #723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions .fixtures.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ fixtures:
mysql: 'https://github.com/puppetlabs/puppetlabs-mysql'
postgresql: 'https://github.com/puppetlabs/puppetlabs-postgresql'
puppet: 'https://github.com/theforeman/puppet-puppet'
systemd: 'https://github.com/camptocamp/puppet-systemd'
selinux_core:
repo: "https://github.com/puppetlabs/puppetlabs-selinux_core"
puppet_version: ">= 6.0.0"
Expand Down
11 changes: 11 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ previous stable release.

### Foreman version compatibility notes

Running without passenger is only supported on Foreman 1.22+.

The parameters `locations_enabled`, `organizations_enabled` and `authentication`
will only have any affect on Foreman 1.20 or older, in newer versions these
settings have been removed.
Expand All @@ -67,6 +69,15 @@ authentication.

For Foreman 1.16 or older, please use the 9.x release series of this module.

## Running without passenger

To use this module without passenger, the `passenger` parameter must be set to
`false`. This will install the `foreman-service` package and ensure the service
is running.

This introduces a soft dependency on `camptocamp-systemd`. This feature is only
available on Foreman 1.22+.

## Types and providers

`foreman_config_entry` can be used to manage settings in Foreman's database, as
Expand Down
8 changes: 5 additions & 3 deletions manifests/config.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@
ensure => absent,
}

file { $::foreman::init_config:
ensure => file,
content => template("foreman/${foreman::init_config_tmpl}.erb"),
if $::foreman::use_foreman_service {
systemd::dropin_file { 'installer.conf':
Copy link
Member

Choose a reason for hiding this comment

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

Why is it called installer.conf ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought it'd make it easy to track where it came from. It ends up as /etc/systemd/system/foreman.service.d/installer.conf

unit => "${::foreman::foreman_service}.service",
content => template('foreman/foreman.service-overrides.erb'),
}
}

file { $::foreman::app_root:
Expand Down
2 changes: 2 additions & 0 deletions manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,8 @@
timeout => 0,
}

$use_foreman_service = ! $passenger
Copy link
Member

Choose a reason for hiding this comment

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

I get why we need to set passenger to false explicitly in the current setup, but I do find not setting use_foreman_service unobvious when looking at the parameters someone would be specifying. If we will reverse this very soon after the 1.22 release then I can live with it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Note this is currently not a parameter but rather a variable inside the body so the user can't override this. In the reverse proxy scenario I'm expanding this logic. Because I couldn't properly figure out an API, I opted not to expose this to the user directly.


include ::foreman::repo
include ::foreman::install
include ::foreman::config
Expand Down
6 changes: 6 additions & 0 deletions manifests/install.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@
}
}

if $::foreman::use_foreman_service {
package { 'foreman-service':
ensure => installed,
}
}

if $::foreman::ipa_authentication and $::foreman::ipa_manage_sssd {
package { 'sssd-dbus':
ensure => installed,
Expand Down
14 changes: 7 additions & 7 deletions manifests/params.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,13 @@
# Configure how many workers should Dynflow use
$dynflow_pool_size = 5

# Define foreman service
$foreman_service = 'foreman'
$foreman_service_ensure = 'running'
$foreman_service_enable = true
$foreman_service_port = 3000
$foreman_service_bind = undef

# Define job processing service properties
$jobs_service = 'dynflowd'
$jobs_service_ensure = 'running'
Expand All @@ -98,9 +105,6 @@
# OS specific paths
case $::osfamily {
'RedHat': {
$init_config = '/etc/sysconfig/foreman'
$init_config_tmpl = 'foreman.sysconfig'

case $::operatingsystem {
'fedora': {
$passenger_ruby = undef
Expand All @@ -119,8 +123,6 @@
$passenger_ruby = '/usr/bin/foreman-ruby'
$passenger_ruby_package = undef
$plugin_prefix = 'ruby-foreman-'
$init_config = '/etc/default/foreman'
$init_config_tmpl = 'foreman.default'
}
'Linux': {
case $::operatingsystem {
Expand All @@ -129,8 +131,6 @@
$passenger_ruby = '/usr/bin/tfm-ruby'
$passenger_ruby_package = 'tfm-rubygem-passenger-native'
$plugin_prefix = 'tfm-rubygem-foreman_'
$init_config = '/etc/sysconfig/foreman'
$init_config_tmpl = 'foreman.sysconfig'
}
default: {
fail("${::hostname}: This module does not support operatingsystem ${::operatingsystem}")
Expand Down
19 changes: 9 additions & 10 deletions manifests/service.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
Boolean $passenger = $::foreman::passenger,
Stdlib::Absolutepath $app_root = $::foreman::app_root,
Boolean $ssl = $::foreman::ssl,
Boolean $use_foreman_service = $::foreman::use_foreman_service,
String $foreman_service = $::foreman::foreman_service,
Stdlib::Ensure::Service $foreman_service_ensure = $::foreman::foreman_service_ensure,
Boolean $foreman_service_enable = $::foreman::foreman_service_enable,
String $jobs_service = $::foreman::jobs_service,
Stdlib::Ensure::Service $jobs_service_ensure = $::foreman::jobs_service_ensure,
Boolean $jobs_service_enable = $::foreman::jobs_service_enable,
Expand All @@ -27,17 +31,12 @@
if $ssl and defined(Class['puppet::server::config']) {
Class['puppet::server::config'] -> Class['foreman::service']
}

$service_ensure = 'stopped'
$service_enabled = false
} else {
$service_ensure = 'running'
$service_enabled = true
}

service {'foreman':
ensure => $service_ensure,
enable => $service_enabled,
hasstatus => true,
if $use_foreman_service {
service { $foreman_service:
ensure => $foreman_service_ensure,
enable => $foreman_service_enable,
}
}
}
15 changes: 6 additions & 9 deletions spec/classes/foreman_service_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
passenger: true,
app_root: '/usr/share/foreman',
ssl: true,
use_foreman_service: false,
foreman_service: 'foreman',
foreman_service_ensure: 'running',
foreman_service_enable: true,
jobs_service: 'dynflower',
jobs_service_ensure: 'stopped',
jobs_service_enable: false
Expand All @@ -35,13 +39,7 @@

it { should contain_service('httpd').that_comes_before('Class[foreman::service]') }
it { should contain_class('apache::service').that_comes_before('Class[foreman::service]') }

it do
should contain_service('foreman')
.with_ensure('stopped')
.with_enable(false)
.with_hasstatus(true)
end
it { should_not contain_service('foreman') }
end

context 'without ssl' do
Expand All @@ -51,14 +49,13 @@
end

context 'without passenger' do
let(:params) { super().merge(passenger: false) }
let(:params) { super().merge(passenger: false, use_foreman_service: true) }
it { is_expected.to compile.with_all_deps }
it { should_not contain_exec('restart_foreman') }
it do
should contain_service('foreman')
.with_ensure('running')
.with_enable(true)
.with_hasstatus(true)
end
end
end
34 changes: 7 additions & 27 deletions spec/classes/foreman_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
it { should contain_class('foreman::install') }
it { should contain_package('foreman-postgresql').with_ensure('present') }
it { should_not contain_package('foreman-journald') }
it { should_not contain_package('foreman-service') }

# config
it do
Expand Down Expand Up @@ -75,26 +76,7 @@
.with_content(/adapter: postgresql/)
end

case facts[:osfamily]
when 'RedHat'
it 'should set the defaults file' do
should contain_file('/etc/sysconfig/foreman')
.with_content(%r{^FOREMAN_HOME=/usr/share/foreman$})
.with_content(/^FOREMAN_USER=foreman$/)
.with_content(/^FOREMAN_ENV=production/)
.with_content(/^FOREMAN_USE_PASSENGER=1$/)
.with_ensure('file')
end
when 'Debian'
it 'should set the defaults file' do
should contain_file('/etc/default/foreman')
.with_content(/^START=no$/)
.with_content(%r{^FOREMAN_HOME=/usr/share/foreman$})
.with_content(/^FOREMAN_USER=foreman$/)
.with_content(/^FOREMAN_ENV=production/)
.with_ensure('file')
end
end
it { should_not contain_systemd__dropin_file('installer.conf') }

it { should contain_file('/usr/share/foreman').with_ensure('directory') }

Expand Down Expand Up @@ -149,6 +131,7 @@

# service
it { should contain_class('foreman::service') }
it { should_not contain_service('foreman') }
it { is_expected.to contain_service('dynflowd').with_ensure('running').with_enable(true) }

it 'should restart passenger' do
Expand All @@ -159,13 +142,6 @@
.with_path('/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin')
end

it do
should contain_service('foreman')
.with_ensure('stopped')
.with_enable(false)
.with_hasstatus(true)
end

# settings
it { should contain_class('foreman::settings').that_requires('Class[foreman::database]') }
end
Expand All @@ -175,6 +151,10 @@

it { should compile.with_all_deps }
it { should_not contain_class('foreman::config::apache') }
it { should contain_package('foreman-service').with_ensure('installed') }
it { should contain_systemd__dropin_file('installer.conf').with_unit('foreman.service') }
it { should contain_service('foreman').with_ensure('running') }
it { should_not contain_exec('restart_foreman') }
end

describe 'with passenger interface' do
Expand Down
21 changes: 0 additions & 21 deletions templates/foreman.default.erb
View file
Open in desktop

This file was deleted.

8 changes: 8 additions & 0 deletions templates/foreman.service-overrides.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[Service]
User=<%= scope['foreman::user'] %>
Environment=FOREMAN_ENV=<%= scope['foreman::rails_env'] %>
Environment=FOREMAN_HOME=<%= scope['foreman::app_root'] %>
<% if scope['foreman::foreman_service_bind'] -%>
Environment=FOREMAN_BIND=<%= scope['foreman::foreman_service_bind'] %>
<% end -%>
Environment=FOREMAN_PORT=<%= scope['foreman::foreman_service_port'] %>
22 changes: 0 additions & 22 deletions templates/foreman.sysconfig.erb
View file
Open in desktop

This file was deleted.