clean up/reorganize endpoints conformance tests, split Endpoints/EndpointSlices #133993

danwinship · 2025-09-10T13:12:07Z

(skip down to "special notes for your reviewer" for conformance-specific details)

What this PR does / why we need it:

Continuing the work on KEP-4974, this cleans up and reorganizes the Endpoints and EndpointSlice controller conformance tests; in particular, it takes the 2 existing conformance tests that look at the outputs of both the Endpoints and EndpointSlice controllers, and splits them into separate (slightly-redundant) Endpoints-only and EndpointSlices-only tests.

With this PR merged, the overall state of Endpoints/EndpointSlice e2e is that there are only 3 files in all of test/e2e that use the Endpoints API:

test/e2e/network/endpoints.go has
- the generic "the Endpoints API works" conformance test (which will always remain a conformance test)
- Two "the Endpoints controller works" conformance tests (which will eventually become non-conformance and will be tagged [Feature:EndpointsController])
test/e2e/network/endpointslicemirroring.go has
- the EndpointSlice mirroring controller tests, including one conformance test (which will eventually become non-conformance and feature-tagged).
test/e2e/network/apiserver.go has
- a conformance test for the kubernetes.default Endpoints (and EndpointSlice). (The apiserver creates both objects directly, not depending on the Endpoints controller, the EndpointSlice controller, or the EndpointSlice mirroring controller; and KEP-4974 does not currently plan any changes to this.)

This means once this PR merges we will be able to set up an e2e job that runs without Endpoints/EndpointSlice mirroring, without having to skip any other tests. (This environment would still be non-conformant at this point, but this allows us to confirm that nothing else depends on those controllers.)

Which issue(s) this PR is related to:

KEP: kubernetes/enhancements#4974

