Updating EndpointSlice e2e tests to be less flaky and easier to debug #93402

robscott · 2020-07-23T23:44:38Z

What type of PR is this?
/kind cleanup
/kind failing-test

What this PR does / why we need it:
This adjusts tests that were waiting for Pods to be ready to instead just wait for them to have IPs assigned to them. This relies on the associated publishNotReadyAddresses field on Services. Additionally this increases the the length of time we'll wait for EndpointSlices to be garbage collected from 12s to 18s. Finally, this adds additional logging to ExpectNoError calls so it's easier to understand where and why a test failed.

Which issue(s) this PR fixes:
I'm hoping this PR will fix #93374 and #92776, but all I can say for sure is that it "works on my cluster".

Special notes for your reviewer:
Although it seems backwards to even be testing for EndpointSlice garbage collection we need some kind of test coverage to make sure that the EndpointSlice controller does not conflict with the garbage collector. Maybe there's a better way to do that than have this long timeout here. It could potentially be more straightforward to just immediately delete both Endpoints and EndpointSlices when a Service is marked for deletion, but that could potentially be undesirable if there are long running finalizers in place.

Does this PR introduce a user-facing change?:

NONE

/cc @aojea @freehan @RobertKielty @hasheddan
/assign @liggitt

robscott · 2020-07-23T23:45:16Z

/sig networking
/priority critical-urgent

k8s-ci-robot · 2020-07-23T23:45:18Z

@robscott: The label(s) sig/test cannot be applied, because the repository doesn't have them

In response to this:

/sig test
/priority critical-urgent

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.

robscott · 2020-07-23T23:47:06Z

/remove-kind feature
/kind failing-test

hasheddan · 2020-07-24T03:32:55Z

test/e2e/network/endpointslice.go

+		// Expect Endpoints resource to not be deleted when Service is.
+		// TODO(robscott): Update this test when this bug is fixed.


If we expect that the Endpoints resource will not be deleted here wouldn't this test always fail?

yep, I don't understand the comment either, the code seems to check they are deleted

🤦 Apparently I momentarily confused this part with another part that is waiting for an Endpoints related fix and thought I'd forgotten to add a TODO here. As it turns out the TODO already existed in the appropriate place: https://github.com/kubernetes/kubernetes/blob/master/test/e2e/network/endpointslice.go#L296-L297. That bug will actually be fixed with an open PR so I need to remember to connect those dots.

liggitt · 2020-07-24T04:39:19Z

test/e2e/network/endpointslice.go

