feat(ecs): Support non-load-balanced services

Fixes spinnaker#2912

clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/CreateServerGroupAtomicOperation.java

@@ -142,26 +142,36 @@ protected RegisterTaskDefinitionRequest makeTaskDefinitionRequest(String ecsServ
     containerEnvironment.add(new KeyValuePair().withName("CLOUD_STACK").withValue(description.getStack()));
     containerEnvironment.add(new KeyValuePair().withName("CLOUD_DETAIL").withValue(description.getFreeFormDetails()));
 
-    PortMapping portMapping = new PortMapping()
-      .withProtocol(description.getPortProtocol() != null ? description.getPortProtocol() : "tcp");
-
-    if (AWSVPC_NETWORK_MODE.equals(description.getNetworkMode())) {
-      portMapping
-        .withHostPort(description.getContainerPort())
-        .withContainerPort(description.getContainerPort());
-    } else {
-      portMapping
-        .withHostPort(0)
-        .withContainerPort(description.getContainerPort());
-    }
+    ContainerDefinition containerDefinition = new ContainerDefinition()
+      .withName(EcsServerGroupNameResolver.getEcsContainerName(newServerGroupName))
+      .withEnvironment(containerEnvironment)
+      .withCpu(description.getComputeUnits())
+      .withMemoryReservation(description.getReservedMemory())
+      .withImage(description.getDockerImageAddress());
 
     Collection<PortMapping> portMappings = new LinkedList<>();
-    portMappings.add(portMapping);
+
+    if (description.getContainerPort() != null) {
+      PortMapping portMapping = new PortMapping()
+        .withProtocol(description.getPortProtocol() != null ? description.getPortProtocol() : "tcp");
+
+      if (AWSVPC_NETWORK_MODE.equals(description.getNetworkMode())) {
+        portMapping
+          .withHostPort(description.getContainerPort())
+          .withContainerPort(description.getContainerPort());
+      } else {
+        portMapping
+          .withHostPort(0)
+          .withContainerPort(description.getContainerPort());
+      }
+
+      portMappings.add(portMapping);
+    }
 
     if (description.getServiceDiscoveryAssociations() != null) {
       for (CreateServerGroupDescription.ServiceDiscoveryAssociation config : description.getServiceDiscoveryAssociations()) {
         if (config.getContainerPort() != null && config.getContainerPort() != 0 && config.getContainerPort() != description.getContainerPort()) {
-          portMapping = new PortMapping().withProtocol("tcp");
+          PortMapping portMapping = new PortMapping().withProtocol("tcp");
           if (AWSVPC_NETWORK_MODE.equals(description.getNetworkMode())) {
             portMapping
               .withHostPort(config.getContainerPort())
@@ -176,13 +186,7 @@ protected RegisterTaskDefinitionRequest makeTaskDefinitionRequest(String ecsServ
       }
     }
 
-    ContainerDefinition containerDefinition = new ContainerDefinition()
-      .withName(EcsServerGroupNameResolver.getEcsContainerName(newServerGroupName))
-      .withEnvironment(containerEnvironment)
-      .withPortMappings(portMappings)
-      .withCpu(description.getComputeUnits())
-      .withMemoryReservation(description.getReservedMemory())
-      .withImage(description.getDockerImageAddress());
+    containerDefinition.setPortMappings(portMappings);
 
     if (!NO_IMAGE_CREDENTIALS.equals(description.getDockerImageCredentialsSecret()) &&
       description.getDockerImageCredentialsSecret() != null) {
@@ -246,8 +250,7 @@ protected RegisterTaskDefinitionRequest makeTaskDefinitionRequest(String ecsServ
 
   private Service createService(AmazonECS ecs, TaskDefinition taskDefinition, String ecsServiceRole,
                                 String newServerGroupName, Service sourceService) {
-    Collection<LoadBalancer> loadBalancers = new LinkedList<>();
-    loadBalancers.add(retrieveLoadBalancer(EcsServerGroupNameResolver.getEcsContainerName(newServerGroupName)));
+    Collection<LoadBalancer> loadBalancers = retrieveLoadBalancers(EcsServerGroupNameResolver.getEcsContainerName(newServerGroupName));
 
     Integer desiredCount = description.getCapacity().getDesired();
     if (sourceService != null &&
@@ -480,12 +483,13 @@ private DeploymentResult makeDeploymentResult(Service service) {
     return result;
   }
 
-  private LoadBalancer retrieveLoadBalancer(String containerName) {
-    LoadBalancer loadBalancer = new LoadBalancer();
-    loadBalancer.setContainerName(containerName);
-    loadBalancer.setContainerPort(description.getContainerPort());
+  private Collection<LoadBalancer> retrieveLoadBalancers(String containerName) {
+    Collection<LoadBalancer> loadBalancers = new LinkedList<>();
+    if (description.getTargetGroup() != null && !description.getTargetGroup().isEmpty()) {
+      LoadBalancer loadBalancer = new LoadBalancer();
+      loadBalancer.setContainerName(containerName);
+      loadBalancer.setContainerPort(description.getContainerPort());
 
-    if (description.getTargetGroup() != null) {
       AmazonElasticLoadBalancing loadBalancingV2 = getAmazonElasticLoadBalancingClient();
 
       DescribeTargetGroupsRequest request = new DescribeTargetGroupsRequest().withNames(description.getTargetGroup());
@@ -499,8 +503,9 @@ private LoadBalancer retrieveLoadBalancer(String containerName) {
         throw new IllegalArgumentException("There is no target group with the name " + description.getTargetGroup() + ".");
       }
 
+      loadBalancers.add(loadBalancer);
     }
-    return loadBalancer;
+    return loadBalancers;
   }
 
   private AWSApplicationAutoScaling getSourceAmazonApplicationAutoScalingClient() {

clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/deploy/validators/EcsCreateServerGroupDescriptionValidator.java

@@ -114,7 +114,7 @@ public void validate(List priorDescriptions, Object description, Errors errors)
       if (createServerGroupDescription.getContainerPort() < 0 || createServerGroupDescription.getContainerPort() > 65535) {
         rejectValue(errors, "containerPort", "invalid");
       }
-    } else {
+    } else if (createServerGroupDescription.getTargetGroup() != null && !createServerGroupDescription.getTargetGroup().isEmpty()) {
       rejectValue(errors, "containerPort", "not.nullable");
     }
 

clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/services/ContainerInformationService.java

@@ -70,35 +70,58 @@ public List<Map<String, Object>> getHealthStatus(String taskId, String serviceNa
     String healthKey = Keys.getTaskHealthKey(accountName, region, taskId);
     TaskHealth taskHealth = taskHealthCacheClient.get(healthKey);
 
-    if (service == null || taskHealth == null) {
-      List<Map<String, Object>> healthMetrics = new ArrayList<>();
+    String taskKey = Keys.getTaskKey(accountName, region, taskId);
+    Task task = taskCacheClient.get(taskKey);
+
+    List<Map<String, Object>> healthMetrics = new ArrayList<>();
 
+    // Load balancer-based health
+    if (service == null || taskHealth == null) {
       Map<String, Object> loadBalancerHealth = new HashMap<>();
       loadBalancerHealth.put("instanceId", taskId);
       loadBalancerHealth.put("state", "Unknown");
       loadBalancerHealth.put("type", "loadBalancer");
 
       healthMetrics.add(loadBalancerHealth);
-      return healthMetrics;
+    } else {
+      List<LoadBalancer> loadBalancers = service.getLoadBalancers();
+      //There should only be 1 based on AWS documentation.
+      if (loadBalancers.size() == 1) {
+        Map<String, Object> loadBalancerHealth = new HashMap<>();
+        loadBalancerHealth.put("instanceId", taskId);
+        loadBalancerHealth.put("state", taskHealth.getState());
+        loadBalancerHealth.put("type", taskHealth.getType());
+
+        healthMetrics.add(loadBalancerHealth);
+      } else if (loadBalancers.size() >= 2) {
+        throw new IllegalArgumentException("Cannot have more than 1 load balancer while checking ECS health.");
+      }
     }
 
-    List<LoadBalancer> loadBalancers = service.getLoadBalancers();
-    //There should only be 1 based on AWS documentation.
-    if (loadBalancers.size() == 1) {
+    // Task-based health
+    if (task != null) {
+      Map<String, Object> taskPlatformHealth = new HashMap<>();
+      taskPlatformHealth.put("instanceId", taskId);
+      taskPlatformHealth.put("type", "ecs");
+      taskPlatformHealth.put("healthClass", "platform");
+      taskPlatformHealth.put("state", toPlatformHealthState(task.getLastStatus()));
+      healthMetrics.add(taskPlatformHealth);
+    }
 
-      List<Map<String, Object>> healthMetrics = new ArrayList<>();
-      Map<String, Object> loadBalancerHealth = new HashMap<>();
-      loadBalancerHealth.put("instanceId", taskId);
-      loadBalancerHealth.put("state", taskHealth.getState());
-      loadBalancerHealth.put("type", taskHealth.getType());
+    return healthMetrics;
+  }
 
-      healthMetrics.add(loadBalancerHealth);
-      return healthMetrics;
-    } else if (loadBalancers.size() >= 2) {
-      throw new IllegalArgumentException("Cannot have more than 1 load balancer while checking ECS health.");
+  private String toPlatformHealthState(String ecsTaskStatus) {
+    switch (ecsTaskStatus) {
+      case "PROVISIONING":
+      case "PENDING":
+      case "ACTIVATING":
+        return "Starting";
+      case "RUNNING":
+        return "Unknown";
+      default:
+        return "Down";
     }
-    return null;
-
   }
 
   public String getClusterArn(String accountName, String region, String taskId) {

clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/CreateServerGroupAtomicOperationSpec.groovy

@@ -364,6 +364,19 @@ class CreateServerGroupAtomicOperationSpec extends CommonAtomicOperation {
     request.getContainerDefinitions().get(0).getLogConfiguration().getOptions() == logOptions
   }
 
+  def 'should allow no port mappings'() {
+    given:
+    def description = Mock(CreateServerGroupDescription)
+    description.getContainerPort() >> null
+    def operation = new CreateServerGroupAtomicOperation(description)
+
+    when:
+    def request = operation.makeTaskDefinitionRequest('arn:aws:iam::test:test-role', 'mygreatapp-stack1-details2-v0011')
+
+    then:
+    request.getContainerDefinitions().get(0).getPortMappings().isEmpty()
+  }
+
   def 'should allow using secret credentials for the docker image'() {
     given:
     def description = Mock(CreateServerGroupDescription)

clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/deploy/validators/EcsCreateServergroupDescriptionValidatorSpec.groovy

@@ -105,6 +105,19 @@ class EcsCreateServergroupDescriptionValidatorSpec extends AbstractValidatorSpec
     0 * errors.rejectValue(_, _)
   }
 
+  void 'should pass without load balancer'() {
+    given:
+    def description = getDescription()
+    description.containerPort = null
+    description.targetGroup = null
+    def errors = Mock(Errors)
+
+    when:
+    validator.validate([], description, errors)
+
+    then:
+    0 * errors.rejectValue(_, _)
+  }
 
   @Override
   AbstractECSDescription getNulledDescription() {

clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/services/ContainerInformationServiceSpec.groovy

@@ -66,12 +66,19 @@ class ContainerInformationServiceSpec extends Specification {
 
     serviceCacheClient.get(_) >> cachedService
     taskHealthCacheClient.get(_) >> cachedTaskHealth
+    taskCacheClient.get(_) >> new Task(lastStatus: 'RUNNING')
 
     def expectedHealthStatus = [
       [
         instanceId: taskId,
         state     : state,
         type      : type
+      ],
+      [
+        instanceId: taskId,
+        state     : 'Unknown',
+        type      :'ecs',
+        healthClass: 'platform'
       ]
     ]
 
@@ -142,6 +149,42 @@ class ContainerInformationServiceSpec extends Specification {
     retrievedHealthStatus == expectedHealthStatus
   }
 
+  def 'should return correct health status based on last task status'() {
+    setup:
+    def taskId = 'task-id'
+    def serviceName = 'test-service-name'
+    taskCacheClient.get(_) >> new Task(lastStatus: lastStatus)
+    def expectedHealthStatus = [
+      [
+        instanceId: taskId,
+        state     : 'Unknown',
+        type      : 'loadBalancer'
+      ],
+      [
+        instanceId: taskId,
+        state     : resultStatus,
+        type      : 'ecs',
+        healthClass: 'platform'
+      ]
+    ]
+    def retrievedHealthStatus = service.getHealthStatus(taskId, serviceName, 'test-account', 'us-west-1')
+
+    expect:
+    retrievedHealthStatus == expectedHealthStatus
+
+    where:
+    lastStatus       | resultStatus
+    'PROVISIONING'   | 'Starting'
+    'PENDING'        | 'Starting'
+    'ACTIVATING'     | 'Starting'
+    'RUNNING'        | 'Unknown'
+    'DEACTIVATING'   | 'Down'
+    'STOPPING'       | 'Down'
+    'DEPROVISIONING' | 'Down'
+    'STOPPED'        | 'Down'
+  }
+
+
   def 'should throw an exception when the service has multiple loadbalancers'() {
     given:
     def cachedService = new Service(