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

[aws-ec2] Allow including/excluding availability zones based on AZ Id #8972

Closed
2 tasks
IvanKraljevic opened this issue Jul 9, 2020 · 7 comments
Closed
2 tasks
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p2

Comments

@IvanKraljevic
Copy link

IvanKraljevic commented Jul 9, 2020

Hi CDK team,
It would be great if you enable us to supply a list of included/excluded AZs (based on AZ id, not name) when a VPC or any other resource that requires a subnet selection is created (for instance, an EKS cluster).

A viable alternative would be to expose the AZ IDs filed on subnets, so we can manually filter unwanted subnets.

Use Case

I have a use case where I need to create an EKS cluster with worker nodes in every available AZ. When I try to deploy my stacks, I get an UnsupportedAvailabilityZoneException stating there aren't enough compute resources in one of the supplied AZs. This is a fairly common issue with EKS (example 1, example 2), and you can usually work around it by omitting the subnets mapped to the problematic AZ.

As the AZ names get shuffled for every AWS account, that workaround doesn't scale if you need to create a big number of clusters spread across a lot of AWS accounts.

My current workaround is to maintain a map of unsupported AZs for every AWS account that I have:

static final Multimap<String, String> ACCOUNT_ID_TO_UNSUPPORTED_AZ = ImmutableMultimap.<String, String>builder()
            .put("my-aws-account-id-1", "us-east-1e")
            .put("my-aws-account-id-2", "us-east-1f")
            ...
            .build();

boolean isAzSupported(final ISubnet s) {
    final var accountId = s.getStack().getAccount();
    if (StringUtils.isEmpty(accountId)) {
        return true;
    }

    return !ACCOUNT_ID_TO_UNSUPPORTED_AZ.get(accountId)
            .contains(s.getAvailabilityZone());
}

Cluster buildCluster(final Construct scope, final String clusterName, final Vpc vpc) {
    final var subnets = vpc.getPrivateSubnets().stream()
            .filter(this::isAzSupported)
            .collect(Collectors.toList());

    return Cluster.Builder.create(scope, "EksCluster")
            .vpc(vpc)
            .vpcSubnets(List.of(SubnetSelection.builder()
                    .subnets(subnets)
                    .build()))
            ....
           .build();
}

The awful thing about this is that I first need to fail in order to figure out the problematic subnet/AZ for each new account, adjust the mapping, have the change reviewed, deployed etc.

Proposed Solution

  1. Expand the VPC builder or subnet configuration so that subnets aren't created in unwanted AZs. Clients should be able to supply the wanted/unwanted AZ by either supplying the AZ names or IDs.
  2. Expand the SubnetSelection with availabilityZoneIds() so that resources aren't placed in unwanted AZs.

An alternative would be to expose getAvailabilityZoneId() on the ISubnet interface. With it, we'd be able to manually filter out unwanted AZs before an EKS cluster is created. It would simplify maintenance because the ACCOUNT_ID_TO_UNSUPPORTED_AZ map from the example above could be simplified to something like UNSUPPORTED_AZS = ['use1-az3',...].

Both proposals would increase developer velocity as the fail-adjust-review-deploy cycle would be avoided.

Other

The following comment suggests the same feature:
#5927 (comment)

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@IvanKraljevic IvanKraljevic added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jul 9, 2020
@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label Jul 9, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Jul 10, 2020

We are aware of this issue, but thanks for reporting.

For my edification--feels like it should be possible to put the physical AZ ids directly into the template. Is that not true?

@rix0rrr rix0rrr added the effort/large Large work item – several weeks of effort label Jul 10, 2020
@IvanKraljevic
Copy link
Author

Hi Rico,
Thanks for the feedback.

Looking at the official CF docs for Subnets and parameters, there are no mentions of AZ IDs so it seem CF works only with AZ names (us-east-1e):

The following links imply the same:
https://stackoverflow.com/questions/51513086/get-availability-zone-from-subnet-parameter-in-cloudformation
https://old.reddit.com/r/aws/comments/a1ldov/availability_zone_ids_in_cloudformation/

I might be able to use the AWS SDK to call DescribeSubnets for the default VPC, and extract the mapping between AZ names and IDs - haven't tried it yet though.

@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Jul 10, 2020
@rix0rrr rix0rrr added the p2 label Aug 12, 2020
@rix0rrr rix0rrr removed their assignment Jun 3, 2021
@github-actions
Copy link

github-actions bot commented Jun 3, 2022

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Jun 3, 2022
@github-actions github-actions bot closed this as completed Jun 8, 2022
@jdix531
Copy link

jdix531 commented Jun 28, 2022

I think this is an issue worth keeping open. The work arounds are fairly cumbersome.

@gravitylow
Copy link

Following up on this -- CFN now supports AvailabilityZoneId for subnets - https://docs.aws.amazon.com/cdk/api/v1/docs/@aws-cdk_aws-ec2.CfnSubnet.html. This should be possible to support now.

@ggallo
Copy link

ggallo commented Mar 13, 2023

Because Workspaces is not available in certain AZs, redesigning my stack led to a chain reaction of dropping everything into Cfn methods - an exercise with errors I am still slogging through - and completely negated the utility of the CDK.

If CDK supported this natively all this pain would be resolved.

@acrodrig
Copy link

@gravitylow just read your comment above. How could the subnet be used in CDK?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

7 participants