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

Fix 1.22 Webhook Create by ensuring v1 is used #3736

Merged
merged 1 commit into from
Nov 13, 2021
Merged

Fix 1.22 Webhook Create by ensuring v1 is used #3736

merged 1 commit into from
Nov 13, 2021

Conversation

ukclivecox
Copy link
Contributor

What this PR does / why we need it:

  • Ensure v1 and not v1beta1 Webhook is created when managerCreateResources=true
  • Add extra logging to help with any failures and fixes missing catches of errors not handled

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

@axsaucedo
Copy link
Contributor

Updates look good, would be worth adding a note on the UPGRADING.md page just as heads up that this would be the case.

I just ran a test locally with 1.11.2 to test --set managerCreateResources=true to validate that the upgrade actually works, and I can confirm there are no issues, but also I've realised that there's actually no difference that this changes add as the webhook loaded from file is created as v1 even in 1.11.2 when managerCreateResources is true.

Another thing I noticed is that the webhook in the helm chart is created with name seldon-validating-webhook-configuration-seldon-system whereas the controller creates it with the name seldon-validating-webhook-configuration - may be worth aligning the names (can be done as separate PR)

/approve

@seldondev
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: axsaucedo

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@seldondev seldondev merged commit c352a5d into SeldonIO:master Nov 13, 2021
@seldondev
Copy link
Collaborator

Failed to merge this PR due to:

failed merging [3736]: [Method Not Allowed]

stephen37 pushed a commit to stephen37/seldon-core that referenced this pull request Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants