ecs: Add test for fargate and EC2 awsvpc

Author: pingleig

terraform/ecs/extra_apps.tf

@@ -15,11 +15,14 @@ resource "aws_ecs_task_definition" "extra_apps" {
   for_each              = var.ecs_extra_apps
   family                = "taskdef-${module.common.testing_id}-${each.value.service_name}"
   container_definitions = data.template_file.extra_apps_defs[each.key].rendered
-  network_mode          = each.value.network_mode
-  cpu                   = each.value.cpu
-  memory                = each.value.memory
-  task_role_arn         = module.basic_components.aoc_iam_role_arn
-  execution_role_arn    = module.basic_components.aoc_iam_role_arn
+  requires_compatibilities = each.value.launch_type == "FARGATE" ? [
+    "FARGATE"] : [
+  "EC2"]
+  network_mode       = each.value.network_mode
+  cpu                = each.value.cpu
+  memory             = each.value.memory
+  task_role_arn      = module.basic_components.aoc_iam_role_arn
+  execution_role_arn = module.basic_components.aoc_iam_role_arn
 }
 
 resource "aws_ecs_service" "extra_apps" {
@@ -34,7 +37,8 @@ resource "aws_ecs_service" "extra_apps" {
   // NOTE: network configuration is only allowed for awsvpc
   // a hack for optional block https://github.com/hashicorp/terraform/issues/19898
   dynamic "network_configuration" {
-    for_each = each.value.network_mode == "awsvpc" ? list(each.value.network_mode) : []
+    for_each = each.value.network_mode == "awsvpc" ? tolist([
+    each.value.network_mode]) : []
     content {
       subnets = module.basic_components.aoc_private_subnet_ids
       security_groups = [
@@ -47,4 +51,8 @@ output "extra_apps_defs_rendered" {
   value = {
     for k, v in data.template_file.extra_apps_defs : k => v.rendered
   }
+}
+
+output "extra_app_task_defs" {
+  value = aws_ecs_task_definition.extra_apps
 }

terraform/ecs/main.tf

@@ -289,7 +289,7 @@ module "validator_without_sample_app" {
   aws_access_key_id     = var.aws_access_key_id
   aws_secret_access_key = var.aws_secret_access_key
 
-  depends_on = [aws_ecs_service.aoc_without_sample_app]
+  depends_on = [aws_ecs_service.aoc_without_sample_app, aws_ecs_service.extra_apps]
 }
 
 data "template_file" "cloudwatch_context" {

terraform/setup/setup.tf

@@ -97,6 +97,15 @@ resource "aws_security_group" "aoc_sg" {
   name   = module.common.aoc_vpc_security_group
   vpc_id = module.vpc.vpc_id
 
+  # Allow all TCP ingress within the VPC so prometheus scrape can work with private IP.
+  # https://stackoverflow.com/questions/49995417/self-reference-not-allowed-in-security-group-definition
+  ingress {
+    from_port = 0
+    to_port = 65535
+    protocol = "tcp"
+    self = true
+  }
+
   ingress {
     from_port   = 22
     to_port     = 22

terraform/testcases/containerinsight_ecs_prometheus/README.md

@@ -5,20 +5,13 @@
 This is e2e test
 for [extension/ecsobserver](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/extension/observer/ecsobserver)
 
-## TODO
-
-- [ ] [cloudwatch_context.json](cloudwatch_context.json)
-    - [ ] we need to update java code to include `taskDefinitionFamily` and `serviceName`
-    - [ ] cluster name is used by other tests, need to pass it when rendering the template, could do this for the
-      default template, it's empty
-- [x] app image repo is hardcoded and not used
-- [ ] log group name for sample applications' container log
-
 ## Usage
 
-Non default image `123456.dkr.ecr.us-west-2.amazonaws.com/aoc:myfeature-0.2`
+If your AWS account is `123456` and you want to run your own image
+`123456.dkr.ecr.us-west-2.amazonaws.com/aoc:myfeature-0.2`.
 
-- `ecs_launch_type` is the launch type for aoc itself, launch type for extra apps are defined in TODO(where?)
+- `ecs_launch_type` is the launch type for aoc itself, launch type for extra apps are defined
+  in [parameters.tfvars](parameters.tfvars)
 - `aoc_image_repo` is repo for aoc without tag
 - `aoc_version` is the image tag for aoc
 - `ecs_extra_apps_image_repo` is the repo for all the extra apps, remaining part of image name and version are defined
@@ -53,7 +46,9 @@ There are multiple sample applications
 
 ```bash
 # Run at project root to make sure the validator code pass style check and compiles
-./gradlew :validator:build 
+./gradlew :validator:build
+# Run at terraform/ecs to run validation without spinning up new infra
+make validate
 ```
 
 - validation config file name is specified
@@ -69,6 +64,12 @@ There are multiple sample applications
 
 ## Problems
 
-`Unknown variable; There is no variable named`
+List of common problems you may encounter.
+
+### Unknown variable
 
-- used wrong variable name in template file
+You are using wrong variable name in template files, or you didn't define the var when rendering template.
+
+```
+Unknown variable; There is no variable named
+```

terraform/testcases/containerinsight_ecs_prometheus/cloudwatch_context.json

@@ -2,11 +2,17 @@
   "clusterName": "${cluster_name}",
   "jmx": {
     "job": "ecssd",
-    "taskDefinitionFamily": "taskdef-${testing_id}-jmx"
+    "taskDefinitionFamilies": [
+      "taskdef-${testing_id}-jmx",
+      "taskdef-${testing_id}-jmxawsvpc",
+      "taskdef-${testing_id}-jmxfargate"
+    ]
   },
   "nginx": {
     "job": "ecssd",
-    "taskDefinitionFamily": "taskdef-${testing_id}-nginx-service",
+    "taskDefinitionFamilies": [
+      "taskdef-${testing_id}-nginx-service"
+    ],
     "serviceName": "aocservice-${testing_id}-nginx-service"
   }
 }

terraform/testcases/containerinsight_ecs_prometheus/parameters.tfvars

@@ -4,7 +4,6 @@ validation_config = "ecs-container-insight-prometheus.yml"
 sample_app_callable = false
 # sample apps that emit ecs metrics
 ecs_extra_apps = {
-  # TODO: need both host network, awspvc and fargate
   jmx = {
     definition   = "jmx.json"
     service_name = "jmx"
@@ -16,6 +15,32 @@ ecs_extra_apps = {
     memory       = 256
   }
 
+  # NOTE: for awsvpc to work form prometheus, we need change security group
+  # to allow all traffic within the VPC
+  jmxawsvpc = {
+    definition   = "jmx.json"
+    service_name = "jmxawsvpc"
+    service_type = "replica"
+    replicas     = 1
+    network_mode = "awsvpc"
+    launch_type  = "EC2"
+    cpu          = 256
+    memory       = 256
+  }
+
+  jmxfargate = {
+    definition   = "jmx.json"
+    service_name = "jmxfargate"
+    service_type = "replica"
+    replicas     = 1
+    network_mode = "awsvpc"
+    launch_type  = "FARGATE"
+    cpu          = 256
+    # Must set cpu and memory for fargate in specific ways
+    # https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ecs-taskdefinition.html#:~:text=ContainerDefinition-,If%20your%20tasks%20will,cpu%20parameter,-512
+    memory = 512
+  }
+
   nginx = {
     definition   = "nginx.json"
     service_name = "nginx-service"

validator/src/main/java/com/amazon/aoc/models/CloudWatchContext.java

@@ -58,7 +58,7 @@ public static class App {
     private String namespace;
     private String job;
     // For ECS
-    private String taskDefinitionFamily;
+    private String[] taskDefinitionFamilies;
     private String serviceName;
   }
 }

validator/src/main/java/com/amazon/aoc/validators/ContainerIInsightECSPrometheusStructuredLogValidator.java

@@ -56,10 +56,15 @@ void init(Context context, FileConfig expectedDataTemplate) throws Exception {
           app.getName() + ".json"));
       String templateInput = mustacheHelper.render(fileConfig, context);
       // NOTE: EKS use namespace, we use task family for matching log event to schema.
-      schemasToValidate.put(app.getTaskDefinitionFamily(), parseJsonSchema(templateInput));
+      for (String taskDefinitionFamily : app.getTaskDefinitionFamilies()) {
+        // We can deploy one workload in different ways (EC2, fargate etc.)
+        // so we have a list of task definition families.
+        schemasToValidate.put(taskDefinitionFamily, parseJsonSchema(templateInput));
+      }
       logStreamNames.add(app.getJob());
     }
-    log.info("apps to validate {}", validateApps.size());
+    log.info("apps to validate {} schema to validate {}", validateApps.size(),
+        schemasToValidate.keySet());
   }
 
   @Override

validator/src/main/java/com/amazon/aoc/validators/ContainerInsightPrometheusMetricsValidator.java

@@ -9,12 +9,14 @@
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
+import lombok.extern.log4j.Log4j2;
 import org.apache.commons.io.FilenameUtils;
 
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
 
+@Log4j2
 public class ContainerInsightPrometheusMetricsValidator extends AbstractCWMetricsValidator {
   private final ObjectMapper mapper = new ObjectMapper(new YAMLFactory());
 
@@ -31,6 +33,7 @@ List<Metric> getExpectedMetrics(
             expectedDataTemplate.getPath().toString(),
           app.getName() + "_metrics.mustache"));
       String templateInput = mustacheHelper.render(fileConfig, context);
+      // log.info("Rendered template {}", templateInput);
       List<Metric> appMetrics = mapper.readValue(templateInput.getBytes(StandardCharsets.UTF_8),
             new TypeReference<List<Metric>>() {
             });

validator/src/main/resources/expected-data-template/container-insight/ecs/prometheus/jmx_metrics.mustache

@@ -1,17 +1,20 @@
+# https://stackoverflow.com/questions/4067093/mustache-read-variables-from-parent-section-in-child-section
+# https://stackoverflow.com/questions/3954913/iterating-over-arrays-with-mustache
+{{#cloudWatchContext.jmx.taskDefinitionFamilies}}
 - metricName: jvm_classes_loaded
   namespace: ECS/ContainerInsights/Prometheus
   dimensions:
     - name: ClusterName
       value: {{cloudWatchContext.clusterName}}
     - name: TaskDefinitionFamily
-      value: {{cloudWatchContext.jmx.taskDefinitionFamily}}
+      value: {{.}}
 - metricName: jvm_memory_pool_bytes_used
   namespace: ECS/ContainerInsights/Prometheus
   dimensions:
     - name: ClusterName
       value: {{cloudWatchContext.clusterName}}
     - name: TaskDefinitionFamily
-      value: {{cloudWatchContext.jmx.taskDefinitionFamily}}
+      value: {{.}}
     - name: pool
       value: Metaspace
 - metricName: jvm_memory_bytes_used
@@ -20,6 +23,7 @@
     - name: ClusterName
       value: {{cloudWatchContext.clusterName}}
     - name: TaskDefinitionFamily
-      value: {{cloudWatchContext.jmx.taskDefinitionFamily}}
+      value: {{.}}
     - name: area
-      value: heap
+      value: heap
+{{/cloudWatchContext.jmx.taskDefinitionFamilies}}

validator/src/main/resources/expected-data-template/container-insight/ecs/prometheus/nginx_metrics.mustache

@@ -1,9 +1,11 @@
+{{#cloudWatchContext.nginx.taskDefinitionFamilies}}
 - metricName: nginx_up
   namespace: ECS/ContainerInsights/Prometheus
   dimensions:
     - name: ClusterName
       value: {{cloudWatchContext.clusterName}}
     - name: TaskDefinitionFamily
-      value: {{cloudWatchContext.nginx.taskDefinitionFamily}}
+      value: {{.}}
     - name: ServiceName
-      value: {{cloudWatchContext.nginx.serviceName}}
+      value: {{cloudWatchContext.nginx.serviceName}}
+{{/cloudWatchContext.nginx.taskDefinitionFamilies}}

validator/src/test/java/com/amazon/aoc/validators/ContainerInsightPrometheusMetricsValidatorTest.java

@@ -7,7 +7,6 @@
 import com.amazon.aoc.fileconfigs.PredefinedExpectedTemplate;
 import com.amazon.aoc.models.CloudWatchContext;
 import com.amazon.aoc.models.Context;
-import com.amazon.aoc.models.ValidationConfig;
 import com.amazon.aoc.services.CloudWatchService;
 import com.amazonaws.services.cloudwatch.model.MetricDataResult;
 import org.junit.Test;
@@ -23,12 +22,6 @@ public class ContainerInsightPrometheusMetricsValidatorTest {
 
   @Test
   public void testValidationSucceed() throws Exception {
-    // fake a validation config
-    ValidationConfig validationConfig = new ValidationConfig();
-    validationConfig.setCallingType("http");
-    validationConfig.setExpectedMetricTemplate(
-        PredefinedExpectedTemplate.CONTAINER_INSIGHT_EKS_PROMETHEUS_METRIC.name());
-
     // mock cloudwatch service
     CloudWatchService cloudWatchService = mock(CloudWatchService.class);
     List<MetricDataResult> metricDataResults = new ArrayList<>();
@@ -39,9 +32,7 @@ public void testValidationSucceed() throws Exception {
     ContainerInsightPrometheusMetricsValidator validator =
         new ContainerInsightPrometheusMetricsValidator();
     validator.init(
-        getContext(),
-        validationConfig,
-        null,
+        getContext(), null, null,
         PredefinedExpectedTemplate.CONTAINER_INSIGHT_EKS_PROMETHEUS_METRIC
     );
     validator.setCloudWatchService(cloudWatchService);
@@ -50,6 +41,25 @@ public void testValidationSucceed() throws Exception {
     validator.validate();
   }
 
+  @Test
+  public void tesECS() throws Exception {
+    CloudWatchService cloudWatchService = mock(CloudWatchService.class);
+    List<MetricDataResult> metricDataResults = new ArrayList<>();
+    metricDataResults.add(new MetricDataResult().withStatusCode("200").withValues(1.0));
+    when(cloudWatchService.getMetricData(any(), any(), any())).thenReturn(metricDataResults);
+
+    ContainerInsightPrometheusMetricsValidator validator =
+        new ContainerInsightPrometheusMetricsValidator();
+    validator.init(
+        getECSContext(), null, null,
+        PredefinedExpectedTemplate.CONTAINER_INSIGHT_ECS_PROMETHEUS_METRIC
+    );
+    validator.setCloudWatchService(cloudWatchService);
+    validator.setMaxRetryCount(1);
+    validator.setInitialSleepTime(0);
+    validator.validate();
+  }
+
   private Context getContext() {
     String namespace = "fakednamespace";
     String testingId = "fakedTesingId";
@@ -74,4 +84,17 @@ private Context getContext() {
     context.setCloudWatchContext(cloudWatchContext);
     return context;
   }
+
+  private Context getECSContext() {
+    CloudWatchContext.App jmx = new CloudWatchContext.App();
+    jmx.setJob("jmx");
+    jmx.setTaskDefinitionFamilies(new String[]{"jmxawsvpc", "jmxfargate"});
+    CloudWatchContext cloudWatchContext = new CloudWatchContext();
+    cloudWatchContext.setJmx(jmx);
+    cloudWatchContext.setClusterName(this.clusterName);
+
+    Context context = getContext();
+    context.setCloudWatchContext(cloudWatchContext);
+    return context;
+  }
 }