Skip to content

Bug 1936407: Backport of BMO code to 4.7 to support different reboot modes#130

Closed
rdoxenham wants to merge 7 commits intoopenshift:release-4.7from
rdoxenham:rdo-bmo-47
Closed

Bug 1936407: Backport of BMO code to 4.7 to support different reboot modes#130
rdoxenham wants to merge 7 commits intoopenshift:release-4.7from
rdoxenham:rdo-bmo-47

Conversation

@rdoxenham
Copy link

Here we're pulling in the BMO code that supports flexible reboot modes to be issued by clients, allowing soft reboots when required, e.g. maintenance, and hard reboots when remediation and workload recovery needs to take place. This code originally landed in metal3-io/baremetal-operator/795, backported to OpenShift in openshift/baremetal-operator/128, and now back to 4.7.z. We had to pull in a few additional commits that landed in 4.8 as dependencies for the three commits to land.

BZ to track: https://bugzilla.redhat.com/show_bug.cgi?id=1936407

Steven Hardy and others added 6 commits March 8, 2021 11:45
When inspect.metal3.io=disabled is specified as an annotation we skip
inspection and return complete immediately from the Inspecting state.

Partially-Implements: metal3-io/metal3-docs#155
This adds the required changes to the provisioner to support
customised reboot annotations, allowing a node to be powered
off (or fenced) more quickly when required. The default
behaviour prior was to attempt a softPowerOff() first and only
attempt a hardPowerOff() if that failed. With this commit and
its counterparts for the baremetal-operator, by setting the
annotation to have an additional mode, e.g. {"mode": "hard"},
the provisioner will immediately power down the node. If this
isn't listed, or is malformed, we still retain the softPowerOff()
behaviour.
The default reboot-interface behaviour is to attempt a soft power
off, and if this fails, revert to a hard power off (PR openshift#294). For high
availability use-cases we require the ability to immediately power-off
a node. This PR attempts to address that requirement and is part of a
wider solution requiring the CAPBM to set the annotation that we have
detailed and implemented in this commit. The baseline provisioner API
changes have been provided in an earlier commit.

CAPBM PR: openshift/cluster-api-provider-baremetal#138
Also see: https://bugzilla.redhat.com/show_bug.cgi?id=1927678
In this commit we add further integration for the RebootMode type
and no longer rely on a boolean for understanding whether the reboot
request was for a hardPowerOff() or softPowerOff(). This will allow
us to expand the modes we support later down the line if required
without any significant modifications required to the provisioner
API.
@openshift-ci-robot
Copy link

@rdoxenham: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

Details

In response to this:

Bug: 1936407 Backport of BMO code to 4.7 to support different reboot modes

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.

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

Details 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

@stbenjam
Copy link
Member

stbenjam commented Mar 8, 2021

/retitle Bug 1936407: Backport of BMO code to 4.7 to support different reboot modes

Misplaced : :-)

@openshift-ci-robot openshift-ci-robot changed the title Bug: 1936407 Backport of BMO code to 4.7 to support different reboot modes Bug 1936407: Backport of BMO code to 4.7 to support different reboot modes Mar 8, 2021
@openshift-ci-robot openshift-ci-robot added the bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. label Mar 8, 2021
@openshift-ci-robot
Copy link

@rdoxenham: This pull request references Bugzilla bug 1936407, which is invalid:

  • expected the bug to target the "4.7.z" release, but it targets "4.8.0" instead
  • expected Bugzilla bug 1936407 to depend on a bug targeting a release in 4.8.0 and in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but no dependents were found

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Bug 1936407: Backport of BMO code to 4.7 to support different reboot modes

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.

@openshift-ci-robot openshift-ci-robot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Mar 8, 2021
@rdoxenham
Copy link
Author

/bugzilla refresh

@openshift-ci-robot
Copy link

@rdoxenham: This pull request references Bugzilla bug 1936407, which is invalid:

  • expected Bugzilla bug 1936407 to depend on a bug targeting a release in 4.8.0 and in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but no dependents were found

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

/bugzilla refresh

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.

@hardys
Copy link

hardys commented Mar 9, 2021

@stbenjam @honza are we OK backporting the changes to the BMH CRD in the additional commits?

That's going to require an update to the CBO, and I wasn't sure if we'd reached consensus re BMO rebase-backports which include that kind of change?

@stbenjam
Copy link
Member

stbenjam commented Mar 9, 2021

@stbenjam @honza are we OK backporting the changes to the BMH CRD in the additional commits?

That's going to require an update to the CBO, and I wasn't sure if we'd reached consensus re BMO rebase-backports which include that kind of change?

Yea that's a good point, we previously back ported some changes already (https://github.com/openshift/baremetal-operator/pull/127/files). I don't have a good sense of what backporting this Delayed change means.

It's been tough since some late arriving bug fixes were built on top of substantial BMO changes that landed upstream around the time 4.7 was cut.

@rdoxenham
Copy link
Author

@stbenjam @hardys @honza - I raised #132 which deals with the conflict resolution, and it's much cleaner. I don't think this will lead to any significant conflict management later down the line. I suggest we close this one and go with the other, and I will retitle it to coincide with what this PR was attempting to be. I'll let you decide how to proceed. Thx!

@hardys
Copy link

hardys commented Mar 10, 2021

@stbenjam @hardys @honza - I raised #132 which deals with the conflict resolution, and it's much cleaner. I don't think this will lead to any significant conflict management later down the line. I suggest we close this one and go with the other, and I will retitle it to coincide with what this PR was attempting to be. I'll let you decide how to proceed. Thx!

Ok sounds good, thanks, lets go with #132

/close

@openshift-ci-robot
Copy link

@hardys: Closed this PR.

Details

In response to this:

@stbenjam @hardys @honza - I raised #132 which deals with the conflict resolution, and it's much cleaner. I don't think this will lead to any significant conflict management later down the line. I suggest we close this one and go with the other, and I will retitle it to coincide with what this PR was attempting to be. I'll let you decide how to proceed. Thx!

Ok sounds good, thanks, lets go with #132

/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.

@openshift-ci-robot
Copy link

@rdoxenham: This pull request references Bugzilla bug 1936407. The bug has been updated to no longer refer to the pull request using the external bug tracker. All external bug links have been closed. The bug has been moved to the NEW state.

Details

In response to this:

Bug 1936407: Backport of BMO code to 4.7 to support different reboot modes

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.

@rdoxenham rdoxenham deleted the rdo-bmo-47 branch March 10, 2021 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants