From 2480f0f36af727e859c0eb0ceb28da65a86f18ae Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Wed, 13 Feb 2019 11:37:32 +0200 Subject: [PATCH] feat(ecs): allow ECS to be used declaratively (#1745) Minor changes to the ECS APIs so that they can be instantiated via a declarative template (see deCDK #1618). Merge `IContainerImage` and the base `ContainerImage` into an abstract class so it's now an "enum-like" class with static methods. It also improves discoverability for all other users and more aligned with how other constructs expose union types (e.g. `lambda.Code`). Normalize the ctor of `ContainerDefinition` to "scope, id, props" so that it can be instantiated as a deCDK resource. --- .../aws-ecs/lib/base/task-definition.ts | 14 +++++--- .../aws-ecs/lib/container-definition.ts | 24 ++++++++----- .../@aws-cdk/aws-ecs/lib/container-image.ts | 34 ++++++++----------- .../aws-ecs/lib/images/asset-image.ts | 12 ++++--- .../@aws-cdk/aws-ecs/lib/images/dockerhub.ts | 5 +-- packages/@aws-cdk/aws-ecs/lib/images/ecr.ts | 5 +-- .../aws-ecs/lib/load-balanced-service-base.ts | 4 +-- packages/@aws-cdk/aws-ecs/package.json | 1 - 8 files changed, 55 insertions(+), 44 deletions(-) diff --git a/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts b/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts index d1b5afd1509cc..d1063e1c23ee9 100644 --- a/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts +++ b/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts @@ -1,6 +1,6 @@ import iam = require('@aws-cdk/aws-iam'); import cdk = require('@aws-cdk/cdk'); -import { ContainerDefinition, ContainerDefinitionProps } from '../container-definition'; +import { ContainerDefinition, ContainerDefinitionOptions } from '../container-definition'; import { CfnTaskDefinition } from '../ecs.generated'; import { isEc2Compatible, isFargateCompatible } from '../util'; @@ -225,14 +225,18 @@ export class TaskDefinition extends cdk.Construct { /** * Create a new container to this task definition */ - public addContainer(id: string, props: ContainerDefinitionProps) { - const container = new ContainerDefinition(this, id, this, props); + public addContainer(id: string, props: ContainerDefinitionOptions) { + return new ContainerDefinition(this, id, { taskDefinition: this, ...props }); + } + + /** + * (internal) Links a container to this task definition. + */ + public _linkContainer(container: ContainerDefinition) { this.containers.push(container); if (this.defaultContainer === undefined && container.essential) { this.defaultContainer = container; } - - return container; } /** diff --git a/packages/@aws-cdk/aws-ecs/lib/container-definition.ts b/packages/@aws-cdk/aws-ecs/lib/container-definition.ts index 13bb3fb321616..7789e02f87de6 100644 --- a/packages/@aws-cdk/aws-ecs/lib/container-definition.ts +++ b/packages/@aws-cdk/aws-ecs/lib/container-definition.ts @@ -1,15 +1,12 @@ import iam = require('@aws-cdk/aws-iam'); import cdk = require('@aws-cdk/cdk'); import { NetworkMode, TaskDefinition } from './base/task-definition'; -import { IContainerImage } from './container-image'; +import { ContainerImage } from './container-image'; import { CfnTaskDefinition } from './ecs.generated'; import { LinuxParameters } from './linux-parameters'; import { LogDriver } from './log-drivers/log-driver'; -/** - * Properties of a container definition - */ -export interface ContainerDefinitionProps { +export interface ContainerDefinitionOptions { /** * The image to use for a container. * @@ -17,7 +14,7 @@ export interface ContainerDefinitionProps { * repositories (repository-url/image:tag). * TODO: Update these to specify using classes of IContainerImage */ - image: IContainerImage; + image: ContainerImage; /** * The CMD value to pass to the container. @@ -173,6 +170,16 @@ export interface ContainerDefinitionProps { logging?: LogDriver; } +/** + * Properties of a container definition + */ +export interface ContainerDefinitionProps extends ContainerDefinitionOptions { + /** + * The task this container definition belongs to. + */ + taskDefinition: TaskDefinition; +} + /** * A definition for a single container in a Task */ @@ -222,14 +229,15 @@ export class ContainerDefinition extends cdk.Construct { */ private readonly links = new Array(); - constructor(scope: cdk.Construct, id: string, taskDefinition: TaskDefinition, private readonly props: ContainerDefinitionProps) { + constructor(scope: cdk.Construct, id: string, private readonly props: ContainerDefinitionProps) { super(scope, id); this.essential = props.essential !== undefined ? props.essential : true; - this.taskDefinition = taskDefinition; + this.taskDefinition = props.taskDefinition; this.memoryLimitSpecified = props.memoryLimitMiB !== undefined || props.memoryReservationMiB !== undefined; props.image.bind(this); if (props.logging) { props.logging.bind(this); } + props.taskDefinition._linkContainer(this); } /** diff --git a/packages/@aws-cdk/aws-ecs/lib/container-image.ts b/packages/@aws-cdk/aws-ecs/lib/container-image.ts index 6acf0fb894339..efdb3f07a3f65 100644 --- a/packages/@aws-cdk/aws-ecs/lib/container-image.ts +++ b/packages/@aws-cdk/aws-ecs/lib/container-image.ts @@ -2,29 +2,11 @@ import ecr = require('@aws-cdk/aws-ecr'); import cdk = require('@aws-cdk/cdk'); import { ContainerDefinition } from './container-definition'; -import { AssetImage, AssetImageProps } from './images/asset-image'; -import { DockerHubImage } from './images/dockerhub'; -import { EcrImage } from './images/ecr'; - -/** - * A container image - */ -export interface IContainerImage { - /** - * Name of the image - */ - readonly imageName: string; - - /** - * Called when the image is used by a ContainerDefinition - */ - bind(containerDefinition: ContainerDefinition): void; -} /** * Constructs for types of container images */ -export class ContainerImage { +export abstract class ContainerImage { /** * Reference an image on DockerHub */ @@ -45,4 +27,18 @@ export class ContainerImage { public static fromAsset(scope: cdk.Construct, id: string, props: AssetImageProps) { return new AssetImage(scope, id, props); } + + /** + * Name of the image + */ + public abstract readonly imageName: string; + + /** + * Called when the image is used by a ContainerDefinition + */ + public abstract bind(containerDefinition: ContainerDefinition): void; } + +import { AssetImage, AssetImageProps } from './images/asset-image'; +import { DockerHubImage } from './images/dockerhub'; +import { EcrImage } from './images/ecr'; diff --git a/packages/@aws-cdk/aws-ecs/lib/images/asset-image.ts b/packages/@aws-cdk/aws-ecs/lib/images/asset-image.ts index 10687aa342765..03ed2eb925914 100644 --- a/packages/@aws-cdk/aws-ecs/lib/images/asset-image.ts +++ b/packages/@aws-cdk/aws-ecs/lib/images/asset-image.ts @@ -1,7 +1,7 @@ import { DockerImageAsset } from '@aws-cdk/assets-docker'; import cdk = require('@aws-cdk/cdk'); import { ContainerDefinition } from '../container-definition'; -import { IContainerImage } from '../container-image'; +import { ContainerImage } from '../container-image'; export interface AssetImageProps { /** @@ -13,16 +13,18 @@ export interface AssetImageProps { /** * An image that will be built at synthesis time */ -export class AssetImage extends DockerImageAsset implements IContainerImage { +export class AssetImage extends ContainerImage { + private readonly asset: DockerImageAsset; constructor(scope: cdk.Construct, id: string, props: AssetImageProps) { - super(scope, id, { directory: props.directory }); + super(); + this.asset = new DockerImageAsset(scope, id, { directory: props.directory }); } public bind(containerDefinition: ContainerDefinition): void { - this.repository.grantPull(containerDefinition.taskDefinition.obtainExecutionRole()); + this.asset.repository.grantPull(containerDefinition.taskDefinition.obtainExecutionRole()); } public get imageName() { - return this.imageUri; + return this.asset.imageUri; } } diff --git a/packages/@aws-cdk/aws-ecs/lib/images/dockerhub.ts b/packages/@aws-cdk/aws-ecs/lib/images/dockerhub.ts index 2b4d04a9f876b..4157807e19504 100644 --- a/packages/@aws-cdk/aws-ecs/lib/images/dockerhub.ts +++ b/packages/@aws-cdk/aws-ecs/lib/images/dockerhub.ts @@ -1,11 +1,12 @@ import { ContainerDefinition } from "../container-definition"; -import { IContainerImage } from "../container-image"; +import { ContainerImage } from "../container-image"; /** * A DockerHub image */ -export class DockerHubImage implements IContainerImage { +export class DockerHubImage extends ContainerImage { constructor(public readonly imageName: string) { + super(); } public bind(_containerDefinition: ContainerDefinition): void { diff --git a/packages/@aws-cdk/aws-ecs/lib/images/ecr.ts b/packages/@aws-cdk/aws-ecs/lib/images/ecr.ts index 424f23fe04d0a..d28dc2dca05e5 100644 --- a/packages/@aws-cdk/aws-ecs/lib/images/ecr.ts +++ b/packages/@aws-cdk/aws-ecs/lib/images/ecr.ts @@ -1,15 +1,16 @@ import ecr = require('@aws-cdk/aws-ecr'); import { ContainerDefinition } from '../container-definition'; -import { IContainerImage } from '../container-image'; +import { ContainerImage } from '../container-image'; /** * An image from an ECR repository */ -export class EcrImage implements IContainerImage { +export class EcrImage extends ContainerImage { public readonly imageName: string; private readonly repository: ecr.IRepository; constructor(repository: ecr.IRepository, tag: string) { + super(); this.imageName = repository.repositoryUriForTag(tag); this.repository = repository; } diff --git a/packages/@aws-cdk/aws-ecs/lib/load-balanced-service-base.ts b/packages/@aws-cdk/aws-ecs/lib/load-balanced-service-base.ts index ab521e8deec1a..d5b50d1fff5dc 100644 --- a/packages/@aws-cdk/aws-ecs/lib/load-balanced-service-base.ts +++ b/packages/@aws-cdk/aws-ecs/lib/load-balanced-service-base.ts @@ -3,7 +3,7 @@ import elbv2 = require('@aws-cdk/aws-elasticloadbalancingv2'); import cdk = require('@aws-cdk/cdk'); import { BaseService } from './base/base-service'; import { ICluster } from './cluster'; -import { IContainerImage } from './container-image'; +import { ContainerImage } from './container-image'; export enum LoadBalancerType { Application, @@ -19,7 +19,7 @@ export interface LoadBalancedServiceBaseProps { /** * The image to start. */ - image: IContainerImage; + image: ContainerImage; /** * The container port of the application load balancer attached to your Fargate service. Corresponds to container port mapping. diff --git a/packages/@aws-cdk/aws-ecs/package.json b/packages/@aws-cdk/aws-ecs/package.json index 2bf92ec06973f..081aa23b6b855 100644 --- a/packages/@aws-cdk/aws-ecs/package.json +++ b/packages/@aws-cdk/aws-ecs/package.json @@ -106,7 +106,6 @@ "exclude": [ "resource-attribute:@aws-cdk/aws-ecs.ICluster.clusterArn", "construct-ctor:@aws-cdk/aws-ecs.BaseService.", - "construct-ctor:@aws-cdk/aws-ecs.ContainerDefinition.", "construct-ctor:@aws-cdk/aws-ecs.LoadBalancedFargateServiceApplet..params[0]" ] }