Merge pull request #187 from dhollinger/MODULES-4753

The exec resources running swapon and swapoff in the lvm::logical_volume
define type were running whenever a swap logical volume was created
or updated.

The logical_volume provider was also running these same commands on a swap
resize. This caused a conflict where the puppet run would error out
when the logical_volume provider attempted to run swapoff on a swap
lvm that had already been unloaded from swap and the lvm would never
be reloaded into swap.

When the exec resources were removed from the lvm::logical_volume
defined type, it created a gap wherein a NEW swap lvm would not be
loaded into swap after being created.

To solve this issue, I added the swapon command and logic that will
run that command against the filesystem resource if the fs_type parameter
is set to 'swap'

Author: hunner

.gitignore

@@ -2,3 +2,5 @@
 .*.swp
 Gemfile.lock
 .bundle/
+.fixtures/modules
+.fixtures/manifests

lib/puppet/provider/filesystem/lvm.rb

@@ -4,7 +4,7 @@
     commands :blkid => 'blkid'
 
     def create
-        mkfs(@resource[:fs_type])
+        mkfs(@resource[:fs_type], @resource[:name])
     end
 
     def exists?
@@ -21,7 +21,7 @@ def fstype
         nil
     end
 
-    def mkfs(fs_type)
+    def mkfs(fs_type, name)
         mkfs_params = { "reiserfs" => "-q" }
 
         mkfs_cmd = @resource[:mkfs_cmd] != nil ?
@@ -33,7 +33,7 @@ def mkfs(fs_type)
                      ["mkfs.#{fs_type}"]
                    end
 
-        mkfs_cmd << @resource[:name]
+        mkfs_cmd << name
 
         if mkfs_params[fs_type]
             mkfs_cmd << mkfs_params[fs_type]
@@ -45,6 +45,11 @@ def mkfs(fs_type)
         end
 
         execute mkfs_cmd
+        if fs_type == 'swap'
+            swap_cmd = ["swapon"]
+            swap_cmd << name
+            execute swap_cmd
+        end
     end
 
 end

lib/puppet/provider/logical_volume/lvm.rb

@@ -129,6 +129,9 @@ def create
 
     def destroy
         name_escaped = "#{@resource[:volume_group].gsub('-','--')}-#{@resource[:name].gsub('-','--')}"
+        if blkid(path) =~ /\bTYPE=\"(swap)\"/
+            swapoff(path)
+        end
         dmsetup('remove', name_escaped)
         lvremove('-f', path)
     end

manifests/logical_volume.pp

@@ -62,13 +62,13 @@
   }
 
   if $ensure == 'present' and $createfs {
-    Logical_volume[$name] ->
-    Filesystem[$lvm_device_path] ->
-    Mount[$mount_title]
-  } elsif $ensure != 'present' and $createfs {
-    Mount[$mount_title] ->
-    Filesystem[$lvm_device_path] ->
     Logical_volume[$name]
+    -> Filesystem[$lvm_device_path]
+    -> Mount[$mount_title]
+  } elsif $ensure != 'present' and $createfs {
+    Mount[$mount_title]
+    -> Filesystem[$lvm_device_path]
+    -> Logical_volume[$name]
   }
 
   logical_volume { $name:
@@ -89,7 +89,7 @@
     mirrorlog        => $mirrorlog,
     no_sync          => $no_sync,
     region_size      => $region_size,
-    alloc            => $alloc
+    alloc            => $alloc,
   }
 
   if $createfs {
@@ -101,30 +101,15 @@
   }
 
   if $createfs or $ensure != 'present' {
-    if $fs_type == 'swap' {
-      if $ensure == 'present' {
-        exec { "swapon for '${mount_title}'":
-          path      => [ '/bin', '/usr/bin', '/sbin' ],
-          command   => "swapon ${lvm_device_path}",
-          unless    => "grep `readlink -f ${lvm_device_path}` /proc/swaps",
-          subscribe => Mount[$mount_title],
-        }
-      } else {
-        exec { "swapoff for '${mount_title}'":
-          path    => [ '/bin', '/usr/bin', '/sbin' ],
-          command => "swapoff ${lvm_device_path}",
-          onlyif  => "grep `readlink -f ${lvm_device_path}` /proc/swaps",
-          notify  => Mount[$mount_title],
-        }
-      }
-    } else {
+    if $fs_type != 'swap' {
       exec { "ensure mountpoint '${fixed_mountpath}' exists":
         path    => [ '/bin', '/usr/bin' ],
         command => "mkdir -p ${fixed_mountpath}",
         unless  => "test -d ${fixed_mountpath}",
         before  => Mount[$mount_title],
       }
     }
+
     mount { $mount_title:
       ensure  => $mount_ensure,
       name    => $fixed_mountpath,

spec/unit/classes/lvm_spec.rb

@@ -111,10 +111,5 @@
       :pass   => 0,
       :dump   => 0
     }) }
