Add more unit test coverage and remove unused functions

cookbooks/aws-parallelcluster-environment/files/cloudwatch/cloudwatch_agent_config_util.py

@@ -136,12 +136,12 @@ def _write_log_configs(log_configs):
 def write_validated_json(input_json):
     """Write validated JSON back to the CloudWatch log configs file."""
     log_configs = _read_log_configs()
-    log_configs["log_configs"].extend(input_json.get("log_configs"))
+    input_json["log_configs"].extend(log_configs.get("log_configs"))
 
     # NOTICE: the input JSON's timestamp_formats dict is the one that is
     # updated, so that those defined in the original config aren't clobbered.
-    log_configs["timestamp_formats"] = input_json["timestamp_formats"].update(log_configs.get("timestamp_formats"))
-    _write_log_configs(log_configs)
+    input_json["timestamp_formats"].update(log_configs.get("timestamp_formats"))
+    _write_log_configs(input_json)
 
 
 def create_backup():

cookbooks/aws-parallelcluster-environment/files/cloudwatch/write_cloudwatch_agent_json.py

@@ -125,11 +125,6 @@ def select_logs(configs, args):
     return selected_configs
 
 
-def get_node_roles(scheudler_plugin_node_roles):
-    node_type_roles_map = {"ALL": ["ComputeFleet", "HeadNode"], "HEAD": ["HeadNode"], "COMPUTE": ["ComputeFleet"]}
-    return node_type_roles_map.get(scheudler_plugin_node_roles)
-
-
 def add_timestamps(configs, timestamps_dict):
     """For each config, set its timestamp_format field based on its timestamp_format_key field."""
     for config in configs:
@@ -156,8 +151,6 @@ def _collect_metric_properties(metric_config):
         # initial dict with default key-value pairs
         collected = {"metrics_collection_interval": DEFAULT_METRICS_COLLECTION_INTERVAL}
         collected.update({key: metric_config[key] for key in desired_keys if key in metric_config})
