LimitRange e2e test improved. #55065

kmrov · 2017-11-03T16:11:23Z

What this PR does / why we need it: Improves the e2e test for LimitRange API by testing Update, Delete and Watch features.

Release note:

NONE

luxas · 2017-11-05T21:02:06Z

/ok-to-test

Also marks it as a Conformance test.

needs @kubernetes/sig-architecture-feature-requests (cc @timothysc) approval in a SIG meeting as well

kmrov · 2017-11-06T00:59:00Z

/retest

timothysc

/lgtm but we need @kubernetes/sig-architecture-pr-reviews to approve any conformance changes

@kmrov please add a release note though to callout any conformance additions.

timothysc · 2017-11-06T21:38:15Z

test/e2e/scheduling/limit_range.go

+		options := metav1.ListOptions{LabelSelector: selector.String()}
+		limitRanges, err := f.ClientSet.CoreV1().LimitRanges(f.Namespace.Name).List(options)
+		Expect(err).NotTo(HaveOccurred(), "failed to query for limitRanges")
+		Expect(len(limitRanges.Items)).To(Equal(0))


I would warn here b/c we've seen a number of test conditions fail on loaded clusters due to waiting on cleanup conditions. I realize that it shouldn't occur, but we've seen patterns of odd cleanup when running (N) overlapping test cases with namespaces names that are not completely unique.

What can you suggest to do to avoid the problems here?

timothysc · 2017-11-06T21:38:38Z

test/e2e/scheduling/limit_range.go

+		Expect(err).NotTo(HaveOccurred(), "failed to set up watch")
+
+		By("Submitting a LimitRange")
+		limitRange, err = f.ClientSet.CoreV1().LimitRanges(f.Namespace.Name).Create(limitRange)


Update vs. Create based on previous value of list.

Should Update at line 140 use a new LimitRange structure, not the old with changed limit value?

luxas · 2017-11-06T21:54:00Z

We definitely need an associated issue here. Is there any tracking issue / doc for changes to the Conformance suite over time?

bgrant0607 · 2017-11-07T16:15:42Z

@kmrov @timothysc
Please do not mark this as conformance now. I'll put it on the SIG Arch. agenda.

dims · 2017-11-16T02:05:09Z

Looks like the hold is for conformance annotation on the tests

bgrant0607 · 2017-11-16T14:55:45Z

Yes, the hold was due to the conformance tag. Since that was removed, I removed the hold.

Thanks for the test improvements.

kmrov · 2017-11-17T08:47:14Z

/priority important-longterm

luxas · 2017-11-17T16:30:36Z

/retest

luxas · 2017-11-17T16:31:03Z

@timothysc LGTM this for real as the Conformance tag was removed?

timothysc

/lgtm
/approve

xiangpengzhao · 2017-11-22T16:37:50Z

