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

Cyclic Reference when trying to make a CodeBuild project to EKS #6989

Closed
yjw113080 opened this issue Mar 25, 2020 · 6 comments
Closed

Cyclic Reference when trying to make a CodeBuild project to EKS #6989

yjw113080 opened this issue Mar 25, 2020 · 6 comments
Assignees
Labels
guidance Question that needs advice or information. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@yjw113080
Copy link

I'm trying to make two separate stack,

  1. creates a bunch of CI/CD stuff, which includes a build stage to deploy to EKS cluster.
  2. creates a EKS cluster and configures it.

When I tried to deploy the stacks, I get the following error.
I guess it's somewhat related to #5760 , #3300?

Reproduction Steps

  1. CicdForAppStack
constructor(scope: cdk.Construct, id: string, props: CommonProps) {
    super(scope, id, props);
...
const deployToMainEKScluster = new codebuild.PipelineProject(this, 'deploy-to-main', {
            environment: {
                buildImage: codebuild.LinuxBuildImage.fromAsset(this, 'custom-image', {
                    directory: './buildimage'
                })
            },
            environmentVariables: { 
                'CLUSTER_NAME': {
                    value: `${props.cluster.clusterName}`
                  },
                'ECR_REPO_URI': {
                value: `${ecrForHelloPy.repositoryUri}`
              } 
            },
            buildSpec: codebuild.BuildSpec.fromObject({
                version: "0.2",
                phases: {
                  install: {
                    commands: [
                      'env',
                      'export TAG=${CODEBUILD_RESOLVED_SOURCE_VERSION}',
                      '/usr/local/bin/entrypoint.sh'                    ]
                  },
                  build: {
                    commands: [
                        `sed -i 's@CONTAINER_IMAGE@'"$ECR_REPO_URI:$TAG"'@' hello-py.yaml`,
                        'kubectl apply -f hello-py.yaml'
                    ]
                  }
                }})
        });
  1. ClusterStack
    I think it's quite ordinary stack for EKS, so I just add the link to the code, instead of pasting it over.
    https://github.com/yjw113080/aws-cdk-eks-multi-region/blob/master/lib/cluster-stack.ts

Error Log

/Users/jiwony/multi-cluster-ts/node_modules/@aws-cdk/core/lib/stack.ts:659
        throw new Error(`'${target.node.path}' depends on '${this.node.path}' (${cycle.join(', ')}). Adding this dependency (${reason}) would create a cyclic reference.`);
              ^
Error: 'CicdForAppStack' depends on 'v2-ClusterStack-ap-northeast-1' (CicdForAppStack -> v2-ClusterStack-ap-northeast-1/demogo-cluster/Resource/Resource/Default.Ref). Adding this dependency (v2-ClusterStack-ap-northeast-1 -> CicdForAppStack/deploy-to-main/Role/Resource.Arn) would create a cyclic reference.
    at ClusterStack._addAssemblyDependency (/Users/jiwony/multi-cluster-ts/node_modules/@aws-cdk/core/lib/stack.ts:659:15)
    at Object.addDependency (/Users/jiwony/multi-cluster-ts/node_modules/@aws-cdk/core/lib/deps.ts:44:20)
    at ClusterStack.addDependency (/Users/jiwony/multi-cluster-ts/node_modules/@aws-cdk/core/lib/stack.ts:364:5)
    at CicdForAppStack.prepareCrossReference (/Users/jiwony/multi-cluster-ts/node_modules/@aws-cdk/core/lib/stack.ts:926:24)
    at ClusterStack.prepare (/Users/jiwony/multi-cluster-ts/node_modules/@aws-cdk/core/lib/stack.ts:769:37)
    at ClusterStack.onPrepare (/Users/jiwony/multi-cluster-ts/node_modules/@aws-cdk/core/lib/construct-compat.ts:105:10)
    at Node.prepare (/Users/jiwony/multi-cluster-ts/node_modules/constructs/lib/construct.ts:441:28)
    at Node.synthesize (/Users/jiwony/multi-cluster-ts/node_modules/constructs/lib/construct.ts:399:10)
    at Function.synth (/Users/jiwony/multi-cluster-ts/node_modules/@aws-cdk/core/lib/construct-compat.ts:225:22)
    at App.synth (/Users/jiwony/multi-cluster-ts/node_modules/@aws-cdk/core/lib/app.ts:141:36)

Environment

  • **CLI Version :**1.30.0 (build 4f54ff7
  • **Framework Version:**1.30.0
  • **OS :**OSX Mojave
  • **Language :**Typescript

Other


This is 🐛 Bug Report

@yjw113080 yjw113080 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 25, 2020
@skinny85 skinny85 self-assigned this Mar 25, 2020
@skinny85
Copy link
Contributor

Hey @yjw113080 ,

the problem is you're relying on the name of the EKS cluster, passing it to the environment variables of your CodeBuild project. If you hardcode the name instead of using cluster.name (since you know the name already, it's set to demogo-${cdk.Stack.of(this).region} in the EKS stack), that will break the cycle.

If that doesn't work, you have to show me your entire code, including the entrypoint (I have no idea what things like ecrForHelloPy are, for example, which makes it hard to know what exactly is the problem).

Thanks,
Adam

@skinny85 skinny85 added guidance Question that needs advice or information. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 25, 2020
@yjw113080
Copy link
Author

yjw113080 commented Mar 26, 2020

Thank you for your comment @skinny85
I do appreciate your guidance.
I do not exactly how the hardcoded name of my cluster would impact this whole stack,
but I did try to remove the hardcoded part, the problem didn't go away.

What I did found was, as soon as I construct the buildspec from here,
I get the error.

you could simply browse this repo
I clipped part of my code.

  1. entrypoint
...
for (const env of environments) {
    const clusterStack = new ClusterStack (app, `ClusterStack`, { env });
    const containerStack = new ContainerStack (app, `ContainerStack`, { env, cluster: clusterStack.cluster, asg: clusterStack.asg});
    const cicdStack = new CicdForAppStack (app, `CicdForAppStack`, { env, cluster: clusterStack.cluster, asg: clusterStack.asg});
}

  1. ClusterStack, which has the EKS defined
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);
...
      this.cluster = new eks.Cluster(this, 'demogo-cluster', {
        mastersRole: clusterAdmin,
        defaultCapacity: 0
      });
  1. CicdStack, which creates the pipeline with buildspec
        const ecrForHelloPy = new ecr.Repository(this, 'ecr-for-hello-py');
