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 multi-version CRD + admission webhook #79495

Merged
merged 1 commit into from Aug 7, 2019

Conversation

@liggitt
Copy link
Member

commented Jun 27, 2019

What type of PR is this?
/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:
xref #73752

Special notes for your reviewer:
This is a smaller workaround to address #73752 specifically for no-op schema conversion in CRDs (which is the only type of conversion supported in 1.13). Backporting the 1.14 fix in #74154 would change 100+ files and change the Admission interface signature

Does this PR introduce a user-facing change?:

Resolves a bug that prevented sending a multi-version custom resource to an admission webhook.

/cc @sttts @jpbetz

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2019

@liggitt: This PR is not for the master branch but does not have the cherry-pick-approved label. Adding the do-not-merge/cherry-pick-not-approved label.

To approve the cherry-pick, please ping the kubernetes/patch-release-team in a comment when ready.

See also Kuberentes Patch Releases

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt

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

@liggitt liggitt changed the title WIP - fix multi-version CRD + admission webhook fix multi-version CRD + admission webhook Jun 27, 2019

@liggitt

This comment has been minimized.

Copy link
Member Author

commented Jun 27, 2019

/priority important-soon

@liggitt liggitt added this to the v1.13 milestone Jun 27, 2019

@liggitt

This comment has been minimized.

Copy link
Member Author

commented Jun 27, 2019

also picked test fix 6b73b50 to address #76393 flake

@liggitt

This comment has been minimized.

Copy link
Member Author

commented Jun 27, 2019

/retest

@liggitt liggitt force-pushed the liggitt:crd-conversion-1.13 branch 2 times, most recently from 77ec232 to b0e2952 Jun 28, 2019

@liggitt

This comment has been minimized.

Copy link
Member Author

commented Jun 29, 2019

/retest

@yliaog

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

/cc @roycaihw

@davidvossel

This comment has been minimized.

Copy link

commented Jul 8, 2019

Is there anything blocking this from being merged?

@@ -36,6 +37,12 @@ func (c *convertor) ConvertToGVK(obj runtime.Object, gvk schema.GroupVersionKind
return obj, nil
}
out, err := c.Scheme.New(gvk)
if runtime.IsNotRegisteredError(err) {

This comment has been minimized.

Copy link
@sttts

sttts Jul 8, 2019

Contributor

add a comment why we do this hack. Otherwise lgtm.

@sttts

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2019

lgtm.

Feel free to self-tag after the comment is added.

@liggitt liggitt force-pushed the liggitt:crd-conversion-1.13 branch from b0e2952 to 4346919 Jul 8, 2019

@liggitt

This comment has been minimized.

Copy link
Member Author

commented Jul 8, 2019

added comment, tagging

@fejta-bot

This comment has been minimized.

Copy link

commented Aug 7, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@k8s-ci-robot k8s-ci-robot merged commit d1c41b5 into kubernetes:release-1.13 Aug 7, 2019

23 checks passed

cla/linuxfoundation liggitt authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Skipped.
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Skipped.
pull-kubernetes-e2e-gce-csi-serial Skipped.
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-iscsi Skipped.
pull-kubernetes-e2e-gce-iscsi-serial Skipped.
pull-kubernetes-e2e-gce-storage-slow Skipped.
pull-kubernetes-godeps Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Skipped.
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details

markusthoemmes added a commit to markusthoemmes/origin that referenced this pull request Aug 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.