Skip to content

Conversation

ecooke-macu
Copy link
Contributor

@ecooke-macu ecooke-macu commented Feb 25, 2023

Closes #525 and Closes #532

@buehler
Copy link
Collaborator

buehler commented Feb 27, 2023

I love your approach for the configuration of webhooks!

I question regarding the save method: Wouldn't it be the cleaner approach, if the save method would correctly check if the object already exists?

So instead of just using resource.Uid() is null, we could either use the try / catch with conflict exception mechanism to determine, or use the get method to search for an existing item. If it does exist -> update, if it doesn't -> create.

wdyt?

edit: I mean here: https://github.com/buehler/dotnet-operator-sdk/blob/master/src/KubeOps.KubernetesClient/KubernetesClient.cs#L116

@ecooke-macu
Copy link
Contributor Author

I went back and forth on whether to do the way you suggested or do it in the try/catch. I can change it to your way if you would like. Wouldn't take much work.

@ecooke-macu
Copy link
Contributor Author

I made the changes to the code to check if the validator or mutator exists and call update instead of delete/save within a try/catch.

@buehler buehler enabled auto-merge (squash) March 1, 2023 01:38
@buehler
Copy link
Collaborator

buehler commented Mar 1, 2023

Perfectly fine ;-)
Maybe the Save methods needs a change to actually search for an existing resource first. This results in two calls, but that should not bother the Kubernetes API.

@buehler buehler merged commit 4bc5868 into dotnet:master Mar 1, 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.

[feature]: Specify failurepolicy when MutatingWebhookConfiguration is created [bug]: Webhook installation is not idempotent

2 participants