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

Auto delete images upon ECR repo removal - CDK v2.70.0 not working #24822

Closed
missourian55 opened this issue Mar 28, 2023 · 11 comments · Fixed by #25789
Closed

Auto delete images upon ECR repo removal - CDK v2.70.0 not working #24822

missourian55 opened this issue Mar 28, 2023 · 11 comments · Fixed by #25789
Labels
@aws-cdk/aws-ecr Related to Amazon Elastic Container Registry bug This issue is a bug. effort/medium Medium work item – several days of effort p2

Comments

@missourian55
Copy link

Describe the bug

Tested this much awaited feature in CDKv2.70.0 and it is still not working as expected.

Error

Resource handler returned message: "The repository with name 'test-app' in registry with id '1234567890' 
cannot be deleted because it still contains images (Service: Ecr, Status Code: 400,
Request ID: 842c370d-f12344XXXXX-ed47d07f0443)" 
(RequestToken: c64e0c80-9xxxxxxca5006c6c, HandlerErrorCode: GeneralServiceException)

ecr: add option to auto delete images upon ECR repository removal (#24572) (7de5b00), closes #15932 #12618 #15932

Expected Behavior

Delete of ECR repo with images should succeed if removalPolicy "Destroy" and autoDelete "true"

const bucket = new ecr.Repository(this, 'Repo', {
  repositoryName: 'delete-even-if-contains-images',
  removalPolicy: cdk.RemovalPolicy.DESTROY,
  autoDeleteImages: true,
});

Current Behavior

Error

Resource handler returned message: "The repository with name 'test-app' in registry with id '1234567890' 
cannot be deleted because it still contains images (Service: Ecr, Status Code: 400,
Request ID: 842c370d-f12344XXXXX-ed47d07f0443)" 
(RequestToken: c64e0c80-9xxxxxxca5006c6c, HandlerErrorCode: GeneralServiceException)

Reproduction Steps

Create a ECR repo similar to below using cdk deploy and add images to it. Afterwards, execute cdk destroy which fails with exception mentioned above

const bucket = new ecr.Repository(this, 'Repo', {
  repositoryName: 'delete-even-if-contains-images',
  removalPolicy: cdk.RemovalPolicy.DESTROY,
  autoDeleteImages: true,
});

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

CDK v2.70.0

Framework Version

No response

Node.js Version

v19.8.0

OS

mac-os M1

Language

Java

Language Version

JDK 17

Other information

No response

@missourian55 missourian55 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 28, 2023
@github-actions github-actions bot added the @aws-cdk/aws-ecr Related to Amazon Elastic Container Registry label Mar 28, 2023
@pahud pahud added the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Mar 28, 2023
@pahud pahud self-assigned this Mar 28, 2023
@pahud
Copy link
Contributor

pahud commented Mar 28, 2023

I can successfully destroy it with cdk 2.70.0 and it works pretty well in my account. Are you using CDK in TypeScript or Java? Your code snippets seems to be TypeScript but you specified Language as Java?

@pahud pahud removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 28, 2023
@missourian55
Copy link
Author

@pahud

The issue i encounter is in Java and I have not tested it in ts

        Repository ecrRepo = Repository.Builder
                .create(this, "DemoRepo")
                .repositoryName("test-app")
                .imageScanOnPush(false)
                .removalPolicy(DESTROY)
                .autoDeleteImages(true)
                .build();

@pahud
Copy link
Contributor

pahud commented Mar 29, 2023

I can destroy it with CDK in Java.

image

This is the Java code I used:

package com.myorg;

import software.constructs.Construct;
import software.amazon.awscdk.Stack;
import software.amazon.awscdk.StackProps;
import software.amazon.awscdk.RemovalPolicy;
import software.amazon.awscdk.services.ecr.Repository;

public class EcrJavaStack extends Stack {
    public EcrJavaStack(final Construct scope, final String id) {
        this(scope, id, null);
    }

    public EcrJavaStack(final Construct scope, final String id, final StackProps props) {
        super(scope, id, props);

        Repository ecrRepo = Repository.Builder
                .create(this, "DemoRepo")
                .repositoryName("test-app")
                .imageScanOnPush(false)
                .removalPolicy(RemovalPolicy.DESTROY)
                .autoDeleteImages(true)
                .build();
    }
}

@pahud pahud added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Mar 29, 2023
@pahud pahud removed their assignment Mar 29, 2023
@pahud pahud removed the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Mar 29, 2023
@missourian55
Copy link
Author

missourian55 commented Mar 29, 2023

The problem still exist and I am attaching a reproducer for you.

Steps to Reproduce

  1. Download and unzip the attached file
  2. ./mvnw clean compile
  3. cdk deploy
  4. Add images to the repository
  5. Verify the images exist through AWS Console
  6. cdk destroy to get the error

My Output

➜  ecr-test git:(main) ✗ cdk --version                                                                                                                                          
2.70.0 (build c13a0f1)

➜  ecr-test git:(main) ✗ node -v                                                                                                                                               
v19.8.0

➜  ecr-test git:(main) java --version                                                                                                                                           <aws:murubhas+child3-Admin>
openjdk 17.0.5 2022-10-18 LTS
OpenJDK Runtime Environment GraalVM 22.3.0 (build 17.0.5+8-LTS)
OpenJDK 64-Bit Server VM GraalVM 22.3.0 (build 17.0.5+8-LTS, mixed mode, sharing)

EcrTestStack: destroying... [1/1]
6:23:36 PM | DELETE_FAILED        | AWS::ECR::Repository        | TestConstructDemoRepo83AF430C
Resource handler returned message: "The repository with name 'test-app' in registry with id '1234567890' cannot be deleted because it still contains images (Service: Ecr, Status Code: 400, Request ID:
b57cafe4-1719-4904-875f-821e150bb319)" (RequestToken: b41acd61-0608-e1cf-a033-5b9bc6930fc9, HandlerErrorCode: GeneralServiceException)


 ❌  EcrTestStack: destroy failed Error: The stack named EcrTestStack is in a failed state. You may need to delete it from the AWS console : DELETE_FAILED (The following resource(s) failed to delete: [TestConstructDemoRepo83AF430C]. ): Resource handler returned message: "The repository with name 'test-app' in registry with id '1234567890' cannot be deleted because it still contains images (Service: Ecr, Status Code: 400, Request ID: b57cafe4-1719-4904-875f-821e150bb319)" (RequestToken: b41acd61-0608-e1cf-a033-5b9bc6930fc9, HandlerErrorCode: GeneralServiceException)
    at destroyStack (/opt/homebrew/Cellar/aws-cdk/2.70.0/libexec/lib/node_modules/aws-cdk/lib/index.js:371:1796)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async CdkToolkit.destroy (/opt/homebrew/Cellar/aws-cdk/2.70.0/libexec/lib/node_modules/aws-cdk/lib/index.js:374:152953)
    at async exec4 (/opt/homebrew/Cellar/aws-cdk/2.70.0/libexec/lib/node_modules/aws-cdk/lib/index.js:429:51795)