-    it { should contain_exec("swapon for '/dev/myvg/swap'") }
-    it { should_not contain_exec("ensure mountpoint 'swap_/dev/myvg/swap' exists") }
-
-    it { should contain_exec("swapoff for '/dev/myvg/swap2'") }
-    it { should_not contain_exec("ensure mountpoint 'swap_/dev/myvg/swap2' exists") }
   end
 end

spec/unit/puppet/provider/filesystem/lvm_spec.rb

@@ -39,6 +39,7 @@
       @resource.expects(:[]).with(:options)
       @provider.expects(:execute).with(['mkswap', '/dev/myvg/mylv'])
       @resource.expects(:[]).with(:mkfs_cmd)
+      @provider.expects(:execute).with(['swapon', '/dev/myvg/mylv'])
       @provider.create
     end
     it "should create an ext4 journal correctly" do

spec/unit/puppet/provider/logical_volume/lvm_spec.rb

@@ -228,18 +228,29 @@
 
   describe 'when destroying' do
     it "should execute 'dmsetup' and 'lvremove'" do
-      @resource.expects(:[]).with(:volume_group).returns('myvg').twice
-      @resource.expects(:[]).with(:name).returns('mylv').twice
+      @resource.expects(:[]).with(:volume_group).returns('myvg').times(3)
+      @resource.expects(:[]).with(:name).returns('mylv').times(3)
+      @provider.expects(:blkid).with('/dev/myvg/mylv')
       @provider.expects(:dmsetup).with('remove', 'myvg-mylv')
       @provider.expects(:lvremove).with('-f', '/dev/myvg/mylv')
       @provider.destroy
     end
     it "should execute 'dmsetup' and 'lvremove' and properly escape names with dashes" do
-      @resource.expects(:[]).with(:volume_group).returns('my-vg').twice
-      @resource.expects(:[]).with(:name).returns('my-lv').twice
+      @resource.expects(:[]).with(:volume_group).returns('my-vg').times(3)
+      @resource.expects(:[]).with(:name).returns('my-lv').times(3)
+      @provider.expects(:blkid).with('/dev/my-vg/my-lv')
       @provider.expects(:dmsetup).with('remove', 'my--vg-my--lv')
       @provider.expects(:lvremove).with('-f', '/dev/my-vg/my-lv')
       @provider.destroy
     end
+    it "should execute 'swapoff', 'dmsetup', and 'lvremove' when lvm is of type swap" do
+      @resource.expects(:[]).with(:volume_group).returns('myvg').times(4)
+      @resource.expects(:[]).with(:name).returns('mylv').times(4)
+      @provider.expects(:blkid).with('/dev/myvg/mylv').returns('TYPE="swap"')
+      @provider.expects(:swapoff).with('/dev/myvg/mylv')
+      @provider.expects(:dmsetup).with('remove', 'myvg-mylv')
+      @provider.expects(:lvremove).with('-f', '/dev/myvg/mylv')
+      @provider.destroy
+    end
   end
 end