Refactor enableServiceDiscovery to base class

aws · Apr 2, 2019 · 92a3072 · 92a3072
1 parent 08e51e4
commit 92a3072
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 112 deletions.
diff --git a/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts b/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts
@@ -132,6 +132,11 @@ export abstract class BaseService extends cdk.Construct
     this.serviceName = this.resource.serviceName;
     this.clusterName = clusterName;
     this.cluster = props.cluster;
+
+    if (props.serviceDiscoveryOptions) {
+      this.enableServiceDiscovery(props.serviceDiscoveryOptions);
+    }
+
   }
 
   /**
@@ -179,14 +184,6 @@ export abstract class BaseService extends cdk.Construct
     });
   }
 
-  /**
-   * Associate Service Discovery (Cloud Map) service
-   */
-  public addServiceRegistry(registry: ServiceRegistry) {
-    const sr = this.renderServiceRegistry(registry);
-    this.serviceRegistries.push(sr);
-  }
-
   /**
    * Return the given named metric for this Service
    */
@@ -264,6 +261,74 @@ export abstract class BaseService extends cdk.Construct
       })
     });
   }
+
+  /**
+   * Associate Service Discovery (Cloud Map) service
+   */
+  private addServiceRegistry(registry: ServiceRegistry) {
+    const sr = this.renderServiceRegistry(registry);
+    this.serviceRegistries.push(sr);
+  }
+
+  /**
+   * Enable CloudMap service discovery for the service
+   */
+  private enableServiceDiscovery(options: ServiceDiscoveryOptions): cloudmap.Service {
+    const sdNamespace = this.cluster.defaultNamespace();
+    if (sdNamespace === undefined) {
+      throw new Error("Cannot enable service discovery if a Cloudmap Namespace has not been created in the cluster.");
+    }
+
+    // Determine DNS type based on network mode
+    const networkMode = this.taskDefinition.networkMode;
+    if (networkMode === NetworkMode.None) {
+      throw new Error("Cannot use a service discovery if NetworkMode is None. Use Bridge, Host or AwsVpc instead.");
+    }
+
+    // Bridge or host network mode requires SRV records
+    let dnsRecordType = options.dnsRecordType;
+
+    if (networkMode === NetworkMode.Bridge || networkMode === NetworkMode.Host) {
+      if (dnsRecordType ===  undefined) {
+        dnsRecordType = cloudmap.DnsRecordType.SRV;
+      }
+      if (dnsRecordType !== cloudmap.DnsRecordType.SRV) {
+        throw new Error("SRV records must be used when network mode is Bridge or Host.");
+      }
+    }
+
+    // Default DNS record type for AwsVpc network mode is A Records
+    if (networkMode === NetworkMode.AwsVpc) {
+      if (dnsRecordType ===  undefined) {
+        dnsRecordType = cloudmap.DnsRecordType.A;
+      }
+    }
+
+    // If the task definition that your service task specifies uses the AWSVPC network mode and a type SRV DNS record is
+    // used, you must specify a containerName and containerPort combination
+    const containerName = dnsRecordType === cloudmap.DnsRecordType.SRV ? this.taskDefinition.defaultContainer!.node.id : undefined;
+    const containerPort = dnsRecordType === cloudmap.DnsRecordType.SRV ? this.taskDefinition.defaultContainer!.containerPort : undefined;
+
+    const cloudmapService = new cloudmap.Service(this, 'CloudmapService', {
+      namespace: sdNamespace,
+      name: options.name,
+      dnsRecordType: dnsRecordType!,
+      customHealthCheck: { failureThreshold: options.failureThreshold || 1 }
+    });
+
+    const serviceArn = cloudmapService.serviceArn;
+
+    // add Cloudmap service to the ECS Service's serviceRegistry
+    this.addServiceRegistry({
+      arn: serviceArn,
+      containerName,
+      containerPort
+    });
+
+    this.cloudmapService = cloudmapService;
+
+    return cloudmapService;
+  }
 }
 
 /**

diff --git a/packages/@aws-cdk/aws-ecs/lib/ec2/ec2-service.ts b/packages/@aws-cdk/aws-ecs/lib/ec2/ec2-service.ts
@@ -1,9 +1,8 @@
 import cloudwatch = require ('@aws-cdk/aws-cloudwatch');
 import ec2 = require('@aws-cdk/aws-ec2');
 import elb = require('@aws-cdk/aws-elasticloadbalancing');
-import cloudmap = require('@aws-cdk/aws-servicediscovery');
 import cdk = require('@aws-cdk/cdk');
-import { BaseService, BaseServiceProps, ServiceDiscoveryOptions } from '../base/base-service';
+import { BaseService, BaseServiceProps } from '../base/base-service';
 import { NetworkMode, TaskDefinition } from '../base/task-definition';
 import { CfnService } from '../ecs.generated';
 import { isEc2Compatible } from '../util';
@@ -106,10 +105,6 @@ export class Ec2Service extends BaseService implements elb.ILoadBalancerTarget {
       this.constraints.push({ type: 'distinctInstance' });
     }
 
-    if (props.serviceDiscoveryOptions) {
-      this.enableServiceDiscovery(props.serviceDiscoveryOptions);
-    }
-
     if (!this.taskDefinition.defaultContainer) {
       throw new Error('A TaskDefinition must have at least one essential container');
     }
@@ -236,65 +231,6 @@ export class Ec2Service extends BaseService implements elb.ILoadBalancerTarget {
     return ret;
   }
 
-  /**
-   * Enable CloudMap service discovery for the service
-   */
-  private enableServiceDiscovery(options: ServiceDiscoveryOptions): cloudmap.Service {
-    const sdNamespace = this.cluster.defaultNamespace();
-    if (sdNamespace === undefined) {
-      throw new Error("Cannot enable service discovery if a Cloudmap Namespace has not been created in the cluster.");
-    }
-
-    // Determine DNS type based on network mode
-    const networkMode = this.taskDefinition.networkMode;
-    if (networkMode === NetworkMode.None) {
-      throw new Error("Cannot use a service discovery if NetworkMode is None. Use Bridge, Host or AwsVpc instead.");
-    }
-
-    // Bridge or host network mode requires SRV records
-    let dnsRecordType = options.dnsRecordType;
-
-    if (networkMode === NetworkMode.Bridge || networkMode === NetworkMode.Host) {
-      if (dnsRecordType ===  undefined) {
-        dnsRecordType = cloudmap.DnsRecordType.SRV;
-      }
-      if (dnsRecordType !== cloudmap.DnsRecordType.SRV) {
-        throw new Error("SRV records must be used when network mode is Bridge or Host.");
-      }
-    }
-
-    // Default DNS record type for AwsVpc network mode is A Records
-    if (networkMode === NetworkMode.AwsVpc) {
-      if (dnsRecordType ===  undefined) {
-        dnsRecordType = cloudmap.DnsRecordType.A;
-      }
-    }
-
-    // If the task definition that your service task specifies uses the AWSVPC network mode and a type SRV DNS record is
-    // used, you must specify a containerName and containerPort combination
-    const containerName = dnsRecordType === cloudmap.DnsRecordType.SRV ? this.taskDefinition.defaultContainer!.node.id : undefined;
-    const containerPort = dnsRecordType === cloudmap.DnsRecordType.SRV ? this.taskDefinition.defaultContainer!.containerPort : undefined;
-
-    const cloudmapService = new cloudmap.Service(this, 'CloudmapService', {
-      namespace: sdNamespace,
-      name: options.name,
-      dnsRecordType: dnsRecordType!,
-      customHealthCheck: { failureThreshold: options.failureThreshold || 1 }
-    });
-
-    const serviceArn = cloudmapService.serviceArn;
-
-    // add Cloudmap service to the ECS Service's serviceRegistry
-    this.addServiceRegistry({
-      arn: serviceArn,
-      containerName,
-      containerPort
-    });
-
-    this.cloudmapService = cloudmapService;
-
-    return cloudmapService;
-  }
 
 }
 