The stack named EcrTestStack is in a failed state. You may need to delete it from the AWS console : DELETE_FAILED (The following resource(s) failed to delete: [TestConstructDemoRepo83AF430C]. ): Resource handler returned message: "The repository with name 'test-app' in registry with id '1234567890' cannot be deleted because it still contains images (Service: Ecr, Status Code: 400, Request ID: b57cafe4-1719-4904-875f-821e150bb319)" (RequestToken: b41acd61-0608-e1cf-a033-5b9bc6930fc9, HandlerErrorCode: GeneralServiceException)

Deleting empty repository is not a problem. But deleting with images even after enabling suitable flags as released in CDKv2.70.0 is not working.

ecr-test.zip

Multi-Arch images in Repo before deletion (includes manifest)
test-app

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Mar 30, 2023
@pahud
Copy link
Contributor

pahud commented Mar 30, 2023

Unfortunately I still can't reproduce it with your ecr-test.zip.

This is my steps:

  1. download your ecr-test.zip into my environment.
  2. unzip it
  3. cdk diff
  4. cdk deploy
$ docker pull busybox
$ docker tag busybox <deducted>.dkr.ecr.us-west-2.amazonaws.com/test-app:latest
$ docker push <deducted>.dkr.ecr.us-west-2.amazonaws.com/test-app:latest

Now I can confirm the pushed image in the console
image

  1. cdk destroy with no error.

image


$ cdk --version
2.72.0 (build 397e46c)
$ mvn --version
Apache Maven 3.8.7 (b89d5959fcde851dcb1c8946a785a163f14e1e29)
Maven home: /usr/local/apache-maven-3.8.7
Java version: 17.0.5, vendor: Amazon.com Inc., runtime: /usr/lib/jvm/java-17-amazon-corretto
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "4.14.255-291-231.527.amzn2.x86_64", arch: "amd64", family: "unix"
$ java --version
openjdk 17.0.5 2022-10-18 LTS
OpenJDK Runtime Environment Corretto-17.0.5.8.1 (build 17.0.5+8-LTS)
OpenJDK 64-Bit Server VM Corretto-17.0.5.8.1 (build 17.0.5+8-LTS, mixed mode, sharing)
$ node -v
v16.17.0

@pahud pahud added needs-reproduction This issue needs reproduction. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Mar 30, 2023
@missourian55
Copy link
Author

missourian55 commented Mar 30, 2023

@pahud Thank you for taking the time to test. I believe the issue arises when you put a multi-architecture image into a repo that has an image index. I updated to 2.72.0 and was still able to duplicate the issue. Can you do the same thing with multi-arch images?

An Example of building multi arch using docker buildx plugin

docker buildx build -f Dockerfile --push --platform linux/amd64,linux/arm64/v8 --tag 1234567890.dkr.ecr.us-east-2.amazonaws.com/test-app:latest .

