-
Notifications
You must be signed in to change notification settings - Fork 311
Add Preparing state to do manual clean #763
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -455,6 +455,9 @@ func (r *BareMetalHostReconciler) registerHost(prov provisioner.Provisioner, inf | |
| if provID != "" && info.host.Status.Provisioning.ID != provID { | ||
| info.log.Info("setting provisioning id", "ID", provID) | ||
| info.host.Status.Provisioning.ID = provID | ||
| if info.host.Status.Provisioning.State == metal3v1alpha1.StatePreparing { | ||
| clearHostProvisioningSettings(info.host) | ||
| } | ||
| dirty = true | ||
| } | ||
|
|
||
|
|
@@ -564,6 +567,44 @@ func (r *BareMetalHostReconciler) actionMatchProfile(prov provisioner.Provisione | |
| return actionComplete{} | ||
| } | ||
|
|
||
| func (r *BareMetalHostReconciler) actionPreparing(prov provisioner.Provisioner, info *reconcileInfo) actionResult { | ||
| info.log.Info("preparing") | ||
|
|
||
| // Save provisioning settings. | ||
| provisioningSettings := info.host.Status.Provisioning.DeepCopy() | ||
| dirty, err := saveHostProvisioningSettings(info.host) | ||
| if err != nil { | ||
| return actionError{errors.Wrap(err, "Could not save the host provisioning settings")} | ||
| } | ||
|
|
||
| // Do prepare(manual clean). | ||
| provResult, started, err := prov.Prepare(dirty) | ||
| if err != nil { | ||
| return actionError{errors.Wrap(err, "error preparing host")} | ||
| } | ||
|
|
||
| if provResult.ErrorMessage != "" { | ||
| info.log.Info("handling cleaning error in controller") | ||
|
zaneb marked this conversation as resolved.
|
||
| clearHostProvisioningSettings(info.host) | ||
| return recordActionFailure(info, metal3v1alpha1.PreparationError, provResult.ErrorMessage) | ||
| } | ||
|
|
||
| if provResult.Dirty { | ||
| result := actionContinue{provResult.RequeueAfter} | ||
| if clearError(info.host) || (dirty && started) { | ||
| // If clearError return true, but started is false, restore provisioningSettings. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment is not really clear to me, can you please elaborate it a little bit?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just describing how the if statement below can come to be true. |
||
| if dirty && !started { | ||
| info.host.Status.Provisioning = *provisioningSettings | ||
|
zaneb marked this conversation as resolved.
zaneb marked this conversation as resolved.
|
||
| } | ||
| return actionUpdate{result} | ||
| } | ||
| return result | ||
| } | ||
|
|
||
|
zaneb marked this conversation as resolved.
|
||
| clearError(info.host) | ||
| return actionComplete{} | ||
| } | ||
|
|
||
| // Start/continue provisioning if we need to. | ||
| func (r *BareMetalHostReconciler) actionProvisioning(prov provisioner.Provisioner, info *reconcileInfo) actionResult { | ||
| hostConf := &hostConfigData{ | ||
|
|
@@ -796,7 +837,9 @@ func (r *BareMetalHostReconciler) actionManageReady(prov provisioner.Provisioner | |
| return actionError{errors.Wrap(err, "Could not save the host provisioning settings")} | ||
| } | ||
| if dirty { | ||
| info.log.Info("updating host provisioning settings") | ||
| info.log.Info("Host provisioning settings have been updated, go back to Preparing state") | ||
| clearHostProvisioningSettings(info.host) | ||
| return actionUpdate{} | ||
|
zaneb marked this conversation as resolved.
|
||
| } | ||
| clearError(info.host) | ||
| return actionComplete{} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Hellcatlk, just to clarify, IIUC @zaneb 's comment then
saveHostProvisioningSettingswill contains the necessary logic to trigger the manual cleaning in the Prepare. Right now in this method just theProvisioning.RootDeviceHintsare checked. If it is expected to be enriched in future for the related RAID section, then probably I'd find more clear to add comment for that within such methodThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct. But note that this function already existed, and this is the right thing to do regardless of whether RAID is added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I was thinking to something more like the
// TODO:in thebuildManualCleaningSteps