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: on-demand capacity reservation support #7726

Merged
merged 16 commits into from
Feb 26, 2025

Conversation

jmdeal
Copy link
Contributor

@jmdeal jmdeal commented Feb 11, 2025

Fixes #N/A

Description
Implements the AWS provider component for ODCR support.

How was this change tested?

Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: #
  • No

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

@jmdeal jmdeal requested a review from a team as a code owner February 11, 2025 13:26
@jmdeal jmdeal requested a review from bwagner5 February 11, 2025 13:26
Copy link

netlify bot commented Feb 11, 2025

Deploy Preview for karpenter-docs-prod canceled.

Name Link
🔨 Latest commit 199602b
🔍 Latest deploy log https://app.netlify.com/sites/karpenter-docs-prod/deploys/67bf0011ecbd3100085a772b

Copy link
Contributor

@jonathan-innis jonathan-innis left a comment

Choose a reason for hiding this comment

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

First pass :) I didn't get through all of the files, though yet. I just looked over the provider and controller ones

Copy link
Contributor

@jonathan-innis jonathan-innis left a comment

Choose a reason for hiding this comment

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

Nice work! Most of the comments were small -- overall, looks reasonable to me

@jmdeal jmdeal force-pushed the feat/reserved-capacity branch 3 times, most recently from c0a96bd to cc6c3ad Compare February 21, 2025 11:44
@coveralls
Copy link

coveralls commented Feb 21, 2025

Pull Request Test Coverage Report for Build 13543211365

Details

  • 938 of 1088 (86.21%) changed or added relevant lines in 27 files are covered.
  • 5 unchanged lines in 4 files lost coverage.
  • Overall coverage increased (+1.5%) to 66.771%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/controllers/nodeclass/controller.go 18 19 94.74%
pkg/apis/v1/ec2nodeclass_status.go 4 6 66.67%
pkg/providers/instancetype/offering/provider.go 142 144 98.61%
pkg/providers/instancetype/types.go 47 49 95.92%
pkg/controllers/controllers.go 0 3 0.0%
pkg/errors/errors.go 0 3 0.0%
pkg/providers/amifamily/resolver.go 47 50 94.0%
pkg/fake/ec2api.go 43 47 91.49%
pkg/providers/capacityreservation/provider.go 52 57 91.23%
pkg/providers/instance/instance.go 117 122 95.9%
Files with Coverage Reduction New Missed Lines %
pkg/controllers/nodeclass/controller.go 1 69.16%
pkg/operator/operator.go 1 7.25%
pkg/providers/amifamily/resolver.go 1 87.5%
pkg/fake/utils.go 2 95.12%
Totals Coverage Status
Change from base Build 13509475622: 1.5%
Covered Lines: 6639
Relevant Lines: 9943

💛 - Coveralls

Copy link
Contributor Author

@jmdeal jmdeal left a comment

Choose a reason for hiding this comment

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

/karpenter snapshot

Copy link
Contributor

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-cc6c3ad5942fc69e00c1e8d2890288086347145e.
To install you must login to the ECR repo with an AWS account:

aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 021119463062.dkr.ecr.us-east-1.amazonaws.com

helm upgrade --install karpenter oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter --version "0-cc6c3ad5942fc69e00c1e8d2890288086347145e" --namespace "kube-system" --create-namespace \
  --set "settings.clusterName=${CLUSTER_NAME}" \
  --set "settings.interruptionQueue=${CLUSTER_NAME}" \
  --set controller.resources.requests.cpu=1 \
  --set controller.resources.requests.memory=1Gi \
  --set controller.resources.limits.cpu=1 \
  --set controller.resources.limits.memory=1Gi \
  --wait

@jmdeal jmdeal force-pushed the feat/reserved-capacity branch from 5095850 to 2e27014 Compare February 21, 2025 17:03
Copy link
Contributor

@jonathan-innis jonathan-innis left a comment

Choose a reason for hiding this comment

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

Really awesome work! We're so close!

