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

[Suggestion] A hook in aws-eks for cdk8s Chart #48

Closed
askulkarni2 opened this issue Mar 5, 2020 · 13 comments
Closed

[Suggestion] A hook in aws-eks for cdk8s Chart #48

askulkarni2 opened this issue Mar 5, 2020 · 13 comments
Assignees
Labels
effort/small 1 day tops feature-request New/Enhanced functionality wanted

Comments

@askulkarni2
Copy link

Is your feature request related to a problem? Please describe.

We would like to abstract the details of provisioning AWS infrastructure (EKS, etc.) from our customers (data scientists). They provide a Spark docker container with the application code and we run it for them. We are leveraging a new capability in Apache Spark so that we can run the job on EKS. We have tested the solution using Cluster.addResource() of the aws-eks module and would love to use cdk8s in order to provide some reusable constructs for our customers.

Describe the solution you'd like
Here's a sample of what we are thinking of building.

main.ts:

import cdk = require('@aws-cdk/core');
import k8s = require('cdk8s');
import eks = require('@aws-cdk/aws-eks');
import { SparkEksJob } from './spark-eks-job';

class SparkPiChart extends k8s.Chart  {
  constructor(scope: cdk.Construct, ns: string) {
    super(scope, ns);

    // my spark job image
    new SparkEksJob(this, 'SparkPi', {
      image: 'mysparkimage',
      command: [
         "/bin/sh",
          "-c",
          "/opt/spark/bin/spark-submit",
          "/opt/spark/examples/jars/spark-examples_2.11-2.4.4.jar"
       ]
    });
  }
}

const jobApp = new k8s.App();
const sparkPi = new SparkPiChart(app, 'spark-pi-job');

// This will be its own construct, but leaving this here for simplicity
const cluster = new eks.Cluster(this, 'job-cluster');

// add the chart 
cluster.addChart(sparkPi);

// synthesize
cdk.app.synth();

spark-eks-job.ts:

import { Construct } from '@aws-cdk/core';
import { App, Chart } from 'cdk8s';

// imported constructs
import { Job } from './imports/k8s';

export interface SparkEksJobOptions {
  /** 
    * The Docker image to use for this service.
  */
  readonly image: string;

  /**
   * Command to run
   */
  readonly command: Array<string>;
}

export class SparkEksJob extends Construct {
  constructor(scope: Construct, ns: string, options: SparkEksJobOptions) {
    super(scope, ns);
    
    // the rest of my k8s Job definition
    new Job(this, 'SparkJob', {
      ...
    });
  }
}

Describe alternatives you've considered
AWS CDK + Helm charts

I can work with you on example implementation/testing.

@askulkarni2 askulkarni2 added the feature-request New/Enhanced functionality wanted label Mar 5, 2020
@tabern
Copy link
Contributor

tabern commented Mar 5, 2020

@askulkarni2 are you thinking this will generate two files, a CFN and K8s manifest, or are you thinking about the CDK8s supporting CRDs for AWS objects (ie: EKS clusters)?

@askulkarni2
Copy link
Author

I had the first option in mind based on our initial CDK based PoC. But I am very curious about the second one. Is there an existing example I can look for reference?

@so0k
Copy link

so0k commented Mar 6, 2020

for option 1, how would you bootstrap the manifests on the clusters? GitOps? you still need to bootstrap the gitops controller then ... kops does this through protokube constantly fetching channel manifests and apply changes when required

for EKS, is there anything that does this via CFN lifecycle hooks & lambdas?

EDIT: just noticed CFN supports helm charts as a resource and can bootstrap the gitops controller that way? https://docs.aws.amazon.com/cdk/api/latest/docs/aws-eks-readme.html#helm-charts

@eladb
Copy link
Contributor

eladb commented Mar 6, 2020

I think this issue is mainly about "option 1" - i.e. we want the k8s manifests to be applied to the EKS cluster as part when the AWS CDK apps is deployed. The aws-eks CDK module already supports applying k8s manifests to the cluster so this should be trivial to achieve.

As for option 2, if cdk8s is going to support any CRD, and therefore, if/when there will be CRDs for AWS resources, they will be supported.

@askulkarni2
Copy link
Author

@so0k we have a CodeBuild to cdk deploy our k8s manifests which uses the feature in aws-eks module that @eladb is talking about. It works quite well.

