Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aws-batch-alpha: unable to import FargateComputeEnvironment with from_fargate_compute_environment_arn #25979

Closed
roketworks opened this issue Jun 14, 2023 · 3 comments · Fixed by #25985
Labels
@aws-cdk/aws-batch Related to AWS Batch bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1

Comments

@roketworks
Copy link
Contributor

Describe the bug

When importing a fargate compute environment that is created outside of the stack the following error occurs:

jsii.errors.JavaScriptError:
  @jsii/kernel.RuntimeError: TypeError: Cannot read properties of undefined (reading 'vpcId')
      at Kernel._ensureSync (/private/var/folders/01/8c7x7kn954573yk8__rn5d9m0000gn/T/tmpduoq304b/lib/program.js:10369:27)
      at Kernel.sinvoke (/private/var/folders/01/8c7x7kn954573yk8__rn5d9m0000gn/T/tmpduoq304b/lib/program.js:9790:34)
      at KernelHost.processRequest (/private/var/folders/01/8c7x7kn954573yk8__rn5d9m0000gn/T/tmpduoq304b/lib/program.js:11544:36)
      at KernelHost.run (/private/var/folders/01/8c7x7kn954573yk8__rn5d9m0000gn/T/tmpduoq304b/lib/program.js:11504:22)
      at Immediate._onImmediate (/private/var/folders/01/8c7x7kn954573yk8__rn5d9m0000gn/T/tmpduoq304b/lib/program.js:11505:46)
      at process.processImmediate (node:internal/timers:478:21)

Expected Behavior

The resource gets imported and can be referenced

Current Behavior

The error occurs and unable reference resources

Reproduction Steps

  • Create 2 batch compute environments, one fargate and one ECS.
# Works
ecs_compute_arn = 'arn:aws:batch:us-east-1:xxx:compute-environment/ecs-compute'
ecs_compute_env = batch.ManagedEc2EcsComputeEnvironment.from_managed_ec2_ecs_compute_environment_arn(
    self, 'FargateComputeEnv', managed_ec2_ecs_compute_environment_arn=ecs_compute_arn
)
queue = batch.JobQueue(self, 'Queue', job_queue_name='queue')
queue.add_compute_environment(ecs_compute_env, 1)

# Error occurs
fargate_compute_arn = 'arn:aws:batch:us-east-1:xxx:compute-environment/fargate-compute'
fargate_compute_env = batch.FargateComputeEnvironment.from_fargate_compute_environment_arn(
    self, 'FargateComputeEnv', fargate_compute_environment_arn=fargate_compute_arn
)
queue = batch.JobQueue(self, 'Queue', job_queue_name='queue')
queue.add_compute_environment(fargate_compute_env, 1)

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.81.0

Framework Version

2.83.1

Node.js Version

v19.8.1

OS

Mac OS 12.6

Language

Python

Language Version

3.11

Other information

No response

@roketworks roketworks added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 14, 2023
@github-actions github-actions bot added the @aws-cdk/aws-batch Related to AWS Batch label Jun 14, 2023
@pahud pahud self-assigned this Jun 14, 2023
@pahud
Copy link
Contributor

pahud commented Jun 14, 2023

Looks like this requires vpc here

But fromFargateComputeEnvironmentArn is not having any vpc info

@pahud pahud added p1 effort/medium Medium work item – several days of effort labels Jun 14, 2023
@pahud pahud removed their assignment Jun 14, 2023
@pahud pahud removed the needs-triage This issue or PR still needs to be triaged. label Jun 14, 2023
@peterwoodworth
Copy link
Contributor

Thanks for reporting this, I'm finding the same behavior.

It's because the import class extends ManagedComputeEnvironmentBase, which requires an IVpc prop. When creating the import class and instance of it, fromFargateComputeEnvironmentArn() passes in undefined as the IVpc.

class Import extends ManagedComputeEnvironmentBase implements IFargateComputeEnvironment {
public readonly computeEnvironmentArn = fargateComputeEnvironmentArn;
public readonly computeEnvironmentName = computeEnvironmentName;
public readonly enabled = true;
}
return new Import(scope, id, {
vpc: undefined as any,
});

However, since the import class extends ManagedComputeEnvironmentBase, it will attempt to reference the IVpc when creating the security groups.

this.securityGroups = props.securityGroups ?? [
new ec2.SecurityGroup(this, 'SecurityGroup', {
vpc: props.vpc,
}),
];

Should probably extend Resource just like ManagedEc2EcsComputeEnvironment does

class Import extends Resource implements IManagedEc2EcsComputeEnvironment {

@peterwoodworth peterwoodworth added good first issue Related to contributions. See CONTRIBUTING.md effort/small Small work item – less than a day of effort and removed effort/medium Medium work item – several days of effort labels Jun 14, 2023
lpizzinidev added a commit to lpizzinidev/aws-cdk that referenced this issue Jun 15, 2023
mergify bot added a commit to lpizzinidev/aws-cdk that referenced this issue Jun 23, 2023
@mergify mergify bot closed this as completed in #25985 Jun 23, 2023
mergify bot pushed a commit that referenced this issue Jun 23, 2023
…rgateComputeEnvironmentArn (#25985)

Change the superclass of the [Import](https://github.com/aws/aws-cdk/blob/104bf32798b02f8f3c3ec5aaa05e31c35b4a38da/packages/%40aws-cdk/aws-batch-alpha/lib/managed-compute-environment.ts#L1071) class instantiated by `fromFargateComputeEnvironmentArn` from `ManagedComputeEnvironmentBase` to `Resource`.
This prevents errors due to the required `vpc` parameter of the old superclass [being passed](https://github.com/aws/aws-cdk/blob/104bf32798b02f8f3c3ec5aaa05e31c35b4a38da/packages/%40aws-cdk/aws-batch-alpha/lib/managed-compute-environment.ts#L1077-L1079) as `undefined`.

Closes #25979.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-batch Related to AWS Batch bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1
Projects
None yet
3 participants