@jmdeal jmdeal force-pushed the feat/reserved-capacity branch 4 times, most recently from 0d396ca to 010638f Compare February 25, 2025 03:17
needed to cache based on subnet zones, since NodeClaims don't get
zonal requirements injected from the nodeclass
@jmdeal jmdeal force-pushed the feat/reserved-capacity branch from 010638f to e3d1cdd Compare February 25, 2025 19:11
@jmdeal jmdeal force-pushed the feat/reserved-capacity branch from 30da57c to 9d6c1b0 Compare February 25, 2025 22:41
Copy link
Contributor Author

@jmdeal jmdeal left a comment

Choose a reason for hiding this comment

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

/karpenter snapshot

Copy link
Contributor

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-9d6c1b00848c0e18c0a009b93686d788452801ca.
To install you must login to the ECR repo with an AWS account:

aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 021119463062.dkr.ecr.us-east-1.amazonaws.com

helm upgrade --install karpenter oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter --version "0-9d6c1b00848c0e18c0a009b93686d788452801ca" --namespace "kube-system" --create-namespace \
  --set "settings.clusterName=${CLUSTER_NAME}" \
  --set "settings.interruptionQueue=${CLUSTER_NAME}" \
  --set controller.resources.requests.cpu=1 \
  --set controller.resources.requests.memory=1Gi \
  --set controller.resources.limits.cpu=1 \
  --set controller.resources.limits.memory=1Gi \
  --wait

Copy link
Contributor

@jonathan-innis jonathan-innis left a comment

Choose a reason for hiding this comment

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

No comments -- though I know we are still waiting on a few more updates and some E2Es

Copy link
Contributor Author

@jmdeal jmdeal left a comment

Choose a reason for hiding this comment

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

/karpenter snapshot

Copy link
Contributor

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-18d6949912e7212a2c68f340d3f03f81ac73ad86.
To install you must login to the ECR repo with an AWS account:

aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 021119463062.dkr.ecr.us-east-1.amazonaws.com

helm upgrade --install karpenter oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter --version "0-18d6949912e7212a2c68f340d3f03f81ac73ad86" --namespace "kube-system" --create-namespace \
  --set "settings.clusterName=${CLUSTER_NAME}" \
  --set "settings.interruptionQueue=${CLUSTER_NAME}" \
  --set controller.resources.requests.cpu=1 \
  --set controller.resources.requests.memory=1Gi \
  --set controller.resources.limits.cpu=1 \
  --set controller.resources.limits.memory=1Gi \
  --wait

@jmdeal jmdeal force-pushed the feat/reserved-capacity branch from 4d4cb55 to 61aa5f5 Compare February 26, 2025 10:52
Copy link
Contributor Author

@jmdeal jmdeal left a comment

Choose a reason for hiding this comment

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

/karpenter snapshot

Copy link
Contributor

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-61aa5f5f4bffce7908f6bc6f54ff4bb26cd4a633.
To install you must login to the ECR repo with an AWS account:

aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 021119463062.dkr.ecr.us-east-1.amazonaws.com

helm upgrade --install karpenter oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter --version "0-61aa5f5f4bffce7908f6bc6f54ff4bb26cd4a633" --namespace "kube-system" --create-namespace \
  --set "settings.clusterName=${CLUSTER_NAME}" \
  --set "settings.interruptionQueue=${CLUSTER_NAME}" \
  --set controller.resources.requests.cpu=1 \
  --set controller.resources.requests.memory=1Gi \
  --set controller.resources.limits.cpu=1 \
  --set controller.resources.limits.memory=1Gi \
  --wait

jonathan-innis
jonathan-innis previously approved these changes Feb 26, 2025
Copy link
Contributor

@jonathan-innis jonathan-innis left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

:shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit:

@jmdeal
Copy link
Contributor Author

jmdeal commented Feb 26, 2025

Unused import 😢

Copy link
Contributor

@jonathan-innis jonathan-innis left a comment

Choose a reason for hiding this comment

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

Spoke too soon 😄

LGTM 🚀

@jmdeal jmdeal merged commit eff767c into aws:main Feb 26, 2025
19 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.

3 participants