]> review.fuel-infra Code Review - puppet-modules/puppetlabs-firewall.git/commitdiff
(MODULES-1341) Recover when deleting absent rules
authorReid Vandewiele <reid@puppetlabs.com>
Wed, 4 Nov 2015 18:40:20 +0000 (10:40 -0800)
committerReid Vandewiele <reid@puppetlabs.com>
Wed, 4 Nov 2015 18:40:20 +0000 (10:40 -0800)
Some types, specifically the resources type, will call Firewall
instances and then use generate to build and add to the catalog firewall
resources very early in a Puppet run. Later, those resources might be
removed as a side effect of another action, such as shutting down the
firewalld service.

Prior to this commit, Puppet would try to delete firewall resources
which were already absent, and throw an error. This commit adds an
exception catcher which will check to see if the rule being removed is
absent, and if so, consider the change a success even if the firewall
command failed. It will adjust the change message to reflect the
uncertainty over how the rule was removed, though it was verified
removed.

lib/puppet/provider/firewall/iptables.rb

index 27c0b362e04997b5b7c461ada810ab09e4d2989d..e6c11e5353f9481b2722c4da714aab25c0cd7617 100644 (file)
@@ -271,7 +271,22 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
 
   def delete
     debug 'Deleting rule %s' % resource[:name]
-    iptables delete_args
+    begin
+      iptables delete_args
+    rescue Puppet::ExecutionFailure => e
+      # Check to see if the iptables rule is already gone. This can sometimes
+      # happen as a side effect of other resource changes. If it's not gone,
+      # raise the error as per usual.
+      raise e unless self.resource.property(:ensure).insync?(:absent)
+
+      # If it's already gone, there is no error. Still record a change, but
+      # adjust the change message to indicate ambiguity over what work Puppet
+      # actually did to remove the resource, vs. what could have been a side
+      # effect of something else puppet did.
+      resource.property(:ensure).singleton_class.send(:define_method, :change_to_s) do |a,b|
+        "ensured absent"
+      end
+    end
   end
 
   def exists?