...
        const deployToMainEKScluster = deployToEKSspec(this, props.cluster, ecrForHelloPy);
...
        const repoToEcrPipeline = new codepipeline.Pipeline(this, 'repo-to-ecr-hello-py', {
...
                {
                    stageName: 'DeployToMainEKScluster',
                    actions: [ new pipelineAction.CodeBuildAction({
                        actionName: 'DeployToMainEKScluster',
                        input: sourceOutput,
                        // project: new codebuild.PipelineProject(this,'test',{buildSpec: codebuild.BuildSpec.fromSourceFilename('buildspec.yml')})
                        project: deployToMainEKScluster
                    })]
                },
  1. deployToMainEKScluster
function deployToEKSspec (scope: cdk.Construct, cluster: eks.Cluster, apprepo: ecr.Repository) :PipelineProject {
    const deployBuildSpec = new codebuild.PipelineProject(scope, `deploy-to-eks`, {
        // environment: {
        //     buildImage: codebuild.LinuxBuildImage.fromAsset(scope, 'custom-image-for-eks', {
        //         directory: './utils/buildimage'
        //     })
        // },
        environmentVariables: { 
            'CLUSTER_NAME': {
                value: `${cluster.clusterName}`
              },
            'ECR_REPO_URI': {
            value: `${apprepo.repositoryUri}`
          } 
        },
        buildSpec: codebuild.BuildSpec.fromObject({
            version: "0.2",
            phases: {
              install: {
                commands: [
                  'env',
                  'export TAG=${CODEBUILD_RESOLVED_SOURCE_VERSION}',
                  '/usr/local/bin/entrypoint.sh'                    ]
              },
              build: {
                commands: [
                    `sed -i 's@CONTAINER_IMAGE@'"$ECR_REPO_URI:$TAG"'@' hello-py.yaml`,
                    'kubectl apply -f hello-py.yaml'
                ]
              }
            }})
    });

    cluster.awsAuth.addMastersRole(deployBuildSpec.role!);
    deployBuildSpec.addToRolePolicy(new iam.PolicyStatement({
      actions: ['eks:DescribeCluster'],
      resources: [`${cluster.clusterArn}`],
    }));

    return deployBuildSpec;

}

@skinny85
Copy link
Contributor

Maybe I wasn't clear. The problem is this:

        environmentVariables: { 
            'CLUSTER_NAME': {
                value: `${cluster.clusterName}`
              },

If you change cluster.clusterName (BTW, the backticks are redundant here) to simply using the name of the cluster in the environment variable directly (and from your code, the name is set to demogo-${cdk.Stack.of(this).region}), that should break the cycle.

@yjw113080
Copy link
Author

It worked! thanks for your guide.
But I'm not sure why getting cluster.clusterName in buildspec would break the cycle.
To be specific, the stack containing buildspec does lots of stuff with the cluster (authorizing the cluster to pull ECR and etc) but that part only causes the issue. Could you help me understanding why?

@skinny85
Copy link
Contributor

skinny85 commented Apr 1, 2020

I think I know this from the error message. Let me reformat it a little:

Error: 'CicdForAppStack' depends on 'v2-ClusterStack-ap-northeast-1':

CicdForAppStack -> v2-ClusterStack-ap-northeast-1/demogo-cluster/Resource/Resource/Default.Ref).

Adding this dependency: 

v2-ClusterStack-ap-northeast-1 -> CicdForAppStack/deploy-to-main/Role/Resource.Arn

would create a cyclic reference

I assume the CicdForAppStack/deploy-to-main/Role/Resource.Arn is because you need to add the role to the resource policy of the ECR repo to allow it access. Because of that, you can't also have a dynamic reference to the Cluster - you have to hard-code it to avoid the dependency.

Does that make sense...?

@yjw113080
Copy link
Author

Ah I got this. Thanks for your explanation!
closing this issue. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
guidance Question that needs advice or information. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

2 participants