)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1fa48ba051dc7bccb38949f718136ae666f41f89","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1bc539ca_cc50a2b9","updated":"2022-12-16 14:03:14.000000000","message":"if the compute service is down hard reboot cannot be used i dont have time to review this curretly full but im not sure this is the correct approach","commit_id":"8f04f6b8a3aed8d8ea46699eabdd48a29e5e9852"},{"author":{"_account_id":29037,"name":"Pierre-Samuel Le Stang","email":"pierre-samuel.le-stang@ovhcloud.com","username":"pslestang"},"change_message_id":"6ba277f08b46094792d3fc00ca2360e6f84c30b9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a93cd779_afb59542","updated":"2022-12-15 23:25:17.000000000","message":"recheck","commit_id":"8f04f6b8a3aed8d8ea46699eabdd48a29e5e9852"},{"author":{"_account_id":29037,"name":"Pierre-Samuel Le Stang","email":"pierre-samuel.le-stang@ovhcloud.com","username":"pslestang"},"change_message_id":"6223ff075cbb1a887d045a0b66d1d2f4fb6e8f71","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b0a7ec78_1e03bc1c","in_reply_to":"1bc539ca_cc50a2b9","updated":"2022-12-16 15:34:22.000000000","message":"We don\u0027t have the compute service status in real time so it\u0027s hard du rely on it\n\nREBOOTING_HARD is also a transient status so it makes sense to handle this status with other transient status.","commit_id":"8f04f6b8a3aed8d8ea46699eabdd48a29e5e9852"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"329e449256883c2fb0304b6cd4755386d8cb9bff","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"18cff9fd_3318bc8d","updated":"2023-01-03 18:34:41.000000000","message":"im not actully back until thursday but just to calrify my concern with this patch was oginginally that it was papering over a deeper error by fixing the sideeffect fo something that should have been prevented before it got in the broken state.\n\nso like dan i think \nhttps://review.opendev.org/c/openstack/nova/+/867832/2/nova/compute/manager.py#1186 is proably fine.\n\nim less sure about https://review.opendev.org/c/openstack/nova/+/867832/2/nova/compute/manager.py#1317\n\n\nthe other thing that is in the back of my mind is if the service was down we invoke hard reboot as an rpc Call i belive not a cast? i will have to check that when im back. if so the oslo_messaging should be setting the mandaotry delivery flag on the amqp message which will ensure the rpc is correctly enqueded in teh rpc queue.\n\nthat however only ensures that the message is placed in the queue not that it ationed on by the compute service.\n\n\nim wondering if the api should refuse to make that rpc call if the service is down or if we should be ensuring that no change to the instnace state is done in the api.\ni will have to check but the latter seams more feaible.\n\nbasicaly i dont think we should allow the api to put the instnace in a trasitive state such as reboot pending. i will have to check the code but i would have assumed the transtation to reboot_pendign shoudl only happen in the compute manager in the compute agent. if that is the case alsready today then the actula case you are protecting against is a restart of the compute agent after the instance has entered the pending state but before it has completed. \n\n\nif the instance state is currently being modifed outside of the compute manager we likely shoudl fix that as a seperate followup  patch as that likely will  not be backportable as we are changing where that happens.\n\nanyway ill try to look into the call flow of this later in the week but that is why i was questioning the apporch orginally. it is proably valid and correct to extend the set of transitional states to include the reboot pending states and we should likely proceed with that but im not sure that is the extent of the bug.\n","commit_id":"aa0e122bf6d227b3bf26adbba3f7754b1b38a2d6"},{"author":{"_account_id":29037,"name":"Pierre-Samuel Le Stang","email":"pierre-samuel.le-stang@ovhcloud.com","username":"pslestang"},"change_message_id":"72cce09edbe606a65a6aae7a3dda367c37132899","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"904941d6_7202fbc5","updated":"2023-01-03 16:07:52.000000000","message":"recheck","commit_id":"aa0e122bf6d227b3bf26adbba3f7754b1b38a2d6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"912c32802472bdb35f86cdca3abc53bbfd4a30da","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0afdeac1_d498f8bb","in_reply_to":"18cff9fd_3318bc8d","updated":"2023-01-03 19:12:27.000000000","message":"\u003e the other thing that is in the back of my mind is if the service was down we invoke hard reboot as an rpc Call i belive not a cast? i will have to check that when im back. if so the oslo_messaging should be setting the mandaotry delivery flag on the amqp message which will ensure the rpc is correctly enqueded in teh rpc queue.\n\nIt\u0027s a cast, which means we\u0027ll queue and return. That also probably means there are more ways to get into this situation, where we cast the message after changing the task state. The cast could be delayed due to network congestion, which could result in other outages, reboots, etc. So more than just threading the needle of firing a reboot seconds before we notice the compute is down.\n \n\u003e im wondering if the api should refuse to make that rpc call if the service is down or if we should be ensuring that no change to the instnace state is done in the api.\n\u003e i will have to check but the latter seams more feaible.\n\nIt can\u0027t do this reliably, because there\u0027s at least a minute between it being down and us noticing, which I believe is the whole premise of this patch.\n\n\u003e basicaly i dont think we should allow the api to put the instnace in a trasitive state such as reboot pending. \n\nIt doesn\u0027t. The compute manager uses the pending state as sort of an ack or progress step. The API puts it into rebooting, and the compute manager moves it to reboot_pending while it actually happens.\n\n\u003e i will have to check the code but i would have assumed the transtation to reboot_pendign shoudl only happen in the compute manager in the compute agent. if that is the case alsready today then the actula case you are protecting against is a restart of the compute agent after the instance has entered the pending state but before it has completed. \n\nIt\u0027s both I think.\n\nThis is not a priority I think, since it\u0027s a very narrow window race mitigation, so I think it can wait until when you\u0027re back. That gives Pierre time to reply to my question about the second case too.","commit_id":"aa0e122bf6d227b3bf26adbba3f7754b1b38a2d6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6f42573e734c8fb00139178aa6d30684f3d625bd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"32cb4190_8539f4b3","updated":"2023-01-04 15:18:51.000000000","message":"I think some residue is left in the tests, but otherwise looks okay to me if that is removed.","commit_id":"0f4073bb333b2894dc030bcf6e9fcd21a9e4ee37"},{"author":{"_account_id":29037,"name":"Pierre-Samuel Le Stang","email":"pierre-samuel.le-stang@ovhcloud.com","username":"pslestang"},"change_message_id":"8f9daae51aad9475a968f6d7ef76e0dfd30ee469","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"25279059_cea1bcab","updated":"2023-01-05 16:32:05.000000000","message":"Finally removed PENDING states as they were caught in previous if.","commit_id":"9a9226564e497e2db4a8ad4d4a9ad3c245a21f54"},{"author":{"_account_id":11583,"name":"Arnaud Morin","email":"arnaud.morin@gmail.com","username":"arnaudmorin"},"change_message_id":"9ee392e1ec8ace6cce167d39e756d1f022ed36f8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"4e1ae6ff_996ada6a","updated":"2023-06-30 10:00:40.000000000","message":"Hey nova team, we would like this patch to move forward.\nIs there anything else we should do in your opinion?","commit_id":"8fabc1b329f312123e69f40fe03c9f8dbd4c3312"},{"author":{"_account_id":29037,"name":"Pierre-Samuel Le Stang","email":"pierre-samuel.le-stang@ovhcloud.com","username":"pslestang"},"change_message_id":"207909e0617f80e8d33e69883958eaad2d94ea31","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"bc3142cb_ecc484c8","updated":"2023-02-23 08:19:56.000000000","message":"recheck","commit_id":"8fabc1b329f312123e69f40fe03c9f8dbd4c3312"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"240e6373820ac681ead0d4ebc7bb4b8ca257fdbe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"e3d48b52_ae392ecf","updated":"2023-07-01 22:39:14.000000000","message":"Thanks for updating that!\n\nUnfortunately I found another issue which is that the test helper method is always mocking power_state to be RUNNING, so the coverage for SHUTDOWN isn\u0027t actually happening. So that just needs to be fixed. It\u0027s possible that replacing the mock with using instance.power_state will cause other unrelated tests to fail, so it\u0027s up to you whether you want to adjust the helper method and keep using it or cover SHUTDOWN some other way.","commit_id":"549a9583c77d3289c2b7fd7e0f210456b1c71159"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"249d3c25ea8279cb12e71a8ad1ce49e8164d1204","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"ce2724fd_794675a3","updated":"2023-08-04 23:44:26.000000000","message":"The changes make sense and the code is clean, and thank you for including helpful code comments throughout. LGTM and thanks for your patience","commit_id":"22183cd9e55b1df30c16d28ada50f515fcadeb57"},{"author":{"_account_id":11583,"name":"Arnaud Morin","email":"arnaud.morin@gmail.com","username":"arnaudmorin"},"change_message_id":"d2d2bb2cc8669ade9a7f8193a29d82be381e6322","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"982e4b1f_758b449b","updated":"2023-07-04 21:32:30.000000000","message":"While fixing the instance power_state in the tests, I figured out that the SHUTDOWN case was already taken into account by the try_reboot if case.\n\nI added a comment about the if statement to notify about this.\n\nThe full logic may need a rewrite, but that\u0027s not the purpose of this change.","commit_id":"22183cd9e55b1df30c16d28ada50f515fcadeb57"},{"author":{"_account_id":11583,"name":"Arnaud Morin","email":"arnaud.morin@gmail.com","username":"arnaudmorin"},"change_message_id":"52bc945804c3d47c918a048029c082e2006bef4f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"32cb3857_f74b0a11","updated":"2024-02-05 14:59:21.000000000","message":"recheck","commit_id":"22183cd9e55b1df30c16d28ada50f515fcadeb57"},{"author":{"_account_id":11583,"name":"Arnaud Morin","email":"arnaud.morin@gmail.com","username":"arnaudmorin"},"change_message_id":"2f03ead76361cb7be06255a1a31f6ce7b0a874fb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"85cfa622_f08970cf","updated":"2023-07-05 09:18:27.000000000","message":"recheck nova-next","commit_id":"22183cd9e55b1df30c16d28ada50f515fcadeb57"},{"author":{"_account_id":11583,"name":"Arnaud Morin","email":"arnaud.morin@gmail.com","username":"arnaudmorin"},"change_message_id":"c94b531e0d68b1b6ba0ed760237007f50367e6e7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"99c018ba_85f67f44","updated":"2024-02-05 19:33:58.000000000","message":"recheck nova-ovs-hybrid-plug","commit_id":"22183cd9e55b1df30c16d28ada50f515fcadeb57"},{"author":{"_account_id":29037,"name":"Pierre-Samuel Le Stang","email":"pierre-samuel.le-stang@ovhcloud.com","username":"pslestang"},"change_message_id":"a936f9c0a72c1f43932d576688dc2f0f7cb248c8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"b14afa41_593c5dd8","updated":"2024-02-06 08:05:45.000000000","message":"recheck nova-ovs-hybrid-plug","commit_id":"22183cd9e55b1df30c16d28ada50f515fcadeb57"},{"author":{"_account_id":11583,"name":"Arnaud Morin","email":"arnaud.morin@gmail.com","username":"arnaudmorin"},"change_message_id":"ed837d9898abd93eca6dcfa76c65a804763f5280","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"fbd3a098_ac1413e4","updated":"2024-02-05 19:33:27.000000000","message":"recheck ova-ovs-hybrid-plug","commit_id":"22183cd9e55b1df30c16d28ada50f515fcadeb57"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e4c8a70f339ebb8cbc938279ecb85f42b70fb72e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"928e42eb_ff03ee14","updated":"2024-03-19 23:12:02.000000000","message":"Still looks good, just had a manual rebase presumably","commit_id":"aa3e8fef7b949ec3ddb3c4eaa348eb004593d29e"},{"author":{"_account_id":29037,"name":"Pierre-Samuel Le Stang","email":"pierre-samuel.le-stang@ovhcloud.com","username":"pslestang"},"change_message_id":"1a2a7169ffbb9fd1e5b1bf5c71f4503aeee181e9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"184e33fe_d6b6d83a","updated":"2024-03-19 15:18:56.000000000","message":"recheck nova-ovs-hybrid-plug","commit_id":"aa3e8fef7b949ec3ddb3c4eaa348eb004593d29e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2f7c251ec3ebfd89ad29ed1fd7f15f532a09faa4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"cd8444ca_17c2157a","updated":"2024-03-20 11:35:27.000000000","message":"this has not changed matiarilly since my previous review (just rebases) and mel is still happy so readding +2w","commit_id":"aa3e8fef7b949ec3ddb3c4eaa348eb004593d29e"}],"nova/compute/manager.py":[{"author":{"_account_id":11583,"name":"Arnaud Morin","email":"arnaud.morin@gmail.com","username":"arnaudmorin"},"change_message_id":"0fbeae217f63b6986b5ce7afca1b2e466e1f0c89","unresolved":true,"context_lines":[{"line_number":1308,"context_line":"        if expect_running and CONF.resume_guests_state_on_host_boot:"},{"line_number":1309,"context_line":"            self._resume_guests_state(context, instance, net_info)"},{"line_number":1310,"context_line":"            # If resume failed the vm_state is ERROR and we can\u0027t expect"},{"line_number":1311,"context_line":"            # the instance tu run with a vm_state to ERROR"},{"line_number":1312,"context_line":"            if instance.vm_state !\u003d vm_states.ERROR:"},{"line_number":1313,"context_line":"                instance.task_state \u003d None"},{"line_number":1314,"context_line":"                instance.save()"}],"source_content_type":"text/x-python","patch_set":1,"id":"b4d179e8_7d41904c","line":1311,"updated":"2023-01-03 10:50:46.000000000","message":"tu --\u003e to","commit_id":"8f04f6b8a3aed8d8ea46699eabdd48a29e5e9852"},{"author":{"_account_id":29037,"name":"Pierre-Samuel Le Stang","email":"pierre-samuel.le-stang@ovhcloud.com","username":"pslestang"},"change_message_id":"c1da2801f2503b47455369b6969d51c0ec9f03a4","unresolved":false,"context_lines":[{"line_number":1308,"context_line":"        if expect_running and CONF.resume_guests_state_on_host_boot:"},{"line_number":1309,"context_line":"            self._resume_guests_state(context, instance, net_info)"},{"line_number":1310,"context_line":"            # If resume failed the vm_state is ERROR and we can\u0027t expect"},{"line_number":1311,"context_line":"            # the instance tu run with a vm_state to ERROR"},{"line_number":1312,"context_line":"            if instance.vm_state !\u003d vm_states.ERROR:"},{"line_number":1313,"context_line":"                instance.task_state \u003d None"},{"line_number":1314,"context_line":"                instance.save()"}],"source_content_type":"text/x-python","patch_set":1,"id":"746747c8_8629487a","line":1311,"in_reply_to":"b4d179e8_7d41904c","updated":"2023-01-03 14:13:17.000000000","message":"Done","commit_id":"8f04f6b8a3aed8d8ea46699eabdd48a29e5e9852"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c75d80a2b7fdafce5c053a0def49595c01a733d8","unresolved":true,"context_lines":[{"line_number":1183,"context_line":"                                      task_states.REBOOTING_HARD,"},{"line_number":1184,"context_line":"                                      task_states.REBOOTING,"},{"line_number":1185,"context_line":"                                      task_states.REBOOT_PENDING,"},{"line_number":1186,"context_line":"                                      task_states.REBOOT_PENDING_HARD,"},{"line_number":1187,"context_line":"                                      task_states.PAUSING,"},{"line_number":1188,"context_line":"                                      task_states.UNPAUSING]):"},{"line_number":1189,"context_line":"            LOG.warning(\"Instance in transitional state \""}],"source_content_type":"text/x-python","patch_set":2,"id":"9f7844f6_6980094d","line":1186,"updated":"2023-01-03 16:23:48.000000000","message":"I think this probably makes sense. The task_state is really a lock on submitting new actions on the instance while another one is potentially queued in rabbit, or just getting started on the compute. We could still have something queued in rabbit when the compute is restarting (the non-pending states above) and the reboot will fail to run when it hits the compute after this, because we\u0027ll be clearing the task_state and the expected_task_states check will fail, which is good.\n\nIf the task_state is in one of these pending ones, it means it\u0027s not queued in rabbit, we started the reboot, and now *we* are restarting, so there\u0027s really nothing we can do other than reset it here I think. If the instance is running (per L1180) then it either rebooted or didn\u0027t, but resetting to active is the best we can do.","commit_id":"aa0e122bf6d227b3bf26adbba3f7754b1b38a2d6"},{"author":{"_account_id":29037,"name":"Pierre-Samuel Le Stang","email":"pierre-samuel.le-stang@ovhcloud.com","username":"pslestang"},"change_message_id":"732ea45fb9fe12cab022b51b21be02cfdc2f09fe","unresolved":false,"context_lines":[{"line_number":1183,"context_line":"                                      task_states.REBOOTING_HARD,"},{"line_number":1184,"context_line":"                                      task_states.REBOOTING,"},{"line_number":1185,"context_line":"                                      task_states.REBOOT_PENDING,"},{"line_number":1186,"context_line":"                                      task_states.REBOOT_PENDING_HARD,"},{"line_number":1187,"context_line":"                                      task_states.PAUSING,"},{"line_number":1188,"context_line":"                                      task_states.UNPAUSING]):"},{"line_number":1189,"context_line":"            LOG.warning(\"Instance in transitional state \""}],"source_content_type":"text/x-python","patch_set":2,"id":"e9529aca_ef56db77","line":1186,"in_reply_to":"9f7844f6_6980094d","updated":"2023-01-04 14:09:41.000000000","message":"Ack","commit_id":"aa0e122bf6d227b3bf26adbba3f7754b1b38a2d6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c75d80a2b7fdafce5c053a0def49595c01a733d8","unresolved":true,"context_lines":[{"line_number":1314,"context_line":"            # the instance to run with a vm_state to ERROR"},{"line_number":1315,"context_line":"            if instance.vm_state !\u003d vm_states.ERROR:"},{"line_number":1316,"context_line":"                instance.task_state \u003d None"},{"line_number":1317,"context_line":"                instance.save()"},{"line_number":1318,"context_line":""},{"line_number":1319,"context_line":"    def _resume_guests_state(self, context, instance, net_info):"},{"line_number":1320,"context_line":"        LOG.info(\u0027Rebooting instance after nova-compute restart.\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"6a1aeba7_a04a7e8e","line":1317,"updated":"2023-01-03 16:23:48.000000000","message":"I\u0027m less sure about this, and I\u0027ll need to do some more thinking on it I guess. Can you describe the sequence of events that results in this specific bit here? Your comment about failure doesn\u0027t seem to match up with what you\u0027re doing.","commit_id":"aa0e122bf6d227b3bf26adbba3f7754b1b38a2d6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6f42573e734c8fb00139178aa6d30684f3d625bd","unresolved":false,"context_lines":[{"line_number":1314,"context_line":"            # the instance to run with a vm_state to ERROR"},{"line_number":1315,"context_line":"            if instance.vm_state !\u003d vm_states.ERROR:"},{"line_number":1316,"context_line":"                instance.task_state \u003d None"},{"line_number":1317,"context_line":"                instance.save()"},{"line_number":1318,"context_line":""},{"line_number":1319,"context_line":"    def _resume_guests_state(self, context, instance, net_info):"},{"line_number":1320,"context_line":"        LOG.info(\u0027Rebooting instance after nova-compute restart.\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"727fbbf8_02a949cc","line":1317,"in_reply_to":"3065889d_9c4b2c6d","updated":"2023-01-04 15:18:51.000000000","message":"Cool, I think that\u0027s the best plan, thanks!","commit_id":"aa0e122bf6d227b3bf26adbba3f7754b1b38a2d6"},{"author":{"_account_id":29037,"name":"Pierre-Samuel Le Stang","email":"pierre-samuel.le-stang@ovhcloud.com","username":"pslestang"},"change_message_id":"732ea45fb9fe12cab022b51b21be02cfdc2f09fe","unresolved":false,"context_lines":[{"line_number":1314,"context_line":"            # the instance to run with a vm_state to ERROR"},{"line_number":1315,"context_line":"            if instance.vm_state !\u003d vm_states.ERROR:"},{"line_number":1316,"context_line":"                instance.task_state \u003d None"},{"line_number":1317,"context_line":"                instance.save()"},{"line_number":1318,"context_line":""},{"line_number":1319,"context_line":"    def _resume_guests_state(self, context, instance, net_info):"},{"line_number":1320,"context_line":"        LOG.info(\u0027Rebooting instance after nova-compute restart.\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"3065889d_9c4b2c6d","line":1317,"in_reply_to":"6a1aeba7_a04a7e8e","updated":"2023-01-04 14:09:41.000000000","message":"I spotted an other situation where the power state were desynced between the DB and the driver which was handled by this fix. As I do not remember it anymore and I\u0027m unable to find the reproducer test I will remove it from this patch and will create a new one once it come back to me 😕","commit_id":"aa0e122bf6d227b3bf26adbba3f7754b1b38a2d6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1c0e8055cfa1f4471cf0d1194ca1368ad42bef22","unresolved":true,"context_lines":[{"line_number":1152,"context_line":"            return"},{"line_number":1153,"context_line":""},{"line_number":1154,"context_line":"        current_power_state \u003d self._get_power_state(instance)"},{"line_number":1155,"context_line":"        try_reboot, reboot_type \u003d self._retry_reboot("},{"line_number":1156,"context_line":"            instance, current_power_state)"},{"line_number":1157,"context_line":""},{"line_number":1158,"context_line":"        if try_reboot:"}],"source_content_type":"text/x-python","patch_set":5,"id":"c564d15d_91c303a3","line":1155,"updated":"2023-01-12 15:03:14.000000000","message":"Okay it took me a bit, but this will return True if we\u0027re in either of the pending states, which will cause us to take L1158, and that\u0027s why we don\u0027t need the pending states added below in the new code.","commit_id":"9a9226564e497e2db4a8ad4d4a9ad3c245a21f54"},{"author":{"_account_id":11583,"name":"Arnaud Morin","email":"arnaud.morin@gmail.com","username":"arnaudmorin"},"change_message_id":"dfdfd19eddf883a71cca922e74a89917fe7b6bb7","unresolved":false,"context_lines":[{"line_number":1152,"context_line":"            return"},{"line_number":1153,"context_line":""},{"line_number":1154,"context_line":"        current_power_state \u003d self._get_power_state(instance)"},{"line_number":1155,"context_line":"        try_reboot, reboot_type \u003d self._retry_reboot("},{"line_number":1156,"context_line":"            instance, current_power_state)"},{"line_number":1157,"context_line":""},{"line_number":1158,"context_line":"        if try_reboot:"}],"source_content_type":"text/x-python","patch_set":5,"id":"24d88b24_a322da60","line":1155,"in_reply_to":"c564d15d_91c303a3","updated":"2023-01-13 08:44:42.000000000","message":"Ack","commit_id":"9a9226564e497e2db4a8ad4d4a9ad3c245a21f54"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"adf54ec1412df777f2d4080518d4ed9b449c6123","unresolved":false,"context_lines":[{"line_number":1347,"context_line":""},{"line_number":1348,"context_line":"        if pending_soft or pending_hard or started_not_running:"},{"line_number":1349,"context_line":"            retry_reboot \u003d True"},{"line_number":1350,"context_line":""},{"line_number":1351,"context_line":"        return retry_reboot, reboot_type"},{"line_number":1352,"context_line":""},{"line_number":1353,"context_line":"    def handle_lifecycle_event(self, event):"}],"source_content_type":"text/x-python","patch_set":5,"id":"fcdedbee_77dd7838","line":1350,"updated":"2023-01-12 17:18:04.000000000","message":"ok yes it will return true already in the pending state as dan noted above","commit_id":"9a9226564e497e2db4a8ad4d4a9ad3c245a21f54"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"23b6eeef306f0a44391526908158da12054bbeb5","unresolved":true,"context_lines":[{"line_number":1181,"context_line":"        # transient state when a host is brutally shutdown or rebooted while a"},{"line_number":1182,"context_line":"        # reboot/pause/unpause is scheduled on client side"},{"line_number":1183,"context_line":"        elif (current_power_state in [power_state.RUNNING,"},{"line_number":1184,"context_line":"                                      power_state.SHUTDOWN] and"},{"line_number":1185,"context_line":"              instance.task_state in [task_states.REBOOT_STARTED,"},{"line_number":1186,"context_line":"                                      task_states.REBOOT_STARTED_HARD,"},{"line_number":1187,"context_line":"                                      task_states.REBOOTING_HARD,"}],"source_content_type":"text/x-python","patch_set":8,"id":"51338441_884f9c12","line":1184,"updated":"2023-02-23 14:57:17.000000000","message":"Won\u0027t this cause us to reset the instance to ACTIVE even though it is powered off (L1198)?","commit_id":"8fabc1b329f312123e69f40fe03c9f8dbd4c3312"},{"author":{"_account_id":11583,"name":"Arnaud Morin","email":"arnaud.morin@gmail.com","username":"arnaudmorin"},"change_message_id":"ae94e18a4694d62b81deb667e2bb060b354c0e28","unresolved":false,"context_lines":[{"line_number":1181,"context_line":"        # transient state when a host is brutally shutdown or rebooted while a"},{"line_number":1182,"context_line":"        # reboot/pause/unpause is scheduled on client side"},{"line_number":1183,"context_line":"        elif (current_power_state in [power_state.RUNNING,"},{"line_number":1184,"context_line":"                                      power_state.SHUTDOWN] and"},{"line_number":1185,"context_line":"              instance.task_state in [task_states.REBOOT_STARTED,"},{"line_number":1186,"context_line":"                                      task_states.REBOOT_STARTED_HARD,"},{"line_number":1187,"context_line":"                                      task_states.REBOOTING_HARD,"}],"source_content_type":"text/x-python","patch_set":8,"id":"4ec6e95d_279352c2","line":1184,"in_reply_to":"1499fe50_ab927d78","updated":"2023-05-04 09:15:52.000000000","message":"Done","commit_id":"8fabc1b329f312123e69f40fe03c9f8dbd4c3312"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d1c662c1d990dcc63dc75337b3580af5fa0b192a","unresolved":false,"context_lines":[{"line_number":1181,"context_line":"        # transient state when a host is brutally shutdown or rebooted while a"},{"line_number":1182,"context_line":"        # reboot/pause/unpause is scheduled on client side"},{"line_number":1183,"context_line":"        elif (current_power_state in [power_state.RUNNING,"},{"line_number":1184,"context_line":"                                      power_state.SHUTDOWN] and"},{"line_number":1185,"context_line":"              instance.task_state in [task_states.REBOOT_STARTED,"},{"line_number":1186,"context_line":"                                      task_states.REBOOT_STARTED_HARD,"},{"line_number":1187,"context_line":"                                      task_states.REBOOTING_HARD,"}],"source_content_type":"text/x-python","patch_set":8,"id":"99cb7436_efbaabcf","line":1184,"in_reply_to":"4ec6e95d_279352c2","updated":"2023-06-30 16:11:17.000000000","message":"Yeah, but we don\u0027t want to set the instance to ACTIVE if it\u0027s in power state SHUTDOWN ... maybe you could just add a conditional for L1198 to only set ACTIVE if it\u0027s RUNNING?","commit_id":"8fabc1b329f312123e69f40fe03c9f8dbd4c3312"},{"author":{"_account_id":29037,"name":"Pierre-Samuel Le Stang","email":"pierre-samuel.le-stang@ovhcloud.com","username":"pslestang"},"change_message_id":"78c0afa506e5e348ba6bb16c0d42a25eb4a86377","unresolved":true,"context_lines":[{"line_number":1181,"context_line":"        # transient state when a host is brutally shutdown or rebooted while a"},{"line_number":1182,"context_line":"        # reboot/pause/unpause is scheduled on client side"},{"line_number":1183,"context_line":"        elif (current_power_state in [power_state.RUNNING,"},{"line_number":1184,"context_line":"                                      power_state.SHUTDOWN] and"},{"line_number":1185,"context_line":"              instance.task_state in [task_states.REBOOT_STARTED,"},{"line_number":1186,"context_line":"                                      task_states.REBOOT_STARTED_HARD,"},{"line_number":1187,"context_line":"                                      task_states.REBOOTING_HARD,"}],"source_content_type":"text/x-python","patch_set":8,"id":"1499fe50_ab927d78","line":1184,"in_reply_to":"51338441_884f9c12","updated":"2023-02-23 23:29:21.000000000","message":"If the instance is really powered off we are not supposed to have a transitional task_states.","commit_id":"8fabc1b329f312123e69f40fe03c9f8dbd4c3312"},{"author":{"_account_id":11583,"name":"Arnaud Morin","email":"arnaud.morin@gmail.com","username":"arnaudmorin"},"change_message_id":"35f2ef37ea4b1179673816cfc1bd2106a9343643","unresolved":false,"context_lines":[{"line_number":1181,"context_line":"        # transient state when a host is brutally shutdown or rebooted while a"},{"line_number":1182,"context_line":"        # reboot/pause/unpause is scheduled on client side"},{"line_number":1183,"context_line":"        elif (current_power_state in [power_state.RUNNING,"},{"line_number":1184,"context_line":"                                      power_state.SHUTDOWN] and"},{"line_number":1185,"context_line":"              instance.task_state in [task_states.REBOOT_STARTED,"},{"line_number":1186,"context_line":"                                      task_states.REBOOT_STARTED_HARD,"},{"line_number":1187,"context_line":"                                      task_states.REBOOTING_HARD,"}],"source_content_type":"text/x-python","patch_set":8,"id":"a361ddf5_3f706b3c","line":1184,"in_reply_to":"99cb7436_efbaabcf","updated":"2023-07-01 14:51:00.000000000","message":"done","commit_id":"8fabc1b329f312123e69f40fe03c9f8dbd4c3312"},{"author":{"_account_id":11583,"name":"Arnaud Morin","email":"arnaud.morin@gmail.com","username":"arnaudmorin"},"change_message_id":"7997df84c2b5b7677998b743780ff7f9296b7edd","unresolved":false,"context_lines":[{"line_number":1186,"context_line":"              instance.task_state in [task_states.REBOOT_STARTED,"},{"line_number":1187,"context_line":"                                      task_states.REBOOT_STARTED_HARD,"},{"line_number":1188,"context_line":"                                      task_states.REBOOTING_HARD,"},{"line_number":1189,"context_line":"                                      task_states.REBOOTING,"},{"line_number":1190,"context_line":"                                      task_states.PAUSING,"},{"line_number":1191,"context_line":"                                      task_states.UNPAUSING]):"},{"line_number":1192,"context_line":"            LOG.warning(\"Instance in transitional state \""}],"source_content_type":"text/x-python","patch_set":10,"id":"7888fedd_67c19c09","line":1189,"updated":"2023-07-04 21:35:58.000000000","message":"We only want to take care of this two REBOOTING states","commit_id":"22183cd9e55b1df30c16d28ada50f515fcadeb57"},{"author":{"_account_id":11583,"name":"Arnaud Morin","email":"arnaud.morin@gmail.com","username":"arnaudmorin"},"change_message_id":"7997df84c2b5b7677998b743780ff7f9296b7edd","unresolved":false,"context_lines":[{"line_number":1348,"context_line":"        started_not_running \u003d (current_task_state in"},{"line_number":1349,"context_line":"                               [task_states.REBOOTING,"},{"line_number":1350,"context_line":"                                task_states.REBOOTING_HARD,"},{"line_number":1351,"context_line":"                                task_states.REBOOT_STARTED,"},{"line_number":1352,"context_line":"                                task_states.REBOOT_STARTED_HARD] and"},{"line_number":1353,"context_line":"                               current_power_state !\u003d power_state.RUNNING)"},{"line_number":1354,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"c218b0e2_de1fd2f7","line":1351,"updated":"2023-07-04 21:35:58.000000000","message":"We also need to take care of REBOOTING states here, as this affect instances in SHUTDOWN power state.\nFor this specific situation, we want to try_reboot just like for other REBOOT_xyz states","commit_id":"22183cd9e55b1df30c16d28ada50f515fcadeb57"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6f42573e734c8fb00139178aa6d30684f3d625bd","unresolved":true,"context_lines":[{"line_number":1594,"context_line":"                                      power_state.SHUTDOWN)"},{"line_number":1595,"context_line":"        mock_get_inst.return_value \u003d \u0027fake-bdm\u0027"},{"line_number":1596,"context_line":"        mock_resume.side_effect \u003d test.TestingException"},{"line_number":1597,"context_line":"        with mock.patch.object(instance, \u0027save\u0027):"},{"line_number":1598,"context_line":"            self.compute._init_instance(\u0027fake-context\u0027, instance)"},{"line_number":1599,"context_line":"        mock_get_power.assert_has_calls([mock.call(instance),"},{"line_number":1600,"context_line":"                                         mock.call(instance)])"}],"source_content_type":"text/x-python","patch_set":3,"id":"fb70dea5_c7c547ac","line":1597,"updated":"2023-01-04 15:18:51.000000000","message":"Was this related to the bit of code you removed? The test passes for me with and without it, so I assume it\u0027s not needed now?","commit_id":"0f4073bb333b2894dc030bcf6e9fcd21a9e4ee37"},{"author":{"_account_id":29037,"name":"Pierre-Samuel Le Stang","email":"pierre-samuel.le-stang@ovhcloud.com","username":"pslestang"},"change_message_id":"e70c52278b03abf2a26d24b8dceeb5a105ccb5a4","unresolved":false,"context_lines":[{"line_number":1594,"context_line":"                                      power_state.SHUTDOWN)"},{"line_number":1595,"context_line":"        mock_get_inst.return_value \u003d \u0027fake-bdm\u0027"},{"line_number":1596,"context_line":"        mock_resume.side_effect \u003d test.TestingException"},{"line_number":1597,"context_line":"        with mock.patch.object(instance, \u0027save\u0027):"},{"line_number":1598,"context_line":"            self.compute._init_instance(\u0027fake-context\u0027, instance)"},{"line_number":1599,"context_line":"        mock_get_power.assert_has_calls([mock.call(instance),"},{"line_number":1600,"context_line":"                                         mock.call(instance)])"}],"source_content_type":"text/x-python","patch_set":3,"id":"2046bd00_cb31066c","line":1597,"in_reply_to":"fb70dea5_c7c547ac","updated":"2023-01-04 16:34:23.000000000","message":"nice catch!\nRemoving it","commit_id":"0f4073bb333b2894dc030bcf6e9fcd21a9e4ee37"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"240e6373820ac681ead0d4ebc7bb4b8ca257fdbe","unresolved":true,"context_lines":[{"line_number":2200,"context_line":"    def _test_init_instance_cleans_reboot_state(self, instance):"},{"line_number":2201,"context_line":"        instance.host \u003d self.compute.host"},{"line_number":2202,"context_line":"        with test.nested("},{"line_number":2203,"context_line":"            mock.patch.object(self.compute, \u0027_get_power_state\u0027,"},{"line_number":2204,"context_line":"                               return_value\u003dpower_state.RUNNING),"},{"line_number":2205,"context_line":"            mock.patch.object(instance, \u0027save\u0027, autospec\u003dTrue),"},{"line_number":2206,"context_line":"            mock.patch.object(objects.Instance, \u0027get_network_info\u0027)"},{"line_number":2207,"context_line":"          ) as ("}],"source_content_type":"text/x-python","patch_set":9,"id":"99d0bd40_f87c05f0","line":2204,"range":{"start_line":2203,"start_character":12,"end_line":2204,"end_character":65},"updated":"2023-07-01 22:39:14.000000000","message":"Hm, I didn\u0027t notice this before but this test helper method isn\u0027t actually using instance.power_state, so the tests for SHUTDOWN are not really being covered.\n\nWith the PS9 with SHUTDOWN the assert for ACTIVE should have failed with the test helper left as-is, but the tests passed.","commit_id":"549a9583c77d3289c2b7fd7e0f210456b1c71159"},{"author":{"_account_id":11583,"name":"Arnaud Morin","email":"arnaud.morin@gmail.com","username":"arnaudmorin"},"change_message_id":"d2d2bb2cc8669ade9a7f8193a29d82be381e6322","unresolved":false,"context_lines":[{"line_number":2200,"context_line":"    def _test_init_instance_cleans_reboot_state(self, instance):"},{"line_number":2201,"context_line":"        instance.host \u003d self.compute.host"},{"line_number":2202,"context_line":"        with test.nested("},{"line_number":2203,"context_line":"            mock.patch.object(self.compute, \u0027_get_power_state\u0027,"},{"line_number":2204,"context_line":"                               return_value\u003dpower_state.RUNNING),"},{"line_number":2205,"context_line":"            mock.patch.object(instance, \u0027save\u0027, autospec\u003dTrue),"},{"line_number":2206,"context_line":"            mock.patch.object(objects.Instance, \u0027get_network_info\u0027)"},{"line_number":2207,"context_line":"          ) as ("}],"source_content_type":"text/x-python","patch_set":9,"id":"a5ea194b_ead8121e","line":2204,"range":{"start_line":2203,"start_character":12,"end_line":2204,"end_character":65},"in_reply_to":"99d0bd40_f87c05f0","updated":"2023-07-04 21:32:30.000000000","message":"I fixed that by using instance.power_state","commit_id":"549a9583c77d3289c2b7fd7e0f210456b1c71159"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"249d3c25ea8279cb12e71a8ad1ce49e8164d1204","unresolved":true,"context_lines":[{"line_number":2242,"context_line":"        instance.power_state \u003d power_state.RUNNING"},{"line_number":2243,"context_line":"        self._test_init_instance_cleans_reboot_state(instance)"},{"line_number":2244,"context_line":""},{"line_number":2245,"context_line":"    def test_init_instance_cleans_shutdown_reboot_started(self):"},{"line_number":2246,"context_line":"        instance \u003d objects.Instance(self.context)"},{"line_number":2247,"context_line":"        instance.uuid \u003d uuids.instance"},{"line_number":2248,"context_line":"        instance.vm_state \u003d vm_states.STOPPED"}],"source_content_type":"text/x-python","patch_set":10,"id":"bc3756c8_0ed13a34","line":2245,"range":{"start_line":2245,"start_character":27,"end_line":2245,"end_character":33},"updated":"2023-08-04 23:44:26.000000000","message":"Nit: I got a bit mixed up reviewing these tests because of the word \"cleans\" instead of \"retries\" (i.e retries a reboot).\n\nOn the tests test_init_instance_retries_reboot_pending and test_init_instance_retries_reboot_pending_hard tests on L2129 and L2138, the names are more intuitive IMHO.","commit_id":"22183cd9e55b1df30c16d28ada50f515fcadeb57"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"249d3c25ea8279cb12e71a8ad1ce49e8164d1204","unresolved":true,"context_lines":[{"line_number":2256,"context_line":"        instance.vm_state \u003d vm_states.STOPPED"},{"line_number":2257,"context_line":"        instance.task_state \u003d task_states.REBOOT_STARTED_HARD"},{"line_number":2258,"context_line":"        instance.power_state \u003d power_state.SHUTDOWN"},{"line_number":2259,"context_line":"        self._test_init_instance_cleans_reboot_state(instance)"},{"line_number":2260,"context_line":""},{"line_number":2261,"context_line":"    def test_init_instance_cleans_shutdown_rebooting(self):"},{"line_number":2262,"context_line":"        instance \u003d objects.Instance(self.context)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5bc63f63_c8742356","line":2259,"updated":"2023-08-04 23:44:26.000000000","message":"Note to self: these two tests test_init_instance_cleans_shutdown_reboot_started and test_init_instance_cleans_shutdown_reboot_started_hard are filling in currently missing test coverage -- they are expected to still pass without the fix in this patch.","commit_id":"22183cd9e55b1df30c16d28ada50f515fcadeb57"}]}