diff options
| -rw-r--r-- | CHANGELOG.md | 2 | ||||
| -rw-r--r-- | lib/chef/provider/ifconfig.rb | 31 | ||||
| -rw-r--r-- | spec/unit/provider/ifconfig/debian_spec.rb | 28 | ||||
| -rw-r--r-- | spec/unit/provider/ifconfig/redhat_spec.rb | 30 | ||||
| -rw-r--r-- | spec/unit/provider/ifconfig_spec.rb | 6 |
5 files changed, 55 insertions, 42 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index daf680b047..0344d0da83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## Unreleased: 12.0.0 +* [**Kazuki Saito**](https://github.com/sakazuki): + CHEF-4933: idempotency fixes for ifconfig provider * [**Kirill Shirinkin**](https://github.com/Fodoj): The knife bootstrap command expands the path of the secret-file * [**Malte Swart**](https://github.com/mswart): diff --git a/lib/chef/provider/ifconfig.rb b/lib/chef/provider/ifconfig.rb index 31f88e5406..ac52100b56 100644 --- a/lib/chef/provider/ifconfig.rb +++ b/lib/chef/provider/ifconfig.rb @@ -19,6 +19,7 @@ require 'chef/log' require 'chef/mixin/command' require 'chef/provider' +require 'chef/resource/file' require 'chef/exceptions' require 'erb' @@ -109,11 +110,11 @@ class Chef :command => command ) Chef::Log.info("#{@new_resource} added") - # Write out the config files - generate_config end end end + # Write out the config files + generate_config end def action_enable @@ -140,12 +141,12 @@ class Chef run_command( :command => command ) - delete_config Chef::Log.info("#{@new_resource} deleted") end else Chef::Log.debug("#{@new_resource} does not exist - nothing to do") end + delete_config end def action_disable @@ -168,27 +169,25 @@ class Chef ! @config_template.nil? and ! @config_path.nil? end + def resource_for_config(path) + Chef::Resource::File.new(path, run_context) + end + def generate_config return unless can_generate_config? b = binding template = ::ERB.new(@config_template) - converge_by ("generate configuration file : #{@config_path}") do - network_file = ::File.new(@config_path, "w") - network_file.puts(template.result(b)) - network_file.close - end - Chef::Log.info("#{@new_resource} created configuration file") + config = resource_for_config(@config_path) + config.content(template.result(b)) + config.run_action(:create) + @new_resource.updated_by_last_action(true) if config.updated? end def delete_config return unless can_generate_config? - require 'fileutils' - if ::File.exist?(@config_path) - converge_by ("delete the #{@config_path}") do - FileUtils.rm_f(@config_path, :verbose => false) - end - end - Chef::Log.info("#{@new_resource} deleted configuration file") + config = resource_for_config(@config_path) + config.run_action(:delete) + @new_resource.updated_by_last_action(true) if config.updated? end private diff --git a/spec/unit/provider/ifconfig/debian_spec.rb b/spec/unit/provider/ifconfig/debian_spec.rb index c6a37fdd5b..f4fd1480e0 100644 --- a/spec/unit/provider/ifconfig/debian_spec.rb +++ b/spec/unit/provider/ifconfig/debian_spec.rb @@ -56,8 +56,6 @@ describe Chef::Provider::Ifconfig::Debian do describe "generate_config" do context "when writing a file" do - let(:config_file_ifcfg) { StringIO.new } - let(:tempfile) { Tempfile.new("rspec-chef-ifconfig-debian") } let(:tempdir_path) { Dir.mktmpdir("rspec-chef-ifconfig-debian-dir") } @@ -67,14 +65,13 @@ describe Chef::Provider::Ifconfig::Debian do before do stub_const("Chef::Provider::Ifconfig::Debian::INTERFACES_FILE", tempfile.path) stub_const("Chef::Provider::Ifconfig::Debian::INTERFACES_DOT_D_DIR", tempdir_path) - expect(File).to receive(:new).with(config_filename_ifcfg, "w").and_return(config_file_ifcfg) end it "should write a network-script" do provider.run_action(:add) - expect(config_file_ifcfg.string).to match(/^iface eth0 inet static\s*$/) - expect(config_file_ifcfg.string).to match(/^\s+address 10\.0\.0\.1\s*$/) - expect(config_file_ifcfg.string).to match(/^\s+netmask 255\.255\.254\.0\s*$/) + expect(File.read(config_filename_ifcfg)).to match(/^iface eth0 inet static\s*$/) + expect(File.read(config_filename_ifcfg)).to match(/^\s+address 10\.0\.0\.1\s*$/) + expect(File.read(config_filename_ifcfg)).to match(/^\s+netmask 255\.255\.254\.0\s*$/) end context "when the interface_dot_d directory does not exist" do @@ -123,7 +120,6 @@ iface eth0 inet static netmask 255.255.254.0 EOF ) - expect(File).to receive(:new).with(config_filename_ifcfg, "w").and_return(config_file_ifcfg) expect(File.exists?(tempdir_path)).to be_true # since the file exists, the enclosing dir must also exist end @@ -139,6 +135,8 @@ EOF before do tempfile.write(expected_string) tempfile.close + + expect(provider).not_to receive(:converge_by).with(/modifying #{tempfile.path} to source #{tempdir_path}/) end it "should preserve all the contents" do @@ -165,6 +163,9 @@ EOF before do tempfile.write("a line\nanother line\n") tempfile.close + + allow(provider).to receive(:converge_by).and_call_original + expect(provider).to receive(:converge_by).with(/modifying #{tempfile.path} to source #{tempdir_path}/).and_call_original end it "should preserve the original contents and add the source line" do @@ -318,8 +319,17 @@ source #{tempdir_path}/* it "should delete network-script if it exists" do current_resource.device new_resource.device - expect(File).to receive(:exist?).with(config_filename_ifcfg).and_return(true) - expect(FileUtils).to receive(:rm_f).with(config_filename_ifcfg, :verbose => false) + + [:exist?, :exists?, :writable?].each do |cmd| + # need to stub :writable? to make why_run? happy + allow(File).to receive(cmd).and_call_original + allow(File).to receive(cmd).with(config_filename_ifcfg).and_return(true) + end + + # stub for Chef::Util::Backup#do_backup + expect(FileUtils).to receive(:cp) + .with(config_filename_ifcfg, /#{Chef::Config[:file_backup_path]}/, :preserve => true) + expect(File).to receive(:delete).with(config_filename_ifcfg) provider.run_action(:delete) end diff --git a/spec/unit/provider/ifconfig/redhat_spec.rb b/spec/unit/provider/ifconfig/redhat_spec.rb index f4b98dfc32..138c2a389d 100644 --- a/spec/unit/provider/ifconfig/redhat_spec.rb +++ b/spec/unit/provider/ifconfig/redhat_spec.rb @@ -37,22 +37,26 @@ describe Chef::Provider::Ifconfig::Redhat do status = double("Status", :exitstatus => 0) @provider.instance_variable_set("@status", status) @provider.current_resource = @current_resource - end + + config_filename = "/etc/sysconfig/network-scripts/ifcfg-#{@new_resource.device}" + @config = double("chef-resource-file") + @provider.should_receive(:resource_for_config).with(config_filename).and_return(@config) + end describe "generate_config for action_add" do - it "should write network-script for centos" do + it "should write network-script for centos" do @provider.stub(:load_current_resource) @provider.stub(:run_command) - config_filename = "/etc/sysconfig/network-scripts/ifcfg-#{@new_resource.device}" - config_file = StringIO.new - File.should_receive(:new).with(config_filename, "w").and_return(config_file) - + @config.should_receive(:content) do |arg| + arg.should match(/^\s*DEVICE=eth0\s*$/) + arg.should match(/^\s*IPADDR=10\.0\.0\.1\s*$/) + arg.should match(/^\s*NETMASK=255\.255\.254\.0\s*$/) + end + @config.should_receive(:run_action).with(:create) + @config.should_receive(:updated?).and_return(true) @provider.run_action(:add) - config_file.string.should match(/^\s*DEVICE=eth0\s*$/) - config_file.string.should match(/^\s*IPADDR=10\.0\.0\.1\s*$/) - config_file.string.should match(/^\s*NETMASK=255\.255\.254\.0\s*$/) - end + end end describe "delete_config for action_delete" do @@ -61,10 +65,8 @@ describe Chef::Provider::Ifconfig::Redhat do @current_resource.device @new_resource.device @provider.stub(:load_current_resource) @provider.stub(:run_command) - config_filename = "/etc/sysconfig/network-scripts/ifcfg-#{@new_resource.device}" - File.should_receive(:exist?).with(config_filename).and_return(true) - FileUtils.should_receive(:rm_f).with(config_filename, :verbose => false) - + @config.should_receive(:run_action).with(:delete) + @config.should_receive(:updated?).and_return(true) @provider.run_action(:delete) end end diff --git a/spec/unit/provider/ifconfig_spec.rb b/spec/unit/provider/ifconfig_spec.rb index fb8d0956d7..b09e365c65 100644 --- a/spec/unit/provider/ifconfig_spec.rb +++ b/spec/unit/provider/ifconfig_spec.rb @@ -72,7 +72,7 @@ describe Chef::Provider::Ifconfig do @provider.stub(:load_current_resource) @provider.should_not_receive(:run_command) @current_resource.inet_addr "10.0.0.1" - @provider.should_not_receive(:generate_config) + @provider.should_receive(:generate_config) @provider.run_action(:add) @new_resource.should_not be_updated @@ -123,7 +123,7 @@ describe Chef::Provider::Ifconfig do it "should not delete interface if it does not exist" do @provider.stub(:load_current_resource) @provider.should_not_receive(:run_command) - @provider.should_not_receive(:delete_config) + @provider.should_receive(:delete_config) @provider.run_action(:delete) @new_resource.should_not be_updated @@ -171,7 +171,7 @@ describe Chef::Provider::Ifconfig do @provider.stub(:load_current_resource) # This is so that nothing actually runs @provider.should_not_receive(:run_command) - @provider.should_not_receive(:delete_config) + @provider.should_receive(:delete_config) @provider.run_action(:delete) @new_resource.should_not be_updated |