Better handling of power management errors by sadasu · Pull Request #841 · metal3-io/baremetal-operator

sadasu · 2021-03-30T22:25:29Z

Attempts to fix #828

This fix should have the following components based on the above bug:

If Ironic accepts the power state change but is unable to carry it out (expressed via LastError from ironic), that should result in PowerOn() and PowerOff() returning an ErrorMessage.
Refactor PowerOff() in such a way that if soft power off fails, it proceeds to hard power off without reporting an error. It is possible that this logic is moved out of here and into Ironic itself.
Add a force flag to the PowerOff() so that it skips soft power off and directly proceeds to hard power off after the previous call results in an error.

sadasu · 2021-04-01T19:06:23Z

/test-integration

sadasu · 2021-04-01T19:09:03Z

/assign @zaneb @andfasano @dtantsur

sadasu · 2021-04-06T17:50:28Z

/test-integration

sadasu · 2021-04-06T18:57:22Z

/test-integration

sadasu · 2021-04-08T16:17:39Z

/test-integration

sadasu · 2021-04-08T16:48:28Z

/test-integration

andfasano

Reviewed the latest change and it looks ok to me. AFAIU, part of the duplication and errors check fix will be addressed in #816

/lgtm

andfasano · 2021-07-02T09:32:01Z

-		case HostLockedError:
-		default:
-			return transientError(errors.Wrap(err, "failed to power on host"))
+		if ironicNode.LastError != "" && !force {


metal3-io-bot · 2021-07-02T09:37:00Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andfasano, elfosardo, zaneb

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:

~~OWNERS~~ [zaneb]

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

furkatgofurov7 · 2021-07-09T06:42:32Z

/test ?

metal3-io-bot · 2021-07-09T06:42:36Z

@furkatgofurov7: The following commands are available to trigger jobs:

/test gofmt
/test gosec
/test gomod
/test markdownlint
/test shellcheck
/test unit
/test generate
/test golint
/test manifestlint

Use /test all to run all jobs.

Details

In response to this:

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

furkatgofurov7 · 2021-07-09T06:43:15Z

/test all

furkatgofurov7 · 2021-07-09T06:45:09Z

/hold

furkatgofurov7 · 2021-07-09T06:48:28Z

/test golint

furkatgofurov7 · 2021-07-09T06:53:20Z

/hold cancel

fmuyassarov · 2021-07-09T07:56:12Z

/test golint

sadasu · 2021-07-09T21:29:10Z

/test-integration

andfasano · 2021-07-12T12:51:38Z

/lgtm

metal3-io-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 30, 2021

sadasu force-pushed the power-mgmt branch from 6ded954 to 683f54d Compare April 1, 2021 19:03

sadasu changed the title ~~WIP: Better handling of power management errors~~ Better handling of power management errors Apr 1, 2021

metal3-io-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 1, 2021

metal3-io-bot assigned andfasano, dtantsur and zaneb Apr 1, 2021

metal3-io-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 5, 2021

sadasu force-pushed the power-mgmt branch from 51a6ae2 to 65bb2c5 Compare April 5, 2021 20:59

dtantsur requested changes Apr 6, 2021

View reviewed changes

Comment thread pkg/provisioner/ironic/ironic.go Outdated

Comment thread pkg/provisioner/ironic/ironic.go

Comment thread pkg/provisioner/ironic/ironic.go Outdated

sadasu force-pushed the power-mgmt branch 2 times, most recently from 59c160d to f60af94 Compare April 6, 2021 17:42

metal3-io-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 6, 2021

sadasu force-pushed the power-mgmt branch from f60af94 to 8f108ac Compare April 6, 2021 18:55

sadasu commented Apr 6, 2021

View reviewed changes

Comment thread controllers/metal3.io/baremetalhost_controller.go

zaneb reviewed Apr 7, 2021

View reviewed changes

Comment thread pkg/provisioner/ironic/ironic.go Outdated

Comment thread pkg/provisioner/ironic/ironic.go Outdated

Comment thread pkg/provisioner/ironic/ironic.go Outdated

Comment thread pkg/provisioner/ironic/ironic.go Outdated

Comment thread controllers/metal3.io/baremetalhost_controller.go

sadasu force-pushed the power-mgmt branch from 8f108ac to 8897e73 Compare April 8, 2021 15:50

metal3-io-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 8, 2021

sadasu force-pushed the power-mgmt branch 2 times, most recently from d646036 to d5f9ef0 Compare April 8, 2021 16:17

andfasano approved these changes Jul 2, 2021

View reviewed changes

metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Jul 2, 2021

metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 9, 2021

metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 9, 2021

sadasu added 5 commits July 9, 2021 17:10

Look at Ironic Node's LastError while managing its power state

5b37ea1

Add a force flag to PowerOff() to force a hardPowerOff after prior error

3684785

Add a force flag to PowerOn() to retry after a prior error

85c0526

Augment unit test for PowerOn() with force flag and LastError

82a2492

Augment PowerOff() tests with cases for LastError and force flag

e4614b3

sadasu force-pushed the power-mgmt branch from 55542d2 to a57aae2 Compare July 9, 2021 21:11

metal3-io-bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 9, 2021

sadasu added 2 commits July 9, 2021 17:24

Improve error handling in PowerOff(), softPowerOff() and hardPowerOff()

67a27dc

Remove SoftPowerOffFailed error

b86a290

sadasu force-pushed the power-mgmt branch from a57aae2 to b86a290 Compare July 9, 2021 21:25

metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Jul 12, 2021

metal3-io-bot merged commit 62372c1 into metal3-io:master Jul 12, 2021

This was referenced Sep 20, 2021

Power off fails for drivers that don't support Soft power off #984

Closed

Fix fallback for ironic drivers that don't support soft power off #985

Merged

This was referenced Oct 1, 2021

Bug 2009850: Fix fallback for ironic drivers that don't support soft power off openshift/baremetal-operator#182

Closed

[release-4.9] Bug 2009850: Fix fallback for ironic drivers that don't support soft power off openshift/baremetal-operator#184

Merged

Conversation

sadasu commented Mar 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sadasu commented Apr 1, 2021

Uh oh!

sadasu commented Apr 1, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sadasu commented Apr 6, 2021

Uh oh!

sadasu commented Apr 6, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sadasu commented Apr 8, 2021

Uh oh!

sadasu commented Apr 8, 2021

Uh oh!

andfasano left a comment

Choose a reason for hiding this comment

Uh oh!

andfasano Jul 2, 2021

Choose a reason for hiding this comment

Uh oh!

metal3-io-bot commented Jul 2, 2021

Uh oh!

furkatgofurov7 commented Jul 9, 2021

Uh oh!

metal3-io-bot commented Jul 9, 2021

Uh oh!

furkatgofurov7 commented Jul 9, 2021

Uh oh!

furkatgofurov7 commented Jul 9, 2021

Uh oh!

furkatgofurov7 commented Jul 9, 2021

Uh oh!

furkatgofurov7 commented Jul 9, 2021

Uh oh!

fmuyassarov commented Jul 9, 2021

Uh oh!

sadasu commented Jul 9, 2021

Uh oh!

andfasano commented Jul 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

sadasu commented Mar 30, 2021 •

edited

Loading