Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Test for and fix idempotency issues

* mounttab: atboot and options (with defaults/empty) fixed
* host, sysctl: comment property fixed
  • Loading branch information...
commit 897b35d2a5fcf23f898037f569e39c6034c8a3cd 1 parent b540657
Dominic Cleal authored
2  lib/puppet/provider/host/augeas.rb
@@ -175,7 +175,7 @@ def comment
175 175
     path = "/files#{self.class.file(resource)}"
176 176
     begin
177 177
       aug = self.class.augopen(resource)
178  
-      aug.get("#{path}/*[canonical = '#{resource[:name]}']/#comment")
  178
+      aug.get("#{path}/*[canonical = '#{resource[:name]}']/#comment") || ""
179 179
     ensure
180 180
       aug.close if aug
181 181
     end
33  lib/puppet/provider/mounttab/augeas.rb
@@ -159,7 +159,16 @@ def options
159 159
         opt = "#{opt}=#{optv}" if optv
160 160
         opts << opt
161 161
       end
162  
-      opts.join(",")
  162
+      opts = opts.join(",")
  163
+
  164
+      # [] and ["defaults"] are synonyms, so return what the user requested if
  165
+      # the current value is one of these to avoid changes
  166
+      empties = ["", "defaults"]
  167
+      if empties.include?(opts) and empties.include?(resource.should(:options))
  168
+        resource.should(:options)
  169
+      else
  170
+        opts
  171
+      end
163 172
     ensure
164 173
       aug.close if aug
165 174
     end
@@ -267,30 +276,10 @@ def pass=(value)
267 276
   end
268 277
 
269 278
   def atboot
270  
-    aug = nil
271  
-    path = "/files#{self.class.file(resource)}"
272  
-    begin
273  
-      aug = self.class.augopen(resource)
274  
-      aug.get("#{path}/*[file = '#{resource[:name]}']/atboot")
275  
-    ensure
276  
-      aug.close if aug
277  
-    end
  279
+    resource.should(:atboot)
278 280
   end
279 281
 
280 282
   def atboot=(value)
281 283
     return  # FIXME: not written
282  
-    aug = nil
283  
-    path = "/files#{self.class.file(resource)}"
284  
-    begin
285  
-      aug = self.class.augopen(resource)
286  
-      # Ensure dump is always set if passno is being set
287  
-      if aug.match("#{path}/*[file = '#{resource[:name]}']/dump").empty?
288  
-        aug.set("#{path}/*[file = '#{resource[:name]}']/dump", "0")
289  
-      end
290  
-      aug.set("#{path}/*[file = '#{resource[:name]}']/passno", value.to_s)
291  
-      aug.save!
292  
-    ensure
293  
-      aug.close if aug
294  
-    end
295 284
   end
296 285
 end
4  lib/puppet/provider/sysctl/augeas.rb
@@ -129,8 +129,8 @@ def comment
129 129
     begin
130 130
       aug = self.class.augopen(resource)
131 131
       comment = aug.get("#{path}/#comment[following-sibling::*[1][self::#{resource[:name]}]][. =~ regexp('#{resource[:name]}:.*')]")
132  
-      comment.sub(/^#{resource[:name]}:\s*/, "") if comment
133  
-      comment
  132
+      comment.sub!(/^#{resource[:name]}:\s*/, "") if comment
  133
+      comment || ""
134 134
     ensure
135 135
       aug.close if aug
136 136
     end
9  spec/lib/augeas_spec/fixtures.rb
@@ -29,6 +29,15 @@ def apply!(resource)
29 29
 
30 30
     # Check for transaction success after, as it's less informative
31 31
     txn.any_failed?.should_not be_true
  32
+
  33
+    # Run the exact same resource, but this time ensure there were absolutely
  34
+    # no changes (as seen by logs) to indicate if it was idempotent or not
  35
+    @logs.clear
  36
+    txn_idempotent = apply(resource)
  37
+    loglevels = Puppet::Util::Log.levels[2, 999]
  38
+    @logs.select { |log| loglevels.include? log.level }.should == []
  39
+    txn_idempotent.any_failed?.should_not be_true
  40
+
32 41
     txn
33 42
   end
34 43
 

0 notes on commit 897b35d

Please sign in to comment.
Something went wrong with that request. Please try again.