aws-quickstart / cdk-eks-blueprints

AWS Quick Start Team
Apache License 2.0
454 stars 205 forks source link

AwsVpcCni: Error Adding Custom Networking When Passing in VPC and SubnetIds #3 #610

Closed cristianmagana closed 1 year ago

cristianmagana commented 1 year ago

Describe the bug

Currently implementing the blueprint custom aws-vpc-cni addOn and getting the below error:

My project is implementing the blueprint via CodePipeline/Wave/Stage which builds two stacks

  1. Custom networking stack with secondary subnets and custom routes
  2. MainProjectStack which has the blueprint implementation.

The full implementation of the cluster has been deployed in my scenario but looks like the newly released aws vpc cni custom networking example #3 )

I'm able to deploy Pattern #1 & Pattern #2 without issue but not able to pass in custom networking to Pattern #3.

export class MainProjectStack extends Stack {
    constructor(scope: Construct, id: string, props: EksConfig & {vpc: IVpc; sn0Id: string; sn1Id: string; sn2Id: string}) {
        super(scope, id, props);

        const addOns = new blueprints.addons.VpcCniAddOn({
            customNetworkingConfig: {
                subnets: [
                    blueprints.getNamedResource('secondary-cidr-subnet-0'),
                    blueprints.getNamedResource('secondary-cidr-subnet-1'),
                    blueprints.getNamedResource('secondary-cidr-subnet-2'),
                ],
            },
            awsVpcK8sCniCustomNetworkCfg: true,
            eniConfigLabelDef: 'topology.kubernetes.io/zone',
        });

        blueprints.EksBlueprint.builder()
            .addOns(addOns)
            .resourceProvider(GlobalResources.Vpc, new DirectVpcProvider(props.vpc))
            .resourceProvider('secondary-subnet-1', new LookupSubnetProvider(props.sn0Id))
            .resourceProvider('secondary-subnet-2', new LookupSubnetProvider(props.sn1Id))
            .resourceProvider('secondary-subnet-3', new LookupSubnetProvider(props.sn2Id))
            .build(this, props.eksClusterName);
    }
}

Error:


adding dev-us-east-1
DEBUG Core add-on vpc-cni is at version v1.12.2-eksbuild.1

/${projectPath}/node_modules/constructs/src/construct.ts:403
      throw new Error(`There is already a Construct with name '${childName}' in ${typeName}${name.length > 0 ? ' [' + name + ']' : ''}`);
            ^
Error: There is already a Construct with name 'EniCustomConfig[object Object]' in Cluster [projectname-eks-dev-us-east-1]
    at Node.addChild (/${projectPath}/node_modules/constructs/src/construct.ts:403:13)
    at new Node (/${projectPath}/node_modules/constructs/src/construct.ts:71:17)
    at new Construct (/${projectPath}/node_modules/constructs/src/construct.ts:463:17)
    at new KubernetesManifest (/${projectPath}/node_modules/aws-cdk-lib/aws-eks/lib/k8s-manifest.js:1:500)
    at Function.KubectlProvider.applyManifestDeployment (/${projectPath}/node_modules/@aws-quickstart/eks-blueprints/lib/addons/helm-addon/kubectl-provider.ts:143:16)
    at KubectlProvider.addManifest (/${projectPath}/node_modules/@aws-quickstart/eks-blueprints/lib/addons/helm-addon/kubectl-provider.ts:155:32)
    at VpcCniAddOn.deploy (/${projectPath}/node_modules/@aws-quickstart/eks-blueprints/lib/addons/vpc-cni/index.ts:200:27)
    at new EksBlueprint (/${projectPath}/node_modules/@aws-quickstart/eks-blueprints/lib/stacks/eks-blueprint-stack.ts:263:34)
    at BlueprintBuilder.build (/${projectPath}/node_modules/@aws-quickstart/eks-blueprints/lib/stacks/eks-blueprint-stack.ts:195:16)
    at new MainProjectStack (/${projectPath}/lib/main-project-stack.ts:120:14)

