Promote resourceQuota e2e verifying 'object count quota' and 'quota scope' to Conformance #78331

mgdevstack · 2019-05-25T05:54:47Z

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:
Promoting following e2e to Conformance -

test/e2e/apimachinery/resource_quota.go: "should create a ResourceQuota and ensure its status is promptly calculated."
Execution time: 13.192 seconds
test/e2e/apimachinery/resource_quota.go: "should create a ResourceQuota and capture the life of a service."
Execution time: 17.307 seconds
test/e2e/apimachinery/resource_quota.go: "should create a ResourceQuota and capture the life of a secret."
Execution time: 23.227 seconds
test/e2e/apimachinery/resource_quota.go: "should create a ResourceQuota and capture the life of a pod."
Execution time: 19.307 seconds
test/e2e/apimachinery/resource_quota.go: "should create a ResourceQuota and capture the life of a configMap."
Execution time: 22.230 seconds
test/e2e/apimachinery/resource_quota.go: "should create a ResourceQuota and capture the life of a replication controller."
Execution time: 17.307 seconds
test/e2e/apimachinery/resource_quota.go: "should create a ResourceQuota and capture the life of a replica set."
Execution time: 17.235 seconds
~~test/e2e/apimachinery/resource_quota.go: "should create a ResourceQuota and capture the life of a persistent volume claim."~~
Execution time: 17.231 seconds
~~test/e2e/apimachinery/resource_quota.go: "should create a ResourceQuota and capture the life of a persistent volume claim with a storage class."~~
Execution time: 17.243 seconds
test/e2e/apimachinery/resource_quota.go: "should verify ResourceQuota with terminating scopes."
Execution time: 22.325 seconds
test/e2e/apimachinery/resource_quota.go: "should verify ResourceQuota with best effort scope."
Execution time: 22.333 seconds
test/e2e/apimachinery/resource_quota.go: "Should be able to update and delete ResourceQuota."
Execution time: 6.206 seconds

Which issue(s) this PR fixes:

Fixes #78831

Special notes for your reviewer:

Do we still need to keep [Feature:ScopeSelectors] labelled here and [Feature:PodPriority] labelled here?

E2E from these context are candidate for Promotion, as [Feature:] labelled e2e are not promotable to Conformance and ScopeSelector and PodPriority is GAed, Could we remove [Feature] and consider for Promotion separately?

No Flakes found in kind cluster.

Does this PR introduce a user-facing change?:

None

/area conformance
/area testing
@kubernetes/sig-api-machinery-pr-reviews
@kubernetes/sig-architecture-pr-reviews
@kubernetes/cncf-conformance-wg

mgdevstack · 2019-05-25T12:12:09Z

/test pull-kubernetes-integration

fedebongio · 2019-05-31T20:04:50Z

/assign @yliaog

timothysc

very minor comment, but I didn't see anything in the tests that I think should be disqualifying.

When you make the update, I'll dig in deeper.

test/e2e/apimachinery/resource_quota.go

mgdevstack · 2019-06-11T06:28:53Z

test/e2e/apimachinery/resource_quota.go