(Also, follow up to #132991)

Special notes for your reviewer:

While this PR adds 2 new conformance tests and modifies some others, it does not change any conformance criteria. It just:

moves some conformance tests between files
fixes the descriptions and names of some conformance tests
splits the 2 existing Endpoints+EndpointSlices creation/deletion conformance tests into 2 Endpoints creation/deletion conformance tests and 2 EndpointSlices creation/deletion conformance tests (without otherwise changing their behavior; in particular, the tests did not previously check explicitly that the Endpoints and the EndpointSlices contained the same data; this was (and remains) checked only implicitly in that they are both required to match the Pods).

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://github.com/kubernetes/enhancements/tree/master/keps/sig-network/4974-deprecate-endpoints

/kind cleanup
/sig network
/area conformance
/triage accepted
/priority important-longterm

aojea · 2025-09-10T13:42:13Z

you got a bunch of linter errors

aojea · 2025-09-10T13:46:26Z

test/conformance/testdata/conformance.yaml

  description: The EndpointSlice controller should create and delete empty EndpointSlices
    for a Service that matches no pods.
-  release: v1.21
+  release: v1.21, v1.35


@dims @BenTheElder can you please ACK this is correct from conformance POV?

the docs say "If the test was modified in subsequent releases then those releases should be included as well (comma separated)" so that seemed to fit here; there were previously 2 tests that tested both Endpoints and EndpointSlices, and now they've each been split into one Endpoints test and one EndpointSlice test.

Yeah, this is fine.
When we have a major behavior change in the test it should get the version added. In this case the change is just the split but ... I don't think the project has been fully consistent over the years in adding these, mostly the first-added version.

cc @dims to confirm

yes, this is fine. thanks @danwinship

aojea · 2025-09-10T13:48:53Z

didn't go in detail but overall looks like a good simplification and split of e2e/conformance test to discriminate between Endpoint / Endpointslices

Missing confirmation from Conformance folks for the correct tagging for these cases and some linter issues

LGTM module the missing parts in ^^

danwinship · 2025-09-10T15:36:20Z

remaining pull-kubernetes-linter-hints failures are just "hey, the Endpoints API is deprecated!"

danwinship · 2025-09-28T20:38:32Z

/retest-required

danwinship · 2025-09-30T21:11:12Z

/retest-required

danwinship · 2025-09-30T22:06:01Z

/retest-required

While createServiceReportErr could be fixed to use framework.ExpectNoErrorWithOffset (and, uh, to log "error creating Service" rather than "error deleting Service"), it's too trivial to really be all that useful.

danwinship · 2025-10-01T16:41:59Z

/retest

test/e2e/network/endpointslice.go

BenTheElder · 2025-10-07T19:54:41Z

linter-hints is just suggestions, we can review and ignore them, it doesn't block merge.

aojea · 2025-10-07T19:55:00Z

test/e2e/network/endpointslice.go

 		ginkgo.By("referencing matching pods with named port")
 		expectEndpointsAndSlices(ctx, cs, f.Namespace.Name, svc2, []*v1.Pod{pod1, pod2}, 2, 2, true)

-		ginkgo.By("creating empty Endpoints and EndpointSlices for no matching Pods")


what is the other test?

The previous test (which starts out as "should create and delete Endpoints and EndpointSlices for a Service with a selector specified" but gets renamed to "should create and delete EndpointSlices for a Service with a selector that matches no pods") tests just the no-endpoints case, while this one tested Services both with and without endpoints.

aojea · 2025-10-07T19:59:23Z

one question about the duplicate tested logic I could not find and one typo that since is in conformance I think is worth fixing it ,

This was being checked by two different conformance tests.

(In particular, fix the wait.PollImmediate calls even in the code that's staying here, not just the code that's moving.)

Move the Endpoints API test from endpointslice.go to endpoints.go Move the "kubernetes.default Service exists" and "kubernetes.default endpoints exist" tests to apiserver.go, since (unlike the rest of service.go/endpointslice.go) they aren't testing the behavior of the Service/EndpointSlice/Endpoints APIs. (No code changes, but fixed a typo in a comment.)

Inline the endpointSlicesEqual() method into the test, since despite its generic-sounding name, it made assumptions specific to this test. Also, port to generic sets.

…ests

aojea · 2025-10-08T20:45:55Z

Kubernetes e2e suite: [It] [sig-api-machinery] Servers with support for API chunking should support continue listing from the last key if the original version has been compacted away, though the list is inconsistent [Slow] [Conformance] expand_more

=== FAIL: test/integration/etcd (0.00s)

does not look related

/retest

aojea · 2025-10-08T20:48:15Z

/lgtm
/approve

k8s-ci-robot · 2025-10-08T20:48:23Z

LGTM label has been added.

Git tree hash: 9dc4387fc4ad181e60dcf5e332b0f1ed6380f137

k8s-ci-robot · 2025-10-08T21:09:08Z

@danwinship: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kubernetes-linter-hints	`c189e9d`	link	false	`/test pull-kubernetes-linter-hints`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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

danwinship · 2025-10-12T15:52:48Z

/assign @dims
for conformance change approval:

While this PR adds 2 new conformance tests and modifies some others, it does not change any conformance criteria. It just:

moves some conformance tests between files

fixes the descriptions and names of some conformance tests

splits the 2 existing Endpoints+EndpointSlices creation/deletion conformance tests into 2 Endpoints creation/deletion conformance tests and 2 EndpointSlices creation/deletion conformance tests (without otherwise changing their behavior; in particular, the tests did not previously check explicitly that the Endpoints and the EndpointSlices contained the same data; this was (and remains) checked only implicitly in that they are both required to match the Pods).

dims · 2025-10-12T16:24:51Z

/approve
/lgtm

/hold please remove hold when ready

k8s-ci-robot · 2025-10-12T16:25:05Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aojea, danwinship, dims

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/conformance/testdata/OWNERS~~ [dims]
~~test/e2e/network/OWNERS~~ [aojea,danwinship,dims]

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

danwinship · 2025-10-13T12:52:50Z

/hold cancel
thanks

danwinship · 2025-10-13T14:06:45Z

/test pull-kubernetes-verify

k8s-ci-robot requested review from BenTheElder and tnqn September 10, 2025 13:12

aojea reviewed Sep 10, 2025

View reviewed changes

danwinship force-pushed the endpoints-slices-e2e-split branch from ca3d96b to 554f613 Compare September 10, 2025 14:26

danwinship moved this to Issues To Triage in conformance-definition Sep 10, 2025

danwinship added this to conformance-definition Sep 10, 2025

danwinship mentioned this pull request Sep 28, 2025

KEP-4974: Add ci-kubernetes-kind-network-deprecate-endpoints kubernetes/test-infra#35606

Merged

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 1, 2025

Remove a "helper" function that doesn't really help much

5ae2372

While createServiceReportErr could be fixed to use framework.ExpectNoErrorWithOffset (and, uh, to log "error creating Service" rather than "error deleting Service"), it's too trivial to really be all that useful.

danwinship force-pushed the endpoints-slices-e2e-split branch from 554f613 to 69ef450 Compare October 1, 2025 13:28

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 1, 2025

danwinship force-pushed the endpoints-slices-e2e-split branch from 69ef450 to 0f42bc8 Compare October 1, 2025 14:05

aojea reviewed Oct 7, 2025

View reviewed changes

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

aojea reviewed Oct 7, 2025

View reviewed changes

danwinship added 6 commits October 7, 2025 17:32

Fix up EndpointSlice conformance test descriptions

ff83235

Remove redundant test of "empty EndpointSlice for Service with no Pods"

07959fd

This was being checked by two different conformance tests.

Fix some linter errors before splitting out code

d46f15b

(In particular, fix the wait.PollImmediate calls even in the code that's staying here, not just the code that's moving.)

Update kubernetes.default endpoints test a bit

7b7145a

Inline the endpointSlicesEqual() method into the test, since despite its generic-sounding name, it made assumptions specific to this test. Also, port to generic sets.

Split Endpoints controller and EndpointSlice controller conformance t…

c189e9d

…ests

danwinship force-pushed the endpoints-slices-e2e-split branch from 0f42bc8 to c189e9d Compare October 7, 2025 21:33

k8s-ci-robot assigned aojea Oct 8, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 8, 2025

k8s-ci-robot assigned dims Oct 12, 2025

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 12, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 12, 2025

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 13, 2025

k8s-ci-robot merged commit b6eb2dd into kubernetes:master Oct 13, 2025
15 of 18 checks passed

k8s-ci-robot added this to the v1.35 milestone Oct 13, 2025

github-project-automation bot moved this from Issues To Triage to Done in conformance-definition Oct 13, 2025

danwinship deleted the endpoints-slices-e2e-split branch October 13, 2025 16:44

clean up/reorganize endpoints conformance tests, split Endpoints/EndpointSlices #133993

clean up/reorganize endpoints conformance tests, split Endpoints/EndpointSlices #133993

Uh oh!

Conversation

danwinship commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it:

Which issue(s) this PR is related to:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Uh oh!

aojea commented Sep 10, 2025

Uh oh!

aojea Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

danwinship Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

BenTheElder Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

BenTheElder Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

dims Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

aojea commented Sep 10, 2025

Uh oh!

danwinship commented Sep 10, 2025

Uh oh!

danwinship commented Sep 28, 2025

Uh oh!

danwinship commented Sep 30, 2025

Uh oh!

danwinship commented Sep 30, 2025

Uh oh!

danwinship commented Oct 1, 2025

Uh oh!

Uh oh!

BenTheElder commented Oct 7, 2025

Uh oh!

aojea Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

danwinship Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

aojea commented Oct 7, 2025

Uh oh!

aojea commented Oct 8, 2025

Uh oh!

aojea commented Oct 8, 2025

Uh oh!

k8s-ci-robot commented Oct 8, 2025

Uh oh!

k8s-ci-robot commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danwinship commented Oct 12, 2025

Uh oh!

dims commented Oct 12, 2025

Uh oh!

k8s-ci-robot commented Oct 12, 2025

Uh oh!

danwinship commented Oct 13, 2025

Uh oh!

danwinship commented Oct 13, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

danwinship commented Sep 10, 2025 •

edited

Loading

k8s-ci-robot commented Oct 8, 2025 •

edited

Loading