BREAKING CHANGE: Refactor NodeClass controller #5949

engedaam · 2024-04-01T01:40:21Z

Fixes #N/A

Description

BREAKING CHANGE: the names for log keys produced by the NodeClass controller will change as such: nodeclass -> nodeclass.status, nodeclass.hash, nodeclass.termination
Refactored the NodeClass controller to be split into three controllers (termination, status, and hash)
This change will keep functionality of each controllers to one operation and easier to reason through
All logic of the NodeClass will remain consistent

How was this change tested?

make presubmit
/karpenter snapshot

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.

netlify · 2024-04-01T01:41:04Z

✅ Deploy Preview for karpenter-docs-prod canceled.

Name	Link
🔨 Latest commit	`046f9bf`
🔍 Latest deploy log	https://app.netlify.com/sites/karpenter-docs-prod/deploys/661a1570a2c1f4000836ddfd

coveralls · 2024-04-01T01:43:39Z

Pull Request Test Coverage Report for Build 8671291017

Details

187 of 253 (73.91%) changed or added relevant lines in 9 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 82.176%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/controllers/nodeclass/status/instanceprofile.go	9	11	81.82%
pkg/controllers/nodeclass/status/launchtemplate.go	7	9	77.78%
pkg/controllers/nodeclass/status/securitygroup.go	17	19	89.47%
pkg/controllers/nodeclass/status/subnet.go	19	21	90.48%
pkg/controllers/controllers.go	0	3	0.0%
pkg/controllers/nodeclass/status/ami.go	16	22	72.73%
pkg/controllers/nodeclass/status/controller.go	40	50	80.0%
pkg/controllers/nodeclass/hash/controller.go	44	58	75.86%
pkg/controllers/nodeclass/termination/controller.go	35	60	58.33%

Totals
Change from base Build 8671279743:	0.0%
Covered Lines:	5371
Relevant Lines:	6536

💛 - Coveralls

engedaam

/karpenter snapshot

github-actions · 2024-04-01T02:01:19Z

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-717e557142261aac1b225dd09c8640dee6826b55.
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-717e557142261aac1b225dd09c8640dee6826b55" --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

engedaam

/karpenter snapshot

github-actions · 2024-04-05T22:43:13Z

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-40f1b7c09ae609b19dca5b20a80a8cf756ad5adf.
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-40f1b7c09ae609b19dca5b20a80a8cf756ad5adf" --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

Nice refactor. I really like the changes! Assuming the logic is remaining consistent everywhere with the changes and the tests haven't changed, I like it.

Didn't look at the tests because I assumed that they were straight-copies from the different Context blocks inside of the nodeclass package suite_test.go. Let me know if this assumption doesn't hold and I can look at specific files.

pkg/controllers/controllers.go

pkg/controllers/nodeclass/hash/controller.go

pkg/controllers/nodeclass/termination/controller.go

pkg/operator/operator.go

engedaam

/karpenter snapshot

github-actions · 2024-04-08T05:58:07Z

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-603ae99a0323d235a03d0f68fc4e4a654518b6db.
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-603ae99a0323d235a03d0f68fc4e4a654518b6db" --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

engedaam

/karpenter snapshot

github-actions · 2024-04-11T16:25:15Z

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-1d5d6890c6330cdce724728e6f6a33521a2697fb.
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-1d5d6890c6330cdce724728e6f6a33521a2697fb" --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

pkg/controllers/nodeclass/events/events.go

pkg/controllers/nodeclass/status/ami.go

pkg/controllers/nodeclass/status/controller.go

pkg/controllers/nodeclass/termination/controller.go

website/content/en/preview/upgrading/upgrade-guide.md

pkg/controllers/nodeclass/status/instanceprofile.go

pkg/controllers/nodeclass/status/controller.go

jonathan-innis

LGTM 🚀

engedaam

/karpenter snapshot

github-actions · 2024-04-12T20:14:52Z

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-0b41c5bad17767fc06164e537ce239a264f66044.
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-0b41c5bad17767fc06164e537ce239a264f66044" --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

…into refactor-nodeclass

jonathan-innis

LGTM 🚀

engedaam force-pushed the refactor-nodeclass branch from 9fb1361 to 717e557 Compare April 1, 2024 01:47

engedaam commented Apr 1, 2024

View reviewed changes

engedaam force-pushed the refactor-nodeclass branch 4 times, most recently from 481921f to 40f1b7c Compare April 5, 2024 22:33

engedaam marked this pull request as ready for review April 5, 2024 22:34

