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.

Sign up for GitHub

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

Jump to bottom

feat(eks): support Bottlerocket Nvidia AMIs #28287

Merged
merged 4 commits into from
Dec 8, 2023
Merged

feat(eks): support Bottlerocket Nvidia AMIs #28287

merged 4 commits into from
Dec 8, 2023

Conversation

lpizzinidev
Copy link
Contributor

Adds support for BOTTLEROCKET_ARM_64_NVIDIA and BOTTLEROCKET_x86_64_NVIDIA AMI types.

Closes #28241.

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

@aws-cdk-automation aws-cdk-automation requested a review from a team December 7, 2023 16:21
@github-actions github-actions bot added distinguished-contributor [Pilot] contributed 50+ PRs to the CDK effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 labels Dec 7, 2023
aws-cdk-automation
aws-cdk-automation previously requested changes Dec 7, 2023
View reviewed changes
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

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

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

@lpizzinidev
Copy link
Contributor Author

Exemption Request.
The default service quota for g4 instances is 0 so running an integration test here will require AWS support.
Also, not sure if it's necessary.

@aws-cdk-automation aws-cdk-automation added the pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. label Dec 7, 2023
@vinayak-kukreja vinayak-kukreja self-assigned this Dec 7, 2023
vinayak-kukreja
vinayak-kukreja previously requested changes Dec 7, 2023
View reviewed changes
Copy link
Contributor

@vinayak-kukreja vinayak-kukreja left a comment

Choose a reason for hiding this comment

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

Hey @lpizzinidev , thank you for this contribution. It looks good to me, could you separate the appconfig alpha change to a separate PR since it is breaking.

packages/@aws-cdk/aws-appconfig-alpha/lib/extension.ts Outdated
@@ -444,7 +444,7 @@ export class Extension extends Resource implements IExtension {
/**
* The name of the extension.
*/
public readonly name?: string;
public readonly name: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Callout:
Breaking Change.

packages/@aws-cdk/aws-appconfig-alpha/lib/extension.ts Outdated
@@ -565,7 +565,7 @@ export interface IExtension extends IResource {
/**
* The name of the extension.
*/
readonly name?: string;
readonly name: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Callout:
Breaking Change.

packages/@aws-cdk/aws-appconfig-alpha/lib/extension.ts Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey, could you add this as a separate PR. We can then mark it as a Breaking Change PR.

@vinayak-kukreja
Copy link
Contributor

Ohh, the build is failing.

@vinayak-kukreja
Copy link
Contributor

I see it is failing for - @aws-cdk/aws-appconfig-alpha:build. We can deal with that in the other PR.

@vinayak-kukreja vinayak-kukreja added the pr-linter/exempt-integ-test The PR linter will not require integ test changes label Dec 7, 2023
@aws-cdk-automation aws-cdk-automation dismissed their stale review December 7, 2023 22:18

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@mergify mergify bot dismissed vinayak-kukreja’s stale review December 8, 2023 07:30

Pull request has been modified.

@lpizzinidev
Copy link
Contributor Author

@vinayak-kukreja
Thanks for the review.
Actually extension.ts was added by mistake as I was reviewing this PR 😅.
My bad, this should be good now.

@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Dec 8, 2023
vinayak-kukreja
vinayak-kukreja approved these changes Dec 8, 2023
View reviewed changes
Copy link
Contributor

@vinayak-kukreja vinayak-kukreja left a comment

Choose a reason for hiding this comment

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

Hey @lpizzinidev , no worries 😄 Thank you for addressing the feedback.

@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 8, 2023

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Dec 8, 2023
@mergify mergify bot merged commit 6aa1b1b into aws:main Dec 8, 2023
13 checks passed
@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 8, 2023

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 00e97d9
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@github-actions github-actions bot mentioned this pull request Jan 1, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vinayak-kukreja vinayak-kukreja vinayak-kukreja approved these changes

@aws-cdk-automation aws-cdk-automation aws-cdk-automation left review comments

Assignees

@vinayak-kukreja vinayak-kukreja

Labels
distinguished-contributor [Pilot] contributed 50+ PRs to the CDK effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 pr-linter/exempt-integ-test The PR linter will not require integ test changes pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

(EKS): (missed AMI type BOTTLEROCKET_x86_64_NVIDIA)
3 participants
@lpizzinidev @vinayak-kukreja @aws-cdk-automation