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

[ECR] Fails to deploy if ECR repository already exists #5140

Closed
cmckni3 opened this issue Nov 20, 2019 · 13 comments
Closed

[ECR] Fails to deploy if ECR repository already exists #5140

cmckni3 opened this issue Nov 20, 2019 · 13 comments
Assignees
Labels
@aws-cdk/aws-ecr Related to Amazon Elastic Container Registry bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@cmckni3
Copy link
Contributor

cmckni3 commented Nov 20, 2019

Reproduction Steps

import cdk = require('@aws-cdk/core');

import ecr = require('@aws-cdk/aws-ecr');

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

    const ecrRepo = new ecr.Repository(this, 'MyECRRepo', {
      repositoryName: 'my-ecr-repo'
    });

    new cdk.CfnOutput(this, 'MyECRRepoURI', {
      value: ecrRepo.repositoryUri,
    });
  }
}

Error Log

I see an error state the ECR Repository with name my-ecr-repo already exists and the CFN deployment cancels and rolls back.

Environment

  • CLI Version :1.16.3
  • Framework Version: 1.16.3
  • OS : macOS
  • **Language : typescript **

Other

This is 🐛 Bug Report

@cmckni3 cmckni3 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 20, 2019
@SomayaB SomayaB added the @aws-cdk/aws-ecr Related to Amazon Elastic Container Registry label Nov 21, 2019
@rix0rrr
Copy link
Contributor

rix0rrr commented Nov 21, 2019

    const ecrRepo = new ecr.Repository(this, 'MyECRRepo', {
      repositoryName: 'my-ecr-repo'
    });

This will want to create a repository with the given name, which will indeed fail if the repository already exists.

The same will happen for any other resource with a hardcoded name. Pick a different name, or leave it out altogether.

@rix0rrr rix0rrr closed this as completed Nov 21, 2019
@cmckni3
Copy link
Contributor Author

cmckni3 commented Nov 21, 2019

Correct, this is the only place I specify a name.

I would imagine resource update would be skipped if the name is the same as the existing resource especially when nothing on the resource has changed in the updated template.

That seems like a CloudFormation issue in that case.

@jeznag
Copy link

jeznag commented Dec 11, 2019

I have this issue too. If there is a problem creating the stack, the ECR repo is not deleted during the rollback and then when I try to deploy again, it breaks with this error.

@cmckni3
Copy link
Contributor Author

cmckni3 commented Dec 11, 2019

yep, it's unfortunate. My issue involved template updates trying to recreate the repo even though the repo hadn't changed.

I may create a custom resource in the future to handle ECR repos. I want a custom resource at the moment anyways to enable image scanning (not supported by CloudFormation).

@thomasvds
Copy link

I'm having this issue too, even when I deploy the stack for the first time and this is the only place where I specify the repository name.

@cmckni3
Copy link
Contributor Author

cmckni3 commented Dec 14, 2019

Yup, create in my case created the repo then update failed while trying to create again. I use a custom resource lambda that checks for existence, update repo properties and enables image scanning.

Specifying a name and failing during an update to the resource is common issue for Cloudformation resources.

@J11522
Copy link
Contributor

J11522 commented Aug 3, 2020

@rix0rrr Could you provide a best-practice on how to approach a situation like this?

E.g. we want to setup a pipeline that pushes to ECR and have our deployment pull from ECR.
Of course we can't delete the repository when the pipeline is removed nor use dynamic names for the ECR as our deployment depends on this.
Hence we would choose static names and only create a new ECR repository in the pipeline if one with the name doesn't exist.

One common workaround found online is to query the ecr for existing repos, however this approach is of course not safe due to the async nature of synth and deploy.
Furthermore this would introduce an anti-pattern as referenced here: #8273

Would this be addresses by the context provider framework?

Judging from the comments here and elsewhere, this seems like a common use case.

I'd be very curious to hear how this should ideally be approached.

@justin-ad
Copy link

justin-ad commented Nov 17, 2020

@rix0rrr @SomayaB Not sure why this was closed, can you please comment on this? Do you believe the current behavior is expected or appropriate? How can one re-deploy a CDK stack that contains ECR repo resources (assuming most people don't want to omit repository_name which causes an additional repo to be created with a unique name suffix with each cdk deploy).

@SomayaB SomayaB reopened this Nov 20, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Nov 23, 2020

This issue was closed because this is expected behavior from CloudFormation, and we have no good way to work around it. You generally do not want to physical-name resources, exactly for this reason.

It's unfortunate that there are issues with rollback+retry, but we purposely set a default removalPolicy of RETAIN (until we have support for stack policies) because the alternative is one of:

  • You delete a stack without realizing it had a repository with a bunch of important images in there, and the repository is gone
  • You cannot delete the stack because the repository is non-empty and CloudFormation will refuse to delete it

(Don't recall which is the exact behavior for ECR, but we generally do this for all stateful resources as both possible consequences are undesirable in their own way).

You can change the removalPolicy to DESTROY if you're sure that you won't make mistakes.


Could you provide a best-practice on how to approach a situation like this?
E.g. we want to setup a pipeline that pushes to ECR and have our deployment pull from ECR.

Our current recommendation around this is using CDK Pipelines which deploys Docker images but hides the fact that there are ECR repositories involved.

As soon as you start caring about the ECR repositories themselves, you are on your own building your own flavor of pipeline. I recommend you create the ECR repo in the pipeline stack (or a separate stack, honestly) and export the names to SSM parameters, then import in the stacks in which you care.

@rix0rrr rix0rrr closed this as completed Nov 23, 2020
@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.

@rcdailey
Copy link

rcdailey commented Jul 6, 2023

@rix0rrr Sorry to bump a 3 year old issue. However, I feel like no one has explicitly stated the obvious issue here? The reason why I want to reuse & adopt existing ECR repositories in my CDK project is because I do not want to lose docker images uploaded there. The solution you suggested WRT leveraging SSM parameter store seems to solve the issue of discoverability / identification of ECR repos with non-deterministic names, but that still means we're destroying and creating ECR repos and, subsequently, the uploaded images with it.

What is the recommended solution for this? How do I retain my previously uploaded docker images? I build my docker images independently of my CDK project and as part of my CI/CD pipeline.

@Josephineci
Copy link

I too am having the same issue. I have tried changing the repository permissions on my ECR repo

@kenberkeley
Copy link

I fixed mine by reusing the previous logical ID.

Example:

// Original code
new Repository(this, "HelloWorldRepo", {
  repositoryName: "hello-world-repo"
})

// Updated code: Resource of type 'AWS::ECR::Repository' with identifier 'xxx' already exists
new Repository(this, "hello-world-repo", {
  repositoryName: "hello-world-repo"
})

The fix is just reverting back to HelloWorldRepo.

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. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

No branches or pull requests

10 participants