Unable to update BMC address

[sebastiaopamplona]

What steps did you take and what happened:
I have a use case where the BMC address changes after the BMH CRD is created (I'm manually changing it via Redfish).
I tried to update the BMH CRD spec.bmc.address accordingly, but I'm unable to do it (I suspect the field is immutable, but I did not find anything in the docs about it).

What did you expect to happen:
I expected the spec.bmc.address to be updated

/kind feature-request

[sebastiaopamplona]

nevermind, I just got this error:

error: baremetalhosts.metal3.io "bmh123" could not be patched: admission webhook "baremetalhost.metal3.io" denied the request: BMC address can not be changed once it is set

is there any plans this would be made possible?

[dtantsur]

I would like to make it possible in at least some way. It's not exactly trivial because there are many states where changing the BMC addresses can lead to weird results. Possibly, we do it through detaching.

[sebastiaopamplona]

I see; in my use case I need to do it imedeately after the first inspection, so my workaround is re-creating the BMH with the new BMC address and settimg the status annotation equal to the status of the original BMH

[dtantsur]

@sebastiaopamplona I'm curious, can you share any details why it happens?

[sebastiaopamplona]

do you mean why I need to do to change the BMC address immediately after the first inspection?

[dtantsur]

@sebastiaopamplona yep, that's my question.

[sebastiaopamplona]

it's a use case where initially I need to use a temporary bmc ip because I don't know much about the server, and after inspection I need to replace it with the final one

[dtantsur]

/triage accepted

I must admit, your case is quite unusual. Yet, I believe that it's good to allow changing BMC addresses in at least some cases.

[metal3-io-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues will close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[dtantsur]

/remove-lifecycle stale

Is #1549 sufficient to close this?

[sebastiaopamplona]

@dtantsur thanks for having this implemented :)

a small question: does the BMH need to be in registering provisioning state? in our case we'd like to be able to change it from the available provisioning state

maybe this was a small bug in the if statement that makes it so that the BMH needs to be in the registering provisioning state and detached (the PR description mentions Registering state or is detached)

[MahnoorAsghar]

@dtantsur thanks for having this implemented :)

a small question: does the BMH need to be in registering provisioning state? in our case we'd like to be able to change it from the available provisioning state

maybe this was a small bug in the if statement that makes it so that the BMH needs to be in the registering provisioning state and detached (the PR description mentions Registering state or is detached)

Hi @sebastiaopamplona!
You can change the BMC address while the BMH is in the available state using the detached annotation.
About the if statement: the error must only arise if all 3 conditions are met, i.e. the BMC address has changed, and the host is not detached, and the host is also not in the Registering state. If any of these conditions are not met, i.e. the BMC address is the same, or the host is Registering or detached, the error must not come up. Therefore, the if condition is correct as per the PR description and intention.

[sebastiaopamplona]

hey @MahnoorAsghar , thank you for the detailed explanation and implementation; as far as i'm concerned, we can close this issue (not sure what's the standard way to go about it, so i did not close it myself)
have a nice day!

[MahnoorAsghar]

/assign MahnoorAsghar

[MahnoorAsghar]

/close

[metal3-io-bot]

@MahnoorAsghar: Closing this issue.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.