Enable running cpu-mgr-multiNUMA e2e tests with Topology manager #88566

Deepthidharwar · 2020-02-26T07:10:15Z

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

/kind cleanup

What this PR does / why we need it:
Currently, TopologyManager e2e tests have a copy of CPU Manager e2e test code and does not support MultiNUMA support. This PR addresses both these issues by code-refactor and re-use.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

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

k8s-ci-robot · 2020-02-26T07:10:23Z

Hi @Deepthidharwar. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

Deepthidharwar · 2020-02-26T12:15:37Z

@vpickard @ConnorDoyle , @fromanirh , Please do review and if it looks ok, need ok-to-test.

Deepthidharwar · 2020-02-26T12:15:47Z

/test pull-kubernetes-node-kubelet-serial-cpu-manager

k8s-ci-robot · 2020-02-26T12:16:01Z

@Deepthidharwar: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test pull-kubernetes-node-kubelet-serial-cpu-manager

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.

nolancon · 2020-02-27T14:32:47Z

/ok-to-test

nolancon · 2020-02-27T14:55:34Z

/ok-to-test

nolancon · 2020-02-27T18:28:15Z

/test pull-kubernetes-node-e2e-containerd

vpickard · 2020-02-27T19:18:10Z

/test pull-kubernetes-node-e2e-containerd

It looks like this test is failing consistently across the board. I have posted a message on #sig-testing to see if the test env needs some tidying up. For reference, here is a link to this job run history:

https://prow.k8s.io/job-history/kubernetes-jenkins/pr-logs/directory/pull-kubernetes-node-e2e-containerd

And here is the failure:

I0227 18:48:05.570] unable to create gce instance with running docker daemon for image cos-73-11647-459-0. could not create instance tmp-node-e2e-0a7f6c8e-cos-73-11647-459-0: [&{Code:QUOTA_EXCEEDED Location: Message:Quota 'IN_USE_ADDRESSES' exceeded. Limit: 69.0 in region us-west1. ForceSendFields:[] NullFields:[]}]

Deepthidharwar · 2020-02-28T05:11:18Z

/retest

Deepthidharwar · 2020-02-28T11:13:04Z

/test pull-kubernetes-node-kubelet-serial-cpu-manager

vpickard · 2020-02-28T14:47:52Z

/test pull-kubernetes-node-kubelet-serial-topology-manager

Deepthidharwar · 2020-02-28T15:13:11Z

Failing test is not cpu-manager's test but SR-IOV device plugin tests. Looking into this.

ffromani · 2020-02-28T15:20:33Z

Failing test is not cpu-manager's test but SR-IOV device plugin tests. Looking into this.

It's a bug I introduced in the tests which triggers when the tests are run on a box without any SRIOV device. The logic which tries to detect if we need to skip the tests for this very reason is flawed.

Deepthidharwar · 2020-02-28T15:23:06Z

@fromanirh , SRIOV detection logic is reporting error on systems that does not have one enabled https://github.com/kubernetes/kubernetes/blob/master/test/e2e_node/topology_manager_test.go#L172 . This should not error out.

ffromani · 2020-02-28T15:25:59Z

@fromanirh , SRIOV detection logic is reporting error on systems that does not have one enabled https://github.com/kubernetes/kubernetes/blob/master/test/e2e_node/topology_manager_test.go#L172 . This should not error out.

Correct. It should read like

if len(pciPerNuma) == 0 {
   return -1, false
}

The code as it is is correct in the flow the tests I added are using. Changing like I wrote above make it work in the general flow.
Sorry for this bug.

Deepthidharwar · 2020-03-02T11:04:07Z

#88721 has the fix for this as discussed above.

Deepthidharwar · 2020-03-02T11:05:12Z

The changes made as part of this patch runs fine on systems with fix in #88721.

ffromani

lgtm assuming the inline comment is handled

ffromani · 2020-03-02T15:43:56Z

test/e2e_node/cpu_manager_test.go

-			e2eskipper.Skipf("Skipping CPU Manager tests since the CPU capacity < 2")
-		}
+	ginkgo.By("by deleting the pods and waiting for container removal")
+	deletePods(f, []string{pod.Name})


If I'm not mistaken, the original code had

waitForContainerRemoval(pod.Spec.Containers[0].Name, pod.Name, pod.Namespace)

here. If so, I'd add it again.

Good catch.

…er-e2e-testsuite. Signed-off-by: Deepthi Dharwar <ddharwar@redhat.com>

Signed-off-by: Deepthi Dharwar <ddharwar@redhat.com>

Deepthidharwar · 2020-03-02T17:20:23Z

/retest

ffromani · 2020-03-02T17:24:18Z

lgtm now. Thanks for the updates!

rphillips · 2020-03-03T15:11:58Z

/lgtm
/assign @derekwaynecarr

derekwaynecarr · 2020-03-05T02:10:00Z

the refactoring was clean. thanks for the follow-up.

/approve
/lgtm

k8s-ci-robot · 2020-03-05T02:10:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Deepthidharwar, derekwaynecarr

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_node/OWNERS~~ [derekwaynecarr]

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

Deepthidharwar · 2020-03-05T06:46:12Z

/kind cleanup

Deepthidharwar · 2020-03-05T06:47:23Z

/retest

Deepthidharwar · 2020-03-05T09:47:04Z

/retest

k8s-ci-robot · 2020-03-05T12:05:48Z

@Deepthidharwar: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-kubernetes-node-kubelet-serial-topology-manager	ebb3bdc023bed37f770aaa9f640757118b2a45e3	link	`/test pull-kubernetes-node-kubelet-serial-topology-manager`

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

