Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adding Kubelet E2E Lock Contention job #20106

Closed
wants to merge 1 commit into from
Closed

Adding Kubelet E2E Lock Contention job #20106

wants to merge 1 commit into from

Conversation

knabben
Copy link
Member

@knabben knabben commented Dec 3, 2020

This added a new tab kubelet-gce-e2e-lock running tests focused on NodeFeature:LockContention with the proper flags for the suite.

Ref #20105

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 3, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: knabben
To complete the pull request process, please assign yujuhong after the PR has been reviewed.
You can assign the PR to them by writing /assign @yujuhong in a comment when ready.

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

Needs approval from an approver in each of these files:

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

@knabben knabben changed the title Adding lock contention E2E job Adding Kubelet E2E Lock Contention job Dec 3, 2020
@k8s-ci-robot k8s-ci-robot added area/config Issues or PRs related to code in /config area/jobs sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Dec 3, 2020
@knabben knabben mentioned this pull request Dec 3, 2020
Closed
@SergeyKanzhelev
Copy link
Member

SergeyKanzhelev commented Dec 21, 2020 •
edited
Loading

/area test

@k8s-ci-robot
Copy link
Contributor

@SergeyKanzhelev: The label(s) /label area/test cannot be applied. These labels are supported: api-review, community/discussion, community/maintenance, community/question, cuj/build-train-deploy, cuj/multi-user, platform/aws, platform/azure, platform/gcp, platform/minikube, platform/other, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash

In response to this:

/label area/test

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.

@spiffxp
Copy link
Member

spiffxp commented Jan 19, 2021

/test pull-test-infra-integration
#20498 (comment)

@knabben knabben closed this Feb 20, 2021
@knabben
Copy link
Member Author

knabben commented Mar 10, 2021

/reopen

@k8s-ci-robot k8s-ci-robot reopened this Mar 10, 2021
@k8s-ci-robot
Copy link
Contributor

@knabben: Reopened this PR.

In response to this:

/reopen

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.

@knabben knabben mentioned this pull request Mar 17, 2021
Merged
@SergeyKanzhelev
Copy link
Member

/assign @ike-ma

@k8s-ci-robot
Copy link
Contributor

@SergeyKanzhelev: GitHub didn't allow me to assign the following users: ike-ma.

Note that only kubernetes members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @ike-ma

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.

@ike-ma
Copy link
Contributor

ike-ma commented Apr 20, 2021

Ack. Review in progress.

ike-ma
ike-ma reviewed Apr 21, 2021
View reviewed changes
config/jobs/kubernetes/sig-node/node-kubelet.yaml
@@ -204,6 +204,37 @@ periodics:
testgrid-tab-name: node-kubelet-orphans
description: "Contains all uncategorized tests, these are tests which are not marked with a [Node] tag. All node tests should be marked with [NodeFeature] or [NodeSpecialFeature] or [NodeAlphaFeature] or [NodeConformance] classification. Also skipped are [Flaky], [Benchmark], [Legacy]."

- name: ci-kubernetes-node-kubelet-lock-contention
interval: 4h
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to understand the context, any particular reason on why we are using a 4 hour intervals? Ditto for the two timeout value at ln#217 and ln#229?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are duplicated, nice catch. the 4 hours interval was a standard for these jobs and there's no particular reason for this job. Any suggestion for a better value?

config/jobs/kubernetes/sig-node/node-kubelet.yaml
- --node-test-args=--kubelet-flags="--exit-on-lock-contention --lock-file=/var/run/kubelet.lock"
- --node-tests=true
- --provider=gce
- --test_args=--nodes=1 --focus="\[NodeFeature:LockContention\]" --restart-kubelet=false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider update --restart-kubelet argument passing based on final decisions kubernetes/kubernetes#97028

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this PR should be merged after kubernetes/kubernetes#97028

@ike-ma ike-ma mentioned this pull request Apr 26, 2021
Closed
@SergeyKanzhelev
Copy link
Member

@knabben can you please reply to @ike-ma

@SergeyKanzhelev
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 30, 2021
This was referenced Jul 1, 2021
Open
Merged
@knabben
Copy link
Member Author

knabben commented Jul 1, 2021

@ipochi will continue the work on:
#22758

/close

@k8s-ci-robot
Copy link
Contributor

@knabben: Closed this PR.

In response to this:

@ipochi will continue the work on:
#22758

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ike-ma ike-ma ike-ma left review comments

@karan karan Awaiting requested review from karan

@sjenning sjenning Awaiting requested review from sjenning

Assignees

@SergeyKanzhelev SergeyKanzhelev

Labels
area/config Issues or PRs related to code in /config area/jobs cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
SIG Node CI/Test Board
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@knabben @k8s-ci-robot @SergeyKanzhelev @spiffxp @ike-ma