engedaam requested a review from a team as a code owner April 5, 2024 22:34

engedaam requested a review from ljosyula April 5, 2024 22:34

engedaam commented Apr 5, 2024

View reviewed changes

jonathan-innis reviewed Apr 7, 2024

View reviewed changes

engedaam force-pushed the refactor-nodeclass branch from 40f1b7c to 5249cd2 Compare April 8, 2024 05:14

engedaam changed the title ~~chore: Refactor nodeclass controller~~ BREAKING CHANGE: Refactor nodeclass controller Apr 8, 2024

engedaam changed the title ~~BREAKING CHANGE: Refactor nodeclass controller~~ BREAKING CHANGE: Refactor NodeClass controller Apr 8, 2024

engedaam force-pushed the refactor-nodeclass branch 3 times, most recently from 64f90a3 to 603ae99 Compare April 8, 2024 05:44

engedaam commented Apr 8, 2024

View reviewed changes

engedaam force-pushed the refactor-nodeclass branch 5 times, most recently from e8e3599 to 1d5d689 Compare April 11, 2024 16:19

engedaam commented Apr 11, 2024

View reviewed changes

jonathan-innis reviewed Apr 12, 2024

View reviewed changes

engedaam force-pushed the refactor-nodeclass branch 2 times, most recently from d533366 to f361736 Compare April 12, 2024 17:07

jonathan-innis reviewed Apr 12, 2024

View reviewed changes

pkg/controllers/nodeclass/status/instanceprofile.go Outdated Show resolved Hide resolved

engedaam force-pushed the refactor-nodeclass branch 2 times, most recently from d72eee8 to 0e3ba57 Compare April 12, 2024 17:26

jonathan-innis reviewed Apr 12, 2024

View reviewed changes

pkg/controllers/nodeclass/status/controller.go Outdated Show resolved Hide resolved

refactor nodeclass controller

0b41c5b

engedaam force-pushed the refactor-nodeclass branch from 0e3ba57 to 0b41c5b Compare April 12, 2024 17:53

jonathan-innis previously approved these changes Apr 12, 2024

View reviewed changes

engedaam commented Apr 12, 2024

View reviewed changes

Merge branch 'main' of https://github.com/aws/karpenter-provider-aws …

3659d54

…into refactor-nodeclass

engedaam dismissed jonathan-innis’s stale review via 3659d54 April 12, 2024 22:42

engedaam enabled auto-merge (squash) April 12, 2024 22:44

jonathan-innis approved these changes Apr 13, 2024

View reviewed changes

Merge branch 'main' into refactor-nodeclass

046f9bf

engedaam merged commit 9a07a6d into aws:main Apr 13, 2024
16 checks passed

engedaam deleted the refactor-nodeclass branch April 14, 2024 01:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BREAKING CHANGE: Refactor NodeClass controller #5949

BREAKING CHANGE: Refactor NodeClass controller #5949

engedaam commented Apr 1, 2024 •

edited

Loading

netlify bot commented Apr 1, 2024 •

edited

Loading

coveralls commented Apr 1, 2024 •

edited

Loading

engedaam left a comment

github-actions bot commented Apr 1, 2024

engedaam left a comment

github-actions bot commented Apr 5, 2024

jonathan-innis left a comment

engedaam left a comment

github-actions bot commented Apr 8, 2024

engedaam left a comment

github-actions bot commented Apr 11, 2024

jonathan-innis left a comment

engedaam left a comment

github-actions bot commented Apr 12, 2024

jonathan-innis left a comment

BREAKING CHANGE: Refactor NodeClass controller #5949

BREAKING CHANGE: Refactor NodeClass controller #5949

Conversation

engedaam commented Apr 1, 2024 • edited Loading

netlify bot commented Apr 1, 2024 • edited Loading

✅ Deploy Preview for karpenter-docs-prod canceled.

coveralls commented Apr 1, 2024 • edited Loading

Pull Request Test Coverage Report for Build 8671291017

Details

💛 - Coveralls

engedaam left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 1, 2024

engedaam left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 5, 2024

jonathan-innis left a comment

Choose a reason for hiding this comment

engedaam left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 8, 2024

engedaam left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 11, 2024

jonathan-innis left a comment

Choose a reason for hiding this comment

engedaam left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 12, 2024

jonathan-innis left a comment

Choose a reason for hiding this comment

engedaam commented Apr 1, 2024 •

edited

Loading

netlify bot commented Apr 1, 2024 •

edited

Loading

coveralls commented Apr 1, 2024 •

edited

Loading