Add parameters to simplify test

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()
-    input_json["log_configs"].extend(log_configs.get("log_configs"))
+    log_configs["log_configs"].extend(input_json.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.
-    input_json["timestamp_formats"].update(log_configs.get("timestamp_formats"))
-    _write_log_configs(input_json)
+    log_configs["timestamp_formats"] = input_json["timestamp_formats"].update(log_configs.get("timestamp_formats"))
+    _write_log_configs(log_configs)
 
 
 def create_backup():

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

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

test/unit/cloudwatch_agent/test_cloudwatch_agent.py

@@ -16,62 +16,72 @@
 
 
 @pytest.mark.parametrize(
-    "error_type",
-    [None, "Duplicates", "Timestamp"],
+    "error_type, expected_error_response, input_json, schema",
+    [
+        (
+            None,
+            None,
+            {
+                "timestamp_formats": {
+                    "month_first": "%b %-d %H:%M:%S",
+                },
+                "log_configs": [{"timestamp_format_key": "month_first", "log_stream_name": "test"}],
+            },
+            {"type": "object", "properties": {"timestamp_formats": {"type": "object"}}},
+        ),
+        (
+            "Duplicates",
+            "The following log_stream_name values are used multiple times",
+            {
+                "timestamp_formats": {
+                    "month_first": "%b %-d %H:%M:%S",
+                },
+                "log_configs": [
+                    {"timestamp_format_key": "month_first", "log_stream_name": "test"},
+                    {"timestamp_format_key": "month_first", "log_stream_name": "test"},
+                ],
+            },
+            {"type": "object", "properties": {"timestamp_formats": {"type": "object"}}},
+        ),
+        (
+            "Timestamp",
+            "contains an invalid timestamp_format_key",
+            {
+                "timestamp_formats": {
+                    "month_first": "%b %-d %H:%M:%S",
+                },
+                "log_configs": [
+                    {"timestamp_format_key": "month_first", "log_stream_name": "test"},
+                    {"timestamp_format_key": "default", "log_stream_name": "test2"},
+                ],
+            },
+            {"type": "object", "properties": {"timestamp_formats": {"type": "object"}}},
+        ),
+        (
+            "FileNotFound",
+            "No file exists",
+            {
+                "timestamp_formats": {
+                    "month_first": "%b %-d %H:%M:%S",
+                },
+                "log_configs": [{"timestamp_format_key": "month_first", "log_stream_name": "test"}],
+            },
+            {"type": "object", "properties": {"timestamp_formats": {"type": "object"}}},
+        ),
+        (
+            "Schema",
+            "Failed validating 'type' in schema",
+            {
+                "timestamp_formats": {
+                    "month_first": "%b %-d %H:%M:%S",
+                },
+                "log_configs": [{"timestamp_format_key": "month_first", "log_stream_name": "test"}],
+            },
+            {"type": "string"},
+        ),
+    ],
 )
-def test_validate_json_content(mocker, 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 == "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"})
-
-    schema = {"type": "object", "properties": {"timestamp_formats": {"type": "object"}}}
-
-    mocker.patch(
-        "cloudwatch_agent_config_util._read_schema",
-        return_value=schema,
-    )
-
-    mocker.patch(
-        "cloudwatch_agent_config_util._read_log_configs",
-        return_value=input_json,
-    )
-
-    try:
-        validate_json(input_json)
-        assert_that(error_type).is_none()
-    except SystemExit as e:
-        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")
-
-
-@pytest.mark.parametrize(
-    "error_type",
-    [None, "FileNotFound", "Schema"],
-)
-def test_validate_json_invalid(mocker, 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":
-        schema = {"type": "string"}
-    else:
-        schema = {"type": "object", "properties": {"timestamp_formats": {"type": "object"}}}
-
+def test_validate_json_from_input(mocker, error_type, expected_error_response, input_json, schema):
     mocker.patch(
         "cloudwatch_agent_config_util._read_schema",
         return_value=schema,
@@ -88,14 +98,11 @@ def test_validate_json_invalid(mocker, error_type):
         assert_that(error_type).is_none()
     except SystemExit as e:
         assert_that(error_type).is_not_none()
-        if error_type == "FileNotFound":
-            assert_that(e.args[0]).contains("No file exists")
-        elif error_type == "Schema":
-            assert_that(e.args[0]).contains("Failed validating 'type' in schema")
+        assert_that(e.args[0]).contains(expected_error_response)
 
 
 @pytest.mark.parametrize("no_error", [True, False])
-def test_validate_json_input(mocker, test_datadir, no_error):
+def test_validate_json_from_file(mocker, test_datadir, no_error):
     input_file_path = os.path.join(test_datadir, "config.json")
 
     schema = {"type": "object", "properties": {"timestamp_formats": {"type": "object"}}}

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

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

test/unit/cloudwatch_agent/test_write_cloudwatch_agent_json.py

@@ -92,39 +92,50 @@ def test_add_instance_log_stream_prefixes():
 
 
 @pytest.mark.parametrize(
-    "dimensions",
+    "platform, length",
+    [("amazon", 2), ("ubuntu", 2)],
+)
+def test_select_configs_for_platform(platform, length):
+    configs = select_configs_for_platform(CONFIGS, platform)
+    assert_that(len(configs)).is_equal_to(length)
+
+
+@pytest.mark.parametrize(
+    "scheduler, length",
+    [("slurm", 2), ("awsbatch", 3)],
+)
+def test_select_configs_for_scheduler(scheduler, length):
+    configs = select_configs_for_scheduler(CONFIGS, scheduler)
+    assert_that(len(configs)).is_equal_to(length)
+
+
+@pytest.mark.parametrize(
+    "node",
     [
-        {"platform": "amazon", "length": 2},
-        {"platform": "ubuntu", "length": 2},
-        {"scheduler": "slurm", "length": 2},
-        {"scheduler": "awsbatch", "length": 3},
+        {"role": "ComputeFleet", "length": 2},
+        {"role": "HeadNode", "length": 3},
     ],
 )
-def test_select_configs_for_dimesion(dimensions):
-    if "platform" in dimensions.keys():
-        configs = select_configs_for_platform(CONFIGS, dimensions["platform"])
-        assert_that(len(configs)).is_equal_to(dimensions["length"])
-    else:
-        configs = select_configs_for_scheduler(CONFIGS, dimensions["scheduler"])
-        assert_that(len(configs)).is_equal_to(dimensions["length"])
+def test_select_configs_for_node_role(node):
+    configs = select_configs_for_node_role(CONFIGS, node["role"])
+    assert_that(len(configs)).is_equal_to(node["length"])
 
 
 @pytest.mark.parametrize(
-    "info",
+    "node_info, length",
     [
-        {"node_info": {"dcv_enabled": "head_node", "directory_service": {"enabled": "true"}}, "length": 3},
-        {"node_info": {"directory_service": {"enabled": "true"}}, "length": 2},
-        {"node_info": {"enabled": "true"}, "length": 1},
+        ({"dcv_enabled": "head_node", "directory_service": {"enabled": "true"}}, 3),
+        ({"directory_service": {"enabled": "true"}}, 2),
+        ({"enabled": "true"}, 1),
     ],
 )
-def test_select_configs_for_feature(mocker, info):
-    node_info = info["node_info"]
+def test_select_configs_for_feature(mocker, node_info, length):
     mocker.patch(
         "write_cloudwatch_agent_json.get_node_info",
         return_value=node_info,
     )
     selected_configs = select_configs_for_feature(CONFIGS)
-    assert_that(len(selected_configs)).is_equal_to(info["length"])
+    assert_that(len(selected_configs)).is_equal_to(length)
 
 
 def test_add_timestamps():
@@ -136,11 +147,9 @@ def test_add_timestamps():
 
 
 def test_filter_output_fields():
-    desired_keys = ["log_stream_name", "file_path", "timestamp_format", "log_group_name"]
     configs = filter_output_fields(CONFIGS)
     for config in configs:
-        for key in config:
-            assert_that(desired_keys).contains(key)
+        assert_that(config).does_not_contain_key("schedulers")
 
 
 def test_create_config():
@@ -164,18 +173,6 @@ def test_select_metrics(mocker):
         assert_that(metric_configs["metrics_collected"][key]).does_not_contain_key("node_roles")
 
 
-@pytest.mark.parametrize(
-    "node",
-    [
-        {"role": "ComputeFleet", "length": 2},
-        {"role": "HeadNode", "length": 3},
-    ],
-)
-def test_select_configs_for_node_role(node):
-    configs = select_configs_for_node_role(CONFIGS, node["role"])
-    assert_that(len(configs)).is_equal_to(node["length"])
-
-
 @pytest.mark.parametrize(
     "dimension",
     [{"name": "append", "type": dict}, {"name": "aggregation", "type": list}],