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

Upgrade AdmissionReview e2e test image to also support v1 #81271

Merged
merged 1 commit into from Aug 15, 2019

Conversation

@jpbetz
Copy link
Contributor

commented Aug 11, 2019

Update webhook test image support v1beta1 and v1 AdmissionReview APIs
Arrange code so that internally, test admission handlers convert v1beta1 API requests to v1 before handling, but can easily handle versions via distinct code paths if needed in the future

What type of PR is this?
/kind cleanup

What this PR does / why we need it:

Special notes for your reviewer:

I am unfamiliar with how to test or publish the agnhost images.

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

/cc @liggitt @roycaihw @sttts @caesarxuchao
/sig api-machinery
/area admission-control

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

commented Aug 12, 2019

@bclau What do I need to do to test the agnhost image change in this PR?

@bclau

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

@bclau What do I need to do to test the agnhost image change in this PR?

So, first of all, since this changes webhook, we'll have to bump the image version. bump the version in test/images/agnhost/VERSION. Next, the agnhost version will also have to be bumped in test/utils/image/manifest.go.

I don't think the image promotion automation finished yet, so someone who can build and push the image to gcr.io will have to build and push it.

/cc @spiffxp

@k8s-ci-robot k8s-ci-robot requested a review from spiffxp Aug 12, 2019

@jpbetz jpbetz force-pushed the jpbetz:webhok-agnhost-v1 branch from 1b6ed0b to d499a45 Aug 12, 2019

test/images/agnhost/webhook/addlabel.go Show resolved Hide resolved
test/images/agnhost/webhook/main.go Outdated Show resolved Hide resolved
test/images/agnhost/webhook/main.go Outdated Show resolved Hide resolved

@liggitt liggitt added this to Required for GA, in progress in Admission Webhooks Aug 13, 2019

@jpbetz jpbetz force-pushed the jpbetz:webhok-agnhost-v1 branch from d499a45 to 0e36b6b Aug 13, 2019

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

commented Aug 13, 2019

@bclau One more question: Is there a recipe for running e2e tests against a not-yet-published agnhost image. E.g steps to publish the image to my local docker repo and update e2e tests to use it?

@jpbetz jpbetz force-pushed the jpbetz:webhok-agnhost-v1 branch 2 times, most recently from 342b193 to c5eb069 Aug 13, 2019

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

commented Aug 13, 2019

@bclau One more question: Is there a recipe for running e2e tests against a not-yet-published agnhost image. E.g steps to publish the image to my local docker repo and update e2e tests to use it?

Nevermind, I found a way to run it locally.

@jpbetz jpbetz force-pushed the jpbetz:webhok-agnhost-v1 branch from c5eb069 to eb94ad6 Aug 13, 2019

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

commented Aug 13, 2019

@bclau @spiffxp This PR updates the agnhost image and contains a test/images/agnhost/VERSION bump. What are next steps to have that image pushed? Can we push it before merging this PR once it is approved or will we need to update test/utils/image/manifest.go in a separate PR?

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

commented Aug 14, 2019

/retest

@jpbetz jpbetz changed the title [WIP] Upgrade AdmissionReview e2e test image to also support v1 Upgrade AdmissionReview e2e test image to also support v1 Aug 14, 2019

@jpbetz jpbetz force-pushed the jpbetz:webhok-agnhost-v1 branch 2 times, most recently from ac1c135 to 1545378 Aug 14, 2019

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

commented Aug 14, 2019

/retest

@liggitt

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

/approve
one godoc nit, lgtm otherwise

/hold
for agn image update coordination and verification of manual testing with v1 admissionreview (xref #81271 (comment))

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Aug 14, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jpbetz, 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

@jpbetz jpbetz force-pushed the jpbetz:webhok-agnhost-v1 branch from 1545378 to dbceb68 Aug 14, 2019

@liggitt

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

/lgtm

@liggitt

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

/retest

@jpbetz jpbetz force-pushed the jpbetz:webhok-agnhost-v1 branch from dbceb68 to 4f7543e Aug 14, 2019

@k8s-ci-robot k8s-ci-robot removed the lgtm label Aug 14, 2019

@liggitt

This comment has been minimized.

Copy link
Member

commented Aug 15, 2019

/retest
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Aug 15, 2019

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

commented Aug 15, 2019

e2e are passing in my development environment with new image.

Steps to reproduce:

 cd test/images/agnhost
 export VERSION=<pick something>                                                                                                                                     
 export REPO=<personal repo, e.g. gcr.io/username-images>
 <edit Dockerfile, remove CROSS_BUILD_COPY, set FROM to "alpine:3.6">
 docker build -t ${REPO}/agnhost:${VERSION} .
 docker push ${REPO}/agnhost:${VERSION}

 <edit test/utils/image/manifest.go, replace e2eRegistry with "${REPO}" and version with "${VERSION}">
 <edit test/e2e/apimachinery/webhook.go, setting "AdmissionReviewVersions: []string{"v1", "v1beta1"}" everywhere>

 kubetest --build --up --test --test_args="--ginkgo.focus=AdmissionWebhook"
@liggitt

This comment has been minimized.

Copy link
Member

commented Aug 15, 2019

thanks

/hold cancel

@k8s-ci-robot k8s-ci-robot merged commit 661e4d5 into kubernetes:master Aug 15, 2019

23 checks passed

cla/linuxfoundation jpbetz 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-iscsi Skipped.
pull-kubernetes-e2e-gce-iscsi-serial Skipped.
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-node-e2e-containerd 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

@k8s-ci-robot k8s-ci-robot added this to the v1.16 milestone Aug 15, 2019

@liggitt liggitt moved this from Required for GA, in progress to Complete in Admission Webhooks Aug 18, 2019

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

commented Aug 18, 2019

@spiffxp @bclau we need a build and push of the agnhost image. This PR bumped the version to 2.5.

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.