diff --git a/lib/puppet/parser/functions/docker_params_changed.rb b/lib/puppet/functions/docker_params_changed.rb similarity index 51% rename from lib/puppet/parser/functions/docker_params_changed.rb rename to lib/puppet/functions/docker_params_changed.rb index 325cbc7f..9cb8c36f 100644 --- a/lib/puppet/parser/functions/docker_params_changed.rb +++ b/lib/puppet/functions/docker_params_changed.rb @@ -1,13 +1,80 @@ # frozen_string_literal: true -require 'open3' -require 'json' +Puppet::Functions.create_function(:docker_params_changed) do + dispatch :detect_changes do + param 'Hash', :opts + return_type 'String' + end + + def run_with_powershell(cmd) + "powershell.exe -Command \"& {#{cmd}}\" " + end -module Puppet::Parser::Functions - # Checks if at least one parammeter is changed - newfunction(:docker_params_changed, type: :rvalue) do |args| - opts = args[0] || {} - return_value = [] + def remove_cidfile(cidfile, osfamily) + delete_command = if osfamily == 'windows' + run_with_powershell("del #{cidfile}") + else + "rm -f #{cidfile}" + end + _stdout, _stderr, _status = Open3.capture3(delete_command) + end + + def start_container(name, osfamily) + start_command = if osfamily == 'windows' + run_with_powershell("docker start #{name}") + else + "docker start #{name}" + end + _stdout, _stderr, _status = Open3.capture3(start_command) + end + + def stop_container(name, osfamily) + stop_command = if osfamily == 'windows' + run_with_powershell("docker stop #{name}") + else + "docker stop #{name}" + end + _stdout, _stderr, _status = Open3.capture3(stop_command) + end + + def remove_container(name, osfamily, stop_wait_time, cidfile) + stop_command = if osfamily == 'windows' + run_with_powershell("docker stop --time=#{stop_wait_time} #{name}") + else + "docker stop --time=#{stop_wait_time} #{name}" + end + _stdout, _stderr, _status = Open3.capture3(stop_command) + + remove_command = if osfamily == 'windows' + run_with_powershell("docker rm -v #{name}") + else + "docker rm -v #{name}" + end + _stdout, _stderr, _status = Open3.capture3(remove_command) + + remove_cidfile(cidfile, osfamily) + end + + def create_container(cmd, osfamily, image) + pull_command = if osfamily == 'windows' + run_with_powershell("docker pull #{image} -q") + else + "docker pull #{image} -q" + end + _stdout, _stderr, _status = Open3.capture3(pull_command) + + create_command = if osfamily == 'windows' + run_with_powershell(cmd) + else + cmd + end + _stdout, _stderr, _status = Open3.capture3(create_command) + end + + def detect_changes(opts) + require 'open3' + require 'json' + return_value = 'No changes detected' if opts['sanitised_title'] && opts['osfamily'] stdout, stderr, status = Open3.capture3("docker inspect #{opts['sanitised_title']}") @@ -61,18 +128,30 @@ module Puppet::Parser::Functions param_changed = true if pp_ports && pp_ports != ports - return_value << if param_changed - 'PARAM_CHANGED' - else - 'NO_CHANGE' - end + if param_changed + remove_container(opts['sanitised_title'], opts['osfamily'], opts['stop_wait_time'], opts['cidfile']) + create_container(opts['command'], opts['osfamily'], opts['image']) + return_value = 'Param changed' + end else - return_value << 'CONTAINER_NOT_FOUND' + create_container(opts['command'], opts['osfamily'], opts['image']) unless File.exist?(opts['cidfile']) + _stdout, _stderr, status = Open3.capture3("docker inspect #{opts['sanitised_title']}") + unless status.to_s.include?('exit 0') + remove_cidfile(opts['cidfile'], opts['osfamily']) + create_container(opts['command'], opts['osfamily'], opts['image']) + end + return_value = 'No changes detected' end else - return_value << 'ARG_REQUIRED_MISSING' + return_value = 'Arg required missing' + end + + if opts['container_running'] + start_container(opts['sanitised_title'], opts['osfamily']) + else + stop_container(opts['sanitised_title'], opts['osfamily']) end - return_value.flatten.join(' ') + return_value end end diff --git a/manifests/run.pp b/manifests/run.pp index e9599931..147cac87 100644 --- a/manifests/run.pp +++ b/manifests/run.pp @@ -414,93 +414,61 @@ $inspect = [ "${docker_command} inspect ${sanitised_title}", ] if $custom_unless { - $exec_unless = $custom_unless + $exec_unless = concat($custom_unless, $inspect) } else { $exec_unless = $inspect } - $detect_changes = docker_params_changed( - { - sanitised_title => $sanitised_title, - osfamily => $facts['os']['family'], - image => $image, - volumes => $volumes, - ports => $ports, + if versioncmp($facts['puppetversion'], '6') < 0 { + exec { "run ${title} with docker": + command => join($run_with_docker_command, ' '), + unless => $exec_unless, + environment => $exec_environment, + path => $exec_path, + provider => $exec_provider, + timeout => $exec_timeout, } - ) - case $detect_changes { - 'CONTAINER_NOT_FOUND': { - exec { "run ${title} with docker": - command => join($run_with_docker_command, ' '), - unless => $exec_unless, - environment => $exec_environment, - path => $exec_path, - provider => $exec_provider, - timeout => $exec_timeout, - } - } - 'PARAM_CHANGED': { + if $running == false { exec { "stop ${title} with docker": command => "${docker_command} stop --time=${stop_wait_time} ${sanitised_title}", - onlyif => "${docker_command} inspect ${sanitised_title}", + onlyif => $container_running_check, environment => $exec_environment, path => $exec_path, provider => $exec_provider, timeout => $exec_timeout, } - - exec { "remove ${title} with docker": - command => "${docker_command} rm -v ${sanitised_title}", - onlyif => "${docker_command} inspect ${sanitised_title}", - environment => $exec_environment, - path => $exec_path, - provider => $exec_provider, - timeout => $exec_timeout, - } - - file { $cidfile: - ensure => absent, - } - - exec { "run ${title} with docker": - command => join($run_with_docker_command, ' '), - unless => $exec_unless, + } else { + exec { "start ${title} with docker": + command => "${docker_command} start ${sanitised_title}", + unless => $container_running_check, environment => $exec_environment, path => $exec_path, provider => $exec_provider, timeout => $exec_timeout, } } - 'ARG_REQUIRED_MISSING': { - fail(translate('sanitised_title and osfamily are required for docker_params_changed function')) - } - default: { - # Use ONLY on debugging - DO NOT UNCOMMENT OTHERWISE - # notify { 'this case should not be executed': - # message => "detect_changes: ${detect_changes}", - # withpath => true, - # } + } else { + $docker_params_changed_args = { + sanitised_title => $sanitised_title, + osfamily => $facts['os']['family'], + command => join($run_with_docker_command, ' '), + cidfile => $cidfile, + image => $image, + volumes => $volumes, + ports => $ports, + stop_wait_time => $stop_wait_time, + container_running => $running, + # logfile_path => ($facts['os']['family'] == 'windows') ? { + # true => ::docker_user_temp_path, + # default => '/tmp', + # }, } - } - if $running == false { - exec { "stop ${title} with docker": - command => "${docker_command} stop --time=${stop_wait_time} ${sanitised_title}", - onlyif => $container_running_check, - environment => $exec_environment, - path => $exec_path, - provider => $exec_provider, - timeout => $exec_timeout, - } - } else { - exec { "start ${title} with docker": - command => "${docker_command} start ${sanitised_title}", - unless => $container_running_check, - environment => $exec_environment, - path => $exec_path, - provider => $exec_provider, - timeout => $exec_timeout, + $detect_changes = Deferred('docker_params_changed', [$docker_params_changed_args]) + + notify { 'docker_params_changed': + message => $detect_changes, } } } diff --git a/spec/acceptance/docker_params_changed_spec.rb b/spec/acceptance/docker_params_changed_spec.rb index 622a7029..664732a0 100644 --- a/spec/acceptance/docker_params_changed_spec.rb +++ b/spec/acceptance/docker_params_changed_spec.rb @@ -21,7 +21,7 @@ docker_image = 'hello-world:linux' end -describe 'docker trigger parammeters change' do +describe 'docker trigger parameters change', if: fetch_puppet_version > 5 do before(:all) do if os[:family] != 'windows' install_pp = "class { 'docker': #{docker_args}}" @@ -56,7 +56,7 @@ class {'docker': #{docker_args}} end it 'creates servercore with first image' do - idempotent_apply(pp1) + expect(docker_run_idempotent_apply(pp1)).to be true end it 'detect image change and apply the change' do @@ -91,7 +91,7 @@ class {'docker': #{docker_args}} end it "creates servercore with #{volumes1}" do - idempotent_apply(pp1) + expect(docker_run_idempotent_apply(pp1)).to be true end it "creates servercore with #{volumes2}" do @@ -123,7 +123,7 @@ class {'docker': #{docker_args}} end it 'creates servercore with ports => ["4444"]' do - idempotent_apply(pp1) + expect(docker_run_idempotent_apply(pp1)).to be true end it 'creates servercore with ports => ["4444", "4445"]' do diff --git a/spec/acceptance/docker_spec.rb b/spec/acceptance/docker_spec.rb index cbcbd94b..ba0e525c 100644 --- a/spec/acceptance/docker_spec.rb +++ b/spec/acceptance/docker_spec.rb @@ -40,6 +40,11 @@ service_name = 'docker' command = 'docker' + before(:all) do + install_pp = "class { 'docker': #{docker_args}}" + apply_manifest(install_pp) + end + context 'When adding system user', win_broken: broken do let(:pp) do " @@ -150,7 +155,11 @@ class { 'docker': end it 'is idempotent' do - apply_manifest(pp, catch_changes: true) + if fetch_puppet_version > 5 + expect(docker_run_idempotent_apply(pp)).to be true + else + apply_manifest(pp, catch_changes: true) + end end describe package(package_name) do diff --git a/spec/defines/run_spec.rb b/spec/defines/run_spec.rb index 81fa35c4..62c7d9ef 100644 --- a/spec/defines/run_spec.rb +++ b/spec/defines/run_spec.rb @@ -206,6 +206,7 @@ ## if %r{windows}.match?(os) facts = windows_facts.merge(os_facts) + facts = facts.merge({ puppetversion: Puppet.version }) os_params = { 'restart' => 'no', @@ -216,7 +217,7 @@ 'docker_ee' => true, } else - facts = {}.merge(os_facts) + facts = { puppetversion: Puppet.version }.merge(os_facts) os_params = {} diff --git a/spec/helper/get_docker_params_changed.rb b/spec/helper/get_docker_params_changed.rb new file mode 100644 index 00000000..7df24db1 --- /dev/null +++ b/spec/helper/get_docker_params_changed.rb @@ -0,0 +1,150 @@ +# frozen_string_literal: true + +def get_docker_params_changed(opts) + require 'open3' + require 'json' + return_value = 'No changes detected' + + if opts['sanitised_title'] && opts['osfamily'] + stdout, stderr, status = Open3.capture3("docker inspect #{opts['sanitised_title']}") + if stderr.to_s == '' && status.to_s.include?('exit 0') + param_changed = false + inspect_hash = JSON.parse(stdout)[0] + + # check if the image was changed + param_changed = true if opts['image'] && opts['image'] != inspect_hash['Config']['Image'] + + # check if something on volumes or mounts was changed(a new volume/mount was added or removed) + param_changed = true if opts['volumes'].is_a?(String) && opts['volumes'].include?(':') && opts['volumes'] != inspect_hash['Mounts'].to_a[0] && opts['osfamily'] != 'windows' + param_changed = true if opts['volumes'].is_a?(String) && !opts['volumes'].include?(':') && opts['volumes'] != inspect_hash['Config']['Volumes'].to_a[0] && opts['osfamily'] != 'windows' + 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' + 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' + true + else + param_changed + end + + pp_paths = opts['volumes'].reject { |item| item.include?(':') } if opts['volumes'].is_a?(Array) && opts['osfamily'] != 'windows' + pp_mounts = opts['volumes'].select { |item| item.include?(':') } if opts['volumes'].is_a?(Array) && opts['osfamily'] != 'windows' + pp_paths = opts['volumes'].select { |item| item.scan(%r{(?=:)}).count == 1 } if opts['volumes'].is_a?(Array) && opts['osfamily'] == 'windows' + pp_mounts = opts['volumes'].select { |item| item.scan(%r{(?=:)}).count == 2 } if opts['volumes'].is_a?(Array) && opts['osfamily'] == 'windows' + + inspect_paths = if inspect_hash['Config']['Volumes'] + inspect_hash['Config']['Volumes'].keys + else + [] + end + param_changed = true if pp_paths != inspect_paths + + names = inspect_hash['Mounts'].map { |item| item.values[1] } if inspect_hash['Mounts'] + pp_names = pp_mounts.map { |item| item.split(':')[0] } if pp_mounts + names = names.select { |item| pp_names.include?(item) } if names && pp_names + destinations = inspect_hash['Mounts'].map { |item| item.values[3] } if inspect_hash['Mounts'] + pp_destinations = pp_mounts.map { |item| item.split(':')[1] } if pp_mounts && opts['osfamily'] != 'windows' + pp_destinations = pp_mounts.map { |item| "#{item.split(':')[1].downcase}:#{item.split(':')[2]}" } if pp_mounts && opts['osfamily'] == 'windows' + destinations = destinations.select { |item| pp_destinations.include?(item) } if destinations && pp_destinations + + param_changed = true if pp_names != names + param_changed = true if pp_destinations != destinations + param_changed = true if pp_mounts != [] && inspect_hash['Mounts'].nil? + + # check if something on ports was changed(some ports were added or removed) + + ports = inspect_hash['HostConfig']['PortBindings'].keys + ports = ports.map { |item| item.split('/')[0] } + pp_ports = opts['ports'].sort if opts['ports'].is_a?(Array) + pp_ports = [opts['ports']] if opts['ports'].is_a?(String) + + param_changed = true if pp_ports && pp_ports != ports + + if param_changed + remove_container(opts['sanitised_title'], opts['osfamily'], opts['stop_wait_time'], opts['cidfile'], log) + create_container(opts['command'], opts['osfamily'], opts['image'], log) + return_value = 'Param changed' + end + else + create_container(opts['command'], opts['osfamily'], opts['image']) unless File.exist?(opts['cidfile']) + _stdout, _stderr, status = Open3.capture3("docker inspect #{opts['sanitised_title']}") + unless status.to_s.include?('exit 0') + remove_cidfile(opts['cidfile'], opts['osfamily']) + create_container(opts['command'], opts['osfamily'], opts['image']) + end + return_value = 'No changes detected' + end + else + return_value = 'Arg required missing' + end + + if opts['container_running'] + start_container(opts['sanitised_title'], opts['osfamily']) + else + stop_container(opts['sanitised_title'], opts['osfamily']) + end + + return_value +end + +def run_with_powershell(cmd) + "powershell.exe -Command \"& {#{cmd}}\" " +end + +def remove_cidfile(cidfile, osfamily) + delete_command = if osfamily == 'windows' + run_with_powershell("del #{cidfile}") + else + "rm -f #{cidfile}" + end + _stdout, _stderr, _status = Open3.capture3(delete_command) +end + +def start_container(name, osfamily) + start_command = if osfamily == 'windows' + run_with_powershell("docker start #{name}") + else + "docker start #{name}" + end + _stdout, _stderr, _status = Open3.capture3(start_command) +end + +def stop_container(name, osfamily) + stop_command = if osfamily == 'windows' + run_with_powershell("docker stop #{name}") + else + "docker stop #{name}" + end + _stdout, _stderr, _status = Open3.capture3(stop_command) +end + +def remove_container(name, osfamily, stop_wait_time, cidfile) + stop_command = if osfamily == 'windows' + run_with_powershell("docker stop --time=#{stop_wait_time} #{name}") + else + "docker stop --time=#{stop_wait_time} #{name}" + end + _stdout, _stderr, _status = Open3.capture3(stop_command) + + remove_command = if osfamily == 'windows' + run_with_powershell("docker rm -v #{name}") + else + "docker rm -v #{name}" + end + _stdout, _stderr, _status = Open3.capture3(remove_command) + + remove_cidfile(cidfile, osfamily) +end + +def create_container(cmd, osfamily, image) + pull_command = if osfamily == 'windows' + run_with_powershell("docker pull #{image} -q") + else + "docker pull #{image} -q" + end + _stdout, _stderr, _status = Open3.capture3(pull_command) + + create_command = if osfamily == 'windows' + run_with_powershell(cmd) + else + cmd + end + _stdout, _stderr, _status = Open3.capture3(create_command) +end diff --git a/spec/shared_examples.rb b/spec/shared_examples.rb index 44e5143e..6a8998b6 100644 --- a/spec/shared_examples.rb +++ b/spec/shared_examples.rb @@ -11,6 +11,7 @@ require 'helper/get_docker_stack_flags' require 'helper/get_docker_swarm_init_flags' require 'helper/get_docker_swarm_join_flags' +require 'helper/get_docker_params_changed' require 'helper/get_values_init' require 'helper/pw_hash' require 'helper/windows_facts' diff --git a/spec/shared_examples/run.rb b/spec/shared_examples/run.rb index 530f3c61..26d27018 100644 --- a/spec/shared_examples/run.rb +++ b/spec/shared_examples/run.rb @@ -190,41 +190,64 @@ # inspect # end - it { - is_expected.to contain_exec("run #{title} with docker").with( - 'command' => run_with_docker_command.join(' '), - ## todo: - ## fix the following strange behavior: - ## expected that the catalogue would contain Exec[run command with docker] with unless set to [["docker inspect command"]] - ## but it is set to [["docker inspect command"], "docker inspect command"] - # 'unless' => exec_unless, - 'environment' => exec_environment, - 'path' => exec_path, - 'provider' => exec_provider, - 'timeout' => exec_timeout, - ) - } - - if !running + if facts[:puppetversion].to_i < 6 it { - is_expected.to contain_exec("stop #{title} with docker").with( - 'command' => "#{docker_command} stop --time=#{stop_wait_time} #{sanitised_title}", - 'onlyif' => container_running_check, + is_expected.to contain_exec("run #{title} with docker").with( + 'command' => run_with_docker_command.join(' '), + ## todo: + ## fix the following strange behavior: + ## expected that the catalogue would contain Exec[run command with docker] with unless set to [["docker inspect command"]] + ## but it is set to [["docker inspect command"], "docker inspect command"] + # 'unless' => exec_unless, 'environment' => exec_environment, 'path' => exec_path, 'provider' => exec_provider, 'timeout' => exec_timeout, ) } + + if !running + it { + is_expected.to contain_exec("stop #{title} with docker").with( + 'command' => "#{docker_command} stop --time=#{stop_wait_time} #{sanitised_title}", + 'onlyif' => container_running_check, + 'environment' => exec_environment, + 'path' => exec_path, + 'provider' => exec_provider, + 'timeout' => exec_timeout, + ) + } + else + it { + is_expected.to contain_exec("start #{title} with docker").with( + 'command' => "#{docker_command} start #{sanitised_title}", + 'unless' => container_running_check, + 'environment' => exec_environment, + 'path' => exec_path, + 'provider' => exec_provider, + 'timeout' => exec_timeout, + ) + } + end else + docker_params_changed_args = { + 'sanitised_title' => sanitised_title, + 'osfamily' => facts[:os]['family'], + 'command' => run_with_docker_command.join(' '), + 'cidfile' => cidfile, + 'image' => image, + 'volumes' => volumes, + 'ports' => ports, + 'stop_wait_time' => stop_wait_time, + 'container_running' => running, + 'logfile_path' => facts[:os]['family'] == 'windows' ? facts['docker_user_temp_path'] : '/tmp', + } + + detect_changes = get_docker_params_changed(docker_params_changed_args) + it { - is_expected.to contain_exec("start #{title} with docker").with( - 'command' => "#{docker_command} start #{sanitised_title}", - 'unless' => container_running_check, - 'environment' => exec_environment, - 'path' => exec_path, - 'provider' => exec_provider, - 'timeout' => exec_timeout, + is_expected.to contain_notify('docker_params_changed').with( + 'message' => detect_changes, ) } end diff --git a/spec/spec_helper_acceptance_local.rb b/spec/spec_helper_acceptance_local.rb index b492e65e..7bb8d910 100644 --- a/spec/spec_helper_acceptance_local.rb +++ b/spec/spec_helper_acceptance_local.rb @@ -36,6 +36,15 @@ def create_remote_file(name, full_name, file_content) end end +def docker_run_idempotent_apply(pp) + apply_manifest(pp) + apply_manifest(pp).stdout.include?('Notice: No changes detected') +end + +def fetch_puppet_version + @fetch_puppet_version ||= run_shell('puppet --version').stdout.to_i +end + RSpec.configure do |c| # Add exclusive filter for Windows untill all the windows functionality is implemented c.filter_run_excluding win_broken: true