@eladb
Copy link
Contributor

eladb commented Mar 6, 2020

@askulkarni2 I think the API you propose (eksCluster.addChart(cdk8sChart)) makes sense and as I said should be easy to implement with the existing eksCluster.addResource(manifest). Let's start with this.

I was also thinking that we should consider a more "deep integration" between the two frameworks, so it will be possible to define constructs that include both AWS CDK resources and cdk8s resources and vend them as abstractions.

eladb pushed a commit that referenced this issue Mar 10, 2020
To simplify interoperability, expose `toJson()` methods on `ApiObject` and on `Chart` which render the Kubernetes JSON for the resource/chart.

Related to #48
eladb pushed a commit that referenced this issue Mar 10, 2020
To simplify interoperability, expose `toJson()` methods on `ApiObject` and on `Chart` which render the Kubernetes JSON for the resource/chart.

Related to #48
eladb pushed a commit that referenced this issue Mar 10, 2020
To improve interoperability of cdk8s with other frameworks, do not impose DNS_LABEL constraints on construct names. Instead, normalize them to DNS_LABEL in case they don't comply.

Related to #48
eladb pushed a commit that referenced this issue Mar 10, 2020
To improve interoperability of cdk8s with other frameworks, do not impose DNS_LABEL constraints on construct names. Instead, normalize them to DNS_LABEL in case they don't comply.

Related to #48
@eladb
Copy link
Contributor

eladb commented Mar 10, 2020

OK, the changes #63 and #64 enable the following experience:

import * as k8s from '../imports/k8s';

export class TestClusterStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    const cluster = new eks.Cluster(this, 'TestCluster');

    // this is where the magic happens - we define a new cdk8s chart within the
    // AWS CDK stack and add it as a resource to our EKS cluster by using `chart.toJson()`.
    const chart = new MyChart(this, 'HelloChart');
    cluster.addResource('HelloChart', ...chart.toJson());
  }
}

class MyChart extends cdk8s.Chart {
  constructor(scope: Construct, id: string) {
    super(scope, id);

    const selector = { app: 'hello-kubernetes' };

    new k8s.Service(this, 'service', {
      spec: {
        type: 'LoadBalancer',
        ports: [ { port: 80, targetPort: k8s.IntOrString.fromNumber(8080) } ],
        selector: selector
      }
    });

    new k8s.Deployment(this, 'deployment', {
      spec: {
        replicas: 3,
        selector: {
          matchLabels: selector
        },
        template: {
          metadata: {
            labels: selector
          },
          spec: {
            containers: [
              { name: 'app', image: 'paulbouwer/hello-kubernetes:1.7', ports: [ { containerPort: 8080 }]}
            ]
          }
        }
      }
    });
  }
}

We can offer an API from the aws-eks side to make this even more seamless: eks.addChart(k8sChart). But this is basically how it can be implemented, and already supported.

@RafalWilinski
Copy link

I've just tried running this:

import * as cdk from "@aws-cdk/core";
import * as eks from "@aws-cdk/aws-eks";
import * as iam from "@aws-cdk/aws-iam";

import { Chart } from "cdk8s";
import { Deployment, Service, IntOrString } from "../imports/k8s";

export class AwsCdkK8SFargateStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    const namespaceSelector = { namespace: "fargate" };

    const clusterAdmin = new iam.Role(this, "AdminRole", {
      assumedBy: new iam.AccountRootPrincipal(),
      managedPolicies: [
        iam.ManagedPolicy.fromAwsManagedPolicyName("AmazonEKSServicePolicy"),
        iam.ManagedPolicy.fromAwsManagedPolicyName("AmazonEKSClusterPolicy")
      ]
    });

    const policyStatement = new iam.PolicyStatement();
    policyStatement.addAllResources();
    policyStatement.addActions(
      "elasticloadbalancing:*",
      "ec2:CreateSecurityGroup",
      "ec2:Describe*"
    );

    clusterAdmin.addToPolicy(policyStatement);

    const cluster = new eks.Cluster(this, "k8s-eks-fargate-cluster", {
      clusterName: "k8s-eks-cluster",
      outputConfigCommand: true,
      mastersRole: clusterAdmin,
      version: "1.15",
      defaultCapacity: 0
    });

    cluster.addFargateProfile("k8s-eks-cluster-fargate-profile", {
      selectors: [namespaceSelector]
    });

    const chart = new HelloKubernetesChart(this, "HelloChart");
    cluster.addResource("HelloChart", ...chart.toJson());
  }
}

class HelloKubernetesChart extends Chart {
  constructor(scope: cdk.Construct, name: string) {
    super(scope, name);

    const label = { app: "hello-k8s" };

    new Service(this, "service", {
      spec: {
        type: "LoadBalancer",
        ports: [{ port: 80, targetPort: IntOrString.fromNumber(8080) }],
        selector: label
      }
    });

    new Deployment(this, "deployment", {
      spec: {
        replicas: 2,
        selector: {
          matchLabels: label
        },
        template: {
          metadata: { labels: label, namespace: "fargate" },
          spec: {
            containers: [
              {
                name: "hello-kubernetes",
                image: "paulbouwer/hello-kubernetes:1.7",
                ports: [{ containerPort: 8080 }]
              }
            ]
          }
        }
      }
    });
  }
}

And encountered following error:

> cdk deploy   

/Users/r/aws-cdk-k8s-fargate/node_modules/@aws-cdk/core/lib/stack.ts:1177
  for (const child of node.node.children) {
                                ^
TypeError: Cannot read property 'children' of undefined
    at cfnElements (/Users/r/aws-cdk-k8s-fargate/node_modules/@aws-cdk/core/lib/stack.ts:1177:33)
    at cfnElements (/Users/r/aws-cdk-k8s-fargate/node_modules/@aws-cdk/core/lib/stack.ts:1181:5)
    at AwsCdkK8SFargateStack.findTokens (/Users/r/aws-cdk-k8s-fargate/node_modules/@aws-cdk/core/lib/stack.ts:1088:27)
    at AwsCdkK8SFargateStack.prepare (/Users/r/aws-cdk-k8s-fargate/node_modules/@aws-cdk/core/lib/stack.ts:747:25)
    at AwsCdkK8SFargateStack.onPrepare (/Users/r/aws-cdk-k8s-fargate/node_modules/@aws-cdk/core/lib/construct-compat.ts:105:10)
    at Node.prepare (/Users/r/aws-cdk-k8s-fargate/node_modules/constructs/lib/construct.ts:441:28)
    at Node.synthesize (/Users/r/aws-cdk-k8s-fargate/node_modules/constructs/lib/construct.ts:399:10)
    at Function.synth (/Users/r/aws-cdk-k8s-fargate/node_modules/@aws-cdk/core/lib/construct-compat.ts:225:22)
    at App.synth (/Users/r/aws-cdk-k8s-fargate/node_modules/@aws-cdk/core/lib/app.ts:141:36)
    at process.App.process.once (/Users/r/aws-cdk-k8s-fargate/node_modules/@aws-cdk/core/lib/app.ts:120:45)

Dependencies:

    "@aws-cdk/aws-eks": "^1.30.0",
    "@aws-cdk/core": "1.30.0",
    "cdk8s": "^0.17.0",

CLI:

$ cdk --version
1.30.0 (build 4f54ff7)

Any ideas?

@eladb
Copy link
Contributor

eladb commented Mar 21, 2020

Use 0.16.0 for now. 0.17.0 is incompatible with CDK 1.30.0. Hopefully this will get resolved in CDK 1.31.0

@eladb eladb added the effort/small 1 day tops label Jun 1, 2020
@J11522
Copy link

J11522 commented Aug 11, 2020

Hey, is there any update on this?
It seems even with the current versions of cdk8s 0.27.0 and CDK 1.57.0 there is an incompatibility.
I've tracked it down to different versions of constructs being required as it seems.

Thanks

@eladb eladb added the p1 label Aug 17, 2020
@eladb eladb assigned eladb and iliapolo and unassigned eladb Aug 17, 2020
@iliapolo
Copy link
Member

To anyone interested in this, please follow and upvote this issue: aws/aws-cdk#9670.

We will be updating the progress over there.

@iliapolo iliapolo added the cdk8s label Sep 14, 2020
@iliapolo
Copy link
Member

@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/small 1 day tops feature-request New/Enhanced functionality wanted
Projects
None yet
Development

No branches or pull requests

7 participants