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 scale and rollback subresources with admission webhooks, add integration tests #76849

Merged
merged 5 commits into from Apr 22, 2019

Conversation

@liggitt
Copy link
Member

commented Apr 19, 2019

What type of PR is this?
/kind feature

What this PR does / why we need it:

  • Fixes scale subresources when admission webhooks are present
  • Fixes rollback subresources when admission webhooks are present
  • Begins complete test coverage of validating and mutating admission webhooks (exempts some resources/subresources/verbs that need custom test implementations)

Which issue(s) this PR fixes:
Fixes #76649
Fixes #76107
Fixes #67221

Does this PR introduce a user-facing change?:

Admission webhooks are now properly called for `scale` and `deployments/rollback` subresources

/cc @caesarxuchao
/sig api-machinery

@liggitt

This comment has been minimized.

Copy link
Member Author

commented Apr 19, 2019

/cc @sbezverk

@k8s-ci-robot k8s-ci-robot requested a review from sbezverk Apr 19, 2019

@liggitt liggitt force-pushed the liggitt:crd_webhook_integration_tests branch from 7a7c18c to bc0a0d3 Apr 19, 2019

@caesarxuchao

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

/assign

@liggitt

This comment has been minimized.

Copy link
Member Author

commented Apr 19, 2019

oh, I need to tag it as an integration test for bazel, and maybe put it in a different package, since it uses the existing etcd setup. Passes locally when run standalone in ~15 seconds

@roycaihw

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

/cc

@k8s-ci-robot k8s-ci-robot requested a review from roycaihw Apr 19, 2019

@caesarxuchao
Copy link
Member

left a comment

Some initial thoughts.

Show resolved Hide resolved test/integration/apimachinery/admission_test.go Outdated
Show resolved Hide resolved test/integration/apimachinery/admission_test.go Outdated
Show resolved Hide resolved test/integration/apimachinery/admission_test.go Outdated
Show resolved Hide resolved test/integration/apimachinery/admission_test.go Outdated
Show resolved Hide resolved test/integration/framework/etcd.go Outdated
Show resolved Hide resolved test/integration/apimachinery/admission_test.go Outdated

@liggitt liggitt force-pushed the liggitt:crd_webhook_integration_tests branch from bc0a0d3 to f09532a Apr 20, 2019

@liggitt liggitt force-pushed the liggitt:crd_webhook_integration_tests branch 2 times, most recently from 0b49198 to ea28d56 Apr 20, 2019

@liggitt liggitt force-pushed the liggitt:crd_webhook_integration_tests branch from ea28d56 to ad2cf9b Apr 20, 2019

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Apr 20, 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 force-pushed the liggitt:crd_webhook_integration_tests branch from ad2cf9b to 4f36558 Apr 20, 2019

@liggitt liggitt changed the title Add integration tests covering admission webhooks Fix scale and rollback subresources with admission webhooks, add integration tests Apr 20, 2019

@liggitt liggitt force-pushed the liggitt:crd_webhook_integration_tests branch 2 times, most recently from ab3102e to e2b3dfa Apr 20, 2019

@liggitt

This comment has been minimized.

Copy link
Member Author

commented Apr 20, 2019

/retest

@liggitt liggitt force-pushed the liggitt:crd_webhook_integration_tests branch from e2b3dfa to eb0ccf4 Apr 20, 2019

@liggitt liggitt force-pushed the liggitt:crd_webhook_integration_tests branch from eb0ccf4 to 69042da Apr 20, 2019

@liggitt

This comment has been minimized.

Copy link
Member Author

commented Apr 20, 2019

Comments addressed, ready for re-review

@caesarxuchao
Copy link
Member

left a comment

Asked for a test to test the "custeromResource/scale" endpoint. Otherwise lgtm.

@caesarxuchao

This comment has been minimized.

Copy link
Member

commented Apr 22, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Apr 22, 2019

@k8s-ci-robot k8s-ci-robot merged commit 716344f into kubernetes:master Apr 22, 2019

20 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 Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-csi-serial Skipped.
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-storage-slow Skipped.
pull-kubernetes-godeps Skipped.
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-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details

@liggitt liggitt referenced this pull request Apr 22, 2019

Closed

Complete integration testing for admission webhooks #76907

5 of 5 tasks complete

@liggitt liggitt deleted the liggitt:crd_webhook_integration_tests branch Apr 22, 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.