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

Fix flaky crd e2e tests #62350

Merged
merged 1 commit into from
Apr 13, 2018
Merged

Fix flaky crd e2e tests #62350

merged 1 commit into from
Apr 13, 2018

Conversation

jennybuckley
Copy link

@jennybuckley jennybuckley commented Apr 10, 2018 •
edited
Loading

Fixes #62345

/sig api-machinery
/kind flake
/priority failing-test

From the issue:

All the tests that rely on creating a CustomResourceDefinition are flaky. This is because the test "AdmissionWebhook Should be able to deny custom resource creation" creates a validating webhook, called deny-crd.k8s.io, which denies all crd creations. This is an issue for any other test which will create a crd that gets run in parallel with that webhook test, causing them to fail with the message failed to create CustomResourceDefinition: admission webhook "deny-crd.k8s.io" denied the request: this webhook denies all requests

This PR changes the test webhook image to support handling a new "/crd" path which would accept and deny crd creations based on their labels, only denying if if the object has the label "webhook-e2e-test":"webhook-disallow", and updates the "AdmissionWebhook Should be able to deny custom resource creation" test to use that label.

Release note:

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 10, 2018
@k8s-ci-robot k8s-ci-robot added kind/flake Categorizes issue or PR as related to a flaky test. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. labels Apr 10, 2018
@jennybuckley
Copy link
Author

tests passed, squashing

liggitt
liggitt reviewed Apr 10, 2018
View reviewed changes
test/e2e/apimachinery/webhook.go Outdated
@@ -1150,24 +1150,15 @@ func registerValidatingWebhookForCRD(f *framework.Framework, context *certContex
Operations: []v1beta1.OperationType{v1beta1.Create},
Rule: v1beta1.Rule{
APIGroups: []string{"apiextensions.k8s.io"},
APIVersions: []string{"*"},
Copy link
Member

Choose a reason for hiding this comment

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

why this change?

Copy link
Author

Choose a reason for hiding this comment

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

I changed this line because the "/crd" endpoint in the webhook image expects version apiextensions.k8s.io/v1beta1. I wanted to make sure that the test wouldn't cause flaky behavior when we add v1 crd and add tests which create v1 crd objects

Copy link
Member

Choose a reason for hiding this comment

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

wouldn't we want to be sure we updated this test and image as part of that?

Copy link
Author

Choose a reason for hiding this comment

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

Okay I'll change this back but add a TODO about what to change to add support for v1

@jennybuckley jennybuckley force-pushed the crd-tests-flaky branch 2 times, most recently from 2fc89a0 to 9e8f27f Compare April 10, 2018 21:42
@jennybuckley
Copy link
Author

jennybuckley commented Apr 11, 2018 •
edited
Loading

@liggitt
Tests are passing, should be good to go.

Recommitting with an added a comment to warn people not to use the label "webhook-e2e-test":"webhook-disallow" on CRDs in other tests.

@jennybuckley jennybuckley force-pushed the crd-tests-flaky branch 3 times, most recently from e4e834a to e59cacd Compare April 11, 2018 21:06
@jennybuckley jennybuckley mentioned this pull request Apr 12, 2018
Merged
@wenjiaswe
Copy link
Contributor

cc @cheftako

@hzxuzhonghu
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 13, 2018
@hzxuzhonghu
Copy link
Member

@jennybuckley This pr also need to cherry-pick to 1.10 and 1.9.

@liggitt
Copy link
Member

liggitt commented Apr 13, 2018

/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 13, 2018
@k8s-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit ce1cab3 into kubernetes:master Apr 13, 2018
@jennybuckley jennybuckley mentioned this pull request Apr 13, 2018
Merged
@jennybuckley
Copy link
Author

@hzxuzhonghu
I don't think I can cherrypick to 1.9 without also picking #61609

@jennybuckley jennybuckley mentioned this pull request Apr 13, 2018
Merged
@hzxuzhonghu
Copy link
Member

Yes, that's true.

k8s-github-robot pushed a commit that referenced this pull request Apr 17, 2018
Merge pull request #62553 from jennybuckley/automated-cherry-pick-of-…
efe3d3a 
…#62350-upstream-release-1.10

Automatic merge from submit-queue.

Automated cherry pick of #62350: Fix flaky crd e2e tests

Cherry pick of #62350 on release-1.10.

#62350: Fix flaky crd e2e tests
k8s-github-robot pushed a commit that referenced this pull request Apr 17, 2018
Merge pull request #62559 from jennybuckley/automated-cherry-pick-of-…
9b8927c 
…#61609-#62350-upstream-release-1.9

Automatic merge from submit-queue.

Automated cherry pick of #61609: Cleanup CRD/CR confusion in webhook e2e tests #62350: Fix flaky crd e2e tests

Cherry pick of #61609 #62350 on release-1.9.

#61609: Cleanup CRD/CR confusion in webhook e2e tests
#62350: Fix flaky crd e2e tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liggitt liggitt liggitt left review comments

@ixdy ixdy Awaiting requested review from ixdy

@luxas luxas Awaiting requested review from luxas

@mkumatag mkumatag Awaiting requested review from mkumatag

Assignees

@liggitt liggitt

@hzxuzhonghu hzxuzhonghu

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. kind/flake Categorizes issue or PR as related to a flaky test. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@jennybuckley @wenjiaswe @hzxuzhonghu @liggitt @k8s-ci-robot @k8s-github-robot