Skip to content

Commit c3b49a0

Browse files
Merge pull request #648 from adrianiurca/main
[MODULES-10734] - improve params detection on docker::run
2 parents ef89ada + 7e7c149 commit c3b49a0

File tree

4 files changed

+286
-9
lines changed

4 files changed

+286
-9
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
require 'open3'
2+
require 'json'
3+
4+
module Puppet::Parser::Functions
5+
# Checks if at least one parammeter is changed
6+
newfunction(:docker_params_changed, type: :rvalue) do |args|
7+
opts = args[0] || {}
8+
return_value = []
9+
10+
if opts['sanitised_title'] && opts['osfamily']
11+
stdout, stderr, status = Open3.capture3("docker inspect #{opts['sanitised_title']}")
12+
if stderr.to_s == '' && status.to_s.include?('exit 0')
13+
param_changed = false
14+
inspect_hash = JSON.parse(stdout)[0]
15+
16+
# check if the image was changed
17+
param_changed = true if opts['image'] && opts['image'] != inspect_hash['Config']['Image']
18+
19+
# check if something on volumes or mounts was changed(a new volume/mount was added or removed)
20+
param_changed = true if opts['volumes'].is_a?(String) && opts['volumes'].include?(':') && opts['volumes'] != inspect_hash['Mounts'].to_a[0] && opts['osfamily'] != 'windows'
21+
param_changed = true if opts['volumes'].is_a?(String) && !opts['volumes'].include?(':') && opts['volumes'] != inspect_hash['Config']['Volumes'].to_a[0] && opts['osfamily'] != 'windows'
22+
param_changed = true if opts['volumes'].is_a?(String) && opts['volumes'].scan(%r{(?=:)}).count == 2 && opts['volumes'] != inspect_hash['Mounts'].to_a[0] && opts['osfamily'] == 'windows'
23+
param_changed = if opts['volumes'].is_a?(String) && opts['volumes'].scan(%r{(?=:)}).count == 1 && opts['volumes'] != inspect_hash['Config']['Volumes'].to_a[0] && opts['osfamily'] == 'windows'
24+
true
25+
else
26+
param_changed
27+
end
28+
29+
pp_paths = opts['volumes'].reject { |item| item.include?(':') } if opts['volumes'].is_a?(Array) && opts['osfamily'] != 'windows'
30+
pp_mounts = opts['volumes'].select { |item| item.include?(':') } if opts['volumes'].is_a?(Array) && opts['osfamily'] != 'windows'
31+
pp_paths = opts['volumes'].select { |item| item.scan(%r{(?=:)}).count == 1 } if opts['volumes'].is_a?(Array) && opts['osfamily'] == 'windows'
32+
pp_mounts = opts['volumes'].select { |item| item.scan(%r{(?=:)}).count == 2 } if opts['volumes'].is_a?(Array) && opts['osfamily'] == 'windows'
33+
34+
inspect_paths = if inspect_hash['Config']['Volumes']
35+
inspect_hash['Config']['Volumes'].keys
36+
else
37+
[]
38+
end
39+
param_changed = true if pp_paths != inspect_paths
40+
41+
names = inspect_hash['Mounts'].map { |item| item.values[1] } if inspect_hash['Mounts']
42+
pp_names = pp_mounts.map { |item| item.split(':')[0] } if pp_mounts
43+
names = names.select { |item| pp_names.include?(item) } if names && pp_names
44+
destinations = inspect_hash['Mounts'].map { |item| item.values[3] } if inspect_hash['Mounts']
45+
pp_destinations = pp_mounts.map { |item| item.split(':')[1] } if pp_mounts && opts['osfamily'] != 'windows'
46+
pp_destinations = pp_mounts.map { |item| "#{item.split(':')[1].downcase}:#{item.split(':')[2]}" } if pp_mounts && opts['osfamily'] == 'windows'
47+
destinations = destinations.select { |item| pp_destinations.include?(item) } if destinations && pp_destinations
48+
49+
param_changed = true if pp_names != names
50+
param_changed = true if pp_destinations != destinations
51+
param_changed = true if pp_mounts != [] && inspect_hash['Mounts'].nil?
52+
53+
# check if something on ports was changed(some ports were added or removed)
54+
55+
ports = inspect_hash['HostConfig']['PortBindings'].keys
56+
ports = ports.map { |item| item.split('/')[0] }
57+
pp_ports = opts['ports'].sort if opts['ports'].is_a?(Array)
58+
pp_ports = [opts['ports']] if opts['ports'].is_a?(String)
59+
60+
param_changed = true if pp_ports && pp_ports != ports
61+
62+
return_value << if param_changed
63+
'PARAM_CHANGED'
64+
else
65+
'NO_CHANGE'
66+
end
67+
else
68+
return_value << 'CONTAINER_NOT_FOUND'
69+
end
70+
else
71+
return_value << 'ARG_REQUIRED_MISSING'
72+
end
73+
74+
return_value.flatten.join(' ')
75+
end
76+
end

