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

feat(eks): helm chart support #5390

Merged
merged 25 commits into from
Dec 23, 2019
Merged

feat(eks): helm chart support #5390

merged 25 commits into from
Dec 23, 2019

Conversation

vlesierse
Copy link
Contributor

@vlesierse vlesierse commented Dec 12, 2019

Initial commit to support Helm using EKS. It makes use of the version 2.0.0-beta1 version of the aws-lambda-layer-kubectl Lambda Layer which includes Helm 3.0.

Example

new HelmChart(this, 'NginxIngress', {
    cluster,
    release: "nginx-ingress",
    namespace: "kube-system",
    chart: "nginx-ingress",
    repository: "https://helm.nginx.com/stable",
    values: {
        controller: {
            kind: "Deployment"
        }
    }
});
// Release with chart from absolute URL
new HelmChart(this, 'MyNginx', {
    cluster,
    chart: "https://example.com/charts/nginx-1.2.3.tgz"
});

Closes #3748 Deploy Helm chart to EKS via CDK

  • Include tests
  • Fix problem with unreachable Helm repository urls within the Lambda
  • Improve documentation
  • Changes according feedback

Additional use cases which are not part of this PR:

  • Support for stable/...and incubator/....
  • Reference charts or repositories using S3 Buckets which can only be accessed with the right IAM role.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@mergify
Copy link
Contributor

mergify bot commented Dec 12, 2019

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist
:

  • PR title type(scope): text
    • type: fix, feat, refactor go into CHANGELOG, chore is hidden
    • scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
    • text: use all lower-case, do not end with a period, do not include issue refs
  • PR Description
    • Rationale: describe rationale of change and approach taken
    • Issues: indicate issues fixed via: fixes #xxx or closes #xxx
    • Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
  • Testing
    • Unit test added. Prefer to add a new test rather than modify existing tests
    • CLI or init templates change? Re-run/add CLI integration tests
  • Documentation
    • README: update module README to describe new features
    • API docs: public APIs must be documented. Copy from official AWS docs when possible
    • Design: for significant features, follow design process

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@pahud
Copy link
Contributor

pahud commented Dec 12, 2019

Hey, according to this comment #5333 (comment) @eladb may release a new custom-resource design. Just FYI.

@vlesierse
Copy link
Contributor Author

Thanks for the heads up. Missed that one... will wait till the different design and adjust accordingly.

As more people are working on this it might be a good thing to bring it together in some way. I believe that CDK can be a great way of managing Kubernetes resources as code...

Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start!

packages/@aws-cdk/aws-eks/lib/helm-release.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-eks/lib/helm-release.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-eks/lib/helm-release.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-eks/lib/helm-release.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-eks/lib/helm-release.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-eks/lib/helm-release/index.py Outdated Show resolved Hide resolved
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@vlesierse vlesierse changed the title feat(eks): Add HelmRelease construct feat(eks): Add HelmChart construct Dec 16, 2019
@mergify
Copy link
Contributor

mergify bot commented Dec 16, 2019

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist
:

  • PR title type(scope): text
    • type: fix, feat, refactor go into CHANGELOG, chore is hidden
    • scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
    • text: use all lower-case, do not end with a period, do not include issue refs
  • PR Description
    • Rationale: describe rationale of change and approach taken
    • Issues: indicate issues fixed via: fixes #xxx or closes #xxx
    • Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
  • Testing
    • Unit test added. Prefer to add a new test rather than modify existing tests
    • CLI or init templates change? Re-run/add CLI integration tests
  • Documentation
    • README: update module README to describe new features
    • API docs: public APIs must be documented. Copy from official AWS docs when possible
    • Design: for significant features, follow design process

3 similar comments
@mergify
Copy link
Contributor

mergify bot commented Dec 16, 2019

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist
:

  • PR title type(scope): text
    • type: fix, feat, refactor go into CHANGELOG, chore is hidden
    • scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
    • text: use all lower-case, do not end with a period, do not include issue refs
  • PR Description
    • Rationale: describe rationale of change and approach taken
    • Issues: indicate issues fixed via: fixes #xxx or closes #xxx
    • Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
  • Testing
    • Unit test added. Prefer to add a new test rather than modify existing tests
    • CLI or init templates change? Re-run/add CLI integration tests
  • Documentation
    • README: update module README to describe new features
    • API docs: public APIs must be documented. Copy from official AWS docs when possible
    • Design: for significant features, follow design process

@mergify
Copy link
Contributor

mergify bot commented Dec 16, 2019

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist
:

  • PR title type(scope): text
    • type: fix, feat, refactor go into CHANGELOG, chore is hidden
    • scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
    • text: use all lower-case, do not end with a period, do not include issue refs
  • PR Description
    • Rationale: describe rationale of change and approach taken
    • Issues: indicate issues fixed via: fixes #xxx or closes #xxx
    • Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
  • Testing
    • Unit test added. Prefer to add a new test rather than modify existing tests
    • CLI or init templates change? Re-run/add CLI integration tests
  • Documentation
    • README: update module README to describe new features
    • API docs: public APIs must be documented. Copy from official AWS docs when possible
    • Design: for significant features, follow design process

