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

test: Add unit test to validate the instancetype cache #5917

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

engedaam
Copy link
Contributor

Fixes #N/A

Description
Add unit test to validate the instancetype cache will give different set of instance type based on input
The tested value currently are

  • kubelet.kubeReserved
  • kubelet.systemReserved
  • kubelet.evictionHard
  • kubelet.evictionSoft
  • kubelet.maxPods
  • nodeClass.instanceStorePolicy
  • nodeClass.amiFamily
  • nodeClass.blockDeviceMapping.rootVolume
  • nodeClass.blockDeviceMapping.volumeSize
  • nodeClass.blockDeviceMapping.deviceName

How was this change tested?

  • N/A

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.

@engedaam engedaam requested a review from a team as a code owner March 22, 2024 16:51
@engedaam engedaam requested a review from jmdeal March 22, 2024 16:51
Copy link

netlify bot commented Mar 22, 2024

Deploy Preview for karpenter-docs-prod canceled.

Name Link
🔨 Latest commit 05d8d34
🔍 Latest deploy log https://app.netlify.com/sites/karpenter-docs-prod/deploys/6616d5ff898d000008b34db0

@engedaam engedaam force-pushed the add-instancetype-cache-test branch 3 times, most recently from 890fe97 to 7cdc5ac Compare March 22, 2024 18:47
@coveralls
Copy link

coveralls commented Mar 22, 2024

Pull Request Test Coverage Report for Build 8635855489

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 82.277%

Totals Coverage Status
Change from base Build 8624754642: 0.03%
Covered Lines: 5334
Relevant Lines: 6483

💛 - Coveralls

@engedaam engedaam force-pushed the add-instancetype-cache-test branch 6 times, most recently from bd72ea8 to bd387e8 Compare March 22, 2024 19:29
go.mod Outdated Show resolved Hide resolved
pkg/providers/instancetype/suite_test.go Show resolved Hide resolved
pkg/providers/instancetype/suite_test.go Outdated Show resolved Hide resolved
pkg/providers/instancetype/suite_test.go Show resolved Hide resolved
@engedaam engedaam force-pushed the add-instancetype-cache-test branch 3 times, most recently from f20cb17 to 7b915b1 Compare March 27, 2024 17:09
Copy link
Contributor

@njtran njtran left a comment

Choose a reason for hiding this comment

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

Is there some sort of reflection we can do to make sure that each of the values that are added into the InstanceTypesProvider.List() has a table entry? It seems like this does the job, but there's no test that would fail if someone just finds + replaces all the instances of the List() function definition to add a new parameter.

pkg/providers/instancetype/suite_test.go Outdated Show resolved Hide resolved
pkg/providers/instancetype/suite_test.go Outdated Show resolved Hide resolved
pkg/providers/instancetype/suite_test.go Outdated Show resolved Hide resolved
pkg/providers/instancetype/suite_test.go Outdated Show resolved Hide resolved
pkg/providers/instancetype/suite_test.go Show resolved Hide resolved
@engedaam engedaam force-pushed the add-instancetype-cache-test branch 3 times, most recently from 2f1ac4e to 66f4c0f Compare April 3, 2024 18:01
@engedaam engedaam force-pushed the add-instancetype-cache-test branch 5 times, most recently from 77fe86d to 2b2dde5 Compare April 10, 2024 16:04
@engedaam engedaam force-pushed the add-instancetype-cache-test branch from 2b2dde5 to 05d8d34 Compare April 10, 2024 18:10
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 🚀

@engedaam engedaam merged commit 07a895c into aws:main Apr 10, 2024
17 checks passed
@engedaam engedaam deleted the add-instancetype-cache-test branch April 10, 2024 18:17
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