From f9bda67d9c8e6fc6c65640c4a2ec48f67fb8a08b Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Tue, 23 May 2017 13:35:38 -0500 Subject: [PATCH 1/6] MODULES-4753 Remove redundant swap exec resources 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. --- manifests/logical_volume.pp | 29 ++++++----------------------- spec/unit/classes/lvm_spec.rb | 5 ----- 2 files changed, 6 insertions(+), 28 deletions(-) diff --git a/manifests/logical_volume.pp b/manifests/logical_volume.pp index caf1a4cc..e69aaeb9 100644 --- a/manifests/logical_volume.pp +++ b/manifests/logical_volume.pp @@ -101,30 +101,13 @@ } 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 { - exec { "ensure mountpoint '${fixed_mountpath}' exists": - path => [ '/bin', '/usr/bin' ], - command => "mkdir -p ${fixed_mountpath}", - unless => "test -d ${fixed_mountpath}", - before => Mount[$mount_title], - } + 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, diff --git a/spec/unit/classes/lvm_spec.rb b/spec/unit/classes/lvm_spec.rb index 6dd9231e..c091c109 100644 --- a/spec/unit/classes/lvm_spec.rb +++ b/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 From 1ed696b545687c7099c20a3d4a599cdf13200141 Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Tue, 23 May 2017 14:26:53 -0500 Subject: [PATCH 2/6] MODULES-4753 Add swapon command to filesystem provider 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' --- lib/puppet/provider/filesystem/lvm.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/puppet/provider/filesystem/lvm.rb b/lib/puppet/provider/filesystem/lvm.rb index f1cef6aa..b427b398 100644 --- a/lib/puppet/provider/filesystem/lvm.rb +++ b/lib/puppet/provider/filesystem/lvm.rb @@ -1,10 +1,10 @@ Puppet::Type.type(:filesystem).provide :lvm do desc "Manages filesystem of a logical volume" - commands :blkid => 'blkid' + commands :blkid => 'blkid', :swapon => 'swapon' 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,9 @@ def mkfs(fs_type) end execute mkfs_cmd + if fs_type == 'swap' + swapon(name) + end end end From 29f1d6472480a9b28aa08f55cff141cd5a5c6146 Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Tue, 23 May 2017 15:09:25 -0500 Subject: [PATCH 3/6] MODULES-4753 Puppet-lint cleanup --- manifests/logical_volume.pp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/manifests/logical_volume.pp b/manifests/logical_volume.pp index e69aaeb9..4a2f0a8f 100644 --- a/manifests/logical_volume.pp +++ b/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 { From cda5e842c00d0760ffdd512a12ea4b464ed96fb6 Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Tue, 23 May 2017 15:23:20 -0500 Subject: [PATCH 4/6] MODULES-4753 Update provider and tests Update the provider changes to be more inline with how the tests are designed. Tests have been added. Gitignore updated to contain the .fixtures/modules and .fixtures/manifests folders. --- .gitignore | 2 ++ lib/puppet/provider/filesystem/lvm.rb | 6 ++++-- spec/unit/puppet/provider/filesystem/lvm_spec.rb | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index b22340eb..c2afac10 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,5 @@ .*.swp Gemfile.lock .bundle/ +.fixtures/modules +.fixtures/manifests \ No newline at end of file diff --git a/lib/puppet/provider/filesystem/lvm.rb b/lib/puppet/provider/filesystem/lvm.rb index b427b398..21cb6c02 100644 --- a/lib/puppet/provider/filesystem/lvm.rb +++ b/lib/puppet/provider/filesystem/lvm.rb @@ -1,7 +1,7 @@ Puppet::Type.type(:filesystem).provide :lvm do desc "Manages filesystem of a logical volume" - commands :blkid => 'blkid', :swapon => 'swapon' + commands :blkid => 'blkid' def create mkfs(@resource[:fs_type], @resource[:name]) @@ -46,7 +46,9 @@ def mkfs(fs_type, name) execute mkfs_cmd if fs_type == 'swap' - swapon(name) + swap_cmd = ["swapon"] + swap_cmd << name + execute swap_cmd end end diff --git a/spec/unit/puppet/provider/filesystem/lvm_spec.rb b/spec/unit/puppet/provider/filesystem/lvm_spec.rb index d8c85b8a..c0d248e1 100644 --- a/spec/unit/puppet/provider/filesystem/lvm_spec.rb +++ b/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 From 536229e6b5be11eefe54810bc13ea1f83b712303 Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Tue, 23 May 2017 16:48:53 -0500 Subject: [PATCH 5/6] MODULES-4753 Allow removal of swap LVMs Added logic to the destroy method that will check blkid for the LVM's type and run swapoff against it if it is of TYPE swap. This allows swap LVMs to be removed rather than simply erroring out. --- lib/puppet/provider/logical_volume/lvm.rb | 3 +++ .../provider/logical_volume/lvm_spec.rb | 19 +++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/lib/puppet/provider/logical_volume/lvm.rb b/lib/puppet/provider/logical_volume/lvm.rb index 749a65b6..777c03a5 100644 --- a/lib/puppet/provider/logical_volume/lvm.rb +++ b/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 diff --git a/spec/unit/puppet/provider/logical_volume/lvm_spec.rb b/spec/unit/puppet/provider/logical_volume/lvm_spec.rb index 2dfdfa65..ac6f9ae5 100644 --- a/spec/unit/puppet/provider/logical_volume/lvm_spec.rb +++ b/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 From a79c3448b9d4dbc412f5fe55dfa027bad8118331 Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Thu, 25 May 2017 12:20:25 -0500 Subject: [PATCH 6/6] MODULES-4753 Add logic preventing a mount on fs_type swap Added logic to prevent a mount on fs_type swap in the logical_volume.pp Update unit test for the logical_volume define. --- manifests/logical_volume.pp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/manifests/logical_volume.pp b/manifests/logical_volume.pp index 4a2f0a8f..8ece160e 100644 --- a/manifests/logical_volume.pp +++ b/manifests/logical_volume.pp @@ -101,11 +101,13 @@ } if $createfs or $ensure != 'present' { - exec { "ensure mountpoint '${fixed_mountpath}' exists": - path => [ '/bin', '/usr/bin' ], - command => "mkdir -p ${fixed_mountpath}", - unless => "test -d ${fixed_mountpath}", - before => Mount[$mount_title], + 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: