Repository: metal3-io/baremetal-operator
Pull request #761 (2021-01-12 20:53:29)

Improve communications between controller and provisioner
Use semantic functions (`retryAfterDelay()`, `operationContinuing()`, `operationComplete()`, `operationFailed()`, `transientError()`) to generate return values from ironic provisioner. This is a first step toward changing the `provisioner.Result` type to include richer information (e.g. to be able to distinguish between a request to retry after a delay and the operation simply continuing).

Stop writing to the BareMetalHost Status subresource in the provisioner, and require all changes to be communicated back to the controller via return values from the Provisioner interface.

Only update the BareMetalHost Status subresource when something has changed, instead of on ~every reconcile.

metal3-io-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/metal3-io/baremetal-operator/pull/761#" title="Author self-approved">zaneb</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/master/OWNERS)~~ [zaneb]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
zaneb: /test-integration
zaneb: /test-integration
zaneb: /cc @dhellmann 

Patch:
diff --git a/OWNERS b/OWNERS
index bb53191f..6f97925b 100644
--- a/OWNERS
+++ b/OWNERS
@@ -9,7 +9,6 @@ approvers:
  - zaneb
 
 reviewers:
- - andfasano
  - honza
  - dukov
  - kashifest
diff --git a/controllers/metal3.io/action_result.go b/controllers/metal3.io/action_result.go
index 5f3a9717..a746f05c 100644
--- a/controllers/metal3.io/action_result.go
+++ b/controllers/metal3.io/action_result.go
@@ -25,17 +25,6 @@ type actionResult interface {
 	Dirty() bool
 }
 
-// actionContinueNoWrite is a result indicating that the current action is still
-// in progress, and that the resource should remain in the same provisioning
-// state without writing the status
-type actionContinueNoWrite struct {
-	actionContinue
-}
-
-func (r actionContinueNoWrite) Dirty() bool {
-	return false
-}
-
 // actionContinue is a result indicating that the current action is still
 // in progress, and that the resource should remain in the same provisioning
 // state.
@@ -51,6 +40,17 @@ func (r actionContinue) Result() (result reconcile.Result, err error) {
 }
 
 func (r actionContinue) Dirty() bool {
+	return false
+}
+
+// actionUpdate is a result indicating that the current action is still
+// in progress, and that the resource should remain in the same provisioning
+// state but write new Status data.
+type actionUpdate struct {
+	actionContinue
+}
+
+func (r actionUpdate) Dirty() bool {
 	return true
 }
 
diff --git a/controllers/metal3.io/baremetalhost_controller.go b/controllers/metal3.io/baremetalhost_controller.go
index b15a9ba9..502116e2 100644
--- a/controllers/metal3.io/baremetalhost_controller.go
+++ b/controllers/metal3.io/baremetalhost_controller.go
@@ -208,7 +208,7 @@ func (r *BareMetalHostReconciler) Reconcile(request ctrl.Request) (result ctrl.R
 		request:        request,
 		bmcCredsSecret: bmcCredsSecret,
 	}
-	prov, err := r.ProvisionerFactory(host, *bmcCreds, info.publishEvent)
+	prov, err := r.ProvisionerFactory(*host, *bmcCreds, info.publishEvent)
 	if err != nil {
 		return ctrl.Result{}, errors.Wrap(err, "failed to create provisioner")
 	}
@@ -419,53 +419,71 @@ func (r *BareMetalHostReconciler) actionUnmanaged(prov provisioner.Provisioner,
 	if info.host.HasBMCDetails() {
 		return actionComplete{}
 	}
-	return actionContinueNoWrite{actionContinue{unmanagedRetryDelay}}
+	return actionContinue{unmanagedRetryDelay}
 }
 
 // Test the credentials by connecting to the management controller.
-func (r *BareMetalHostReconciler) actionRegistering(prov provisioner.Provisioner, info *reconcileInfo) actionResult {
+func (r *BareMetalHostReconciler) registerHost(prov provisioner.Provisioner, info *reconcileInfo) actionResult {
 	info.log.Info("registering and validating access to management controller",
 		"credentials", info.host.Status.TriedCredentials)
+	dirty := false
 
 	credsChanged := !info.host.Status.TriedCredentials.Match(*info.bmcCredsSecret)
 	if credsChanged {
 		info.log.Info("new credentials")
 		info.host.UpdateTriedCredentials(*info.bmcCredsSecret)
 		info.postSaveCallbacks = append(info.postSaveCallbacks, updatedCredentials.Inc)
+		dirty = true
 	}
 
-	provResult, err := prov.ValidateManagementAccess(credsChanged, info.host.Status.ErrorType == metal3v1alpha1.RegistrationError)
+	provResult, provID, err := prov.ValidateManagementAccess(credsChanged, info.host.Status.ErrorType == metal3v1alpha1.RegistrationError)
 	if err != nil {
 		noManagementAccess.Inc()
 		return actionError{errors.Wrap(err, "failed to validate BMC access")}
 	}
 
-	info.log.Info("response from validate", "provResult", provResult)
-
 	if provResult.ErrorMessage != "" {
 		return recordActionFailure(info, metal3v1alpha1.RegistrationError, provResult.ErrorMessage)
 	}
 
+	if provID != "" && info.host.Status.Provisioning.ID != provID {
+		info.log.Info("setting provisioning id", "ID", provID)
+		info.host.Status.Provisioning.ID = provID
+		dirty = true
+	}
+
 	if provResult.Dirty {
 		info.log.Info("host not ready", "wait", provResult.RequeueAfter)
-		clearError(info.host)
-		return actionContinue{provResult.RequeueAfter}
+		result := actionContinue{provResult.RequeueAfter}
+		if clearError(info.host) {
+			dirty = true
+		}
+		if dirty {
+			return actionUpdate{result}
+		}
+		return result
 	}
 
 	// Reaching this point means the credentials are valid and worked,
 	// so clear any previous error and record the success in the
 	// status block.
-	info.log.Info("updating credentials success status fields")
-	registeredNewCreds := !info.host.Status.GoodCredentials.Match(*info.bmcCredsSecret)
-	info.host.UpdateGoodCredentials(*info.bmcCredsSecret)
-	info.log.Info("clearing previous error message")
-	clearError(info.host)
-
-	if registeredNewCreds {
+	if !info.host.Status.GoodCredentials.Match(*info.bmcCredsSecret) {
+		info.log.Info("updating credentials success status fields")
+		info.host.UpdateGoodCredentials(*info.bmcCredsSecret)
 		info.publishEvent("BMCAccessValidated", "Verified access to BMC")
+		dirty = true
+	} else {
+		info.log.Info("verified access to the BMC")
+	}
+	if clearError(info.host) {
+		info.log.Info("clearing previous error message")
+		dirty = true
 	}
 
-	return actionComplete{}
+	if dirty {
+		return actionComplete{}
+	}
+	return nil
 }
 
 // Ensure we have the information about the hardware on the host.
@@ -481,12 +499,15 @@ func (r *BareMetalHostReconciler) actionInspecting(prov provisioner.Provisioner,
 		return recordActionFailure(info, metal3v1alpha1.InspectionError, provResult.ErrorMessage)
 	}
 
-	clearError(info.host)
-
 	if provResult.Dirty || details == nil {
-		return actionContinue{provResult.RequeueAfter}
+		result := actionContinue{provResult.RequeueAfter}
+		if clearError(info.host) {
+			return actionUpdate{result}
+		}
+		return result
 	}
 
+	clearError(info.host)
 	info.host.Status.HardwareDetails = details
 	return actionComplete{}
 }
@@ -548,7 +569,7 @@ func (r *BareMetalHostReconciler) actionProvisioning(prov provisioner.Provisione
 		if err := r.Update(context.TODO(), info.host); err != nil {
 			return actionError{errors.Wrap(err, "failed to remove reboot annotations from host")}
 		}
-		return actionContinueNoWrite{}
+		return actionContinue{}
 	}
 
 	provResult, err := prov.Provision(hostConf)
@@ -565,8 +586,11 @@ func (r *BareMetalHostReconciler) actionProvisioning(prov provisioner.Provisione
 		// Go back into the queue and wait for the Provision() method
 		// to return false, indicating that it has no more work to
 		// do.
-		clearError(info.host)
-		return actionContinue{provResult.RequeueAfter}
+		result := actionContinue{provResult.RequeueAfter}
+		if clearError(info.host) {
+			return actionUpdate{result}
+		}
+		return result
 	}
 
 	// If the provisioner had no work, ensure the image settings match.
@@ -598,8 +622,11 @@ func (r *BareMetalHostReconciler) actionDeprovisioning(prov provisioner.Provisio
 		return recordActionFailure(info, metal3v1alpha1.RegistrationError, provResult.ErrorMessage)
 	}
 	if provResult.Dirty {
-		clearError(info.host)
-		return actionContinue{provResult.RequeueAfter}
+		result := actionContinue{provResult.RequeueAfter}
+		if clearError(info.host) {
+			return actionUpdate{result}
+		}
+		return result
 	}
 
 	info.log.Info("deprovisioning")
@@ -614,15 +641,18 @@ func (r *BareMetalHostReconciler) actionDeprovisioning(prov provisioner.Provisio
 	}
 
 	if provResult.Dirty {
-		clearError(info.host)
-		return actionContinue{provResult.RequeueAfter}
+		result := actionContinue{provResult.RequeueAfter}
+		if clearError(info.host) {
+			return actionUpdate{result}
+		}
+		return result
 	}
 
 	if clearRebootAnnotations(info.host) {
 		if err = r.Update(context.TODO(), info.host); err != nil {
 			return actionError{errors.Wrap(err, "failed to remove reboot annotations from host")}
 		}
-		return actionContinueNoWrite{}
+		return actionContinue{}
 	}
 
 	// After the provisioner is done, clear the provisioning settings
@@ -638,18 +668,16 @@ func (r *BareMetalHostReconciler) manageHostPower(prov provisioner.Provisioner,
 	var provResult provisioner.Result
 
 	// Check the current status and save it before trying to update it.
-	provResult, err := prov.UpdateHardwareState()
+	hwState, err := prov.UpdateHardwareState()
 	if err != nil {
 		return actionError{errors.Wrap(err, "failed to update the host power status")}
 	}
 
-	if provResult.ErrorMessage != "" {
-		return recordActionFailure(info, metal3v1alpha1.PowerManagementError, provResult.ErrorMessage)
-	}
-
-	if provResult.Dirty {
+	if hwState.PoweredOn != nil && *hwState.PoweredOn != info.host.Status.PoweredOn {
+		info.log.Info("updating power status", "discovered", *hwState.PoweredOn)
+		info.host.Status.PoweredOn = *hwState.PoweredOn
 		clearError(info.host)
-		return actionContinue{provResult.RequeueAfter}
+		return actionUpdate{}
 	}
 
 	desiredPowerOnState := info.host.Spec.Online
@@ -662,7 +690,7 @@ func (r *BareMetalHostReconciler) manageHostPower(prov provisioner.Provisioner,
 				return actionError{errors.Wrap(err, "failed to remove reboot annotation from host")}
 			}
 
-			return actionContinueNoWrite{}
+			return actionContinue{}
 		}
 	}
 
@@ -677,7 +705,7 @@ func (r *BareMetalHostReconciler) manageHostPower(prov provisioner.Provisioner,
 	// a delay.
 	steadyStateResult := actionContinue{time.Second * 60}
 	if info.host.Status.PoweredOn == desiredPowerOnState {
-		return actionContinueNoWrite{steadyStateResult}
+		return steadyStateResult
 	}
 
 	info.log.Info("power state change needed",
@@ -708,8 +736,11 @@ func (r *BareMetalHostReconciler) manageHostPower(prov provisioner.Provisioner,
 			}
 			powerChangeAttempts.With(metricLabels).Inc()
 		})
-		clearError(info.host)
-		return actionContinue{provResult.RequeueAfter}
+		result := actionContinue{provResult.RequeueAfter}
+		if clearError(info.host) {
+			return actionUpdate{result}
+		}
+		return result
 	}
 
 	// The provisioner did not have to do anything to change the power
@@ -717,7 +748,7 @@ func (r *BareMetalHostReconciler) manageHostPower(prov provisioner.Provisioner,
 	// host status field.
 	info.host.Status.PoweredOn = info.host.Spec.Online
 	info.host.Status.ErrorCount = 0
-	return steadyStateResult
+	return actionUpdate{steadyStateResult}
 }
 
 // A host reaching this action handler should be provisioned or externally
@@ -733,8 +764,11 @@ func (r *BareMetalHostReconciler) actionManageSteadyState(prov provisioner.Provi
 		return recordActionFailure(info, metal3v1alpha1.RegistrationError, provResult.ErrorMessage)
 	}
 	if provResult.Dirty {
-		clearError(info.host)
-		return actionContinue{provResult.RequeueAfter}
+		result := actionContinue{provResult.RequeueAfter}
+		if clearError(info.host) {
+			return actionUpdate{result}
+		}
+		return result
 	}
 
 	return r.manageHostPower(prov, info)
diff --git a/controllers/metal3.io/host_state_machine.go b/controllers/metal3.io/host_state_machine.go
index 449782ce..45ba5bb0 100644
--- a/controllers/metal3.io/host_state_machine.go
+++ b/controllers/metal3.io/host_state_machine.go
@@ -63,7 +63,7 @@ func recordStateBegin(host *metal3v1alpha1.BareMetalHost, state metal3v1alpha1.P
 	}
 }
 
-func recordStateEnd(info *reconcileInfo, host *metal3v1alpha1.BareMetalHost, state metal3v1alpha1.ProvisioningState, time metav1.Time) {
+func recordStateEnd(info *reconcileInfo, host *metal3v1alpha1.BareMetalHost, state metal3v1alpha1.ProvisioningState, time metav1.Time) (changed bool) {
 	if prevMetric := host.OperationMetricForState(state); prevMetric != nil {
 		if !prevMetric.Start.IsZero() && prevMetric.End.IsZero() {
 			prevMetric.End = time
@@ -71,8 +71,10 @@ func recordStateEnd(info *reconcileInfo, host *metal3v1alpha1.BareMetalHost, sta
 				observer := stateTime[state].With(hostMetricLabels(info.request))
 				observer.Observe(prevMetric.Duration().Seconds())
 			})
+			changed = true
 		}
 	}
+	return
 }
 
 func (hsm *hostStateMachine) updateHostStateFrom(initialState metal3v1alpha1.ProvisioningState,
@@ -170,8 +172,6 @@ func (hsm *hostStateMachine) ensureRegistered(info *reconcileInfo) (result actio
 		return
 	}
 
-	needsReregister := false
-
 	switch hsm.NextState {
 	case metal3v1alpha1.StateNone, metal3v1alpha1.StateUnmanaged:
 		// We haven't yet reached the Registration state, so don't attempt
@@ -182,30 +182,24 @@ func (hsm *hostStateMachine) ensureRegistered(info *reconcileInfo) (result actio
 		return
 	case metal3v1alpha1.StateRegistering:
 	default:
-		needsReregister = (hsm.Host.Status.ErrorType == metal3v1alpha1.RegistrationError ||
-			!hsm.Host.Status.GoodCredentials.Match(*info.bmcCredsSecret))
-		if needsReregister {
+		if hsm.Host.Status.ErrorType == metal3v1alpha1.RegistrationError ||
+			!hsm.Host.Status.GoodCredentials.Match(*info.bmcCredsSecret) {
 			info.log.Info("Retrying registration")
 			recordStateBegin(hsm.Host, metal3v1alpha1.StateRegistering, metav1.Now())
 		}
 	}
 
-	result = hsm.Reconciler.actionRegistering(hsm.Provisioner, info)
-	if _, complete := result.(actionComplete); complete {
-		if hsm.NextState != metal3v1alpha1.StateRegistering {
-			recordStateEnd(info, hsm.Host, metal3v1alpha1.StateRegistering, metav1.Now())
-		}
-		if needsReregister {
-			// Host was re-registered, so requeue and run the state machine on
-			// the next reconcile
-			result = actionContinue{}
-		} else {
-			// Allow the state machine to run, either because we were just
-			// reconfirming an existing registration, or because we are in the
-			// Registering state
-			result = nil
+	result = hsm.Reconciler.registerHost(hsm.Provisioner, info)
+	_, complete := result.(actionComplete)
+	if (result == nil || complete) &&
+		hsm.NextState != metal3v1alpha1.StateRegistering {
+		if recordStateEnd(info, hsm.Host, metal3v1alpha1.StateRegistering, metav1.Now()) {
+			result = actionUpdate{}
 		}
 	}
+	if complete {
+		result = actionUpdate{}
+	}
 	return
 }
 
diff --git a/main.go b/main.go
index 1a8d477d..e5283838 100644
--- a/main.go
+++ b/main.go
@@ -118,21 +118,24 @@ func main() {
 		os.Exit(1)
 	}
 
-	provisionerFactory := func(host *metal3iov1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publish provisioner.EventPublisher) (provisioner.Provisioner, error) {
+	provisionerFactory := func(host metal3iov1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publish provisioner.EventPublisher) (provisioner.Provisioner, error) {
 		isUnmanaged := host.Spec.ExternallyProvisioned && !host.HasBMCDetails()
 
+		hostCopy := host.DeepCopy()
+
 		if runInTestMode {
 			ctrl.Log.Info("using test provisioner")
-			return fixture.New(host, bmcCreds, publish)
+			fix := fixture.Fixture{}
+			return fix.New(*hostCopy, bmcCreds, publish)
 		} else if runInDemoMode {
 			ctrl.Log.Info("using demo provisioner")
-			return demo.New(host, bmcCreds, publish)
+			return demo.New(*hostCopy, bmcCreds, publish)
 		} else if isUnmanaged {
 			ctrl.Log.Info("using empty provisioner")
-			return empty.New(host, bmcCreds, publish)
+			return empty.New(*hostCopy, bmcCreds, publish)
 		}
 		ironic.LogStartup()
-		return ironic.New(host, bmcCreds, publish)
+		return ironic.New(*hostCopy, bmcCreds, publish)
 	}
 
 	if err = (&metal3iocontroller.BareMetalHostReconciler{
diff --git a/pkg/provisioner/demo/demo.go b/pkg/provisioner/demo/demo.go
index 9fc1bc81..606bb99a 100644
--- a/pkg/provisioner/demo/demo.go
+++ b/pkg/provisioner/demo/demo.go
@@ -46,7 +46,7 @@ const (
 // and uses Ironic to manage the host.
 type demoProvisioner struct {
 	// the host to be managed by this provisioner
-	host *metal3v1alpha1.BareMetalHost
+	host metal3v1alpha1.BareMetalHost
 	// the bmc credentials
 	bmcCreds bmc.Credentials
 	// a logger configured for this host
@@ -56,7 +56,7 @@ type demoProvisioner struct {
 }
 
 // New returns a new Ironic Provisioner
-func New(host *metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher) (provisioner.Provisioner, error) {
+func New(host metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher) (provisioner.Provisioner, error) {
 	p := &demoProvisioner{
 		host:      host,
 		bmcCreds:  bmcCreds,
@@ -68,7 +68,7 @@ func New(host *metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher
 
 // ValidateManagementAccess tests the connection information for the
 // host to verify that the location and credentials work.
-func (p *demoProvisioner) ValidateManagementAccess(credentialsChanged, force bool) (result provisioner.Result, err error) {
+func (p *demoProvisioner) ValidateManagementAccess(credentialsChanged, force bool) (result provisioner.Result, provID string, err error) {
 	p.log.Info("testing management access")
 
 	hostName := p.host.ObjectMeta.Name
@@ -88,14 +88,14 @@ func (p *demoProvisioner) ValidateManagementAccess(credentialsChanged, force boo
 
 	default:
 		if p.host.Status.Provisioning.ID == "" {
-			p.host.Status.Provisioning.ID = p.host.ObjectMeta.Name
+			provID = p.host.ObjectMeta.Name
 			p.log.Info("setting provisioning id",
 				"provisioningID", p.host.Status.Provisioning.ID)
 			result.Dirty = true
 		}
 	}
 
-	return result, nil
+	return
 }
 
 // InspectHardware updates the HardwareDetails field of the host with
@@ -174,12 +174,10 @@ func (p *demoProvisioner) InspectHardware(force bool) (result provisioner.Result
 // UpdateHardwareState fetches the latest hardware state of the server
 // and updates the HardwareDetails field of the host with details. It
 // is expected to do this in the least expensive way possible, such as
-// reading from a cache, and return dirty only if any state
-// information has changed.
-func (p *demoProvisioner) UpdateHardwareState() (result provisioner.Result, err error) {
+// reading from a cache.
+func (p *demoProvisioner) UpdateHardwareState() (hwState provisioner.HardwareState, err error) {
 	p.log.Info("updating hardware state")
-	result.Dirty = false
-	return result, nil
+	return
 }
 
 // Adopt allows an externally-provisioned server to be adopted.
diff --git a/pkg/provisioner/empty/empty.go b/pkg/provisioner/empty/empty.go
index a50af7fc..0fbc4880 100644
--- a/pkg/provisioner/empty/empty.go
+++ b/pkg/provisioner/empty/empty.go
@@ -15,14 +15,14 @@ type emptyProvisioner struct {
 }
 
 // New returns a new Empty Provisioner
-func New(host *metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher) (provisioner.Provisioner, error) {
+func New(host metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher) (provisioner.Provisioner, error) {
 	return &emptyProvisioner{}, nil
 }
 
 // ValidateManagementAccess tests the connection information for the
 // host to verify that the location and credentials work.
-func (p *emptyProvisioner) ValidateManagementAccess(credentialsChanged, force bool) (provisioner.Result, error) {
-	return provisioner.Result{}, nil
+func (p *emptyProvisioner) ValidateManagementAccess(credentialsChanged, force bool) (provisioner.Result, string, error) {
+	return provisioner.Result{}, "", nil
 }
 
 // InspectHardware updates the HardwareDetails field of the host with
@@ -36,10 +36,9 @@ func (p *emptyProvisioner) InspectHardware(force bool) (provisioner.Result, *met
 // UpdateHardwareState fetches the latest hardware state of the server
 // and updates the HardwareDetails field of the host with details. It
 // is expected to do this in the least expensive way possible, such as
-// reading from a cache, and return dirty only if any state
-// information has changed.
-func (p *emptyProvisioner) UpdateHardwareState() (provisioner.Result, error) {
-	return provisioner.Result{}, nil
+// reading from a cache.
+func (p *emptyProvisioner) UpdateHardwareState() (provisioner.HardwareState, error) {
+	return provisioner.HardwareState{}, nil
 }
 
 // Adopt allows an externally-provisioned server to be adopted.
diff --git a/pkg/provisioner/fixture/fixture.go b/pkg/provisioner/fixture/fixture.go
index dcb365b3..13626438 100644
--- a/pkg/provisioner/fixture/fixture.go
+++ b/pkg/provisioner/fixture/fixture.go
@@ -45,53 +45,57 @@ func (cd *fixtureHostConfigData) MetaData() (string, error) {
 // and uses Ironic to manage the host.
 type fixtureProvisioner struct {
 	// the host to be managed by this provisioner
-	host *metal3v1alpha1.BareMetalHost
+	host metal3v1alpha1.BareMetalHost
 	// the bmc credentials
 	bmcCreds bmc.Credentials
 	// a logger configured for this host
 	log logr.Logger
 	// an event publisher for recording significant events
 	publisher provisioner.EventPublisher
+	// state storage for the Host
+	state *Fixture
+}
+
+type Fixture struct {
+	// counter to set the provisioner as ready
+	BecomeReadyCounter int
+	// state to manage deletion
+	Deleted bool
 	// state to manage the two-step adopt process
 	adopted bool
-	// counter to set the provisioner as ready
-	becomeReadyCounter int
+	// state to manage provisioning
+	image metal3v1alpha1.Image
+	// state to manage power
+	poweredOn bool
 }
 
 // New returns a new Ironic FixtureProvisioner
-func New(host *metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher) (provisioner.Provisioner, error) {
-	return NewMock(host, bmcCreds, publisher, 0)
-}
-
-// NewMock is used in tests to build a fixture provisioner and inject additional test parameters
-func NewMock(host *metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher, becomeReadyCounter int) (provisioner.Provisioner, error) {
+func (f *Fixture) New(host metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher) (provisioner.Provisioner, error) {
 	p := &fixtureProvisioner{
-		host:               host,
-		bmcCreds:           bmcCreds,
-		log:                log.WithValues("host", host.Name),
-		publisher:          publisher,
-		becomeReadyCounter: becomeReadyCounter,
+		host:      *host.DeepCopy(),
+		bmcCreds:  bmcCreds,
+		log:       log.WithValues("host", host.Name),
+		publisher: publisher,
+		state:     f,
 	}
 	return p, nil
 }
 
 // ValidateManagementAccess tests the connection information for the
 // host to verify that the location and credentials work.
-func (p *fixtureProvisioner) ValidateManagementAccess(credentialsChanged, force bool) (result provisioner.Result, err error) {
+func (p *fixtureProvisioner) ValidateManagementAccess(credentialsChanged, force bool) (result provisioner.Result, provID string, err error) {
 	p.log.Info("testing management access")
 
 	// Fill in the ID of the host in the provisioning system
 	if p.host.Status.Provisioning.ID == "" {
-		p.host.Status.Provisioning.ID = "temporary-fake-id"
-		p.log.Info("setting provisioning id",
-			"provisioningID", p.host.Status.Provisioning.ID)
+		provID = "temporary-fake-id"
 		result.Dirty = true
 		result.RequeueAfter = time.Second * 5
 		p.publisher("Registered", "Registered new host")
-		return result, nil
+		return
 	}
 
-	return result, nil
+	return
 }
 
 // InspectHardware updates the HardwareDetails field of the host with
@@ -159,21 +163,20 @@ func (p *fixtureProvisioner) InspectHardware(force bool) (result provisioner.Res
 // UpdateHardwareState fetches the latest hardware state of the server
 // and updates the HardwareDetails field of the host with details. It
 // is expected to do this in the least expensive way possible, such as
-// reading from a cache, and return dirty only if any state
-// information has changed.
-func (p *fixtureProvisioner) UpdateHardwareState() (result provisioner.Result, err error) {
+// reading from a cache.
+func (p *fixtureProvisioner) UpdateHardwareState() (hwState provisioner.HardwareState, err error) {
 	if !p.host.NeedsProvisioning() {
+		hwState.PoweredOn = &p.state.poweredOn
 		p.log.Info("updating hardware state")
-		result.Dirty = false
 	}
-	return result, nil
+	return
 }
 
 // Adopt allows an externally-provisioned server to be adopted.
 func (p *fixtureProvisioner) Adopt(force bool) (result provisioner.Result, err error) {
 	p.log.Info("adopting host")
-	if p.host.Spec.ExternallyProvisioned && !p.adopted {
-		p.adopted = true
+	if p.host.Spec.ExternallyProvisioned && !p.state.adopted {
+		p.state.adopted = true
 		result.Dirty = true
 		result.RequeueAfter = provisionRequeueDelay
 	}
@@ -187,10 +190,10 @@ func (p *fixtureProvisioner) Provision(hostConf provisioner.HostConfigData) (res
 	p.log.Info("provisioning image to host",
 		"state", p.host.Status.Provisioning.State)
 
-	if p.host.Status.Provisioning.Image.URL == "" {
+	if p.state.image.URL == "" {
 		p.publisher("ProvisioningComplete", "Image provisioning completed")
 		p.log.Info("moving to done")
-		p.host.Status.Provisioning.Image = *p.host.Spec.Image
+		p.state.image = *p.host.Spec.Image
 		result.Dirty = true
 		result.RequeueAfter = provisionRequeueDelay
 	}
@@ -211,10 +214,10 @@ func (p *fixtureProvisioner) Deprovision(force bool) (result provisioner.Result,
 	// necessary once we really have Fixture doing the deprovisioning
 	// and we can monitor it's status.
 
-	if p.host.Status.HardwareDetails != nil {
+	if p.state.image.URL != "" {
 		p.publisher("DeprovisionStarted", "Image deprovisioning started")
 		p.log.Info("clearing hardware details")
-		p.host.Status.HardwareDetails = nil
+		p.state.image = metal3v1alpha1.Image{}
 		result.Dirty = true
 		return result, nil
 	}
@@ -229,9 +232,9 @@ func (p *fixtureProvisioner) Deprovision(force bool) (result provisioner.Result,
 func (p *fixtureProvisioner) Delete() (result provisioner.Result, err error) {
 	p.log.Info("deleting host")
 
-	if p.host.Status.Provisioning.ID != "" {
+	if !p.state.Deleted {
 		p.log.Info("clearing provisioning id")
-		p.host.Status.Provisioning.ID = ""
+		p.state.Deleted = true
 		result.Dirty = true
 		return result, nil
 	}
@@ -244,10 +247,10 @@ func (p *fixtureProvisioner) Delete() (result provisioner.Result, err error) {
 func (p *fixtureProvisioner) PowerOn() (result provisioner.Result, err error) {
 	p.log.Info("ensuring host is powered on")
 
-	if !p.host.Status.PoweredOn {
+	if !p.state.poweredOn {
 		p.publisher("PowerOn", "Host powered on")
 		p.log.Info("changing status")
-		p.host.Status.PoweredOn = true
+		p.state.poweredOn = true
 		result.Dirty = true
 		return result, nil
 	}
@@ -260,10 +263,10 @@ func (p *fixtureProvisioner) PowerOn() (result provisioner.Result, err error) {
 func (p *fixtureProvisioner) PowerOff() (result provisioner.Result, err error) {
 	p.log.Info("ensuring host is powered off")
 
-	if p.host.Status.PoweredOn {
+	if p.state.poweredOn {
 		p.publisher("PowerOff", "Host powered off")
 		p.log.Info("changing status")
-		p.host.Status.PoweredOn = false
+		p.state.poweredOn = false
 		result.Dirty = true
 		return result, nil
 	}
@@ -275,9 +278,9 @@ func (p *fixtureProvisioner) PowerOff() (result provisioner.Result, err error) {
 func (p *fixtureProvisioner) IsReady() (result bool, err error) {
 	p.log.Info("checking provisioner status")
 
-	if p.becomeReadyCounter > 0 {
-		p.becomeReadyCounter--
+	if p.state.BecomeReadyCounter > 0 {
+		p.state.BecomeReadyCounter--
 	}
 
-	return p.becomeReadyCounter == 0, nil
+	return p.state.BecomeReadyCounter == 0, nil
 }
diff --git a/pkg/provisioner/ironic/ironic.go b/pkg/provisioner/ironic/ironic.go
index efca3c4b..8dc7c6fe 100644
--- a/pkg/provisioner/ironic/ironic.go
+++ b/pkg/provisioner/ironic/ironic.go
@@ -105,7 +105,7 @@ func init() {
 // and uses Ironic to manage the host.
 type ironicProvisioner struct {
 	// the host to be managed by this provisioner
-	host *metal3v1alpha1.BareMetalHost
+	host metal3v1alpha1.BareMetalHost
 	// a shorter path to the provisioning status data structure
 	status *metal3v1alpha1.ProvisionStatus
 	// access parameters for the BMC
@@ -137,7 +137,7 @@ func LogStartup() {
 
 // A private function to construct an ironicProvisioner (rather than a
 // Provisioner interface) in a consistent way for tests.
-func newProvisionerWithSettings(host *metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher, ironicURL string, ironicAuthSettings clients.AuthConfig, inspectorURL string, inspectorAuthSettings clients.AuthConfig) (*ironicProvisioner, error) {
+func newProvisionerWithSettings(host metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher, ironicURL string, ironicAuthSettings clients.AuthConfig, inspectorURL string, inspectorAuthSettings clients.AuthConfig) (*ironicProvisioner, error) {
 	tlsConf := clients.TLSConfig{
 		TrustedCAFile:      ironicTrustedCAFile,
 		InsecureSkipVerify: ironicInsecure,
@@ -156,7 +156,7 @@ func newProvisionerWithSettings(host *metal3v1alpha1.BareMetalHost, bmcCreds bmc
 		clientIronic, clientInspector)
 }
 
-func newProvisionerWithIronicClients(host *metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher, clientIronic *gophercloud.ServiceClient, clientInspector *gophercloud.ServiceClient) (*ironicProvisioner, error) {
+func newProvisionerWithIronicClients(host metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher, clientIronic *gophercloud.ServiceClient, clientInspector *gophercloud.ServiceClient) (*ironicProvisioner, error) {
 
 	bmcAccess, err := bmc.NewAccessDetails(host.Spec.BMC.Address, host.Spec.BMC.DisableCertificateVerification)
 	if err != nil {
@@ -182,7 +182,7 @@ func newProvisionerWithIronicClients(host *metal3v1alpha1.BareMetalHost, bmcCred
 
 // New returns a new Ironic Provisioner using the global configuration
 // for finding the Ironic services.
-func New(host *metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher) (provisioner.Provisioner, error) {
+func New(host metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher) (provisioner.Provisioner, error) {
 	var err error
 	if clientIronicSingleton == nil || clientInspectorSingleton == nil {
 		tlsConf := clients.TLSConfig{
@@ -333,14 +333,15 @@ func (p *ironicProvisioner) findExistingHost() (ironicNode *nodes.Node, err erro
 //
 // FIXME(dhellmann): We should rename this method to describe what it
 // actually does.
-func (p *ironicProvisioner) ValidateManagementAccess(credentialsChanged, force bool) (result provisioner.Result, err error) {
+func (p *ironicProvisioner) ValidateManagementAccess(credentialsChanged, force bool) (result provisioner.Result, provID string, err error) {
 	var ironicNode *nodes.Node
 
 	p.log.Info("validating management access")
 
 	ironicNode, err = p.findExistingHost()
 	if err != nil {
-		return result, errors.Wrap(err, "failed to find existing host")
+		result, err = transientError(errors.Wrap(err, "failed to find existing host"))
+		return
 	}
 
 	// Some BMC types require a MAC address, so ensure we have one
@@ -348,8 +349,8 @@ func (p *ironicProvisioner) ValidateManagementAccess(credentialsChanged, force b
 	if p.bmcAccess.NeedsMAC() && p.host.Spec.BootMACAddress == "" {
 		msg := fmt.Sprintf("BMC driver %s requires a BootMACAddress value", p.bmcAccess.Type())
 		p.log.Info(msg)
-		result.ErrorMessage = msg
-		return result, nil
+		result, err = operationFailed(msg)
+		return
 	}
 
 	driverInfo := p.bmcAccess.DriverInfo(p.bmcCreds)
@@ -358,6 +359,8 @@ func (p *ironicProvisioner) ValidateManagementAccess(credentialsChanged, force b
 	driverInfo["deploy_kernel"] = deployKernelURL
 	driverInfo["deploy_ramdisk"] = deployRamdiskURL
 
+	result, err = operationComplete()
+
 	// If we have not found a node yet, we need to create one
 	if ironicNode == nil {
 		p.log.Info("registering host in ironic")
@@ -380,15 +383,14 @@ func (p *ironicProvisioner) ValidateManagementAccess(credentialsChanged, force b
 			}).Extract()
 		// FIXME(dhellmann): Handle 409 and 503? errors here.
 		if err != nil {
-			return result, errors.Wrap(err, "failed to register host in ironic")
+			result, err = transientError(errors.Wrap(err, "failed to register host in ironic"))
+			return
 		}
 		p.publisher("Registered", "Registered new host")
 
 		// Store the ID so other methods can assume it is set and so
 		// we can find the node again later.
-		p.status.ID = ironicNode.UUID
-		result.Dirty = true
-		p.log.Info("setting provisioning id", "ID", p.status.ID)
+		provID = ironicNode.UUID
 
 		// If we know the MAC, create a port. Otherwise we will have
 		// to do this after we run the introspection step.
@@ -396,7 +398,7 @@ func (p *ironicProvisioner) ValidateManagementAccess(credentialsChanged, force b
 			enable := true
 			p.log.Info("creating port for node in ironic", "MAC",
 				p.host.Spec.BootMACAddress)
-			_, err := ports.Create(
+			_, err = ports.Create(
 				p.client,
 				ports.CreateOpts{
 					NodeUUID:   ironicNode.UUID,
@@ -404,7 +406,8 @@ func (p *ironicProvisioner) ValidateManagementAccess(credentialsChanged, force b
 					PXEEnabled: &enable,
 				}).Extract()
 			if err != nil {
-				return result, errors.Wrap(err, "failed to create port in ironic")
+				result, err = transientError(errors.Wrap(err, "failed to create port in ironic"))
+				return
 			}
 		}
 
@@ -421,9 +424,10 @@ func (p *ironicProvisioner) ValidateManagementAccess(credentialsChanged, force b
 		}
 
 		if imageData != nil {
-			updates, err := p.getImageUpdateOptsForNode(ironicNode, imageData)
-			if err != nil {
-				return result, errors.Wrap(err, "Could not get Image options for node")
+			updates, optsErr := p.getImageUpdateOptsForNode(ironicNode, imageData)
+			if optsErr != nil {
+				result, err = transientError(errors.Wrap(optsErr, "Could not get Image options for node"))
+				return
 			}
 			if len(updates) != 0 {
 				_, err = nodes.Update(p.client, ironicNode.UUID, updates).Extract()
@@ -431,11 +435,11 @@ func (p *ironicProvisioner) ValidateManagementAccess(credentialsChanged, force b
 				case nil:
 				case gophercloud.ErrDefault409:
 					p.log.Info("could not update host settings in ironic, busy")
-					result.Dirty = true
-					result.RequeueAfter = provisionRequeueDelay
-					return result, nil
+					result, err = retryAfterDelay(provisionRequeueDelay)
+					return
 				default:
-					return result, errors.Wrap(err, "failed to update host settings in ironic")
+					result, err = transientError(errors.Wrap(err, "failed to update host settings in ironic"))
+					return
 				}
 			}
 		}
@@ -444,13 +448,7 @@ func (p *ironicProvisioner) ValidateManagementAccess(credentialsChanged, force b
 		// node in ironic by looking it up. We need to check its
 		// settings against what we have in the host, and change them
 		// if there are differences.
-		if p.status.ID != ironicNode.UUID {
-			// Store the ID so other methods can assume it is set and
-			// so we can find the node using that value next time.
-			p.status.ID = ironicNode.UUID
-			result.Dirty = true
-			p.log.Info("setting provisioning id", "ID", p.status.ID)
-		}
+		provID = ironicNode.UUID
 
 		if ironicNode.Name == "" {
 			updates := nodes.UpdateOpts{
@@ -465,11 +463,11 @@ func (p *ironicProvisioner) ValidateManagementAccess(credentialsChanged, force b
 			case nil:
 			case gophercloud.ErrDefault409:
 				p.log.Info("could not update ironic node name, busy")
-				result.Dirty = true
-				result.RequeueAfter = provisionRequeueDelay
-				return result, nil
+				result, err = retryAfterDelay(provisionRequeueDelay)
+				return
 			default:
-				return result, errors.Wrap(err, "failed to update ironc node name")
+				result, err = transientError(errors.Wrap(err, "failed to update ironc node name"))
+				return
 			}
 			p.log.Info("updated ironic node name")
 
@@ -490,11 +488,11 @@ func (p *ironicProvisioner) ValidateManagementAccess(credentialsChanged, force b
 			case nil:
 			case gophercloud.ErrDefault409:
 				p.log.Info("could not update host driver settings, busy")
-				result.Dirty = true
-				result.RequeueAfter = provisionRequeueDelay
-				return result, nil
+				result, err = retryAfterDelay(provisionRequeueDelay)
+				return
 			default:
-				return result, errors.Wrap(err, "failed to update host driver settings")
+				result, err = transientError(errors.Wrap(err, "failed to update host driver settings"))
+				return
 			}
 			p.log.Info("updated host driver settings")
 			// We don't return here because we also have to set the
@@ -521,48 +519,47 @@ func (p *ironicProvisioner) ValidateManagementAccess(credentialsChanged, force b
 
 		// If ironic is reporting an error, stop working on the node.
 		if ironicNode.LastError != "" && !(credentialsChanged || force) {
-			result.ErrorMessage = ironicNode.LastError
-			return result, nil
+			result, err = operationFailed(ironicNode.LastError)
+			return
 		}
 
 		if ironicNode.TargetProvisionState == string(nodes.TargetManage) {
 			// We have already tried to manage the node and did not
 			// get an error, so do nothing and keep trying.
-			result.Dirty = true
-			result.RequeueAfter = provisionRequeueDelay
-			return result, nil
+			result, err = operationContinuing(provisionRequeueDelay)
+			return
 		}
 
-		return p.changeNodeProvisionState(
+		result, err = p.changeNodeProvisionState(
 			ironicNode,
 			nodes.ProvisionStateOpts{Target: nodes.TargetManage},
 		)
+		return
 
 	case nodes.Verifying:
 		// If we're still waiting for the state to change in Ironic,
 		// return true to indicate that we're dirty and need to be
 		// reconciled again.
-		result.RequeueAfter = provisionRequeueDelay
-		result.Dirty = true
-		return result, nil
+		result, err = operationContinuing(provisionRequeueDelay)
+		return
 
 	case nodes.Manageable:
 		p.log.Info("have manageable host")
-		return result, nil
+		return
 
 	case nodes.Available:
 		// The host is fully registered (and probably wasn't cleanly
 		// deleted previously)
 		p.log.Info("have available host")
-		return result, nil
+		return
 
 	case nodes.Active:
 		// The host is already running, maybe it's a master?
 		p.log.Info("have active host", "image_source", ironicNode.InstanceInfo["image_source"])
-		return result, nil
+		return
 
 	default:
-		return result, nil
+		return
 	}
 }
 
@@ -579,14 +576,15 @@ func (p *ironicProvisioner) tryChangeNodeProvisionState(ironicNode *nodes.Node,
 		success = true
 	case gophercloud.ErrDefault409:
 		p.log.Info("could not change state of host, busy")
+		result, err = retryAfterDelay(provisionRequeueDelay)
+		return
 	default:
-		err = errors.Wrap(changeResult.Err,
-			fmt.Sprintf("failed to change provisioning state to %q", opts.Target))
+		result, err = transientError(errors.Wrap(changeResult.Err,
+			fmt.Sprintf("failed to change provisioning state to %q", opts.Target)))
 		return
 	}
 
-	result.Dirty = true
-	result.RequeueAfter = provisionRequeueDelay
+	result, err = operationContinuing(provisionRequeueDelay)
 	return
 }
 
@@ -604,11 +602,12 @@ func (p *ironicProvisioner) InspectHardware(force bool) (result provisioner.Resu
 
 	ironicNode, err := p.findExistingHost()
 	if err != nil {
-		err = errors.Wrap(err, "failed to find existing host")
+		result, err = transientError(errors.Wrap(err, "failed to find existing host"))
 		return
 	}
 	if ironicNode == nil {
-		return result, nil, provisioner.NeedsRegistration
+		result, err = transientError(provisioner.NeedsRegistration)
+		return
 	}
 
 	status, err := introspection.GetIntrospectionStatus(p.inspector, ironicNode.UUID).Extract()
@@ -617,9 +616,7 @@ func (p *ironicProvisioner) InspectHardware(force bool) (result provisioner.Resu
 			switch nodes.ProvisionState(ironicNode.ProvisionState) {
 			case nodes.Inspecting, nodes.InspectWait:
 				p.log.Info("inspection already started")
-				result.Dirty = true
-				result.RequeueAfter = introspectionRequeueDelay
-				err = nil
+				result, err = operationContinuing(introspectionRequeueDelay)
 				return
 			default:
 				if nodes.ProvisionState(ironicNode.ProvisionState) == nodes.InspectFail && !force {
@@ -645,10 +642,10 @@ func (p *ironicProvisioner) InspectHardware(force bool) (result provisioner.Resu
 				case nil:
 				case gophercloud.ErrDefault409:
 					p.log.Info("could not update host settings in ironic, busy")
-					result.Dirty = true
+					result, err = retryAfterDelay(provisionRequeueDelay)
 					return
 				default:
-					err = errors.Wrap(err, "failed to update host boot mode settings in ironic")
+					result, err = transientError(errors.Wrap(err, "failed to update host boot mode settings in ironic"))
 					return
 				}
 
@@ -664,18 +661,17 @@ func (p *ironicProvisioner) InspectHardware(force bool) (result provisioner.Resu
 				return
 			}
 		}
-		err = errors.Wrap(err, "failed to extract hardware inspection status")
+		result, err = transientError(errors.Wrap(err, "failed to extract hardware inspection status"))
 		return
 	}
 	if !status.Finished {
 		p.log.Info("inspection in progress", "started_at", status.StartedAt)
-		result.Dirty = true // make sure we check back
-		result.RequeueAfter = introspectionRequeueDelay
+		result, err = operationContinuing(introspectionRequeueDelay)
 		return
 	}
 	if status.Error != "" {
 		p.log.Info("inspection failed", "error", status.Error)
-		result.ErrorMessage = status.Error
+		result, err = operationFailed(status.Error)
 		return
 	}
 
@@ -684,52 +680,44 @@ func (p *ironicProvisioner) InspectHardware(force bool) (result provisioner.Resu
 	introData := introspection.GetIntrospectionData(p.inspector, ironicNode.UUID)
 	data, err := introData.Extract()
 	if err != nil {
-		err = errors.Wrap(err, "failed to retrieve hardware introspection data")
+		result, err = transientError(errors.Wrap(err, "failed to retrieve hardware introspection data"))
 		return
 	}
 	p.log.Info("received introspection data", "data", introData.Body)
 
 	details = hardwaredetails.GetHardwareDetails(data)
 	p.publisher("InspectionComplete", "Hardware inspection completed")
+	result, err = operationComplete()
 	return
 }
 
 // UpdateHardwareState fetches the latest hardware state of the server
 // and updates the HardwareDetails field of the host with details. It
 // is expected to do this in the least expensive way possible, such as
-// reading from a cache, and return dirty only if any state
-// information has changed.
-func (p *ironicProvisioner) UpdateHardwareState() (result provisioner.Result, err error) {
+// reading from a cache.
+func (p *ironicProvisioner) UpdateHardwareState() (hwState provisioner.HardwareState, err error) {
 	p.log.Info("updating hardware state")
 
 	ironicNode, err := p.findExistingHost()
 	if err != nil {
-		return result, errors.Wrap(err, "failed to find existing host")
+		err = errors.Wrap(err, "failed to find existing host")
+		return
 	}
 	if ironicNode == nil {
-		return result, provisioner.NeedsRegistration
+		err = provisioner.NeedsRegistration
+		return
 	}
 
-	var discoveredVal bool
 	switch ironicNode.PowerState {
-	case powerOn:
-		discoveredVal = true
-	case powerOff:
-		discoveredVal = false
+	case powerOn, powerOff:
+		discoveredVal := ironicNode.PowerState == powerOn
+		hwState.PoweredOn = &discoveredVal
 	case powerNone:
 		p.log.Info("could not determine power state", "value", ironicNode.PowerState)
-		return result, nil
 	default:
 		p.log.Info("unknown power state", "value", ironicNode.PowerState)
-		return result, nil
-	}
-
-	if discoveredVal != p.host.Status.PoweredOn {
-		p.log.Info("updating power status", "discovered", discoveredVal)
-		p.host.Status.PoweredOn = discoveredVal
-		result.Dirty = true
 	}
-	return result, nil
+	return
 }
 
 func (p *ironicProvisioner) getImageUpdateOptsForNode(ironicNode *nodes.Node, imageData *metal3v1alpha1.Image) (updates nodes.UpdateOpts, err error) {
@@ -1009,17 +997,16 @@ func (p *ironicProvisioner) setUpForProvisioning(ironicNode *nodes.Node, hostCon
 
 	updates, err := p.getUpdateOptsForNode(ironicNode)
 	if err != nil {
-		return result, errors.Wrap(err, "failed to update opts for node")
+		return transientError(errors.Wrap(err, "failed to update opts for node"))
 	}
 	_, err = nodes.Update(p.client, ironicNode.UUID, updates).Extract()
 	switch err.(type) {
 	case nil:
 	case gophercloud.ErrDefault409:
 		p.log.Info("could not update host settings in ironic, busy")
-		result.Dirty = true
-		return result, nil
+		return retryAfterDelay(provisionRequeueDelay)
 	default:
-		return result, errors.Wrap(err, "failed to update host settings in ironic")
+		return transientError(errors.Wrap(err, "failed to update host settings in ironic"))
 	}
 
 	p.log.Info("validating host settings")
@@ -1029,15 +1016,12 @@ func (p *ironicProvisioner) setUpForProvisioning(ironicNode *nodes.Node, hostCon
 	case nil:
 	case gophercloud.ErrDefault409:
 		p.log.Info("could not validate host during registration, busy")
-		result.Dirty = true
-		return result, nil
+		return retryAfterDelay(provisionRequeueDelay)
 	default:
-		return result, errors.Wrap(err, "failed to validate host during registration")
+		return transientError(errors.Wrap(err, "failed to validate host during registration"))
 	}
 	if errorMessage != "" {
-		result.ErrorMessage = errorMessage
-		result.Dirty = true // validateNode() would have set the errors
-		return result, nil
+		return operationFailed(errorMessage)
 	}
 
 	// If validation is successful we can start moving the host
@@ -1058,18 +1042,16 @@ func (p *ironicProvisioner) Adopt(force bool) (result provisioner.Result, err er
 	var ironicNode *nodes.Node
 
 	if ironicNode, err = p.findExistingHost(); err != nil {
-		err = errors.Wrap(err, "could not find host to adpot")
-		return
+		return transientError(errors.Wrap(err, "could not find host to adpot"))
 	}
 	if ironicNode == nil {
-		err = provisioner.NeedsRegistration
-		return
+		return transientError(provisioner.NeedsRegistration)
 	}
 
 	switch nodes.ProvisionState(ironicNode.ProvisionState) {
 	case nodes.Enroll, nodes.Verifying:
-		err = fmt.Errorf("Invalid state for adopt: %s",
-			ironicNode.ProvisionState)
+		return transientError(fmt.Errorf("Invalid state for adopt: %s",
+			ironicNode.ProvisionState))
 	case nodes.Manageable:
 		return p.changeNodeProvisionState(
 			ironicNode,
@@ -1078,8 +1060,7 @@ func (p *ironicProvisioner) Adopt(force bool) (result provisioner.Result, err er
 			},
 		)
 	case nodes.Adopting:
-		result.RequeueAfter = provisionRequeueDelay
-		result.Dirty = true
+		return operationContinuing(provisionRequeueDelay)
 	case nodes.AdoptFail:
 		if force {
 			return p.changeNodeProvisionState(
@@ -1089,13 +1070,13 @@ func (p *ironicProvisioner) Adopt(force bool) (result provisioner.Result, err er
 				},
 			)
 		} else {
-			result.ErrorMessage = fmt.Sprintf("Host adoption failed: %s",
-				ironicNode.LastError)
+			return operationFailed(fmt.Sprintf("Host adoption failed: %s",
+				ironicNode.LastError))
 		}
 	case nodes.Active:
 	default:
 	}
-	return
+	return operationComplete()
 }
 
 // Provision writes the image from the host spec to the host. It may
@@ -1105,10 +1086,10 @@ func (p *ironicProvisioner) Provision(hostConf provisioner.HostConfigData) (resu
 	var ironicNode *nodes.Node
 
 	if ironicNode, err = p.findExistingHost(); err != nil {
-		return result, errors.Wrap(err, "could not find host to receive image")
+		return transientError(errors.Wrap(err, "could not find host to receive image"))
 	}
 	if ironicNode == nil {
-		return result, provisioner.NeedsRegistration
+		return transientError(provisioner.NeedsRegistration)
 	}
 
 	p.log.Info("provisioning image to host", "state", ironicNode.ProvisionState)
@@ -1128,8 +1109,6 @@ func (p *ironicProvisioner) Provision(hostConf provisioner.HostConfigData) (resu
 		"same", ironicHasSameImage,
 		"provisionState", ironicNode.ProvisionState)
 
-	result.RequeueAfter = provisionRequeueDelay
-
 	// Ironic has the settings it needs, see if it finds any issues
 	// with them.
 	switch nodes.ProvisionState(ironicNode.ProvisionState) {
@@ -1144,13 +1123,11 @@ func (p *ironicProvisioner) Provision(hostConf provisioner.HostConfigData) (resu
 			// top of relational databases...
 			if ironicNode.LastError == "" {
 				p.log.Info("failed but error message not available")
-				result.Dirty = true
-				return result, nil
+				return retryAfterDelay(0)
 			}
 			p.log.Info("found error", "msg", ironicNode.LastError)
-			result.ErrorMessage = fmt.Sprintf("Image provisioning failed: %s",
-				ironicNode.LastError)
-			return result, nil
+			return operationFailed(fmt.Sprintf("Image provisioning failed: %s",
+				ironicNode.LastError))
 		}
 		p.log.Info("recovering from previous failure")
 		if provResult, err := p.setUpForProvisioning(ironicNode, hostConf); err != nil || provResult.Dirty || provResult.ErrorMessage != "" {
@@ -1186,17 +1163,17 @@ func (p *ironicProvisioner) Provision(hostConf provisioner.HostConfigData) (resu
 		// Retrieve cloud-init user data
 		userData, err := hostConf.UserData()
 		if err != nil {
-			return result, errors.Wrap(err, "could not retrieve user data")
+			return transientError(errors.Wrap(err, "could not retrieve user data"))
 		}
 
 		// Retrieve cloud-init network_data.json. Default value is empty
 		networkDataRaw, err := hostConf.NetworkData()
 		if err != nil {
-			return result, errors.Wrap(err, "could not retrieve network data")
+			return transientError(errors.Wrap(err, "could not retrieve network data"))
 		}
 		var networkData map[string]interface{}
 		if err = yaml.Unmarshal([]byte(networkDataRaw), &networkData); err != nil {
-			return result, errors.Wrap(err, "failed to unmarshal network_data.json from secret")
+			return transientError(errors.Wrap(err, "failed to unmarshal network_data.json from secret"))
 		}
 
 		// Retrieve cloud-init meta_data.json with falback to default
@@ -1209,11 +1186,11 @@ func (p *ironicProvisioner) Provision(hostConf provisioner.HostConfigData) (resu
 		}
 		metaDataRaw, err := hostConf.MetaData()
 		if err != nil {
-			return result, errors.Wrap(err, "could not retrieve metadata")
+			return transientError(errors.Wrap(err, "could not retrieve metadata"))
 		}
 		if metaDataRaw != "" {
 			if err = yaml.Unmarshal([]byte(metaDataRaw), &metaData); err != nil {
-				return result, errors.Wrap(err, "failed to unmarshal metadata from secret")
+				return transientError(errors.Wrap(err, "failed to unmarshal metadata from secret"))
 			}
 		}
 
@@ -1225,7 +1202,7 @@ func (p *ironicProvisioner) Provision(hostConf provisioner.HostConfigData) (resu
 				NetworkData: networkData,
 			}
 			if err != nil {
-				return result, errors.Wrap(err, "failed to build config drive")
+				return transientError(errors.Wrap(err, "failed to build config drive"))
 			}
 			p.log.Info("triggering provisioning with config drive")
 		} else {
@@ -1245,15 +1222,14 @@ func (p *ironicProvisioner) Provision(hostConf provisioner.HostConfigData) (resu
 		p.publisher("ProvisioningComplete",
 			fmt.Sprintf("Image provisioning completed for %s", p.host.Spec.Image.URL))
 		p.log.Info("finished provisioning")
-		return result, nil
+		return operationComplete()
 
 	default:
 		// wait states like cleaning and clean wait
 		p.log.Info("waiting for host to become available",
 			"state", ironicNode.ProvisionState,
 			"deploy step", ironicNode.DeployStep)
-		result.Dirty = true
-		return result, nil
+		return operationContinuing(provisionRequeueDelay)
 	}
 }
 
@@ -1273,11 +1249,11 @@ func (p *ironicProvisioner) setMaintenanceFlag(ironicNode *nodes.Node, value boo
 	case nil:
 	case gophercloud.ErrDefault409:
 		p.log.Info("could not set host maintenance flag, busy")
+		return retryAfterDelay(provisionRequeueDelay)
 	default:
-		return result, errors.Wrap(err, "failed to set host maintenance flag")
+		return transientError(errors.Wrap(err, "failed to set host maintenance flag"))
 	}
-	result.Dirty = true
-	return result, nil
+	return operationContinuing(0)
 }
 
 // Deprovision removes the host from the image. It may be called
@@ -1288,10 +1264,10 @@ func (p *ironicProvisioner) Deprovision(force bool) (result provisioner.Result,
 
 	ironicNode, err := p.findExistingHost()
 	if err != nil {
-		return result, errors.Wrap(err, "failed to find existing host")
+		return transientError(errors.Wrap(err, "failed to find existing host"))
 	}
 	if ironicNode == nil {
-		return result, provisioner.NeedsRegistration
+		return transientError(provisioner.NeedsRegistration)
 	}
 
 	p.log.Info("deprovisioning host",
@@ -1337,27 +1313,21 @@ func (p *ironicProvisioner) Deprovision(force bool) (result provisioner.Result,
 
 	case nodes.Available:
 		p.publisher("DeprovisioningComplete", "Image deprovisioning completed")
-		return result, nil
+		return operationComplete()
 
 	case nodes.Deleting:
 		p.log.Info("deleting")
 		// Transitions to Cleaning upon completion
-		result.Dirty = true
-		result.RequeueAfter = deprovisionRequeueDelay
-		return result, nil
+		return operationContinuing(deprovisionRequeueDelay)
 
 	case nodes.Cleaning:
 		p.log.Info("cleaning")
 		// Transitions to Available upon completion
-		result.Dirty = true
-		result.RequeueAfter = deprovisionRequeueDelay
-		return result, nil
+		return operationContinuing(deprovisionRequeueDelay)
 
 	case nodes.CleanWait:
 		p.log.Info("cleaning")
-		result.Dirty = true
-		result.RequeueAfter = deprovisionRequeueDelay
-		return result, nil
+		return operationContinuing(deprovisionRequeueDelay)
 
 	case nodes.Active:
 		p.log.Info("starting deprovisioning")
@@ -1368,7 +1338,8 @@ func (p *ironicProvisioner) Deprovision(force bool) (result provisioner.Result,
 		)
 
 	default:
-		return result, fmt.Errorf("Unhandled ironic state %s", ironicNode.ProvisionState)
+		// FIXME(zaneb): this error is unlikely to actually be transient
+		return transientError(fmt.Errorf("Unhandled ironic state %s", ironicNode.ProvisionState))
 	}
 }
 
@@ -1379,11 +1350,11 @@ func (p *ironicProvisioner) Delete() (result provisioner.Result, err error) {
 
 	ironicNode, err := p.findExistingHost()
 	if err != nil {
-		return result, errors.Wrap(err, "failed to find existing host")
+		return transientError(errors.Wrap(err, "failed to find existing host"))
 	}
 	if ironicNode == nil {
 		p.log.Info("no node found, already deleted")
-		return result, nil
+		return operationComplete()
 	}
 
 	p.log.Info("deleting host",
@@ -1425,14 +1396,14 @@ func (p *ironicProvisioner) Delete() (result provisioner.Result, err error) {
 		p.log.Info("removed")
 	case gophercloud.ErrDefault409:
 		p.log.Info("could not remove host, busy")
+		return retryAfterDelay(provisionRequeueDelay)
 	case gophercloud.ErrDefault404:
 		p.log.Info("did not find host to delete, OK")
 	default:
-		return result, errors.Wrap(err, "failed to remove host")
+		return transientError(errors.Wrap(err, "failed to remove host"))
 	}
 
-	result.Dirty = true
-	return result, nil
+	return operationContinuing(0)
 }
 
 func (p *ironicProvisioner) changePower(ironicNode *nodes.Node, target nodes.TargetPowerState) (result provisioner.Result, err error) {
@@ -1443,9 +1414,7 @@ func (p *ironicProvisioner) changePower(ironicNode *nodes.Node, target nodes.Tar
 			"state", ironicNode.ProvisionState,
 			"target state", ironicNode.TargetProvisionState,
 		)
-		result.Dirty = true
-		result.RequeueAfter = powerRequeueDelay
-		return result, nil
+		return operationContinuing(powerRequeueDelay)
 	}
 
 	powerStateOpts := nodes.PowerStateOpts{
@@ -1462,12 +1431,11 @@ func (p *ironicProvisioner) changePower(ironicNode *nodes.Node, target nodes.Tar
 
 	switch changeResult.Err.(type) {
 	case nil:
-		result.Dirty = true
 		p.log.Info("power change OK")
+		return operationContinuing(0)
 	case gophercloud.ErrDefault409:
 		p.log.Info("host is locked, trying again after delay", "delay", powerRequeueDelay)
-		result.Dirty = true
-		result.RequeueAfter = powerRequeueDelay
+		result, _ = retryAfterDelay(powerRequeueDelay)
 		return result, HostLockedError{Address: p.host.Spec.BMC.Address}
 	case gophercloud.ErrDefault400:
 		// Error 400 Bad Request means target power state is not supported by vendor driver
@@ -1475,10 +1443,8 @@ func (p *ironicProvisioner) changePower(ironicNode *nodes.Node, target nodes.Tar
 		return result, SoftPowerOffUnsupportedError{Address: p.host.Spec.BMC.Address}
 	default:
 		p.log.Info("power change error", "message", changeResult.Err)
-		return result, errors.Wrap(changeResult.Err, "failed to change power state")
+		return transientError(errors.Wrap(changeResult.Err, "failed to change power state"))
 	}
-
-	return result, nil
 }
 
 // PowerOn ensures the server is powered on independently of any image
@@ -1488,7 +1454,7 @@ func (p *ironicProvisioner) PowerOn() (result provisioner.Result, err error) {
 
 	ironicNode, err := p.findExistingHost()
 	if err != nil {
-		return result, errors.Wrap(err, "failed to find existing host")
+		return transientError(errors.Wrap(err, "failed to find existing host"))
 	}
 
 	p.log.Info("checking current state",
@@ -1498,13 +1464,14 @@ func (p *ironicProvisioner) PowerOn() (result provisioner.Result, err error) {
 	if ironicNode.PowerState != powerOn {
 		if ironicNode.TargetPowerState == powerOn {
 			p.log.Info("waiting for power status to change")
-			result.RequeueAfter = powerRequeueDelay
-			result.Dirty = true
-			return result, nil
+			return operationContinuing(powerRequeueDelay)
 		}
 		result, err = p.changePower(ironicNode, nodes.PowerOn)
-		if err != nil {
-			return result, errors.Wrap(err, "failed to power on host")
+		switch err.(type) {
+		case nil:
+		case HostLockedError:
+		default:
+			return transientError(errors.Wrap(err, "failed to power on host"))
 		}
 		p.publisher("PowerOn", "Host powered on")
 	}
@@ -1525,12 +1492,9 @@ func (p *ironicProvisioner) PowerOff() (result provisioner.Result, err error) {
 		case SoftPowerOffUnsupportedError, SoftPowerOffFailed:
 			return p.hardPowerOff()
 		case HostLockedError:
-			result.RequeueAfter = powerRequeueDelay
-			result.Dirty = true
-			return result, nil
+			return retryAfterDelay(powerRequeueDelay)
 		default:
-			result.RequeueAfter = powerRequeueDelay
-			return result, err
+			return transientError(err)
 		}
 	}
 	return result, nil
@@ -1542,24 +1506,23 @@ func (p *ironicProvisioner) hardPowerOff() (result provisioner.Result, err error
 
 	ironicNode, err := p.findExistingHost()
 	if err != nil {
-		return result, errors.Wrap(err, "failed to find existing host")
+		return transientError(errors.Wrap(err, "failed to find existing host"))
 	}
 
 	if ironicNode.PowerState != powerOff {
 		if ironicNode.TargetPowerState == powerOff {
 			p.log.Info("waiting for power status to change")
-			result.RequeueAfter = powerRequeueDelay
-			result.Dirty = true
-			return result, nil
+			return operationContinuing(powerRequeueDelay)
 		}
 		result, err = p.changePower(ironicNode, nodes.PowerOff)
 		if err != nil {
-			return result, errors.Wrap(err, "failed to power off host")
+			return transientError(errors.Wrap(err, "failed to power off host"))
 		}
 		p.publisher("PowerOff", "Host powered off")
+		return result, err
 	}
 
-	return result, nil
+	return operationComplete()
 }
 
 // softPowerOff sends 'soft power off' request to BM node.
@@ -1572,7 +1535,7 @@ func (p *ironicProvisioner) softPowerOff() (result provisioner.Result, err error
 
 	ironicNode, err := p.findExistingHost()
 	if err != nil {
-		return result, errors.Wrap(err, "failed to find existing host")
+		return transientError(errors.Wrap(err, "failed to find existing host"))
 	}
 
 	if ironicNode.PowerState != powerOff {
@@ -1580,9 +1543,7 @@ func (p *ironicProvisioner) softPowerOff() (result provisioner.Result, err error
 		// If the target state is either powerOff or softPowerOff, then we should wait
 		if targetState == powerOff || targetState == softPowerOff {
 			p.log.Info("waiting for power status to change")
-			result.RequeueAfter = powerRequeueDelay
-			result.Dirty = true
-			return result, nil
+			return operationContinuing(powerRequeueDelay)
 		}
 		// If the target state is unset while the last error is set,
 		// then the last execution of soft power off has failed.
@@ -1591,8 +1552,7 @@ func (p *ironicProvisioner) softPowerOff() (result provisioner.Result, err error
 		}
 		result, err = p.changePower(ironicNode, nodes.SoftPowerOff)
 		if err != nil {
-			result.RequeueAfter = powerRequeueDelay
-			return result, err
+			return transientError(err)
 		}
 		p.publisher("PowerOff", "Host soft powered off")
 	}
diff --git a/pkg/provisioner/ironic/result.go b/pkg/provisioner/ironic/result.go
new file mode 100644
index 00000000..cb9057e8
--- /dev/null
+++ b/pkg/provisioner/ironic/result.go
@@ -0,0 +1,36 @@
+package ironic
+
+import (
+	"time"
+
+	"github.com/metal3-io/baremetal-operator/pkg/provisioner"
+)
+
+func retryAfterDelay(delay time.Duration) (provisioner.Result, error) {
+	// TODO(zaneb): this is currently indistinguishable from the result of
+	// operationContinuing() from the caller's perspective. Changes are
+	// required to the Result structure to enable this to be distinguished.
+	return provisioner.Result{
+		Dirty:        true,
+		RequeueAfter: delay,
+	}, nil
+}
+
+func operationContinuing(delay time.Duration) (provisioner.Result, error) {
+	return provisioner.Result{
+		Dirty:        true,
+		RequeueAfter: delay,
+	}, nil
+}
+
+func operationComplete() (provisioner.Result, error) {
+	return provisioner.Result{}, nil
+}
+
+func operationFailed(message string) (provisioner.Result, error) {
+	return provisioner.Result{ErrorMessage: message}, nil
+}
+
+func transientError(err error) (provisioner.Result, error) {
+	return provisioner.Result{}, err
+}
diff --git a/pkg/provisioner/provisioner.go b/pkg/provisioner/provisioner.go
index e5e01f2c..70c9b380 100644
--- a/pkg/provisioner/provisioner.go
+++ b/pkg/provisioner/provisioner.go
@@ -17,7 +17,7 @@ Package provisioning defines the API for talking to the provisioning backend.
 type EventPublisher func(reason, message string)
 
 // Factory is the interface for creating new Provisioner objects.
-type Factory func(host *metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publish EventPublisher) (Provisioner, error)
+type Factory func(host metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publish EventPublisher) (Provisioner, error)
 
 // HostConfigData retrieves host configuration data
 type HostConfigData interface {
@@ -43,7 +43,7 @@ type Provisioner interface {
 	// of credentials it has are different from the credentials it has
 	// previously been using, without implying that either set of
 	// credentials is correct.
-	ValidateManagementAccess(credentialsChanged, force bool) (result Result, err error)
+	ValidateManagementAccess(credentialsChanged, force bool) (result Result, provID string, err error)
 
 	// InspectHardware updates the HardwareDetails field of the host with
 	// details of devices discovered on the hardware. It may be called
@@ -54,9 +54,8 @@ type Provisioner interface {
 	// UpdateHardwareState fetches the latest hardware state of the
 	// server and updates the HardwareDetails field of the host with
 	// details. It is expected to do this in the least expensive way
-	// possible, such as reading from a cache, and return dirty only
-	// if any state information has changed.
-	UpdateHardwareState() (result Result, err error)
+	// possible, such as reading from a cache.
+	UpdateHardwareState() (hwState HardwareState, err error)
 
 	// Adopt brings an externally-provisioned host under management by
 	// the provisioner.
@@ -102,4 +101,11 @@ type Result struct {
 	ErrorMessage string
 }
 
+// HardwareState holds the response from an UpdateHardwareState call
+type HardwareState struct {
+	// PoweredOn is a pointer to a bool indicating whether the Host is currently
+	// powered on. The value is nil if the power state cannot be determined.
+	PoweredOn *bool
+}
+
 var NeedsRegistration = errors.New("Host not registered")


Test patch:
diff --git a/controllers/metal3.io/baremetalhost_controller_test.go b/controllers/metal3.io/baremetalhost_controller_test.go
index 3c02a2b9..e00d8e7d 100644
--- a/controllers/metal3.io/baremetalhost_controller_test.go
+++ b/controllers/metal3.io/baremetalhost_controller_test.go
@@ -23,8 +23,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	metal3v1alpha1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
-	"github.com/metal3-io/baremetal-operator/pkg/bmc"
-	"github.com/metal3-io/baremetal-operator/pkg/provisioner"
 	"github.com/metal3-io/baremetal-operator/pkg/provisioner/fixture"
 	"github.com/metal3-io/baremetal-operator/pkg/utils"
 )
@@ -88,7 +86,7 @@ func newDefaultHost(t *testing.T) *metal3v1alpha1.BareMetalHost {
 	return newDefaultNamedHost(t.Name(), t)
 }
 
-func newTestReconcilerWithProvisionerFactory(factory provisioner.Factory, initObjs ...runtime.Object) *BareMetalHostReconciler {
+func newTestReconcilerWithFixture(fix *fixture.Fixture, initObjs ...runtime.Object) *BareMetalHostReconciler {
 
 	c := fakeclient.NewFakeClient(initObjs...)
 
@@ -99,13 +97,14 @@ func newTestReconcilerWithProvisionerFactory(factory provisioner.Factory, initOb
 	return &BareMetalHostReconciler{
 		Client:             c,
 		Scheme:             scheme.Scheme,
-		ProvisionerFactory: factory,
+		ProvisionerFactory: fix.New,
 		Log:                ctrl.Log.WithName("controllers").WithName("BareMetalHost"),
 	}
 }
 
 func newTestReconciler(initObjs ...runtime.Object) *BareMetalHostReconciler {
-	return newTestReconcilerWithProvisionerFactory(fixture.New, initObjs...)
+	fix := fixture.Fixture{}
+	return newTestReconcilerWithFixture(&fix, initObjs...)
 }
 
 type DoneFunc func(host *metal3v1alpha1.BareMetalHost, result reconcile.Result) bool
@@ -1033,12 +1032,12 @@ func TestDeleteHost(t *testing.T) {
 			host.DeletionTimestamp = &now
 			host.Status.Provisioning.ID = "made-up-id"
 			badSecret := newBMCCredsSecret("bmc-creds-no-user", "", "Pass")
-			r := newTestReconciler(host, badSecret)
+			fix := fixture.Fixture{}
+			r := newTestReconcilerWithFixture(&fix, host, badSecret)
 
 			tryReconcile(t, r, host,
 				func(host *metal3v1alpha1.BareMetalHost, result reconcile.Result) bool {
-					t.Logf("provisioning id: %q", host.Status.Provisioning.ID)
-					return host.Status.Provisioning.ID == ""
+					return fix.Deleted
 				},
 			)
 		})
@@ -1156,22 +1155,12 @@ func TestUpdateRootDeviceHints(t *testing.T) {
 func TestProvisionerIsReady(t *testing.T) {
 	host := newDefaultHost(t)
 
-	var prov provisioner.Provisioner
-	r := newTestReconcilerWithProvisionerFactory(func(host *metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher) (provisioner provisioner.Provisioner, err error) {
-		if prov == nil {
-			prov, err = fixture.NewMock(host, bmcCreds, publisher, 5)
-		}
-		return prov, err
-	}, host)
+	fix := fixture.Fixture{BecomeReadyCounter: 5}
+	r := newTestReconcilerWithFixture(&fix, host)
 
 	tryReconcile(t, r, host,
 		func(host *metal3v1alpha1.BareMetalHost, result reconcile.Result) bool {
-			if prov == nil {
-				return false
-			}
-
-			ready, _ := prov.IsReady()
-			return ready
+			return host.Status.Provisioning.State != metal3v1alpha1.StateNone
 		},
 	)
 }
diff --git a/controllers/metal3.io/host_state_machine_test.go b/controllers/metal3.io/host_state_machine_test.go
index 96834305..51c1a971 100644
--- a/controllers/metal3.io/host_state_machine_test.go
+++ b/controllers/metal3.io/host_state_machine_test.go
@@ -18,7 +18,7 @@ import (
 
 func testStateMachine(host *metal3v1alpha1.BareMetalHost) *hostStateMachine {
 	r := newTestReconciler()
-	p, _ := r.ProvisionerFactory(host, bmc.Credentials{},
+	p, _ := r.ProvisionerFactory(*host.DeepCopy(), bmc.Credentials{},
 		func(reason, message string) {})
 	return newHostStateMachine(host, r, p, true)
 }
@@ -366,8 +366,8 @@ func (m *mockProvisioner) setNextResult(dirty bool) {
 	}
 }
 
-func (m *mockProvisioner) ValidateManagementAccess(credentialsChanged, force bool) (result provisioner.Result, err error) {
-	return m.nextResult, err
+func (m *mockProvisioner) ValidateManagementAccess(credentialsChanged, force bool) (result provisioner.Result, provID string, err error) {
+	return m.nextResult, "", err
 }
 
 func (m *mockProvisioner) InspectHardware(force bool) (result provisioner.Result, details *metal3v1alpha1.HardwareDetails, err error) {
@@ -375,8 +375,8 @@ func (m *mockProvisioner) InspectHardware(force bool) (result provisioner.Result
 	return m.nextResult, details, err
 }
 
-func (m *mockProvisioner) UpdateHardwareState() (result provisioner.Result, err error) {
-	return m.nextResult, err
+func (m *mockProvisioner) UpdateHardwareState() (hwState provisioner.HardwareState, err error) {
+	return
 }
 
 func (m *mockProvisioner) Adopt(force bool) (result provisioner.Result, err error) {
diff --git a/pkg/provisioner/ironic/delete_test.go b/pkg/provisioner/ironic/delete_test.go
index 16b8e438..00a1eb9a 100644
--- a/pkg/provisioner/ironic/delete_test.go
+++ b/pkg/provisioner/ironic/delete_test.go
@@ -50,7 +50,7 @@ func TestDelete(t *testing.T) {
 				},
 			).DeleteError(nodeUUID, http.StatusConflict),
 			expectedDirty:        true,
-			expectedRequestAfter: 0,
+			expectedRequestAfter: provisionRequeueDelay,
 		},
 		{
 			name: "delete-host-not-found",
@@ -122,7 +122,7 @@ func TestDelete(t *testing.T) {
 			).NodeUpdateError(nodeUUID, http.StatusConflict),
 
 			expectedDirty:        true,
-			expectedRequestAfter: 0,
+			expectedRequestAfter: provisionRequeueDelay,
 		},
 		{
 			name: "not-in-maintenance-update",
diff --git a/pkg/provisioner/ironic/ironic_test.go b/pkg/provisioner/ironic/ironic_test.go
index fbf2cba0..864609a3 100644
--- a/pkg/provisioner/ironic/ironic_test.go
+++ b/pkg/provisioner/ironic/ironic_test.go
@@ -16,10 +16,10 @@ func init() {
 	logf.SetLogger(logf.ZapLogger(true))
 }
 
-func makeHost() *metal3v1alpha1.BareMetalHost {
+func makeHost() metal3v1alpha1.BareMetalHost {
 	rotational := true
 
-	return &metal3v1alpha1.BareMetalHost{
+	return metal3v1alpha1.BareMetalHost{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "myhost",
 			Namespace: "myns",
diff --git a/pkg/provisioner/ironic/power_test.go b/pkg/provisioner/ironic/power_test.go
index 890fd19c..8aeda724 100644
--- a/pkg/provisioner/ironic/power_test.go
+++ b/pkg/provisioner/ironic/power_test.go
@@ -73,7 +73,6 @@ func TestPowerOn(t *testing.T) {
 			}).WithNodeStatesPower(nodeUUID, http.StatusConflict).WithNodeStatesPowerUpdate(nodeUUID, http.StatusConflict),
 			expectedRequestAfter: 10,
 			expectedDirty:        true,
-			expectedError:        true,
 		},
 	}
 
diff --git a/pkg/provisioner/ironic/provision_test.go b/pkg/provisioner/ironic/provision_test.go
index 8c463ffa..947cecab 100644
--- a/pkg/provisioner/ironic/provision_test.go
+++ b/pkg/provisioner/ironic/provision_test.go
@@ -22,6 +22,7 @@ func TestProvision(t *testing.T) {
 		ironic               *testserver.IronicMock
 		expectedDirty        bool
 		expectedError        bool
+		expectedErrorMessage bool
 		expectedRequestAfter int
 	}{
 		{
@@ -31,7 +32,8 @@ func TestProvision(t *testing.T) {
 				UUID:           nodeUUID,
 			}),
 			expectedRequestAfter: 0,
-			expectedDirty:        true,
+			expectedDirty:        false,
+			expectedErrorMessage: true,
 		},
 		{
 			name: "cleanFail state",
@@ -58,7 +60,8 @@ func TestProvision(t *testing.T) {
 				UUID:           nodeUUID,
 			}),
 			expectedRequestAfter: 0,
-			expectedDirty:        true,
+			expectedDirty:        false,
+			expectedErrorMessage: true,
 		},
 		{
 			name: "active state",
@@ -66,7 +69,7 @@ func TestProvision(t *testing.T) {
 				ProvisionState: string(nodes.Active),
 				UUID:           nodeUUID,
 			}),
-			expectedRequestAfter: 10,
+			expectedRequestAfter: 0,
 			expectedDirty:        false,
 		},
 		{
@@ -108,6 +111,11 @@ func TestProvision(t *testing.T) {
 
 			assert.Equal(t, tc.expectedDirty, result.Dirty)
 			assert.Equal(t, time.Second*time.Duration(tc.expectedRequestAfter), result.RequeueAfter)
+			if !tc.expectedErrorMessage {
+				assert.Equal(t, "", result.ErrorMessage)
+			} else {
+				assert.NotEqual(t, "", result.ErrorMessage)
+			}
 			if !tc.expectedError {
 				assert.NoError(t, err)
 			} else {
diff --git a/pkg/provisioner/ironic/updatehardwarestate_test.go b/pkg/provisioner/ironic/updatehardwarestate_test.go
index 63944b81..e4e89b0b 100644
--- a/pkg/provisioner/ironic/updatehardwarestate_test.go
+++ b/pkg/provisioner/ironic/updatehardwarestate_test.go
@@ -3,7 +3,6 @@ package ironic
 import (
 	"net/http"
 	"testing"
-	"time"
 
 	"github.com/gophercloud/gophercloud/openstack/baremetal/v1/nodes"
 	"github.com/stretchr/testify/assert"
@@ -24,9 +23,7 @@ func TestUpdateHardwareState(t *testing.T) {
 		hostCurrentlyPowered bool
 		hostName             string
 
-		expectedDirty        bool
-		expectedRequestAfter int
-		expectedResultError  string
+		expectUnreadablePower bool
 
 		expectedPublish string
 		expectedError   string
@@ -36,6 +33,7 @@ func TestUpdateHardwareState(t *testing.T) {
 			ironic: testserver.NewIronic(t).Ready().Node(nodes.Node{
 				UUID: nodeUUID,
 			}),
+			expectUnreadablePower: true,
 		},
 		{
 			name: "updated-power-on-state",
@@ -52,8 +50,6 @@ func TestUpdateHardwareState(t *testing.T) {
 				PowerState: "power on",
 			}),
 			hostCurrentlyPowered: false,
-
-			expectedDirty: true,
 		},
 		{
 			name: "updated-power-off-state",
@@ -70,8 +66,6 @@ func TestUpdateHardwareState(t *testing.T) {
 				PowerState: "power off",
 			}),
 			hostCurrentlyPowered: true,
-
-			expectedDirty: true,
 		},
 		{
 			name: "no-power",
@@ -79,7 +73,8 @@ func TestUpdateHardwareState(t *testing.T) {
 				UUID:       nodeUUID,
 				PowerState: "None",
 			}),
-			hostCurrentlyPowered: true,
+			hostCurrentlyPowered:  true,
+			expectUnreadablePower: true,
 		},
 		{
 			name: "node-not-found",
@@ -88,6 +83,8 @@ func TestUpdateHardwareState(t *testing.T) {
 			ironic:   testserver.NewIronic(t).Ready().NodeError(nodeUUID, http.StatusGatewayTimeout),
 
 			expectedError: "failed to find existing host: failed to find node by ID 33ce8659-7400-4c68-9535-d10766f07a58: Expected HTTP response code \\[200\\].*",
+
+			expectUnreadablePower: true,
 		},
 		{
 			name: "node-not-found-by-name",
@@ -96,12 +93,16 @@ func TestUpdateHardwareState(t *testing.T) {
 			ironic:   testserver.NewIronic(t).Ready().NoNode(nodeUUID).NodeError("myhost", http.StatusGatewayTimeout),
 
 			expectedError: "failed to find existing host: failed to find node by name worker-0: EOF",
+
+			expectUnreadablePower: true,
 		},
 		{
 			name:   "not-ironic-node",
 			ironic: testserver.NewIronic(t).Ready().NoNode(nodeUUID).NoNode("myhost"),
 
 			expectedError: "Host not registered",
+
+			expectUnreadablePower: true,
 		},
 	}
 
@@ -136,11 +137,9 @@ func TestUpdateHardwareState(t *testing.T) {
 				t.Fatalf("could not create provisioner: %s", err)
 			}
 
-			result, err := prov.UpdateHardwareState()
+			hwStatus, err := prov.UpdateHardwareState()
 
-			assert.Equal(t, tc.expectedDirty, result.Dirty)
-			assert.Equal(t, time.Second*time.Duration(tc.expectedRequestAfter), result.RequeueAfter)
-			assert.Equal(t, tc.expectedResultError, result.ErrorMessage)
+			assert.Equal(t, tc.expectUnreadablePower, hwStatus.PoweredOn == nil)
 
 			assert.Equal(t, tc.expectedPublish, publishedMsg)
 			if tc.expectedError == "" {
diff --git a/pkg/provisioner/ironic/updateopts_test.go b/pkg/provisioner/ironic/updateopts_test.go
index 88714db7..d712064f 100644
--- a/pkg/provisioner/ironic/updateopts_test.go
+++ b/pkg/provisioner/ironic/updateopts_test.go
@@ -83,7 +83,7 @@ func TestGetUpdateOptsForNodeWithRootHints(t *testing.T) {
 }
 
 func TestGetUpdateOptsForNodeVirtual(t *testing.T) {
-	host := &metal3v1alpha1.BareMetalHost{
+	host := metal3v1alpha1.BareMetalHost{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "myhost",
 			Namespace: "myns",
@@ -188,7 +188,7 @@ func TestGetUpdateOptsForNodeVirtual(t *testing.T) {
 }
 
 func TestGetUpdateOptsForNodeDell(t *testing.T) {
-	host := &metal3v1alpha1.BareMetalHost{
+	host := metal3v1alpha1.BareMetalHost{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "myhost",
 			Namespace: "myns",
diff --git a/pkg/provisioner/ironic/validatemanagementaccess_test.go b/pkg/provisioner/ironic/validatemanagementaccess_test.go
index 13f49777..378d62ff 100644
--- a/pkg/provisioner/ironic/validatemanagementaccess_test.go
+++ b/pkg/provisioner/ironic/validatemanagementaccess_test.go
@@ -33,7 +33,7 @@ func TestValidateManagementAccessNoMAC(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, err := prov.ValidateManagementAccess(false, false)
+	result, _, err := prov.ValidateManagementAccess(false, false)
 	if err != nil {
 		t.Fatalf("error from ValidateManagementAccess: %s", err)
 	}
@@ -63,7 +63,7 @@ func TestValidateManagementAccessMACOptional(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, err := prov.ValidateManagementAccess(false, false)
+	result, _, err := prov.ValidateManagementAccess(false, false)
 	if err != nil {
 		t.Fatalf("error from ValidateManagementAccess: %s", err)
 	}
@@ -95,13 +95,13 @@ func TestValidateManagementAccessCreateNode(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, err := prov.ValidateManagementAccess(false, false)
+	result, provID, err := prov.ValidateManagementAccess(false, false)
 	if err != nil {
 		t.Fatalf("error from ValidateManagementAccess: %s", err)
 	}
 	assert.Equal(t, "", result.ErrorMessage)
 	assert.NotEqual(t, "", createdNode.UUID)
-	assert.Equal(t, createdNode.UUID, host.Status.Provisioning.ID)
+	assert.Equal(t, createdNode.UUID, provID)
 }
 
 func TestValidateManagementAccessExistingNode(t *testing.T) {
@@ -130,12 +130,12 @@ func TestValidateManagementAccessExistingNode(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, err := prov.ValidateManagementAccess(false, false)
+	result, provID, err := prov.ValidateManagementAccess(false, false)
 	if err != nil {
 		t.Fatalf("error from ValidateManagementAccess: %s", err)
 	}
 	assert.Equal(t, "", result.ErrorMessage)
-	assert.Equal(t, "uuid", host.Status.Provisioning.ID)
+	assert.Equal(t, "uuid", provID)
 }
 
 func TestValidateManagementAccessExistingNodeContinue(t *testing.T) {
@@ -193,7 +193,7 @@ func TestValidateManagementAccessExistingNodeContinue(t *testing.T) {
 				t.Fatalf("could not create provisioner: %s", err)
 			}
 
-			result, err := prov.ValidateManagementAccess(false, false)
+			result, _, err := prov.ValidateManagementAccess(false, false)
 			if err != nil {
 				t.Fatalf("error from ValidateManagementAccess: %s", err)
 			}
@@ -238,7 +238,7 @@ func TestValidateManagementAccessExistingNodeWaiting(t *testing.T) {
 				t.Fatalf("could not create provisioner: %s", err)
 			}
 
-			result, err := prov.ValidateManagementAccess(false, false)
+			result, _, err := prov.ValidateManagementAccess(false, false)
 			if err != nil {
 				t.Fatalf("error from ValidateManagementAccess: %s", err)
 			}
@@ -280,12 +280,12 @@ func TestValidateManagementAccessNewCredentials(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, err := prov.ValidateManagementAccess(true, false)
+	result, provID, err := prov.ValidateManagementAccess(true, false)
 	if err != nil {
 		t.Fatalf("error from ValidateManagementAccess: %s", err)
 	}
 	assert.Equal(t, "", result.ErrorMessage)
-	assert.Equal(t, "uuid", host.Status.Provisioning.ID)
+	assert.Equal(t, "uuid", provID)
 
 	updates := ironic.GetLastNodeUpdateRequestFor("uuid")
 	newValues := updates[0].Value.(map[string]interface{})
@@ -327,12 +327,12 @@ func TestValidateManagementAccessLinkExistingIronicNodeByMAC(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, err := prov.ValidateManagementAccess(false, false)
+	result, provID, err := prov.ValidateManagementAccess(false, false)
 	if err != nil {
 		t.Fatalf("error from ValidateManagementAccess: %s", err)
 	}
 	assert.Equal(t, "", result.ErrorMessage)
-	assert.NotEqual(t, "", host.Status.Provisioning.ID)
+	assert.NotEqual(t, "", provID)
 }
 
 func TestValidateManagementAccessExistingPortWithWrongUUID(t *testing.T) {
@@ -370,7 +370,7 @@ func TestValidateManagementAccessExistingPortWithWrongUUID(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	_, err = prov.ValidateManagementAccess(false, false)
+	_, _, err = prov.ValidateManagementAccess(false, false)
 	assert.EqualError(t, err, "failed to find existing host: port exists but linked node doesn't random-wrong-id: Resource not found")
 }
 
@@ -412,7 +412,7 @@ func TestValidateManagementAccessExistingPortButHasName(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	_, err = prov.ValidateManagementAccess(false, false)
+	_, _, err = prov.ValidateManagementAccess(false, false)
 	assert.EqualError(t, err, "failed to find existing host: node found by MAC but has a name: wrong-name")
 }
 
@@ -450,10 +450,10 @@ func TestValidateManagementAccessAddTwoHostsWithSameMAC(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, err := prov.ValidateManagementAccess(false, false)
+	result, provID, err := prov.ValidateManagementAccess(false, false)
 	if err != nil {
 		t.Fatalf("error from ValidateManagementAccess: %s", err)
 	}
 	assert.Equal(t, "", result.ErrorMessage)
-	assert.NotEqual(t, "", host.Status.Provisioning.ID)
+	assert.NotEqual(t, "", provID)
 }