Skip to content

Commit 5590ac9

Browse files
gmarcianigmarciani
committed
[Storage] Execute deletion of shared storage mount dir after lazy unmounting without recursion and only if the directory is empty.
Signed-off-by: Giacomo Marciani <[email protected]>
1 parent c06a394 commit 5590ac9

File tree

6 files changed

+93
-56
lines changed

6 files changed

+93
-56
lines changed

‎cookbooks/aws-parallelcluster-environment/resources/efs/partial/_mount_umount.rb‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,9 @@
118118
owner 'root'
119119
group 'root'
120120
mode '1777'
121-
recursive true
121+
recursive false
122122
action :delete
123+
only_if { Dir.empty?(efs_shared_dir.to_s) }
123124
end
124125
end
125126
end

‎cookbooks/aws-parallelcluster-environment/resources/lustre/partial/_mount_unmount.rb‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,9 @@
9090
owner 'root'
9191
group 'root'
9292
mode '1777'
93-
recursive true
93+
recursive false
9494
action :delete
95+
only_if { Dir.empty?(fsx.shared_dir) }
9596
end
9697
end
9798
end

‎cookbooks/aws-parallelcluster-environment/resources/volume.rb‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,9 @@
112112

113113
# Delete the shared directories
114114
directory shared_dir do
115-
recursive true
115+
recursive false
116116
action :delete
117+
only_if { Dir.empty?(shared_dir) }
117118
end
118119
end
119120

‎cookbooks/aws-parallelcluster-environment/spec/unit/resources/efs_spec.rb‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,8 @@ def mock_already_installed(package, expected_version, installed)
355355
before do
356356
stub_command("mount | grep ' /shared_dir_1 '").and_return(false)
357357
stub_command("mount | grep ' /shared_dir_2 '").and_return(true)
358+
allow(Dir).to receive(:empty?).with("/shared_dir_1").and_return(true)
359+
allow(Dir).to receive(:empty?).with("/shared_dir_2").and_return(false)
358360
end
359361

360362
it 'unmounts efs' do
@@ -378,10 +380,13 @@ def mock_already_installed(package, expected_version, installed)
378380
.with(path: "/etc/fstab")
379381
.with(pattern: " #{shared_dir} ")
380382
end
383+
end
381384

382-
it "deletes shared dir #{shared_dir}" do
383-
is_expected.to delete_directory(shared_dir)
384-
end
385+
it "deletes shared dir only if empty" do
386+
is_expected.to delete_directory('/shared_dir_1')
387+
.with(recursive: false)
388+
389+
is_expected.not_to delete_directory('/shared_dir_2')
385390
end
386391
end
387392
end

‎cookbooks/aws-parallelcluster-environment/spec/unit/resources/lustre_mount_spec.rb‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,8 @@
362362
before do
363363
stub_command("mount | grep ' /shared_dir_1 '").and_return(false)
364364
stub_command("mount | grep ' /shared_dir_2 '").and_return(true)
365+
allow(Dir).to receive(:empty?).with("/shared_dir_1").and_return(true)
366+
allow(Dir).to receive(:empty?).with("/shared_dir_2").and_return(false)
365367
end
366368

367369
it 'unmounts fsx only if mounted' do
@@ -384,9 +386,10 @@
384386
.with(pattern: "lustre_id_2.fsx.REGION.amazonaws.com@tcp:/mount_name_2 *")
385387
end
386388

387-
it 'deletes shared dir' do
389+
it 'deletes shared dir only if empty' do
388390
is_expected.to delete_directory('/shared_dir_1')
389-
is_expected.to delete_directory('/shared_dir_2')
391+
.with(recursive: false)
392+
is_expected.not_to delete_directory('/shared_dir_2')
390393
end
391394
end
392395

@@ -414,6 +417,8 @@
414417
before do
415418
stub_command("mount | grep ' /shared_dir_1 '").and_return(false)
416419
stub_command("mount | grep ' /shared_dir_2 '").and_return(true)
420+
allow(Dir).to receive(:empty?).with("/shared_dir_1").and_return(true)
421+
allow(Dir).to receive(:empty?).with("/shared_dir_2").and_return(false)
417422
end
418423

419424
it 'unmounts fsx only if mounted' do
@@ -436,9 +441,10 @@
436441
.with(pattern: "ontap_id_2.fsx.REGION.amazonaws.com:/junction_path_2 *")
437442
end
438443

439-
it 'deletes shared dir' do
444+
it 'deletes shared dir only if empty' do
440445
is_expected.to delete_directory('/shared_dir_1')
441-
is_expected.to delete_directory('/shared_dir_2')
446+
.with(recursive: false)
447+
is_expected.not_to delete_directory('/shared_dir_2')
442448
end
443449
end
444450

@@ -466,6 +472,8 @@
466472
before do
467473
stub_command("mount | grep ' /filecache_dir_1 '").and_return(false)
468474
stub_command("mount | grep ' /filecache_dir_2 '").and_return(true)
475+
allow(Dir).to receive(:empty?).with("/filecache_dir_1").and_return(true)
476+
allow(Dir).to receive(:empty?).with("/filecache_dir_2").and_return(false)
469477
end
470478

471479
it 'unmounts fsx only if mounted' do
@@ -488,9 +496,10 @@
488496
.with(pattern: "filecache_dns_name_2@tcp:/filecache_mount_name_2 *")
489497
end
490498

491-
it 'deletes shared dir' do
499+
it 'deletes shared dir only if empty' do
492500
is_expected.to delete_directory('/filecache_dir_1')
493-
is_expected.to delete_directory('/filecache_dir_2')
501+
.with(recursive: false)
502+
is_expected.not_to delete_directory('/filecache_dir_2')
494503
end
495504
end
496505
end

‎cookbooks/aws-parallelcluster-environment/spec/unit/resources/volume_spec.rb‎

Lines changed: 64 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -120,64 +120,84 @@
120120
for_all_oses do |platform, version|
121121
context "on #{platform}#{version}" do
122122
context "when not mounted" do
123-
cached(:chef_run) do
124-
runner = runner(platform: platform, version: version, step_into: ['volume'])
125-
runner.converge_dsl('aws-parallelcluster-environment') do
126-
volume 'unmount' do
127-
shared_dir 'SHARED_DIR'
128-
action :unmount
123+
[true, false].each do |is_dir_empty|
124+
context "when shared dir #{is_dir_empty ? 'is' : 'is not'} empty" do
125+
cached(:chef_run) do
126+
runner = runner(platform: platform, version: version, step_into: ['volume'])
127+
runner.converge_dsl('aws-parallelcluster-environment') do
128+
volume 'unmount' do
129+
shared_dir 'SHARED_DIR'
130+
action :unmount
131+
end
132+
end
129133
end
130-
end
131-
end
132134

133-
before do
134-
stub_command("mount | grep ' /SHARED_DIR '").and_return(false)
135-
end
135+
before do
136+
stub_command("mount | grep ' /SHARED_DIR '").and_return(false)
137+
allow(Dir).to receive(:empty?).with("/SHARED_DIR").and_return(is_dir_empty)
138+
end
136139

137-
it 'does not unmount volume' do
138-
is_expected.not_to run_execute('unmount volume')
139-
end
140+
it 'does not unmount volume' do
141+
is_expected.not_to run_execute('unmount volume')
142+
end
140143

141-
it "removes volume /SHARED_DIR from /etc/fstab" do
142-
is_expected.to edit_delete_lines("remove volume /SHARED_DIR from /etc/fstab")
143-
.with(path: "/etc/fstab")
144-
.with(pattern: " /SHARED_DIR ")
145-
end
144+
it "removes volume /SHARED_DIR from /etc/fstab" do
145+
is_expected.to edit_delete_lines("remove volume /SHARED_DIR from /etc/fstab")
146+
.with(path: "/etc/fstab")
147+
.with(pattern: " /SHARED_DIR ")
148+
end
146149

147-
it "deletes shared dir /SHARED_DIR" do
148-
is_expected.to delete_directory('/SHARED_DIR')
150+
it "deletes shared dir only if empty" do
151+
if is_dir_empty
152+
is_expected.to delete_directory('/SHARED_DIR')
153+
.with(recursive: false)
154+
else
155+
is_expected.not_to delete_directory('/SHARED_DIR')
156+
end
157+
end
158+
end
149159
end
150160
end
151161

152162
context "when mounted" do
153-
cached(:chef_run) do
154-
runner = runner(platform: platform, version: version, step_into: ['volume'])
155-
runner.converge_dsl('aws-parallelcluster-environment') do
156-
volume 'unmount' do
157-
shared_dir '/SHARED_DIR'
158-
action :unmount
163+
[true, false].each do |is_dir_empty|
164+
context "when shared dir #{is_dir_empty ? 'is' : 'is not'} empty" do
165+
cached(:chef_run) do
166+
runner = runner(platform: platform, version: version, step_into: ['volume'])
167+
runner.converge_dsl('aws-parallelcluster-environment') do
168+
volume 'unmount' do
169+
shared_dir '/SHARED_DIR'
170+
action :unmount
171+
end
172+
end
159173
end
160-
end
161-
end
162174

163-
before do
164-
stub_command("mount | grep ' /SHARED_DIR '").and_return(true)
165-
end
175+
before do
176+
stub_command("mount | grep ' /SHARED_DIR '").and_return(true)
177+
allow(Dir).to receive(:empty?).with("/SHARED_DIR").and_return(is_dir_empty)
178+
end
166179

167-
it 'unmounts volume' do
168-
is_expected.to unmount_volume('unmount')
169-
is_expected.to run_execute('unmount volume /SHARED_DIR')
170-
.with(command: "umount -fl /SHARED_DIR")
171-
end
180+
it 'unmounts volume' do
181+
is_expected.to unmount_volume('unmount')
182+
is_expected.to run_execute('unmount volume /SHARED_DIR')
183+
.with(command: "umount -fl /SHARED_DIR")
184+
end
172185

173-
it "removes volume /SHARED_DIR from /etc/fstab" do
174-
is_expected.to edit_delete_lines("remove volume /SHARED_DIR from /etc/fstab")
175-
.with(path: "/etc/fstab")
176-
.with(pattern: " /SHARED_DIR ")
177-
end
186+
it "removes volume /SHARED_DIR from /etc/fstab" do
187+
is_expected.to edit_delete_lines("remove volume /SHARED_DIR from /etc/fstab")
188+
.with(path: "/etc/fstab")
189+
.with(pattern: " /SHARED_DIR ")
190+
end
178191

179-
it "deletes shared dir /SHARED_DIR" do
180-
is_expected.to delete_directory('/SHARED_DIR')
192+
it "deletes shared dir only if empty" do
193+
if is_dir_empty
194+
is_expected.to delete_directory('/SHARED_DIR')
195+
.with(recursive: false)
196+
else
197+
is_expected.not_to delete_directory('/SHARED_DIR')
198+
end
199+
end
200+
end
181201
end
182202
end
183203
end

0 commit comments

Comments
 (0)