e2e node pod overhead #88558

egernst · 2020-02-26T02:11:09Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Adds e2e_node test for the PodOverhead feature, and helps ensure this feature behaves as expected.

NONE

egernst · 2020-02-26T17:58:20Z

/test pull-kubernetes-e2e-kind-ipv6

BenTheElder · 2020-02-27T06:37:04Z

I have a bit less context on what a node_e2e test should ideally look like, I would like sig node to review this
/uncc
[will observe though!]

test/e2e_node/runtimeclass_test.go

tallclair · 2020-02-27T18:50:14Z

test/e2e_node/runtimeclass_test.go

+			Containers: []v1.Container{
+				{
+					Image:   busyboxImage,
+					Name:    "container" + string(uuid.NewUUID()),


why randomize the container name?

Do e2e_node run in parallel? I suppose since it isn't a "real" cluster we shouldn't expect there could be a pod running already with this name?

I saw this was best practice in the scheduler e2e tests.

The container name doesn't need to be unique, just the pod name.

test/e2e_node/runtimeclass_test.go

egernst · 2020-02-28T16:05:56Z

Updated, PTAL @tallclair

mrunalp · 2020-02-28T18:03:38Z

@derekwaynecarr ptal

tallclair

Just a few nits

tallclair · 2020-02-28T22:10:34Z

test/e2e_node/runtimeclass_test.go

+			Containers: []v1.Container{
+				{
+					Image:   busyboxImage,
+					Name:    "container" + string(uuid.NewUUID()),


The container name doesn't need to be unique, just the pod name.

test/e2e_node/runtimeclass_test.go

This test will verify that the Pod cgroup created takes Overhead into account. Signed-off-by: Eric Ernst <eric@amperecomputing.com>

tallclair

/lgtm
/approve

Thanks!

k8s-ci-robot · 2020-02-28T23:16:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: egernst, tallclair

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~~ [tallclair]

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

k8s-ci-robot · 2020-02-28T23:27:11Z

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

Test name	Commit	Details	Rerun command
pull-kubernetes-e2e-gce-csi-serial	bd3be7d6134f87de0d05a4b1510a54b14237c2e4	link	`/test pull-kubernetes-e2e-gce-csi-serial`
pull-kubernetes-e2e-gce-storage-slow	bd3be7d6134f87de0d05a4b1510a54b14237c2e4	link	`/test pull-kubernetes-e2e-gce-storage-slow`
pull-kubernetes-e2e-gce-storage-snapshot	bd3be7d6134f87de0d05a4b1510a54b14237c2e4	link	`/test pull-kubernetes-e2e-gce-storage-snapshot`
pull-kubernetes-e2e-gce-alpha-features	afe5c71807f2a08fee36d01ac9d6bd3ce8123a05	link	`/test pull-kubernetes-e2e-gce-alpha-features`

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.

egernst · 2020-02-28T23:45:56Z

/test pull-kubernetes-e2e-kind-ipv6

tallclair · 2020-03-05T19:04:59Z

/milestone v1.18

egernst · 2020-03-05T21:21:06Z

/test tide ??

derekwaynecarr

I need to try this, I am confused by my own math.

derekwaynecarr · 2020-03-05T21:49:44Z

test/e2e_node/runtimeclass_test.go

+	"github.com/onsi/ginkgo"
+)
+
+// makePodToVerifyCgroups returns a pod that verifies the existence of the specified cgroups.


nit: "makePodToVerifyCgroupSize"

derekwaynecarr · 2020-03-05T21:51:21Z

test/e2e_node/runtimeclass_test.go

+					podUID        string
+					handler       string
+				)
+				ginkgo.By("Creating a RuntimeClass with Overhead definied", func() {


nit: "defined"

derekwaynecarr · 2020-03-05T21:58:38Z

test/e2e_node/runtimeclass_test.go

+				})
+				ginkgo.By("Checking if the pod cgroup was created appropriately", func() {
+					cgroupsToVerify := []string{"pod" + podUID}
+					pod := makePodToVerifyCgroupSize(cgroupsToVerify, "30000", "251658240")


just tracking my mental math...

200m+100m=300m cpu

3000 * 1024 / 1000 = 3072

is what i would have expected for cpu.

this is checking cpu.cfs_quota_us not cpu.shares. however, this test should also check cpu.shares.

That would be if you were checking shares, which isn't the relevant setting. Kubelet will set cpu.cfs_quota_us to enforce the CPU limit (which should correspond to the total limits+pod overhead) -- see the check at [1].

CPU quota is based relative to a period (set to 100,000), so we are checking for 30,000, which shows a limiting to effectively. 300 milliCPU.

[1] - https://github.com/kubernetes/kubernetes/pull/88558/files#diff-c3ca917e18d307eabb48f941504b035eR53

Shameless plug to gist @mcastelino put together, discussing shares v. quota: https://gist.github.com/mcastelino/b8ce9a70b00ee56036dadd70ded53e9f#what-happens

Thanks. I saw CFS and mentally said shares and started doing math. Value for quota is right.

egernst force-pushed the e2e_node-PodOverhead branch from bd3be7d to 1bf9651 Compare February 26, 2020 02:11

egernst changed the title ~~E2e node pod overhead~~ e2e node pod overhead Feb 26, 2020

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

k8s-ci-robot requested review from BenTheElder and derekwaynecarr February 26, 2020 02:12

egernst force-pushed the e2e_node-PodOverhead branch from 1bf9651 to 012c060 Compare February 26, 2020 02:14

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 26, 2020

egernst force-pushed the e2e_node-PodOverhead branch from 012c060 to 867e44d Compare February 26, 2020 02:15

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 26, 2020

egernst force-pushed the e2e_node-PodOverhead branch 2 times, most recently from 5c6206d to b7b9a13 Compare February 26, 2020 05:45

egernst mentioned this pull request Feb 26, 2020

Tracking PodOverhead graduation to beta for 1.18 release #88339

Closed

6 tasks

k8s-ci-robot removed the request for review from BenTheElder February 27, 2020 06:37

egernst force-pushed the e2e_node-PodOverhead branch from b7b9a13 to afe5c71 Compare February 27, 2020 16:12

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. sig/network Categorizes an issue or PR as relevant to SIG Network. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 27, 2020

egernst force-pushed the e2e_node-PodOverhead branch from afe5c71 to 8526c1b Compare February 27, 2020 16:40

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 27, 2020

tallclair reviewed Feb 27, 2020

View reviewed changes

egernst force-pushed the e2e_node-PodOverhead branch from 8526c1b to 1b72565 Compare February 28, 2020 16:01

tallclair reviewed Feb 28, 2020

View reviewed changes

egernst force-pushed the e2e_node-PodOverhead branch from 1b72565 to fc77f75 Compare February 28, 2020 23:00

e2e_node add test for PodOverhead feature

aa12e1f

This test will verify that the Pod cgroup created takes Overhead into account. Signed-off-by: Eric Ernst <eric@amperecomputing.com>

egernst force-pushed the e2e_node-PodOverhead branch from fc77f75 to aa12e1f Compare February 28, 2020 23:00

tallclair reviewed Feb 28, 2020

View reviewed changes

k8s-ci-robot assigned tallclair Feb 28, 2020

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

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

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

derekwaynecarr reviewed Mar 5, 2020

View reviewed changes

k8s-ci-robot merged commit e23e720 into kubernetes:master Mar 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

e2e node pod overhead #88558

e2e node pod overhead #88558

egernst commented Feb 26, 2020

egernst commented Feb 26, 2020

BenTheElder commented Feb 27, 2020

tallclair Feb 27, 2020

egernst Feb 27, 2020

tallclair Feb 28, 2020

egernst commented Feb 28, 2020

mrunalp commented Feb 28, 2020

tallclair left a comment

tallclair Feb 28, 2020

tallclair left a comment

k8s-ci-robot commented Feb 28, 2020

k8s-ci-robot commented Feb 28, 2020 •

edited

egernst commented Feb 28, 2020

tallclair commented Mar 5, 2020

egernst commented Mar 5, 2020

derekwaynecarr left a comment

derekwaynecarr Mar 5, 2020

derekwaynecarr Mar 5, 2020

derekwaynecarr Mar 5, 2020

sjenning Mar 5, 2020

egernst Mar 5, 2020

egernst Mar 5, 2020 •

edited

derekwaynecarr Mar 6, 2020

e2e node pod overhead #88558

e2e node pod overhead #88558

Conversation

egernst commented Feb 26, 2020

egernst commented Feb 26, 2020

BenTheElder commented Feb 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

egernst commented Feb 28, 2020

mrunalp commented Feb 28, 2020

tallclair left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tallclair left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Feb 28, 2020

k8s-ci-robot commented Feb 28, 2020 • edited

egernst commented Feb 28, 2020

tallclair commented Mar 5, 2020

egernst commented Mar 5, 2020

derekwaynecarr 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

Choose a reason for hiding this comment

egernst Mar 5, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-ci-robot commented Feb 28, 2020 •

edited

egernst Mar 5, 2020 •

edited