fix: Modify conversion webhook check to check for v1 version existence #1914
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jonathan-innis The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
jonathan-innis
force-pushed
the
modify-conversion-webhook-check
branch
from
6bb735e to
2e949b2
Compare
engedaam
reviewed
Jan 14, 2025
jmdeal
reviewed
Jan 15, 2025
jonathan-innis
force-pushed
the
modify-conversion-webhook-check
branch
2 times, most recently
from
0d81d83 to
0e525f7
Compare
jonathan-innis
force-pushed
the
modify-conversion-webhook-check
branch
from
0e525f7 to
c97a838
Compare
|
/lgtm |
k8s-ci-robot
added
the
do-not-merge/hold
k8s-ci-robot
added
the
lgtm
|
/unhold No more changes required from my side |
|
/unhold |
k8s-ci-robot
removed
the
do-not-merge/hold
k8s-ci-robot
merged commit 9fc1bbe
into
kubernetes-sigs:release-v0.37.x
12 checks passed
jonathan-innis
added a commit
to jonathan-innis/karpenter
that referenced
this pull request
Jan 15, 2025
jonathan-innis
added a commit
to jonathan-innis/karpenter
that referenced
this pull request
Jan 15, 2025
jonathan-innis
added a commit
to jonathan-innis/karpenter
that referenced
this pull request
Jan 15, 2025
jonathan-innis
added a commit
to jonathan-innis/karpenter
that referenced
this pull request
Jan 15, 2025
This was referenced Jan 15, 2025
Merged
Merged
Merged
jonathan-innis
added a commit
to jonathan-innis/karpenter
that referenced
this pull request
Jan 15, 2025
jonathan-innis
added a commit
to jonathan-innis/karpenter
that referenced
this pull request
Jan 15, 2025
jonathan-innis
added a commit
to jonathan-innis/karpenter
that referenced
this pull request
Jan 15, 2025
jonathan-innis
added a commit
to jonathan-innis/karpenter
that referenced
this pull request
Jan 15, 2025
This was referenced Jan 23, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #N/A
Description
This change updates the conversion webhook check that we added to the latest pre-v1.0 patch versions so that we crash when we have an issue calling the v1 conversion endpoint converting from v1beta1, not when we get any error -- this allows the v1 CRD to be deployed after the application is deployed.
This also logs errors if we get any unknown errors when calling this API just in case there are errors that we are getting in this health check that are unexpected and we don't see.
This also ensures that this health check will continually fire throughout the lifetime of the process so that if the CRD is updated and deployed later after the application is deployed or if the NodePools aren't deployed until after the process is started, the check continues to be executed.
We also need to ensure that we don't fail immediately when we restart the container so that we give an unhealthy container a chance to become healthy again -- if a Karpenter container was crahsing with a blocked network path to the conversion webhook and that network path has now opened back up again -- we need to allow for that container to go back to ready -- we can make that happen by ensuring that we don't crash on the first failure but checking that we have consecutive failures.
How was this change tested?
make presubmitBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.