Cookstyle and code-linters corrections

* Correcting Kitchen and Unit tests
* Adding share_compute_fleet_dna.py for tox checks

Author: Himani Anil Deshpande

cookbooks/aws-parallelcluster-environment/files/cfn_hup_configuration/share_compute_fleet_dna.py

@@ -11,30 +11,36 @@
 # limitations under the License.
 
 
-import configparser
 import argparse
-from email import message_from_string
+import base64
+import configparser
 import json
+import logging
 import os
+from email import message_from_string
+
 import boto3
-from botocore.config import Config
 import yaml
-import base64
-import logging
+from botocore.config import Config
 from retrying import retry
 
 COMPUTE_FLEET_SHARED_LOCATION = "/opt/parallelcluster/shared/"
 
-COMPUTE_FLEET_SHARED_DNA_LOCATION = COMPUTE_FLEET_SHARED_LOCATION + 'dna/'
+COMPUTE_FLEET_SHARED_DNA_LOCATION = COMPUTE_FLEET_SHARED_LOCATION + "dna/"
 
-COMPUTE_FLEET_LAUNCH_TEMPLATE_CONFIG = COMPUTE_FLEET_SHARED_LOCATION + 'launch-templates-config.json'
+COMPUTE_FLEET_LAUNCH_TEMPLATE_CONFIG = COMPUTE_FLEET_SHARED_LOCATION + "launch-templates-config.json"
 
 logger = logging.getLogger(__name__)
 logging.basicConfig(level=logging.INFO)
 
+
 def get_compute_launch_template_ids(lt_config_file_name):
-    """Load launch-templates-config.json which contains ID, Version number and Logical ID of all queues in Compute Fleet's Launch Template.
-    The format of launch-templates-config.json is
+    """
+    Load launch-templates-config.json.
+
+    It contains ID, Version number and Logical ID of all queues in Compute Fleet's Launch Template.
+
+    The format of launch-templates-config.json:
      {
         "Queues": {
             "queue1": {
@@ -61,24 +67,25 @@ def get_compute_launch_template_ids(lt_config_file_name):
             }
         }
      }
+
     """
     lt_config = None
     try:
-        with open(lt_config_file_name, 'r') as file:
+        with open(lt_config_file_name, "r") as file:
             lt_config = json.loads(file.read())
     except Exception as err:
-        logger.warn("Unable to read %s due to %s", lt_config_file_name, err)
+        logger.warning("Unable to read %s due to %s", lt_config_file_name, err)
 
-    return  lt_config
+    return lt_config
 
 
 def share_compute_fleet_dna(args):
-    """Creates all dna.json for each queue in cluster."""
+    """Create dna.json for each queue in cluster."""
     lt_config = get_compute_launch_template_ids(COMPUTE_FLEET_LAUNCH_TEMPLATE_CONFIG)
     if lt_config:
-        all_queues = lt_config.get('Queues')
+        all_queues = lt_config.get("Queues")
         for _, queues in all_queues.items():
-            compute_resources = queues.get('ComputeResources')
+            compute_resources = queues.get("ComputeResources")
             for _, compute_res in compute_resources.items():
                 get_latest_dna_data(compute_res, COMPUTE_FLEET_SHARED_DNA_LOCATION, args)
 
@@ -98,63 +105,62 @@ def parse_proxy_config():
 @retry(stop_max_attempt_number=5, wait_fixed=3000)
 def get_user_data(lt_id, lt_version, region_name):
     """
-    Calls EC2 DescribeLaunchTemplateVersions API to get UserData from Launch Template specified.
+    Get UserData from specified Launch Template using EC2 DescribeLaunchTemplateVersions API.
+
     :param lt_id: Launch Template ID (eg: lt-12345678901234567)
     :param lt_version: Launch Template latest Version Number (eg: 2)
     :param region_name: AWS region name (eg: us-east-1)
-    :return: User_data in MIME format
+    :return: string of user_data in MIME format
     """
     decoded_data = None
     try:
         proxy_config = parse_proxy_config()
 
         ec2_client = boto3.client("ec2", region_name=region_name, config=proxy_config)
         response = ec2_client.describe_launch_template_versions(
-            LaunchTemplateId= lt_id,
+            LaunchTemplateId=lt_id,
             Versions=[
                 lt_version,
             ],
-        ).get('LaunchTemplateVersions')
-        decoded_data = base64.b64decode(response[0]['LaunchTemplateData']['UserData'], validate=True).decode('utf-8')
+        ).get("LaunchTemplateVersions")
+        decoded_data = base64.b64decode(response[0]["LaunchTemplateData"]["UserData"], validate=True).decode("utf-8")
     except Exception as err:
         if hasattr(err, "message"):
             err = err.message
         logger.error(
-            "Unable to get UserData for launch template %s with version %s.\nException: %s",
-            lt_id, lt_version, err
+            "Unable to get UserData for launch template %s with version %s.\nException: %s", lt_id, lt_version, err
         )
 
     return decoded_data
 
 
 def get_write_directives_section(user_data):
-    """
-    Parses MIME formatted UserData that we get from EC2 to extract write_files section from cloud-config section.
-    """
+    """Get write_files section from cloud-config section of MIME formatted UserData."""
     write_directives_section = None
     try:
         data = message_from_string(user_data)
         for cloud_config_section in data.walk():
-            if cloud_config_section.get_content_type() == 'text/cloud-config':
-                write_directives_section = yaml.safe_load(cloud_config_section._payload).get('write_files')
+            if cloud_config_section.get_content_type() == "text/cloud-config":
+                write_directives_section = yaml.safe_load(cloud_config_section._payload).get("write_files")
     except Exception as err:
         logger.error("Error occurred while parsing write_files section.\nException: %s", err)
     return write_directives_section
 
 
 def write_dna_files(write_files_section, shared_storage_loc):
     """
-    Writes the dna.json in shared location after extracting it from write_files section of UserData.
+    After extracting dna.json from write_files section of UserData, write it in shared location.
+
     :param write_files_section: Entire write_files section from UserData
     :param shared_storage_loc: Shared Storage Location of where to write dna.json
     :return: None
     """
     try:
-        file_path = shared_storage_loc+"-dna.json"
+        file_path = shared_storage_loc + "-dna.json"
         for data in write_files_section:
-            if data['path'] in ['/tmp/dna.json']:
-                with open(file_path,"w") as file:
-                    file.write(json.dumps(json.loads(data['content']),indent=4))
+            if data["path"] in ["/tmp/dna.json"]:
+                with open(file_path, "w", encoding="utf-8") as file:
+                    file.write(json.dumps(json.loads(data["content"]), indent=4))
     except Exception as err:
         if hasattr(err, "message"):
             err = err.message
@@ -163,16 +169,19 @@ def write_dna_files(write_files_section, shared_storage_loc):
 
 def get_latest_dna_data(resource, output_location, args):
     """
