From 24f5595250cc0631f02d91da630ea54b7eac1f19 Mon Sep 17 00:00:00 2001 From: NAshwini Date: Mon, 20 Mar 2023 21:27:55 +0530 Subject: Updated unit spec according to remount_require? Signed-off-by: NAshwini --- lib/chef/provider/mount.rb | 5 +++++ lib/chef/provider/mount/linux.rb | 2 ++ lib/chef/provider/mount/mount.rb | 5 +++++ spec/unit/provider/mount_spec.rb | 44 +++++++++++++++++++++++++++++++++++++--- 4 files changed, 53 insertions(+), 3 deletions(-) diff --git a/lib/chef/provider/mount.rb b/lib/chef/provider/mount.rb index b099c267e6..17c4b9e8cf 100644 --- a/lib/chef/provider/mount.rb +++ b/lib/chef/provider/mount.rb @@ -118,6 +118,11 @@ class Chef raise Chef::Exceptions::UnsupportedAction, "#{self} does not implement #mount_options_unchanged?" end + # should actually check if the filesystem is mounted and new resource have any difference (not just return current_resource) and return true/false + def remount_require? + raise Chef::Exceptions::UnsupportedAction, "#{self} does not implement #remount_require?" + end + # It's entirely plausible that a site might prefer UUIDs or labels, so # we need to be able to update fstab to conform with their wishes # without necessarily needing to remount the device. diff --git a/lib/chef/provider/mount/linux.rb b/lib/chef/provider/mount/linux.rb index 57e0f8c324..0213f6a1e0 100644 --- a/lib/chef/provider/mount/linux.rb +++ b/lib/chef/provider/mount/linux.rb @@ -80,6 +80,8 @@ class Chef end @current_resource.mounted(mounted) end + + # should actually check if the filesystem is mounted and new resource have any difference (not just return current_resource) and return true/false def remount_require? remount_require = true real_mount_point = shell_out("findmnt --kernel #{@new_resource.device}| tail -1 | awk '{print$4}'").stdout.split(",") diff --git a/lib/chef/provider/mount/mount.rb b/lib/chef/provider/mount/mount.rb index 2021d6ece5..aacec71a89 100644 --- a/lib/chef/provider/mount/mount.rb +++ b/lib/chef/provider/mount/mount.rb @@ -255,6 +255,11 @@ class Chef @current_resource.pass == @new_resource.pass end + # should actually check if the filesystem is mounted and new resource have any difference (not just return current_resource) and return true/false + def remount_require? + raise Chef::Exceptions::UnsupportedAction, "#{self} does not implement #remount_require?" + end + # It will update or delete the entry from fstab. def edit_fstab(remove: false) if @current_resource.enabled diff --git a/spec/unit/provider/mount_spec.rb b/spec/unit/provider/mount_spec.rb index 7800d0c666..115fe04e2d 100644 --- a/spec/unit/provider/mount_spec.rb +++ b/spec/unit/provider/mount_spec.rb @@ -33,6 +33,7 @@ describe Chef::Provider::Mount do new_resource.name "/tmp/foo" new_resource.mount_point "/tmp/foo" new_resource.fstype "ext3" + new_resource.options %w{nodev nosuid} new_resource end @@ -90,9 +91,10 @@ describe Chef::Provider::Mount do new_resource.supports[:remount] = true end - it "should remount the filesystem if it is mounted" do + it "should remount the filesystem if it is mounted and remount_require? is true" do allow(current_resource).to receive(:mounted).and_return(true) - expect(provider).to receive(:remount_fs).and_return(true) + allow(provider).to receive(:remount_require?).and_return(true) + expect(provider).to receive(:remount_fs) provider.run_action(:remount) expect(new_resource).to be_updated_by_last_action end @@ -111,7 +113,10 @@ describe Chef::Provider::Mount do allow(current_resource).to receive(:mounted).and_return(true) end - it "should try a umount/remount of the filesystem" do + it "should try a umount and mount of the filesystem if it is mounted and remount_require? is true" do + allow(current_resource).to receive(:mounted).and_return(true) + allow(provider).to receive(:remount_require?).and_return(true) + expect(provider).not_to receive(:remount_fs) expect(provider).to receive(:umount_fs) expect(provider).to receive(:mounted?).and_return(true, false) expect(provider).to receive(:mount_fs) @@ -123,10 +128,43 @@ describe Chef::Provider::Mount do provider.unmount_retries = 1 expect(provider).to receive(:umount_fs) expect(provider).to receive(:mounted?).and_return(true, true) + expect(provider).to receive(:remount_require?).and_return(true) expect { provider.run_action(:remount) }.to raise_error(Chef::Exceptions::Mount) end end + describe "when the filesystem should not be remounted" do + it "should remount the filesystem if it is mounted is false and remount_require? is false" do + allow(current_resource).to receive(:mounted).and_return(false) + allow(provider).to receive(:remount_require?).and_return(false) + expect(provider).not_to receive(:remount_fs) + expect(provider).not_to receive(:umount_fs) + expect(provider).not_to receive(:mount_fs) + provider.run_action(:remount) + expect(new_resource).not_to be_updated_by_last_action + end + + it "should remount the filesystem if it is mounted is false and remount_require? is true" do + allow(current_resource).to receive(:mounted).and_return(false) + allow(provider).to receive(:remount_require?).and_return(true) + expect(provider).not_to receive(:remount_fs) + expect(provider).not_to receive(:umount_fs) + expect(provider).not_to receive(:mount_fs) + provider.run_action(:remount) + expect(new_resource).not_to be_updated_by_last_action + end + + it "should remount the filesystem if it is mounted is true and remount_require? is false" do + allow(current_resource).to receive(:mounted).and_return(true) + allow(provider).to receive(:remount_require?).and_return(false) + expect(provider).not_to receive(:remount_fs) + expect(provider).not_to receive(:umount_fs) + expect(provider).not_to receive(:mount_fs) + provider.run_action(:remount) + expect(new_resource).not_to be_updated_by_last_action + end + end + describe "when enabling the filesystem to be mounted" do it "should enable the mount if it isn't enable" do allow(current_resource).to receive(:enabled).and_return(false) -- cgit v1.2.1