Repository: metal3-io/baremetal-operator
Pull request #967 (2021-09-06 07:13:19)

Fix comment for detached annotation and add StateAvailable 
Addresses small comment from [965](https://github.com/metal3-io/baremetal-operator/pull/965#discussion_r702077235). 

Upd.: Also, adds Hosts in StateAvialable to allow using a detached annotation. 

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

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **dtantsur** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @dtantsur` in a comment when ready.

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

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

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

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":["dtantsur"]} -->
furkatgofurov7: /assign @zaneb 
/cc @kashifest 
/test-integration
furkatgofurov7: /cc @maelk 
maelk: /lgtm
kashifest: lgtm
metal3-io-bot: New changes are detected. LGTM label has been removed.
metal3-io-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please ask for approval from **zaneb** after the PR has been reviewed.

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

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

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

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"]} -->
metal3-io-bot: @furkatgofurov7: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
unit | 4e72137a1a04cbcefa39a47fa0687cfbf1a5046f | [link](https://prow.apps.test.metal3.io/view/gs/metal3-prow/pr-logs/pull/metal3-io_baremetal-operator/967/unit/1435539187985027072) | `/test unit`

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->

Patch:
diff --git a/controllers/metal3.io/host_state_machine.go b/controllers/metal3.io/host_state_machine.go
index 03a76f6d..3c93cd5b 100644
--- a/controllers/metal3.io/host_state_machine.go
+++ b/controllers/metal3.io/host_state_machine.go
@@ -254,9 +254,9 @@ func (hsm *hostStateMachine) checkDetachedHost(info *reconcileInfo) (result acti
 	// provisioner and take no further action
 	// Note this doesn't change the current state, only the OperationalStatus
 	if hasDetachedAnnotation(hsm.Host) {
-		// Only allow detaching hosts in Provisioned/ExternallyProvisioned states
+		// Only allow detaching hosts in Provisioned/ExternallyProvisioned/Ready/Available states
 		switch info.host.Status.Provisioning.State {
-		case metal3v1alpha1.StateProvisioned, metal3v1alpha1.StateExternallyProvisioned, metal3v1alpha1.StateReady:
+		case metal3v1alpha1.StateProvisioned, metal3v1alpha1.StateExternallyProvisioned, metal3v1alpha1.StateReady, metal3v1alpha1.StateAvailable:
 			return hsm.Reconciler.detachHost(hsm.Provisioner, info)
 		}
 	}
diff --git a/docs/api.md b/docs/api.md
index a8f138c3..26c12dfa 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -574,7 +574,8 @@ changing power state, or deprovisioning), but still allows the BMH status
 to be updated unlike the `paused` anotation. While in this state the
 OperationalStatus field will be `detached` but the provisioning state will
 be unmodified.  This API only has any effect for BareMetalHost resources
-that are in either `Provisioned` or `ExternallyProvisioned` state.
+that are in either `Provisioned`, `ExternallyProvisioned` or `Ready`/`Available`
+state.
 
 Please note only the existence of the annotation is important to treat the BMH
 as detached and the value of the annotation is always ignored.


Test patch:
diff --git a/controllers/metal3.io/host_state_machine_test.go b/controllers/metal3.io/host_state_machine_test.go
index 88666edf..697b9363 100644
--- a/controllers/metal3.io/host_state_machine_test.go
+++ b/controllers/metal3.io/host_state_machine_test.go
@@ -300,10 +300,28 @@ func TestDetach(t *testing.T) {
 			ExpectedOperationalStatus: metal3v1alpha1.OperationalStatusOK,
 			ExpectedState:             metal3v1alpha1.StatePreparing,
 		},
+		{
+			Scenario:                  "DetachAvailableHost",
+			Host:                      host(metal3v1alpha1.StateAvailable).SetImageURL("").SetStatusPoweredOn(false).build(),
+			HasDetachedAnnotation:     true,
+			ExpectedDetach:            true,
+			ExpectedDirty:             true,
+			ExpectedOperationalStatus: metal3v1alpha1.OperationalStatusDetached,
+			ExpectedState:             metal3v1alpha1.StateAvailable,
+		},
+		{
+			Scenario:                  "AttachAvailableHost",
+			Host:                      host(metal3v1alpha1.StateAvailable).SetImageURL("").SetStatusPoweredOn(false).build(),
+			HasDetachedAnnotation:     false,
+			ExpectedDetach:            false,
+			ExpectedDirty:             true,
+			ExpectedOperationalStatus: metal3v1alpha1.OperationalStatusOK,
+			ExpectedState:             metal3v1alpha1.StateAvailable,
+		},
 		{
 			Scenario:                  "AvailableHost",
 			Host:                      host(metal3v1alpha1.StateAvailable).build(),
-			HasDetachedAnnotation:     true,
+			HasDetachedAnnotation:     false,
 			ExpectedDetach:            false,
 			ExpectedDirty:             true,
 			ExpectedOperationalStatus: metal3v1alpha1.OperationalStatusOK,