Skip to content

Revert "[NodeKiller] Schedule reboot and disconnect from node" #1140

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
Merged
merged 1 commit into from
Mar 30, 2020
Merged

Revert "[NodeKiller] Schedule reboot and disconnect from node" #1140

merged 1 commit into from
Mar 30, 2020

Conversation

jprzychodzen
Copy link
Contributor

@jprzychodzen jprzychodzen commented Mar 30, 2020 •
edited
Loading

This reverts commit f1647f4.

Required to make Node Killer work again - with shutdown it silently fails.

/hold

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 30, 2020
@jprzychodzen
Copy link
Contributor Author

/unhold
/assign @mm4tt

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 30, 2020
@jprzychodzen
Copy link
Contributor Author

@mcastellin Please be aware about this change.

@mm4tt
Copy link
Contributor

mm4tt commented Mar 30, 2020

FTR, we know that something is wrong with the "shutdown" command, here is an example where 6 nodes were killed, but none of them properly rebooted - https://gke-scalability-prow.corp.goog/log?job=ci-kubernetes-e2e-scalability-experimental-load-containerd-jprzychodzen&id=1244575398428151814
We don't know what exactly is wrong, though. We manually verified that reverting this change helps, nodes started getting rebooted properly.

/lgtm
/approve

I'm re-opening the #1109 in that case.

@mm4tt mm4tt mentioned this pull request Mar 30, 2020
Open
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 30, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jprzychodzen, mm4tt

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:

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 k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 30, 2020
@k8s-ci-robot k8s-ci-robot merged commit 53ca1f3 into kubernetes:master Mar 30, 2020
@jprzychodzen jprzychodzen deleted the revertnk branch March 30, 2020 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krzysied krzysied Awaiting requested review from krzysied

@wojtek-t wojtek-t Awaiting requested review from wojtek-t

Assignees

@mm4tt mm4tt

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. 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. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jprzychodzen @mm4tt @k8s-ci-robot