@mergify
Copy link
Contributor

mergify bot commented Dec 16, 2019

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist
:

  • PR title type(scope): text
    • type: fix, feat, refactor go into CHANGELOG, chore is hidden
    • scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
    • text: use all lower-case, do not end with a period, do not include issue refs
  • PR Description
    • Rationale: describe rationale of change and approach taken
    • Issues: indicate issues fixed via: fixes #xxx or closes #xxx
    • Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
  • Testing
    • Unit test added. Prefer to add a new test rather than modify existing tests
    • CLI or init templates change? Re-run/add CLI integration tests
  • Documentation
    • README: update module README to describe new features
    • API docs: public APIs must be documented. Copy from official AWS docs when possible
    • Design: for significant features, follow design process

@eladb eladb changed the title feat(eks): Add HelmChart construct feat(eks): helm chart support Dec 16, 2019
@mergify
Copy link
Contributor

mergify bot commented Dec 16, 2019

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist
:

  • PR title type(scope): text
    • type: fix, feat, refactor go into CHANGELOG, chore is hidden
    • scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
    • text: use all lower-case, do not end with a period, do not include issue refs
  • PR Description
    • Rationale: describe rationale of change and approach taken
    • Issues: indicate issues fixed via: fixes #xxx or closes #xxx
    • Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
  • Testing
    • Unit test added. Prefer to add a new test rather than modify existing tests
    • CLI or init templates change? Re-run/add CLI integration tests
  • Documentation
    • README: update module README to describe new features
    • API docs: public APIs must be documented. Copy from official AWS docs when possible
    • Design: for significant features, follow design process

1 similar comment
@mergify
Copy link
Contributor

mergify bot commented Dec 16, 2019

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist
:

  • PR title type(scope): text
    • type: fix, feat, refactor go into CHANGELOG, chore is hidden
    • scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
    • text: use all lower-case, do not end with a period, do not include issue refs
  • PR Description
    • Rationale: describe rationale of change and approach taken
    • Issues: indicate issues fixed via: fixes #xxx or closes #xxx
    • Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
  • Testing
    • Unit test added. Prefer to add a new test rather than modify existing tests
    • CLI or init templates change? Re-run/add CLI integration tests
  • Documentation
    • README: update module README to describe new features
    • API docs: public APIs must be documented. Copy from official AWS docs when possible
    • Design: for significant features, follow design process

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

eladb
eladb previously approved these changes Dec 23, 2019
Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. A couple of minor changes.

packages/@aws-cdk/aws-eks/lib/helm-chart.ts Show resolved Hide resolved
packages/@aws-cdk/aws-eks/lib/helm-chart.ts Outdated Show resolved Hide resolved
@mergify mergify bot dismissed eladb’s stale review December 23, 2019 12:59

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

eladb
eladb previously requested changes Dec 23, 2019
Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a README entry?

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot dismissed eladb’s stale review December 23, 2019 14:17

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks so much for this great contribution. I know many people would love this!

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Dec 23, 2019

Thank you for contributing! Your pull request is now being automatically merged.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Dec 23, 2019

Thank you for contributing! Your pull request is now being automatically merged.

@mergify mergify bot merged commit 394313e into aws:master Dec 23, 2019
@pahud
Copy link
Contributor

pahud commented Dec 24, 2019

@vlesierse Thanks for the awesome work!

eladb pushed a commit that referenced this pull request Dec 24, 2019
* Added HelmRelease construct

* feat(eks): Add HelmRelease construct

* Fix some linting problems

* Remove trailing whitespace

* Add the possibility to specify the chart version

* Changes after code review

* Add shell=True to command execution

* Execute helm command in /tmp

* Write a correct values.yaml

* Add resources to integration tests

* Change require to import

* Lazy add HelmChartHandler

* Add integration tests for Helm

* Added convenience addChart to Cluster

* Fix integration test.

* Change addChart method to use options pattern

* Added @default and truncate default chart name

* Added the Helm entry to the README.md

Co-authored-by: Elad Ben-Israel <benisrae@amazon.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
@itajaja
Copy link

itajaja commented Jan 13, 2020

Hi, I just updated to cdk@1.20 and I see a very big diff, mostly role changes, is it due to this change? I see some helm-related assets, even if I am not doing anything with helm. is there a way to opt out? is this going to deploy helm?

here's an example of asset in the cdk.out folder:

image

@eladb
Copy link
Contributor

eladb commented Jan 13, 2020

Please see #5544 for details

@itajaja
Copy link

itajaja commented Jan 13, 2020

perfect, thanks!

@itajaja
Copy link

itajaja commented Jan 13, 2020

@eladb still, I will see helm assets, even though I am not using helm. is that expected?

@eladb
Copy link
Contributor

eladb commented Jan 13, 2020

Yes, the kubectl custom resource provider serves both helm and kubectl. Feel free to ignore.

@itajaja
Copy link

itajaja commented Jan 13, 2020

thanks for the clarification 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[aws-eks]: Deploy Helm chart to EKS via CDK
6 participants