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.

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

✨ Add support for Disablepoweroff #2229

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions apis/metal3.io/v1alpha1/baremetalhost_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,11 @@ type BareMetalHostSpec struct {
// eventually populated by inspection.
// +optional
Architecture string `json:"architecture,omitempty"`

// When set to true, power off of the node will be disabled,
// instead, a reboot will be used in place of power on/off
// +optional
DisablePowerOff bool `json:"disablePowerOff,omitempty"`
}

// AutomatedCleaningMode is the interface to enable/disable automated cleaning
Expand Down
11 changes: 11 additions & 0 deletions apis/metal3.io/v1alpha1/baremetalhost_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ func (host *BareMetalHost) validateHost() []error {
errs = append(errs, annotationErrors...)
}

if err := validatePowerStatus(host); err != nil {
errs = append(errs, err)
}

return errs
}

Expand Down Expand Up @@ -350,3 +354,10 @@ func (host *BareMetalHost) validateCrossNamespaceSecretReferences() []error {
}
return errs
}

func validatePowerStatus(host *BareMetalHost) error {
if host.Spec.DisablePowerOff && !host.Spec.Online {
return fmt.Errorf("node can't simultaneously have online set to false and have power off disabled")
}
return nil
}
20 changes: 20 additions & 0 deletions apis/metal3.io/v1alpha1/baremetalhost_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,26 @@ func TestValidateCreate(t *testing.T) {
oldBMH: nil,
wantedErr: "", // Should be valid
},
{
name: "disablePowerOff",
newBMH: &BareMetalHost{
Spec: BareMetalHostSpec{
DisablePowerOff: true,
Online: true,
},
},
wantedErr: "",
},
{
name: "disablePowerOffErr",
newBMH: &BareMetalHost{
Spec: BareMetalHostSpec{
DisablePowerOff: true,
Online: false,
},
},
wantedErr: "node can't simultaneously have online set to false and have power off disabled",
},
}

for _, tt := range tests {
Expand Down
5 changes: 5 additions & 0 deletions config/base/crds/bases/metal3.io_baremetalhosts.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,11 @@ spec:
description: Description is a human-entered text used to help identify
the host.
type: string
disablePowerOff:
description: |-
When set to true, power off of the node will be disabled,
instead, a reboot will be used in place of power on/off
type: boolean
externallyProvisioned:
description: |-
ExternallyProvisioned means something else has provisioned the
Expand Down
5 changes: 5 additions & 0 deletions config/render/capm3.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,11 @@ spec:
description: Description is a human-entered text used to help identify
the host.
type: string
disablePowerOff:
description: |-
When set to true, power off of the node will be disabled,
instead, a reboot will be used in place of power on/off
type: boolean
externallyProvisioned:
description: |-
ExternallyProvisioned means something else has provisioned the
Expand Down
18 changes: 18 additions & 0 deletions controllers/metal3.io/baremetalhost_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,11 @@ func setErrorMessage(host *metal3api.BareMetalHost, errType metal3api.ErrorType,
}

func (r *BareMetalHostReconciler) actionPowerOffBeforeDeleting(prov provisioner.Provisioner, info *reconcileInfo) actionResult {
if info.host.Spec.DisablePowerOff {
info.log.Info("Skipping host powered off as Power Off has been disabled")
return actionComplete{}
}

info.log.Info("host ready to be powered off")
provResult, err := prov.PowerOff(
metal3api.RebootModeHard,
Expand Down Expand Up @@ -840,6 +845,7 @@ func (r *BareMetalHostReconciler) registerHost(prov provisioner.Provisioner, inf
PreprovisioningImage: preprovImg,
PreprovisioningNetworkData: preprovisioningNetworkData,
HasCustomDeploy: hasCustomDeploy(info.host),
DisablePowerOff: info.host.Spec.DisablePowerOff,
},
credsChanged,
info.host.Status.ErrorType == metal3api.RegistrationError)
Expand Down Expand Up @@ -1588,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 {
Expand Down
8 changes: 8 additions & 0 deletions pkg/provisioner/ironic/clients/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,15 @@ func (af AvailableFeatures) HasDataImage() bool {
return af.MaxVersion >= 89
}

func (af AvailableFeatures) HasDisablePowerOff() bool {
return af.MaxVersion >= 95
}

func (af AvailableFeatures) ChooseMicroversion() string {
if af.HasDisablePowerOff() {
return "1.95"
}

if af.HasDataImage() {
return "1.89"
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/provisioner/ironic/clients/features_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
)

func TestAvailableFeatures_ChooseMicroversion(t *testing.T) {
microVersion := "1.89"
microVersion := "1.95"
type fields struct {
MaxVersion int
}
Expand All @@ -25,7 +25,7 @@ func TestAvailableFeatures_ChooseMicroversion(t *testing.T) {
{
name: fmt.Sprintf("MaxVersion = %d return %s", 89, microVersion),
feature: fields{
MaxVersion: 89,
MaxVersion: 95,
},
want: microVersion,
},
Expand Down
12 changes: 10 additions & 2 deletions pkg/provisioner/ironic/ironic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
13 changes: 13 additions & 0 deletions pkg/provisioner/ironic/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,15 @@ func (p *ironicProvisioner) Register(data provisioner.ManagementAccessData, cred
return result, "", err
}

// Refuse to manage a node that has Disabled Power off if not supported by ironic,
// accidentally powering it off would require a arctic expedition to the data center
if data.DisablePowerOff && !p.availableFeatures.HasDisablePowerOff() {
msg := "current ironic version does not support DisablePowerOff, refusing to manage node"
p.log.Info(msg)
result, err = operationFailed(msg)
return result, "", err
}

var ironicNode *nodes.Node
updater := clients.UpdateOptsBuilder(p.log)

Expand Down Expand Up @@ -129,6 +138,9 @@ func (p *ironicProvisioner) Register(data provisioner.ManagementAccessData, cred
updater.SetTopLevelOpt("driver_info", driverInfo, ironicNode.DriverInfo)
}

// The updater only updates disable_power_off if it has changed
updater.SetTopLevelOpt("disable_power_off", data.DisablePowerOff, ironicNode.DisablePowerOff)

// We don't return here because we also have to set the
// target provision state to manageable, which happens
// below.
Expand Down Expand Up @@ -232,6 +244,7 @@ func (p *ironicProvisioner) enrollNode(data provisioner.ManagementAccessData, bm
PowerInterface: bmcAccess.PowerInterface(),
RAIDInterface: bmcAccess.RAIDInterface(),
VendorInterface: bmcAccess.VendorInterface(),
DisablePowerOff: &data.DisablePowerOff,
Properties: map[string]interface{}{
"capabilities": buildCapabilitiesValue(nil, data.BootMode),
},
Expand Down
57 changes: 57 additions & 0 deletions pkg/provisioner/ironic/register_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1417,3 +1417,60 @@ func TestSetExternalURLRemoving(t *testing.T) {

assert.Equal(t, nil, updatedDriverInfo["external_http_url"])
}

func TestRegisterDisablePowerOff(t *testing.T) {
// Create a host with disable power off enabled
host := makeHost()
host.Spec.DisablePowerOff = true

// Set up ironic server to return the node
ironic := testserver.NewIronic(t).
Node(nodes.Node{
UUID: host.Status.Provisioning.ID,
}).NodeUpdate(nodes.Node{
UUID: host.Status.Provisioning.ID,
})
ironic.Start()
defer ironic.Stop()

auth := clients.AuthConfig{Type: clients.NoAuth}
prov, err := newProvisionerWithSettings(host, bmc.Credentials{}, nil, ironic.Endpoint(), auth)
if err != nil {
t.Fatalf("could not create provisioner: %s", err)
}

prov.TryInit()
result, _, err := prov.Register(provisioner.ManagementAccessData{DisablePowerOff: true}, false, false)
if err != nil {
t.Fatalf("error from Register: %s", err)
}
assert.Equal(t, "", result.ErrorMessage)
}

func TestRegisterDisablePowerOffNotAvail(t *testing.T) {
// Create a host with disable power off enabled
host := makeHost()

// Set up ironic server to return the node
ironic := testserver.NewIronic(t).WithVersion("1.87").
Node(nodes.Node{
UUID: host.Status.Provisioning.ID,
}).NodeUpdate(nodes.Node{
UUID: host.Status.Provisioning.ID,
})
ironic.Start()
defer ironic.Stop()

auth := clients.AuthConfig{Type: clients.NoAuth}
prov, err := newProvisionerWithSettings(host, bmc.Credentials{}, nil, ironic.Endpoint(), auth)
if err != nil {
t.Fatalf("could not create provisioner: %s", err)
}

prov.TryInit()
result, _, err := prov.Register(provisioner.ManagementAccessData{DisablePowerOff: true}, false, false)
if err != nil {
t.Fatalf("error from Register: %s", err)
}
assert.Equal(t, "current ironic version does not support DisablePowerOff, refusing to manage node", result.ErrorMessage)
}
11 changes: 9 additions & 2 deletions pkg/provisioner/ironic/testserver/ironic.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type IronicMock struct {
// NewIronic builds an ironic mock server.
func NewIronic(t *testing.T) *IronicMock {
t.Helper()
server := New(t, "ironic").AddDefaultResponse("/v1/?", "", http.StatusOK, versionedRootResult)
server := New(t, "ironic").AddDefaultResponse("/v1/?", "", http.StatusOK, fmt.Sprintf(versionedRootResult, "1.95"))
return &IronicMock{
MockServer: server,
CreatedNodes: 0,
Expand All @@ -42,10 +42,17 @@ const versionedRootResult = `
"links": [ { "href": "/v1/", "rel": "self" } ],
"status": "CURRENT",
"min_version": "1.1",
"version": "1.87"
"version": "%s"
}
}`

// If your calling this, call it before you add any other Responses.
func (m *IronicMock) WithVersion(v string) *IronicMock {
m.ClearDatabase()
m.AddDefaultResponse("/v1/?", "", http.StatusOK, fmt.Sprintf(versionedRootResult, v))
return m
}

// WithDefaultResponses sets a valid answer for all the API calls.
func (m *IronicMock) WithDefaultResponses() *IronicMock {
m.AddDefaultResponseJSON("/v1/nodes/{id}", "", http.StatusOK, nodes.Node{
Expand Down
1 change: 1 addition & 0 deletions pkg/provisioner/provisioner.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ type ManagementAccessData struct {
PreprovisioningImage *PreprovisioningImage
PreprovisioningNetworkData string
HasCustomDeploy bool
DisablePowerOff bool
}

type AdoptData struct {
Expand Down
Loading