tests: Removes node created by test #78862

claudiubelu · 2019-06-10T15:54:08Z

What type of PR is this?

/kind bug
/kind cleanup

What this PR does / why we need it:

The test "A node shouldn't be able to create another node" could create
a node during its run, but it doesn't delete it in this case.

This PR addresses this issue.

This will also reduce the run time of the test from 200 seconds to 20 (the extra time was spent waiting for the undeleted foo node to become Ready).

Which issue(s) this PR fixes:

Fixes #78861

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

mikedanese · 2019-06-11T16:36:54Z

test/e2e/auth/node_authz.go

@@ -175,6 +175,12 @@ var _ = SIGDescribe("[Feature:NodeAuthorizer]", func() {
 		}
 		ginkgo.By(fmt.Sprintf("Create node foo by user: %v", asUser))
 		_, err := c.CoreV1().Nodes().Create(node)


This create is not expected to succeed. In the case where there's not a bug in node authorization, there's no node to delete.

Yes, that's what it is expected to happen. But it managed to create a node for me, and having it exist prolongs the execution time of every test in the test suite by a lot.

Does the test fail?

Why don't you make the test pass or skip the test?

That would solve the issue for me, but other people might still hit this issue. It is always a good idea to cleanup test created resources, even if the test verifies that it can't create the resources, but it did.

I see no reason no to go forth with this, the failure will still be a failure, but at least there won't be a zombie node laying around.

fejta-bot · 2019-10-08T14:18:42Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

claudiubelu · 2019-10-08T15:34:58Z

/remove-lifecycle stale

claudiubelu · 2019-11-14T18:03:11Z

/test pull-kubernetes-node-e2e-containerd
/test pull-kubernetes-e2e-kind

claudiubelu · 2019-11-14T18:24:05Z

/test pull-kubernetes-node-e2e-containerd

claudiubelu · 2019-11-18T14:23:37Z

/test pull-kubernetes-node-e2e-containerd

claudiubelu · 2020-02-11T08:14:44Z

/retest

claudiubelu · 2020-02-11T11:48:01Z

/test pull-kubernetes-e2e-gce

claudiubelu · 2020-02-11T18:47:31Z

/test pull-kubernetes-e2e-gce

claudiubelu · 2020-02-12T14:09:23Z

/test pull-kubernetes-e2e-gce

claudiubelu · 2020-07-14T12:38:17Z

/remove-lifecycle stale

claudiubelu · 2020-08-03T09:33:40Z

/retest

fejta-bot · 2020-11-01T10:23:35Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

fejta-bot · 2020-12-01T11:08:18Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

claudiubelu · 2020-12-02T14:24:23Z

/remove-lifecycle rotten

oomichi

/cc @oomichi

oomichi · 2020-12-21T19:37:09Z

test/e2e/auth/node_authz.go

@@ -174,6 +174,12 @@ var _ = SIGDescribe("[Feature:NodeAuthorizer]", func() {
 		}
 		ginkgo.By(fmt.Sprintf("Create node foo by user: %v", asUser))
 		_, err := c.CoreV1().Nodes().Create(context.TODO(), node, metav1.CreateOptions{})
+		defer func() {
+			err := f.ClientSet.CoreV1().Nodes().Delete(context.TODO(), node.Name, metav1.DeleteOptions{})


It is better to write NOTE why we need this cleanup, because in the expected test path the node creation is failed and we don't need to do cleanup here.

Done, wrote a comment explaining it.

oomichi · 2020-12-21T19:38:46Z

test/e2e/auth/node_authz.go

+		defer func() {
+			err := f.ClientSet.CoreV1().Nodes().Delete(context.TODO(), node.Name, metav1.DeleteOptions{})
+			if err != nil {
+				framework.Logf("Failed to get delete node %v, err: %v", node, err)


In the expected test path, this Delete() call is always failed.
I don't think it is worth to put any log which contains Failed ... even but the log level is not Error.

The test "A node shouldn't be able to create another node" could create a node during its run, but it doesn't delete it in this case. This commit addresses this issue.

oomichi · 2021-01-19T20:43:11Z

Thanks for updating.

/test pull-kubernetes-e2e-gce-ubuntu-containerd
/approve

k8s-ci-robot · 2021-01-19T20:43:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: claudiubelu, oomichi

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/e2e/auth/OWNERS~~ [oomichi]

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

claudiubelu · 2021-01-20T14:43:54Z

/test pull-kubernetes-e2e-gce-ubuntu-containerd

claudiubelu · 2021-01-20T16:04:00Z

/test pull-kubernetes-e2e-gce-ubuntu-containerd

adelina-t · 2021-01-21T09:53:42Z

/lgtm

k8s-ci-robot requested review from mikedanese and smarterclayton June 10, 2019 15:55

mikedanese reviewed Jun 11, 2019

View reviewed changes

mikedanese added the sig/auth Categorizes an issue or PR as relevant to SIG Auth. label Jun 11, 2019

claudiubelu force-pushed the tests/cleanup-node branch from 941f8b8 to 91e2abd Compare July 10, 2019 14:08

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 8, 2019

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 8, 2019

claudiubelu force-pushed the tests/cleanup-node branch from 91e2abd to 61582e7 Compare November 14, 2019 13:36

claudiubelu force-pushed the tests/cleanup-node branch 2 times, most recently from 8330cc5 to 94e9cdd Compare February 10, 2020 17:43

claudiubelu force-pushed the tests/cleanup-node branch from 94e9cdd to abdeaa3 Compare April 13, 2020 14:07

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 12, 2020

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 14, 2020

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 1, 2020

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Dec 1, 2020

k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Dec 2, 2020

oomichi reviewed Dec 21, 2020

View reviewed changes

k8s-ci-robot requested a review from oomichi December 21, 2020 19:38

tests: Removes node created by test

ee9be7c

The test "A node shouldn't be able to create another node" could create a node during its run, but it doesn't delete it in this case. This commit addresses this issue.

claudiubelu force-pushed the tests/cleanup-node branch from d22bc22 to 51a6031 Compare January 15, 2021 16:12

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 15, 2021

claudiubelu force-pushed the tests/cleanup-node branch from 51a6031 to ee9be7c Compare January 15, 2021 16:14

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 15, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 19, 2021

k8s-ci-robot assigned adelina-t Jan 21, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 21, 2021

k8s-ci-robot merged commit e1ba754 into kubernetes:master Jan 21, 2021

k8s-ci-robot added this to the v1.21 milestone Jan 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: Removes node created by test #78862

tests: Removes node created by test #78862

claudiubelu commented Jun 10, 2019

mikedanese Jun 11, 2019

claudiubelu Jun 12, 2019

mikedanese Jun 12, 2019

claudiubelu Jun 12, 2019

mikedanese Jun 14, 2019

claudiubelu Jun 14, 2019

fejta-bot commented Oct 8, 2019

claudiubelu commented Oct 8, 2019

claudiubelu commented Nov 14, 2019

claudiubelu commented Nov 14, 2019

claudiubelu commented Nov 18, 2019

claudiubelu commented Feb 11, 2020

claudiubelu commented Feb 11, 2020

claudiubelu commented Feb 11, 2020

claudiubelu commented Feb 12, 2020

claudiubelu commented Jul 14, 2020

claudiubelu commented Aug 3, 2020

fejta-bot commented Nov 1, 2020

fejta-bot commented Dec 1, 2020

claudiubelu commented Dec 2, 2020

oomichi left a comment

oomichi Dec 21, 2020

claudiubelu Jan 15, 2021

oomichi Dec 21, 2020

claudiubelu Jan 15, 2021

oomichi commented Jan 19, 2021

k8s-ci-robot commented Jan 19, 2021

claudiubelu commented Jan 20, 2021

claudiubelu commented Jan 20, 2021

adelina-t commented Jan 21, 2021

tests: Removes node created by test #78862

tests: Removes node created by test #78862

Conversation

claudiubelu commented Jun 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fejta-bot commented Oct 8, 2019

claudiubelu commented Oct 8, 2019

claudiubelu commented Nov 14, 2019

claudiubelu commented Nov 14, 2019

claudiubelu commented Nov 18, 2019

claudiubelu commented Feb 11, 2020

claudiubelu commented Feb 11, 2020

claudiubelu commented Feb 11, 2020

claudiubelu commented Feb 12, 2020

claudiubelu commented Jul 14, 2020

claudiubelu commented Aug 3, 2020

fejta-bot commented Nov 1, 2020

fejta-bot commented Dec 1, 2020

claudiubelu commented Dec 2, 2020

oomichi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oomichi commented Jan 19, 2021

k8s-ci-robot commented Jan 19, 2021

claudiubelu commented Jan 20, 2021

claudiubelu commented Jan 20, 2021

adelina-t commented Jan 21, 2021