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: Add blockDeviceMappings fields to support multiple EBS volumes #41

Merged
merged 4 commits into from
Aug 18, 2024

Conversation

lindarr915
Copy link
Contributor

BREAKING CHANGE: remove blockDevice fields in helm values

BREAKING CHANGE: remove blockDevice fields in helm values
@lindarr915 lindarr915 requested a review from a team as a code owner July 29, 2024 03:50
@lindarr915 lindarr915 changed the title feat: add blockDeviceMappings fields to support multiple EBS volumes feat: Add blockDeviceMappings fields to support multiple EBS volumes Jul 29, 2024
@lindarr915
Copy link
Contributor Author

Relate to the issue #40

Copy link
Contributor

@vara-bonthu vara-bonthu left a comment

Choose a reason for hiding this comment

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

@lindarr915 Thanks, I have added one comment. Please make sure this change has been tested locally in your environment. I normally deploy the helm chart changes by pointing to local folder path with the changes.

blockDevice:
deviceName: /dev/xvda
blockDeviceMappings:
- deviceName: /dev/xvda
volumeSize: 100Gi
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to add ebs: here? Check the docs here https://karpenter.sh/docs/concepts/nodeclasses/#specblockdevicemappings

Copy link
Contributor Author

@lindarr915 lindarr915 Jul 29, 2024

Choose a reason for hiding this comment

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

Yes, I should have added ebs: here. My local test cases only covers overwriting the default values. Let me fix this.

BREAKING CHANGE: remove blockDevice fields in helm values
Comment on lines 4 to 5
version: 0.0.2
appVersion: 0.0.2
Copy link
Contributor

Choose a reason for hiding this comment

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

Changing this version might not make a difference since we are not publishing these Helm charts. You can keep it as 0.0.1.

I will create a new tag for the Data Add-ons, so that existing blueprints will still work with the old tags, and the new ones will need updating to below.

  source  = "aws-ia/eks-data-addons/aws"
  version = "~> 1.40" 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted back to 0.0.1

@@ -1,6 +1,6 @@
locals {
namespace = "karpenter-resources"
version = "0.0.1"
version = "0.0.2"
Copy link
Contributor

Choose a reason for hiding this comment

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

you can revert this change to 0.0.1

Copy link
Contributor

@lusoal lusoal left a comment

Choose a reason for hiding this comment

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

LGTM

@askulkarni2 askulkarni2 merged commit 9dc6d06 into aws-ia:main Aug 18, 2024
7 checks passed
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.

4 participants