WIP

packages/@aws-cdk/aws-ecs/lib/base/base-service.ts

@@ -6,13 +6,19 @@ import iam = require('@aws-cdk/aws-iam');
 import cloudmap = require ('@aws-cdk/aws-servicediscovery');
 import cdk = require('@aws-cdk/cdk');
 import { NetworkMode, TaskDefinition } from '../base/task-definition';
+import { ICluster } from '../cluster';
 import { CfnService } from '../ecs.generated';
 import { ScalableTaskCount } from './scalable-task-count';
 
 /**
  * Basic service properties
  */
 export interface BaseServiceProps {
+  /**
+   * Cluster where service will be deployed
+   */
+  cluster: ICluster;
+
   /**
    * Number of desired copies of running tasks
    *
@@ -64,11 +70,6 @@ export abstract class BaseService extends cdk.Construct
    */
   public readonly connections: ec2.Connections = new ec2.Connections();
 
-  /**
-   * Cloudmap Service to enable service discovery
-   */
-  protected cloudmapService: cloudmap.Service;
-
   /**
    * ARN of this service
    */
@@ -89,6 +90,8 @@ export abstract class BaseService extends cdk.Construct
    */
   public readonly taskDefinition: TaskDefinition;
 
+  protected cloudmapService: cloudmap.Service;
+  protected cluster: ICluster;
   protected loadBalancers = new Array<CfnService.LoadBalancerProperty>();
   protected networkConfiguration?: CfnService.NetworkConfigurationProperty;
   protected serviceRegistries = new Array<CfnService.ServiceRegistryProperty>();
@@ -122,6 +125,7 @@ export abstract class BaseService extends cdk.Construct
     this.serviceArn = this.resource.serviceArn;
     this.serviceName = this.resource.serviceName;
     this.clusterName = clusterName;
+    this.cluster = props.cluster;
   }
 
   /**
@@ -250,8 +254,20 @@ const EPHEMERAL_PORT_RANGE = new ec2.TcpPortRange(32768, 65535);
  * Options for enabling service discovery on an ECS service
  */
 export interface ServiceDiscoveryOptions {
-  namespace: cloudmap.INamespace,
+  /**
+   * Name of the cloudmap service to attach to the ECS Service
+   */
   name?: string,
-  dnsType?: cloudmap.DnsRecordType,
+  /**
+   * The DNS type of the record that you want AWS Cloud Map to create. Supported record types
+   * include A, AAAA, A_AAAA, CNAME, and SRV.
+   */
+  dnsRecordType?: cloudmap.DnsRecordType,
+
+  /**
+   * The number of 30-second intervals that you want Cloud Map to wait after receiving an
+   * UpdateInstanceCustomHealthStatus request before it changes the health status of a service instance.
+   * NOTE: This is used for a HealthCheckCustomConfig, since HealthCheckConfig is only for PublicDnsNamespaces and PrivateDnsNamespaces or HttpNamespaces would be more conventional.
+   */
   failureThreshold?: number,
 }

packages/@aws-cdk/aws-ecs/lib/cluster.ts