diff --git a/packages/@aws-cdk/aws-ecs/lib/fargate/fargate-service.ts b/packages/@aws-cdk/aws-ecs/lib/fargate/fargate-service.ts
@@ -1,7 +1,6 @@
 import ec2 = require('@aws-cdk/aws-ec2');
-import cloudmap = require('@aws-cdk/aws-servicediscovery');
 import cdk = require('@aws-cdk/cdk');
-import { BaseService, BaseServiceProps, ServiceDiscoveryOptions } from '../base/base-service';
+import { BaseService, BaseServiceProps } from '../base/base-service';
 import { TaskDefinition } from '../base/task-definition';
 import { isFargateCompatible } from '../util';
 
@@ -67,47 +66,10 @@ export class FargateService extends BaseService {
 
     this.configureAwsVpcNetworking(props.cluster.vpc, props.assignPublicIp, props.vpcSubnets, props.securityGroup);
 
-    if (props.serviceDiscoveryOptions) {
-      this.enableServiceDiscovery(props.serviceDiscoveryOptions);
-    }
-
     if (!props.taskDefinition.defaultContainer) {
       throw new Error('A TaskDefinition must have at least one essential container');
     }
   }
-
-  private enableServiceDiscovery(options: ServiceDiscoveryOptions): cloudmap.Service {
-    const sdNamespace = this.cluster.defaultNamespace();
-    if (sdNamespace === undefined) {
-      throw new Error("Cannot enable service discovery if a Cloudmap Namespace has not been created in the cluster.");
-    }
-
-    const dnsRecordType = options.dnsRecordType === undefined
-     ? cloudmap.DnsRecordType.A : options.dnsRecordType;
-
-    // If the task definition that your service task specifies uses the awsvpc network mode and a type SRV DNS record
-    // is used, you must specify a containerName and containerPort combination
-    const containerName = dnsRecordType === cloudmap.DnsRecordType.SRV ? this.taskDefinition.defaultContainer!.node.id : undefined;
-    const containerPort = dnsRecordType === cloudmap.DnsRecordType.SRV ? this.taskDefinition.defaultContainer!.containerPort : undefined;
-
-    const cloudmapService = new cloudmap.Service(this, 'CloudmapService', {
-      namespace: sdNamespace,
-      name: options.name,
-      dnsRecordType: dnsRecordType!,
-      customHealthCheck: { failureThreshold: options.failureThreshold || 1 }
-    });
-
-    const serviceArn = cloudmapService.serviceArn;
-
-    // add Cloudmap service to the ECS Service's serviceRegistry
-    this.addServiceRegistry({
-      arn: serviceArn,
-      containerName,
-      containerPort
-    });
-
-    return cloudmapService;
-  }
 }
 
 /**