-        if "append_dimensions" in metric_config and "ClusterName" in metric_config["append_dimensions"]:
-            collected.update({"append_dimensions": {"ClusterName": get_node_info().get("stack_name")}})
         return collected
 
     return {
@@ -211,15 +204,6 @@ def create_config(log_configs, metric_configs):
     return cw_agent_config
 
 
-def get_dict_value(value, attributes, default=None):
-    """Get key value from dictionary and return default if the key does not exist."""
-    for key in attributes.split("."):
-        value = value.get(key, None)
-        if value is None:
-            return default
-    return value
-
-
 def main():
     """Create cloudwatch agent config file."""
     args = parse_args()

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

@@ -40,7 +40,7 @@
 
   describe file('/usr/local/bin/write_cloudwatch_agent_json.py') do
     it { should exist }
-    its('sha256sum') { should eq 'f24283f824072456394ab01cd014d37a977fc63fe951c9dd4a7df982bf6e5a82' }
+    its('sha256sum') { should eq '29ac1f73e0403095f40a4217e737d58d3786962a2c5427eb9c690627841ae17a' }
     its('owner') { should eq 'root' }
     its('group') { should eq 'root' }
     its('mode') { should cmp '0755' }
@@ -64,7 +64,7 @@
 
   describe file('/usr/local/bin/cloudwatch_agent_config_util.py') do
     it { should exist }
-    its('sha256sum') { should eq '57a69fe5ceebeb98c067ab5975bf566e610659a4751810da18f441f583791c07' }
+    its('sha256sum') { should eq 'e7ba2d332d3e96ff6099bb420e841bde23d303aecaf2a5109ad712216be1042d' }
     its('owner') { should eq 'root' }
     its('group') { should eq 'root' }
     its('mode') { should cmp '0644' }

test/unit/cloudwatch_agent/test_cloudwatch_agent.py

@@ -8,46 +8,138 @@
 # 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.
+import os
+
 import pytest
 from assertpy import assert_that
-from cloudwatch_agent_config_util import validate_json
+from cloudwatch_agent_config_util import validate_json, write_validated_json
 
 
 @pytest.mark.parametrize(
     "error_type",
-    [None, "Duplicates", "Schema", "Timestamp"],
+    [None, "Duplicates", "Timestamp", "FileNotFound", "Schema"],
 )
-def test_validate_json(mocker, error_type):
+def test_validate_json(mocker, test_datadir, error_type):
     input_json = {
         "timestamp_formats": {
             "month_first": "%b %-d %H:%M:%S",
         },
         "log_configs": [{"timestamp_format_key": "month_first", "log_stream_name": "test"}],
     }
-    if error_type == "Schema":
-        input_json = "ERROR"
-    elif error_type == "Duplicates":
+
+    if error_type == "Duplicates":
         input_json["log_configs"].append({"timestamp_format_key": "month_first", "log_stream_name": "test"})
     elif error_type == "Timestamp":
         input_json["log_configs"].append({"timestamp_format_key": "default", "log_stream_name": "test2"})
-        print(input_json)
     schema = {"type": "object", "properties": {"timestamp_formats": {"type": "object"}}}
-    mocker.patch(
-        "cloudwatch_agent_config_util._read_json_at",
-        return_value=input_json,
-    )
+
+    if error_type == "Schema":
+        schema = {"type": "string"}
+
     mocker.patch(
         "cloudwatch_agent_config_util._read_schema",
         return_value=schema,
     )
+
+    if error_type != "FileNotFound":
+        mocker.patch(
+            "cloudwatch_agent_config_util._read_log_configs",
+            return_value=input_json,
+        )
+
+    if error_type == "InvalidJSON":
+        mocker.patch(
+            "builtins.open",
+            side_effect=ValueError,
+        )
     try:
         validate_json(input_json)
-        validate_json()
         assert_that(error_type).is_none()
     except SystemExit as e:
-        if error_type == "Schema":
-            assert_that(e.args[0]).contains("Failed validating 'type' in schema")
-        elif error_type == "Duplicates":
+        assert_that(error_type).is_not_none()
+        if error_type == "Duplicates":
             assert_that(e.args[0]).contains("The following log_stream_name values are used multiple times: test")
         elif error_type == "Timestamp":
             assert_that(e.args[0]).contains("contains an invalid timestamp_format_key")
+        elif error_type == "FileNotFound":
+            assert_that(e.args[0]).contains("No file exists")
+        elif error_type == "InvalidJSON":
+            assert_that(e.args[0]).contains("contains invalid JSON")
+        elif error_type == "Schema":
+            assert_that(e.args[0]).contains("Failed validating 'type' in schema")
+
+
+@pytest.mark.parametrize("no_error", [True, False])
+def test_validate_json_input(mocker, test_datadir, no_error):
+    input_file_path = os.path.join(test_datadir, "config.json")
+    input_file = open(input_file_path, encoding="utf-8")
+
+    schema = {"type": "object", "properties": {"timestamp_formats": {"type": "object"}}}
+
+    if not no_error:
+        mocker.patch(
+            "builtins.open",
+            side_effect=ValueError,
+        )
+    else:
+        mocker.patch(
+            "builtins.open",
+            return_value=input_file,
+        )
+    mocker.patch(
+        "cloudwatch_agent_config_util._read_schema",
+        return_value=schema,
+    )
+
+    mocker.patch(
+        "cloudwatch_agent_config_util._validate_timestamp_keys",
+        return_value=None,
+    )
+
+    input_json = {
+        "timestamp_formats": {
+            "month_first": "%b %-d %H:%M:%S",
+        },
+        "log_configs": [{"timestamp_format_key": "month_first", "log_stream_name": "test"}],
+    }
+
+    try:
+        validate_json()
+        assert_that(no_error).is_true()
+    except SystemExit as e:
+        assert_that(no_error).is_false()
+        assert_that(e.args[0]).contains("invalid JSON")
+    finally:
+        input_file.close()
+
+
+def test_write_validated_json(mocker, test_datadir, tmpdir):
+    input_json = {
+        "timestamp_formats": {
+            "month_first": "%b %-d %H:%M:%S",
+        },
+        "log_configs": [{"timestamp_format_key": "month_first", "log_stream_name": "test"}],
+    }
+
+    input_json2 = {
+        "timestamp_formats": {
+            "default": "%Y-%m-%d %H:%M:%S,%f",
+        },
+        "log_configs": [{"timestamp_format_key": "month_first", "log_stream_name": "test2"}],
+    }
+
+    output_file = f"{tmpdir}/output.json"
+
+    mocker.patch(
+        "cloudwatch_agent_config_util._read_json_at",
+        return_value=input_json,
+    )
+
+    mocker.patch("os.environ.get", return_value=output_file)
+
+    write_validated_json(input_json2)
+
+    expected = os.path.join(test_datadir, "output.json")
+
+    with open(output_file, encoding="utf-8") as f, open(expected, encoding="utf-8") as exp_f:
+        assert_that(f.read()).is_equal_to(exp_f.read())

test/unit/cloudwatch_agent/test_cloudwatch_agent/test_validate_json_input/config.json

@@ -0,0 +1,11 @@
+{
+  "timestamp_formats": {
+    "month_first": "%b %-d %H:%M:%S"
+  },
+  "log_configs": [
+    {
+      "timestamp_format_key": "month_first",
+      "log_stream_name": "test"
+    }
+  ]
+}

test/unit/cloudwatch_agent/test_cloudwatch_agent/test_write_validated_json/output.json

@@ -0,0 +1,16 @@
+{
+  "timestamp_formats": {
+    "default": "%Y-%m-%d %H:%M:%S,%f",
+    "month_first": "%b %-d %H:%M:%S"
+  },
+  "log_configs": [
+    {
+      "timestamp_format_key": "month_first",
+      "log_stream_name": "test2"
+    },
+    {
+      "timestamp_format_key": "month_first",
+      "log_stream_name": "test"
+    }
+  ]
+}