Remove docker node periodics, ensure CRI replacements #24595

ehashman · 2021-12-08T23:02:34Z

Going through tab by tab on node-kubelet-serial

It is a mistake to try to do the presubmits in the same PR, let's do that separately.

Follow-ups:

/sig node

Fixes #24592

ehashman · 2021-12-09T00:43:56Z

Tests pass, ready for review.

Coverage for this is provided by ci-containerd-node-e2e and ci-crio-cgroupv1-node-e2e-conformance.

Covered by ci-cri-containerd-e2e-cos-gce-alpha-features and new cri-o job ci-crio-cgroupv1-node-e2e-alpha.

Replace with ci-kubernetes-node-kubelet-containerd-eviction and ci-crio-cgroupv1-node-e2e-eviction.

Covered by ci-cri-containerd-node-e2e-features and ci-crio-cgroupv1-node-e2e-features.

Covered by ci-cri-containerd-e2e-cos-gce-flaky and ci-crio-cgroupv1-node-e2e-flaky.

SergeyKanzhelev · 2021-12-10T21:58:50Z

config/jobs/kubernetes/sig-node/containerd.yaml

  annotations:
    testgrid-dashboards: sig-node-containerd
    testgrid-tab-name: node-e2e-features
+- name: ci-cri-containerd-node-e2e-unlabelled


why renaming from orphans?

Because it's a pretty insensitive name; new name is more descriptive and inclusive.

It's running on docker, and it's also running no tests. I'm just removing it for now.

Replace it with new jobs ci-cri-containerd-node-e2e-unlabelled and ci-crio-cgroupv1-node-e2e-unlabelled which have more descriptive names.

These are covered by ci-kubernetes-node-kubelet-serial-containerd and ci-kubernetes-node-kubelet-serial-cri-o jobs. DevicePluginProbe tests should be readded in a follow-up; they are currently failing. See kubernetes/kubernetes#106635

SergeyKanzhelev · 2021-12-10T22:01:17Z

config/jobs/kubernetes/sig-node/crio.yaml

    testgrid-tab-name: ci-crio-cgroupv1-node-e2e-conformance
    testgrid-alert-email: [email protected]
    description: "OWNER: sig-node; runs NodeConformance e2e tests with crio master and cgroup v1"
+- name: ci-crio-cgroupv1-node-e2e-alpha


is it a new test or I am missing some renaming? no objections, just trying to understand

maybe doing as a separate PR would be easier to review

New test to add 2 container runtimes of coverage for alpha features. Explained in each commit comment which jobs were replacing each removal. Added jobs in containerd config are similar.

maybe doing as a separate PR would be easier to review

you may want to review commit by commit, that's why I split it up that way

I will file follow-up issues for those and alert the owners.

SergeyKanzhelev · 2021-12-10T22:09:20Z

config/jobs/kubernetes/sig-node/node-kubelet.yaml

-      - --test-args=--kubelet-flags="--cgroups-per-qos=true --cgroup-root=/"
-      - --image-config-file=/home/prow/go/src/k8s.io/test-infra/jobs/e2e_node/image-config.yaml
-
- name: ci-kubernetes-node-kubelet-alpha


is the

test-infra/config/jobs/kubernetes/sig-node/containerd.yaml

Line 457 in 1722302

name: ci-cri-containerd-e2e-cos-gce-alpha-features

containerd match for it?

From 26adeb7 commit message:

Remove ci-kubernetes-node-kubelet-alpha/-kubetest2

Covered by ci-cri-containerd-e2e-cos-gce-alpha-features and new cri-o
job ci-crio-cgroupv1-node-e2e-alpha.

dims · 2021-12-10T22:10:54Z

/approve

k8s-ci-robot · 2021-12-10T22:11:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, ehashman

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~config/jobs/kubernetes/sig-node/OWNERS~~ [dims,ehashman]
~~config/testgrids/kubernetes/sig-node/OWNERS~~ [dims,ehashman]

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

SergeyKanzhelev · 2021-12-10T22:14:09Z

config/jobs/kubernetes/sig-node/crio.yaml

    testgrid-tab-name: ci-crio-cgroupv1-node-e2e-features
    testgrid-alert-email: [email protected]
    description: "OWNER: sig-node; runs NodeFeatures e2e tests with crio master and cgroup v1"
+- name: ci-crio-cgroupv1-node-e2e-eviction


I understand now that you are trying to add cri-o replacement for docker as well. Would it be better to have the names pattern match between runtimes?

between ci-kubernetes-node-kubelet-containerd-eviction and ci-crio-cgroupv1-node-e2e-eviction, can we match the name pattern or cross reference tests?

Can we rename in a separate PR? All of the job names are kind of out of sync and I know SIG Testing had previously suggested we take the runtimes out of the job names entirely.

ehashman · 2021-12-10T22:34:12Z

/retest-required

looks unrelated, maybe a flake?

k8s-ci-robot · 2021-12-10T22:39:28Z

@ehashman: Updated the job-config configmap in namespace default at cluster test-infra-trusted using the following files:

key containerd.yaml using file config/jobs/kubernetes/sig-node/containerd.yaml
key crio.yaml using file config/jobs/kubernetes/sig-node/crio.yaml
key node-docker.yaml using file ``
key node-kubelet.yaml using file config/jobs/kubernetes/sig-node/node-kubelet.yaml

Details

In response to this:

Going through tab by tab on node-kubelet-serial

It is a mistake to try to do the presubmits in the same PR, let's do that separately.

Follow-ups:

Move docker-based CPU manager serial tests to CRI #24616

Move docker-based hugepages test to CRI #24617

Move Docker-based Ubuntu swap test to containerd #24618

Move Docker-based SIG Node performance tests to CRI #24619

Move docker-based node presubmits to CRI #24620

/sig node

Fixes #24592

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.

k8s-ci-robot requested review from MHBauer and bart0sh December 8, 2021 23:05

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 8, 2021

ehashman force-pushed the remove-docker-node branch from 86cfe31 to fbe6e99 Compare December 8, 2021 23:15

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

ehashman force-pushed the remove-docker-node branch 2 times, most recently from d3148be to 9045cd8 Compare December 8, 2021 23:21

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 8, 2021

ehashman force-pushed the remove-docker-node branch from 95b6ecd to f779bba Compare December 8, 2021 23:50

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. area/testgrid and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 8, 2021

ehashman force-pushed the remove-docker-node branch 2 times, most recently from eff0fe2 to 848cb15 Compare December 9, 2021 00:13

k8s-ci-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 9, 2021

ehashman changed the title ~~[WIP] Remove docker node tests, ensure CRI replacements~~ [WIP] Remove docker node periodics, ensure CRI replacements Dec 9, 2021

ehashman force-pushed the remove-docker-node branch from 848cb15 to 7074300 Compare December 9, 2021 00:26

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 9, 2021

ehashman changed the title ~~[WIP] Remove docker node periodics, ensure CRI replacements~~ Remove docker node periodics, ensure CRI replacements Dec 9, 2021

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 Dec 9, 2021

k8s-ci-robot assigned SergeyKanzhelev Dec 9, 2021

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

ehashman force-pushed the remove-docker-node branch from d5c7824 to 1722302 Compare December 9, 2021 19:05

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Dec 9, 2021

ehashman added 5 commits December 10, 2021 13:49

Remove ci-kubernetes-node-kubelet/-kubetest2 jobs

812b5b1

Coverage for this is provided by ci-containerd-node-e2e and ci-crio-cgroupv1-node-e2e-conformance.

Remove ci-kubernetes-node-kubelet-alpha/-kubetest2

26adeb7

Covered by ci-cri-containerd-e2e-cos-gce-alpha-features and new cri-o job ci-crio-cgroupv1-node-e2e-alpha.

Remove ci-kubernetes-node-kubelet-eviction job

fe6286c

Replace with ci-kubernetes-node-kubelet-containerd-eviction and ci-crio-cgroupv1-node-e2e-eviction.

Remove node-kubelet-features/-kubetest2 jobs

3297c38

Covered by ci-cri-containerd-node-e2e-features and ci-crio-cgroupv1-node-e2e-features.

Remove ci-kubernetes-node-kubelet-flaky job

f90230a

Covered by ci-cri-containerd-e2e-cos-gce-flaky and ci-crio-cgroupv1-node-e2e-flaky.

SergeyKanzhelev reviewed Dec 10, 2021

View reviewed changes

ehashman added 3 commits December 10, 2021 13:59

Remove ci-kubernetes-node-kubelet-lock-contention

1b54c26

It's running on docker, and it's also running no tests. I'm just removing it for now.

Remove ci-kubernetes-node-kubelet-orphans job

bf2be43

Replace it with new jobs ci-cri-containerd-node-e2e-unlabelled and ci-crio-cgroupv1-node-e2e-unlabelled which have more descriptive names.

SergeyKanzhelev reviewed Dec 10, 2021

View reviewed changes

ehashman added 3 commits December 10, 2021 14:01

Disable failing docker tests that require special configs

75ca709

I will file follow-up issues for those and alert the owners.

Remove sig-node-docker dashboard and jobs

6825878

Remove unused TestGroups

4aa39f6

ehashman force-pushed the remove-docker-node branch from 1722302 to 4aa39f6 Compare December 10, 2021 22:02

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

SergeyKanzhelev reviewed Dec 10, 2021

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 10, 2021

ehashman mentioned this pull request Dec 10, 2021

Rename SIG Node periodic job names for consistency #24641

Closed

k8s-ci-robot merged commit fe0ecac into kubernetes:master Dec 10, 2021

k8s-ci-robot added this to the v1.24 milestone Dec 10, 2021

Remove docker node periodics, ensure CRI replacements #24595

Remove docker node periodics, ensure CRI replacements #24595

Uh oh!

Conversation

ehashman commented Dec 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ehashman commented Dec 9, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dims commented Dec 10, 2021

Uh oh!

k8s-ci-robot commented Dec 10, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ehashman commented Dec 10, 2021

Uh oh!

k8s-ci-robot commented Dec 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ehashman commented Dec 8, 2021 •

edited

Loading