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: Adding static drift hash annotation #400

Merged
merged 1 commit into from
Jul 19, 2023
Merged

feat: Adding static drift hash annotation #400

merged 1 commit into from
Jul 19, 2023

Conversation

engedaam
Copy link
Contributor

Fixes #N/A

Description

  • Karpenter will create a provisioner hash of the fields that the team considers to be statically drifted
  • The will contain the values of each of those fields

How was this change tested?

  • make battletest

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 July 10, 2023 16:28
@engedaam engedaam requested a review from bwagner5 July 10, 2023 16:28
@coveralls
Copy link

coveralls commented Jul 10, 2023 •
edited
Loading

Pull Request Test Coverage Report for Build 5593479926

  • 32 of 41 (78.05%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 81.559%
Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/controllers/provisioner/controller.go 17 26 65.38%
Totals Coverage Status
Change from base Build 5590852323: 0.1%
Covered Lines: 7063
Relevant Lines: 8660
💛 - Coveralls

njtran
njtran reviewed Jul 10, 2023
View reviewed changes
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.

nice work! Some comments and nits.

pkg/apis/v1alpha5/provisioner.go Outdated Show resolved Hide resolved
pkg/apis/v1alpha5/provisioner.go Outdated Show resolved Hide resolved
pkg/apis/v1alpha5/provisioner.go Outdated Show resolved Hide resolved
pkg/controllers/provisioner/controller.go Outdated Show resolved Hide resolved
pkg/controllers/provisioner/controller.go Outdated Show resolved Hide resolved
pkg/controllers/provisioner/suite_test.go Outdated Show resolved Hide resolved
pkg/utils/functional/functional.go Outdated Show resolved Hide resolved
@njtran njtran mentioned this pull request Jul 12, 2023
Open
njtran
njtran reviewed Jul 14, 2023
View reviewed changes
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.

nice work! some more comments.

pkg/apis/v1alpha5/suite_test.go Outdated Show resolved Hide resolved
pkg/apis/v1alpha5/suite_test.go Outdated Show resolved Hide resolved
pkg/apis/v1alpha5/suite_test.go Show resolved Hide resolved
pkg/controllers/provisioner/suite_test.go Outdated Show resolved Hide resolved
pkg/controllers/provisioner/suite_test.go Outdated Show resolved Hide resolved
pkg/controllers/provisioner/suite_test.go Outdated Show resolved Hide resolved
pkg/controllers/provisioner/suite_test.go Outdated Show resolved Hide resolved
@engedaam engedaam force-pushed the static_drift_annotation branch 2 times, most recently from 6db4d72 to f490947 Compare July 14, 2023 20:42
jonathan-innis
jonathan-innis reviewed Jul 14, 2023
View reviewed changes
pkg/apis/v1alpha5/provisioner.go Outdated Show resolved Hide resolved
pkg/apis/v1alpha5/provisioner.go Outdated Show resolved Hide resolved
pkg/apis/v1alpha5/provisioner.go Outdated Show resolved Hide resolved
pkg/controllers/provisioner/controller.go Outdated Show resolved Hide resolved
pkg/controllers/provisioner/controller.go Show resolved Hide resolved
pkg/apis/v1alpha5/provisioner.go Outdated Show resolved Hide resolved
pkg/apis/v1alpha5/suite_test.go Outdated Show resolved Hide resolved
@engedaam engedaam force-pushed the static_drift_annotation branch 7 times, most recently from aac6fc6 to 99d6129 Compare July 18, 2023 15:31
njtran
njtran reviewed Jul 18, 2023
View reviewed changes
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.

Some more comments on tests. Good work though!

pkg/apis/v1alpha5/suite_test.go Outdated Show resolved Hide resolved
pkg/apis/v1alpha5/provisioner.go Outdated Show resolved Hide resolved
pkg/apis/v1alpha5/suite_test.go Outdated Show resolved Hide resolved
pkg/controllers/provisioner/controller.go Outdated Show resolved Hide resolved
pkg/controllers/provisioner/suite_test.go Show resolved Hide resolved
pkg/controllers/provisioner/suite_test.go Show resolved Hide resolved
@engedaam engedaam force-pushed the static_drift_annotation branch 4 times, most recently from bd1f780 to 047a83b Compare July 18, 2023 23:22
njtran
njtran approved these changes Jul 19, 2023
View reviewed changes
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.

LGTM!

@engedaam engedaam merged commit b7a0a8f into kubernetes-sigs:main Jul 19, 2023
6 checks passed
@engedaam engedaam deleted the static_drift_annotation branch July 19, 2023 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonathan-innis jonathan-innis jonathan-innis left review comments

@njtran njtran njtran approved these changes

@bwagner5 bwagner5 Awaiting requested review from bwagner5 bwagner5 was automatically assigned from aws/karpenter

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@engedaam @coveralls @njtran @jonathan-innis