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

Bug 1838431: Machine Remediation Backport to 4.4 #73

Closed
wants to merge 27 commits into from
Closed

Bug 1838431: Machine Remediation Backport to 4.4 #73

wants to merge 27 commits into from

Conversation

n1r1
Copy link

@n1r1 n1r1 commented Jun 3, 2020

Manual cherry pick of #59 in order to backport machine remediation feature to 4.4

n1r1 added 27 commits June 3, 2020 10:26
@n1r1
Adding Machine Remediation controller to reboot unhealthy hosts after…
ca2df67 
… node deletion

Signed-off-by: Nir <niry@redhat.com>
@n1r1
add RBAC rules for machines
8f7c0d2 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
Changed MAO unhealthy annotation to the new one and added logs
a30bfbc 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
Update code to match enhancement design, without extra annotation
92ec2af
@n1r1
Add machine remediation controller to main.go
dea4dc0 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
Add machine remediation unit test
925cc68 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
Revert back to extra annotation usage, as MHC keeps annotating the ma…
a820991 
…chine, resulting in reboot loops

This reverts commit 562a1b4.

Signed-off-by: Nir <niry@redhat.com>
@n1r1
Move machine remediation logic from a separate contoller into actuator
ba77cec 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
Remediate only if machine object wasn't changed previously to avoid c…
7462ce6 
…onflicts

Signed-off-by: Nir <niry@redhat.com>
@n1r1
Change ensureAnnotation return values order (err is now last)
7fcd957 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
Fix requestPowerOff and requestPowerOn comments
5d53d4f 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
Change rebootAnnotation to requestPowerOffAnnotation
096b7ce 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
Add 5 second delay when waiting for a power off
e90af7e 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
fix remediation-in-progress comment
948b9bc 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
Rephrased some logs
ceb9bc6 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
Change test utilities functions to get an object name as an argument
bd2cc58 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
Improve readability
d9ba6d8 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
Avoid adding power off annotation if it is already exist
f2b0a19 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
Add more comments
4b2a5db 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
Rename remediation-in-progress to powered-off-for-remediation
9814405 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
Avoid calling delete if node already has DeletionTimestamp and requeu…
cee5078 
…e after a succesful delete

Signed-off-by: Nir <niry@redhat.com>
@n1r1
Change last condition in remediateIfNeeded to improve readability
3d9338f 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
Rename reboot annotation
fd8e112 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
More readability improvments
cec7911 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
Fail test if deleteNode didn't return a requque request
315518d 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
Add machine remediation to README
e9cbb42 
Signed-off-by: Nir <niry@redhat.com>
@n1r1
fix typo
04643e4 
Signed-off-by: Nir <niry@redhat.com>
@openshift-ci-robot openshift-ci-robot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Jun 3, 2020
@openshift-ci-robot
Copy link

@n1r1: This pull request references Bugzilla bug 1838431, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

6 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.4.z) matches configured target release for branch (4.4.z)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
  • dependent bug Bugzilla bug 1838430 is in the state VERIFIED, which is one of the valid states (VERIFIED, RELEASE_PENDING, CLOSED (ERRATA))
  • dependent Bugzilla bug 1838430 targets the "4.5.0" release, which is one of the valid target releases: 4.5.0, 4.5.z
  • bug has dependents

In response to this:

Bug 1838431: Machine Remediation Backport to 4.4

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.

@n1r1
Copy link
Author

n1r1 commented Jun 3, 2020

/assign @mhrivnak

@stbenjam
Copy link
Member

stbenjam commented Jun 3, 2020

CAPBM is missing e2e-metal-ipi. openshift/release#9462 will enable it, I'd like to see that pass first.

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 3, 2020
@stbenjam
Copy link
Member

stbenjam commented Jun 4, 2020

/refresh

@stbenjam
Copy link
Member

stbenjam commented Jun 4, 2020

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 4, 2020
@stbenjam
Copy link
Member

stbenjam commented Jun 4, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 4, 2020
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mhrivnak, n1r1, stbenjam

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 4, 2020
@eparis
Copy link
Member

eparis commented Jun 5, 2020

/hold
we do not backport features to z-streams. Please provide justification why we would consider doing this. In a private comment in the BZ might be appropriate since the business justification would obviously contain customer information.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 5, 2020
@n1r1
Copy link
Author

n1r1 commented Jun 10, 2020

@eparis, I've added justification to bugzilla.
thanks

@bparees
Copy link

bparees commented Jun 18, 2020

@eparis will be following up on this, but for now it's not being approved pending PM justification.

@n1r1
Copy link
Author

n1r1 commented Jun 21, 2020

Per our bugzilla discussion, I'm closing this PR

@n1r1 n1r1 closed this Jun 21, 2020
@openshift-ci-robot
Copy link

@n1r1: This pull request references Bugzilla bug 1838431. The bug has been updated to no longer refer to the pull request using the external bug tracker.

In response to this:

Bug 1838431: Machine Remediation Backport to 4.4

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

@mhrivnak mhrivnak mhrivnak approved these changes

@hardys hardys Awaiting requested review from hardys

@russellb russellb Awaiting requested review from russellb

Assignees

@mhrivnak mhrivnak

@stbenjam stbenjam

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@n1r1 @openshift-ci-robot @stbenjam @eparis @bparees @mhrivnak