Avoid change permission of shared FS on Compute nodes

Signed-off-by: Francesco Giordano <[email protected]>

Author: francesco-giordano

CHANGELOG.md

@@ -46,6 +46,7 @@ This file is used to list changes made in each version of the AWS ParallelCluste
   - xdcv: `2023.0.547-1`
   - gl: `2023.0.1027-1`
   - web_viewer: `2023.0.15022-1`
+- Avoid to reset FSx and EFS shared folders permissions when mounting them in the compute nodes.
 
 **BUG FIXES**
 - Fix an issue that was causing misalignment of compute nodes IP on instances with multiple network interfaces.

cookbooks/aws-parallelcluster-common/resources/efs/partial/_mount_umount.rb

@@ -81,6 +81,7 @@
       owner 'root'
       group 'root'
       mode '1777'
+      only_if { node['cluster']['node_type'] == "HeadNode" }
     end
   end
 end

cookbooks/aws-parallelcluster-common/resources/lustre/partial/_mount_unmount.rb

@@ -61,7 +61,7 @@
       owner 'root'
       group 'root'
       mode '1777'
-      only_if { fsx.can_change_shared_dir_permissions }
+      only_if { fsx.can_change_shared_dir_permissions && node['cluster']['node_type'] == "HeadNode" }
     end
   end
 end

cookbooks/aws-parallelcluster-common/spec/unit/resources/efs_spec.rb

@@ -253,80 +253,85 @@ def mock_already_installed(package, expected_version, installed)
 
 describe 'efs:mount' do
   for_all_oses do |platform, version|
-    context "on #{platform}#{version}" do
-      cached(:chef_run) do
-        runner = ChefSpec::Runner.new(
-          platform: platform, version: version,
-          step_into: ['efs']
-        ) do |node|
-          node.override['cluster']['region'] = "REGION"
-          node.override['cluster']['aws_domain'] = "DOMAIN"
-        end
-        runner.converge_dsl do
-          efs 'mount' do
-            efs_fs_id_array %w(id_1 id_2 id_3)
-            shared_dir_array %w(shared_dir_1 /shared_dir_2 /shared_dir_3)
-            efs_encryption_in_transit_array %w(true true not_true)
-            efs_iam_authorization_array %w(not_true true true)
-            action :mount
+    %w(HeadNode ComputeFleet).each do |node_type|
+      context "on #{platform}#{version} and node type #{node_type}" do
+        cached(:chef_run) do
+          runner = ChefSpec::Runner.new(
+            platform: platform, version: version,
+            step_into: ['efs']
+          ) do |node|
+            node.override['cluster']['region'] = "REGION"
+            node.override['cluster']['aws_domain'] = "DOMAIN"
+            node.override['cluster']['node_type'] = node_type
+          end
+          runner.converge_dsl do
+            efs 'mount' do
+              efs_fs_id_array %w(id_1 id_2 id_3)
+              shared_dir_array %w(shared_dir_1 /shared_dir_2 /shared_dir_3)
+              efs_encryption_in_transit_array %w(true true not_true)
+              efs_iam_authorization_array %w(not_true true true)
+              action :mount
+            end
           end
         end
-      end
 
-      before do
-        stub_command("mount | grep ' /shared_dir_1 '").and_return(false)
-        stub_command("mount | grep ' /shared_dir_2 '").and_return(true)
-        stub_command("mount | grep ' /shared_dir_3 '").and_return(true)
-      end
-
-      it 'creates shared directory' do
-        %w(/shared_dir_1 /shared_dir_2 /shared_dir_3).each do |shared_dir|
-          is_expected.to create_directory(shared_dir)
-            .with(owner: 'root')
-            .with(group: 'root')
-            .with(mode: '1777')
-          # .with(recursive: true) # even if we set recursive a true, the test fails
+        before do
+          stub_command("mount | grep ' /shared_dir_1 '").and_return(false)
+          stub_command("mount | grep ' /shared_dir_2 '").and_return(true)
+          stub_command("mount | grep ' /shared_dir_3 '").and_return(true)
         end
-      end
 
-      it 'mounts shared dir if not already mounted' do
-        is_expected.to mount_mount('/shared_dir_1')
-          .with(device: 'id_1:/')
-          .with(fstype: 'efs')
-          .with(dump: 0)
-          .with(pass: 0)
-          .with(options: %w(_netdev noresvport tls))
-          .with(retries: 10)
-          .with(retry_delay: 60)
-      end
+        it 'creates shared directory' do
+          %w(/shared_dir_1 /shared_dir_2 /shared_dir_3).each do |shared_dir|
+            is_expected.to create_directory(shared_dir)
+              .with(owner: 'root')
+              .with(group: 'root')
+              .with(mode: '1777')
+            # .with(recursive: true) # even if we set recursive a true, the test fails
+          end
+        end
 
-      it 'enables shared dir mount if already mounted' do
-        is_expected.to enable_mount('/shared_dir_2')
-          .with(device: 'id_2.efs.REGION.DOMAIN:/')
-          .with(fstype: 'efs')
-          .with(dump: 0)
-          .with(pass: 0)
-          .with(options: %w(_netdev noresvport tls iam))
-          .with(retries: 10)
-          .with(retry_delay: 6)
+        it 'mounts shared dir if not already mounted' do
+          is_expected.to mount_mount('/shared_dir_1')
+            .with(device: 'id_1:/')
+            .with(fstype: 'efs')
+            .with(dump: 0)
+            .with(pass: 0)
+            .with(options: %w(_netdev noresvport tls))
+            .with(retries: 10)
+            .with(retry_delay: 60)
+        end
 
-        is_expected.to enable_mount('/shared_dir_3')
-          .with(device: 'id_3.efs.REGION.DOMAIN:/')
-          .with(fstype: 'efs')
-          .with(dump: 0)
-          .with(pass: 0)
-          .with(options: %w(_netdev noresvport))
-          .with(retries: 10)
-          .with(retry_delay: 6)
-      end
+        it 'enables shared dir mount if already mounted' do
+          is_expected.to enable_mount('/shared_dir_2')
+            .with(device: 'id_2.efs.REGION.DOMAIN:/')
+            .with(fstype: 'efs')
+            .with(dump: 0)
+            .with(pass: 0)
+            .with(options: %w(_netdev noresvport tls iam))
+            .with(retries: 10)
+            .with(retry_delay: 6)
+
+          is_expected.to enable_mount('/shared_dir_3')
+            .with(device: 'id_3.efs.REGION.DOMAIN:/')
+            .with(fstype: 'efs')
+            .with(dump: 0)
+            .with(pass: 0)
+            .with(options: %w(_netdev noresvport))
+            .with(retries: 10)
+            .with(retry_delay: 6)
+        end
 
-      it 'changes permissions' do
-        %w(/shared_dir_1 /shared_dir_2 /shared_dir_3).each do |shared_dir|
-          is_expected.to create_directory("change permissions for #{shared_dir}")
-            .with(path: shared_dir)
-            .with(owner: 'root')
-            .with(group: 'root')
-            .with(mode: '1777')
+        if node_type == "HeadNode"
+          it 'changes permissions' do
+            %w(/shared_dir_1 /shared_dir_2 /shared_dir_3).each do |shared_dir|
+              is_expected.to create_directory("change permissions for #{shared_dir}")
+                .with(path: shared_dir)
+                .with(owner: 'root')
+                .with(group: 'root')
+                .with(mode: '1777')
+            end
+          end
         end
       end
     end