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.

Sign up for GitHub

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

Jump to bottom

Adding support for Kubernetes 1.22 through validation webhook v1 #3667

Merged
merged 6 commits into from
Oct 19, 2021
Merged

Adding support for Kubernetes 1.22 through validation webhook v1 #3667

merged 6 commits into from
Oct 19, 2021

Conversation

axsaucedo
Copy link
Contributor

@axsaucedo axsaucedo commented Oct 12, 2021 •
edited
Loading

Fixes #3618

PR includes:

  • Updating to controllergen 0.5.0
  • Updating seldondeployment_webhook.go annotations for v1 validation webhook
  • Generating manifests with make -C operator manifests_all
  • Updating kustomize files to point to v1, and update to reference only 1 webhook (as new one generated dropped v1alphaX)
  • Generating kustomize with make -C operator generate-resources
  • Updating split_resources.py to also only reference first webhook
  • Generate helm charts with make -C operator/helm create_helper

Tested successfully in k8s 1.22 using:

  • kind create cluster --name seldon --image kindest/node:v1.22.1
  • helm upgrade --install seldon-core helm-charts/seldon-core-operator/ --namespace seldon-system --set istio.enabled="true" --set istio.gateway="seldon-gateway.istio-system.svc.cluster.local" --set ambassador.enabled="true"
  • istioctl install -y (v1.11)

OUtstanding:

  • Validate the currently commented line in split_resources "cert-manager.io/inject-ca-from" in res["metadata"]["annotations"]
  • Tested with K8s 1.22
  • Tested with K8s 1.18

@axsaucedo axsaucedo requested a review from ukclivecox October 12, 2021 08:01
@seldondev
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign axsaucedo
You can assign the PR to them by writing /assign @axsaucedo in a comment when ready.

The full list of commands accepted by this bot can be found 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

@axsaucedo
Copy link
Contributor Author

/test integration

@axsaucedo
Copy link
Contributor Author

/test notebooks

@axsaucedo
Copy link
Contributor Author

/test notebooks

@axsaucedo
Copy link
Contributor Author

/test integration

2 similar comments
@axsaucedo
Copy link
Contributor Author

/test integration

@axsaucedo
Copy link
Contributor Author

/test integration

ukclivecox
ukclivecox reviewed Oct 12, 2021
View reviewed changes
helm-charts/seldon-core-operator/templates/webhook.yaml
@@ -67,121 +84,5 @@ webhooks:
resources:
- seldondeployments
sideEffects: None
- clientConfig:
Copy link
Contributor

Choose a reason for hiding this comment

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

Big deletion here - is this correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was automatically removed when running controller-gen against v1 instead of v1beta1

