Skip to content

Commit 7622d53

Browse files
TheMeierTheMeier
authored
Merge pull request #401 from voxpupuli/broker_systemd_manage_unit
refactor: use system::mange_unit for broker service
2 parents 394d62e + 23e80f6 commit 7622d53

File tree

4 files changed

+83
-21
lines changed

4 files changed

+83
-21
lines changed

‎manifests/broker/config.pp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
assert_private()
2121

2222
if ($manage_service and $service_restart) {
23-
$config_notify = Service[$service_name]
23+
$config_notify = Service["${service_name}.service"]
2424
} else {
2525
$config_notify = undef
2626
}

‎manifests/broker/service.pp‎

Lines changed: 55 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -35,27 +35,67 @@
3535
'KAFKA_OPTS' => $opts,
3636
'LOG_DIR' => $log_dir,
3737
}
38-
$environment = deep_merge($env_defaults, $env)
38+
$environment = deep_merge($env_defaults, $env).map |$k, $v| { "'${k}=${v}'" }
3939

4040
include systemd
4141

42-
if ($service_restart) {
43-
$config_notify = Service[$service_name]
44-
} else {
45-
$config_notify = undef
42+
$start_flag = $daemon_start ? {
43+
true => '-daemon ',
44+
default => '',
4645
}
4746

48-
file { "/etc/systemd/system/${service_name}.service":
49-
ensure => file,
50-
mode => '0644',
51-
content => template('kafka/unit.erb'),
52-
notify => $config_notify,
47+
$exec_stop_command = $exec_stop ? {
48+
true => "${bin_dir}/kafka-server-stop.sh",
49+
default => undef,
5350
}
54-
service { $service_name:
55-
ensure => $service_ensure,
56-
enable => true,
57-
hasstatus => true,
58-
hasrestart => true,
51+
52+
$type_content = $daemon_start ? {
53+
true => 'forking',
54+
default => 'simple',
55+
}
56+
57+
$active_content = $service_ensure == 'running' ? {
58+
true => true,
59+
default => false,
60+
}
61+
62+
$unit_entry = {
63+
'Description' => 'Apache Kafka server (broker)',
64+
'Documentation' => 'Documentation=http://kafka.apache.org/documentation.html',
65+
'After' => $service_requires.empty ? {
66+
true => undef,
67+
default => $service_requires,
68+
},
69+
'Wants' => $service_requires.empty ? {
70+
true => undef,
71+
default => $service_requires,
72+
},
73+
}.filter |$k, $v| { $v != undef }
74+
75+
$service_entry = {
76+
'User' => $user_name,
77+
'Group' => $group_name,
78+
'SyslogIdentifier' => $service_name,
79+
'Environment' => $environment,
80+
'ExecStart' => "${bin_dir}/kafka-server-start.sh ${start_flag}${config_dir}/server.properties",
81+
'ExecStop' => $exec_stop_command,
82+
'Restart' => 'on-failure',
83+
'Type' => $type_content,
84+
'LimitCORE' => $limit_core,
85+
'LimitNOFILE' => $limit_nofile,
86+
'TimeoutStopSec' => $timeout_stop,
87+
}.filter |$k, $v| { $v != undef }
88+
89+
systemd::manage_unit { "${service_name}.service":
90+
ensure => 'present',
91+
enable => true,
92+
active => $active_content,
93+
service_restart => $service_restart,
94+
unit_entry => $unit_entry,
95+
service_entry => $service_entry,
96+
install_entry => {
97+
'WantedBy' => 'multi-user.target',
98+
},
5999
}
60100
}
61101
}

‎spec/acceptance/broker_spec.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ class { 'kafka::broker':
185185
config => {
186186
'zookeeper.connect' => 'localhost:2181',
187187
},
188-
heap_opts => '-Xmx512M -Xmx512M',
188+
heap_opts => '-Xmx512M -Xms512M',
189189
log4j_opts => '-Dlog4j.configuration=file:/tmp/log4j.properties',
190190
jmx_opts => '-Dcom.sun.management.jmxremote',
191191
opts => '-Djava.security.policy=/some/path/my.policy',
@@ -202,7 +202,7 @@ class { 'kafka::broker':
202202
it { is_expected.to be_owned_by 'root' }
203203
it { is_expected.to be_grouped_into 'root' }
204204
it { is_expected.to contain "Environment='KAFKA_JMX_OPTS=-Dcom.sun.management.jmxremote'" }
205-
it { is_expected.to contain "Environment='KAFKA_HEAP_OPTS=-Xmx512M -Xmx512M'" }
205+
it { is_expected.to contain "Environment='KAFKA_HEAP_OPTS=-Xmx512M -Xms512M'" }
206206
it { is_expected.to contain "Environment='KAFKA_LOG4J_OPTS=-Dlog4j.configuration=file:/tmp/log4j.properties'" }
207207
it { is_expected.to contain "Environment='KAFKA_OPTS=-Djava.security.policy=/some/path/my.policy'" }
208208
it { is_expected.to contain "Environment='LOG_DIR=/some/path/to/logs'" }

‎spec/classes/broker_spec.rb‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@
2323
it { is_expected.to contain_class('kafka::broker::service').that_comes_before('Class[kafka::broker]') }
2424
it { is_expected.to contain_class('kafka::broker') }
2525

26+
it {
27+
is_expected.to contain_file('/etc/systemd/system/kafka.service').
28+
with_owner('root').
29+
with_content(%r{^Environment='KAFKA_HEAP_OPTS=-Xmx1G -Xms1G'$})
30+
}
31+
2632
context 'with invalid mirror_url' do
2733
let(:params) { { 'mirror_url' => 'invalid' } }
2834

@@ -69,10 +75,14 @@
6975
end
7076

7177
context 'defaults' do
72-
it { is_expected.to contain_file('/etc/systemd/system/kafka.service').with_notify('Service[kafka]') }
78+
it {
79+
is_expected.to contain_file('/etc/systemd/system/kafka.service').
80+
with_notify('["Systemd::Daemon_reload[kafka.service]", "Service[kafka.service]"]')
81+
}
82+
7383
it { is_expected.not_to contain_file('/etc/systemd/system/kafka.service').with_content %r{^LimitNOFILE=} }
7484
it { is_expected.not_to contain_file('/etc/systemd/system/kafka.service').with_content %r{^LimitCORE=} }
75-
it { is_expected.to contain_service('kafka') }
85+
it { is_expected.to contain_service('kafka.service') }
7686
end
7787

7888
context 'limit_nofile set' do
@@ -87,13 +97,25 @@
8797
it { is_expected.to contain_file('/etc/systemd/system/kafka.service').with_content %r{^LimitCORE=infinity$} }
8898
end
8999

90-
context 'service_requires set', if: os_facts['service_provider'] == 'systemd' do
100+
context 'service_requires set' do
91101
let(:params) { super().merge(service_requires: ['dummy.target']) }
92102

93103
it { is_expected.to contain_file('/etc/systemd/system/kafka.service').with_content %r{^After=dummy\.target$} }
94104
it { is_expected.to contain_file('/etc/systemd/system/kafka.service').with_content %r{^Wants=dummy\.target$} }
95105
end
96106

107+
context 'multiple service_requires set' do
108+
let(:params) { super().merge(service_requires: ['dummy.target', 'another.target']) }
109+
110+
it {
111+
is_expected.to contain_file('/etc/systemd/system/kafka.service').
112+
with_content(%r{^After=dummy\.target$}).
113+
with_content(%r{^Wants=dummy\.target$}).
114+
with_content(%r{^After=another\.target$}).
115+
with_content(%r{^Wants=another\.target$})
116+
}
117+
end
118+
97119
context 'service_restart false' do
98120
let(:params) { super().merge(service_restart: false) }
99121

0 commit comments

Comments
 (0)