@@ -22,7 +22,7 @@ import (
 	"time"

 	appsv1 "k8s.io/api/apps/v1"
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"


Note for reviewer: I suspect, recent gofmt or goimport adding this named import alias.

mgdevstack · 2019-06-11T14:21:51Z

/test pull-kubernetes-bazel-build

timothysc

/lgtm

I still need to go through the tests in detail but moving columns in case others have more time than me to approve.

test/e2e/apimachinery/resource_quota.go

bgrant0607 · 2019-07-19T18:21:31Z

test/conformance/testdata/conformance.txt

+test/e2e/apimachinery/resource_quota.go: "should create a ResourceQuota and capture the life of a configMap."
+test/e2e/apimachinery/resource_quota.go: "should create a ResourceQuota and capture the life of a replication controller."
+test/e2e/apimachinery/resource_quota.go: "should create a ResourceQuota and capture the life of a replica set."
+test/e2e/apimachinery/resource_quota.go: "should create a ResourceQuota and capture the life of a persistent volume claim."


Please remove the PV and PVC tests. They aren't ready for conformance yet. We don't have any PV or PVC tests yet.

bgrant0607 · 2019-07-19T19:01:21Z

test/e2e/apimachinery/resource_quota.go

+		Create a ConfigMap. Its creation MUST be successful and resource usage count against the ConfigMap object MUST be captured in ResourceQuotaStatus of the ResourceQuota.
+		Delete the ConfigMap. Deletion MUST succeed and resource usage count against the ConfigMap object MUST be released from ResourceQuotaStatus of the ResourceQuota.
+	*/
+	framework.ConformanceIt("should create a ResourceQuota and capture the life of a configMap.", func() {


Does this test really need to be so complicated, or did it just copy the secret test?

@mikedanese asked a similar question when this test was first introduced https://github.com/kubernetes/kubernetes/pull/68812/files#r230931734

Per comments below this is making an allowance for ca.crt to show up

Why do we need to do that? Is this just related to how we have configured the e2e cluster? If there is this race condition in pod startup, it would seem that users can hit this too, we shouldn't just work around it.

spiffxp

I agree with removing the PV/PVC tests but otherwise feel like clarifying comments and such are scope creep.

spiffxp · 2019-07-23T00:11:15Z

test/e2e/apimachinery/resource_quota.go

+		Description: Create a ResourceQuota. Creation MUST be successful and its ResourceQuotaStatus MUST match to expected used and total allowed resource quota count within namespace.
+		Create a Secret. Its creation MUST be successful and resource usage count against the Secret object and resourceQuota object MUST be captured in ResourceQuotaStatus of the ResourceQuota.
+		Delete the Secret. Deletion MUST succeed and resource usage count against the Secret object MUST be released from ResourceQuotaStatus of the ResourceQuota.
+	*/


It would be nice but I wouldn't block promoting on the lack of such a comment. The test has been like this for a while.

ref @smarterclayton #40340

On contended servers the service account controller can slow down,
leading to the count changing during a run. Wait up to 5s for the count
to stabilize, assuming that updates come at a consistent rate, and are
not held indefinitely.

spiffxp · 2019-07-23T00:33:14Z

test/e2e/apimachinery/resource_quota.go

+		Create a ConfigMap. Its creation MUST be successful and resource usage count against the ConfigMap object MUST be captured in ResourceQuotaStatus of the ResourceQuota.
+		Delete the ConfigMap. Deletion MUST succeed and resource usage count against the ConfigMap object MUST be released from ResourceQuotaStatus of the ResourceQuota.
+	*/
+	framework.ConformanceIt("should create a ResourceQuota and capture the life of a configMap.", func() {


@mikedanese asked a similar question when this test was first introduced https://github.com/kubernetes/kubernetes/pull/68812/files#r230931734

Per comments below this is making an allowance for ca.crt to show up

spiffxp · 2019-07-23T00:35:14Z

/lgtm cancel
please resolve the PV/PVC comments

spiffxp · 2019-07-23T00:35:53Z

/remove-kind api-change

…cope' to Conformance

xichengliudui · 2019-07-26T05:44:29Z

/retest

spiffxp · 2019-07-26T16:43:40Z

/lgtm

oomichi · 2019-07-31T01:47:20Z

There are not any big changes which seem related to flake in this year at least since f58c2ae (There is a lot of cleanup changes btw)
Non-GA API are not called in these tests.
There are not any skipping methods related to cloud providers.

/lgtm

spiffxp · 2019-08-07T21:08:03Z

/assign @bgrant0607 @smarterclayton
Looking for /approve

smarterclayton · 2019-08-08T14:22:19Z

/approve

Quota is fundamental to a Kube system

k8s-ci-robot · 2019-08-08T14:22:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mgdevstack, smarterclayton

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:

~~test/OWNERS~~ [smarterclayton]
~~test/conformance/testdata/OWNERS~~ [smarterclayton]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

fejta-bot · 2019-08-08T16:39:38Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2019-08-08T20:30:14Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

k8s-ci-robot requested review from timothysc and xichengliudui May 25, 2019 05:56

k8s-ci-robot assigned yliaog May 31, 2019

timothysc suggested changes Jun 11, 2019

View reviewed changes

test/e2e/apimachinery/resource_quota.go Outdated Show resolved Hide resolved

timothysc added this to the v1.16 milestone Jun 11, 2019

mgdevstack force-pushed the master-promote-resourcequota branch from 6ff2fb2 to 2b15e34 Compare June 11, 2019 06:05

mgdevstack commented Jun 11, 2019

View reviewed changes

timothysc added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Jun 11, 2019

k8s-ci-robot removed the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Jun 11, 2019

timothysc self-assigned this Jun 11, 2019

timothysc approved these changes Jun 11, 2019

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 11, 2019

yliaog reviewed Jun 11, 2019

View reviewed changes

test/e2e/apimachinery/resource_quota.go Outdated Show resolved Hide resolved

devaips mentioned this pull request Jun 12, 2019

Promote ResourceQuota Tests to Conformance #78831

Closed

mgdevstack force-pushed the master-promote-resourcequota branch from 2b15e34 to 2c6fbc1 Compare June 12, 2019 02:46

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 12, 2019

bgrant0607 reviewed Jul 19, 2019

View reviewed changes

spiffxp reviewed Jul 23, 2019

View reviewed changes

k8s-ci-robot assigned spiffxp Jul 23, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 23, 2019

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 23, 2019

k8s-ci-robot removed the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Jul 23, 2019

Promote resourceQuota e2e verifying 'object count quota' and 'quota s…

a921970

…cope' to Conformance

mgdevstack force-pushed the master-promote-resourcequota branch from bdd82e2 to a921970 Compare July 25, 2019 14:32

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 26, 2019

k8s-ci-robot assigned oomichi Jul 31, 2019

timothysc removed their assignment Aug 2, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 8, 2019

k8s-ci-robot merged commit 9d3d201 into kubernetes:master Aug 9, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Promote resourceQuota e2e verifying 'object count quota' and 'quota scope' to Conformance #78331

Promote resourceQuota e2e verifying 'object count quota' and 'quota scope' to Conformance #78331

mgdevstack commented May 25, 2019 •

edited

Loading

mgdevstack commented May 25, 2019

fedebongio commented May 31, 2019

timothysc left a comment

mgdevstack Jun 11, 2019 •

edited

Loading

mgdevstack commented Jun 11, 2019

timothysc left a comment

bgrant0607 Jul 19, 2019

mgdevstack Jul 25, 2019

bgrant0607 Jul 19, 2019

spiffxp Jul 23, 2019

johnbelamaric Aug 6, 2019

spiffxp left a comment •

edited

Loading

spiffxp Jul 23, 2019

spiffxp Jul 23, 2019

spiffxp commented Jul 23, 2019

spiffxp commented Jul 23, 2019

xichengliudui commented Jul 26, 2019

spiffxp commented Jul 26, 2019

oomichi commented Jul 31, 2019

spiffxp commented Aug 7, 2019

smarterclayton commented Aug 8, 2019

k8s-ci-robot commented Aug 8, 2019

fejta-bot commented Aug 8, 2019

fejta-bot commented Aug 8, 2019

Promote resourceQuota e2e verifying 'object count quota' and 'quota scope' to Conformance #78331

Promote resourceQuota e2e verifying 'object count quota' and 'quota scope' to Conformance #78331

Conversation

mgdevstack commented May 25, 2019 • edited Loading

mgdevstack commented May 25, 2019

fedebongio commented May 31, 2019

timothysc left a comment

Choose a reason for hiding this comment

mgdevstack Jun 11, 2019 • edited Loading

Choose a reason for hiding this comment

mgdevstack commented Jun 11, 2019

timothysc left a comment

Choose a reason for hiding this comment

bgrant0607 Jul 19, 2019

Choose a reason for hiding this comment

mgdevstack Jul 25, 2019

Choose a reason for hiding this comment

bgrant0607 Jul 19, 2019

Choose a reason for hiding this comment

spiffxp Jul 23, 2019

Choose a reason for hiding this comment

johnbelamaric Aug 6, 2019

Choose a reason for hiding this comment

spiffxp left a comment • edited Loading

Choose a reason for hiding this comment

spiffxp Jul 23, 2019

Choose a reason for hiding this comment

spiffxp Jul 23, 2019

Choose a reason for hiding this comment

spiffxp commented Jul 23, 2019

spiffxp commented Jul 23, 2019

xichengliudui commented Jul 26, 2019

spiffxp commented Jul 26, 2019

oomichi commented Jul 31, 2019

spiffxp commented Aug 7, 2019

smarterclayton commented Aug 8, 2019

k8s-ci-robot commented Aug 8, 2019

fejta-bot commented Aug 8, 2019

fejta-bot commented Aug 8, 2019

mgdevstack commented May 25, 2019 •

edited

Loading

mgdevstack Jun 11, 2019 •

edited

Loading

spiffxp left a comment •

edited

Loading