@timothysc a comment /approve no-issue is needed here :)
or
@kmrov you can create an issue for this PR and update the PR body accordingly to associate the PR with the issue (e.g., add Fixes: #issue-number to PR body), then the PR can get the approved label automatically (as @timothysc already commented /approve)

timothysc · 2017-11-22T17:05:53Z

/approve no-issue

k8s-github-robot · 2017-11-22T17:05:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kmrov, timothysc

Associated issue requirement bypassed by: timothysc

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~test/e2e/scheduling/OWNERS~~ [timothysc]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

spiffxp · 2017-11-22T22:26:32Z

/status in-progress
though honestly at this point it doesn't really matter, since it's approved+lgtm+passing

jberkus · 2017-11-22T23:58:34Z

/priority critical-urgent

/remove-priority important-longterm

adjusting priorities for code freeze

k8s-github-robot · 2017-11-23T00:00:22Z

[MILESTONENOTIFIER] Milestone Pull Request Current

@k82cn @kmrov @timothysc

Note: This pull request is marked as priority/critical-urgent, and must be updated every 1 day during code freeze.

Example update:

ACK.  In progress
ETA: DD/MM/YYYY
Risks: Complicated fix required

Pull Request Labels

sig/architecture sig/testing: Pull Request will be escalated to these SIGs if needed.
priority/critical-urgent: Never automatically move pull request out of a release milestone; continually escalate to contributor and SIG through all available channels.
kind/feature: New functionality.

Help

k8s-github-robot · 2017-11-23T03:49:33Z

Automatic merge from submit-queue (batch tested with PRs 52767, 55065, 55148, 56228, 56221). If you want to cherry-pick this change to another branch, please follow the instructions here.

k8s-github-robot assigned timothysc and k82cn Nov 3, 2017

k8s-ci-robot added sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. kind/feature Categorizes issue or PR as related to a new feature. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 5, 2017

kmrov force-pushed the master branch from fa18074 to 7101d09 Compare November 6, 2017 00:58

kmrov force-pushed the master branch from 7101d09 to 25da29f Compare November 6, 2017 01:09

timothysc approved these changes Nov 6, 2017

View reviewed changes

timothysc added this to the v1.9 milestone Nov 6, 2017

timothysc added the sig/testing Categorizes an issue or PR as relevant to SIG Testing. label Nov 6, 2017

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Nov 13, 2017

k8s-github-robot added the milestone/incomplete-labels label Nov 14, 2017

bgrant0607 added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 15, 2017

kmrov force-pushed the master branch 2 times, most recently from 8f4f43d to 41c20f4 Compare November 16, 2017 13:20

kmrov changed the title ~~LimitRange e2e test improved and marked as Conformance.~~ LimitRange e2e test improved. Nov 16, 2017

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Nov 16, 2017

kmrov force-pushed the master branch from 7fbb68a to 4f2f2e5 Compare November 16, 2017 13:31

LimitRange e2e test improved.

c77923d

kmrov force-pushed the master branch from 4f2f2e5 to c77923d Compare November 16, 2017 13:46

bgrant0607 removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 16, 2017

k8s-ci-robot added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Nov 17, 2017

k8s-github-robot added milestone/needs-approval and removed milestone/incomplete-labels labels Nov 17, 2017

timothysc approved these changes Nov 22, 2017

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 22, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 22, 2017

timothysc added the status/approved-for-milestone label Nov 22, 2017

k8s-github-robot added milestone/needs-attention and removed milestone/needs-approval labels Nov 22, 2017

k8s-ci-robot added the status/in-progress label Nov 22, 2017

k8s-github-robot removed the milestone/needs-attention label Nov 22, 2017

k8s-ci-robot added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. and removed priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Nov 22, 2017

xiangpengzhao mentioned this pull request Nov 23, 2017

MILESTONENOTIFIER comment is not so clear kubernetes/sig-release#37

Closed

k8s-github-robot merged commit 47611b3 into kubernetes:master Nov 23, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LimitRange e2e test improved. #55065

LimitRange e2e test improved. #55065

kmrov commented Nov 3, 2017 •

edited

Loading

luxas commented Nov 5, 2017

kmrov commented Nov 6, 2017

timothysc left a comment •

edited

Loading

timothysc Nov 6, 2017

kmrov Nov 10, 2017

timothysc Nov 6, 2017

kmrov Nov 10, 2017

luxas commented Nov 6, 2017

bgrant0607 commented Nov 7, 2017

dims commented Nov 16, 2017

bgrant0607 commented Nov 16, 2017

kmrov commented Nov 17, 2017

luxas commented Nov 17, 2017

luxas commented Nov 17, 2017

timothysc left a comment

xiangpengzhao commented Nov 22, 2017

timothysc commented Nov 22, 2017

k8s-github-robot commented Nov 22, 2017

spiffxp commented Nov 22, 2017

jberkus commented Nov 22, 2017

k8s-github-robot commented Nov 23, 2017

k8s-github-robot commented Nov 23, 2017

LimitRange e2e test improved. #55065

LimitRange e2e test improved. #55065

Conversation

kmrov commented Nov 3, 2017 • edited Loading

luxas commented Nov 5, 2017

kmrov commented Nov 6, 2017

timothysc left a comment • edited Loading

Choose a reason for hiding this comment

timothysc Nov 6, 2017

Choose a reason for hiding this comment

kmrov Nov 10, 2017

Choose a reason for hiding this comment

timothysc Nov 6, 2017

Choose a reason for hiding this comment

kmrov Nov 10, 2017

Choose a reason for hiding this comment

luxas commented Nov 6, 2017

bgrant0607 commented Nov 7, 2017

dims commented Nov 16, 2017

bgrant0607 commented Nov 16, 2017

kmrov commented Nov 17, 2017

luxas commented Nov 17, 2017

luxas commented Nov 17, 2017

timothysc left a comment

Choose a reason for hiding this comment

xiangpengzhao commented Nov 22, 2017

timothysc commented Nov 22, 2017

k8s-github-robot commented Nov 22, 2017

spiffxp commented Nov 22, 2017

jberkus commented Nov 22, 2017

k8s-github-robot commented Nov 23, 2017

k8s-github-robot commented Nov 23, 2017

kmrov commented Nov 3, 2017 •

edited

Loading

timothysc left a comment •

edited

Loading