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

[custom-resources] is being re-deployed on every deployment #9322

Closed
moltar opened this issue Jul 29, 2020 · 11 comments · Fixed by #9515
Closed

[custom-resources] is being re-deployed on every deployment #9322

moltar opened this issue Jul 29, 2020 · 11 comments · Fixed by #9515
Assignees
Labels
@aws-cdk/custom-resources Related to AWS CDK Custom Resources bug This issue is a bug. effort/small Small work item – less than a day of effort in-progress This issue is being actively worked on. p2

Comments

@moltar
Copy link
Contributor

moltar commented Jul 29, 2020

Not sure if this is actually a bug, or a feature, but when I am using custom resources, notably AwsCustomResource, it seems to redeploy every time I am doing a deploy. Which takes a few minutes each time.

Reproduction Steps

Use AwsCustomResource and supply onCreate with some basic call.

Error Log

No errors.

Environment

  • CLI Version: 1.55.0
  • Framework Version: 1.55.0
  • Node.js Version: v12.18.2
  • OS: macos
  • Language (Version): TS

Other

N/A


This is 🐛 Bug Report

@moltar moltar added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 29, 2020
@SomayaB SomayaB changed the title [custom-resource] is being re-deployed on every deployment [custom-resources] is being re-deployed on every deployment Jul 30, 2020
@github-actions github-actions bot added the @aws-cdk/custom-resources Related to AWS CDK Custom Resources label Jul 30, 2020
@eladb
Copy link
Contributor

eladb commented Aug 4, 2020

@jogold any insights?

@eladb eladb added effort/small Small work item – less than a day of effort p2 labels Aug 4, 2020
@jogold
Copy link
Contributor

jogold commented Aug 4, 2020

@moltar this should indeed not happen if you only specify a onCreate. Can you share some code that reproduces the issue?

@moltar
Copy link
Contributor Author

moltar commented Aug 4, 2020

I'm supplying onUpdate too, sorry for misleading original issue.

@jogold
Copy link
Contributor

jogold commented Aug 4, 2020

I'm supplying onUpdate too, sorry for misleading original issue.

And some properties of your resource change on every deploy?

The long wait time is likely due to the installation of the latest AWS SDK. Will be fixed together with #9289.

@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Aug 4, 2020
jogold added a commit to jogold/aws-cdk that referenced this issue Aug 7, 2020
…Resource

Fix latest AWS SDK installation for `AwsCustomResource` and add option to
disable it.

Closes aws#9289
Closes aws#9322
@SomayaB SomayaB added the in-progress This issue is being actively worked on. label Aug 8, 2020
@moltar
Copy link
Contributor Author

moltar commented Aug 15, 2020

Here's my custom resource:

export interface DatabaseMigrationCustomResourceProps {
  vpc: IVpc
  databaseClusterSecret: ISecret
}