@@ -143,7 +143,7 @@ var _ = SIGDescribe("EndpointSlice", func() {
 		}

 		// Expect EndpointSlice resource to be deleted when Service is.
-		if err := wait.PollImmediate(2*time.Second, 12*time.Second, func() (bool, error) {
+		if err := wait.PollImmediate(2*time.Second, 18*time.Second, func() (bool, error) {


this is a surprisingly specific time... why not wait.ForeverTestTimeout (30 seconds)?

Somehow I think I'd thought the only reason it would take longer than this would be a legitimate failure, clearly that's not always the case. Also, I didn't know about wait.ForeverTestTimeout - I've updated all 4 instances of 12 second times to use that instead, thanks!

test/e2e/network/endpointslice.go

This adjusts tests that were waiting for Pods to be ready to instead just wait for them to have IPs assigned to them. This relies on the associated publishNotReadyAddresses field on Services. Additionally this increases the the length of time we'll wait for EndpointSlices to be garbage collected from 12s to 30s. Finally, this adds additional logging to ExpectNoError calls so it's easier to understand where and why a test failed.

liggitt · 2020-07-24T17:41:09Z

/lgtm
/approve
/milestone v1.19

k8s-ci-robot · 2020-07-24T17:41:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, robscott

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/network/OWNERS~~ [liggitt]

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

fejta-bot · 2020-07-24T21:23:44Z

/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.

robscott · 2020-07-25T00:16:53Z

/retest

fejta-bot · 2020-07-25T03:20:51Z

/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.

robscott · 2020-07-25T04:27:20Z

/retest

fejta-bot · 2020-07-25T12:26:43Z

/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 · 2020-07-25T18:23:45Z

/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 assigned liggitt Jul 23, 2020

k8s-ci-robot requested review from aojea, freehan, hasheddan and RobertKielty July 23, 2020 23:44

k8s-ci-robot added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Jul 23, 2020

k8s-ci-robot added kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. and removed kind/feature Categorizes issue or PR as related to a new feature. labels Jul 23, 2020

This was referenced Jul 23, 2020

[Flaky test] EndpointSlice should create Endpoints and EndpointSlices for Pods matching a Service #93374

Closed

[sig-network] EndpointSlice should create and delete Endpoints and EndpointSlices for a Service with a selector specified #92776

Closed

hasheddan reviewed Jul 24, 2020

View reviewed changes

robscott force-pushed the endpointslice-test-fix branch from 3278b0d to 10be96b Compare July 24, 2020 04:07

liggitt reviewed Jul 24, 2020

View reviewed changes

test/e2e/network/endpointslice.go Outdated Show resolved Hide resolved

robscott force-pushed the endpointslice-test-fix branch from 10be96b to 3fea696 Compare July 24, 2020 17:30

robscott force-pushed the endpointslice-test-fix branch from 3fea696 to 3e4745d Compare July 24, 2020 17:35

k8s-ci-robot added this to the v1.19 milestone Jul 24, 2020

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

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 24, 2020

k8s-ci-robot merged commit 17a2e5e into kubernetes:master Jul 25, 2020

robscott deleted the endpointslice-test-fix branch March 11, 2021 04:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updating EndpointSlice e2e tests to be less flaky and easier to debug #93402

Updating EndpointSlice e2e tests to be less flaky and easier to debug #93402

robscott commented Jul 23, 2020 •

edited

Loading

robscott commented Jul 23, 2020 •

edited

Loading

k8s-ci-robot commented Jul 23, 2020

robscott commented Jul 23, 2020

hasheddan Jul 24, 2020

aojea Jul 24, 2020

robscott Jul 24, 2020

liggitt Jul 24, 2020

robscott Jul 24, 2020

liggitt commented Jul 24, 2020

k8s-ci-robot commented Jul 24, 2020

fejta-bot commented Jul 24, 2020

robscott commented Jul 25, 2020

fejta-bot commented Jul 25, 2020

robscott commented Jul 25, 2020

fejta-bot commented Jul 25, 2020

fejta-bot commented Jul 25, 2020

		// Expect Endpoints resource to not be deleted when Service is.
		// TODO(robscott): Update this test when this bug is fixed.

Updating EndpointSlice e2e tests to be less flaky and easier to debug #93402

Updating EndpointSlice e2e tests to be less flaky and easier to debug #93402

Conversation

robscott commented Jul 23, 2020 • edited Loading

robscott commented Jul 23, 2020 • edited Loading

k8s-ci-robot commented Jul 23, 2020

robscott commented Jul 23, 2020

hasheddan Jul 24, 2020

Choose a reason for hiding this comment

aojea Jul 24, 2020

Choose a reason for hiding this comment

robscott Jul 24, 2020

Choose a reason for hiding this comment

liggitt Jul 24, 2020

Choose a reason for hiding this comment

robscott Jul 24, 2020

Choose a reason for hiding this comment

liggitt commented Jul 24, 2020

k8s-ci-robot commented Jul 24, 2020

fejta-bot commented Jul 24, 2020

robscott commented Jul 25, 2020

fejta-bot commented Jul 25, 2020

robscott commented Jul 25, 2020

fejta-bot commented Jul 25, 2020

fejta-bot commented Jul 25, 2020

robscott commented Jul 23, 2020 •

edited

Loading

robscott commented Jul 23, 2020 •

edited

Loading