Skip to content

Commit

Permalink
fix(batch): fixing build / tests
Browse files Browse the repository at this point in the history
  • Loading branch information
andrestone committed Mar 3, 2020
1 parent d9c7f42 commit cea5eb1
Show file tree
Hide file tree
Showing 5 changed files with 247 additions and 54 deletions.
12 changes: 8 additions & 4 deletions packages/@aws-cdk/aws-batch/lib/compute-environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,15 @@ export interface ComputeResources {
readonly launchTemplate?: LaunchTemplateSpecification;

/**
* The IAM role applied to EC2 resources in the compute environment.
* The Amazon ECS instance profile applied to Amazon EC2 instances in a compute environment. You can specify
* the short name or full Amazon Resource Name (ARN) of an instance profile. For example, ecsInstanceRole or
* arn:aws:iam::<aws_account_id>:instance-profile/ecsInstanceRole . For more information, see Amazon ECS
* Instance Role in the AWS Batch User Guide.
*
* @default a new role will be created.
* @link https://docs.aws.amazon.com/batch/latest/userguide/instance_IAM_role.html
*/
readonly instanceRole?: iam.IRole;
readonly instanceRole?: string;

/**
* The types of EC2 instances that may be launched in the compute environment. You can specify instance
Expand Down Expand Up @@ -342,8 +346,8 @@ export class ComputeEnvironment extends Resource implements IComputeEnvironment
desiredvCpus: props.computeResources.desiredvCpus,
ec2KeyPair: props.computeResources.ec2KeyPair,
imageId: props.computeResources.image && props.computeResources.image.getImage(this).imageId,
instanceRole: props.computeResources.instanceRole
? props.computeResources.instanceRole.roleArn
instanceRole: props.computeResources.instanceRole
? props.computeResources.instanceRole
: new iam.CfnInstanceProfile(this, 'Instance-Profile', {
roles: [ new iam.Role(this, 'Ecs-Instance-Role', {
assumedBy: new iam.ServicePrincipal('ec2.amazonaws.com'),
Expand Down
57 changes: 32 additions & 25 deletions packages/@aws-cdk/aws-batch/test/compute-environment.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { expect, haveResource, haveResourceLike, ResourcePart } from '@aws-cdk/assert';
import {expect, haveResource, haveResourceLike, ResourcePart} from '@aws-cdk/assert';
import '@aws-cdk/assert/jest';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as ecs from '@aws-cdk/aws-ecs';
import * as iam from '@aws-cdk/aws-iam';
import * as cdk from '@aws-cdk/core';
import { throws } from 'assert';
import * as batch from '../lib';
import {throws} from 'assert';
import * as batch from "../lib";

describe('Batch Compute Evironment', () => {
let expectedManagedDefaultComputeProps: any;
Expand Down Expand Up @@ -34,7 +34,7 @@ describe('Batch Compute Evironment', () => {
AllocationStrategy: batch.AllocationStrategy.BEST_FIT,
InstanceRole: {
'Fn::GetAtt': [
'testcomputeenvResourceInstanceRole7FD819B9',
'testcomputeenvInstanceProfileCBD87EAB',
'Arn'
]
},
Expand All @@ -59,37 +59,39 @@ describe('Batch Compute Evironment', () => {
});

describe('when validating props', () => {
test('should deny setting compute resources when using type managed', () => {
test('should deny setting compute resources when using type unmanaged', () => {
// THEN
throws(() => {
// WHEN
new batch.ComputeEnvironment(stack, 'test-compute-env', {
managed: false,
computeResources: {
vpc,
vpc
},
});
});
});

test('should deny if creating an unmanged environment with no provided compute resource props', () => {
test('should deny if creating a managed environment with no provided compute resource props', () => {
// THEN
throws(() => {
// WHEN
new batch.ComputeEnvironment(stack, 'test-compute-env', {
managed: false,
managed: true,
});
});
});
});

describe('using spot resources', () => {
test('should provide a spotfleet role if one is not given', () => {
test('should provide a spot fleet role if one is not given and allocationStrategy is BEST_FIT', () => {
// WHEN
new batch.ComputeEnvironment(stack, 'test-compute-env', {
managed: false,
managed: true,
computeResources: {
type: batch.ComputeResourceType.SPOT,
vpc,
allocationStrategy: batch.AllocationStrategy.BEST_FIT,
vpc
},
});

Expand Down Expand Up @@ -124,7 +126,7 @@ describe('Batch Compute Evironment', () => {
throws(() => {
// WHEN
new batch.ComputeEnvironment(stack, 'test-compute-env', {
managed: false,
managed: true,
computeResources: {
vpc,
type: batch.ComputeResourceType.SPOT,
Expand All @@ -139,7 +141,7 @@ describe('Batch Compute Evironment', () => {
throws(() => {
// WHEN
new batch.ComputeEnvironment(stack, 'test-compute-env', {
managed: false,
managed: true,
computeResources: {
vpc,
type: batch.ComputeResourceType.SPOT,
Expand All @@ -155,9 +157,9 @@ describe('Batch Compute Evironment', () => {
test('renders the correct cloudformation properties', () => {
// WHEN
const props = {
allocationStrategy: batch.AllocationStrategy.BEST_FIT,
computeEnvironmentName: 'my-test-compute-env',
computeResources: {
allocationStrategy: batch.AllocationStrategy.BEST_FIT,
vpc,
computeResourcesTags: new cdk.Tag('foo', 'bar'),
desiredvCpus: 1,
Expand All @@ -166,9 +168,14 @@ describe('Batch Compute Evironment', () => {
generation: ec2.AmazonLinuxGeneration.AMAZON_LINUX_2,
hardwareType: ecs.AmiHardwareType.STANDARD,
}),
instanceRole: new iam.Role(stack, 'test-compute-env-instance-role', {
assumedBy: new iam.ServicePrincipal('batch.amazonaws.com'),
}),
instanceRole: new iam.CfnInstanceProfile(stack, 'Instance-Profile', {
roles: [ new iam.Role(stack, 'Ecs-Instance-Role', {
assumedBy: new iam.ServicePrincipal('ec2.amazonaws.com'),
managedPolicies: [
iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AmazonEC2ContainerServiceforEC2Role')
]
}).roleName]
}).attrArn,
instanceTypes: [
ec2.InstanceType.of(ec2.InstanceClass.T2, ec2.InstanceSize.MICRO),
],
Expand All @@ -186,7 +193,7 @@ describe('Batch Compute Evironment', () => {
},
} as batch.ComputeResources,
enabled: false,
managed: false,
managed: true,
};

new batch.ComputeEnvironment(stack, 'test-compute-env', props);
Expand All @@ -211,7 +218,7 @@ describe('Batch Compute Evironment', () => {
},
InstanceRole: {
'Fn::GetAtt': [
props.computeResources.instanceRole ? `${props.computeResources.instanceRole.node.uniqueId}F3B86D94` : '',
props.computeResources.instanceRole ? "InstanceProfile" : '',
'Arn'
]
},
Expand Down Expand Up @@ -251,7 +258,7 @@ describe('Batch Compute Evironment', () => {
test('should default to a best_fit strategy', () => {
// WHEN
new batch.ComputeEnvironment(stack, 'test-compute-env', {
managed: false,
managed: true,
computeResources: {
vpc,
},
Expand Down Expand Up @@ -303,7 +310,7 @@ describe('Batch Compute Evironment', () => {
test('should default to 0', () => {
// WHEN
new batch.ComputeEnvironment(stack, 'test-compute-env', {
managed: false,
managed: true,
computeResources: {
vpc,
},
Expand All @@ -323,7 +330,7 @@ describe('Batch Compute Evironment', () => {
test('should default to 256', () => {
// WHEN
new batch.ComputeEnvironment(stack, 'test-compute-env', {
managed: false,
managed: true,
computeResources: {
vpc,
},
Expand All @@ -342,7 +349,7 @@ describe('Batch Compute Evironment', () => {
test('should generate a role for me', () => {
// WHEN
new batch.ComputeEnvironment(stack, 'test-compute-env', {
managed: false,
managed: true,
computeResources: {
vpc,
},
Expand All @@ -358,7 +365,7 @@ describe('Batch Compute Evironment', () => {
test('should default to optimal matching', () => {
// WHEN
new batch.ComputeEnvironment(stack, 'test-compute-env', {
managed: false,
managed: true,
computeResources: {
vpc,
},
Expand All @@ -377,7 +384,7 @@ describe('Batch Compute Evironment', () => {
test('should default to EC2', () => {
// WHEN
new batch.ComputeEnvironment(stack, 'test-compute-env', {
managed: false,
managed: true,
computeResources: {
vpc,
},
Expand Down
Loading

0 comments on commit cea5eb1

Please sign in to comment.