-
Notifications
You must be signed in to change notification settings - Fork 125
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
Enable ironic cleaning #28
Conversation
Enable ironic to clean disk metadata only.
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.
I tried this in a virtual env with dev-scripts and it looks like it causes a problem in the terraform state machine, it appears terraform is not expecting "clean wait":
level=debug msg="\t* ironic_node_v1.openshift-master-2: cannot go from state 'clean wait' to state 'manageable'"
level=debug
level=debug msg="2019/04/13 10:33:08 [ERROR] root.masters: eval: *terraform.EvalSequence, err: 1 error occurred:"
We'll have to make changes to the installer before we can merge this.
I wonder if we just need to add a new state? // Change a node to "available" state
func (workflow *provisionStateWorkflow) toAvailable() (done bool, err error) {
switch state := workflow.d.Get("provision_state").(string); state {
case "available":
// We're done!
return true, nil
case "cleaning":
// Not done, no error - Ironic is working
log.Printf("[DEBUG] Node %s is '%s', waiting for Ironic to finish.", workflow.d.Id(), state)
return false, nil
case "manageable":
// From manageable, we can go to provide
log.Printf("[DEBUG] Node %s is '%s', going to change to 'available'", workflow.d.Id(), state)
return workflow.changeProvisionState(nodes.TargetProvide)
default:
// Otherwise we have to get into manageable state first
log.Printf("[DEBUG] Node %s is '%s', going to change to 'manageable'.", workflow.d.Id(), state)
_, err := workflow.toManageable()
if err != nil {
return true, err
}
return false, nil
}
return false, nil
} |
@stbenjam Would you happen to know? ^ |
Yes, that's all it needs. openshift-metal3/terraform-provider-ironic#16 |
The installer needs an update to pull in the new terraform provider - that's here, openshift-metal3/kni-installer#45. You can test this updated container with that, it should work now. I also filed openshift-metal3/terraform-provider-ironic#17 for me to ensure I handle all the possible state transitions, there's a few more that aren't covered, mostly |
With the terraform change to add support for clean-wait this should fe fine. |
I’d not merge this until the installer change is in |
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.
Good to merge when we are ready.
@bfournie What else is holding this up? |
openshift-metal3/kni-installer#45 has been merged, this should be good to go. |
Enable ironic to clean disk metadata only.