-    Function to get latest User Data, extract relevant details and write dna.json.
+    Get latest User Data, extract relevant details and write dna.json.
+
     :param resource: Resource containing LT ID, Version and Logical id
     :param output_location: Shared Storage Location were we want to write dna.json
     :param args: Command Line arguments
     :rtype: None
     """
-    user_data = get_user_data(resource.get('LaunchTemplate').get('Id'), resource.get('LaunchTemplate').get('Version'), args.region)
+    user_data = get_user_data(
+        resource.get("LaunchTemplate").get("Id"), resource.get("LaunchTemplate").get("Version"), args.region
+    )
     if user_data:
         write_directives = get_write_directives_section(user_data)
-        write_dna_files(write_directives, output_location+resource.get('LaunchTemplate').get("LogicalId"))
+        write_dna_files(write_directives, output_location + resource.get("LaunchTemplate").get("LogicalId"))
 
 
 def cleanup(directory_loc):
@@ -183,7 +192,7 @@ def cleanup(directory_loc):
             if os.path.isfile(f_path):
                 os.remove(f_path)
         except Exception as err:
-            logger.warn(f"Unable to delete %s due to %s", f_path, err)
+            logger.warning("Unable to delete %s due to %s", f_path, err)
 
 
 def _parse_cli_args():
@@ -224,9 +233,11 @@ def main():
     except Exception as err:
         if hasattr(err, "message"):
             err = err.message
-        logger.exception("Encountered exception when fetching latest dna.json for ComputeFleet, exiting gracefully: %s", err)
+        logger.exception(
+            "Encountered exception when fetching latest dna.json for ComputeFleet, exiting gracefully: %s", err
+        )
         raise SystemExit(0)
 
 
 if __name__ == "__main__":
-    main()
+    main()

cookbooks/aws-parallelcluster-environment/recipes/install/cfn_bootstrap.rb

@@ -94,7 +94,7 @@
 
 # Add cfn-hup runner
 template "#{node['cluster']['scripts_dir']}/cfn-hup-runner.sh" do
-  source "cfn_bootstrap/cfn-hup-runner.sh.erb"
+  source "cfn_hup_configuration/cfn-hup-runner.sh.erb"
   owner 'root'
   group 'root'
   mode '0744'

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

@@ -77,7 +77,7 @@
 
         it 'adds cfn-hup runner' do
           is_expected.to create_template("#{node['cluster']['scripts_dir']}/cfn-hup-runner.sh").with(
-            source: "cfn_bootstrap/cfn-hup-runner.sh.erb",
+            source: "cfn_hup_configuration/cfn-hup-runner.sh.erb",
             owner: 'root',
             group: 'root',
             mode: '0744',

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

@@ -27,7 +27,6 @@
       aws-parallelcluster-environment::raid
       aws-parallelcluster-environment::efs
       aws-parallelcluster-environment::fsx
-      aws-parallelcluster-environment::config_cfn_hup
     )
     @expected_recipes.each do |recipe_name|
       allow_any_instance_of(Chef::Recipe).to receive(:include_recipe).with(recipe_name) do

cookbooks/aws-parallelcluster-environment/spec/unit/resources/cfn_hup_configuration_spec.rb

@@ -18,7 +18,7 @@ def self.configure(chef_run)
 LAUNCH_TEMPLATE_ID = "LAUNCH_TEMPLATE_ID".freeze
 SCRIPT_DIR = "SCRIPT_DIR".freeze
 MONITOR_SHARED_DIR = "MONITOR_SHARED_DIR".freeze
-NODE_BOOTSTRAP_TIMEOUT = "1800"
+NODE_BOOTSTRAP_TIMEOUT = "1800".freeze
 
 describe 'cfn_hup_configuration:configure' do
   for_all_oses do |platform, version|
@@ -29,8 +29,8 @@ def self.configure(chef_run)
             runner = runner(platform: platform, version: version, step_into: ['cfn_hup_configuration']) do |node|
               allow_any_instance_of(Object).to receive(:get_metadata_token).and_return("IMDS_TOKEN")
               allow_any_instance_of(Object).to receive(:get_metadata_with_token)
-                                                 .with("IMDS_TOKEN", URI("http://169.254.169.254/latest/meta-data/iam/security-credentials"))
-                                                 .and_return(INSTANCE_ROLE_NAME)
+                .with("IMDS_TOKEN", URI("http://169.254.169.254/latest/meta-data/iam/security-credentials"))
+                .and_return(INSTANCE_ROLE_NAME)
               node.override["cluster"]["node_type"] = node_type
               node.override["cluster"]["region"] = AWS_REGION
               node.override["cluster"]["aws_domain"] = AWS_DOMAIN
@@ -49,63 +49,63 @@ def self.configure(chef_run)
           %w(/etc/cfn /etc/cfn/hooks.d).each do |dir|
             it "creates the directory #{dir}" do
               is_expected.to create_directory(dir)
-                               .with(owner: 'root')
-                               .with(group: 'root')
-                               .with(mode:  "0700")
-                               .with(recursive: true)
+                .with(owner: 'root')
+                .with(group: 'root')
+                .with(mode:  "0700")
+                .with(recursive: true)
             end
           end
 
           it "creates the file /etc/cfn/cfn-hup.conf" do
             is_expected.to create_template("/etc/cfn/cfn-hup.conf")
-                             .with(source: 'cfn_hup_configuration/cfn-hup.conf.erb')
-                             .with(user: "root")
-                             .with(group: "root")
-                             .with(mode: "0400")
-                             .with(variables: {
-                               stack_id: STACK_ID,
-                               region: AWS_REGION,
-                               cloudformation_url: CLOUDFORMATION_URL,
-                               cfn_init_role: INSTANCE_ROLE_NAME,
-                             })
+              .with(source: 'cfn_hup_configuration/cfn-hup.conf.erb')
+              .with(user: "root")
+              .with(group: "root")
+              .with(mode: "0400")
+              .with(variables: {
+                stack_id: STACK_ID,
+                region: AWS_REGION,
+                cloudformation_url: CLOUDFORMATION_URL,
+                cfn_init_role: INSTANCE_ROLE_NAME,
+             })
           end
 
           it "creates the file /etc/cfn/hooks.d/pcluster-update.conf" do
             is_expected.to create_template("/etc/cfn/hooks.d/pcluster-update.conf")
-                             .with(source: 'cfn_hup_configuration/cfn-hook-update.conf.erb')
-                             .with(user: "root")
-                             .with(group: "root")
-                             .with(mode: "0400")
-                             .with(variables: {
-                               stack_id: STACK_ID,
-                               region: AWS_REGION,
-                               cloudformation_url: CLOUDFORMATION_URL,
-                               cfn_init_role: INSTANCE_ROLE_NAME,
-                               launch_template_resource_id: LAUNCH_TEMPLATE_ID,
-                               update_hook_script_dir: SCRIPT_DIR,
-                               node_bootstrap_timeout: NODE_BOOTSTRAP_TIMEOUT,
-                             })
+              .with(source: 'cfn_hup_configuration/cfn-hook-update.conf.erb')
+              .with(user: "root")
+              .with(group: "root")
+              .with(mode: "0400")
+              .with(variables: {
+                stack_id: STACK_ID,
+                region: AWS_REGION,
+                cloudformation_url: CLOUDFORMATION_URL,
+                cfn_init_role: INSTANCE_ROLE_NAME,
+                launch_template_resource_id: LAUNCH_TEMPLATE_ID,
+                update_hook_script_dir: SCRIPT_DIR,
+                node_bootstrap_timeout: NODE_BOOTSTRAP_TIMEOUT,
+             })
           end
 
           if %(ComputeFleet).include?(node_type)
             it "creates the file #{SCRIPT_DIR}/cfn-hup-update-action.sh" do
               is_expected.to create_template("#{SCRIPT_DIR}/cfn-hup-update-action.sh")
-                               .with(source: "cfn_hup_configuration/#{node_type}/cfn-hup-update-action.sh.erb")
-                               .with(user: "root")
-                               .with(group: "root")
-                               .with(mode: "0700")
-                               .with(variables: {
-                                 monitor_shared_dir: "#{MONITOR_SHARED_DIR}/dna",
-                                 launch_template_resource_id: LAUNCH_TEMPLATE_ID,
+                .with(source: "cfn_hup_configuration/#{node_type}/cfn-hup-update-action.sh.erb")
+                .with(user: "root")
+                .with(group: "root")
+                .with(mode: "0700")
+                .with(variables: {
+                   monitor_shared_dir: "#{MONITOR_SHARED_DIR}/dna",
+                   launch_template_resource_id: LAUNCH_TEMPLATE_ID,
                                })
             end
           elsif node_type == 'HeadNode'
             it "creates #{SCRIPT_DIR}/share_compute_fleet_dna.py" do
               is_expected.to create_if_missing_cookbook_file("#{SCRIPT_DIR}/share_compute_fleet_dna.py")
-                               .with(source: 'cfn_hup_configuration/share_compute_fleet_dna.py')
-                               .with(user: 'root')
-                               .with(group: 'root')
-                               .with(mode: '0700')
+                .with(source: 'cfn_hup_configuration/share_compute_fleet_dna.py')
+                .with(user: 'root')
+                .with(group: 'root')
+                .with(mode: '0700')
             end
 
             it "creates the directory #{MONITOR_SHARED_DIR}/dna" do

cookbooks/aws-parallelcluster-environment/test/controls/cfn_hup_configuration_spec.rb

@@ -42,7 +42,7 @@
     its('group') { should eq 'root' }
   end
 
-  describe directory("#{node['cluster']['base_dir']}/dna") do
+  describe directory("#{node['cluster']['shared_dir']}/dna") do
     it { should exist }
   end
 end

cookbooks/aws-parallelcluster-platform/spec/unit/resources/fetch_dna_files_spec.rb

@@ -35,7 +35,7 @@ def self.cleanup(chef_run)
             node.override['cluster']['region'] = region
             node.override['kitchen'] = true
           end
-            ConvergeFetchDnaFiles.share(runner, extra_chef_attribute_location: "#{kitchen_instance_types_data_path}")
+          ConvergeFetchDnaFiles.share(runner, extra_chef_attribute_location: "#{kitchen_instance_types_data_path}")
         end
         cached(:node) { chef_run.node }
 
@@ -46,7 +46,7 @@ def self.cleanup(chef_run)
         # end
 
         it 'runs share_compute_fleet_dna.py to get dna files' do
-          is_expected.to run_execute('Share dna.json with ComputeFleet').with(
+          is_expected.to run_execute('Run share_compute_fleet_dna.py to get user_data.sh and share dna.json with ComputeFleet').with(
             command: "#{cookbook_virtualenv_path}/bin/python #{node['cluster']['scripts_dir']}/share_compute_fleet_dna.py" \
               " --region #{node['cluster']['region']}"
           )

cookbooks/aws-parallelcluster-slurm/recipes/update/update_head_node.rb

@@ -284,4 +284,4 @@ def update_nodes_in_queue(strategy, queues)
 
 fetch_dna_files 'Cleanup' do
   action :cleanup
-end
+end