)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a9c389a66cbc2bb36c6687cd8ba6e70cbc6d5757","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The neturonv2/api already receives information about which resource"},{"line_number":10,"context_line":"provider fullfills the resource needs of each neutron port. So in this"},{"line_number":11,"context_line":"patch we include this infomation in the binding:profile of the neutron"},{"line_number":12,"context_line":"port during the port binding."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Also make sure that this data is removed from the binding:profile when"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":67,"id":"9fdfeff1_88f92681","line":11,"range":{"start_line":11,"start_character":22,"end_line":11,"end_character":32},"updated":"2019-02-25 17:47:50.000000000","message":"information","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"}],"nova/compute/manager.py":[{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"inactive contributor"},"change_message_id":"6000200c44e4b4cc9a105b6291f48d50ad0b5a85","unresolved":false,"context_lines":[{"line_number":1356,"context_line":"        if self.driver.instance_exists(instance):"},{"line_number":1357,"context_line":"            raise exception.InstanceExists(name\u003dinstance.name)"},{"line_number":1358,"context_line":""},{"line_number":1359,"context_line":"    def _allocate_network_async(self, context, instance, requested_networks,"},{"line_number":1360,"context_line":"                                macs, security_groups, is_vpn, allocations):"},{"line_number":1361,"context_line":"        \"\"\"Method used to allocate networks in the background."},{"line_number":1362,"context_line":""},{"line_number":1363,"context_line":"        Broken out for testing."}],"source_content_type":"text/x-python","patch_set":8,"id":"5f7c97a3_ac61f313","line":1360,"range":{"start_line":1359,"start_character":9,"end_line":1360,"end_character":74},"updated":"2018-06-06 13:29:48.000000000","message":"Shouldn\u0027t we differentiate between the two meanings of \u0027allocate\u0027 in this signature? I mean something like this:\n\n def _allocate_...(..., placement_allocations):","commit_id":"7c85e374e9cb74abd1ce30ac2175181ada57956b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"282ecb83bb08b78c9023ffdc5057dcb8fa735395","unresolved":false,"context_lines":[{"line_number":1356,"context_line":"        if self.driver.instance_exists(instance):"},{"line_number":1357,"context_line":"            raise exception.InstanceExists(name\u003dinstance.name)"},{"line_number":1358,"context_line":""},{"line_number":1359,"context_line":"    def _allocate_network_async(self, context, instance, requested_networks,"},{"line_number":1360,"context_line":"                                macs, security_groups, is_vpn, allocations):"},{"line_number":1361,"context_line":"        \"\"\"Method used to allocate networks in the background."},{"line_number":1362,"context_line":""},{"line_number":1363,"context_line":"        Broken out for testing."}],"source_content_type":"text/x-python","patch_set":8,"id":"5f7c97a3_ca350edd","line":1360,"range":{"start_line":1359,"start_character":9,"end_line":1360,"end_character":74},"in_reply_to":"5f7c97a3_ac61f313","updated":"2018-06-08 15:13:33.000000000","message":"Renamed the new thing as the structure of that data also changed to contain the RP uuid per port the port\u0027s allocation is made from.","commit_id":"7c85e374e9cb74abd1ce30ac2175181ada57956b"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"inactive contributor"},"change_message_id":"6000200c44e4b4cc9a105b6291f48d50ad0b5a85","unresolved":false,"context_lines":[{"line_number":1999,"context_line":""},{"line_number":2000,"context_line":"                try:"},{"line_number":2001,"context_line":"                    allocs \u003d ("},{"line_number":2002,"context_line":"                        self.reportclient.get_allocations_for_consumer("},{"line_number":2003,"context_line":"                            context,"},{"line_number":2004,"context_line":"                            instance.uuid))"},{"line_number":2005,"context_line":"                except Exception:"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f7c97a3_ccbf8f5e","line":2002,"range":{"start_line":2002,"start_character":24,"end_line":2002,"end_character":70},"updated":"2018-06-06 13:29:48.000000000","message":"side note: I understand you just moved this code from below in this patch but it somewhat bothers me that the allocations may have changed since they were recorded in the first place and the time when we\u0027re reading them here. Allocations could have been passed down from where they were made. However all this may be a very theoretical problem without any practical consequences ever.","commit_id":"7c85e374e9cb74abd1ce30ac2175181ada57956b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f31da48771f9997c3dd00b642deebc3a6ff1e099","unresolved":false,"context_lines":[{"line_number":1999,"context_line":""},{"line_number":2000,"context_line":"                try:"},{"line_number":2001,"context_line":"                    allocs \u003d ("},{"line_number":2002,"context_line":"                        self.reportclient.get_allocations_for_consumer("},{"line_number":2003,"context_line":"                            context,"},{"line_number":2004,"context_line":"                            instance.uuid))"},{"line_number":2005,"context_line":"                except Exception:"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f7c97a3_4bb1e98e","line":2002,"range":{"start_line":2002,"start_character":24,"end_line":2002,"end_character":70},"in_reply_to":"5f7c97a3_ccbf8f5e","updated":"2018-06-08 15:33:41.000000000","message":"So the following Nova API actions could influence the server\u0027s allocations but all of them are guarded with instance state checks (linked below). During the commented code path the instance is in BUILDING state so all these operations are rejected from the nova-api service.\n\n* resize https://github.com/openstack/nova/blob/f9a419e40ff6440d1b85ff175ab862090a970460/nova/compute/api.py#L3257\n* migrate calls the same compute.api.resize() call as above\n* live-migrate https://github.com/openstack/nova/blob/f9a419e40ff6440d1b85ff175ab862090a970460/nova/compute/api.py#L4190\n* evacuate https://github.com/openstack/nova/blob/f9a419e40ff6440d1b85ff175ab862090a970460/nova/compute/api.py#L4319\n* unshelve after shelve_offload https://github.com/openstack/nova/blob/f9a419e40ff6440d1b85ff175ab862090a970460/nova/compute/api.py#L3431\n\ninterface_attach could effect the bandwidth allocation too but that code does not exists yet and out of scope from the current spec.\n\nSo at the moment this is theoretical problem only.","commit_id":"7c85e374e9cb74abd1ce30ac2175181ada57956b"}],"nova/network/neutronv2/api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"412451b9b40d105f5345af40464f96fba3db54a2","unresolved":false,"context_lines":[{"line_number":55,"context_line":"DEFAULT_SECGROUP \u003d \u0027default\u0027"},{"line_number":56,"context_line":"BINDING_PROFILE \u003d \u0027binding:profile\u0027"},{"line_number":57,"context_line":"BINDING_HOST_ID \u003d \u0027binding:host_id\u0027"},{"line_number":58,"context_line":"MIGRATING_ATTR \u003d \u0027migrating_to\u0027"},{"line_number":59,"context_line":"L3_NETWORK_TYPES \u003d [\u0027vxlan\u0027, \u0027gre\u0027, \u0027geneve\u0027]"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_89edd1b8","line":58,"updated":"2019-02-22 13:37:48.000000000","message":"I would suggest adding a constant here for the \u0027allocation\u0027 key in the BINDING_PROFILE like we have for the migrating_to key that also goes in the binding:profile.","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"301c608c9f0e4a877eba75973539a2b2059f3410","unresolved":false,"context_lines":[{"line_number":55,"context_line":"DEFAULT_SECGROUP \u003d \u0027default\u0027"},{"line_number":56,"context_line":"BINDING_PROFILE \u003d \u0027binding:profile\u0027"},{"line_number":57,"context_line":"BINDING_HOST_ID \u003d \u0027binding:host_id\u0027"},{"line_number":58,"context_line":"MIGRATING_ATTR \u003d \u0027migrating_to\u0027"},{"line_number":59,"context_line":"L3_NETWORK_TYPES \u003d [\u0027vxlan\u0027, \u0027gre\u0027, \u0027geneve\u0027]"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_243dacba","line":58,"in_reply_to":"9fdfeff1_89edd1b8","updated":"2019-02-26 16:16:37.000000000","message":"Done in a fup.","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6b261cf375c435930c0900dbf8a6f520a49deb31","unresolved":false,"context_lines":[{"line_number":649,"context_line":"            except exception.PortNotFound:"},{"line_number":650,"context_line":"                LOG.debug(\u0027Unable to show port %s as it no longer \u0027"},{"line_number":651,"context_line":"                          \u0027exists.\u0027, port_id)"},{"line_number":652,"context_line":"                return"},{"line_number":653,"context_line":"            except Exception:"},{"line_number":654,"context_line":"                # NOTE: In case we can\u0027t retrieve the binding:profile or"},{"line_number":655,"context_line":"                # network info assume that they are empty"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_1cdd7413","line":652,"updated":"2019-02-25 21:19:39.000000000","message":"this made me think of something... what happens to the allocation for an instance against, say, a network bandwidth resource provider if the port is deleted in Neutron and the allocation still exists in placement? Do we have any way of preventing the port from being deleted and orphaning the allocation (partially) in placement?","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5b9d487b19cb1e6c19f0a89a767bfff9823f0174","unresolved":false,"context_lines":[{"line_number":649,"context_line":"            except exception.PortNotFound:"},{"line_number":650,"context_line":"                LOG.debug(\u0027Unable to show port %s as it no longer \u0027"},{"line_number":651,"context_line":"                          \u0027exists.\u0027, port_id)"},{"line_number":652,"context_line":"                return"},{"line_number":653,"context_line":"            except Exception:"},{"line_number":654,"context_line":"                # NOTE: In case we can\u0027t retrieve the binding:profile or"},{"line_number":655,"context_line":"                # network info assume that they are empty"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_54d7504c","line":652,"in_reply_to":"9fdfeff1_1cdd7413","updated":"2019-02-25 22:18:56.000000000","message":"\u003e this made me think of something... what happens to the allocation\n \u003e for an instance against, say, a network bandwidth resource provider\n \u003e if the port is deleted in Neutron and the allocation still exists\n \u003e in placement? Do we have any way of preventing the port from being\n \u003e deleted and orphaning the allocation (partially) in placement?\n\nGood question, and it probably depends on what event we get from neutron.\n\nAnd this also probably applies to https://review.openstack.org/#/c/622421/ later in the series.\n\nIf the attached port is deleted, then we should process that here:\n\nhttps://github.com/openstack/nova/blob/29e5b0ad7bde210f95885656768f0480d06882c0/nova/compute/manager.py#L8083\n\nBut that doesn\u0027t currently cleanup the allocation (which is probably another case for stashing information about the port\u0027s requested_resources/allocations in the info cache because if the port is gone, we can\u0027t get the allocation RP UUID off of it).\n\nBut if the port is simply updated to drop the device_id (detached), I\u0027m not sure if we get any event. We might get network-changed but that just refreshes the info cache:\n\nhttps://github.com/openstack/nova/blob/29e5b0ad7bde210f95885656768f0480d06882c0/nova/compute/manager.py#L8182\n\nIt\u0027s definitely a leaked allocation right now though. Neutron should probably reject a change to null out the device_id while the port has an allocation on the binding profile.","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"301c608c9f0e4a877eba75973539a2b2059f3410","unresolved":false,"context_lines":[{"line_number":649,"context_line":"            except exception.PortNotFound:"},{"line_number":650,"context_line":"                LOG.debug(\u0027Unable to show port %s as it no longer \u0027"},{"line_number":651,"context_line":"                          \u0027exists.\u0027, port_id)"},{"line_number":652,"context_line":"                return"},{"line_number":653,"context_line":"            except Exception:"},{"line_number":654,"context_line":"                # NOTE: In case we can\u0027t retrieve the binding:profile or"},{"line_number":655,"context_line":"                # network info assume that they are empty"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_613ba6e9","line":652,"in_reply_to":"9fdfeff1_54d7504c","updated":"2019-02-26 16:16:37.000000000","message":"The whole allocation is held by the instance.uuid as the consumer in placement. Detaching port is possible via the nova api and that handles the allocation change in placement (later patch in this series). \n\nDetaching the port manually in neutron feels wrong to me but I did not know about _process_instance_vif_deleted_event().\n\nWhen the instance is deleted then the whole allocation is deleted including every port allocation of the instance so at least at that point the allocation is not leaked.\n\nI will look into this and make a bugreport if I did not found an easy solution.","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6b261cf375c435930c0900dbf8a6f520a49deb31","unresolved":false,"context_lines":[{"line_number":671,"context_line":"            # NOTE: We\u0027re doing this to remove the binding information"},{"line_number":672,"context_line":"            # for the physical device but don\u0027t want to overwrite the other"},{"line_number":673,"context_line":"            # information in the binding profile."},{"line_number":674,"context_line":"            for profile_key in (\u0027pci_vendor_info\u0027, \u0027pci_slot\u0027):"},{"line_number":675,"context_line":"                if profile_key in port_profile:"},{"line_number":676,"context_line":"                    del port_profile[profile_key]"},{"line_number":677,"context_line":"            # NOTE(gibi): remove information about the resource allocation"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_dc304c58","line":674,"updated":"2019-02-25 21:19:39.000000000","message":"why not just add \u0027allocation\u0027 to the above?","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"301c608c9f0e4a877eba75973539a2b2059f3410","unresolved":false,"context_lines":[{"line_number":671,"context_line":"            # NOTE: We\u0027re doing this to remove the binding information"},{"line_number":672,"context_line":"            # for the physical device but don\u0027t want to overwrite the other"},{"line_number":673,"context_line":"            # information in the binding profile."},{"line_number":674,"context_line":"            for profile_key in (\u0027pci_vendor_info\u0027, \u0027pci_slot\u0027):"},{"line_number":675,"context_line":"                if profile_key in port_profile:"},{"line_number":676,"context_line":"                    del port_profile[profile_key]"},{"line_number":677,"context_line":"            # NOTE(gibi): remove information about the resource allocation"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_04ae68e6","line":674,"in_reply_to":"9fdfeff1_9414d8ea","updated":"2019-02-26 16:16:37.000000000","message":"Done in a fup","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5b9d487b19cb1e6c19f0a89a767bfff9823f0174","unresolved":false,"context_lines":[{"line_number":671,"context_line":"            # NOTE: We\u0027re doing this to remove the binding information"},{"line_number":672,"context_line":"            # for the physical device but don\u0027t want to overwrite the other"},{"line_number":673,"context_line":"            # information in the binding profile."},{"line_number":674,"context_line":"            for profile_key in (\u0027pci_vendor_info\u0027, \u0027pci_slot\u0027):"},{"line_number":675,"context_line":"                if profile_key in port_profile:"},{"line_number":676,"context_line":"                    del port_profile[profile_key]"},{"line_number":677,"context_line":"            # NOTE(gibi): remove information about the resource allocation"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_9414d8ea","line":674,"in_reply_to":"9fdfeff1_dc304c58","updated":"2019-02-25 22:18:56.000000000","message":"Oh good point - the fup is still available so it should just do that.","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"412451b9b40d105f5345af40464f96fba3db54a2","unresolved":false,"context_lines":[{"line_number":674,"context_line":"            for profile_key in (\u0027pci_vendor_info\u0027, \u0027pci_slot\u0027):"},{"line_number":675,"context_line":"                if profile_key in port_profile:"},{"line_number":676,"context_line":"                    del port_profile[profile_key]"},{"line_number":677,"context_line":"            # NOTE(gibi): remove information about the resource allocation"},{"line_number":678,"context_line":"            # of this port"},{"line_number":679,"context_line":"            if \u0027allocation\u0027 in port_profile:"},{"line_number":680,"context_line":"                del port_profile[\u0027allocation\u0027]"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_89a631ab","line":677,"updated":"2019-02-22 13:37:48.000000000","message":"This is one place we change the port binding but what about the delete_port_binding method? That\u0027s currently only used during live migration, but I\u0027m also writing its use into the cross-cell resize flow. If we delete the active port binding in delete_port_binding, do we also need to update the port binding:profile to remove any allocation? Or will neutron handle that for us? I\u0027m assuming they don\u0027t because otherwise we wouldn\u0027t have to make this change when setting binding:host_id to None.","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9ee823473e2e7a7a7c4ebdf014b56b8de397e6d5","unresolved":false,"context_lines":[{"line_number":674,"context_line":"            for profile_key in (\u0027pci_vendor_info\u0027, \u0027pci_slot\u0027):"},{"line_number":675,"context_line":"                if profile_key in port_profile:"},{"line_number":676,"context_line":"                    del port_profile[profile_key]"},{"line_number":677,"context_line":"            # NOTE(gibi): remove information about the resource allocation"},{"line_number":678,"context_line":"            # of this port"},{"line_number":679,"context_line":"            if \u0027allocation\u0027 in port_profile:"},{"line_number":680,"context_line":"                del port_profile[\u0027allocation\u0027]"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_546f5a27","line":677,"in_reply_to":"9fdfeff1_22a397b3","updated":"2019-02-25 16:59:36.000000000","message":"Yeah, I think we need to handle binding:profile via the bindings API for migrate. The delete part is OK as is as it deletes the whole binding, including the profile. But the bind_ports_to_host() call needs to take care of the allocation key in the binding profile. I\u0027ve added a TODO in a fup.","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"18581f6e4058020f3bdf4d6f86b6e5f160107bf6","unresolved":false,"context_lines":[{"line_number":674,"context_line":"            for profile_key in (\u0027pci_vendor_info\u0027, \u0027pci_slot\u0027):"},{"line_number":675,"context_line":"                if profile_key in port_profile:"},{"line_number":676,"context_line":"                    del port_profile[profile_key]"},{"line_number":677,"context_line":"            # NOTE(gibi): remove information about the resource allocation"},{"line_number":678,"context_line":"            # of this port"},{"line_number":679,"context_line":"            if \u0027allocation\u0027 in port_profile:"},{"line_number":680,"context_line":"                del port_profile[\u0027allocation\u0027]"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_ddf38f9e","line":677,"in_reply_to":"9fdfeff1_69fc2d3e","updated":"2019-02-22 16:33:18.000000000","message":"So as far as I see only server move operations are calling delete_port_binding(). Right now those operations are rejected. When I add support for server move operations then I have to take care that \u0027allocation\u0027 key is added to port binding _before_ the binding is activated during the move. And also I need to make sure that when the old binding is deleted (either due to confirm or rollback) then the \u0027allocation\u0027 is also removed from the binding.","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9b21c98c775c995d91009d08077eb0264d37a5b8","unresolved":false,"context_lines":[{"line_number":674,"context_line":"            for profile_key in (\u0027pci_vendor_info\u0027, \u0027pci_slot\u0027):"},{"line_number":675,"context_line":"                if profile_key in port_profile:"},{"line_number":676,"context_line":"                    del port_profile[profile_key]"},{"line_number":677,"context_line":"            # NOTE(gibi): remove information about the resource allocation"},{"line_number":678,"context_line":"            # of this port"},{"line_number":679,"context_line":"            if \u0027allocation\u0027 in port_profile:"},{"line_number":680,"context_line":"                del port_profile[\u0027allocation\u0027]"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_69fc2d3e","line":677,"in_reply_to":"9fdfeff1_89a631ab","updated":"2019-02-22 13:40:59.000000000","message":"Maybe the answer to this depends on the answer to my questions below on L3234, because during live migration we only delete port bindings in really two cases:\n\n1. delete the source host port bindings in _post_live_migration after we\u0027ve successfully migrated to the dest host and activated the dest host port bindings\n\n2. delete the dest host port bindings in _rollback_live_migration because the live migration failed.\n\nIn either case, I assume the underlying port bindings matter less for the port profile allocation since the allocation shouldn\u0027t change regardless of which port binding is active.","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0991fc259a80eeff5da8aa82fb8c3c07e18df08e","unresolved":false,"context_lines":[{"line_number":674,"context_line":"            for profile_key in (\u0027pci_vendor_info\u0027, \u0027pci_slot\u0027):"},{"line_number":675,"context_line":"                if profile_key in port_profile:"},{"line_number":676,"context_line":"                    del port_profile[profile_key]"},{"line_number":677,"context_line":"            # NOTE(gibi): remove information about the resource allocation"},{"line_number":678,"context_line":"            # of this port"},{"line_number":679,"context_line":"            if \u0027allocation\u0027 in port_profile:"},{"line_number":680,"context_line":"                del port_profile[\u0027allocation\u0027]"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_22a397b3","line":677,"in_reply_to":"9fdfeff1_ddf38f9e","updated":"2019-02-22 19:36:52.000000000","message":"\u003e So as far as I see only server move operations are calling delete_port_binding(). Right now those operations are rejected. \n\nYeah definitely a move operation thing right now. Maybe we need a TODO or something so we don\u0027t forget about it?\n\n\u003e When I add support for server move operations then I have to take care that \u0027allocation\u0027 key is added to port binding _before_ the binding is activated during the move. And also I need to make sure that when the old binding is deleted (either due to confirm or rollback) then the \u0027allocation\u0027 is also removed from the binding.\n\nBut the allocation is on the port binding:profile at least how you\u0027re dealing with it in this change.\n\nDo you mean the actual \"profile\" entry on the port binding resource itself?\n\nhttps://github.com/openstack/neutron-lib/blob/888a28d5982a763d8fc8f8211b3c00f763e8b099/neutron_lib/api/definitions/portbindings_extended.py#L62","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"412451b9b40d105f5345af40464f96fba3db54a2","unresolved":false,"context_lines":[{"line_number":676,"context_line":"                    del port_profile[profile_key]"},{"line_number":677,"context_line":"            # NOTE(gibi): remove information about the resource allocation"},{"line_number":678,"context_line":"            # of this port"},{"line_number":679,"context_line":"            if \u0027allocation\u0027 in port_profile:"},{"line_number":680,"context_line":"                del port_profile[\u0027allocation\u0027]"},{"line_number":681,"context_line":"            port_req_body[\u0027port\u0027][BINDING_PROFILE] \u003d port_profile"},{"line_number":682,"context_line":""},{"line_number":683,"context_line":"            # NOTE: For internal DNS integration (network does not have a"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_09ad619a","line":680,"range":{"start_line":679,"start_character":12,"end_line":680,"end_character":46},"updated":"2019-02-22 13:37:48.000000000","message":"nit: this could just be port_profile.pop(\u0027allocation\u0027, None)","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9ee823473e2e7a7a7c4ebdf014b56b8de397e6d5","unresolved":false,"context_lines":[{"line_number":676,"context_line":"                    del port_profile[profile_key]"},{"line_number":677,"context_line":"            # NOTE(gibi): remove information about the resource allocation"},{"line_number":678,"context_line":"            # of this port"},{"line_number":679,"context_line":"            if \u0027allocation\u0027 in port_profile:"},{"line_number":680,"context_line":"                del port_profile[\u0027allocation\u0027]"},{"line_number":681,"context_line":"            port_req_body[\u0027port\u0027][BINDING_PROFILE] \u003d port_profile"},{"line_number":682,"context_line":""},{"line_number":683,"context_line":"            # NOTE: For internal DNS integration (network does not have a"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_b91d5f63","line":680,"range":{"start_line":679,"start_character":12,"end_line":680,"end_character":46},"in_reply_to":"9fdfeff1_09ad619a","updated":"2019-02-25 16:59:36.000000000","message":"Done in a fup.","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"412451b9b40d105f5345af40464f96fba3db54a2","unresolved":false,"context_lines":[{"line_number":1085,"context_line":"        for port in requested_ports_dict.values():"},{"line_number":1086,"context_line":"            # only communicate the allocations if the port has resource"},{"line_number":1087,"context_line":"            # requests"},{"line_number":1088,"context_line":"            if port.get(\u0027resource_request\u0027, None):"},{"line_number":1089,"context_line":"                profile \u003d port.get(BINDING_PROFILE, {})"},{"line_number":1090,"context_line":"                # NOTE(gibi): In the resource provider mapping there can be"},{"line_number":1091,"context_line":"                # more than one RP fulfilling a request group. But resource"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_2905c57a","line":1088,"range":{"start_line":1088,"start_character":42,"end_line":1088,"end_character":48},"updated":"2019-02-22 13:37:48.000000000","message":"nit: This is redundant with .get().","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9ee823473e2e7a7a7c4ebdf014b56b8de397e6d5","unresolved":false,"context_lines":[{"line_number":1085,"context_line":"        for port in requested_ports_dict.values():"},{"line_number":1086,"context_line":"            # only communicate the allocations if the port has resource"},{"line_number":1087,"context_line":"            # requests"},{"line_number":1088,"context_line":"            if port.get(\u0027resource_request\u0027, None):"},{"line_number":1089,"context_line":"                profile \u003d port.get(BINDING_PROFILE, {})"},{"line_number":1090,"context_line":"                # NOTE(gibi): In the resource provider mapping there can be"},{"line_number":1091,"context_line":"                # more than one RP fulfilling a request group. But resource"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_d922a3a3","line":1088,"range":{"start_line":1088,"start_character":42,"end_line":1088,"end_character":48},"in_reply_to":"9fdfeff1_2905c57a","updated":"2019-02-25 16:59:36.000000000","message":"Done in a fup","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6b261cf375c435930c0900dbf8a6f520a49deb31","unresolved":false,"context_lines":[{"line_number":1091,"context_line":"                # more than one RP fulfilling a request group. But resource"},{"line_number":1092,"context_line":"                # requests of a Neutron port is always mapped to a"},{"line_number":1093,"context_line":"                # numbered request group that is always fulfilled by one"},{"line_number":1094,"context_line":"                # resource provider. So we only pass that single RP UUID here."},{"line_number":1095,"context_line":"                profile[\u0027allocation\u0027] \u003d resource_provider_mapping["},{"line_number":1096,"context_line":"                    port[\u0027id\u0027]][0]"},{"line_number":1097,"context_line":"                port[BINDING_PROFILE] \u003d profile"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_9c272413","line":1094,"updated":"2019-02-25 21:19:39.000000000","message":"✔","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a9c389a66cbc2bb36c6687cd8ba6e70cbc6d5757","unresolved":false,"context_lines":[{"line_number":1095,"context_line":"                profile[\u0027allocation\u0027] \u003d resource_provider_mapping["},{"line_number":1096,"context_line":"                    port[\u0027id\u0027]][0]"},{"line_number":1097,"context_line":"                port[BINDING_PROFILE] \u003d profile"},{"line_number":1098,"context_line":""},{"line_number":1099,"context_line":"        # Create ports from the list of ordered_networks. The returned"},{"line_number":1100,"context_line":"        # requests_and_created_ports variable is a list of 2-item tuples of"},{"line_number":1101,"context_line":"        # the form (NetworkRequest, created_port_id). Note that a tuple pair"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_88da86e6","line":1098,"updated":"2019-02-25 17:47:50.000000000","message":"Not that it reads any better, but could this reduce to\n\n port.setdefault(BINDING_PROFILE, {})[\u0027allocation\u0027] \u003d resource_provider_mapping[port[\u0027id\u0027]][0]\n\n?","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"301c608c9f0e4a877eba75973539a2b2059f3410","unresolved":false,"context_lines":[{"line_number":1095,"context_line":"                profile[\u0027allocation\u0027] \u003d resource_provider_mapping["},{"line_number":1096,"context_line":"                    port[\u0027id\u0027]][0]"},{"line_number":1097,"context_line":"                port[BINDING_PROFILE] \u003d profile"},{"line_number":1098,"context_line":""},{"line_number":1099,"context_line":"        # Create ports from the list of ordered_networks. The returned"},{"line_number":1100,"context_line":"        # requests_and_created_ports variable is a list of 2-item tuples of"},{"line_number":1101,"context_line":"        # the form (NetworkRequest, created_port_id). Note that a tuple pair"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_7ed26136","line":1098,"in_reply_to":"9fdfeff1_88da86e6","updated":"2019-02-26 16:16:37.000000000","message":"I\u0027m feel setdefault makes it a bit more harder to understand.","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"412451b9b40d105f5345af40464f96fba3db54a2","unresolved":false,"context_lines":[{"line_number":1104,"context_line":"        # pre-existing port so one wasn\u0027t created here. The ports will be"},{"line_number":1105,"context_line":"        # updated later in _update_ports_for_instance to be bound to the"},{"line_number":1106,"context_line":"        # instance and compute host."},{"line_number":1107,"context_line":"        # TODO(gibi): if a port created here has resource request then we have"},{"line_number":1108,"context_line":"        # to abort as that resource request was not considered during the"},{"line_number":1109,"context_line":"        # scheduling of the instance and therefore there is no allocation in"},{"line_number":1110,"context_line":"        # placement for that port."}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_69474dbe","line":1107,"range":{"start_line":1107,"start_character":8,"end_line":1107,"end_character":22},"updated":"2019-02-22 13:37:48.000000000","message":"We already handled this:\n\nhttps://review.openstack.org/#/c/569459/67/nova/network/neutronv2/api.py@524","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c8fd086e23b915a5b99e7d202534c26070a1656d","unresolved":false,"context_lines":[{"line_number":1104,"context_line":"        # pre-existing port so one wasn\u0027t created here. The ports will be"},{"line_number":1105,"context_line":"        # updated later in _update_ports_for_instance to be bound to the"},{"line_number":1106,"context_line":"        # instance and compute host."},{"line_number":1107,"context_line":"        # TODO(gibi): if a port created here has resource request then we have"},{"line_number":1108,"context_line":"        # to abort as that resource request was not considered during the"},{"line_number":1109,"context_line":"        # scheduling of the instance and therefore there is no allocation in"},{"line_number":1110,"context_line":"        # placement for that port."}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_dbf4edf3","line":1107,"range":{"start_line":1107,"start_character":8,"end_line":1107,"end_character":22},"in_reply_to":"9fdfeff1_69474dbe","updated":"2019-02-23 13:34:05.000000000","message":"You are correct. Let me remove this TODO.","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9ee823473e2e7a7a7c4ebdf014b56b8de397e6d5","unresolved":false,"context_lines":[{"line_number":1104,"context_line":"        # pre-existing port so one wasn\u0027t created here. The ports will be"},{"line_number":1105,"context_line":"        # updated later in _update_ports_for_instance to be bound to the"},{"line_number":1106,"context_line":"        # instance and compute host."},{"line_number":1107,"context_line":"        # TODO(gibi): if a port created here has resource request then we have"},{"line_number":1108,"context_line":"        # to abort as that resource request was not considered during the"},{"line_number":1109,"context_line":"        # scheduling of the instance and therefore there is no allocation in"},{"line_number":1110,"context_line":"        # placement for that port."}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_14f55257","line":1107,"range":{"start_line":1107,"start_character":8,"end_line":1107,"end_character":22},"in_reply_to":"9fdfeff1_dbf4edf3","updated":"2019-02-25 16:59:36.000000000","message":"Done. Removed in a fup.","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"412451b9b40d105f5345af40464f96fba3db54a2","unresolved":false,"context_lines":[{"line_number":3231,"context_line":"            # If the host hasn\u0027t changed, like in the case of resizing to the"},{"line_number":3232,"context_line":"            # same host, there is nothing to do."},{"line_number":3233,"context_line":"            if p.get(BINDING_HOST_ID) !\u003d host:"},{"line_number":3234,"context_line":"                updates[BINDING_HOST_ID] \u003d host"},{"line_number":3235,"context_line":"                # If the host changed, the AZ could have also changed so we"},{"line_number":3236,"context_line":"                # need to update the device_owner."},{"line_number":3237,"context_line":"                updates[\u0027device_owner\u0027] \u003d ("}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_49dfe90b","line":3234,"range":{"start_line":3234,"start_character":16,"end_line":3234,"end_character":47},"updated":"2019-02-22 13:37:48.000000000","message":"If we change the port binding host here, do we need to do anything about the allocation on the port profile? I guess since we won\u0027t allow attaching ports with resource requests maybe we\u0027re OK for now, meaning the allocation for the given port should be the same across hosts (port bindings) since you can\u0027t change that during something like a resize since the allocations are based on the port\u0027s requested resources rather than something like the flavor.","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"18581f6e4058020f3bdf4d6f86b6e5f160107bf6","unresolved":false,"context_lines":[{"line_number":3231,"context_line":"            # If the host hasn\u0027t changed, like in the case of resizing to the"},{"line_number":3232,"context_line":"            # same host, there is nothing to do."},{"line_number":3233,"context_line":"            if p.get(BINDING_HOST_ID) !\u003d host:"},{"line_number":3234,"context_line":"                updates[BINDING_HOST_ID] \u003d host"},{"line_number":3235,"context_line":"                # If the host changed, the AZ could have also changed so we"},{"line_number":3236,"context_line":"                # need to update the device_owner."},{"line_number":3237,"context_line":"                updates[\u0027device_owner\u0027] \u003d ("}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_9d42e7aa","line":3234,"range":{"start_line":3234,"start_character":16,"end_line":3234,"end_character":47},"in_reply_to":"9fdfeff1_49dfe90b","updated":"2019-02-22 16:33:18.000000000","message":"I think we need to change the \u0027allocation\u0027 here as well when we start supporting server move operations. When a server is moved to a new host then on that new host a different RP will provide resource for a given port so this update is necessary during the move.","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"}],"nova/tests/fixtures.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"957eb815748338f546bb57be6598954db6450e69","unresolved":false,"context_lines":[{"line_number":1169,"context_line":"            }"},{"line_number":1170,"context_line":"        ],"},{"line_number":1171,"context_line":"        \u0027tenant_id\u0027: tenant_id,"},{"line_number":1172,"context_line":"        \u0027resource_request\u0027: {"},{"line_number":1173,"context_line":"            \"resources\": {\"VCPU\": 1},"},{"line_number":1174,"context_line":"        }"},{"line_number":1175,"context_line":"    }"},{"line_number":1176,"context_line":""},{"line_number":1177,"context_line":"    nw_info \u003d [{"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_eeeec320","line":1174,"range":{"start_line":1172,"start_character":8,"end_line":1174,"end_character":9},"updated":"2018-05-25 00:06:53.000000000","message":"eh?","commit_id":"7be36d59ac1cbfa75504cb7eda04844d0b32d06a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"18db6609a1dc07fd5773ad9f5f98176502818baf","unresolved":false,"context_lines":[{"line_number":1169,"context_line":"            }"},{"line_number":1170,"context_line":"        ],"},{"line_number":1171,"context_line":"        \u0027tenant_id\u0027: tenant_id,"},{"line_number":1172,"context_line":"        \u0027resource_request\u0027: {"},{"line_number":1173,"context_line":"            \"resources\": {\"VCPU\": 1},"},{"line_number":1174,"context_line":"        }"},{"line_number":1175,"context_line":"    }"},{"line_number":1176,"context_line":""},{"line_number":1177,"context_line":"    nw_info \u003d [{"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_e6124358","line":1174,"range":{"start_line":1172,"start_character":8,"end_line":1174,"end_character":9},"in_reply_to":"5f7c97a3_eeeec320","updated":"2018-05-29 15:25:43.000000000","message":"As the a_c placement query started to work for nested resource I could remove this nastiness from the test. :)","commit_id":"7be36d59ac1cbfa75504cb7eda04844d0b32d06a"}],"nova/tests/functional/test_servers.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"eef08b54db39e0c79788af3e0a48e6d1bd04672e","unresolved":false,"context_lines":[{"line_number":3994,"context_line":"        # assert that unbind removes the allocation from the binding"},{"line_number":3995,"context_line":"        updated_port \u003d self.neutron.show_port(port[\u0027id\u0027])[\u0027port\u0027]"},{"line_number":3996,"context_line":"        binding_profile \u003d updated_port[\u0027binding:profile\u0027]"},{"line_number":3997,"context_line":"        self.assertNotIn(\u0027allocation\u0027, binding_profile)"},{"line_number":3998,"context_line":""},{"line_number":3999,"context_line":"    def test_boot_server_with_two_ports_one_having_resource_request(self):"},{"line_number":4000,"context_line":"        non_qos_port \u003d self.neutron.port_1"}],"source_content_type":"text/x-python","patch_set":19,"id":"5f7c97a3_d3a5a6b5","line":3997,"updated":"2018-06-21 14:12:02.000000000","message":"These asserts still fails as I need to implement the required behavior.","commit_id":"31d556e0a8fed127f90dd73ab2b160e6e10a73c8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a5ddfcf9ea13d2037ccb47943155af7d1046689b","unresolved":false,"context_lines":[{"line_number":3994,"context_line":"        # assert that unbind removes the allocation from the binding"},{"line_number":3995,"context_line":"        updated_port \u003d self.neutron.show_port(port[\u0027id\u0027])[\u0027port\u0027]"},{"line_number":3996,"context_line":"        binding_profile \u003d updated_port[\u0027binding:profile\u0027]"},{"line_number":3997,"context_line":"        self.assertNotIn(\u0027allocation\u0027, binding_profile)"},{"line_number":3998,"context_line":""},{"line_number":3999,"context_line":"    def test_boot_server_with_two_ports_one_having_resource_request(self):"},{"line_number":4000,"context_line":"        non_qos_port \u003d self.neutron.port_1"}],"source_content_type":"text/x-python","patch_set":19,"id":"5f7c97a3_993d0d2e","line":3997,"in_reply_to":"5f7c97a3_d3a5a6b5","updated":"2018-06-21 15:28:36.000000000","message":"Done","commit_id":"31d556e0a8fed127f90dd73ab2b160e6e10a73c8"}],"nova/tests/unit/network/test_neutronv2.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"412451b9b40d105f5345af40464f96fba3db54a2","unresolved":false,"context_lines":[{"line_number":4892,"context_line":"                          mock.sentinel.ctx, mock_inst, False,"},{"line_number":4893,"context_line":"                          requested_networks\u003dnw_req)"},{"line_number":4894,"context_line":""},{"line_number":4895,"context_line":"    @mock.patch(\u0027nova.objects.virtual_interface.VirtualInterface.create\u0027)"},{"line_number":4896,"context_line":"    @mock.patch(\u0027nova.network.neutronv2.api.API.\u0027"},{"line_number":4897,"context_line":"                \u0027_check_external_network_attach\u0027)"},{"line_number":4898,"context_line":"    @mock.patch(\u0027nova.network.neutronv2.api.API._show_port\u0027)"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_a9269515","line":4895,"range":{"start_line":4895,"start_character":4,"end_line":4895,"end_character":73},"updated":"2019-02-22 13:37:48.000000000","message":"This indicates we\u0027re mocking too deep. Can\u0027t we just mock out _update_ports_for_instance?","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9ee823473e2e7a7a7c4ebdf014b56b8de397e6d5","unresolved":false,"context_lines":[{"line_number":4892,"context_line":"                          mock.sentinel.ctx, mock_inst, False,"},{"line_number":4893,"context_line":"                          requested_networks\u003dnw_req)"},{"line_number":4894,"context_line":""},{"line_number":4895,"context_line":"    @mock.patch(\u0027nova.objects.virtual_interface.VirtualInterface.create\u0027)"},{"line_number":4896,"context_line":"    @mock.patch(\u0027nova.network.neutronv2.api.API.\u0027"},{"line_number":4897,"context_line":"                \u0027_check_external_network_attach\u0027)"},{"line_number":4898,"context_line":"    @mock.patch(\u0027nova.network.neutronv2.api.API._show_port\u0027)"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_68b88210","line":4895,"range":{"start_line":4895,"start_character":4,"end_line":4895,"end_character":73},"in_reply_to":"9fdfeff1_a9269515","updated":"2019-02-25 16:59:36.000000000","message":"see answer under your next comment.","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"412451b9b40d105f5345af40464f96fba3db54a2","unresolved":false,"context_lines":[{"line_number":4894,"context_line":""},{"line_number":4895,"context_line":"    @mock.patch(\u0027nova.objects.virtual_interface.VirtualInterface.create\u0027)"},{"line_number":4896,"context_line":"    @mock.patch(\u0027nova.network.neutronv2.api.API.\u0027"},{"line_number":4897,"context_line":"                \u0027_check_external_network_attach\u0027)"},{"line_number":4898,"context_line":"    @mock.patch(\u0027nova.network.neutronv2.api.API._show_port\u0027)"},{"line_number":4899,"context_line":"    @mock.patch(\u0027nova.network.neutronv2.api.API._update_port\u0027)"},{"line_number":4900,"context_line":"    @mock.patch(\u0027nova.network.neutronv2.api.get_client\u0027)"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_c92199fe","line":4897,"range":{"start_line":4897,"start_character":17,"end_line":4897,"end_character":47},"updated":"2019-02-22 13:37:48.000000000","message":"same - can\u0027t we mock out _validate_requested_network_ids?\n\nI guess it\u0027s up to you if you want to slim this down - you definitely want to know that the port\u0027s profile was updated with the allocation, but (1) you can do that by asserting the port in the dict passed to _update_ports_for_instance, and (2) you already validate that in the functional test wrinkle. Given (2), I would make this unit test as simple as possible.","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9ee823473e2e7a7a7c4ebdf014b56b8de397e6d5","unresolved":false,"context_lines":[{"line_number":4894,"context_line":""},{"line_number":4895,"context_line":"    @mock.patch(\u0027nova.objects.virtual_interface.VirtualInterface.create\u0027)"},{"line_number":4896,"context_line":"    @mock.patch(\u0027nova.network.neutronv2.api.API.\u0027"},{"line_number":4897,"context_line":"                \u0027_check_external_network_attach\u0027)"},{"line_number":4898,"context_line":"    @mock.patch(\u0027nova.network.neutronv2.api.API._show_port\u0027)"},{"line_number":4899,"context_line":"    @mock.patch(\u0027nova.network.neutronv2.api.API._update_port\u0027)"},{"line_number":4900,"context_line":"    @mock.patch(\u0027nova.network.neutronv2.api.get_client\u0027)"}],"source_content_type":"text/x-python","patch_set":67,"id":"9fdfeff1_282d9a94","line":4897,"range":{"start_line":4897,"start_character":17,"end_line":4897,"end_character":47},"in_reply_to":"9fdfeff1_c92199fe","updated":"2019-02-25 16:59:36.000000000","message":"both _validate_requested_port_ids() and _update_ports_for_instance() does many things and returns somewhat complex data. So I was not keen to simulate those behaviors in the test. It felt like internal behavior. In the other hand this test case now simulates the communication with neutron which is definitely external to the tested code.","commit_id":"7e58233c3fc278080470c0aed946feb68af5ad37"}]}