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] Generalized build-order and/or simplified multi-dependencies #6299

Closed
michaelmoussa opened this issue Feb 16, 2020 · 3 comments
Closed
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. management/rfc request for comments p2

Comments

@michaelmoussa
Copy link
Contributor

I'd like a way to more concisely flag dependencies between multiple resources, or specify a particular "build order" explicitly when necessary.

Use Case

CDK has always been pretty good about figuring out dependencies on its own, but my recent attempts to use it with FargateCluster, FargateProfile, KubernetesResource, and HelmChart constructs proved that when CDK isn't able to figure out the deps on its own, it's really painful. I was ultimately successful, and I think the solution can be generalized to be integrated fairly cleanly into CDK core if desired.

To avoid getting too deep into the weeds, I'll summarize the problem I was having:

Resources I needed to build:

  • FargateCluster
  • FargateProfile
  • KubernetesResource "A" and KubernetesResource "B" (k8sA and k8sB for brevity)
  • HelmChart "A", HelmChart "B", and HelmChart "C" (HelmA, HelmB, and HelmC` for brevity)

FargateCluster is an auto-detected dependency of FargateProfile, k8sA&B and HelmA/B/C. So far so good...

❗️ k8sA&B and HelmA/B/C can technically be created without FargateProfile being completed, but the resulting pods will be stuck in a Pending state forever until they are manually replaced because k8s tries to schedule them before there's any FargateProfile available to figure out where to put them. We can work around this by doing const fargateProfile = cluster.node.findChild('fargate-profile-' + cluster.fargateProfileName) and then explicitly calling <resource>.node.addDependency(fargateProfile) (for each k8s & helm resource... and remembering to do that for every k8s-related resource that is added in the future).

❗️ HelmB and HelmC can technically be created without HelmA, but attempting to do so before HelmA (which, in my case, was the MetricsServer chart) has fully deployed will result in a Error: could not get apiVersions from Kubernetes: unable to retrieve the complete list of server APIs: metrics.k8s.io/v1beta1: the server is currently unable to handle the request. This can be worked around by using the --wait flag on the helm invocation (see #6276 for relevant PR) and adding an explicit dependency on HelmA to HelmB/C (and remembering to add it for any charts that are added in the future).

Proposed Solution

  1. Eliminate the need to call cluster.findChild(...) to find the FargateProfile (and find any other profiles) by automatically adding any of the cluster's associated FargateProfile resources as dependencies on any resources that depend on the cluster itself. This should easily be doable in conjunction with [aws-eks] Can't define a fargate cluster with more than one profile #6084.

  2. Create a simplified mechanism to add one resource as a dependency to several other resources without needing to bundle those resources into a separate construct.

  3. Extend mechanism in (2) to perhaps define a more complex dependency hierarchy.

  4. Assume that a dependency on a HelmChart resource implies that the --wait flag needs to be used (since helm install finishes pretty much immediately and has the actual install happen asynchronously in the background).

This is what my CDK code would look like after #6276 is merged without any other CDK modifications:

const cluster = new FargateCluster(...);
const fargateProfile = cluster.node.findChild('fargate-profile' + cluster.fargateProfileName);

const k8sA = new KubernetesResource(...);
k8sA.node.addDependency(fargateProfile);

const helmA = cluster.addChart(..., {... wait: true});
helmA.node.addDependency(fargateProfile);

new class extends cdk.Construct {
  constructor(parent: cdk.Construct, name: string) {
    super(parent, name);

    const helmB = cluster.addChart(...);
    helmB.node.addDependency(helmA);
    const helmC = cluster.addChart(...);
    helmC.node.addDependency(helmA);

    const k8sB = new KubernetesResource(...);
  }
}(this, '...').node.addDependency(k8sA);

For those unfamiliar, the new class extends cdk.Construct { ...}(this, '...') is an anonymous class and can be thought of along the same lines as you'd think of a self-executing anonymous function (e.g. (function (x) { console.log('Hello ' + x); })('world');. I used this technique as a shortcut to avoid having to create a separate "wrapper" Construct in a separate file and pass a bunch of parameters to it. This way, the cluster, fargateProfile, helmA, etc., values are still in scope.

Anyway, here's how I'd envision the above code looking if there was a generalized multi-dependency API available instead:

const cluster = new FargateCluster(...);

this.chainOrSomeOtherMuchBetterName([
  [
    new KubernetesResource(...) /* k8sA */,
    cluster.addChart(...) /* helmA */
  ],
  [
    new KubernetesResource(...) /* k8sB */,
    cluster.addChart(...) /* helmB */
    cluster.addChart(...) /* helmC */
  ]
]);

OK, what's going on there? How does CDK know what to do when? Here's how implementation of what I suggest above would allow us to get rid of a lot of the code needed earlier:

  1. We can get rid of the .findChild('fargate-profile') call because both KubernetesResource and HelmChart already depend on cluster, and my proposed changes would automatically create a dependency on associated FargateProfile constructs for anything that depends on the FargateCluster.

  2. The .chainOrSometOtherMuchBetterName(...) method (sorry, I'm terrible at naming things!) accepts an array of arrays such that all resources listed in element N (a) automatically have a dependency on all resources in element N-1, and (b) do not necessarily have dependencies on each other. In other words, after cluster and its FargateProfile is done building (since, remember, everything depending on cluster now also implicitly depends on FargateProfile), k8sA and helmA start building concurrently. Once both k8sA and helmA are done building (i.e. all resources in element 0), k8sB, helmB, and helmC start building concurrently.

Other

I'm not too sure if this capability would be widely useful outside of the EKS space, but implementing the solution should be easy to generalize, so we might as well unless someone thinks it would be particularly harmful to allow where not necessary.

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

This is a 🚀 Feature Request

@michaelmoussa michaelmoussa added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Feb 16, 2020
@SomayaB SomayaB added management/rfc request for comments @aws-cdk/aws-ecs Related to Amazon Elastic Container @aws-cdk/core Related to core CDK functionality and removed @aws-cdk/aws-ecs Related to Amazon Elastic Container labels Feb 17, 2020
@eladb eladb changed the title RFC: Generalized build-order and/or simplified multi-dependencies EKS: Generalized build-order and/or simplified multi-dependencies Feb 23, 2020
@eladb eladb added @aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service and removed @aws-cdk/core Related to core CDK functionality labels Feb 23, 2020
@eladb
Copy link
Contributor

eladb commented Feb 23, 2020

Interesting ideas. Let's split this up into two issues:

  1. Ensuring that if a FargateCluster is defined, all KubernetesResources and HelmCharts will automatically depend on it so that it is always created first. We should probably implement that together with [aws-eks] Can't define a fargate cluster with more than one profile #6084

  2. Add something like the "sequence/chain" API that will allow users to specify groups of resources they want created concurrently and in sequence.

@eladb eladb added the effort/medium Medium work item – several days of effort label Feb 23, 2020
@michaelmoussa
Copy link
Contributor Author

@eladb sounds good! Will you create the separate issue or would you like me to?

As I mentioned previously, I can work on #6084, #6381, and the two items described in this issue, but it will be a few more weeks before I can spend any time on them. If they're not high enough priority right now to do ASAP, feel free to assign them to me to work on as time permits.

@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Mar 4, 2020
@eladb eladb added the p2 label Mar 9, 2020
@iliapolo iliapolo changed the title EKS: Generalized build-order and/or simplified multi-dependencies [aws-eks] Generalized build-order and/or simplified multi-dependencies Aug 16, 2020
@eladb eladb removed their assignment Feb 25, 2021
@iliapolo iliapolo removed their assignment Jun 27, 2021
@github-actions
Copy link

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 28, 2022
@github-actions github-actions bot closed this as completed Jul 3, 2022
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 closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. management/rfc request for comments p2
Projects
None yet
Development

No branches or pull requests

4 participants