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-eks] Can't define a fargate cluster with more than one profile #6084

Closed
michaelmoussa opened this issue Feb 4, 2020 · 1 comment · Fixed by #8374
Closed

[aws-eks] Can't define a fargate cluster with more than one profile #6084

michaelmoussa opened this issue Feb 4, 2020 · 1 comment · Fixed by #8374
Assignees
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service bug This issue is a bug. p1

Comments

@michaelmoussa
Copy link
Contributor

Only one Fargate profile can be in a CREATING state at a time, so attempting to create a cluster from scratch along with a custom profile (in addition to the one FargateCluster creates by default) results in an error, followed by a lengthy (and ultimately unsuccessful) rollback.

PR #6074 makes it possible to create one custom profile for a FargateCluster (as opposed to only the default before), but does not address the root cause. This issue is meant to track a fix that would allow multiple profiles.

Reproduction Steps

const vpc = new Vpc(this, 'VPC', {maxAzs: 2});
const cluster = new FargateCluster(this, 'Cluster', {
  clusterName: 'my-app',
  mastersRole: new Role(this, 'ClusterAdminRole', {
    assumedBy: new AccountRootPrincipal()}
  ),
  vpc,
});
cluster.addFargateProfile('MyAppFargateProfile', {
  fargateProfileName: 'my-app',
  selectors: [
    {namespace: 'my-app'}
  ],
  vpc
});

Possible solution

The following is copy-and-pasted from this comment in #6074 to keep the discussion in one place.

Looks good but I am wondering if this actually solves the root cause. It sounds like addFargateProfile cannot be used more than once against a cluster because CFN will always try to create all the profiles at once.

Technically you could use addFargateProfile(...) to add a new profile to a cluster that had already been established using CDK. That would succeed, but attempting to re-create the whole stack from scratch would indeed fail.

So, either we simply remove this capability (and perhaps allow people to specific an array of profile options) or we somehow discover all the profiles associated with a cluster a serialize their creation by adding explicit dependencies between them. I prefer the latter option.

The latter seems doable. We can modify the FargateProfileResourceHandler to list all profiles associated to the cluster and describe each one to ensure they all have the status ACTIVE before requesting the current one to be built. That should result in the profiles being created sequentially, and we can have it wait 30-60 seconds before checking to see if it's OK to to proceed.

It's not ideal and I'm not sure if this approach is taken anywhere else in CDK, but it avoids the need to somehow capture all profile creation requests to set up multiple cross-dependencies, and we don't need to introduce any kind of independent queue or state file to manage everything. The individual profile request Custom Resource functions should be able to wait politely for their turns.

There is still a possibility of a race condition if there are several pending profiles and two see an opportunity to get themselves created at the same time, but it's unlikely (or, we could address that by not treating an error due to profile creation in progress being a failure, and just have it keep waiting).


This is 🐛 Bug Report

@michaelmoussa michaelmoussa added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 4, 2020
@SomayaB SomayaB added the @aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service label Feb 4, 2020
@eladb eladb added the p1 label Feb 5, 2020
@eladb
Copy link
Contributor

eladb commented Feb 5, 2020

I think what we should do is to automatically synthesize a "DependsOn" relationship (at the CFN level) between all FargateProfile resources that are attached to the same cluster. This will result in the serialization of the profile creation and updates.

@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Feb 11, 2020
@SomayaB SomayaB added the in-progress This issue is being actively worked on. label Jun 5, 2020
@mergify mergify bot closed this as completed in #8374 Jun 10, 2020
mergify bot pushed a commit that referenced this issue Jun 10, 2020
Adds a DependsOn Fargate profile resources when more than one Fargate profiles
exists on the same cluster.

fixes #6084

----

Tested via:
```ts
    const vpc = new Vpc(this, 'VPC', {maxAzs: 2});
    const cluster = new FargateCluster(this, 'Cluster', {
      clusterName: 'my-app',
      mastersRole: new Role(this, 'ClusterAdminRole', {
        assumedBy: new AccountRootPrincipal()}
      ),
      vpc,
    });
    const profile1 = cluster.addFargateProfile('MyCustomFargateProfile1', {
      fargateProfileName: 'my-app',
      selectors: [
        {namespace: 'my-app'}
      ],
      vpc
    });
    const profile2 = cluster.addFargateProfile('MyCustomFargateProfile2', {
      fargateProfileName: 'my-app2',
      selectors: [
        {namespace: 'my-app2'}
      ],
      vpc
    });
```

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@iliapolo iliapolo changed the title Can't define a fargate cluster with more than one profile [aws-eks] Can't define a fargate cluster with more than one profile Aug 16, 2020
@iliapolo iliapolo removed the in-progress This issue is being actively worked on. label Aug 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service bug This issue is a bug. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants