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

webhook: cache miss fallback to direct client for ScaledObject #6186

Conversation

wozniakjan
Copy link
Member

@wozniakjan wozniakjan commented Sep 25, 2024

When creating Deployment and ScaledObject as part of a single helm chart / manifests file, it's possible to hit a race condition when webhook rejects ScaledObject because the cached kubernetes client doesn't see the Deployment yet. This PR adds a fallback option with --cache-miss-to-direct-client command line parameter, when set to true, if there was a cache miss (GET returned error IsNotFound), validating webhook will attempt the same GET but using the direct REST kubernetes client.

Checklist

Fixes #5973

@wozniakjan wozniakjan force-pushed the webhook/cache_miss_fallback_to_direct_client branch from 9895f0d to 0796a20 Compare September 25, 2024 09:02
Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>
@wozniakjan wozniakjan force-pushed the webhook/cache_miss_fallback_to_direct_client branch from 0796a20 to 6d38faf Compare September 25, 2024 09:19
@wozniakjan
Copy link
Member Author

for helm, this is just a matter of

extraArgs:
  webhooks:
    cache-miss-to-direct-client: "true"

@wozniakjan wozniakjan marked this pull request as ready for review September 27, 2024 11:11
@wozniakjan wozniakjan requested a review from a team as a code owner September 27, 2024 11:11
@wozniakjan
Copy link
Member Author

wozniakjan commented Sep 27, 2024

/run-e2e
Update: You can check the progress here

@wozniakjan wozniakjan enabled auto-merge (squash) September 27, 2024 18:33
@wozniakjan
Copy link
Member Author

wozniakjan commented Sep 27, 2024

/run-e2e internal
Update: You can check the progress here

@JorTurFer
Copy link
Member

I think that the failure is related with this PR and not with your changes -> #6196

@zroubalik
Copy link
Member

zroubalik commented Oct 7, 2024

/run-e2e internal
Update: You can check the progress here

@zroubalik
Copy link
Member

zroubalik commented Oct 7, 2024

/run-e2e internal
Update: You can check the progress here

@JorTurFer JorTurFer closed this Oct 7, 2024
auto-merge was automatically disabled October 7, 2024 16:45

Pull request was closed

@JorTurFer JorTurFer reopened this Oct 7, 2024
@JorTurFer
Copy link
Member

JorTurFer commented Oct 7, 2024

/run-e2e internal
Update: You can check the progress here

@zroubalik
Copy link
Member

zroubalik commented Oct 7, 2024

/run-e2e azureeventgridtopic
Update: You can check the progress here

@zroubalik zroubalik self-assigned this Oct 8, 2024
@zroubalik zroubalik merged commit 0b7ac6d into kedacore:main Oct 11, 2024
18 checks passed
rickbrouwer pushed a commit to rickbrouwer/keda that referenced this pull request Oct 21, 2024
…ore#6186)

Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>
Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com>
mpechner-akasa pushed a commit to nrichardson-akasa/keda that referenced this pull request Nov 29, 2024
…ore#6186)

Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>
Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com>
Signed-off-by: michael pechner <mike.pechner@akasa.com>
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.

Keda admission webhook gives error "Deployment.apps not found" when deployment should exist
3 participants