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 Webhook failures in k8s 1.22+ #216

Merged
merged 1 commit into from
Feb 14, 2022

Conversation

Samze
Copy link
Contributor

@Samze Samze commented Feb 11, 2022

What this PR does / why we need it

Kubernetes 1.22 added a new subresource field kubernetes/kubernetes#100970. The knative pkg which is used the webhook had an issue that was resolved here knative/serving#11448.

This change bumps the knative pkg dependency which includes the fix but keeps it pinned on the 0.22 release branch.

In addition it adds 1.22.5 to the k8s testing matrix in Github actions.

fixes #214

Describe testing done for PR

  1. Created k8s 1.22/1.23 on Kind
  2. ko apply -f config/ with this change.
  3. k apply -f samples/spring-pet-clinic
  4. Verified the binding is successful.

Note

This bumps the knative pkg only two commits https://github.com/knative/pkg/commits/release-0.22 so should be a relatively low risk change.

@codecov
Copy link

codecov bot commented Feb 11, 2022

Codecov Report

Merging #216 (2d93189) into main (4074a7a) will not change coverage.
The diff coverage is n/a.

❗ Current head 2d93189 differs from pull request most recent head 96f8c33. Consider uploading reports for the commit 96f8c33 to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #216   +/-   ##
=======================================
  Coverage   94.34%   94.34%           
=======================================
  Files          16       16           
  Lines         637      637           
=======================================
  Hits          601      601           
  Misses         25       25           
  Partials       11       11           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4074a7a...96f8c33. Read the comment docs.

@@ -10,5 +10,5 @@ require (
k8s.io/apimachinery v0.19.16
k8s.io/client-go v0.19.16
k8s.io/code-generator v0.19.16
knative.dev/pkg v0.0.0-20210331065221-952fdd90dbb0 // pin to branch release-0.22
Copy link
Member

@rashedkvm rashedkvm Feb 12, 2022

Choose a reason for hiding this comment

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

Yes, the updated knative.dev/pkg@release-0.22 fixed the issue. Curious why is it pinned to an older version? Maybe worth testing with the latest knative.dev/pkg@release-1.2. I will update my sandbox with the latest and try using the samples.

Copy link
Member

Choose a reason for hiding this comment

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

I just verified knative.dev/pkg@release-1.2 has breaking changes. So the latest patch from knative.dev/pkg@release-0.22 is sufficient for now.

Kubernetes 1.22 added a new subresource field kubernetes/kubernetes#100970. The knative pkg which is used the webhook had an issue that was resolved here knative/serving#11448.

This change bumps the knative pkg dependency which includes the fix but keeps it pinned on the 0.22 release branch.

In addition it adds 1.22.5 to the k8s testing matrix in Github actions.

fixes vmware-tanzu#214
@rashedkvm rashedkvm merged commit 2386e29 into vmware-tanzu:main Feb 14, 2022
xyloman pushed a commit to xyloman/tanzu-application-platform-local-setup that referenced this pull request Feb 23, 2022
xyloman added a commit to xyloman/tanzu-application-platform-local-setup that referenced this pull request Feb 24, 2022
* Set kind cluster version to 1.21.9

Webook failures with service bindings
- vmware-tanzu/servicebinding#216
- vmware-tanzu/servicebinding#214

* kindest node 1.21.2 appears to be the latest version of k8s available for 1.21.x branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-not-required CLA not required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ServiceBinding reconciliation fails on kubernetes v1.22+
3 participants