export class DatabaseMigrationCustomResource extends AwsCustomResource {
  constructor(
    scope: Construct,
    { vpc, databaseClusterSecret }: DatabaseMigrationCustomResourceProps,
  ) {
    const { name } = DatabaseMigrationCustomResource

    const taskRole = new iam.Role(scope, `${name}TaskRole`, {
      assumedBy: new iam.ServicePrincipal(ServicePrincipals.ECS_TASKS),
      description: 'Role for database migration Fargate Task to migrate the database.',
      inlinePolicies: {
        getSecretValue: new iam.PolicyDocument({
          statements: [
            new iam.PolicyStatement({
              actions: ['secretsmanager:GetSecretValue'],
              resources: [databaseClusterSecret.secretArn],
            }),
          ],
        }),
      },
    })

    const task = new ecs.FargateTaskDefinition(scope, `${name}Task`, {
      taskRole,
      memoryLimitMiB: 8192,
      cpu: 1024,
    })

    task.addContainer(`${name}TaskContainer`, {
      image: ecs.ContainerImage.fromAsset('./'),
      command: ['src/bin/migrate'],
      environment: {
        [DATABASE_URL]: databaseClusterSecret.secretArn,
      },
      logging: new ecs.AwsLogDriver({
        streamPrefix: `${name}TaskContainer`,
      }),
    })

    const ecsCluster = new ecs.Cluster(scope, `${name}TaskCluster`, {
      vpc,
    })

    /**
     * Role used by the AwsCustomResource to apply to the Lambda function that executes
     * on the events.
     */
    const role = new iam.Role(scope, `${name}TaskLambdaRole`, {
      assumedBy: new iam.ServicePrincipal(ServicePrincipals.LAMBDA),
      description:
        'Role used by the AwsCustomResource to apply to the Lambda function that executes on the events.',
      managedPolicies: [
        iam.ManagedPolicy.fromAwsManagedPolicyName(ManagedPolicies.AWS_LAMBDA_BASIC_EXECUTION_ROLE),
        iam.ManagedPolicy.fromAwsManagedPolicyName(
          ManagedPolicies.AWS_LAMBDA_VPC_ACCESS_EXECUTION_ROLE,
        ),
      ],
      inlinePolicies: {
        runTask: new iam.PolicyDocument({
          statements: [
            new iam.PolicyStatement({
              actions: ['iam:PassRole'],
              resources: [ensure(task.executionRole).roleArn, taskRole.roleArn],
            }),

            new iam.PolicyStatement({
              actions: ['ecs:RunTask'],
              resources: [task.taskDefinitionArn],
              // further limits access to a specific cluster only
              conditions: {
                ArnEquals: {
                  'ecs:cluster': ecsCluster.clusterArn,
                },
              },
            }),
          ],
        }),
      },
    })

    const parameters: ECS.Types.RunTaskRequest = {
      launchType: 'FARGATE',
      count: 1,
      cluster: ecsCluster.clusterArn,
      taskDefinition: task.taskDefinitionArn,
      networkConfiguration: {
        awsvpcConfiguration: {
          assignPublicIp: 'ENABLED',
          subnets: vpc.selectSubnets().subnetIds,
        },
      },
    }

    const awsSdkCall: AwsSdkCall = {
      service: 'ECS',
      action: 'runTask',
      physicalResourceId: {
        id: task.taskDefinitionArn,
      },
      parameters,
    }

    super(scope, name, {
      resourceType: `Custom::${name}`,
      policy: AwsCustomResourcePolicy.fromSdkCalls({
        resources: AwsCustomResourcePolicy.ANY_RESOURCE,
      }),
      role,
      onCreate: awsSdkCall,
      onUpdate: awsSdkCall,
    })
  }
}

Here's what I get in the events:

image

@moltar
Copy link
Contributor Author

moltar commented Aug 15, 2020

I am thinking maybe I shouldn't be piling all of that into the AwsCustomResource sub class but create a nested stack instead?

@moltar
Copy link
Contributor Author

moltar commented Aug 16, 2020

Switched to:

export class DatabaseMigrationCustomResource extends Construct {

but it made no difference.

A simple one character change to the app code will create 4 CF changes, and the deployment takes ~ 6 minutes to complete.

@mergify mergify bot closed this as completed in #9515 Aug 17, 2020
mergify bot pushed a commit that referenced this issue Aug 17, 2020
…mResource (#9515)

Fix latest AWS SDK installation for `AwsCustomResource` and add option to
disable it.

Closes #9289
Closes #9322


----

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

jogold commented Aug 17, 2020

@moltar you can now disable the latest SDK installation, it will speed up things.

@moltar
Copy link
Contributor Author

moltar commented Aug 17, 2020

@moltar you can now disable the latest SDK installation, it will speed up things.

Thank you for looking into this.

According to the log it seems to only add a max of one minute. But I'm seeing a tiny change take 6 minutes to deploy. That's without the upload and so on. Just CF changes.

@jogold
Copy link
Contributor

jogold commented Aug 17, 2020

I think that's because your have multiple custom resources. Let's discuss this again when #9515 is released.

@kisharma24
Copy link

do you have any logic where this migration will happen automatically with every pipeline deployment? (maybe using timestamp or something)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/custom-resources Related to AWS CDK Custom Resources bug This issue is a bug. effort/small Small work item – less than a day of effort in-progress This issue is being actively worked on. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants