)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"24ca599c28adaec441881f63770ee6f93af3d9e7","unresolved":false,"context_lines":[{"line_number":18,"context_line":"interface. This method calls boot.prepare_instance() by default as"},{"line_number":19,"context_line":"AgentRescue.unrescue() used to do. \u0027direct\u0027 deploy interface sets the"},{"line_number":20,"context_line":"boot device to \u0027disk\u0027 for a whole disk image in this method because"},{"line_number":21,"context_line":"the boot device may be changed to boot a rescue ramdisk at rescuing."},{"line_number":22,"context_line":"\u0027ansible\u0027 deploy interface always sets the  boot device to \u0027disk\u0027."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Closes-Bug: 1750958"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"df7087c5_d1b75f3a","line":21,"range":{"start_line":21,"start_character":16,"end_line":21,"end_character":30},"updated":"2018-03-07 11:29:02.000000000","message":"sorry, I don\u0027t really follow. prepare_instance looks at the boot option in this logic https://github.com/openstack/ironic/blob/8f2e487b2bd9168822268fcca1ab43ff93084afe/ironic/drivers/modules/pxe.py#L647-L653\n\nDoes the rescuing process change the boot option? If so, we need to change it to restore the original on unrescue","commit_id":"0862bd95bb75df9d34d9a3a79f4ac99a39f57c34"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"ffb5d3a63083194c19e60282dd9632b1b32eada3","unresolved":false,"context_lines":[{"line_number":18,"context_line":"interface. This method calls boot.prepare_instance() by default as"},{"line_number":19,"context_line":"AgentRescue.unrescue() used to do. \u0027direct\u0027 deploy interface sets the"},{"line_number":20,"context_line":"boot device to \u0027disk\u0027 for a whole disk image in this method because"},{"line_number":21,"context_line":"the boot device may be changed to boot a rescue ramdisk at rescuing."},{"line_number":22,"context_line":"\u0027ansible\u0027 deploy interface always sets the  boot device to \u0027disk\u0027."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Closes-Bug: 1750958"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"df7087c5_7188d31e","line":21,"range":{"start_line":21,"start_character":16,"end_line":21,"end_character":30},"in_reply_to":"df7087c5_d1b75f3a","updated":"2018-03-07 11:44:33.000000000","message":"ah, I see, the logic there is pretty convoluted, indeed on deploy finish this happens in the reboot_to_instance. We really need to consider refactoring at some point.","commit_id":"0862bd95bb75df9d34d9a3a79f4ac99a39f57c34"}],"ironic/drivers/base.py":[{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"8b1240220b49b42ebbcb41c51fe817e0ece33aa8","unresolved":false,"context_lines":[{"line_number":427,"context_line":"                    \u0027the driver %(driver)s does not support heartbeating\u0027,"},{"line_number":428,"context_line":"                    {\u0027node\u0027: task.node.uuid, \u0027driver\u0027: task.node.driver})"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"    def prepare_instance_to_unrescue(self, task):"},{"line_number":431,"context_line":"        \"\"\"Prepare the boot of instance for unrescue."},{"line_number":432,"context_line":""},{"line_number":433,"context_line":"        :param task: a TaskManager instance containing the node to act on."}],"source_content_type":"text/x-python","patch_set":3,"id":"ff6b8bd7_5c211f9f","line":430,"range":{"start_line":430,"start_character":4,"end_line":430,"end_character":49},"updated":"2018-03-06 04:58:05.000000000","message":"Need to add a tescase for this in https://github.com/openstack/ironic/blob/master/ironic/tests/unit/drivers/test_base.py","commit_id":"5f941d84c6bd9588b73c6c7559100919c42035a5"},{"author":{"_account_id":13689,"name":"Hironori Shiina","email":"Hironori.Shiina@fujitsu.com","username":"shiina"},"change_message_id":"4e352a960ce4b02ab6ca74c29d1c83db9ea24dbb","unresolved":false,"context_lines":[{"line_number":427,"context_line":"                    \u0027the driver %(driver)s does not support heartbeating\u0027,"},{"line_number":428,"context_line":"                    {\u0027node\u0027: task.node.uuid, \u0027driver\u0027: task.node.driver})"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"    def prepare_instance_to_unrescue(self, task):"},{"line_number":431,"context_line":"        \"\"\"Prepare the boot of instance for unrescue."},{"line_number":432,"context_line":""},{"line_number":433,"context_line":"        :param task: a TaskManager instance containing the node to act on."}],"source_content_type":"text/x-python","patch_set":3,"id":"df7087c5_c27c78b7","line":430,"range":{"start_line":430,"start_character":4,"end_line":430,"end_character":49},"in_reply_to":"ff6b8bd7_5c211f9f","updated":"2018-03-07 02:45:07.000000000","message":"Done","commit_id":"5f941d84c6bd9588b73c6c7559100919c42035a5"}],"ironic/drivers/modules/pxe.py":[{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"d56a73de678b0357bef99008ae5b93535f87282a","unresolved":false,"context_lines":[{"line_number":607,"context_line":"                iscsi_boot\u003dTrue)"},{"line_number":608,"context_line":"            boot_device \u003d boot_devices.PXE"},{"line_number":609,"context_line":""},{"line_number":610,"context_line":"        elif boot_option !\u003d \"local\" and not iwdi:"},{"line_number":611,"context_line":"            if task.driver.storage.should_write_image(task):"},{"line_number":612,"context_line":"                # Make sure that the instance kernel/ramdisk is cached."},{"line_number":613,"context_line":"                # This is for the takeover scenario for active nodes."}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa0c359_340b327d","line":610,"range":{"start_line":610,"start_character":36,"end_line":610,"end_character":48},"updated":"2018-02-26 15:12:08.000000000","message":"Even for whole disk image, we boot it using PXE by chain loading the bootloaders. Only if the boot_option is set to \u0027local\u0027; disk is used as boot device for whole disk images.\nhttps://github.com/openstack/ironic/blob/master/ironic/drivers/modules/pxe_config.template#L14-L16\nhttps://github.com/openstack/ironic/blob/master/ironic/drivers/modules/ipxe_config.template#L38-L39\nhttps://github.com/openstack/ironic/blob/master/ironic/drivers/modules/pxe_grub_config.template#L15-L17","commit_id":"70f7e213e3a2a8ba2e601f1c98be8282f60e4c9a"},{"author":{"_account_id":13689,"name":"Hironori Shiina","email":"Hironori.Shiina@fujitsu.com","username":"shiina"},"change_message_id":"8c90f6b87ec055190ce8e66256977b0360c5f74f","unresolved":false,"context_lines":[{"line_number":607,"context_line":"                iscsi_boot\u003dTrue)"},{"line_number":608,"context_line":"            boot_device \u003d boot_devices.PXE"},{"line_number":609,"context_line":""},{"line_number":610,"context_line":"        elif boot_option !\u003d \"local\" and not iwdi:"},{"line_number":611,"context_line":"            if task.driver.storage.should_write_image(task):"},{"line_number":612,"context_line":"                # Make sure that the instance kernel/ramdisk is cached."},{"line_number":613,"context_line":"                # This is for the takeover scenario for active nodes."}],"source_content_type":"text/x-python","patch_set":2,"id":"1f9dbf25_9b534be3","line":610,"range":{"start_line":610,"start_character":36,"end_line":610,"end_character":48},"in_reply_to":"3fa0c359_2b9f4c02","updated":"2018-02-27 10:37:20.000000000","message":"Thank you for the correction. I overlooked iscsi deploy. It seems necessary to change the behavior for unrescue depending on a deploy interface. I don\u0027t think we can easily change agent deploy to use PXE for whole disk images. It would break environments with the neutron network interface because a node is not connected to a provisioning network at unrescue.","commit_id":"70f7e213e3a2a8ba2e601f1c98be8282f60e4c9a"},{"author":{"_account_id":13689,"name":"Hironori Shiina","email":"Hironori.Shiina@fujitsu.com","username":"shiina"},"change_message_id":"8bce1d8e98ec9089c41ed848b10c339aa6a0f6ab","unresolved":false,"context_lines":[{"line_number":607,"context_line":"                iscsi_boot\u003dTrue)"},{"line_number":608,"context_line":"            boot_device \u003d boot_devices.PXE"},{"line_number":609,"context_line":""},{"line_number":610,"context_line":"        elif boot_option !\u003d \"local\" and not iwdi:"},{"line_number":611,"context_line":"            if task.driver.storage.should_write_image(task):"},{"line_number":612,"context_line":"                # Make sure that the instance kernel/ramdisk is cached."},{"line_number":613,"context_line":"                # This is for the takeover scenario for active nodes."}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa0c359_a1ff03a9","line":610,"range":{"start_line":610,"start_character":36,"end_line":610,"end_character":48},"in_reply_to":"3fa0c359_340b327d","updated":"2018-02-26 16:24:32.000000000","message":"Actually, we don\u0027t use PXE for a whole disk image[1]. Because \u0027root_uuid_or_disk_id\u0027 is stored only if a user image is not a whole disk image[2], this method cannot configure PXE with whole disk image if requested.\n\n[1] https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/agent.py#L291\n[2] https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/agent.py#L284","commit_id":"70f7e213e3a2a8ba2e601f1c98be8282f60e4c9a"},{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"42c4cfc37022fb5981ed862c0ce14731fc82855a","unresolved":false,"context_lines":[{"line_number":607,"context_line":"                iscsi_boot\u003dTrue)"},{"line_number":608,"context_line":"            boot_device \u003d boot_devices.PXE"},{"line_number":609,"context_line":""},{"line_number":610,"context_line":"        elif boot_option !\u003d \"local\" and not iwdi:"},{"line_number":611,"context_line":"            if task.driver.storage.should_write_image(task):"},{"line_number":612,"context_line":"                # Make sure that the instance kernel/ramdisk is cached."},{"line_number":613,"context_line":"                # This is for the takeover scenario for active nodes."}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa0c359_2b9f4c02","line":610,"range":{"start_line":610,"start_character":36,"end_line":610,"end_character":48},"in_reply_to":"3fa0c359_a1ff03a9","updated":"2018-02-27 03:51:49.000000000","message":"This is true when \u0027agent\u0027 deploy interface is used. If one were to use \u0027iscsi\u0027 deploy interface, we do save disk_uuid and perform PXE boot if boot_option is not \u0027local\u0027; please see [1] and [2]. This is a difference in behavior between two deploy interfaces. This patch [3] is attempting to provide a common behavior to these two deploy interfaces.\n\n[1] https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/iscsi_deploy.py#L255-L257\n[2] https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/iscsi_deploy.py#L332-L341\n[3] https://review.openstack.org/#/c/499050","commit_id":"70f7e213e3a2a8ba2e601f1c98be8282f60e4c9a"}],"ironic/tests/unit/drivers/modules/test_iscsi_deploy.py":[{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"8b1240220b49b42ebbcb41c51fe817e0ece33aa8","unresolved":false,"context_lines":[{"line_number":925,"context_line":"            reboot_and_finish_deploy_mock.assert_called_once_with("},{"line_number":926,"context_line":"                task.driver.deploy, task)"},{"line_number":927,"context_line":""},{"line_number":928,"context_line":"    @mock.patch.object(pxe.PXEBoot, \u0027prepare_instance\u0027, autospec\u003dTrue)"},{"line_number":929,"context_line":"    def test_prepare_instance_to_unrescue(self, mock_prepare):"},{"line_number":930,"context_line":"        with task_manager.acquire(self.context, self.node.uuid) as task:"},{"line_number":931,"context_line":"            task.driver.deploy.prepare_instance_to_unrescue(task)"},{"line_number":932,"context_line":"            mock_prepare.assert_called_once_with(task.driver.boot, task)"},{"line_number":933,"context_line":""},{"line_number":934,"context_line":""},{"line_number":935,"context_line":"# Cleanup of iscsi_deploy with pxe boot interface"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff6b8bd7_9c27b7b7","line":932,"range":{"start_line":928,"start_character":4,"end_line":932,"end_character":72},"updated":"2018-03-06 04:58:05.000000000","message":"I think this testcase is not required as iscsi deploy interface do not explicitly defines prepare_instance_to_unrescue(). Having a testcase in https://github.com/openstack/ironic/blob/master/ironic/tests/unit/drivers/test_base.py should suffice.","commit_id":"5f941d84c6bd9588b73c6c7559100919c42035a5"},{"author":{"_account_id":13689,"name":"Hironori Shiina","email":"Hironori.Shiina@fujitsu.com","username":"shiina"},"change_message_id":"4e352a960ce4b02ab6ca74c29d1c83db9ea24dbb","unresolved":false,"context_lines":[{"line_number":925,"context_line":"            reboot_and_finish_deploy_mock.assert_called_once_with("},{"line_number":926,"context_line":"                task.driver.deploy, task)"},{"line_number":927,"context_line":""},{"line_number":928,"context_line":"    @mock.patch.object(pxe.PXEBoot, \u0027prepare_instance\u0027, autospec\u003dTrue)"},{"line_number":929,"context_line":"    def test_prepare_instance_to_unrescue(self, mock_prepare):"},{"line_number":930,"context_line":"        with task_manager.acquire(self.context, self.node.uuid) as task:"},{"line_number":931,"context_line":"            task.driver.deploy.prepare_instance_to_unrescue(task)"},{"line_number":932,"context_line":"            mock_prepare.assert_called_once_with(task.driver.boot, task)"},{"line_number":933,"context_line":""},{"line_number":934,"context_line":""},{"line_number":935,"context_line":"# Cleanup of iscsi_deploy with pxe boot interface"}],"source_content_type":"text/x-python","patch_set":3,"id":"df7087c5_e275fccd","line":932,"range":{"start_line":928,"start_character":4,"end_line":932,"end_character":72},"in_reply_to":"ff6b8bd7_9c27b7b7","updated":"2018-03-07 02:45:07.000000000","message":"Done","commit_id":"5f941d84c6bd9588b73c6c7559100919c42035a5"}]}