@@ -58,7 +58,7 @@ export class Cluster extends cdk.Construct implements ICluster {
   /**
    * The service discovery namespace created in this cluster
    */
-  public serviceDiscoveryNamespace: cloudmap.INamespace; // FIXME should only be populated in constructor?
+  private _serviceDiscoveryNamespace?: cloudmap.INamespace;
 
   /**
    * Whether the cluster has EC2 capacity associated with it
@@ -93,11 +93,15 @@ export class Cluster extends cdk.Construct implements ICluster {
       vpc,
     });
 
-    this.serviceDiscoveryNamespace = sdNamespace;
+    this._serviceDiscoveryNamespace = sdNamespace;
 
     return sdNamespace;
   }
 
+  public serviceDiscoveryNamespace() : cloudmap.INamespace {
+    return this._serviceDiscoveryNamespace;
+  }
+
   /**
    * Add a default-configured AutoScalingGroup running the ECS-optimized AMI to this Cluster
    *
@@ -282,9 +286,9 @@ export interface ICluster extends cdk.IConstruct {
   readonly hasEc2Capacity: boolean;
 
   /**
-   * Cloudmap namespace created in the cluster
+   * Getter for Cloudmap namespace created in the cluster
    */
-  readonly serviceDiscoveryNamespace: cloudmap.INamespace;
+  serviceDiscoveryNamespace(): cloudmap.INamespace;
 
   /**
    * Export the Cluster
@@ -353,7 +357,7 @@ class ImportedCluster extends cdk.Construct implements ICluster {
   /**
    * Cloudmap namespace created in the cluster
    */
-  public serviceDiscoveryNamespace: cloudmap.INamespace;
+  private _serviceDiscoveryNamespace: cloudmap.INamespace;
 
   /**
    * Whether the cluster has EC2 capacity
@@ -378,6 +382,9 @@ class ImportedCluster extends cdk.Construct implements ICluster {
       i++;
     }
   }
+  public serviceDiscoveryNamespace() : cloudmap.INamespace {
+    return this._serviceDiscoveryNamespace;
+  }
 
   public export() {
     return this.props;

packages/@aws-cdk/aws-ecs/lib/ec2/ec2-service.ts

@@ -5,19 +5,13 @@ import cloudmap = require ('@aws-cdk/aws-servicediscovery');
 import cdk = require('@aws-cdk/cdk');
 import { BaseService, BaseServiceProps, ServiceDiscoveryOptions } from '../base/base-service';
 import { NetworkMode, TaskDefinition } from '../base/task-definition';
-import { ICluster } from '../cluster';
 import { CfnService } from '../ecs.generated';
 import { isEc2Compatible } from '../util';
 
 /**
  * Properties to define an ECS service
  */
 export interface Ec2ServiceProps extends BaseServiceProps {
-  /**
-   * Cluster where service will be deployed
-   */
-  cluster: ICluster;
-
   /**
    * Task Definition used for running tasks in the service
    */
@@ -71,7 +65,6 @@ export class Ec2Service extends BaseService implements elb.ILoadBalancerTarget {
   private readonly constraints: CfnService.PlacementConstraintProperty[];
   private readonly strategies: CfnService.PlacementStrategyProperty[];
   private readonly daemon: boolean;
-  private readonly cluster: ICluster;
 
   constructor(scope: cdk.Construct, id: string, props: Ec2ServiceProps) {
     if (props.daemon && props.desiredCount !== undefined) {
@@ -96,7 +89,6 @@ export class Ec2Service extends BaseService implements elb.ILoadBalancerTarget {
       schedulingStrategy: props.daemon ? 'DAEMON' : 'REPLICA',
     }, props.cluster.clusterName, props.taskDefinition);
 
-    this.cluster = props.cluster;
     this.clusterName = props.cluster.clusterName;
     this.constraints = [];
     this.strategies = [];
@@ -203,7 +195,8 @@ export class Ec2Service extends BaseService implements elb.ILoadBalancerTarget {
    * Enable CloudMap service discovery for the service
    */
   public enableServiceDiscovery(options: ServiceDiscoveryOptions): cloudmap.Service {
-    if (this.cluster.serviceDiscoveryNamespace === undefined) {
+    const sdNamespace = this.cluster.serviceDiscoveryNamespace;
+    if (sdNamespace === undefined) {
       throw new Error("Cannot enable service discovery if a Cloudmap Namespace has not been created in the cluster.");
     }
 
@@ -213,25 +206,26 @@ export class Ec2Service extends BaseService implements elb.ILoadBalancerTarget {
     if (networkMode === NetworkMode.None) {
       throw new Error("Cannot use a service discovery if NetworkMode is None. Use Host or AwsVpc instead.");
     }
+
     // Bridge or host network mode requires SRV records
-    let dnsType = options.dnsType;
+    let dnsRecordType = options.dnsRecordType;
 
     if (networkMode === NetworkMode.Bridge || networkMode === NetworkMode.Host) {
-      if (dnsType !== cloudmap.DnsRecordType.Srv) {
+      if (dnsRecordType !== cloudmap.DnsRecordType.Srv) {
         throw new Error("Cannot use a service discovery if NetworkMode is None. Use Host or AwsVpc instead.");
       }
-      dnsType = cloudmap.DnsRecordType.Srv;
+      dnsRecordType = cloudmap.DnsRecordType.Srv;
     }
 
-    if (networkMode === NetworkMode.AwsVpc && options.dnsType === undefined ) {
+    if (networkMode === NetworkMode.AwsVpc && options.dnsRecordType === undefined ) {
       // ECS does not currently support IPv6, so AAAA records not possible
-      dnsType = cloudmap.DnsRecordType.A;
+      dnsRecordType = cloudmap.DnsRecordType.A;
     }
 
     const cloudmapService = new cloudmap.Service(this, 'CloudmapService', {
-      namespace: options.namespace,
+      namespace: sdNamespace,
       name: options.name,
-      dnsType: dnsType!,
+      dnsRecordType: dnsRecordType!,
       healthCheckCustomConfig: { failureThreshold: options.failureThreshold || 1 }
     });
 

packages/@aws-cdk/aws-ecs/lib/fargate/fargate-service.ts

@@ -2,18 +2,12 @@ import ec2 = require('@aws-cdk/aws-ec2');
 import cdk = require('@aws-cdk/cdk');
 import { BaseService, BaseServiceProps } from '../base/base-service';
 import { TaskDefinition } from '../base/task-definition';
-import { ICluster } from '../cluster';
 import { isFargateCompatible } from '../util';
 
 /**
  * Properties to define a Fargate service
  */
 export interface FargateServiceProps extends BaseServiceProps {
-  /**
-   * Cluster where service will be deployed
-   */
-  cluster: ICluster; // should be required? do we assume 'default' exists?
-
   /**
    * Task Definition used for running tasks in the service
    */

packages/@aws-cdk/aws-ecs/test/ec2/integ.sd-bridge-nw.ts

@@ -63,11 +63,9 @@ const frontendService = new ecs.Ec2Service(stack, "FrontendService", {
 // NOTE: Passing in the cluster's serviceDiscoveryNamespaceId is a little clunky, would have liked to have
 // enableServiceDiscovery not take any argument and look it up on the cluster, but that would have involved adding
 // cluster as a member on service which may have called for a breaking change. This is a also little more explicit
-const namespace = cluster.serviceDiscoveryNamespace;
 
 frontendService.enableServiceDiscovery({
   name: "frontend",
-  namespace,
 });
 
 // apiService.enableServiceDiscovery({