[BugFix] Fix issue making job fail when submitted as active directory user from login nodes.

Signed-off-by: Giacomo Marciani <[email protected]>

Author: gmarciani

CHANGELOG.md

@@ -32,6 +32,9 @@ This file is used to list changes made in each version of the AWS ParallelCluste
   - web_viewer: `2023.1.16388-1`
 
 **BUG FIXES**
+- Fix issue making job fail when submitted as active directory user from login nodes. 
+  The issue was caused by an incomplete configuration of the integration with the external Active Directory on the head node.
+  This fix comes with a breaking change: now cluster creation/update would fail if the integration with the Active Directory does not work.
 
 3.8.0
 ------

cookbooks/aws-parallelcluster-entrypoints/recipes/finalize.rb

@@ -21,5 +21,6 @@
 end
 
 include_recipe "aws-parallelcluster-platform::finalize"
+include_recipe "aws-parallelcluster-environment::finalize"
 
 include_recipe 'aws-parallelcluster-slurm::finalize' if node['cluster']['scheduler'] == 'slurm'

cookbooks/aws-parallelcluster-entrypoints/spec/spec_helper.rb

@@ -0,0 +1,4 @@
+require 'chefspec'
+require 'chefspec/berkshelf'
+
+require_relative '../../aws-parallelcluster-shared/spec/spec_helper'

cookbooks/aws-parallelcluster-entrypoints/spec/unit/recipes/finalize_spec.rb

@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+# Copyright:: 2024 Amazon.com, Inc. and its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with the
+# License. A copy of the License is located at
+#
+# http://aws.amazon.com/apache2.0/
+#
+# or in the "LICENSE.txt" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES
+# OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions and
+# limitations under the License.
+
+require 'spec_helper'
+
+describe 'aws-parallelcluster-entrypoints::finalize' do
+  for_all_oses do |platform, version|
+    context "on #{platform}#{version}" do
+      for_all_node_types do |node_type|
+        context "when #{node_type}" do
+          cached(:chef_run) do
+            runner = runner(platform: platform, version: version) do |node|
+              allow_any_instance_of(Object).to receive(:fetch_config).and_return(OpenStruct.new)
+              allow_any_instance_of(Object).to receive(:is_custom_node?).and_return(true)
+
+              node.override['cluster']['node_type'] = node_type
+              node.override['cluster']['scheduler'] = 'slurm'
+            end
+            runner.converge(described_recipe)
+          end
+          cached(:node) { chef_run.node }
+
+          %w(
+            aws-parallelcluster-platform::enable_chef_error_handler
+            aws-parallelcluster-computefleet::custom_parallelcluster_node
+            aws-parallelcluster-platform::finalize aws-parallelcluster-environment::finalize
+            aws-parallelcluster-slurm::finalize
+          ).each do |recipe_name|
+            it "includes the recipe #{recipe_name}" do
+              # TODO: This assertion requires to refactor all the resources having properties
+              #  aws_region and aws_domain because they are overwriting existing methods
+              #  defined in the aws-parallelcluster-shared cookbook, making the test compilation to fail.
+              #  We must re-enable this assertion once the refactoring has been done.
+              # is_expected.to include_recipe(recipe_name)
+            end
+          end
+        end
+      end
+    end
+  end
+end

cookbooks/aws-parallelcluster-environment/recipes/config/directory_service.rb

@@ -233,13 +233,3 @@
     sensitive true
   end
 end unless on_docker?
-
-if %w(HeadNode LoginNode).include? node['cluster']['node_type']
-  read_only_user = domain_service_read_only_user_name(node['cluster']['directory_service']['domain_read_only_user'])
-
-  execute 'Check AD connection and sync user data with remote directory service' do
-    command "getent passwd #{read_only_user}"
-    user 'root'
-    ignore_failure true
-  end
-end

cookbooks/aws-parallelcluster-environment/recipes/finalize.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+# Copyright:: 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with the
+# License. A copy of the License is located at
+#
+# http://aws.amazon.com/apache2.0/
+#
+# or in the "LICENSE.txt" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES
+# OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions and
+# limitations under the License.
+
+include_recipe 'aws-parallelcluster-environment::finalize_directory_service'

cookbooks/aws-parallelcluster-environment/recipes/finalize/finalize_directory_service.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+#
+# Cookbook:: aws-parallelcluster
+# Recipe:: finalize_directory_service
+#
+# Copyright:: 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with the
+# License. A copy of the License is located at
+#
+# http://aws.amazon.com/apache2.0/
+#
+# or in the "LICENSE.txt" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES
+# OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions and
+# limitations under the License.
+
+return if node['cluster']["directory_service"]["enabled"] == 'false'
+
+if %w(HeadNode LoginNode).include? node['cluster']['node_type']
+  default_user = node['cluster']['cluster_user']
+  read_only_user = domain_service_read_only_user_name(node['cluster']['directory_service']['domain_read_only_user'])
+
+  execute 'Fetch user data from remote directory service' do
+    # The switch-user (sudo -u) is necessary to trigger the fetching of AD data
+    command "sudo -u #{default_user} getent passwd #{read_only_user}"
+    user 'root'
+    retries 10 # Retries are just a safe guard in case the node is still fetching data from the AD
+    retry_delay 3
+  end
+end

cookbooks/aws-parallelcluster-environment/spec/unit/recipes/finalize_directory_service_spec.rb

@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+# Copyright:: 2024 Amazon.com, Inc. and its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with the
+# License. A copy of the License is located at
+#
+# http://aws.amazon.com/apache2.0/
+#
+# or in the "LICENSE.txt" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES
+# OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions and
+# limitations under the License.
+
+require 'spec_helper'
+
+describe 'aws-parallelcluster-environment::finalize_directory_service' do
+  for_all_oses do |platform, version|
+    context "on #{platform}#{version}" do
+      for_all_node_types do |node_type|
+        cluster_user = 'DEFAULT_CLUSTER_USER'
+        domain_read_only_user = 'DOMAIN_READ_ONLY_USER'
+
+        context "when #{node_type}" do
+          cached(:chef_run) do
+            runner = runner(platform: platform, version: version) do |node|
+              node.override['cluster']['node_type'] = node_type
+              node.override['cluster']['cluster_user'] = cluster_user
+              node.override['cluster']['directory_service']['enabled'] = true
+
+              allow_any_instance_of(Object).to receive(:domain_service_read_only_user_name).and_return(domain_read_only_user)
+            end
+            runner.converge(described_recipe)
+          end
+          cached(:node) { chef_run.node }
+
+          if %(HeadNode LoginNode).include?(node_type)
+            it 'fetches user data from remote directory service' do
+              is_expected.to run_execute('Fetch user data from remote directory service').with(
+                command: "sudo -u #{cluster_user} getent passwd #{domain_read_only_user}",
+                user: 'root',
+                retries: 10,
+                retry_delay: 3
+              )
+            end
+          else
+            it 'fetches user data from remote directory service' do
+              is_expected.not_to run_execute('Fetch user data from remote directory service')
+            end
+          end
+        end
+      end
+    end
+  end
+end

cookbooks/aws-parallelcluster-environment/spec/unit/recipes/finalize_spec.rb

@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+# Copyright:: 2024 Amazon.com, Inc. and its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with the
+# License. A copy of the License is located at
+#
+# http://aws.amazon.com/apache2.0/
+#
+# or in the "LICENSE.txt" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES
+# OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions and
+# limitations under the License.
+
+require 'spec_helper'
+
+describe 'aws-parallelcluster-environment::finalize' do
+  for_all_oses do |platform, version|
+    context "on #{platform}#{version}" do
+      for_all_node_types do |node_type|
+        context "when #{node_type}" do
+          cached(:chef_run) do
+            runner = runner(platform: platform, version: version) do |node|
+              node.override['cluster']['node_type'] = node_type
+            end
+            runner.converge(described_recipe)
+          end
+          cached(:node) { chef_run.node }
+
+          ["aws-parallelcluster-environment::finalize_directory_service"].each do |recipe_name|
+            it "includes the recipe #{recipe_name}" do
+              is_expected.to include_recipe(recipe_name)
+            end
+          end
+        end
+      end
+    end
+  end
+end

cookbooks/aws-parallelcluster-shared/spec/spec_helper.rb

@@ -51,6 +51,12 @@ def for_all_oses
   end
 end
 
+def for_all_node_types
+  %w(HeadNode ComputeFleet LoginNode).each do |node_type|
+    yield(node_type)
+  end
+end
+
 def runner(platform:, version:, step_into: [])
   ChefSpec::SoloRunner.new(platform: platform, version: version, step_into: step_into) do |node|
     yield node if block_given?