Ironic should set the node power-state to off when registering a node

[Steve Kowalik]

In commit 42cd4ca2a20415542ab99217fb384b1453bc9dd4 in tripleo-incubator, in scripts/register-nodes, the code was changed so that nodes were powered off immediately after registeration. This sounds like something that Ironic should do without having to set the power state manually.

[aeva black (tenbrae)]

Copying conversation snippet from IRC as it provides some background on this bug:

06:56:19 < lifeless> devananda: because in a rack of unknown state, having machines running that aren't undergoing maintenance or actively deployed is a bad idea
06:56:45 < lifeless> devananda: and we found that ironic takes quite some time to assert the power state post-registration - long enough for deploys to get broken by it
06:57:06 < lifeless> devananda: so someone put a patch into tripleo-incubator to power off nodes post registration
06:57:28 < lifeless> devananda: I believe that workarounds in incubator are a misfeature :)

[Michael Davies (mrda)]

Just some more discussion in IRC this morning:

@devananda | asserting power state post-registration seems reasonable to me
@devananda | however
@devananda | it doesn't make sense in some ways given today's api
@devananda | you register a node. but you can't register it WITH a power state
@devananda | so then ironic checks the node's current power state and /might/ go turn it off

              mrda | so adding an optional desired_power_state might be the way forward?
              mrda | ...to the api

@devananda | mrda: dont think that's good either

...the discussion continues... ;)

[Michael Davies (mrda)]

mrda | Should we simply power off nodes post registration if they're on already?

devananda | https://github.com/openstack/ironic/blob/master/ironic/api/controllers/v1/node.py#L637 and
devananda | https://github.com/openstack/ironic/blob/master/ironic/conductor/manager.py#L97
devananda | so... I think it's a reasonable thing to do IFF force_power_state_during_sync=TRUE
devananda | even if that option is TRUE, the creation of a node is simply a DB insert. it doesn't pass anything to conductor today -- so this would be a bit of a larger change

mrda | ok, so when a node is registered, if force_power_state_during_sync is True, power off the node (since the default power state in the DB is off)?

devananda | and create another point where API requests could starve RPC threads

mrda | ok, so it sounds like you're reluctant for this change to happen
mrda | ...unless some clever code can be constructed to not create a new problem :)

devananda | https://github.com/openstack/ironic/blob/master/ironic/conductor/manager.py#L530
devananda | so current behavior is, if node has no power state recorded in the db, we just update the DB with the actual state regardless
devananda | so this is a behavior change, and possibly an RPC API change

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/97693

[OpenStack Infra (hudson-openstack)]

Change abandoned by Michael Davies (<email address hidden>) on branch: master
Review: https://review.openstack.org/97693
Reason: See comments - this patch is abandoned in favour of https://review.openstack.org/#/c/85529

[Michael Davies (mrda)]

I've discussed this issue with Chris Krelle - see http://eavesdrop.openstack.org/irclogs/%23openstack-ironic/%23openstack-ironic.2014-06-11.log from 2014-06-11T21:22:35 to 2014-06-11T21:52:25

Basically Chris has a competing patch in https://review.openstack.org/#/c/85529 that does the same thing in the conductor that I'm proposing here for the api. We've decided his approach is better, so I'll abandon my patch.

I'm marking this as WONTFIX as Chris' patch doesn't directly address the concerns raised in this bug - if an Ironic consumer wants a newly registered node to be powered off immediately then they should explicitly do so as per https://github.com/openstack/tripleo-incubator/blob/master/scripts/register-nodes#L72 otherwise they should wait for the periodic task that'll turn power state NONE into Power_OFF eventually.