e2e testing for PreferSameZone/PreferSameNode #130945

danwinship · 2025-03-20T15:49:13Z

What this PR does / why we need it:

e2e tests for PreferSameZone/PreferSameNode

This also extends the existing PreferClose test to test the case where the client and server are in the same zone but not on the same node.

This required a lot of refactoring to avoid just totally duplicating everything multiple times...

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/issues/3015

/kind cleanup
/sig network
/assign @aojea @gauravkghildiyal

k8s-ci-robot · 2025-03-20T15:49:21Z

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

aojea · 2025-03-20T15:52:13Z

/test pull-kubernetes-e2e-kind-multizone

danwinship · 2025-03-20T15:57:32Z

/hold
to make sure we get all the required test coverage

aojea · 2025-03-20T16:20:01Z

see the error message with instructions to fix it

�[1m�[38;5;9mUnknown Decorator�[0m
  �[38;5;243mk8s.io/kubernetes/test/e2e/network/traffic_distribution.go:398�[0m
    [It] node was passed an unknown decorator:
    'framework.label{parts:[]string{"Feature", "PreferSame Traffic Distribution"},
    explanation:"If you see this as part of an 'Unknown Decorator' error from
    Ginkgo, then you need to replace the ginkgo.It/Context/Describe call with the
    corresponding framework.It/Context/Describe or (if available)
    f.It/Context/Describe.", alphaBetaLevel:"", offByDefault:false}'

    �[1mLearn more at:�[0m �[38;5;14m�[4mhttp://onsi.github.io/ginkgo/#node-decorators-overview�[0m

danwinship · 2025-03-20T17:17:12Z

/test pull-kubernetes-e2e-kind-multizone

danwinship · 2025-03-20T18:51:00Z

/test pull-kubernetes-e2e-kind-multizone

The features are always enabled, so the tests don't need to be conditional.

danwinship · 2025-03-20T19:33:13Z

/test pull-kubernetes-e2e-kind-multizone

aojea · 2025-03-20T19:58:21Z

test/e2e/network/topology_hints.go

@@ -41,7 +40,7 @@ import (
 	admissionapi "k8s.io/pod-security-admission/api"
 )

-var _ = common.SIGDescribe(feature.TopologyHints, func() {


Feature in the e2e framework terminology indicates this test needs to enable something to work, does not run in a normal cluster

this test requires a cluster to have zones so we need to add the Feature label or it will be picked by any normal test ... we must not rely on skips

aojea · 2025-03-20T20:12:20Z

test/e2e/network/traffic_distribution.go

 			}
 		}
+		if len(nodeForZone) < 3 {


wasn't 2 nodes per zone in 3 zones?

There's a separate test for that added later.

I didn't want to remove the existing test because OpenShift CI runs in clusters with only 1 schedulable node per zone

danwinship · 2025-03-24T14:44:37Z

/test pull-kubernetes-e2e-kind-multizone

danwinship · 2025-03-24T15:29:25Z

/test pull-kubernetes-e2e-kind-multizone

danwinship · 2025-03-24T18:24:43Z

/test pull-kubernetes-e2e-kind-multizone

k8s-ci-robot · 2025-03-24T20:52:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danwinship

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/feature/OWNERS~~ [danwinship]
~~test/e2e/network/OWNERS~~ [danwinship]

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

Remove endpointSlicesHaveSameZoneHints check. We are testing that connections end up at the right endpoints. We don't need to validate _why_ they go to the right endpoints, which is already tested by other tests anyway. (Also, validating the hints becomes more complicated in the same-node case, where there may or may not also be same-zone hints depending on cluster configuration.) Remove DeferCleanup calls; we don't need to delete anything manually because namespaced resources will automatically be deleted when the test case's namespace is deleted. Remove a setting of pod.NodeName that was redundant with e2epod.SetNodeSelection().

(No other changes to the code)

Split the logic of creating the clients and the servers apart from the logic of checking which clients connect to which servers. Add some extra complexity to support additional use cases (like multiple endpoints on the same node).

(Identical to the PreferClose test.)

The existing TrafficDistribution test didn't really distinguish "same zone" from "same node". Add another test that makes sure there are at least 2 nodes in each zone so it can do that. (Keep the original test as well to avoid losing coverage in CI systems with single-schedulable-node-per-zone clusters.)

danwinship · 2025-03-25T00:56:42Z

/retest
/milestone v1.33

gauravkghildiyal · 2025-03-25T21:27:15Z

/lgtm

Thanks!

k8s-ci-robot · 2025-03-25T21:27:21Z

LGTM label has been added.

Git tree hash: 545aca66f2d24b467cc1b50156a16bcc57cdcd2f

k8s-triage-robot · 2025-03-26T00:40:11Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-ci-robot assigned aojea Mar 20, 2025

k8s-ci-robot added the release-note-none Denotes a PR that doesn't merit a release note. label Mar 20, 2025

k8s-ci-robot assigned gauravkghildiyal Mar 20, 2025

k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Mar 20, 2025

k8s-ci-robot requested review from robscott and smarterclayton March 20, 2025 15:49

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

danwinship mentioned this pull request Mar 20, 2025

PreferSameNode Traffic Distribution (formerly PreferLocal traffic policy / Node-level topology) kubernetes/enhancements#3015

Open

8 tasks

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

danwinship mentioned this pull request Mar 20, 2025

integration testing for PreferSameZone/PreferSameNode #130944

Merged

danwinship force-pushed the prefersame-e2e-test branch from e3a206c to 8d4acf6 Compare March 20, 2025 16:37

danwinship force-pushed the prefersame-e2e-test branch from 8d4acf6 to 035b921 Compare March 20, 2025 17:56

Remove TopologyHints and TrafficDistribution feature flags

2670462

The features are always enabled, so the tests don't need to be conditional.

danwinship force-pushed the prefersame-e2e-test branch from 035b921 to 9201739 Compare March 20, 2025 19:32

aojea reviewed Mar 20, 2025

View reviewed changes

k8s-ci-robot requested a review from aojea March 24, 2025 14:44

danwinship force-pushed the prefersame-e2e-test branch from 08bf379 to 99a8289 Compare March 24, 2025 18:24

danwinship force-pushed the prefersame-e2e-test branch from 99a8289 to 0758cb4 Compare March 24, 2025 20:52

danwinship marked this pull request as ready for review March 24, 2025 20:52

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 24, 2025

k8s-ci-robot requested a review from bowei March 24, 2025 20:52

danwinship added 8 commits March 24, 2025 18:46

Remove a level of indentation in the TrafficDistribution e2e

b1a0fea

(No other changes to the code)

Abstract the logic of the TrafficDistribution test

bc81a86

Split the logic of creating the clients and the servers apart from the logic of checking which clients connect to which servers. Add some extra complexity to support additional use cases (like multiple endpoints on the same node).

Streamling the node/zone-picking logic

10cd54b

Split out the pieces of the TrafficDistribution test

23aff5d

Add test for trafficDistribution: PreferSameZone

7956b37

(Identical to the PreferClose test.)

Add a tests for PreferSameNode

478a6f9

danwinship force-pushed the prefersame-e2e-test branch from 0758cb4 to 478a6f9 Compare March 24, 2025 22:47

k8s-ci-robot added this to the v1.33 milestone Mar 25, 2025

danwinship mentioned this pull request Mar 25, 2025

cleanup CI hardcoded ssh skips #131038

Open

k8s-infra-ci-robot added this to [sig-release] Bug Triage Mar 25, 2025

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

k8s-ci-robot merged commit c90a4b1 into kubernetes:master Mar 26, 2025
18 checks passed

github-project-automation bot moved this to Done in [sig-release] Bug Triage Mar 26, 2025

danwinship deleted the prefersame-e2e-test branch March 26, 2025 08:52

e2e testing for PreferSameZone/PreferSameNode #130945

e2e testing for PreferSameZone/PreferSameNode #130945

Uh oh!

Conversation

danwinship commented Mar 20, 2025

What this PR does / why we need it:

Does this PR introduce a user-facing change?

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

Uh oh!

k8s-ci-robot commented Mar 20, 2025

Uh oh!

aojea commented Mar 20, 2025

Uh oh!

danwinship commented Mar 20, 2025

Uh oh!

aojea commented Mar 20, 2025

Uh oh!

danwinship commented Mar 20, 2025

Uh oh!

danwinship commented Mar 20, 2025

Uh oh!

danwinship commented Mar 20, 2025

Uh oh!

aojea Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

aojea Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aojea Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

danwinship Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

danwinship commented Mar 24, 2025

Uh oh!

danwinship commented Mar 24, 2025

Uh oh!

danwinship commented Mar 24, 2025

Uh oh!

k8s-ci-robot commented Mar 24, 2025

Uh oh!

danwinship commented Mar 25, 2025

Uh oh!

gauravkghildiyal commented Mar 25, 2025

Uh oh!

k8s-ci-robot commented Mar 25, 2025

Uh oh!

k8s-triage-robot commented Mar 26, 2025

Uh oh!

Uh oh!

Uh oh!

aojea Mar 21, 2025 •

edited

Loading