lib/puppet/parser/functions/docker_run_flags.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ module Puppet::Parser::Functions
2323
end
2424

2525
if opts['net'].is_a? String
26-
flags << "--net #{opts['net']}"
26+
flags << "--net #{opts['net'].shellescape}"
2727
end
2828

2929
if opts['memory_limit']

manifests/run.pp

+64-8
Original file line numberDiff line numberDiff line change
@@ -400,18 +400,74 @@
400400
$inspect = [ "${docker_command} inspect ${sanitised_title}", ]
401401

402402
if $custom_unless {
403-
$exec_unless = concat($custom_unless, $inspect)
403+
$exec_unless = $custom_unless
404404
} else {
405405
$exec_unless = $inspect
406406
}
407407

408-
exec { "run ${title} with docker":
409-
command => join($run_with_docker_command, ' '),
410-
unless => $exec_unless,
411-
environment => $exec_environment,
412-
path => $exec_path,
413-
provider => $exec_provider,
414-
timeout => $exec_timeout,
408+
$detect_changes = docker_params_changed(
409+
{
410+
sanitised_title => $sanitised_title,
411+
osfamily => $facts['os']['family'],
412+
image => $image,
413+
volumes => $volumes,
414+
ports => $ports,
415+
}
416+
)
417+
418+
case $detect_changes {
419+
'CONTAINER_NOT_FOUND': {
420+
exec { "run ${title} with docker":
421+
command => join($run_with_docker_command, ' '),
422+
unless => $exec_unless,
423+
environment => $exec_environment,
424+
path => $exec_path,
425+
provider => $exec_provider,
426+
timeout => $exec_timeout,
427+
}
428+
}
429+
'PARAM_CHANGED': {
430+
exec { "stop ${title} with docker":
431+
command => "${docker_command} stop --time=${stop_wait_time} ${sanitised_title}",
432+
onlyif => "${docker_command} inspect ${sanitised_title}",
433+
environment => $exec_environment,
434+
path => $exec_path,
435+
provider => $exec_provider,
436+
timeout => $exec_timeout,
437+
}
438+
439+
exec { "remove ${title} with docker":
440+
command => "${docker_command} rm -v ${sanitised_title}",
441+
onlyif => "${docker_command} inspect ${sanitised_title}",
442+
environment => $exec_environment,
443+
path => $exec_path,
444+
provider => $exec_provider,
445+
timeout => $exec_timeout,
446+
}
447+
448+
file { $cidfile:
449+
ensure => absent,
450+
}
451+
452+
exec { "run ${title} with docker":
453+
command => join($run_with_docker_command, ' '),
454+
unless => $exec_unless,
455+
environment => $exec_environment,
456+
path => $exec_path,
457+
provider => $exec_provider,
458+
timeout => $exec_timeout,
459+
}
460+
}
461+
'ARG_REQUIRED_MISSING': {
462+
fail(translate('sanitised_title and osfamily are required for docker_params_changed function'))
463+
}
464+
default: {
465+
# Use ONLY on debugging - DO NOT UNCOMMENT OTHERWISE
466+
# notify { 'this case should not be executed':
467+
# message => "detect_changes: ${detect_changes}",
468+
# withpath => true,
469+
# }
470+
}
415471
}
416472

417473
if $running == false {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
require 'spec_helper_acceptance'
2+
3+
if os[:family] == 'windows'
4+
os_name = run_shell('systeminfo | findstr /R /C:"OS Name"')
5+
raise 'Could not retrieve systeminfo for Windows box' if os_name.exit_code != 0
6+
os_name = os_name.stdout.split(%r{\s}).include?('2016') ? 'win-2016' : 'win-2019'
7+
docker_args = 'docker_ee => true'
8+
docker_network = 'nat'
9+
volume_location = 'C:\\'
10+
docker_image = if os_name == 'win-2016'
11+
'stefanscherer/nanoserver:sac2016'
12+
else
13+
'stefanscherer/nanoserver:10.0.17763.1040'
14+
end
15+
else
16+
docker_args = if os[:family] == 'redhat'
17+
"repo_opt => '--enablerepo=localmirror-extras'"
18+
else
19+
''
20+
end
21+
docker_network = 'bridge'
22+
volume_location = '/opt'
23+
docker_image = 'hello-world:linux'
24+
end
25+
26+
describe 'docker trigger parammeters change' do
27+
before(:all) do
28+
if os[:family] != 'windows'
29+
install_pp = "class { 'docker': #{docker_args}}"
30+
apply_manifest(install_pp)
31+
end
32+
run_shell("mkdir #{volume_location}/volume_1")
33+
run_shell("mkdir #{volume_location}/volume_2")
34+
end
35+
36+
context 'when image is changed' do
37+
image_changed = if os[:family] == 'windows'
38+
if os_name == 'win-2016'
39+
'stefanscherer/nanoserver:10.0.14393.2551'
40+
else
41+
'stefanscherer/nanoserver:1809'
42+
end
43+
else
44+
'hello-world:latest'
45+
end
46+
let(:pp1) do
47+
"
48+
class {'docker': #{docker_args}}
49+
docker::run {'servercore': image => '#{docker_image}', restart => 'always', net => '#{docker_network}' }
50+
"
51+
end
52+
53+
let(:pp2) do
54+
"
55+
class {'docker': #{docker_args}}
56+
docker::run {'servercore': image => '#{image_changed}', restart => 'always', net => '#{docker_network}' }
57+
"
58+
end
59+
60+
it 'creates servercore with first image' do
61+
idempotent_apply(pp1)
62+
end
63+
64+
it 'detect image change and apply the change' do
65+
apply_manifest(pp2, catch_failures: true)
66+
run_shell('docker inspect --format="{{ .Config.Image }}" servercore') do |r|
67+
expect(r.stdout).to match(%r{#{image_changed}})
68+
end
69+
end
70+
end
71+
72+
context 'when volumes parameter is changed' do
73+
if os[:family] == 'windows'
74+
volumes1 = "volumes => ['volume-1:C:\\volume_1']"
75+
volumes2 = "volumes => ['volume-1:C:\\volume_1', 'volume-2:C:\\volume_2']"
76+
else
77+
volumes1 = "volumes => ['volume-1:#{volume_location}/volume_1']"
78+
volumes2 = "volumes => ['volume-1:#{volume_location}/volume_1', 'volume-2:#{volume_location}/volume_2']"
79+
end
80+
81+
let(:pp1) do
82+
"
83+
class {'docker': #{docker_args}}
84+
docker::run {'servercore': image => '#{docker_image}', restart => 'always', net => '#{docker_network}', #{volumes1}}
85+
"
86+
end
87+
88+
let(:pp2) do
89+
"
90+
class {'docker': #{docker_args}}
91+
docker::run {'servercore': image => '#{docker_image}', restart => 'always', net => '#{docker_network}', #{volumes2}}
92+
"
93+
end
94+
95+
it "creates servercore with #{volumes1}" do
96+
idempotent_apply(pp1)
97+
end
98+
99+
it "creates servercore with #{volumes2}" do
100+
apply_manifest(pp2, catch_failures: true)
101+
run_shell('docker inspect servercore --format="{{ json .Mounts }}"') do |r|
102+
inspect_result = JSON.parse(r.stdout)
103+
inspect_result = inspect_result.map { |item| item['Name'] }.sort
104+
expect(inspect_result).to eq(['volume-1', 'volume-2'])
105+
end
106+
end
107+
end
108+
109+
context 'when ports parameter is changed' do
110+
ports1 = "ports => ['4444']"
111+
ports2 = "ports => ['4444', '4445']"
112+
113+
let(:pp1) do
114+
"
115+
class {'docker': #{docker_args}}
116+
docker::run {'servercore': image => '#{docker_image}', restart => 'always', net => '#{docker_network}', #{ports1}}
117+
"
118+
end
119+
120+
let(:pp2) do
121+
"
122+
class {'docker': #{docker_args}}
123+
docker::run {'servercore': image => '#{docker_image}', restart => 'always', net => '#{docker_network}', #{ports2}}
124+
"
125+
end
126+
127+
it 'creates servercore with ports => ["4444"]' do
128+
idempotent_apply(pp1)
129+
end
130+
131+
it 'creates servercore with ports => ["4444", "4445"]' do
132+
apply_manifest(pp2, catch_failures: true)
133+
run_shell('docker inspect servercore --format="{{ json .HostConfig.PortBindings }}"') do |r|
134+
inspect_result = JSON.parse(r.stdout)
135+
inspect_result = inspect_result.keys.map { |item| item.split('/')[0] }.sort
136+
expect(inspect_result).to eq(['4444', '4445'])
137+
end
138+
end
139+
end
140+
141+
after(:all) do
142+
run_shell("rm -r #{volume_location}/volume_1")
143+
run_shell("rm -r #{volume_location}/volume_2")
144+
end
145+
end

0 commit comments

Comments
 (0)