From ad55480d78190915b786c65851cc45f34ff98f44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Ezr?= Date: Wed, 16 Oct 2019 03:46:51 +0200 Subject: [PATCH] Fixes #28067 - dynflow sidekiq services config --- .fixtures.yml | 1 + Gemfile | 1 + manifests/config.pp | 13 +++++ manifests/dynflow/worker.pp | 26 ++++++++++ manifests/init.pp | 9 ++-- manifests/install.pp | 6 +++ manifests/params.pp | 4 +- manifests/service.pp | 14 +++--- metadata.json | 4 ++ spec/acceptance/foreman_basic_spec.rb | 13 ++++- spec/acceptance/foreman_journald_spec.rb | 17 +++++-- spec/acceptance/foreman_prometheus_spec.rb | 13 ++++- spec/acceptance/foreman_reverse_proxy_spec.rb | 13 ++++- spec/acceptance/foreman_rex_cockpit_spec.rb | 13 ++++- spec/acceptance/foreman_statsd_spec.rb | 13 ++++- spec/classes/foreman_install_spec.rb | 11 +++++ spec/classes/foreman_service_spec.rb | 5 +- spec/classes/foreman_spec.rb | 22 ++++++++- spec/defines/foreman_dynflow_worker_spec.rb | 49 +++++++++++++++++++ spec/spec_helper_acceptance.rb | 3 ++ templates/dynflow_worker.yml.erb | 5 ++ templates/settings.yaml.erb | 3 ++ 22 files changed, 235 insertions(+), 23 deletions(-) create mode 100644 manifests/dynflow/worker.pp create mode 100644 spec/defines/foreman_dynflow_worker_spec.rb create mode 100644 templates/dynflow_worker.yml.erb diff --git a/.fixtures.yml b/.fixtures.yml index 0c3954a61..84e4ea51a 100644 --- a/.fixtures.yml +++ b/.fixtures.yml @@ -12,6 +12,7 @@ fixtures: extlib: 'https://github.com/voxpupuli/puppet-extlib' postgresql: 'https://github.com/puppetlabs/puppetlabs-postgresql' puppet: 'https://github.com/theforeman/puppet-puppet' + redis: 'https://github.com/voxpupuli/puppet-redis' systemd: 'https://github.com/camptocamp/puppet-systemd' selinux_core: repo: "https://github.com/puppetlabs/puppetlabs-selinux_core" diff --git a/Gemfile b/Gemfile index cca64d552..5297fcf25 100644 --- a/Gemfile +++ b/Gemfile @@ -30,6 +30,7 @@ gem 'beaker', '>= 4.2.0', {"groups"=>["system_tests"]} gem 'beaker-docker', {"groups"=>["system_tests"]} gem 'beaker-hostgenerator', '>= 1.1.10', {"groups"=>["system_tests"]} gem 'beaker-puppet', {"groups"=>["system_tests"]} +gem 'beaker-vagrant', {"groups"=>["system_tests"]} gem 'beaker-rspec', {"groups"=>["system_tests"]} gem 'beaker-module_install_helper', {"groups"=>["system_tests"]} gem 'beaker-puppet_install_helper', {"groups"=>["system_tests"]} diff --git a/manifests/config.pp b/manifests/config.pp index 013c50315..dbb6f5294 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -6,6 +6,19 @@ Class['puppet::server::install'] -> Class['foreman::config'] } + if $::foreman::jobs_manage_service { + if $::foreman::jobs_sidekiq_redis_url != undef { + $jobs_redis_url = $::foreman::jobs_sidekiq_redis_url + } else { + include ::redis + $jobs_redis_url = "redis://localhost:${::redis::port}/6" + } + + file { '/etc/foreman/dynflow': + ensure => directory, + } + } + concat::fragment {'foreman_settings+01-header.yaml': target => '/etc/foreman/settings.yaml', content => template('foreman/settings.yaml.erb'), diff --git a/manifests/dynflow/worker.pp b/manifests/dynflow/worker.pp new file mode 100644 index 000000000..73fc9a27a --- /dev/null +++ b/manifests/dynflow/worker.pp @@ -0,0 +1,26 @@ +# Initialize the dynflow worker +# @param concurrency +# Defines number of threads used for job processing +define foreman::dynflow::worker ( + String $service_name = $name, + Integer[1] $concurrency = $foreman::dynflow_pool_size, + Array[String] $queues = ['default', 'remote_execution'], + String $config_owner = 'root', + String $config_group = $::foreman::group, + Stdlib::Ensure::Service $service_ensure = $::foreman::jobs_service_ensure, + Boolean $service_enable = $::foreman::jobs_service_enable +) { + + file { "/etc/foreman/dynflow/${service_name}.yml": + ensure => file, + owner => $config_owner, + group => $config_group, + mode => '0644', + content => template('foreman/dynflow_worker.yml.erb'), + } + + service { "dynflow-sidekiq@${service_name}": + ensure => $service_ensure, + enable => $service_enable, + } +} diff --git a/manifests/init.pp b/manifests/init.pp index 36ca074fd..a78bb6b80 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -179,14 +179,16 @@ # # $telemetry_logger_level:: Telemetry debugging logs level # -# $dynflow_pool_size:: How many workers should Dynflow use +# $dynflow_pool_size:: How many threads per worker process should Dynflow use # -# $jobs_service:: Name of the service for running the background Dynflow executor +# $jobs_manage_service:: Whether to manage the dynflow services # # $jobs_service_enable:: Whether the Dynflow executor should be enabled # # $jobs_service_ensure:: Whether the Dynflow executor should be running or stopped # +# $jobs_sidekiq_redis_url:: If set, the redis server is not managed and we use the defined url to connect +# # $hsts_enabled:: Should HSTS enforcement in https requests be enabled # # $cors_domains:: List of domains that show be allowed for Cross-Origin Resource Sharing. This requires Foreman 1.22+ @@ -278,9 +280,10 @@ Boolean $telemetry_logger_enabled = $::foreman::params::telemetry_logger_enabled, Enum['DEBUG', 'INFO', 'WARN', 'ERROR', 'FATAL'] $telemetry_logger_level = $::foreman::params::telemetry_logger_level, Stdlib::Port $dynflow_pool_size = $::foreman::params::dynflow_pool_size, - String $jobs_service = $::foreman::params::jobs_service, + Boolean $jobs_manage_service = $::foreman::params::jobs_manage_service, Stdlib::Ensure::Service $jobs_service_ensure = $::foreman::params::jobs_service_ensure, Boolean $jobs_service_enable = $::foreman::params::jobs_service_enable, + Optional[Redis::RedisUrl] $jobs_sidekiq_redis_url = $::foreman::params::jobs_sidekiq_redis_url, Boolean $hsts_enabled = $::foreman::params::hsts_enabled, Array[Stdlib::HTTPUrl] $cors_domains = $::foreman::params::cors_domains, ) inherits foreman::params { diff --git a/manifests/install.pp b/manifests/install.pp index 20dbd7d9e..57aa15ef8 100644 --- a/manifests/install.pp +++ b/manifests/install.pp @@ -25,6 +25,12 @@ } } + if $::foreman::jobs_manage_service { + package { 'foreman-dynflow-sidekiq': + ensure => installed, + } + } + if $::foreman::ipa_authentication and $::foreman::ipa_manage_sssd { package { 'sssd-dbus': ensure => installed, diff --git a/manifests/params.pp b/manifests/params.pp index d17661e22..be29ee855 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -85,9 +85,11 @@ $foreman_service_port = 3000 # Define job processing service properties - $jobs_service = 'dynflowd' + $jobs_manage_service = true $jobs_service_ensure = 'running' $jobs_service_enable = true + $jobs_sidekiq_redis_url = undef + $hsts_enabled = true diff --git a/manifests/service.pp b/manifests/service.pp index f18daa939..5873db585 100644 --- a/manifests/service.pp +++ b/manifests/service.pp @@ -8,13 +8,15 @@ 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, + Boolean $jobs_manage_service = $::foreman::jobs_manage_service, ) { - service { $jobs_service: - ensure => $jobs_service_ensure, - enable => $jobs_service_enable, + if $jobs_manage_service { + foreman::dynflow::worker { 'orchestrator': + concurrency => 1, + queues => ['dynflow_orchestrator'], + } + + foreman::dynflow::worker { 'worker': } } if $apache { diff --git a/metadata.json b/metadata.json index c14f003e6..bd7f95141 100644 --- a/metadata.json +++ b/metadata.json @@ -35,6 +35,10 @@ { "name": "puppet/extlib", "version_requirement": ">= 3.0.0 < 5.0.0" + }, + { + "name": "puppet/redis", + "version_requirement": ">= 5.0.0 < 6.0.0" } ], "requirements": [ diff --git a/spec/acceptance/foreman_basic_spec.rb b/spec/acceptance/foreman_basic_spec.rb index 4a0052916..1c700984f 100644 --- a/spec/acceptance/foreman_basic_spec.rb +++ b/spec/acceptance/foreman_basic_spec.rb @@ -28,6 +28,12 @@ class { '::apache::mod::passenger': manage_repo => false, } + if $facts['os']['family'] == 'RedHat' and $facts['os']['name'] != 'Fedora' { + class { 'redis::globals': + scl => 'rh-redis5', + } + } + $directory = '/etc/foreman' $certificate = "${directory}/certificate.pem" $key = "${directory}/key.pem" @@ -68,7 +74,12 @@ class { '::foreman': it { is_expected.to be_running } end - describe service('dynflowd') do + describe service('dynflow-sidekiq@orchestrator') do + it { is_expected.to be_enabled } + it { is_expected.to be_running } + end + + describe service('dynflow-sidekiq@worker') do it { is_expected.to be_enabled } it { is_expected.to be_running } end diff --git a/spec/acceptance/foreman_journald_spec.rb b/spec/acceptance/foreman_journald_spec.rb index 90bb43b10..9ca7404cd 100644 --- a/spec/acceptance/foreman_journald_spec.rb +++ b/spec/acceptance/foreman_journald_spec.rb @@ -28,6 +28,12 @@ class { '::apache::mod::passenger': manage_repo => false, } + if $facts['os']['family'] == 'RedHat' and $facts['os']['name'] != 'Fedora' { + class { 'redis::globals': + scl => 'rh-redis5', + } + } + $directory = '/etc/foreman' $certificate = "${directory}/certificate.pem" $key = "${directory}/key.pem" @@ -69,7 +75,12 @@ class { '::foreman': it { is_expected.to be_running } end - describe service('dynflowd') do + describe service('dynflow-sidekiq@orchestrator') do + it { is_expected.to be_enabled } + it { is_expected.to be_running } + end + + describe service('dynflow-sidekiq@worker') do it { is_expected.to be_enabled } it { is_expected.to be_running } end @@ -97,7 +108,7 @@ class { '::foreman': its(:stdout) { is_expected.to match(%r{Redirected to https://#{host_inventory['fqdn']}/users/login}) } end - describe command('journalctl -u dynflowd'), unless: ENV['TRAVIS'] == 'true' && os[:family] == 'redhat' && os[:release] =~ /^7\./ do - its(:stdout) { is_expected.to match(%r{Dynflow Executor: start in progress}) } + describe command('journalctl -u dynflow-sidekiq@orchestrator'), unless: ENV['TRAVIS'] == 'true' && os[:family] == 'redhat' && os[:release] =~ /^7\./ do + its(:stdout) { is_expected.to match(%r{Everything ready for world: }) } end end diff --git a/spec/acceptance/foreman_prometheus_spec.rb b/spec/acceptance/foreman_prometheus_spec.rb index 7b1d34285..d2f1cf30a 100644 --- a/spec/acceptance/foreman_prometheus_spec.rb +++ b/spec/acceptance/foreman_prometheus_spec.rb @@ -28,6 +28,12 @@ class { '::apache::mod::passenger': manage_repo => false, } + if $facts['os']['family'] == 'RedHat' and $facts['os']['name'] != 'Fedora' { + class { 'redis::globals': + scl => 'rh-redis5', + } + } + $directory = '/etc/foreman' $certificate = "${directory}/certificate.pem" $key = "${directory}/key.pem" @@ -69,7 +75,12 @@ class { '::foreman': it { is_expected.to be_running } end - describe service('dynflowd') do + describe service('dynflow-sidekiq@orchestrator') do + it { is_expected.to be_enabled } + it { is_expected.to be_running } + end + + describe service('dynflow-sidekiq@worker') do it { is_expected.to be_enabled } it { is_expected.to be_running } end diff --git a/spec/acceptance/foreman_reverse_proxy_spec.rb b/spec/acceptance/foreman_reverse_proxy_spec.rb index 8405c9047..b146edc26 100644 --- a/spec/acceptance/foreman_reverse_proxy_spec.rb +++ b/spec/acceptance/foreman_reverse_proxy_spec.rb @@ -18,6 +18,12 @@ let(:pp) do <<-EOS + if $facts['os']['family'] == 'RedHat' and $facts['os']['name'] != 'Fedora' { + class { 'redis::globals': + scl => 'rh-redis5', + } + } + $directory = '/etc/foreman' $certificate = "${directory}/certificate.pem" $key = "${directory}/key.pem" @@ -59,7 +65,12 @@ class { '::foreman': it { is_expected.to be_running } end - describe service('dynflowd') do + describe service('dynflow-sidekiq@orchestrator') do + it { is_expected.to be_enabled } + it { is_expected.to be_running } + end + + describe service('dynflow-sidekiq@worker') do it { is_expected.to be_enabled } it { is_expected.to be_running } end diff --git a/spec/acceptance/foreman_rex_cockpit_spec.rb b/spec/acceptance/foreman_rex_cockpit_spec.rb index a361115d9..5c92e129b 100644 --- a/spec/acceptance/foreman_rex_cockpit_spec.rb +++ b/spec/acceptance/foreman_rex_cockpit_spec.rb @@ -18,6 +18,12 @@ let(:pp) do <<-EOS + if $facts['os']['family'] == 'RedHat' and $facts['os']['name'] != 'Fedora' { + class { 'redis::globals': + scl => 'rh-redis5', + } + } + $directory = '/etc/foreman' $certificate = "${directory}/certificate.pem" $key = "${directory}/key.pem" @@ -59,7 +65,12 @@ class { 'foreman': it { is_expected.to be_running } end - describe service('dynflowd') do + describe service('dynflow-sidekiq@orchestrator') do + it { is_expected.to be_enabled } + it { is_expected.to be_running } + end + + describe service('dynflow-sidekiq@worker') do it { is_expected.to be_enabled } it { is_expected.to be_running } end diff --git a/spec/acceptance/foreman_statsd_spec.rb b/spec/acceptance/foreman_statsd_spec.rb index ffeca2824..0f8e75dd0 100644 --- a/spec/acceptance/foreman_statsd_spec.rb +++ b/spec/acceptance/foreman_statsd_spec.rb @@ -28,6 +28,12 @@ class { '::apache::mod::passenger': manage_repo => false, } + if $facts['os']['family'] == 'RedHat' and $facts['os']['name'] != 'Fedora' { + class { 'redis::globals': + scl => 'rh-redis5', + } + } + $directory = '/etc/foreman' $certificate = "${directory}/certificate.pem" $key = "${directory}/key.pem" @@ -69,7 +75,12 @@ class { '::foreman': it { is_expected.to be_running } end - describe service('dynflowd') do + describe service('dynflow-sidekiq@orchestrator') do + it { is_expected.to be_enabled } + it { is_expected.to be_running } + end + + describe service('dynflow-sidekiq@worker') do it { is_expected.to be_enabled } it { is_expected.to be_running } end diff --git a/spec/classes/foreman_install_spec.rb b/spec/classes/foreman_install_spec.rb index 5270000b1..f21b977d3 100644 --- a/spec/classes/foreman_install_spec.rb +++ b/spec/classes/foreman_install_spec.rb @@ -19,6 +19,17 @@ it { should contain_package('foreman-postgresql').that_requires('Class[foreman::repo]') } end + describe 'sidekiq jobs' do + context 'with jobs_manage_service disabled' do + let(:params) { super().merge(jobs_manage_service: false) } + it { is_expected.not_to contain_package('foreman-dynflow-sidekiq') } + end + context 'with jobs_manage_service enabled' do + let(:params) { super().merge(jobs_manage_service: true) } + it { is_expected.to contain_package('foreman-dynflow-sidekiq') } + end + end + context 'with SELinux enabled' do let(:facts) { super().merge(selinux: true) } it { should contain_package('foreman-selinux') } diff --git a/spec/classes/foreman_service_spec.rb b/spec/classes/foreman_service_spec.rb index 26f1c53ea..58a4d00d4 100644 --- a/spec/classes/foreman_service_spec.rb +++ b/spec/classes/foreman_service_spec.rb @@ -15,9 +15,7 @@ foreman_service: 'foreman', foreman_service_ensure: 'running', foreman_service_enable: true, - jobs_service: 'dynflower', - jobs_service_ensure: 'stopped', - jobs_service_enable: false + jobs_manage_service: false } end @@ -28,7 +26,6 @@ context 'with ssl' do let(:params) { super().merge(ssl: true) } it { is_expected.to compile.with_all_deps } - it { is_expected.to contain_service('dynflower').with_ensure('stopped').with_enable(false) } it 'should restart passenger' do should contain_exec('restart_foreman') diff --git a/spec/classes/foreman_spec.rb b/spec/classes/foreman_spec.rb index b8b731c17..29fa37868 100644 --- a/spec/classes/foreman_spec.rb +++ b/spec/classes/foreman_spec.rb @@ -128,10 +128,22 @@ it { should contain_foreman__rake('db:seed') } it { should contain_foreman__rake('apipie:cache:index') } + # jobs + it { should contain_class('redis') } + it { should contain_redis__instance('default') } + it { should contain_file('/etc/foreman/dynflow').with_ensure('directory') } + it { + should contain_foreman__dynflow__worker('orchestrator') + .with_concurrency(1) + .with_queues(['dynflow_orchestrator']) + } + it { should contain_foreman__dynflow__worker('worker') } + # 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 { is_expected.to contain_service('dynflow-sidekiq@orchestrator').with_ensure('running').with_enable(true) } + it { is_expected.to contain_service('dynflow-sidekiq@worker').with_ensure('running').with_enable(true) } it 'should restart passenger' do should contain_exec('restart_foreman') @@ -392,6 +404,14 @@ .that_requires(['Class[Foreman::Service]', 'Service[httpd]']) end end + + describe 'with custom redis' do + context 'with redis_url' do + let(:params) { super().merge(jobs_sidekiq_redis_url: 'redis://127.0.0.1:4333/') } + it { should_not contain_class('redis') } + it { should_not contain_class('redis::instance') } + end + end end end end diff --git a/spec/defines/foreman_dynflow_worker_spec.rb b/spec/defines/foreman_dynflow_worker_spec.rb new file mode 100644 index 000000000..7ef0d26eb --- /dev/null +++ b/spec/defines/foreman_dynflow_worker_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe 'foreman::dynflow::worker' do + let :title do 'test_worker' end + + on_os_under_test.each do |os, facts| + context "on #{os}" do + let(:facts) { facts } + + let :pre_condition do + 'include foreman' + end + + context 'default parameters' do + it { + should contain_file('/etc/foreman/dynflow/test_worker.yml') + .with_ensure('file') + .with_owner('root') + .with_group('foreman') + .with_mode('0644') + .with_content(/:concurrency: 5/) + .with_content(/:queues:\n - default\n - remote_execution/) + } + it { + should contain_service('dynflow-sidekiq@test_worker') + .with_ensure('running') + .with_enable(true) + } + end + + context 'with custom concurrency and queues' do + let :params do + { + concurrency: 10, + queues: ['katello'] + } + end + + it { should contain_file('/etc/foreman/dynflow/test_worker.yml') + .with_ensure('file') + .with_owner('root') + .with_group('foreman') + .with_mode('0644') + .with_content(/:concurrency: 10/) + .with_content(/:queues:\n - katello\n$/) } + end + end + end +end diff --git a/spec/spec_helper_acceptance.rb b/spec/spec_helper_acceptance.rb index 2cacf971a..0953b7c1c 100644 --- a/spec/spec_helper_acceptance.rb +++ b/spec/spec_helper_acceptance.rb @@ -29,6 +29,9 @@ on host, 'sed -i "s/keepcache=.*/keepcache=1/" /etc/yum.conf' # refresh check if cache needs refresh on next yum command on host, 'yum clean expire-cache' + + host.install_package('centos-release-scl-rh') + host.install_package('rh-redis5-redis') end end end diff --git a/templates/dynflow_worker.yml.erb b/templates/dynflow_worker.yml.erb new file mode 100644 index 000000000..0145ba7c7 --- /dev/null +++ b/templates/dynflow_worker.yml.erb @@ -0,0 +1,5 @@ +:concurrency: <%= scope['concurrency'] %> +:queues: +<% scope['queues'].each do |queue| -%> + - <%= queue %> +<% end %> diff --git a/templates/settings.yaml.erb b/templates/settings.yaml.erb index 9aa1bd2e2..3322e2760 100644 --- a/templates/settings.yaml.erb +++ b/templates/settings.yaml.erb @@ -72,8 +72,11 @@ # logging level as in Logger::LEVEL :level: '<%= scope.lookupvar("foreman::telemetry_logger_level") %>' +<% if scope.lookupvar("foreman::jobs_manage_service") -%> :dynflow: :pool_size: <%= scope.lookupvar("foreman::dynflow_pool_size") %> + :redis_url: <%= scope.lookupvar("foreman::config::jobs_redis_url") %> +<% end %> <% if scope.lookupvar("foreman::apache") && !scope.lookupvar("foreman::passenger") -%> # Configure reverse proxy headers