nolancon · 2020-03-05T12:13:21Z

/test pull-kubernetes-e2e-gce

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Feb 26, 2020

k8s-ci-robot requested review from resouer and tallclair February 26, 2020 07:12

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 27, 2020

Deepthidharwar force-pushed the topology-mgr-numa-tests branch from 6ed8596 to ebb3bdc Compare February 28, 2020 09:32

Deepthidharwar mentioned this pull request Mar 2, 2020

FIX SRIOV detection logic not to error out if device not present on the system #88721

Merged

ffromani reviewed Mar 2, 2020

View reviewed changes

Deepthidharwar added 2 commits March 2, 2020 22:36

Refactor CPUMananger-e2e-tests so that it be reused by topology-manag…

4abbce4

…er-e2e-testsuite. Signed-off-by: Deepthi Dharwar <ddharwar@redhat.com>

Enable topology-manager-e2e tests to run on MultiNUMA nodes.

1ede096

Signed-off-by: Deepthi Dharwar <ddharwar@redhat.com>

Deepthidharwar force-pushed the topology-mgr-numa-tests branch from ebb3bdc to 1ede096 Compare March 2, 2020 17:08

k8s-ci-robot assigned derekwaynecarr and rphillips Mar 3, 2020

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

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

derekwaynecarr mentioned this pull request Mar 5, 2020

Update TopologyManager Feature Gate #87650

Merged

k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Mar 5, 2020

k8s-ci-robot merged commit 1f2e196 into kubernetes:master Mar 5, 2020

k8s-ci-robot added this to the v1.18 milestone Mar 5, 2020

ffromani mentioned this pull request Mar 10, 2021

REQUEST: New membership for fromanirh kubernetes/org#2560

Closed

6 tasks

pacoxu mentioned this pull request Jan 18, 2024

Node Topology Manager kubernetes/enhancements#693

Closed

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable running cpu-mgr-multiNUMA e2e tests with Topology manager #88566

Enable running cpu-mgr-multiNUMA e2e tests with Topology manager #88566

Deepthidharwar commented Feb 26, 2020 •

edited

k8s-ci-robot commented Feb 26, 2020

Deepthidharwar commented Feb 26, 2020

Deepthidharwar commented Feb 26, 2020

k8s-ci-robot commented Feb 26, 2020

nolancon commented Feb 27, 2020

nolancon commented Feb 27, 2020

nolancon commented Feb 27, 2020

vpickard commented Feb 27, 2020 •

edited

Deepthidharwar commented Feb 28, 2020

Deepthidharwar commented Feb 28, 2020

vpickard commented Feb 28, 2020

Deepthidharwar commented Feb 28, 2020 •

edited

ffromani commented Feb 28, 2020 •

edited

Deepthidharwar commented Feb 28, 2020

ffromani commented Feb 28, 2020 •

edited

Deepthidharwar commented Mar 2, 2020

Deepthidharwar commented Mar 2, 2020

ffromani left a comment

ffromani Mar 2, 2020

Deepthidharwar Mar 2, 2020

Deepthidharwar commented Mar 2, 2020

ffromani commented Mar 2, 2020

rphillips commented Mar 3, 2020

derekwaynecarr commented Mar 5, 2020

k8s-ci-robot commented Mar 5, 2020

Deepthidharwar commented Mar 5, 2020

Deepthidharwar commented Mar 5, 2020

Deepthidharwar commented Mar 5, 2020

k8s-ci-robot commented Mar 5, 2020 •

edited

nolancon commented Mar 5, 2020

Enable running cpu-mgr-multiNUMA e2e tests with Topology manager #88566

Enable running cpu-mgr-multiNUMA e2e tests with Topology manager #88566

Conversation

Deepthidharwar commented Feb 26, 2020 • edited

k8s-ci-robot commented Feb 26, 2020

Deepthidharwar commented Feb 26, 2020

Deepthidharwar commented Feb 26, 2020

k8s-ci-robot commented Feb 26, 2020

nolancon commented Feb 27, 2020

nolancon commented Feb 27, 2020

nolancon commented Feb 27, 2020

vpickard commented Feb 27, 2020 • edited

Deepthidharwar commented Feb 28, 2020

Deepthidharwar commented Feb 28, 2020

vpickard commented Feb 28, 2020

Deepthidharwar commented Feb 28, 2020 • edited

ffromani commented Feb 28, 2020 • edited

Deepthidharwar commented Feb 28, 2020

ffromani commented Feb 28, 2020 • edited

Deepthidharwar commented Mar 2, 2020

Deepthidharwar commented Mar 2, 2020

ffromani left a comment

Choose a reason for hiding this comment

ffromani Mar 2, 2020

Choose a reason for hiding this comment

Deepthidharwar Mar 2, 2020

Choose a reason for hiding this comment

Deepthidharwar commented Mar 2, 2020

ffromani commented Mar 2, 2020

rphillips commented Mar 3, 2020

derekwaynecarr commented Mar 5, 2020

k8s-ci-robot commented Mar 5, 2020

Deepthidharwar commented Mar 5, 2020

Deepthidharwar commented Mar 5, 2020

Deepthidharwar commented Mar 5, 2020

k8s-ci-robot commented Mar 5, 2020 • edited

nolancon commented Mar 5, 2020

Deepthidharwar commented Feb 26, 2020 •

edited

vpickard commented Feb 27, 2020 •

edited

Deepthidharwar commented Feb 28, 2020 •

edited

ffromani commented Feb 28, 2020 •

edited

ffromani commented Feb 28, 2020 •

edited

k8s-ci-robot commented Mar 5, 2020 •

edited