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

removed flaky watch code and added NewIndexerInformerWatcher for LimitRange test case #76328

Merged
merged 1 commit into from Jul 15, 2019
Merged

removed flaky watch code and added NewIndexerInformerWatcher for LimitRange test case #76328

merged 1 commit into from Jul 15, 2019

Conversation

pontiyaraja
Copy link
Contributor

@pontiyaraja pontiyaraja commented Apr 9, 2019 •
edited

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug

/kind cleanup

/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:

  • fixed the time flaky
  • removed flaky watch code and added NewIndexerInformerWatcher

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

  • removed flaky watch code and added NewIndexerInformerWatcher

Does this PR introduce a user-facing change?:

NONE

/area conformance
/area testing
/sig-testing
@kubernetes/cncf-conformance-wg
@spiffxp
@bgrant0607
@brahmaroutu

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. area/conformance Issues or PRs related to kubernetes conformance tests cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. area/test sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Apr 9, 2019
@pontiyaraja
Copy link
Contributor Author

/test pull-kubernetes-conformance-image-test
/test pull-kubernetes-e2e-gce-100-performance
/test pull-kubernetes-e2e-gce
/test pull-kubernetes-kubemark-e2e-gce-big

neolit123
neolit123 reviewed Apr 9, 2019
View reviewed changes
Copy link
Member

@neolit123 neolit123 left a comment

Choose a reason for hiding this comment

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

added minor comments but it seems that the test changes and promoting the PR to conformance should be separate PRs.

test/conformance/testdata/conformance.txt Outdated Show resolved Hide resolved
test/e2e/scheduling/limit_range.go Outdated Show resolved Hide resolved
test/e2e/scheduling/limit_range.go Outdated Show resolved Hide resolved
@neolit123
Copy link
Member

/priority important-longterm

@k8s-ci-robot k8s-ci-robot added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Apr 9, 2019
@pontiyaraja
Copy link
Contributor Author

added minor comments but it seems that the test changes and promoting the PR to conformance should be separate PRs.
This test can not promote to conformance without these changes, removed flaky watch.

@neolit123
Copy link
Member

This test can not promote to conformance without these changes, removed flaky watch.

will leave this to the approvers. my impression was that PRs that promote to conformance should only do the .txt addition and the It -> ConformanceIt change.

@bsalamat
Copy link
Member

I am not familiar enough with conformance testing and the requirements there to be able to review this PR.
@timothysc Could you please help here?

@pontiyaraja pontiyaraja changed the title promoted e2e should create a LimitRange with defaults and ensure pod has those defaults applied to conformance removed flaky watch code and added NewIndexerInformerWatcher for LimitRange test case Apr 22, 2019
@pontiyaraja
Copy link
Contributor Author

/retest

bsalamat
bsalamat reviewed Apr 22, 2019
View reviewed changes
Copy link
Member

@bsalamat bsalamat left a comment

Choose a reason for hiding this comment

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

What are we trying to fix here? Is this test flaky? I don't see flakiness in our test dashboard.

@pontiyaraja
Copy link
Contributor Author

What are we trying to fix here? Is this test flaky? I don't see flakiness in our test dashboard.

@bsalamat please check this issue #68066

timothysc
timothysc approved these changes Apr 26, 2019
View reviewed changes
Copy link
Member

@timothysc timothysc left a comment

Choose a reason for hiding this comment

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

/approve
/hold
/assign @timothysc

please address comments then lgtm

test/e2e/scheduling/limit_range.go Show resolved Hide resolved
test/e2e/scheduling/limit_range.go Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 26, 2019
@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

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. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jul 3, 2019
@pontiyaraja
Copy link
Contributor Author

/test cla/linuxfoundation

@pontiyaraja
Copy link
Contributor Author

/retest

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jul 6, 2019
@pontiyaraja
Copy link
Contributor Author

@pontiyaraja CCLA has been revoked and he may signup for Individual CLA. To close this PR, #79742 is created.

CLA added now @mgdevstack

@k8s-ci-robot k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 7, 2019
@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

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. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 7, 2019
@pontiyaraja pontiyaraja force-pushed the limitRangeCNF branch 3 times, most recently from 9522bf1 to 518ebac Compare July 7, 2019 05:45
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jul 7, 2019
@timothysc timothysc removed this from Needs Approval in conformance-definition Jul 8, 2019
@pontiyaraja
Copy link
Contributor Author

/test pull-kubernetes-e2e-gce-100-performance

timothysc
timothysc approved these changes Jul 15, 2019
View reviewed changes
Copy link
Member

@timothysc timothysc left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 15, 2019
@timothysc timothysc removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 15, 2019
@k8s-ci-robot k8s-ci-robot merged commit d0dc9c8 into kubernetes:master Jul 15, 2019
@Huang-Wei Huang-Wei mentioned this pull request Aug 8, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neolit123 neolit123 neolit123 left review comments

@bsalamat bsalamat bsalamat left review comments

@timothysc timothysc timothysc approved these changes

@smarterclayton smarterclayton Awaiting requested review from smarterclayton

@mgdevstack mgdevstack Awaiting requested review from mgdevstack

Assignees

@timothysc timothysc

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/conformance Issues or PRs related to kubernetes conformance tests area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note-none Denotes a PR that doesn't merit a release note. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/testing Categorizes an issue or PR as relevant to SIG Testing. 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
9 participants
@pontiyaraja @neolit123 @bsalamat @k8s-ci-robot @brahmaroutu @smarterclayton @mgdevstack @timothysc @bgrant0607