Expected Behavior

The aws-vpc-cni pattern #3%3B) to take in the vpc and subnetsIds and provision the EKS specific configuration for the custom networking.

Current Behavior

Getting error of There is already a Construct with name 'EniCustomConfig[object Object]' in Cluster [projectname-eks-dev-us-east-1]

Reproduction Steps

CodePipeline/Wave/Stage to implement Custom Networking Stack which passes IVpc and subnetId to MainProjectStack which implements blueprint.

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.66.1

EKS Blueprints Version

1.6.0

Node.js Version

18.14.2

Environment details (OS name and version, etc.)

mac monterrey 12.2.1

Other information

No response

shapirov103 commented 1 year ago

in pattern 3, it looks like we have a documentation issue that needs to be fixed. Numeration of subnet providers is incorrect, should be 0, 1, 2 in both cases as shown here:

          blueprints.getNamedResource("secondary-cidr-subnet-0"),
          blueprints.getNamedResource("secondary-cidr-subnet-1"),
          blueprints.getNamedResource("secondary-cidr-subnet-2"),
      ]   
  },
  awsVpcK8sCniCustomNetworkCfg: true,
  eniConfigLabelDef: 'topology.kubernetes.io/zone'
});

const blueprint = blueprints.EksBlueprint.builder()
  .addOns(addOn)
  .resourceProvider(blueprints.GlobalResources.Vpc, new DirectVpcProvider(yourVpcId))
  .resourceProvider("secondary-subnet-0", new LookupSubnetProvider(subnet1Id)
  .resourceProvider("secondary-subnet-1", new LookupSubnetProvider(subnet2Id)
  .resourceProvider("secondary-subnet-2", new LookupSubnetProvider(subnet3Id)
elamaran11 commented 1 year ago

Yes. This actually have to be

          blueprints.getNamedResource("secondary-cidr-subnet-0"),
          blueprints.getNamedResource("secondary-cidr-subnet-1"),
          blueprints.getNamedResource("secondary-cidr-subnet-2"),
      ]   
  },
  awsVpcK8sCniCustomNetworkCfg: true,
  eniConfigLabelDef: 'topology.kubernetes.io/zone'
});

const blueprint = blueprints.EksBlueprint.builder()
  .addOns(addOn)
  .resourceProvider(blueprints.GlobalResources.Vpc, new DirectVpcProvider(yourVpcId))
  .resourceProvider("secondary-cidr-subnet-0", new LookupSubnetProvider(subnet1Id)
  .resourceProvider("secondary-cidr-subnet-1", new LookupSubnetProvider(subnet2Id)
  .resourceProvider("secondary-cidr-subnet-2", new LookupSubnetProvider(subnet3Id)
cristianmagana commented 1 year ago

Yes, I tried that along with any arbitrary name in the resource provider. Still a nogo.

elamaran11 commented 1 year ago

@cristianmagana Can you also change this line too :

.resourceProvider(blueprints.GlobalResources.Vpc, new VpcProvider(yourVpcId))
cristianmagana commented 1 year ago

Using the VpcProvider vs the DirectVpcProvider throws errors:

Error: All arguments to Vpc.fromLookup() must be concrete (no Tokens)

shapirov103 commented 1 year ago

Using the VpcProvider vs the DirectVpcProvider throws errors:

Error: All arguments to Vpc.fromLookup() must be concrete (no Tokens)

In this case the vpcId passed to the VpcProvider must be an actual string like "vpc-06795ec62eb45cfa9". If you attempt to pass a VPC created in a separate stack, then DirectVpcProvider is the one to use.

cristianmagana commented 1 year ago

Yes of course. The issue is that the VpcId does not exist when synthesizing the stacks and feeds in the token. So using the VpcProvider does not look to be an option.

In the case of using the DirectVpcProvider this is able to take in the IVpc. Looking at the error stack its looks like the synthesizing is creating Kubernetes Manifests with the name of EniCustomConfig[object Object] where the naming of the manifest needs to be something unique.

Potentially changing the line 194: from: name: "EniCustomConfig" + subnet, to: name: "EniCustomConfig" + subnet.subnetId or using string interpolation name: 'EniCustomConfig-${subnet.subnetId}'.

https://github.com/aws-quickstart/cdk-eks-blueprints/blob/7eac4181d2244f8da4214c5ffc3365964f6cc908/lib/addons/vpc-cni/index.ts#L193-L198

shapirov103 commented 1 year ago

Pattern 3/ was addressing a case when VPC ID is known as well as subnetId. We don't have yet an example for using cross-stack references. For this case LookupSubnetProvider won't work, you need something like a DirectSubnetProvider (similar to the DirectVpcProvider).

If you pass your subnets as is in your props object (as ISubnet, as opposed to just id) you can use this example for now:

const blueprint = blueprints.EksBlueprint.builder()
  .addOns(addOn)
  .resourceProvider("secondary-cidr-subnet-0", {
        provide(_context): ISubnet { return props.subnet[0]; } // assuming you passed an array of subnets in your props as subnets: ISubnet[]
   })
   ...
cristianmagana commented 1 year ago

I'm not sure that tracks.

Where pattern 2 using VpcProvider(vpcId?: string ...) provisions the VPC and custom networking as part of the blueprint. That works.

But in pattern 3 the code shows using a DirectVpcProvider(vpc: IVpc) (which is not the issue, we are able to use the IVpc object).

I've tried using the verbiage above in a clean project implementing the case of knowing the VpcId and SubnetIds beforehand which would need to use a VpcProvider. But it continues to fail with the same error:

Error: There is already a Construct with name 'EniCustomConfig[object Object]' in Cluster [projectname-eks-prod-us-east-1]

import "source-map-support/register";
import * as cdk from "aws-cdk-lib";
import * as blueprints from "@aws-quickstart/eks-blueprints";
import {
  LookupSubnetProvider,
  VpcProvider,
} from "@aws-quickstart/eks-blueprints";

const app = new cdk.App();

const addOns = new blueprints.addons.VpcCniAddOn({
  customNetworkingConfig: {
    subnets: [
      blueprints.getNamedResource("secondary-cidr-subnet-0"),
      blueprints.getNamedResource("secondary-cidr-subnet-1"),
      blueprints.getNamedResource("secondary-cidr-subnet-2"),
    ],
  },
  awsVpcK8sCniCustomNetworkCfg: true,
  eniConfigLabelDef: "topology.kubernetes.io/zone",
});

blueprints.EksBlueprint.builder()
  .addOns(addOns)
  .resourceProvider(blueprints.GlobalResources.Vpc, new VpcProvider("vpc-12345a5c86aa1cbc7"))
  .resourceProvider("secondary-subnet-0", new LookupSubnetProvider("subnet-01234567890abcdea"))
  .resourceProvider("secondary-subnet-1", new LookupSubnetProvider("subnet-01234567890abcdeb"))
  .resourceProvider("secondary-subnet-2", new LookupSubnetProvider("subnet-01234567890abcdec"))
  .build(app, "test");

I'm continuing to believe the issue is the manifestDeployment name of the name: "EniCustomConfig" + subnet. Where subnet is an object and we are trying to add that to the naming of the manifest.

https://github.com/aws-quickstart/cdk-eks-blueprints/blob/7eac4181d2244f8da4214c5ffc3365964f6cc908/lib/addons/vpc-cni/index.ts#L193-L198

elamaran11 commented 1 year ago

Hi @cristianmagana. The context name of the subnets should match as I mentioned in above comment. Either remove -CIDR in the named resource or add it to resource provider to subnet context names for consistency. Basically the absolutely value is empty so that’s why you are getting error on array of ENIConfig + subnet. Please let us know if you still see the issue we can troubleshoot. We are working on PR to fix the name conflict on context in the documentation.