)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"e58ee42534e2502ac0100baa21b835a5b957a51c","unresolved":false,"context_lines":[{"line_number":7,"context_line":"agent: add soft_reboot_on_deploy option"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When using an IPA based agent deployment I\u0027ve found that some"},{"line_number":10,"context_line":"baremetal machines (with buggy BIOS) don\u0027t like the new"},{"line_number":11,"context_line":"soft shutdown we perform during deployment. Specifically"},{"line_number":12,"context_line":"these machines don\u0027t start back up after a soft powerdown"},{"line_number":13,"context_line":"via the agent, even though they are configured to power on"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"fa80f949_d5a2154b","line":10,"updated":"2015-11-05 07:05:25.000000000","message":"Also maybe bad/broken hardware.","commit_id":"52eb7ad0e6c01ff5faddbb8216ec930c2e8566c8"}],"doc/source/deploy/drivers.rst":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"db7521cf127649fc1b8521f3511b6f4a51e44e72","unresolved":false,"context_lines":[{"line_number":22,"context_line":"  :maxdepth: 1"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"  ../drivers/base"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"DRAC"},{"line_number":28,"context_line":"----"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5aef4532_c93444fd","line":25,"updated":"2016-03-11 11:01:25.000000000","message":"Not sure I agree with this documentation, we have \"Base driver\" is not actually a usable driver? It\u0027s an implementation detail.","commit_id":"203ed100061a4c83a0bba091ae3a33ba52493cec"}],"ironic/drivers/base.py":[{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"e1433e92e55e6e64bbb04f53e6923aee2b468eff","unresolved":false,"context_lines":[{"line_number":499,"context_line":""},{"line_number":500,"context_line":"        return driver_info"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    def get_enable_soft_reboot(self, task):"},{"line_number":503,"context_line":"        \"\"\"Does this node support soft rebooting via an OS agent."},{"line_number":504,"context_line":""},{"line_number":505,"context_line":"        Hook to allow individual power drivers to enable attempts to"}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_ee00c4ac","line":502,"updated":"2015-11-09 16:24:36.000000000","message":"This public method used only for agent_* drivers, so is not good to modify base interface. Proper implementation of  https://review.openstack.org/#/c/186700/ should be here IMO. Now is better add a new driver parameter only for agent driver.","commit_id":"f447544899c3d6c6ee836fb7a2e6bcb7bd79a319"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"873d541942c6363268626203823ccdef8a802388","unresolved":false,"context_lines":[{"line_number":499,"context_line":""},{"line_number":500,"context_line":"        return driver_info"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    def get_enable_soft_reboot(self, task):"},{"line_number":503,"context_line":"        \"\"\"Does this node support soft rebooting via an OS agent."},{"line_number":504,"context_line":""},{"line_number":505,"context_line":"        Hook to allow individual power drivers to enable attempts to"}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_7426cbcf","line":502,"in_reply_to":"da85f559_b2f45e87","updated":"2015-11-10 06:46:59.000000000","message":"Yes, you are right, agent and pxe with IPA (not legacy) ramdisk. \"I consider this a critical issues\" - not sure, probably bad/broken hardware. But you try to extend driver\u0027s power interface w/o spec. I\u0027m not a spec lover, so no score.","commit_id":"f447544899c3d6c6ee836fb7a2e6bcb7bd79a319"},{"author":{"_account_id":360,"name":"Dan Prince","email":"dprince@redhat.com","username":"dan-prince"},"change_message_id":"8367295a6279a24520d1f722daa6245cdfd79917","unresolved":false,"context_lines":[{"line_number":499,"context_line":""},{"line_number":500,"context_line":"        return driver_info"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    def get_enable_soft_reboot(self, task):"},{"line_number":503,"context_line":"        \"\"\"Does this node support soft rebooting via an OS agent."},{"line_number":504,"context_line":""},{"line_number":505,"context_line":"        Hook to allow individual power drivers to enable attempts to"}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_b2f45e87","line":502,"in_reply_to":"da85f559_ee00c4ac","updated":"2015-11-09 17:53:58.000000000","message":"Is there a power interface that applies only to agent_* drivers? Any agent_* or pxe* driver that use IPA would potentially hit this code path because they share agent_base_vendor.py. Note that I\u0027m using pxe_iboot and several machines in my dev environment are broken without a fix for this bug.\n\nFair enough on pointing out the spec to improve the power interface with regards to capabilities. That could work... and I\u0027d rather see us not add temporary interfaces to Ironic to address this bug if we don\u0027t have to. Yuriy: You did ask me to add this to node.driver_info in patchset 1 which is what I\u0027ve done here though...\n\nI consider this a critical issues (it effects 2 of my local dev/test machines). The legacy ramdisk is deprecated, and the new IPA drivers don\u0027t have defaults that make them drop-in/upgrade friendly. Perhaps disabling soft reboot attempts globally would make sense (I don\u0027t think that functionally hurts anything as it is basically what the previous ramdisk did... simply putting an fsync into the IPA agent should resolve concerns there)\n\nOr perhaps we could agree on improving just the power driver interface and quickly address that in Ironic ahead of the rest of this spec?","commit_id":"f447544899c3d6c6ee836fb7a2e6bcb7bd79a319"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"70f1d26a9b850037fec36dde773e3c1edcb6e2a7","unresolved":false,"context_lines":[{"line_number":496,"context_line":"        node_info \u003d node.driver_info or {}"},{"line_number":497,"context_line":"        driver_info \u003d {}"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":"        enable_soft_reboot \u003d node_info.get(\u0027enable_soft_reboot\u0027, True)"},{"line_number":500,"context_line":"        try:"},{"line_number":501,"context_line":"            driver_info[\u0027enable_soft_reboot\u0027] \u003d ("},{"line_number":502,"context_line":"                strutils.bool_from_string(enable_soft_reboot, strict\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a710552_0f9533e3","line":499,"updated":"2015-12-15 01:00:01.000000000","message":"properties available in a node\u0027s driver_info are meant to be available to the user via GET /v1/drivers/properties [1]. This new \u0027enable_soft_reboot\u0027 would need to be hooked into that too.\n\nhttp://docs.openstack.org/developer/ironic/webapi/v1.html#drivers","commit_id":"203ed100061a4c83a0bba091ae3a33ba52493cec"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"70f1d26a9b850037fec36dde773e3c1edcb6e2a7","unresolved":false,"context_lines":[{"line_number":503,"context_line":"                                          default\u003dTrue))"},{"line_number":504,"context_line":"        except ValueError:"},{"line_number":505,"context_line":"            raise exception.InvalidParameterValue("},{"line_number":506,"context_line":"                _(\"enable_soft_reboot must be a boolean.\"))"},{"line_number":507,"context_line":""},{"line_number":508,"context_line":"        return driver_info"},{"line_number":509,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5a710552_8f54c354","line":506,"updated":"2015-12-15 01:00:01.000000000","message":"s/boolean/Boolean/","commit_id":"203ed100061a4c83a0bba091ae3a33ba52493cec"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"26396bca45280aa33d866d12c98f5111dadff832","unresolved":false,"context_lines":[{"line_number":507,"context_line":""},{"line_number":508,"context_line":"        return driver_info"},{"line_number":509,"context_line":""},{"line_number":510,"context_line":"    def get_enable_soft_reboot(self, task):"},{"line_number":511,"context_line":"        \"\"\"Does this node support soft rebooting via an OS agent."},{"line_number":512,"context_line":""},{"line_number":513,"context_line":"        Hook to allow individual power drivers to enable attempts to"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a710552_8fd123fe","line":510,"updated":"2015-12-15 00:54:13.000000000","message":"I\u0027d actually prefer if it was more like \u0027enable_soft_reboot\u0027 returns True if soft rebooting is enabled, False otherwise. the \u0027get_\u0027 part doesn\u0027t seem to do much for me. And then code using it might look like \u0027if enable_soft_reboot()\u0027 vs \u0027if get_enable_soft_reboot()\u0027.","commit_id":"203ed100061a4c83a0bba091ae3a33ba52493cec"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"26396bca45280aa33d866d12c98f5111dadff832","unresolved":false,"context_lines":[{"line_number":511,"context_line":"        \"\"\"Does this node support soft rebooting via an OS agent."},{"line_number":512,"context_line":""},{"line_number":513,"context_line":"        Hook to allow individual power drivers to enable attempts to"},{"line_number":514,"context_line":"        soft reboot via the Ironic agent (IPA)."},{"line_number":515,"context_line":""},{"line_number":516,"context_line":"        :returns: boolean True if this node should attempt a soft reboot."},{"line_number":517,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"5a710552_efb467d2","line":514,"updated":"2015-12-15 00:54:13.000000000","message":"Hmm. Anything added to the PowerInterface needs to be generic. Does it need to be specific to IPA? Also, what is the default behaviour? And is it backwards compatible?","commit_id":"203ed100061a4c83a0bba091ae3a33ba52493cec"}],"ironic/drivers/modules/agent_base_vendor.py":[{"author":{"_account_id":14810,"name":"Kan","email":"sofaopenstack@gmail.com","username":"Kan"},"change_message_id":"6516822b189367385a8ebc9a1e6610b413b70796","unresolved":false,"context_lines":[{"line_number":54,"context_line":"               help\u003d_(\u0027Amount of time (in seconds) to wait between polling \u0027"},{"line_number":55,"context_line":"                      \u0027power state after trigger soft poweroff.\u0027)),"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    cfg.BoolOpt(\u0027soft_reboot_on_deploy\u0027, default\u003dTrue,"},{"line_number":58,"context_line":"                help\u003d_(\u0027whether to attempt a soft reboot via the agent \u0027"},{"line_number":59,"context_line":"                       \u0027when finishing a deployment.\u0027)),"},{"line_number":60,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa80f949_44f8b149","line":57,"updated":"2015-11-05 10:17:36.000000000","message":"nit: please put the \u0027default\u003dTrue,\u0027 on the next line to be consistent.","commit_id":"52eb7ad0e6c01ff5faddbb8216ec930c2e8566c8"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"583e0290ef72093657f6096422f9ae87ebab47e8","unresolved":false,"context_lines":[{"line_number":597,"context_line":""},{"line_number":598,"context_line":"        try:"},{"line_number":599,"context_line":"            try:"},{"line_number":600,"context_line":"                if CONF.agent.soft_reboot_on_deploy:"},{"line_number":601,"context_line":"                    self._client.power_off(node)"},{"line_number":602,"context_line":"                    _wait_until_powered_off(task)"},{"line_number":603,"context_line":"            except Exception as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa80f949_75f9e97f","line":600,"updated":"2015-11-05 06:55:33.000000000","message":"I think this is dangerous option, because soft off guarantees that data were written on the disk. Therefore it should not be applied to all nodes. For mixed hardware environment better add new property to node.driver_info. User can enable it only on some nodes.","commit_id":"52eb7ad0e6c01ff5faddbb8216ec930c2e8566c8"},{"author":{"_account_id":14810,"name":"Kan","email":"sofaopenstack@gmail.com","username":"Kan"},"change_message_id":"6516822b189367385a8ebc9a1e6610b413b70796","unresolved":false,"context_lines":[{"line_number":597,"context_line":""},{"line_number":598,"context_line":"        try:"},{"line_number":599,"context_line":"            try:"},{"line_number":600,"context_line":"                if CONF.agent.soft_reboot_on_deploy:"},{"line_number":601,"context_line":"                    self._client.power_off(node)"},{"line_number":602,"context_line":"                    _wait_until_powered_off(task)"},{"line_number":603,"context_line":"            except Exception as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa80f949_a4d15dcf","line":600,"in_reply_to":"fa80f949_75f9e97f","updated":"2015-11-05 10:17:36.000000000","message":"+1","commit_id":"52eb7ad0e6c01ff5faddbb8216ec930c2e8566c8"},{"author":{"_account_id":360,"name":"Dan Prince","email":"dprince@redhat.com","username":"dan-prince"},"change_message_id":"267b31b9616dd41538998c0a2bbdafd2becb25c6","unresolved":false,"context_lines":[{"line_number":597,"context_line":""},{"line_number":598,"context_line":"        try:"},{"line_number":599,"context_line":"            try:"},{"line_number":600,"context_line":"                if CONF.agent.soft_reboot_on_deploy:"},{"line_number":601,"context_line":"                    self._client.power_off(node)"},{"line_number":602,"context_line":"                    _wait_until_powered_off(task)"},{"line_number":603,"context_line":"            except Exception as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa80f949_4d58ce32","line":600,"in_reply_to":"fa80f949_a4d15dcf","updated":"2015-11-05 13:15:23.000000000","message":"Not sure I agree this is dangerous. As someone who uses external power supplies I find this reboot to be a bit of a time waster because the _wait_until_powered_off will always hit the time out in such cases anyway (because there is now way for the power controller to query the state of the host). If we are really worried about data loss the think to do would be fsync (or something) in IPA before the provisioning command notifies Ironic it is complete... not rely on this reboot to do that for us.\n\nThat said, I do agree adding this to node.driver_info is probably a cleaner per node solution and probably works better for external power supplies anyways (some of which may even want to set this by default)","commit_id":"52eb7ad0e6c01ff5faddbb8216ec930c2e8566c8"}]}