)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"cc5f97dda86c73778b15f66f9c5187587f51cc51","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"e5104296_7e898e83","updated":"2024-02-15 23:32:12.000000000","message":"If someone revives this later -- this code as written is potentially dangerous. It has an assumption baked in that node.last_error is relevant to Nova\u0027s last call, which is not guaranteed to be true. Instead, we should allow Ironic to error *then* fetch the error and return if appropriate. \n\nIf someone picks this up, feel free to reach out to me and I\u0027ll be glad to review.","commit_id":"bf17dbbfc09803b8eb4d68f0b48dd675e3a51163"}],"nova/virt/ironic/driver.py":[{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"dfcc4b8696c1c852d75c974d91a40d24d81ba1f2","unresolved":false,"context_lines":[{"line_number":1134,"context_line":"                timer \u003d loopingcall.FixedIntervalLoopingCall("},{"line_number":1135,"context_line":"                    self._wait_for_power_state, instance, \u0027soft power off\u0027,"},{"line_number":1136,"context_line":"                    exception.InstancePowerOffFailure)"},{"line_number":1137,"context_line":"                timer.start(interval\u003dCONF.ironic.api_retry_interval).wait()"},{"line_number":1138,"context_line":"                LOG.info(\u0027Successfully soft powered off Ironic node %s\u0027,"},{"line_number":1139,"context_line":"                         node.uuid, instance\u003dinstance)"},{"line_number":1140,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f1a1f67_c113feb7","line":1137,"updated":"2017-07-21 16:34:24.000000000","message":"There is no guaranty that node gets to the state want.\nI mean you\u0027re removing check: node.power_state \u003d\u003d ironic_states.POWER_OFF, is it not important?","commit_id":"39055cc5992b476030de264210eb8c98bc22dbb7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"7cbbd421d7b5db3735a773aaa9fe9c587e67c9c6","unresolved":false,"context_lines":[{"line_number":1134,"context_line":"                timer \u003d loopingcall.FixedIntervalLoopingCall("},{"line_number":1135,"context_line":"                    self._wait_for_power_state, instance, \u0027soft power off\u0027,"},{"line_number":1136,"context_line":"                    exception.InstancePowerOffFailure)"},{"line_number":1137,"context_line":"                timer.start(interval\u003dCONF.ironic.api_retry_interval).wait()"},{"line_number":1138,"context_line":"                LOG.info(\u0027Successfully soft powered off Ironic node %s\u0027,"},{"line_number":1139,"context_line":"                         node.uuid, instance\u003dinstance)"},{"line_number":1140,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_6b6f4692","line":1137,"in_reply_to":"1f1a1f67_c113feb7","updated":"2017-11-22 11:45:27.000000000","message":"What he said. Surely we still want to assert that we\u0027re getting to the expected state and not just NOSTATE? Perhaps ironic guarantees that we can only get to one of two states, but you should probably note that here","commit_id":"39055cc5992b476030de264210eb8c98bc22dbb7"},{"author":{"_account_id":13689,"name":"Hironori Shiina","email":"Hironori.Shiina@fujitsu.com","username":"shiina"},"change_message_id":"9b44a49e94b0c42f7111921e7574e6e7fe3175c3","unresolved":false,"context_lines":[{"line_number":1134,"context_line":"                timer \u003d loopingcall.FixedIntervalLoopingCall("},{"line_number":1135,"context_line":"                    self._wait_for_power_state, instance, \u0027soft power off\u0027,"},{"line_number":1136,"context_line":"                    exception.InstancePowerOffFailure)"},{"line_number":1137,"context_line":"                timer.start(interval\u003dCONF.ironic.api_retry_interval).wait()"},{"line_number":1138,"context_line":"                LOG.info(\u0027Successfully soft powered off Ironic node %s\u0027,"},{"line_number":1139,"context_line":"                         node.uuid, instance\u003dinstance)"},{"line_number":1140,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_8277b6a0","line":1137,"in_reply_to":"ff82abbf_6b6f4692","updated":"2017-11-27 10:43:31.000000000","message":"Sorry for the late reply. The last error is always set when the power action gets failed. I added a note.","commit_id":"39055cc5992b476030de264210eb8c98bc22dbb7"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"a6b22012eb869777cc07638c562b7e105ade1aad","unresolved":false,"context_lines":[{"line_number":485,"context_line":"            # NOTE(hshiina): The last error is set when ironic fails to change"},{"line_number":486,"context_line":"            # the power state. Otherwise, the power state is successfully"},{"line_number":487,"context_line":"            # changed."},{"line_number":488,"context_line":"            if node.last_error:"},{"line_number":489,"context_line":"                LOG.error("},{"line_number":490,"context_line":"                    \u0027Failed to set power state to %(state)s for ironic node \u0027"},{"line_number":491,"context_line":"                    \u0027%(node)s: %(error)s\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_e1363852","line":488,"range":{"start_line":488,"start_character":0,"end_line":488,"end_character":31},"updated":"2018-09-11 17:20:35.000000000","message":"When I did read this code, I had a question why we need to put this check in the above \"ironic_states.NOSTATE\" condition.\nAnd I got the reason from the commit message:\n\n After requesting a power action, Ironic virt driver waits\n for a target state of the node to get None, which means the\n action finishes.\n\nSo how about adding it a little bit for easy understanding and maintenance in long-term here?","commit_id":"bf17dbbfc09803b8eb4d68f0b48dd675e3a51163"}]}