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

chore: Remove unneeded webhooks for machines/provisioners #234

Conversation

jonathan-innis
Copy link
Member

@jonathan-innis jonathan-innis commented Mar 9, 2023

Fixes #

Description

  • Remove unneeded webhooks for machines/provisioners

How was this change tested?

make presubmit
helm diff upgrade --namespace karpenter karpenter charts/karpenter --reuse-values --three-way-merge

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

@jonathan-innis jonathan-innis requested a review from a team as a code owner March 9, 2023 01:02
@coveralls
Copy link

coveralls commented Mar 9, 2023

Pull Request Test Coverage Report for Build 4371972886

  • 0 of 1 (0.0%) changed or added relevant line in 1 file are covered.
  • 4 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.1%) to 80.828%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/apis/v1alpha5/provisioner_defaults.go 0 1 0.0%
Files with Coverage Reduction New Missed Lines %
pkg/controllers/machine/liveness.go 2 85.0%
pkg/operator/controller/typed.go 2 93.55%
Totals Coverage Status
Change from base Build 4359092715: -0.1%
Covered Lines: 6539
Relevant Lines: 8090

💛 - Coveralls

@ellistarn
Copy link
Contributor

What behavior are we driving with machine webhooks?

@jonathan-innis jonathan-innis force-pushed the add-webhooks-for-machines branch 3 times, most recently from de26bd2 to c1feb40 Compare March 9, 2023 06:50
@jonathan-innis jonathan-innis changed the title chore: Add webhooks for machines chore: Remove unneeded webhooks for machines/provisioners Mar 9, 2023
@jonathan-innis
Copy link
Member Author

What behavior are we driving with machine webhooks?

It's a good point. I was adding it for consistency but opting to just rip out the webhooks that we don't need right now

@jonathan-innis jonathan-innis force-pushed the add-webhooks-for-machines branch 2 times, most recently from 3a592b3 to 9ab1f55 Compare March 9, 2023 06:54
@jonathan-innis jonathan-innis enabled auto-merge (squash) March 9, 2023 06:55
Copy link
Contributor

@bwagner5 bwagner5 left a comment

Choose a reason for hiding this comment

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

LGTM

pkg/controllers/machine/controller.go Show resolved Hide resolved
@jonathan-innis jonathan-innis merged commit 7c33b16 into kubernetes-sigs:main Mar 9, 2023
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