From f374c004fc589b7a1b7af485ea9e0bd6de930af5 Mon Sep 17 00:00:00 2001 From: Derek Higgins Date: Fri, 31 Jan 2025 15:44:05 +0000 Subject: [PATCH] Reboot instead of power on/off If DisablePowerOff is enabled we can't power off for the reboot annotation, instead we call a reboot action and clear the annotation. Signed-off-by: Derek Higgins --- .../metal3.io/baremetalhost_controller.go | 12 ++++++ .../baremetalhost_controller_test.go | 42 +++++++++++++++++++ pkg/provisioner/fixture/fixture.go | 24 +++++++---- pkg/provisioner/ironic/ironic.go | 12 +++++- 4 files changed, 81 insertions(+), 9 deletions(-) diff --git a/controllers/metal3.io/baremetalhost_controller.go b/controllers/metal3.io/baremetalhost_controller.go index 8340a2500d..eead000747 100644 --- a/controllers/metal3.io/baremetalhost_controller.go +++ b/controllers/metal3.io/baremetalhost_controller.go @@ -1594,6 +1594,18 @@ func (r *BareMetalHostReconciler) manageHostPower(prov provisioner.Provisioner, return actionError{errors.Wrap(err, "failed to manage power state of host")} } + // If DisablePowerOff was enabled then prov.PowerOff above will have rebooted instead of powering off, in this case + // the operation is complete (no need to power on) and any reboot annotation can be removed + if info.host.Spec.DisablePowerOff { + if _, suffixlessAnnotationExists := info.host.Annotations[metal3api.RebootAnnotationPrefix]; suffixlessAnnotationExists { + delete(info.host.Annotations, metal3api.RebootAnnotationPrefix) + if err = r.Update(info.ctx, info.host); err != nil { + return actionError{errors.Wrap(err, "failed to remove reboot annotation from host")} + } + return actionContinue{} + } + } + if provResult.ErrorMessage != "" { if !desiredPowerOnState && desiredRebootMode == metal3api.RebootModeSoft && info.host.Status.ErrorType != metal3api.PowerManagementError { diff --git a/controllers/metal3.io/baremetalhost_controller_test.go b/controllers/metal3.io/baremetalhost_controller_test.go index 4d683afc98..b5f2f73643 100644 --- a/controllers/metal3.io/baremetalhost_controller_test.go +++ b/controllers/metal3.io/baremetalhost_controller_test.go @@ -797,6 +797,48 @@ func TestRebootWithSuffixedAnnotation(t *testing.T) { ) } +// TestRebootWithSuffixedAnnotation tests a full reboot cycle, with suffixed annotation +// to verify that controller holds power off until annotation removal. +func TestRebootWithSuffixedAnnotationPowerOffDisabled(t *testing.T) { + host := newDefaultHost(t) + host.Status.PoweredOn = true + host.Status.Provisioning.State = metal3api.StateProvisioned + host.Spec.Online = true + host.Spec.Image = &metal3api.Image{URL: "foo", Checksum: "123"} + host.Spec.Image.URL = "foo" + host.Status.Provisioning.Image.URL = "foo" + host.Spec.DisablePowerOff = true + + fix := fixture.Fixture{DisablePowerOff: true, PoweredOn: true} + r := newTestReconcilerWithFixture(&fix, host) + + // bmh reconcils until credentials are verified as valid + tryReconcile(t, r, host, + func(host *metal3api.BareMetalHost, result reconcile.Result) bool { + return host.Status.GoodCredentials.Reference != nil + }, + ) + + assert.True(t, host.Status.PoweredOn) + assert.Equal(t, fix.RebootCalled, false) + + // Add the reboot annotation + host.Annotations = make(map[string]string) + host.Annotations[metal3api.RebootAnnotationPrefix] = "{\"mode\":\"soft\"}" + err := r.Update(context.TODO(), host) + assert.NoError(t, err) + + tryReconcile(t, r, host, + func(host *metal3api.BareMetalHost, result reconcile.Result) bool { + _, ok := host.Annotations[metal3api.RebootAnnotationPrefix] + return !ok + }, + ) + + assert.True(t, host.Status.PoweredOn) + assert.Equal(t, fix.RebootCalled, true) +} + // TestRebootWithServicing tests full reboot cycle with suffixless // annotation and servicing. func TestRebootWithServicing(t *testing.T) { diff --git a/pkg/provisioner/fixture/fixture.go b/pkg/provisioner/fixture/fixture.go index 51fa446e39..f9bac981ac 100644 --- a/pkg/provisioner/fixture/fixture.go +++ b/pkg/provisioner/fixture/fixture.go @@ -73,12 +73,16 @@ type Fixture struct { BecomeReadyCounter int // state to manage deletion Deleted bool + // state to manage DisablePowerOff + DisablePowerOff bool + // state to manage power + PoweredOn bool + // Has reboot been called + RebootCalled bool // state to manage the two-step adopt process adopted bool // state to manage provisioning image metal3api.Image - // state to manage power - poweredOn bool // state to manage inspection inspectionStarted bool @@ -211,7 +215,7 @@ func (p *fixtureProvisioner) InspectHardware(_ provisioner.InspectData, _, _, _ // is expected to do this in the least expensive way possible, such as // reading from a cache. func (p *fixtureProvisioner) UpdateHardwareState() (hwState provisioner.HardwareState, err error) { - hwState.PoweredOn = &p.state.poweredOn + hwState.PoweredOn = &p.state.PoweredOn p.log.Info("updating hardware state") return } @@ -334,10 +338,10 @@ func (p *fixtureProvisioner) Detach() (result provisioner.Result, err error) { func (p *fixtureProvisioner) PowerOn(_ bool) (result provisioner.Result, err error) { p.log.Info("ensuring host is powered on") - if !p.state.poweredOn { + if !p.state.PoweredOn { p.publisher("PowerOn", "Host powered on") p.log.Info("changing status") - p.state.poweredOn = true + p.state.PoweredOn = true result.Dirty = true return result, nil } @@ -350,10 +354,16 @@ func (p *fixtureProvisioner) PowerOn(_ bool) (result provisioner.Result, err err func (p *fixtureProvisioner) PowerOff(_ metal3api.RebootMode, _ bool) (result provisioner.Result, err error) { p.log.Info("ensuring host is powered off") - if p.state.poweredOn { + if p.state.DisablePowerOff { + p.state.RebootCalled = true + result.Dirty = true + return result, nil + } + + if p.state.PoweredOn { p.publisher("PowerOff", "Host powered off") p.log.Info("changing status") - p.state.poweredOn = false + p.state.PoweredOn = false result.Dirty = true return result, nil } diff --git a/pkg/provisioner/ironic/ironic.go b/pkg/provisioner/ironic/ironic.go index 18f08cedec..13844cce03 100644 --- a/pkg/provisioner/ironic/ironic.go +++ b/pkg/provisioner/ironic/ironic.go @@ -1642,13 +1642,21 @@ func (p *ironicProvisioner) PowerOff(rebootMode metal3api.RebootMode, force bool } if rebootMode == metal3api.RebootModeSoft && !force { - result, err = p.changePower(ironicNode, nodes.SoftPowerOff) + powerTarget := nodes.SoftPowerOff + if ironicNode.DisablePowerOff { + powerTarget = nodes.SoftRebooting + } + result, err = p.changePower(ironicNode, powerTarget) if !errors.As(err, &softPowerOffUnsupportedError{}) { return result, err } } // Reboot mode is hard, force flag is set, or soft power off is not supported - return p.changePower(ironicNode, nodes.PowerOff) + powerTarget := nodes.PowerOff + if ironicNode.DisablePowerOff { + powerTarget = nodes.Rebooting + } + return p.changePower(ironicNode, powerTarget) } return operationComplete()