)]}'
{"ironic/drivers/modules/ilo/boot.py":[{"author":{"_account_id":13719,"name":"Naohiro Tamura","email":"naohirot.openstack@gmail.com","username":"nao"},"change_message_id":"1dd61125eb60fedb6e0d5362024217649cba0fef","unresolved":false,"context_lines":[{"line_number":298,"context_line":"        # treat this as a no-op and move on otherwise we would modify"},{"line_number":299,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":300,"context_line":"        if (node.provision_state !\u003d states.DEPLOYING and"},{"line_number":301,"context_line":"                node.provision_state !\u003d states.CLEANING):"},{"line_number":302,"context_line":"            return"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        # Clear ilo_boot_iso if it\u0027s a glance image to force recreate"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_5bd040f6","line":301,"range":{"start_line":301,"start_character":16,"end_line":301,"end_character":56},"updated":"2016-04-20 10:26:53.000000000","message":"Shivanand,\n\nDoesn\u0027t ilo currently boot from vmedia for \u0027inspecting\u0027?","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"fb79eb3b1192f0fc25b99ae1b63b376df95ee4cb","unresolved":false,"context_lines":[{"line_number":298,"context_line":"        # treat this as a no-op and move on otherwise we would modify"},{"line_number":299,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":300,"context_line":"        if (node.provision_state !\u003d states.DEPLOYING and"},{"line_number":301,"context_line":"                node.provision_state !\u003d states.CLEANING):"},{"line_number":302,"context_line":"            return"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        # Clear ilo_boot_iso if it\u0027s a glance image to force recreate"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_6ac27aea","line":301,"range":{"start_line":301,"start_character":16,"end_line":301,"end_character":56},"in_reply_to":"1a122d0e_5bd040f6","updated":"2016-04-20 14:21:04.000000000","message":"iLO drivers performs only OOB inspection. But there are plans to extend it to perform in-band inspection later.","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":13719,"name":"Naohiro Tamura","email":"naohirot.openstack@gmail.com","username":"nao"},"change_message_id":"eaca1532b7caea1febc92725bce22166e1790478","unresolved":false,"context_lines":[{"line_number":298,"context_line":"        # treat this as a no-op and move on otherwise we would modify"},{"line_number":299,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":300,"context_line":"        if (node.provision_state !\u003d states.DEPLOYING and"},{"line_number":301,"context_line":"                node.provision_state !\u003d states.CLEANING):"},{"line_number":302,"context_line":"            return"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        # Clear ilo_boot_iso if it\u0027s a glance image to force recreate"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_7b050e44","line":301,"range":{"start_line":301,"start_character":16,"end_line":301,"end_character":56},"in_reply_to":"1a122d0e_6ac27aea","updated":"2016-04-21 01:49:48.000000000","message":"I understood.","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"}],"ironic/drivers/modules/irmc/boot.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"795ed8bb6fef349752067f447a9cdd1735f4366c","unresolved":false,"context_lines":[{"line_number":604,"context_line":"        \"\"\""},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        # NOTE(TheJulia): If this method is being called by something"},{"line_number":607,"context_line":"        # aside from a deployment, such as conductor takeover, we should"},{"line_number":608,"context_line":"        # treat this as a no-op and move on otherwise we would modify"},{"line_number":609,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":610,"context_line":"        if (task.node.provision_state !\u003d states.DEPLOYING and"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_a20800db","line":607,"range":{"start_line":607,"start_character":23,"end_line":607,"end_character":33},"updated":"2016-04-21 14:00:16.000000000","message":"nit: mention cleaning as part of this comment","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":15074,"name":"Aparna","email":"aparnavtce@gmail.com","username":"Aparna"},"change_message_id":"533608c28605cf85b632e2a44551e32b81642d39","unresolved":false,"context_lines":[{"line_number":604,"context_line":"        \"\"\""},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        # NOTE(TheJulia): If this method is being called by something"},{"line_number":607,"context_line":"        # aside from a deployment, such as conductor takeover, we should"},{"line_number":608,"context_line":"        # treat this as a no-op and move on otherwise we would modify"},{"line_number":609,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":610,"context_line":"        if (task.node.provision_state !\u003d states.DEPLOYING and"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_b1b62ada","line":607,"range":{"start_line":607,"start_character":23,"end_line":607,"end_character":33},"in_reply_to":"1a122d0e_a20800db","updated":"2016-05-09 09:21:54.000000000","message":"I will fix it in next patch","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":13719,"name":"Naohiro Tamura","email":"naohirot.openstack@gmail.com","username":"nao"},"change_message_id":"1dd61125eb60fedb6e0d5362024217649cba0fef","unresolved":false,"context_lines":[{"line_number":603,"context_line":"        :raises: IRMCOperationError, if some operation on iRMC fails."},{"line_number":604,"context_line":"        \"\"\""},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        # NOTE(TheJulia): If this method is being called by something"},{"line_number":607,"context_line":"        # aside from a deployment, such as conductor takeover, we should"},{"line_number":608,"context_line":"        # treat this as a no-op and move on otherwise we would modify"},{"line_number":609,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":610,"context_line":"        if (task.node.provision_state !\u003d states.DEPLOYING and"},{"line_number":611,"context_line":"                task.node.provision_state !\u003d states.CLEANING):"},{"line_number":612,"context_line":"            return"},{"line_number":613,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_3ba3c401","line":610,"range":{"start_line":606,"start_character":0,"end_line":610,"end_character":61},"updated":"2016-04-20 10:26:53.000000000","message":"This fix seems Okay.\nBut I wondered why this bug happened because I tested cleaning and confirmed that it worked.\n\nThen I noticed that L.606-610 has been merged very recently by the patch https://review.openstack.org/#/c/300674/.\n\nI think vmedia mount is necessary not only deploy and cleaning, but also inspect and rescue later.\n\nSo putting this condition for the conductor fail-over in each driver is not appropriate, I think.\n\nWhat do you think, Julia and Shivanand?\n\nBest regards,\nNaohiro","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":13719,"name":"Naohiro Tamura","email":"naohirot.openstack@gmail.com","username":"nao"},"change_message_id":"eaca1532b7caea1febc92725bce22166e1790478","unresolved":false,"context_lines":[{"line_number":603,"context_line":"        :raises: IRMCOperationError, if some operation on iRMC fails."},{"line_number":604,"context_line":"        \"\"\""},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        # NOTE(TheJulia): If this method is being called by something"},{"line_number":607,"context_line":"        # aside from a deployment, such as conductor takeover, we should"},{"line_number":608,"context_line":"        # treat this as a no-op and move on otherwise we would modify"},{"line_number":609,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":610,"context_line":"        if (task.node.provision_state !\u003d states.DEPLOYING and"},{"line_number":611,"context_line":"                task.node.provision_state !\u003d states.CLEANING):"},{"line_number":612,"context_line":"            return"},{"line_number":613,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_7b286e9a","line":610,"range":{"start_line":606,"start_character":0,"end_line":610,"end_character":61},"in_reply_to":"1a122d0e_2a01a206","updated":"2016-04-21 01:49:48.000000000","message":"Shivanand, Julia,\n\nDo you mean we move the check to each vendor\u0027s library such as proliantutils or python-scciclinet?\n\nCan we change the check condition to something like the following pseudo code?\n\nif task !\u003d takeover:","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"fb79eb3b1192f0fc25b99ae1b63b376df95ee4cb","unresolved":false,"context_lines":[{"line_number":603,"context_line":"        :raises: IRMCOperationError, if some operation on iRMC fails."},{"line_number":604,"context_line":"        \"\"\""},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        # NOTE(TheJulia): If this method is being called by something"},{"line_number":607,"context_line":"        # aside from a deployment, such as conductor takeover, we should"},{"line_number":608,"context_line":"        # treat this as a no-op and move on otherwise we would modify"},{"line_number":609,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":610,"context_line":"        if (task.node.provision_state !\u003d states.DEPLOYING and"},{"line_number":611,"context_line":"                task.node.provision_state !\u003d states.CLEANING):"},{"line_number":612,"context_line":"            return"},{"line_number":613,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_2a01a206","line":610,"range":{"start_line":606,"start_character":0,"end_line":610,"end_character":61},"in_reply_to":"1a122d0e_3ba3c401","updated":"2016-04-20 14:21:04.000000000","message":"May be we can extend this check to include the states where in mounting of virtual media is required as and when required.","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":15074,"name":"Aparna","email":"aparnavtce@gmail.com","username":"Aparna"},"change_message_id":"7f59373b5df3fb46fb3115fb22a7287f50f3096d","unresolved":false,"context_lines":[{"line_number":603,"context_line":"        :raises: IRMCOperationError, if some operation on iRMC fails."},{"line_number":604,"context_line":"        \"\"\""},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        # NOTE(TheJulia): If this method is being called by something"},{"line_number":607,"context_line":"        # aside from a deployment, such as conductor takeover, we should"},{"line_number":608,"context_line":"        # treat this as a no-op and move on otherwise we would modify"},{"line_number":609,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":610,"context_line":"        if (task.node.provision_state !\u003d states.DEPLOYING and"},{"line_number":611,"context_line":"                task.node.provision_state !\u003d states.CLEANING):"},{"line_number":612,"context_line":"            return"},{"line_number":613,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_21ce3e2e","line":610,"range":{"start_line":606,"start_character":0,"end_line":610,"end_character":61},"in_reply_to":"1a122d0e_4c093fdd","updated":"2016-05-10 05:47:55.000000000","message":"I think checking for \u0027node take over\u0027 seems like complex process. There won\u0027t be much checking needed other than deploying, cleaning and also inspection once it is added.","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":13719,"name":"Naohiro Tamura","email":"naohirot.openstack@gmail.com","username":"nao"},"change_message_id":"e2f32a1d2dff68ed34edc09fb039547eca1faeb2","unresolved":false,"context_lines":[{"line_number":603,"context_line":"        :raises: IRMCOperationError, if some operation on iRMC fails."},{"line_number":604,"context_line":"        \"\"\""},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        # NOTE(TheJulia): If this method is being called by something"},{"line_number":607,"context_line":"        # aside from a deployment, such as conductor takeover, we should"},{"line_number":608,"context_line":"        # treat this as a no-op and move on otherwise we would modify"},{"line_number":609,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":610,"context_line":"        if (task.node.provision_state !\u003d states.DEPLOYING and"},{"line_number":611,"context_line":"                task.node.provision_state !\u003d states.CLEANING):"},{"line_number":612,"context_line":"            return"},{"line_number":613,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_4c093fdd","line":610,"range":{"start_line":606,"start_character":0,"end_line":610,"end_character":61},"in_reply_to":"1a122d0e_6251c8d8","updated":"2016-04-22 05:07:32.000000000","message":"Lucas,\n\nThanks for the comment, I agree that we should not access private field.\n\nBut my point is that the condition should not check \u0027node.provision_state\u0027, therefore I presented that there is at least one alternative exit.\n\nIf we can agree, the matter of accessing private field could be avoided easily, as we all know :-), by the code like this:\n\n if task.get_purpose() \u003d\u003d \u0027node take over\u0027:\n    return\n\n or\n\n if task.is_take_over():\n   return\n\nI assumed to add the public method, get_purpose() or is_take_over() in the TaskManager class.\n\nBest regards,\nNaohiro","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"795ed8bb6fef349752067f447a9cdd1735f4366c","unresolved":false,"context_lines":[{"line_number":603,"context_line":"        :raises: IRMCOperationError, if some operation on iRMC fails."},{"line_number":604,"context_line":"        \"\"\""},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        # NOTE(TheJulia): If this method is being called by something"},{"line_number":607,"context_line":"        # aside from a deployment, such as conductor takeover, we should"},{"line_number":608,"context_line":"        # treat this as a no-op and move on otherwise we would modify"},{"line_number":609,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":610,"context_line":"        if (task.node.provision_state !\u003d states.DEPLOYING and"},{"line_number":611,"context_line":"                task.node.provision_state !\u003d states.CLEANING):"},{"line_number":612,"context_line":"            return"},{"line_number":613,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_6251c8d8","line":610,"range":{"start_line":606,"start_character":0,"end_line":610,"end_character":61},"in_reply_to":"1a122d0e_71288f9a","updated":"2016-04-21 14:00:16.000000000","message":"@Naohiro attributes prefixed with \u0027_\u0027 are considered \"internal\" we should not rely on them outside the objects context like that. So -1 for this check","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":13719,"name":"Naohiro Tamura","email":"naohirot.openstack@gmail.com","username":"nao"},"change_message_id":"aced7b0041cc05049c797dcd5651177c4429400c","unresolved":false,"context_lines":[{"line_number":603,"context_line":"        :raises: IRMCOperationError, if some operation on iRMC fails."},{"line_number":604,"context_line":"        \"\"\""},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        # NOTE(TheJulia): If this method is being called by something"},{"line_number":607,"context_line":"        # aside from a deployment, such as conductor takeover, we should"},{"line_number":608,"context_line":"        # treat this as a no-op and move on otherwise we would modify"},{"line_number":609,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":610,"context_line":"        if (task.node.provision_state !\u003d states.DEPLOYING and"},{"line_number":611,"context_line":"                task.node.provision_state !\u003d states.CLEANING):"},{"line_number":612,"context_line":"            return"},{"line_number":613,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_71288f9a","line":610,"range":{"start_line":606,"start_character":0,"end_line":610,"end_character":61},"in_reply_to":"1a122d0e_7b286e9a","updated":"2016-04-21 03:07:33.000000000","message":"At least, we can write like this:\n\nif task._purpose \u003d\u003d \u0027node take over\u0027:\n   return","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":13719,"name":"Naohiro Tamura","email":"naohirot.openstack@gmail.com","username":"nao"},"change_message_id":"76364224e5f46b5d1f1ad0ee7958a4dce2f7d2c2","unresolved":false,"context_lines":[{"line_number":603,"context_line":"        :raises: IRMCOperationError, if some operation on iRMC fails."},{"line_number":604,"context_line":"        \"\"\""},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        # NOTE(TheJulia): If this method is being called by something"},{"line_number":607,"context_line":"        # aside from a deployment, such as conductor takeover, we should"},{"line_number":608,"context_line":"        # treat this as a no-op and move on otherwise we would modify"},{"line_number":609,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":610,"context_line":"        if (task.node.provision_state !\u003d states.DEPLOYING and"},{"line_number":611,"context_line":"                task.node.provision_state !\u003d states.CLEANING):"},{"line_number":612,"context_line":"            return"},{"line_number":613,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9abb7d3a_f664c3b9","line":610,"range":{"start_line":606,"start_character":0,"end_line":610,"end_character":61},"in_reply_to":"9abb7d3a_d0b3a3e7","updated":"2016-06-01 05:32:53.000000000","message":"Thanks for the comment, Julia. I hope you are doing well now.\n\nGenerally speaking, yes, I understood what you meant, the current\nimplementation needs some re-factoring.\n\njulia\u003e That being said, I _think_ each driver should have some knowledge of\njulia\u003e the cases of when, and when it will work, and the duality of\njulia\u003e prepare_ramdisk makes this difficult, especially when vmedia\njulia\u003e operations are in use. \n\nHowever in this particular case, isn\u0027t the following quoted my\nunderstanding right?\nMore specifically, does PXEBoot process need to prepare boot by\ncalling prepare_ramdisk() as opposed to iLO/iRMC during the conductor\ntakeover?\n\nnaohiro\u003e You are right. But PXEBoot process doesn\u0027t need to prepare boot by\nnaohiro\u003e calling prepare_ramdisk() as iLO/iRMC doesn\u0027t need to prepare boot.\n\nBest regards,\nNaohiro","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d9061dcae115d16c3ee60304d9b318b2f4625ef2","unresolved":false,"context_lines":[{"line_number":603,"context_line":"        :raises: IRMCOperationError, if some operation on iRMC fails."},{"line_number":604,"context_line":"        \"\"\""},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        # NOTE(TheJulia): If this method is being called by something"},{"line_number":607,"context_line":"        # aside from a deployment, such as conductor takeover, we should"},{"line_number":608,"context_line":"        # treat this as a no-op and move on otherwise we would modify"},{"line_number":609,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":610,"context_line":"        if (task.node.provision_state !\u003d states.DEPLOYING and"},{"line_number":611,"context_line":"                task.node.provision_state !\u003d states.CLEANING):"},{"line_number":612,"context_line":"            return"},{"line_number":613,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9abb7d3a_b66692b8","line":610,"range":{"start_line":606,"start_character":0,"end_line":610,"end_character":61},"in_reply_to":"9abb7d3a_f664c3b9","updated":"2016-06-01 17:26:54.000000000","message":"I\u0027m doing well, Thanks :)\n\nAs I presently understand the driver model, and the model and the existing river interaction, it does, especially depending on the driver\u0027s capabilities and what settings a user has defined, because one could be netbooting.\n\nUltimately though, a driver could opt to carry their own prepare() method to meet exactly what their capabilities are, although most are just relying upon the base drivers, hence some of the necessity of the route taken. :(\n\nexamples, although via agent: https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/agent.py#L317\n\nThe only other option that I see is to build a capabilities interface to inform the agent/iscsi driver code of what is available from the driver, but that is not a here and no thing, and ultimately we should fix this bug for the moment then work on trying to better address necessity/need of making calls depending on configurations/capabilities.","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"a76c0b2885d4e7e17cb1c79e4a09af725870e1e3","unresolved":false,"context_lines":[{"line_number":603,"context_line":"        :raises: IRMCOperationError, if some operation on iRMC fails."},{"line_number":604,"context_line":"        \"\"\""},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        # NOTE(TheJulia): If this method is being called by something"},{"line_number":607,"context_line":"        # aside from a deployment, such as conductor takeover, we should"},{"line_number":608,"context_line":"        # treat this as a no-op and move on otherwise we would modify"},{"line_number":609,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":610,"context_line":"        if (task.node.provision_state !\u003d states.DEPLOYING and"},{"line_number":611,"context_line":"                task.node.provision_state !\u003d states.CLEANING):"},{"line_number":612,"context_line":"            return"},{"line_number":613,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9abb7d3a_d0b3a3e7","line":610,"range":{"start_line":606,"start_character":0,"end_line":610,"end_character":61},"in_reply_to":"dab17558_0ca49805","updated":"2016-05-31 15:37:59.000000000","message":"Sorry for my absence.  Summit and medical things.\n\nWe have created a bit of a tangled weave of things, and perhaps it might be time to re-think how the conductor interacts with drivers some, since so much of that interaction is presently built around the use of ramdisks and PXE which is truthfully how we\u0027ve gotten to this point.  But that is likely another conversation, for another time.\n\nThat being said, I _think_ each driver should have some knowledge of the cases of when, and when it will work, and the duality of prepare_ramdisk makes this difficult, especially when vmedia operations are in use.  \n\nSo the choice, as I see it is have it at the driver level, or similar logic in the agent and iscsi drivers driver layers, however potentially those will only be invoked if the driver only directly leverages the parent driver code, and does not implement a portion of the interface code on it\u0027s own.  This leads me to really think that at the driver level, as proposed, is the best place because the driver and it\u0027s maintainers should ideally know the driver\u0027s capabilities better than the conductor.  Hopefully that makes sense.","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":13719,"name":"Naohiro Tamura","email":"naohirot.openstack@gmail.com","username":"nao"},"change_message_id":"604ad7d48295c3335a299243ab0060dfc87b644c","unresolved":false,"context_lines":[{"line_number":603,"context_line":"        :raises: IRMCOperationError, if some operation on iRMC fails."},{"line_number":604,"context_line":"        \"\"\""},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        # NOTE(TheJulia): If this method is being called by something"},{"line_number":607,"context_line":"        # aside from a deployment, such as conductor takeover, we should"},{"line_number":608,"context_line":"        # treat this as a no-op and move on otherwise we would modify"},{"line_number":609,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":610,"context_line":"        if (task.node.provision_state !\u003d states.DEPLOYING and"},{"line_number":611,"context_line":"                task.node.provision_state !\u003d states.CLEANING):"},{"line_number":612,"context_line":"            return"},{"line_number":613,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_c3c29b33","line":610,"range":{"start_line":606,"start_character":0,"end_line":610,"end_character":61},"in_reply_to":"dab17558_21ce3e2e","updated":"2016-05-11 01:36:02.000000000","message":"Aparna, Lucus, Shivanand, Julia,\n\nThanks for the comment.\nBasically I think this code should be put in the caller side of prepare_ramdisk().\n\nCurrently prepare_ramdisk() is implemented by only three drivers, pxe.py, ilo/boot.py and irmc/boot.py.\n\nThe caller of prepare_ramdisk() must be here:\nhttps://github.com/openstack/ironic/blob/master/ironic/drivers/modules/iscsi_deploy.py#L640\n\nIf vmedia should not be attached during the takeover task,\nprepare_ramdisk() of PXEBoot also should not be called as well, I believe.\nhttps://github.com/openstack/ironic/blob/master/ironic/drivers/modules/pxe.py#L388\n\nIf so, can we move the state check code to iscsi_deploy.py#L640?\n\nBest regards,\nNaohiro","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":13719,"name":"Naohiro Tamura","email":"naohirot.openstack@gmail.com","username":"nao"},"change_message_id":"30b4d5ea73097c90e71de0a3fb695d76d1ea1aa8","unresolved":false,"context_lines":[{"line_number":603,"context_line":"        :raises: IRMCOperationError, if some operation on iRMC fails."},{"line_number":604,"context_line":"        \"\"\""},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        # NOTE(TheJulia): If this method is being called by something"},{"line_number":607,"context_line":"        # aside from a deployment, such as conductor takeover, we should"},{"line_number":608,"context_line":"        # treat this as a no-op and move on otherwise we would modify"},{"line_number":609,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":610,"context_line":"        if (task.node.provision_state !\u003d states.DEPLOYING and"},{"line_number":611,"context_line":"                task.node.provision_state !\u003d states.CLEANING):"},{"line_number":612,"context_line":"            return"},{"line_number":613,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_0ca49805","line":610,"range":{"start_line":606,"start_character":0,"end_line":610,"end_character":61},"in_reply_to":"dab17558_3e59d935","updated":"2016-05-12 01:04:43.000000000","message":"Thanks for the feedback, Aparna!\n\n\u003e PXEBoot process doesn\u0027t do vmedia attach.\n\nYou are right. But PXEBoot process doesn\u0027t need to prepare boot by\ncalling prepare_ramdisk() as iLO/iRMC doesn\u0027t need to prepare boot.\n\n\u003e I think adding the code to the caller side won\u0027t be effective.\n\u003e So Isn\u0027t it better to keep the code in prepare_ramdisk() of\n\u003e ilo/boot.py and irmc/boot.py instead of adding the checking wherever\n\u003e the prepare_ramdisk() is being called?.\n\nIf we don\u0027t need to boot at all, both pxe boot and vmedia boot, during\ntakeover task, it seems that it would be more effective to put\nthe state check code in the caller side, because:\n1) we can remove the duplicated state check code. This improves\nmaintainability.\n2) we can stop calling unnecessary prepare_ramdisk() for PXEBoot. This\nimproves consistency.\n\nWDYT, Aparna, Lucus, Shivanand, Julia?","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":15074,"name":"Aparna","email":"aparnavtce@gmail.com","username":"Aparna"},"change_message_id":"5f9182cee7177ae9c9c2db1551d7055f4a979972","unresolved":false,"context_lines":[{"line_number":603,"context_line":"        :raises: IRMCOperationError, if some operation on iRMC fails."},{"line_number":604,"context_line":"        \"\"\""},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        # NOTE(TheJulia): If this method is being called by something"},{"line_number":607,"context_line":"        # aside from a deployment, such as conductor takeover, we should"},{"line_number":608,"context_line":"        # treat this as a no-op and move on otherwise we would modify"},{"line_number":609,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":610,"context_line":"        if (task.node.provision_state !\u003d states.DEPLOYING and"},{"line_number":611,"context_line":"                task.node.provision_state !\u003d states.CLEANING):"},{"line_number":612,"context_line":"            return"},{"line_number":613,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_3e59d935","line":610,"range":{"start_line":606,"start_character":0,"end_line":610,"end_character":61},"in_reply_to":"dab17558_c3c29b33","updated":"2016-05-11 16:11:20.000000000","message":"Thanks Naohiro for your comment.\n\nPXEBoot process doesn\u0027t do vmedia attach. I think adding the code to the caller side won\u0027t be effective. So Isn\u0027t it better to keep the code in prepare_ramdisk() of ilo/boot.py and irmc/boot.py instead of adding the checking wherever the prepare_ramdisk() is being called?.","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":13719,"name":"Naohiro Tamura","email":"naohirot.openstack@gmail.com","username":"nao"},"change_message_id":"1930bda22625994dff1b749d418d4c1849464037","unresolved":false,"context_lines":[{"line_number":603,"context_line":"        :raises: IRMCOperationError, if some operation on iRMC fails."},{"line_number":604,"context_line":"        \"\"\""},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        # NOTE(TheJulia): If this method is being called by something"},{"line_number":607,"context_line":"        # aside from deployment and clean, such as conductor takeover, we"},{"line_number":608,"context_line":"        # should treat this as a no-op and move on otherwise we would modify"},{"line_number":609,"context_line":"        # the state of the node due to virtual media operations."},{"line_number":610,"context_line":"        if (task.node.provision_state !\u003d states.DEPLOYING and"}],"source_content_type":"text/x-python","patch_set":5,"id":"9abb7d3a_82bdf103","line":607,"range":{"start_line":606,"start_character":26,"end_line":607,"end_character":69},"updated":"2016-06-02 06:48:55.000000000","message":"Julia,\n\nThanks for the detailed reply to the patch set 3, let me put my comment here because I have a basic question to the premise of this issue, whether or not the takeover happens besides states.ACTIVE.\n\nIt seems the comment L.606-607 is not right, because this method \"prepare_ramdisk()\" is not called when the conductor takeover started if I didn\u0027t overlook something important.\n\nPlease look at the code below, takeover is skipped when \"node.provision_state !\u003d states.ACTIVE\", so it happens only when \"node.provision_state \u003d\u003d states.ACTIVE\".\n\nhttps://github.com/openstack/ironic/blob/master/ironic/conductor/manager.py#L1396-L1397\n\nAnd then when \"node.provision_state \u003d\u003d states.ACTIVE\", this method \"prepare_ramdisk()\" is not called .\n\nhttps://github.com/openstack/ironic/blob/master/ironic/drivers/modules/iscsi_deploy.py#L629-L640\n\nSo regarding the bug https://bugs.launchpad.net/ironic/+bug/1565104\n\n\u003e Bug Description:\n\u003e Take over calls task.driver.deploy.prepare(). With drivers that\n\u003e utilize virtual media, the deployment ISO is presently asserted and\n\u003e attached to the virtual media interface of the node. When this is\n\u003e performed as part of a takeover, this is an incorrect action these\n\u003e virtual media steps are limited to the deployment steps of a node.\n\nThis bug description seems not right.\n\nWDYT?\n\nBest regards,\nNaohiro","commit_id":"b12d184b73f6decb20f733ae642a54ae49e90b88"}],"ironic/tests/unit/drivers/modules/irmc/test_boot.py":[{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"a5c8eb66c9348f70554db6de7efce576aab493f0","unresolved":false,"context_lines":[{"line_number":938,"context_line":"    @mock.patch.object(irmc_boot, \u0027_setup_deploy_iso\u0027, spec_set\u003dTrue,"},{"line_number":939,"context_line":"                       autospec\u003dTrue)"},{"line_number":940,"context_line":"    def test_prepare_ramdisk_not_deploying_not_cleaning(self, mock_is_image):"},{"line_number":941,"context_line":"        \"\"\"Ensure ramdisk build operations are blocked when not deploying\"\"\""},{"line_number":942,"context_line":""},{"line_number":943,"context_line":"        for state in states.STABLE_STATES:"},{"line_number":944,"context_line":"            mock_is_image.reset_mock()"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_d4abccd4","line":941,"range":{"start_line":941,"start_character":55,"end_line":941,"end_character":73},"updated":"2016-04-19 04:40:31.000000000","message":"nit: Please update this comment accordingly.","commit_id":"046c574205863e2915403f1fcd2c67dc5fd2f482"},{"author":{"_account_id":15074,"name":"Aparna","email":"aparnavtce@gmail.com","username":"Aparna"},"change_message_id":"e3d840ad12f9947dd015596890cbf22a60c098e7","unresolved":false,"context_lines":[{"line_number":938,"context_line":"    @mock.patch.object(irmc_boot, \u0027_setup_deploy_iso\u0027, spec_set\u003dTrue,"},{"line_number":939,"context_line":"                       autospec\u003dTrue)"},{"line_number":940,"context_line":"    def test_prepare_ramdisk_not_deploying_not_cleaning(self, mock_is_image):"},{"line_number":941,"context_line":"        \"\"\"Ensure ramdisk build operations are blocked when not deploying\"\"\""},{"line_number":942,"context_line":""},{"line_number":943,"context_line":"        for state in states.STABLE_STATES:"},{"line_number":944,"context_line":"            mock_is_image.reset_mock()"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_388f7376","line":941,"range":{"start_line":941,"start_character":55,"end_line":941,"end_character":73},"in_reply_to":"1a122d0e_d4abccd4","updated":"2016-04-19 15:50:20.000000000","message":"Done","commit_id":"046c574205863e2915403f1fcd2c67dc5fd2f482"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"795ed8bb6fef349752067f447a9cdd1735f4366c","unresolved":false,"context_lines":[{"line_number":927,"context_line":"        self.node.provision_state \u003d states.DEPLOYING"},{"line_number":928,"context_line":"        self.node.save()"},{"line_number":929,"context_line":"        self._test_prepare_ramdisk()"},{"line_number":930,"context_line":"        self.node.refresh()"},{"line_number":931,"context_line":""},{"line_number":932,"context_line":"    def test_prepare_ramdisk_glance_image_cleaning(self):"},{"line_number":933,"context_line":"        self.node.provision_state \u003d states.CLEANING"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_0825935b","line":930,"updated":"2016-04-21 14:00:16.000000000","message":"refresh() not needed","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":15074,"name":"Aparna","email":"aparnavtce@gmail.com","username":"Aparna"},"change_message_id":"533608c28605cf85b632e2a44551e32b81642d39","unresolved":false,"context_lines":[{"line_number":927,"context_line":"        self.node.provision_state \u003d states.DEPLOYING"},{"line_number":928,"context_line":"        self.node.save()"},{"line_number":929,"context_line":"        self._test_prepare_ramdisk()"},{"line_number":930,"context_line":"        self.node.refresh()"},{"line_number":931,"context_line":""},{"line_number":932,"context_line":"    def test_prepare_ramdisk_glance_image_cleaning(self):"},{"line_number":933,"context_line":"        self.node.provision_state \u003d states.CLEANING"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_31d23a01","line":930,"in_reply_to":"1a122d0e_0825935b","updated":"2016-05-09 09:21:54.000000000","message":"I will fix it in next patch","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"795ed8bb6fef349752067f447a9cdd1735f4366c","unresolved":false,"context_lines":[{"line_number":933,"context_line":"        self.node.provision_state \u003d states.CLEANING"},{"line_number":934,"context_line":"        self.node.save()"},{"line_number":935,"context_line":"        self._test_prepare_ramdisk()"},{"line_number":936,"context_line":"        self.node.refresh()"},{"line_number":937,"context_line":""},{"line_number":938,"context_line":"    @mock.patch.object(irmc_boot, \u0027_setup_deploy_iso\u0027, spec_set\u003dTrue,"},{"line_number":939,"context_line":"                       autospec\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_281ccf12","line":936,"updated":"2016-04-21 14:00:16.000000000","message":"ditto","commit_id":"a741fa4e2a8de4a8001d821345906eda4c541427"}],"releasenotes/notes/bug-1570283-6cdc62e4ef43cb02.yaml":[{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"badc2daec769419448019d855eb5f9db167f4ddc","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  -Fixes the issue of not performing deployment vmedia operations"},{"line_number":4,"context_line":"   during cleaning."}],"source_content_type":"text/x-yaml","patch_set":4,"id":"dab17558_54ce952e","line":3,"range":{"start_line":3,"start_character":21,"end_line":3,"end_character":65},"updated":"2016-05-10 09:01:33.000000000","message":"This sentence looks bit confusing. May be you mean \u0027not attaching virtual media during cleaning operation for vmedia based drivers.\u0027","commit_id":"3ba81257248a93ca5d1b6379da5007166efad588"}]}