test-app

My Dockerfile (BusyBox multi-arch) from this file

FROM busybox

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Mar 31, 2023
@missourian55
Copy link
Author

@pahud Are you able to reproduce this(Unable to delete images when ECR contains multi-arch images and index)?

@hkford
Copy link
Contributor

hkford commented May 2, 2023

@pahud You can reproduce this issue by the following CDK code.

Prerequisite

  • CDK v2.70.0
  • Docker v20.10.23
  • Docker Buildx plugin

CDK code

import { Stack, StackProps, RemovalPolicy, CfnOutput } from 'aws-cdk-lib';
import { Construct } from 'constructs';
import {
  aws_ecr as ecr,
} from 'aws-cdk-lib';

export class EcrDebugStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    const multiArchRepo = new ecr.Repository(this, "MultiArchitectureRepository", {
      removalPolicy: RemovalPolicy.DESTROY,
      autoDeleteImages: true,
    });

    const stack = Stack.of(this);

    const region = stack.region;
    const accountId = stack.account;

    new CfnOutput(this, 'DockerLoginCommand', {
      value: `aws ecr get-login-password --region ${region} | docker login --username AWS --password-stdin ${accountId}.dkr.ecr.${region}.amazonaws.com`
    });

    new CfnOutput(this, 'DockerBuildxPushCommand', {
      value: `docker buildx build --platform linux/arm64,linux/amd64 --tag ${multiArchRepo.repositoryUri}:latest --push .`
    });
  }
}

Reproduction steps

Once you deploy the stack, run EcrDebugStack.DockerLoginCommand and EcrDebugStack.DockerBuildxPushCommand. This command pushes multi architecture Docker manifest list to ECR repository. After pushing the manifest list, run cdk destroy and you will see "the repository cannot be deleted because it still contains images".

Why the error occurs?

If manifest list is tagged and points to a manifest then the manifest list is not deleted before the manifest list. When you delete CloudFormation stack auto-delete-images-handler tries to delete manifest list but it cannot and the repository still has images, so deleting stack fails.
cf> GitHub issue comment

How to fix this?

Fix auto-delete-images-handler as code below.

async function emptyRepository(params: ECR.ListImagesRequest) {
  const listedImages = await ecr.listImages(params).promise();

  const imageIds = listedImages?.imageIds ?? [];
  // retrieve tagged images
  const imageIdsTagged = imageIds.filter((imageId) => 'imageTag' in imageId);
  const nextToken = listedImages.nextToken ?? null;
  if (imageIds.length === 0) {
    return;
  }
  
  // delete tagged images first
  await ecr.batchDeleteImage({
    repositoryName: params.repositoryName,
    imageIds: imageIdsTagged,
  }).promise();

  await ecr.batchDeleteImage({
    repositoryName: params.repositoryName,
    imageIds: imageIds,
  }).promise();

  if (nextToken) {
    await emptyRepository({
      ...params,
      nextToken,
    });
  }
}

@pahud pahud added bug This issue is a bug. p2 effort/medium Medium work item – several days of effort labels May 2, 2023
@pahud pahud self-assigned this May 2, 2023
@pahud
Copy link
Contributor

pahud commented May 2, 2023

Thank you @hkford

I can reproduce this now and I saw the error message as:

1:18:24 PM | DELETE_FAILED        | AWS::ECR::Repository        | MultiArchitectureRepositoryE8EC151A
Resource handler returned message: "The repository with name 'demostack-multiarchitecturerepositorye8ec151a-5ngwzcsg0lmd' in registry with id '903
779448426' cannot be deleted because it still contains images (Service: Ecr, Status Code: 400, Request ID: 935246f5-f9a6-421c-8cdb-65996a3bd717)"
(RequestToken: 44dd6e4d-6cea-5797-6470-b4e0e111fd59, HandlerErrorCode: GeneralServiceException)

Yes I believe we should fix the handler to batch delete all images. Thank you for your PR and feel free to let me know if you need any further help.

@pahud pahud removed @aws-cdk/aws-ecr Related to Amazon Elastic Container Registry needs-reproduction This issue needs reproduction. labels May 2, 2023
@pahud pahud added the @aws-cdk/aws-ecr Related to Amazon Elastic Container Registry label May 2, 2023
@pahud pahud removed their assignment May 2, 2023
@missourian55
Copy link
Author

@pahud Can you pls approve the PR and push the change?
#25391

@mergify mergify bot closed this as completed in #25789 Jun 6, 2023
mergify bot pushed a commit that referenced this issue Jun 6, 2023
…st (#25789)

Fixes #24822

As I commented on #24822 (comment), auto delete container images in ECR repository fails when it has container manifest list. I fix custom resource Lambda function to delete tagged images first.

----

*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

github-actions bot commented Jun 6, 2023

⚠️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-ecr Related to Amazon Elastic Container Registry bug This issue is a bug. effort/medium Medium work item – several days of effort p2
Projects
None yet
3 participants