)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ba2cbb3527fee72edf61bf824f4329419d788529","unresolved":false,"context_lines":[{"line_number":6483,"context_line":""},{"line_number":6484,"context_line":"            self.network_api.setup_instance_network_on_host("},{"line_number":6485,"context_line":"                context, instance, self.host,"},{"line_number":6486,"context_line":"                provider_mappings\u003dprovider_mappings)"},{"line_number":6487,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":6488,"context_line":"                                                                 instance)"},{"line_number":6489,"context_line":"            with self.rt.instance_claim(context, instance, node, allocations,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_778ffa75","line":6486,"updated":"2020-02-07 14:46:29.000000000","message":"Hmm, should this have been done inside the try-except block before? If not, what additional failure modes are introduced now? The only functional test I see for this is for a failure to connect to neutron, which seems like something that could have happened all along","commit_id":"7df68e69b34d71275e96fe6aa2deaaf99eab3049"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d835d7e023a4f390b5ffabf72d336081468cf75a","unresolved":false,"context_lines":[{"line_number":6483,"context_line":""},{"line_number":6484,"context_line":"            self.network_api.setup_instance_network_on_host("},{"line_number":6485,"context_line":"                context, instance, self.host,"},{"line_number":6486,"context_line":"                provider_mappings\u003dprovider_mappings)"},{"line_number":6487,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":6488,"context_line":"                                                                 instance)"},{"line_number":6489,"context_line":"            with self.rt.instance_claim(context, instance, node, allocations,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_fc3734ad","line":6486,"in_reply_to":"3fa7e38b_75e4c00b","updated":"2020-02-10 15:01:41.000000000","message":"Here are the separate bug [1] and backportable bugfix [2] for the unshelve allocation leak.\n\n[1] https://bugs.launchpad.net/nova/+bug/1862633\n[2] https://review.opendev.org/#/c/706868/","commit_id":"7df68e69b34d71275e96fe6aa2deaaf99eab3049"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2f24ff2ac2cf58a8eb5bc9b9b19a329b2cb4e734","unresolved":false,"context_lines":[{"line_number":6483,"context_line":""},{"line_number":6484,"context_line":"            self.network_api.setup_instance_network_on_host("},{"line_number":6485,"context_line":"                context, instance, self.host,"},{"line_number":6486,"context_line":"                provider_mappings\u003dprovider_mappings)"},{"line_number":6487,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":6488,"context_line":"                                                                 instance)"},{"line_number":6489,"context_line":"            with self.rt.instance_claim(context, instance, node, allocations,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_75e4c00b","line":6486,"in_reply_to":"3fa7e38b_778ffa75","updated":"2020-02-10 09:36:59.000000000","message":"One failure mode that is introduced by the current patch is when the instance has a qos port so a mapping is expected by nova.network.neutron.API._update_port_binding_for_instance()  If that mapping is not provided then setup_instance_network_on_host will raise PortUpdateFailed exception. This error case is a software error as nova should always have mapping for the qos ports from placement.\n\nThe functional test you found covers this as well (but being a bit more generic)\n\nRegarding the baseline error cases, before this patch any exception raised from setup_instance_network_on_host or get_instance_nw_info was logged by wrap_exception decorator on nova.compute.manager.ComputeManager.unshelve_instance then re-raised. Such exception the dropped as the unshelve_instance RPC is a cast from the conductor. :/\n\nYou are right the functional test case I added for the neutron error case is valid for the non qos case, even before this patch. I can add such testcase in a separate patch to see if we have an allocation leak (I suspect we have).","commit_id":"7df68e69b34d71275e96fe6aa2deaaf99eab3049"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a98650d7e5d0545366aaac8f21140e2f09606558","unresolved":false,"context_lines":[{"line_number":6483,"context_line":""},{"line_number":6484,"context_line":"            self.network_api.setup_instance_network_on_host("},{"line_number":6485,"context_line":"                context, instance, self.host,"},{"line_number":6486,"context_line":"                provider_mappings\u003dprovider_mappings)"},{"line_number":6487,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":6488,"context_line":"                                                                 instance)"},{"line_number":6489,"context_line":"            with self.rt.instance_claim(context, instance, node, allocations,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_eea285ac","line":6486,"in_reply_to":"3fa7e38b_fc3734ad","updated":"2020-02-19 16:59:17.000000000","message":"\\o/ thanks!","commit_id":"7df68e69b34d71275e96fe6aa2deaaf99eab3049"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ba2cbb3527fee72edf61bf824f4329419d788529","unresolved":false,"context_lines":[{"line_number":6485,"context_line":"                context, instance, self.host,"},{"line_number":6486,"context_line":"                provider_mappings\u003dprovider_mappings)"},{"line_number":6487,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":6488,"context_line":"                                                                 instance)"},{"line_number":6489,"context_line":"            with self.rt.instance_claim(context, instance, node, allocations,"},{"line_number":6490,"context_line":"                                        limits):"},{"line_number":6491,"context_line":"                self.driver.spawn(context, instance, image_meta,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_37f662d7","line":6488,"updated":"2020-02-07 14:46:29.000000000","message":"Ditto","commit_id":"7df68e69b34d71275e96fe6aa2deaaf99eab3049"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2f24ff2ac2cf58a8eb5bc9b9b19a329b2cb4e734","unresolved":false,"context_lines":[{"line_number":6485,"context_line":"                context, instance, self.host,"},{"line_number":6486,"context_line":"                provider_mappings\u003dprovider_mappings)"},{"line_number":6487,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":6488,"context_line":"                                                                 instance)"},{"line_number":6489,"context_line":"            with self.rt.instance_claim(context, instance, node, allocations,"},{"line_number":6490,"context_line":"                                        limits):"},{"line_number":6491,"context_line":"                self.driver.spawn(context, instance, image_meta,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_753cc02c","line":6488,"in_reply_to":"3fa7e38b_37f662d7","updated":"2020-02-10 09:36:59.000000000","message":"Yeah, nova.network.neutron.API._build_network_info_model could have failed before this patch due to neutron error.","commit_id":"7df68e69b34d71275e96fe6aa2deaaf99eab3049"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e743eea7fe03b8b3f0639c62a7685f1c73fef17e","unresolved":false,"context_lines":[{"line_number":6471,"context_line":"                utils.get_image_from_system_metadata("},{"line_number":6472,"context_line":"                    instance.system_metadata))"},{"line_number":6473,"context_line":""},{"line_number":6474,"context_line":"        provider_mappings \u003d \\"},{"line_number":6475,"context_line":"            self._get_request_group_mapping(request_spec)"},{"line_number":6476,"context_line":""},{"line_number":6477,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_daa00c93","line":6474,"range":{"start_line":6474,"start_character":28,"end_line":6474,"end_character":29},"updated":"2020-03-06 16:59:25.000000000","message":"Our convention has always been to use an open paren as a line break, other than in the db_api code. This looks out of place to me...","commit_id":"3db615fd7567a823cae2f2b4ee7abbdd7054df38"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b69ee930f88e57ef04500e663cb15027c6907518","unresolved":false,"context_lines":[{"line_number":6471,"context_line":"                utils.get_image_from_system_metadata("},{"line_number":6472,"context_line":"                    instance.system_metadata))"},{"line_number":6473,"context_line":""},{"line_number":6474,"context_line":"        provider_mappings \u003d \\"},{"line_number":6475,"context_line":"            self._get_request_group_mapping(request_spec)"},{"line_number":6476,"context_line":""},{"line_number":6477,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_4792654d","line":6474,"range":{"start_line":6474,"start_character":28,"end_line":6474,"end_character":29},"in_reply_to":"1fa4df85_82922256","updated":"2020-03-18 14:39:00.000000000","message":"Done","commit_id":"3db615fd7567a823cae2f2b4ee7abbdd7054df38"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"46f25d5d122866c4175cee7fe19ccecd804a4705","unresolved":false,"context_lines":[{"line_number":6471,"context_line":"                utils.get_image_from_system_metadata("},{"line_number":6472,"context_line":"                    instance.system_metadata))"},{"line_number":6473,"context_line":""},{"line_number":6474,"context_line":"        provider_mappings \u003d \\"},{"line_number":6475,"context_line":"            self._get_request_group_mapping(request_spec)"},{"line_number":6476,"context_line":""},{"line_number":6477,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_82922256","line":6474,"range":{"start_line":6474,"start_character":28,"end_line":6474,"end_character":29},"in_reply_to":"1fa4df85_daa00c93","updated":"2020-03-08 02:49:05.000000000","message":"Agree, use paren as a line break, we already do this in many patches.","commit_id":"3db615fd7567a823cae2f2b4ee7abbdd7054df38"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e743eea7fe03b8b3f0639c62a7685f1c73fef17e","unresolved":false,"context_lines":[{"line_number":6476,"context_line":""},{"line_number":6477,"context_line":"        try:"},{"line_number":6478,"context_line":"            if provider_mappings:"},{"line_number":6479,"context_line":"                compute_utils\\"},{"line_number":6480,"context_line":"                    .update_pci_request_spec_with_allocated_interface_name("},{"line_number":6481,"context_line":"                        context, self.reportclient, instance,"},{"line_number":6482,"context_line":"                        provider_mappings)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_fab068df","line":6479,"range":{"start_line":6479,"start_character":29,"end_line":6479,"end_character":30},"updated":"2020-03-06 16:59:25.000000000","message":"Same. Given now wrapped this whole statement is, I\u0027d tend to prefer just capturing a reference to the function and using that to execute.","commit_id":"3db615fd7567a823cae2f2b4ee7abbdd7054df38"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b69ee930f88e57ef04500e663cb15027c6907518","unresolved":false,"context_lines":[{"line_number":6476,"context_line":""},{"line_number":6477,"context_line":"        try:"},{"line_number":6478,"context_line":"            if provider_mappings:"},{"line_number":6479,"context_line":"                compute_utils\\"},{"line_number":6480,"context_line":"                    .update_pci_request_spec_with_allocated_interface_name("},{"line_number":6481,"context_line":"                        context, self.reportclient, instance,"},{"line_number":6482,"context_line":"                        provider_mappings)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_67ac418c","line":6479,"range":{"start_line":6479,"start_character":29,"end_line":6479,"end_character":30},"in_reply_to":"1fa4df85_fab068df","updated":"2020-03-18 14:39:00.000000000","message":"Done","commit_id":"3db615fd7567a823cae2f2b4ee7abbdd7054df38"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"46f25d5d122866c4175cee7fe19ccecd804a4705","unresolved":false,"context_lines":[{"line_number":6477,"context_line":"        try:"},{"line_number":6478,"context_line":"            if provider_mappings:"},{"line_number":6479,"context_line":"                compute_utils\\"},{"line_number":6480,"context_line":"                    .update_pci_request_spec_with_allocated_interface_name("},{"line_number":6481,"context_line":"                        context, self.reportclient, instance,"},{"line_number":6482,"context_line":"                        provider_mappings)"},{"line_number":6483,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_c2857a08","line":6480,"updated":"2020-03-08 02:49:05.000000000","message":"Just a QA: I saw in update_pci_request_spec_with_allocated_interface_name() https://opendev.org/openstack/nova/src/branch/master/nova/compute/utils.py#L1501.\n\nThere are two exception scenarios here, but I found few places to test it, maybe that doesnot related this change.\nhttps://opendev.org/openstack/nova/src/branch/master/nova/compute/utils.py#L1512-L1517","commit_id":"3db615fd7567a823cae2f2b4ee7abbdd7054df38"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b69ee930f88e57ef04500e663cb15027c6907518","unresolved":false,"context_lines":[{"line_number":6477,"context_line":"        try:"},{"line_number":6478,"context_line":"            if provider_mappings:"},{"line_number":6479,"context_line":"                compute_utils\\"},{"line_number":6480,"context_line":"                    .update_pci_request_spec_with_allocated_interface_name("},{"line_number":6481,"context_line":"                        context, self.reportclient, instance,"},{"line_number":6482,"context_line":"                        provider_mappings)"},{"line_number":6483,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_a7031942","line":6480,"in_reply_to":"1fa4df85_c2857a08","updated":"2020-03-18 14:39:00.000000000","message":"I\u0027ve added an extra unit test where the update raises. Such exception is handled below in the except Exception block, the re-raised. Then the rpc handler will log it as the rpc request is async (cast).","commit_id":"3db615fd7567a823cae2f2b4ee7abbdd7054df38"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e743eea7fe03b8b3f0639c62a7685f1c73fef17e","unresolved":false,"context_lines":[{"line_number":6479,"context_line":"                compute_utils\\"},{"line_number":6480,"context_line":"                    .update_pci_request_spec_with_allocated_interface_name("},{"line_number":6481,"context_line":"                        context, self.reportclient, instance,"},{"line_number":6482,"context_line":"                        provider_mappings)"},{"line_number":6483,"context_line":""},{"line_number":6484,"context_line":"            self.network_api.setup_instance_network_on_host("},{"line_number":6485,"context_line":"                context, instance, self.host,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_5a9bbc16","line":6482,"updated":"2020-03-06 16:59:25.000000000","message":"Maybe I\u0027m missing it, but is this covered in unit tests anywhere? I\u0027m guessing your functional test(s) wouldn\u0027t work if this wasn\u0027t right, but it also seems like something we\u0027d want to cover specifically in a unit test, more than just checking for the effect.","commit_id":"3db615fd7567a823cae2f2b4ee7abbdd7054df38"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b69ee930f88e57ef04500e663cb15027c6907518","unresolved":false,"context_lines":[{"line_number":6479,"context_line":"                compute_utils\\"},{"line_number":6480,"context_line":"                    .update_pci_request_spec_with_allocated_interface_name("},{"line_number":6481,"context_line":"                        context, self.reportclient, instance,"},{"line_number":6482,"context_line":"                        provider_mappings)"},{"line_number":6483,"context_line":""},{"line_number":6484,"context_line":"            self.network_api.setup_instance_network_on_host("},{"line_number":6485,"context_line":"                context, instance, self.host,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_87c05d2c","line":6482,"in_reply_to":"1fa4df85_5a9bbc16","updated":"2020-03-18 14:39:00.000000000","message":"the fact that the update is called is covered in nova.tests.unit.compute.test_shelve.ShelveComputeManagerTestCase.test_unshelve_with_resource_request(), the whole effect covered in the functional tests.\n\nOr do you mean unit test coverage for update_pci_request_spec_with_allocated_interface_name itself? I guess that was lost during some of the moving of the logic in that function.\n\nAdded coverage now.","commit_id":"3db615fd7567a823cae2f2b4ee7abbdd7054df38"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e743eea7fe03b8b3f0639c62a7685f1c73fef17e","unresolved":false,"context_lines":[{"line_number":6496,"context_line":"                                  block_device_info\u003dblock_device_info)"},{"line_number":6497,"context_line":"        except Exception:"},{"line_number":6498,"context_line":"            with excutils.save_and_reraise_exception(logger\u003dLOG):"},{"line_number":6499,"context_line":"                LOG.exception(\u0027Unshelving failed\u0027, instance\u003dinstance)"},{"line_number":6500,"context_line":"                # Cleanup allocations created by the scheduler on this host"},{"line_number":6501,"context_line":"                # since we failed to spawn the instance. We do this both if"},{"line_number":6502,"context_line":"                # the instance claim failed with ComputeResourcesUnavailable"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_9abff4a8","line":6499,"updated":"2020-03-06 16:59:25.000000000","message":"Is this a relevant change? Seems like a random opinion-based change to me.","commit_id":"3db615fd7567a823cae2f2b4ee7abbdd7054df38"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b69ee930f88e57ef04500e663cb15027c6907518","unresolved":false,"context_lines":[{"line_number":6496,"context_line":"                                  block_device_info\u003dblock_device_info)"},{"line_number":6497,"context_line":"        except Exception:"},{"line_number":6498,"context_line":"            with excutils.save_and_reraise_exception(logger\u003dLOG):"},{"line_number":6499,"context_line":"                LOG.exception(\u0027Unshelving failed\u0027, instance\u003dinstance)"},{"line_number":6500,"context_line":"                # Cleanup allocations created by the scheduler on this host"},{"line_number":6501,"context_line":"                # since we failed to spawn the instance. We do this both if"},{"line_number":6502,"context_line":"                # the instance claim failed with ComputeResourcesUnavailable"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_a776b9d7","line":6499,"in_reply_to":"1fa4df85_229d6e65","updated":"2020-03-18 14:39:00.000000000","message":"The failure can be other than the driver.spawn fails as update_pci_request_spec_with_allocated_interface_name can fail too. Also for the log reader this is not a spawn operation but an unshelve operation.","commit_id":"3db615fd7567a823cae2f2b4ee7abbdd7054df38"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"46f25d5d122866c4175cee7fe19ccecd804a4705","unresolved":false,"context_lines":[{"line_number":6496,"context_line":"                                  block_device_info\u003dblock_device_info)"},{"line_number":6497,"context_line":"        except Exception:"},{"line_number":6498,"context_line":"            with excutils.save_and_reraise_exception(logger\u003dLOG):"},{"line_number":6499,"context_line":"                LOG.exception(\u0027Unshelving failed\u0027, instance\u003dinstance)"},{"line_number":6500,"context_line":"                # Cleanup allocations created by the scheduler on this host"},{"line_number":6501,"context_line":"                # since we failed to spawn the instance. We do this both if"},{"line_number":6502,"context_line":"                # the instance claim failed with ComputeResourcesUnavailable"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_229d6e65","line":6499,"in_reply_to":"1fa4df85_9abff4a8","updated":"2020-03-08 02:49:05.000000000","message":"It looks like gibi gave the root cause of instance failure, which is an optimization. Instead of fuzzy log printing.","commit_id":"3db615fd7567a823cae2f2b4ee7abbdd7054df38"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9a71ef6d5e22385ace557a670a493821fe4e0a0b","unresolved":false,"context_lines":[{"line_number":6496,"context_line":"                                  block_device_info\u003dblock_device_info)"},{"line_number":6497,"context_line":"        except Exception:"},{"line_number":6498,"context_line":"            with excutils.save_and_reraise_exception(logger\u003dLOG):"},{"line_number":6499,"context_line":"                LOG.exception(\u0027Unshelving failed\u0027, instance\u003dinstance)"},{"line_number":6500,"context_line":"                # Cleanup allocations created by the scheduler on this host"},{"line_number":6501,"context_line":"                # since we failed to spawn the instance. We do this both if"},{"line_number":6502,"context_line":"                # the instance claim failed with ComputeResourcesUnavailable"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_a4223200","line":6499,"in_reply_to":"1fa4df85_a776b9d7","updated":"2020-03-18 15:46:47.000000000","message":"Since nova doesn\u0027t have an externally-visible spawn operation, I don\u0027t think this is confusing. Everywhere in our code, we refer to actually creating a VM in the driver as spawn, which this is doing, and of course, is reflected in the actual driver call we\u0027re making.\n\nSo I do think it\u0027s opinion and not related to this patch (you\u0027re not changing the spawning), but I won\u0027t make a huge deal out of it.","commit_id":"3db615fd7567a823cae2f2b4ee7abbdd7054df38"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9a71ef6d5e22385ace557a670a493821fe4e0a0b","unresolved":false,"context_lines":[{"line_number":6507,"context_line":"        provider_mappings \u003d self._get_request_group_mapping(request_spec)"},{"line_number":6508,"context_line":""},{"line_number":6509,"context_line":"        try:"},{"line_number":6510,"context_line":"            if provider_mappings:"},{"line_number":6511,"context_line":"                update \u003d ("},{"line_number":6512,"context_line":"                    compute_utils."},{"line_number":6513,"context_line":"                        update_pci_request_spec_with_allocated_interface_name)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1fa4df85_87a29899","line":6510,"updated":"2020-03-18 15:46:47.000000000","message":"I don\u0027t think you\u0027ve got a test assertion that we don\u0027t call the pci update below if there were no provider mappings...","commit_id":"f5a25fd6cdc0dc386ecb3665287ba30fa7aa353b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5b985a0871a27f4d1bacd92817bbc4e915df0d32","unresolved":false,"context_lines":[{"line_number":6507,"context_line":"        provider_mappings \u003d self._get_request_group_mapping(request_spec)"},{"line_number":6508,"context_line":""},{"line_number":6509,"context_line":"        try:"},{"line_number":6510,"context_line":"            if provider_mappings:"},{"line_number":6511,"context_line":"                update \u003d ("},{"line_number":6512,"context_line":"                    compute_utils."},{"line_number":6513,"context_line":"                        update_pci_request_spec_with_allocated_interface_name)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1fa4df85_aa451917","line":6510,"in_reply_to":"1fa4df85_87a29899","updated":"2020-03-18 18:11:07.000000000","message":"Done. Added a NonCallableMock to nova.tests.unit.compute.test_shelve.ShelveComputeManagerTestCase.test_unshelve() test case","commit_id":"f5a25fd6cdc0dc386ecb3665287ba30fa7aa353b"}],"nova/conductor/manager.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ba2cbb3527fee72edf61bf824f4329419d788529","unresolved":false,"context_lines":[{"line_number":937,"context_line":""},{"line_number":938,"context_line":"                    port_res_req \u003d ("},{"line_number":939,"context_line":"                        self.network_api.get_requested_resource_for_instance("},{"line_number":940,"context_line":"                            context, instance.uuid))"},{"line_number":941,"context_line":"                    # NOTE(gibi): When cyborg or other module wants to handle"},{"line_number":942,"context_line":"                    # similar non-nova resources then here we have to collect"},{"line_number":943,"context_line":"                    # all the external resource requests in a single list and"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_57e19efc","line":940,"updated":"2020-02-07 14:46:29.000000000","message":"This can fail but we\u0027re handling this via a generic Exception handler below","commit_id":"7df68e69b34d71275e96fe6aa2deaaf99eab3049"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2f24ff2ac2cf58a8eb5bc9b9b19a329b2cb4e734","unresolved":false,"context_lines":[{"line_number":937,"context_line":""},{"line_number":938,"context_line":"                    port_res_req \u003d ("},{"line_number":939,"context_line":"                        self.network_api.get_requested_resource_for_instance("},{"line_number":940,"context_line":"                            context, instance.uuid))"},{"line_number":941,"context_line":"                    # NOTE(gibi): When cyborg or other module wants to handle"},{"line_number":942,"context_line":"                    # similar non-nova resources then here we have to collect"},{"line_number":943,"context_line":"                    # all the external resource requests in a single list and"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_d579f4f9","line":940,"in_reply_to":"3fa7e38b_57e19efc","updated":"2020-02-10 09:36:59.000000000","message":"yepp, we are covered here. \n\nI think the compute side error handling is incomplete as the implementer thought that the exception handler here would clean up, but RPC is a cast so such cleanup never happens.","commit_id":"7df68e69b34d71275e96fe6aa2deaaf99eab3049"}],"nova/network/neutron.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ba2cbb3527fee72edf61bf824f4329419d788529","unresolved":false,"context_lines":[{"line_number":3433,"context_line":"            # NOTE(gibi): during live migration the conductor already sets the"},{"line_number":3434,"context_line":"            # allocation key in the port binding"},{"line_number":3435,"context_line":"            if (p.get(\u0027resource_request\u0027) and"},{"line_number":3436,"context_line":"                    (migration is None or"},{"line_number":3437,"context_line":"                     migration[\u0027migration_type\u0027] !\u003d constants.LIVE_MIGRATION)):"},{"line_number":3438,"context_line":"                if not provider_mappings:"},{"line_number":3439,"context_line":"                    # TODO(gibi): Remove this check when compute RPC API is"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_d7b6eefc","line":3436,"updated":"2020-02-07 14:46:29.000000000","message":"The \u0027migration\u0027 object is unset in an unshelve case, right? A comment here explaining what conditions would trigger this would be helpful (unshelve, resize, and cold migrate?)","commit_id":"7df68e69b34d71275e96fe6aa2deaaf99eab3049"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2f24ff2ac2cf58a8eb5bc9b9b19a329b2cb4e734","unresolved":false,"context_lines":[{"line_number":3433,"context_line":"            # NOTE(gibi): during live migration the conductor already sets the"},{"line_number":3434,"context_line":"            # allocation key in the port binding"},{"line_number":3435,"context_line":"            if (p.get(\u0027resource_request\u0027) and"},{"line_number":3436,"context_line":"                    (migration is None or"},{"line_number":3437,"context_line":"                     migration[\u0027migration_type\u0027] !\u003d constants.LIVE_MIGRATION)):"},{"line_number":3438,"context_line":"                if not provider_mappings:"},{"line_number":3439,"context_line":"                    # TODO(gibi): Remove this check when compute RPC API is"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_155d4c7b","line":3436,"in_reply_to":"3fa7e38b_5778be2a","updated":"2020-02-10 09:36:59.000000000","message":"good points. Done.","commit_id":"7df68e69b34d71275e96fe6aa2deaaf99eab3049"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fbc8ccba79296493586d01d622e3a925ce978704","unresolved":false,"context_lines":[{"line_number":3433,"context_line":"            # NOTE(gibi): during live migration the conductor already sets the"},{"line_number":3434,"context_line":"            # allocation key in the port binding"},{"line_number":3435,"context_line":"            if (p.get(\u0027resource_request\u0027) and"},{"line_number":3436,"context_line":"                    (migration is None or"},{"line_number":3437,"context_line":"                     migration[\u0027migration_type\u0027] !\u003d constants.LIVE_MIGRATION)):"},{"line_number":3438,"context_line":"                if not provider_mappings:"},{"line_number":3439,"context_line":"                    # TODO(gibi): Remove this check when compute RPC API is"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_5778be2a","line":3436,"in_reply_to":"3fa7e38b_d7b6eefc","updated":"2020-02-07 14:48:59.000000000","message":"unshelve, *evacuate*, and cold migrate, rather","commit_id":"7df68e69b34d71275e96fe6aa2deaaf99eab3049"}],"nova/tests/functional/test_servers.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ba2cbb3527fee72edf61bf824f4329419d788529","unresolved":false,"context_lines":[{"line_number":7524,"context_line":""},{"line_number":7525,"context_line":"        # Simulate that port update fails during unshelve due to neutron is"},{"line_number":7526,"context_line":"        # unavailable"},{"line_number":7527,"context_line":"        from neutronclient.common import exceptions as neutron_exception"},{"line_number":7528,"context_line":"        with mock.patch("},{"line_number":7529,"context_line":"                \u0027nova.tests.fixtures.NeutronFixture.\u0027"},{"line_number":7530,"context_line":"                \u0027update_port\u0027) as mock_update_port:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_b7525210","line":7527,"updated":"2020-02-07 14:46:29.000000000","message":"Any reason this is here instead of at the top?","commit_id":"7df68e69b34d71275e96fe6aa2deaaf99eab3049"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2f24ff2ac2cf58a8eb5bc9b9b19a329b2cb4e734","unresolved":false,"context_lines":[{"line_number":7524,"context_line":""},{"line_number":7525,"context_line":"        # Simulate that port update fails during unshelve due to neutron is"},{"line_number":7526,"context_line":"        # unavailable"},{"line_number":7527,"context_line":"        from neutronclient.common import exceptions as neutron_exception"},{"line_number":7528,"context_line":"        with mock.patch("},{"line_number":7529,"context_line":"                \u0027nova.tests.fixtures.NeutronFixture.\u0027"},{"line_number":7530,"context_line":"                \u0027update_port\u0027) as mock_update_port:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_55dbc4e9","line":7527,"in_reply_to":"3fa7e38b_b7525210","updated":"2020-02-10 09:36:59.000000000","message":"I forget to move it. :)\nDone.","commit_id":"7df68e69b34d71275e96fe6aa2deaaf99eab3049"}],"nova/tests/unit/compute/test_shelve.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9a71ef6d5e22385ace557a670a493821fe4e0a0b","unresolved":false,"context_lines":[{"line_number":602,"context_line":"        mock_update_pci.assert_called_once_with("},{"line_number":603,"context_line":"            self.context, self.compute.reportclient, instance,"},{"line_number":604,"context_line":"            {uuids.port_1: [uuids.rp1]})"},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"    @mock.patch.object(objects.InstanceList, \u0027get_by_filters\u0027)"},{"line_number":607,"context_line":"    def test_shelved_poll_none_offloaded(self, mock_get_by_filters):"},{"line_number":608,"context_line":"        # Test instances are not offloaded when shelved_offload_time is -1"}],"source_content_type":"text/x-python","patch_set":9,"id":"1fa4df85_87d058f9","line":605,"updated":"2020-03-18 15:46:47.000000000","message":"No assert that setup_instance_network() does not get called?","commit_id":"f5a25fd6cdc0dc386ecb3665287ba30fa7aa353b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5b985a0871a27f4d1bacd92817bbc4e915df0d32","unresolved":false,"context_lines":[{"line_number":602,"context_line":"        mock_update_pci.assert_called_once_with("},{"line_number":603,"context_line":"            self.context, self.compute.reportclient, instance,"},{"line_number":604,"context_line":"            {uuids.port_1: [uuids.rp1]})"},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"    @mock.patch.object(objects.InstanceList, \u0027get_by_filters\u0027)"},{"line_number":607,"context_line":"    def test_shelved_poll_none_offloaded(self, mock_get_by_filters):"},{"line_number":608,"context_line":"        # Test instances are not offloaded when shelved_offload_time is -1"}],"source_content_type":"text/x-python","patch_set":9,"id":"1fa4df85_8af85dc4","line":605,"in_reply_to":"1fa4df85_87d058f9","updated":"2020-03-18 18:11:07.000000000","message":"Done","commit_id":"f5a25fd6cdc0dc386ecb3665287ba30fa7aa353b"}],"nova/tests/unit/network/test_neutron.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ba2cbb3527fee72edf61bf824f4329419d788529","unresolved":false,"context_lines":[{"line_number":4638,"context_line":"            {\u0027id\u0027: \u0027fake-port-1\u0027,"},{"line_number":4639,"context_line":"             \u0027binding:vnic_type\u0027: \u0027normal\u0027,"},{"line_number":4640,"context_line":"             constants.BINDING_HOST_ID: \u0027old-host\u0027,"},{"line_number":4641,"context_line":"             constants.BINDING_PROFILE:"},{"line_number":4642,"context_line":"                 {\u0027allocation\u0027: uuids.source_compute_rp},"},{"line_number":4643,"context_line":"             \u0027resource_request\u0027: mock.sentinel.resource_request}]}"},{"line_number":4644,"context_line":"        list_ports_mock \u003d mock.Mock(return_value\u003dfake_ports)"},{"line_number":4645,"context_line":"        get_client_mock.return_value.list_ports \u003d list_ports_mock"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_375742f8","line":4642,"range":{"start_line":4641,"start_character":39,"end_line":4642,"end_character":18},"updated":"2020-02-07 14:46:29.000000000","message":"style nit: could you drag this up to the previous line?","commit_id":"7df68e69b34d71275e96fe6aa2deaaf99eab3049"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2f24ff2ac2cf58a8eb5bc9b9b19a329b2cb4e734","unresolved":false,"context_lines":[{"line_number":4638,"context_line":"            {\u0027id\u0027: \u0027fake-port-1\u0027,"},{"line_number":4639,"context_line":"             \u0027binding:vnic_type\u0027: \u0027normal\u0027,"},{"line_number":4640,"context_line":"             constants.BINDING_HOST_ID: \u0027old-host\u0027,"},{"line_number":4641,"context_line":"             constants.BINDING_PROFILE:"},{"line_number":4642,"context_line":"                 {\u0027allocation\u0027: uuids.source_compute_rp},"},{"line_number":4643,"context_line":"             \u0027resource_request\u0027: mock.sentinel.resource_request}]}"},{"line_number":4644,"context_line":"        list_ports_mock \u003d mock.Mock(return_value\u003dfake_ports)"},{"line_number":4645,"context_line":"        get_client_mock.return_value.list_ports \u003d list_ports_mock"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_35f08866","line":4642,"range":{"start_line":4641,"start_character":39,"end_line":4642,"end_character":18},"in_reply_to":"3fa7e38b_375742f8","updated":"2020-02-10 09:36:59.000000000","message":"Done","commit_id":"7df68e69b34d71275e96fe6aa2deaaf99eab3049"}]}