ukclivecox
ukclivecox reviewed Oct 12, 2021
View reviewed changes
operator/config/cert/patch_webhook.yaml
@@ -1,9 +1,3 @@
- op: replace
path: /webhooks/0/clientConfig/caBundle
value: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURRVENDQWltZ0F3SUJBZ0lVRkgwbG9uSUFGK2NjUGJnMmdjRlQ5R2JCaVhvd0RRWUpLb1pJaHZjTkFRRUwKQlFBd0x6RXRNQ3NHQTFVRUF3d2tRV1J0YVhOemFXOXVJRU52Ym5SeWIyeHNaWElnVjJWaWFHOXZheUJFWlcxdgpJRU5CTUNBWERURTVNVEF3TWpFek1qRXlNRm9ZRHpJNU9Ua3hNakF6TVRNeU1USXdXakF2TVMwd0t3WURWUVFECkRDUkJaRzFwYzNOcGIyNGdRMjl1ZEhKdmJHeGxjaUJYWldKb2IyOXJJRVJsYlc4Z1EwRXdnZ0VpTUEwR0NTcUcKU0liM0RRRUJBUVVBQTRJQkR3QXdnZ0VLQW9JQkFRQyt5UFhjWVRUc0NKODl6VEl3MzdyK0FndTlEcitrOFNxRQpMbUdvaW5qZmZBZTRUUHFDRTU5U21RS2VLZGJnZTVxdE1hcWwwTmhZUDNjRGlZR0hIeUlxRGU1L1Z3SHpzaDJECklxNDlaMDIwRUc0aWo2YzhScGt2S2FDd2VvOWVDZk9CRFFkWXZkUzk4b0xOMFE0WEFaSHdMR3pSdmNabmNtZmYKRHdBb2trZHUyU0h3eFc3T1RHYW5uaDYvak85K3I3ZnJqOTBFZURlNGNRelRGbVR3ZjZrWVZsUzluV1FiYndRago0VGVkUE44eFdrakw3RXhWUWZvTkQ3djltckg3T1o1azdQS2VuLzVnNUJqVmFOZDBlQ0hESml4K3VKLzZuK3QvCkxkZDZoeTdSVWQxSzVvMEtnSnQ2T1ZsSTNZczRDVUNTdVRpTTdydDRkdXVQalA2b3JNOXBBZ01CQUFHalV6QlIKTUIwR0ExVWREZ1FXQkJUa3kvSEQwV0RBeFJoL0t0UmZSWFdnWldLR1FqQWZCZ05WSFNNRUdEQVdnQlRreS9IRAowV0RBeFJoL0t0UmZSWFdnWldLR1FqQVBCZ05WSFJNQkFmOEVCVEFEQVFIL01BMEdDU3FHU0liM0RRRUJDd1VBCkE0SUJBUUJxc0FzMGFDVjFSMWNRa05vTkxQeEZ6R3hiUmJSRll1WHhDdHFXNTdNU3JDUmlSOXVjWUdWYW9Jb1YKZ1g0T2x2MXdxRC95eWU1aVhSVG0wV0ZrcVhpa3hqdmQ1L3NhSVlXOVRGQW53b0ZRMUJiRUhhb2RtbkJqaW4vNQp2bHRsZmJpanRNbUhSbHNZWm81aHZ4cFFJbXJ2azJveUErdDA3S1I5TWh4T0hRVGs5RXJkckd0RG1zc1p3cHpXCkxHS2NCMEJPbEtPTEVCRmFBUEdST2hUamdXdXJqejJ6TXdwS2JoNENnL0YyWXgvek8yaHFjbTRoNmUyU2RueFoKVDM1VGF5UzdHYnRNaG9oeU1WMTc2SVdRTmJzWkw3Rm1PK0xqU0JPR3FnT2ZmWWpGTzd5aENJdUdGblN2L2pBSgozSGhJVXhaN0lhajc2bE1VK0JjWEF5VjN6SmRkCi0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K
- op: replace
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the webhooks for v1alpha1 and v1alpha2 were deleted from the manifest, which means that this would error out as there's only one after running controllergen

@axsaucedo
Copy link
Contributor Author

/test integration

@axsaucedo
Copy link
Contributor Author

/test integration

@axsaucedo
Copy link
Contributor Author

Seems integration test succeeded
image

@axsaucedo
Copy link
Contributor Author

/test notebooks

@axsaucedo
Copy link
Contributor Author

NOtebook tests pass
image

@ukclivecox ukclivecox mentioned this pull request Oct 15, 2021
Closed
@ukclivecox ukclivecox merged commit 586de2b into SeldonIO:master Oct 19, 2021
@axsaucedo axsaucedo mentioned this pull request Nov 4, 2021
Closed
stephen37 pushed a commit to stephen37/seldon-core that referenced this pull request Dec 21, 2021
@axsaucedo @ukclivecox
Adding support for Kubernetes 1.22 through validation webhook v1 (Sel…
f32002c 
…donIO#3667)

* Added updates for validation webhook v1

* Updated copy commands for operator

* Updated to include split resources

* Updated the alpha versions

* Added upgrading page documentation

Co-authored-by: cliveseldon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ukclivecox ukclivecox ukclivecox left review comments

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

helm chart not compatible with k8s v1.22
3 participants
@axsaucedo @seldondev @ukclivecox