)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3bc17e621dcc71cec6c12fb8fb2f1fbca789e354","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"There are two things required here. Firstly, we need to start"},{"line_number":10,"context_line":"(consistently) storing the physnet and tunneled status of various"},{"line_number":11,"context_line":"networks in \u0027Instance.info_cache.network_meta\u0027. Once we have this"},{"line_number":12,"context_line":"information, we can use it to populate the"},{"line_number":13,"context_line":"\u0027RequestSpec.network_metadata\u0027, which can be consumed by later changes"},{"line_number":14,"context_line":"in the series."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5f7c97a3_7c2f1b89","line":11,"range":{"start_line":11,"start_character":33,"end_line":11,"end_character":45},"updated":"2018-07-24 16:12:49.000000000","message":"network_info","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3bc17e621dcc71cec6c12fb8fb2f1fbca789e354","unresolved":false,"context_lines":[{"line_number":13,"context_line":"\u0027RequestSpec.network_metadata\u0027, which can be consumed by later changes"},{"line_number":14,"context_line":"in the series."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Note that the only move operation that this won\u0027t handle is rebuilds,"},{"line_number":17,"context_line":"which result in a XML being regenerated but do not call to the scheduler"},{"line_number":18,"context_line":"or make a claim. This would require a far more extensive change and is"},{"line_number":19,"context_line":"probably unnecessary here."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5f7c97a3_fc9ecbb5","line":16,"range":{"start_line":16,"start_character":0,"end_line":16,"end_character":69},"updated":"2018-07-24 16:12:49.000000000","message":"Rebuild isn\u0027t a move operation at all, however, we call the scheduler during rebuild if the image changes (which it can), and we have the NUMATopologyFilter marked to run on rebuild because the image properties can affect the numa placement:\n\nhttps://github.com/openstack/nova/blob/50713f9cb77ef87002ec67a7c7bf33cfe0318404/nova/scheduler/filters/numa_topology_filter.py#L26\n\nhttps://github.com/openstack/nova/blob/50713f9cb77ef87002ec67a7c7bf33cfe0318404/nova/scheduler/filters/numa_topology_filter.py#L38-L40\n\nGiven ^, you need to handle the case that the image changes during rebuild and we call the scheduler to validate the new image.","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c0b00799bdc924e34dbcb5449cebdb536ff6c8c6","unresolved":false,"context_lines":[{"line_number":13,"context_line":"\u0027RequestSpec.network_metadata\u0027, which can be consumed by later changes"},{"line_number":14,"context_line":"in the series."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Note that the only move operation that this won\u0027t handle is rebuilds"},{"line_number":17,"context_line":"where the image is unchanged, which result in a XML being regenerated"},{"line_number":18,"context_line":"but do not call to the scheduler or make a claim, nor live migrations"},{"line_number":19,"context_line":"with a host specified, which bypass both scheduler and claims. Fixing"},{"line_number":20,"context_line":"these would require a far more extensive change and is probably"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"5f7c97a3_46c60918","line":17,"range":{"start_line":16,"start_character":0,"end_line":17,"end_character":29},"updated":"2018-07-24 18:20:54.000000000","message":"nit: this is still not a move operation. I would just remove this part about rebuild not being handled b/c it is handled properly. The add about forced live migration is good, so I\u0027d keep that and add the same about forced evacuate.","commit_id":"53c91f3d8528e60256b7f2bac1b9e6228d38ce0c"}],"nova/conductor/manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3bc17e621dcc71cec6c12fb8fb2f1fbca789e354","unresolved":false,"context_lines":[{"line_number":908,"context_line":"                # We only create a new allocation on the specified host if"},{"line_number":909,"context_line":"                # we\u0027re doing an evacuate since that is a move operation."},{"line_number":910,"context_line":"                if host !\u003d instance.host:"},{"line_number":911,"context_line":"                    # If a destination host is forced for evacuate, create"},{"line_number":912,"context_line":"                    # allocations against it in Placement."},{"line_number":913,"context_line":"                    self._allocate_for_evacuate_dest_host("},{"line_number":914,"context_line":"                        context, instance, host, request_spec)"},{"line_number":915,"context_line":"            else:"},{"line_number":916,"context_line":"                # At this point, the user is either:"},{"line_number":917,"context_line":"                #"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_fc21eb50","line":914,"range":{"start_line":911,"start_character":20,"end_line":914,"end_character":62},"updated":"2018-07-24 16:12:49.000000000","message":"We should be aware of this; this is the case that the admin has forced an evacuate to a specific host and it bypasses the scheduler so we won\u0027t go through the NUMATopologyFilter. In this case, limits will be None so the rebuild_claim in the compute won\u0027t do any validation that the instance\u0027s requested network numa affinity works for the host in the hardware.numa_fit_instance_to_host call during the claim in the RT. Granted, forcing a host during evacuate also means we don\u0027t check any cpu/ram/disk limits if the Core/RAM/DiskFilters were enabled (which they aren\u0027t if you\u0027re using placement). But is this something we\u0027d want to handle, i.e. calculating the network_metadata and putting that into the limits passed to rebuild_instance for the evacuate on the dest host?\n\nWhat happens if we don\u0027t? Worst case is the guest isn\u0027t pinned to the optimal NUMA nodes for it\u0027s network topology?","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"292dddbefdf2e4e729c2c4b8403d2ece55c2fb5a","unresolved":false,"context_lines":[{"line_number":908,"context_line":"                # We only create a new allocation on the specified host if"},{"line_number":909,"context_line":"                # we\u0027re doing an evacuate since that is a move operation."},{"line_number":910,"context_line":"                if host !\u003d instance.host:"},{"line_number":911,"context_line":"                    # If a destination host is forced for evacuate, create"},{"line_number":912,"context_line":"                    # allocations against it in Placement."},{"line_number":913,"context_line":"                    self._allocate_for_evacuate_dest_host("},{"line_number":914,"context_line":"                        context, instance, host, request_spec)"},{"line_number":915,"context_line":"            else:"},{"line_number":916,"context_line":"                # At this point, the user is either:"},{"line_number":917,"context_line":"                #"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_8618a1cd","line":914,"range":{"start_line":911,"start_character":20,"end_line":914,"end_character":62},"in_reply_to":"5f7c97a3_5327adb6","updated":"2018-07-24 18:18:36.000000000","message":"\u003e The workaround would likely be doing a rebuild, right?\n\nRebuild doesn\u0027t do a claim so it\u0027s not going to do anything through the hardware module - unless by rebuild you mean a fresh destroy/spawn of the guest. Then yeah that might fix it.","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e0e2677e025a5efc43cc14743ccc97684e040234","unresolved":false,"context_lines":[{"line_number":908,"context_line":"                # We only create a new allocation on the specified host if"},{"line_number":909,"context_line":"                # we\u0027re doing an evacuate since that is a move operation."},{"line_number":910,"context_line":"                if host !\u003d instance.host:"},{"line_number":911,"context_line":"                    # If a destination host is forced for evacuate, create"},{"line_number":912,"context_line":"                    # allocations against it in Placement."},{"line_number":913,"context_line":"                    self._allocate_for_evacuate_dest_host("},{"line_number":914,"context_line":"                        context, instance, host, request_spec)"},{"line_number":915,"context_line":"            else:"},{"line_number":916,"context_line":"                # At this point, the user is either:"},{"line_number":917,"context_line":"                #"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_5327adb6","line":914,"range":{"start_line":911,"start_character":20,"end_line":914,"end_character":62},"in_reply_to":"5f7c97a3_fc21eb50","updated":"2018-07-24 17:09:15.000000000","message":"\u003e What happens if we don\u0027t? Worst case is the guest isn\u0027t pinned to the optimal NUMA nodes for it\u0027s network topology?\n\nYes, exactly, which isn\u0027t the worst thing in the world, IMO. Given that we\u0027re not doing claims and scheduling here either, I assume this is also broken for CPU pinning, which this stuff would always be used with. The workaround would likely be doing a rebuild, right?","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3bc17e621dcc71cec6c12fb8fb2f1fbca789e354","unresolved":false,"context_lines":[{"line_number":915,"context_line":"            else:"},{"line_number":916,"context_line":"                # At this point, the user is either:"},{"line_number":917,"context_line":"                #"},{"line_number":918,"context_line":"                # 1. Doing a rebuild on the same host (not evacuate) and"},{"line_number":919,"context_line":"                #    specified a new image."},{"line_number":920,"context_line":"                # 2. Evacuating and specified a host but are not forcing it."},{"line_number":921,"context_line":"                #"},{"line_number":922,"context_line":"                # In either case, the API passes host\u003dNone but sets up the"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_bcd61354","line":919,"range":{"start_line":918,"start_character":16,"end_line":919,"end_character":43},"updated":"2018-07-24 16:12:49.000000000","message":"see","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3bc17e621dcc71cec6c12fb8fb2f1fbca789e354","unresolved":false,"context_lines":[{"line_number":951,"context_line":"                                                                instance,"},{"line_number":952,"context_line":"                                                                image_ref)"},{"line_number":953,"context_line":"                    request_spec.ensure_project_and_user_id(instance)"},{"line_number":954,"context_line":"                    request_spec.ensure_network_metadata(instance)"},{"line_number":955,"context_line":"                    compute_utils.heal_reqspec_is_bfv("},{"line_number":956,"context_line":"                        context, request_spec, instance)"},{"line_number":957,"context_line":"                    host_lists \u003d self._schedule_instances(context,"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_5ce81f14","line":954,"updated":"2018-07-24 16:12:49.000000000","message":"OK you might not have realized it (from the commit message), but this is where we\u0027ll call the scheduler for either evacuate or rebuild where the image changes during rebuild. So you\u0027re covered, but the commit message should be updated.","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e0e2677e025a5efc43cc14743ccc97684e040234","unresolved":false,"context_lines":[{"line_number":951,"context_line":"                                                                instance,"},{"line_number":952,"context_line":"                                                                image_ref)"},{"line_number":953,"context_line":"                    request_spec.ensure_project_and_user_id(instance)"},{"line_number":954,"context_line":"                    request_spec.ensure_network_metadata(instance)"},{"line_number":955,"context_line":"                    compute_utils.heal_reqspec_is_bfv("},{"line_number":956,"context_line":"                        context, request_spec, instance)"},{"line_number":957,"context_line":"                    host_lists \u003d self._schedule_instances(context,"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_135935fc","line":954,"in_reply_to":"5f7c97a3_5ce81f14","updated":"2018-07-24 17:09:15.000000000","message":"Done","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"}],"nova/conductor/tasks/live_migrate.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3bc17e621dcc71cec6c12fb8fb2f1fbca789e354","unresolved":false,"context_lines":[{"line_number":75,"context_line":"            # filters to run on the specified host, like a scheduler hint."},{"line_number":76,"context_line":"            self.destination, dest_node \u003d self._find_destination()"},{"line_number":77,"context_line":"        else:"},{"line_number":78,"context_line":"            # This is the case that the user specified the \u0027force\u0027 flag when"},{"line_number":79,"context_line":"            # live migrating with a specific destination host so the scheduler"},{"line_number":80,"context_line":"            # is bypassed. There are still some minimal checks performed here"},{"line_number":81,"context_line":"            # though."}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_1c406706","line":78,"updated":"2018-07-24 16:12:49.000000000","message":"Lets remember it\u0027s possible to bypass the scheduler during live migration and force to a specified destination host. We don\u0027t do any claims during live migration so I guess there isn\u0027t anything to do for numa stuff in this case, especially since live migration is not numa-aware, which is blueprint numa-aware-live-migration so I think it just means we don\u0027t have anything to worry about here.","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e0e2677e025a5efc43cc14743ccc97684e040234","unresolved":false,"context_lines":[{"line_number":75,"context_line":"            # filters to run on the specified host, like a scheduler hint."},{"line_number":76,"context_line":"            self.destination, dest_node \u003d self._find_destination()"},{"line_number":77,"context_line":"        else:"},{"line_number":78,"context_line":"            # This is the case that the user specified the \u0027force\u0027 flag when"},{"line_number":79,"context_line":"            # live migrating with a specific destination host so the scheduler"},{"line_number":80,"context_line":"            # is bypassed. There are still some minimal checks performed here"},{"line_number":81,"context_line":"            # though."}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_f33a7995","line":78,"in_reply_to":"5f7c97a3_1c406706","updated":"2018-07-24 17:09:15.000000000","message":"Agreed. There\u0027s very little we can do about that, unfortunately.","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"}],"nova/network/neutronv2/api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3bc17e621dcc71cec6c12fb8fb2f1fbca789e354","unresolved":false,"context_lines":[{"line_number":2589,"context_line":"                             if fixed_ip.is_in_subnet(subnet)]"},{"line_number":2590,"context_line":"        return subnets"},{"line_number":2591,"context_line":""},{"line_number":2592,"context_line":"    def _nw_info_build_network(self, context, port, networks, subnets):"},{"line_number":2593,"context_line":"        neutron \u003d get_client(context, admin\u003dTrue)"},{"line_number":2594,"context_line":"        network_name \u003d None"},{"line_number":2595,"context_line":"        network_mtu \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_bc9cd35c","line":2592,"range":{"start_line":2592,"start_character":37,"end_line":2592,"end_character":44},"updated":"2018-07-24 16:12:49.000000000","message":"nit: like in _get_subnets_from_port, we could pass the context and optionally a client object that\u0027s already created (which I think it would be in the cases that _nw_info_build_network is called) and then we don\u0027t need to construct another client if we can just pass one through. That could be a follow up optimization though.","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e0e2677e025a5efc43cc14743ccc97684e040234","unresolved":false,"context_lines":[{"line_number":2589,"context_line":"                             if fixed_ip.is_in_subnet(subnet)]"},{"line_number":2590,"context_line":"        return subnets"},{"line_number":2591,"context_line":""},{"line_number":2592,"context_line":"    def _nw_info_build_network(self, context, port, networks, subnets):"},{"line_number":2593,"context_line":"        neutron \u003d get_client(context, admin\u003dTrue)"},{"line_number":2594,"context_line":"        network_name \u003d None"},{"line_number":2595,"context_line":"        network_mtu \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_d3bc1d09","line":2592,"range":{"start_line":2592,"start_character":37,"end_line":2592,"end_character":44},"in_reply_to":"5f7c97a3_bc9cd35c","updated":"2018-07-24 17:09:15.000000000","message":"Ah, I was wondering why we did that. Let\u0027s keep it this way just because time is short and it\u0027s an easy clean up for after","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3bc17e621dcc71cec6c12fb8fb2f1fbca789e354","unresolved":false,"context_lines":[{"line_number":2590,"context_line":"        return subnets"},{"line_number":2591,"context_line":""},{"line_number":2592,"context_line":"    def _nw_info_build_network(self, context, port, networks, subnets):"},{"line_number":2593,"context_line":"        neutron \u003d get_client(context, admin\u003dTrue)"},{"line_number":2594,"context_line":"        network_name \u003d None"},{"line_number":2595,"context_line":"        network_mtu \u003d None"},{"line_number":2596,"context_line":"        for net in networks:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_3c9be379","line":2593,"range":{"start_line":2593,"start_character":38,"end_line":2593,"end_character":48},"updated":"2018-07-24 16:12:49.000000000","message":"I\u0027m just assuming we need admin here based on the API reference for the extensions involved in _get_physnet_tunneled_info:\n\nhttps://developer.openstack.org/api-ref/network/v2/index.html#multiple-provider-extension\n\nhttps://developer.openstack.org/api-ref/network/v2/index.html#provider-extended-attributes","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e0e2677e025a5efc43cc14743ccc97684e040234","unresolved":false,"context_lines":[{"line_number":2590,"context_line":"        return subnets"},{"line_number":2591,"context_line":""},{"line_number":2592,"context_line":"    def _nw_info_build_network(self, context, port, networks, subnets):"},{"line_number":2593,"context_line":"        neutron \u003d get_client(context, admin\u003dTrue)"},{"line_number":2594,"context_line":"        network_name \u003d None"},{"line_number":2595,"context_line":"        network_mtu \u003d None"},{"line_number":2596,"context_line":"        for net in networks:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_73b98914","line":2593,"range":{"start_line":2593,"start_character":38,"end_line":2593,"end_character":48},"in_reply_to":"5f7c97a3_3c9be379","updated":"2018-07-24 17:09:15.000000000","message":"Yup, \u0027zactly","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"15e8282bf78ddd7fdf20ec61a117069f991c5874","unresolved":false,"context_lines":[{"line_number":2653,"context_line":"        network[\u0027subnets\u0027] \u003d subnets"},{"line_number":2654,"context_line":"        physnet, tunneled \u003d self._get_physnet_tunneled_info("},{"line_number":2655,"context_line":"            context, neutron, port[\u0027network_id\u0027])"},{"line_number":2656,"context_line":"        network[\u0027physical_network\u0027], network[\u0027tunneled\u0027] \u003d physnet, tunneled"},{"line_number":2657,"context_line":""},{"line_number":2658,"context_line":"        if should_create_bridge is not None:"},{"line_number":2659,"context_line":"            network[\u0027should_create_bridge\u0027] \u003d should_create_bridge"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_b06397de","line":2656,"updated":"2018-07-24 16:22:13.000000000","message":"Are you sure this works? Because I don\u0027t see any setter on the Network model object that automatically puts stuff into \u0027meta\u0027:\n\nhttps://github.com/openstack/nova/blob/50713f9cb77ef87002ec67a7c7bf33cfe0318404/nova/network/model.py#L324\n\nThe unit tests on the request spec are working b/c they are constructing the Network model object with the meta keys directly, not like this. To resolve this, you\u0027d need to get the physical_network and tunneled keys and pass them into Netowrk init during construction.","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e0e2677e025a5efc43cc14743ccc97684e040234","unresolved":false,"context_lines":[{"line_number":2653,"context_line":"        network[\u0027subnets\u0027] \u003d subnets"},{"line_number":2654,"context_line":"        physnet, tunneled \u003d self._get_physnet_tunneled_info("},{"line_number":2655,"context_line":"            context, neutron, port[\u0027network_id\u0027])"},{"line_number":2656,"context_line":"        network[\u0027physical_network\u0027], network[\u0027tunneled\u0027] \u003d physnet, tunneled"},{"line_number":2657,"context_line":""},{"line_number":2658,"context_line":"        if should_create_bridge is not None:"},{"line_number":2659,"context_line":"            network[\u0027should_create_bridge\u0027] \u003d should_create_bridge"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_53048dc5","line":2656,"in_reply_to":"5f7c97a3_b06397de","updated":"2018-07-24 17:09:15.000000000","message":"I\u0027m not sure how but it does. See the original version of this, which set the \u0027physical_network\u0027 attribute on the object without issue.\n\nThe functional tests are killing me atm but I\u0027ll manually test this again in the morning to be sure.","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bd371388fbab4107c21c8c075fb24cf50d459046","unresolved":false,"context_lines":[{"line_number":2653,"context_line":"        network[\u0027subnets\u0027] \u003d subnets"},{"line_number":2654,"context_line":"        physnet, tunneled \u003d self._get_physnet_tunneled_info("},{"line_number":2655,"context_line":"            context, neutron, port[\u0027network_id\u0027])"},{"line_number":2656,"context_line":"        network[\u0027physical_network\u0027], network[\u0027tunneled\u0027] \u003d physnet, tunneled"},{"line_number":2657,"context_line":""},{"line_number":2658,"context_line":"        if should_create_bridge is not None:"},{"line_number":2659,"context_line":"            network[\u0027should_create_bridge\u0027] \u003d should_create_bridge"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_e9028608","line":2656,"updated":"2018-07-24 18:28:11.000000000","message":"This doesn\u0027t work. I changed your request spec test and it shows it doesn\u0027t work:\n\ndiff --git a/nova/tests/unit/objects/test_request_spec.py b/nova/tests/unit/objects/test_request_spec.py\nindex 8f07d39..adabf3e 100644\n--- a/nova/tests/unit/objects/test_request_spec.py\n+++ b/nova/tests/unit/objects/test_request_spec.py\n@@ -509,8 +509,9 @@ class _TestRequestSpecObject(object):\n         network_b \u003d fake_network_cache_model.new_network({\n             \u0027physical_network\u0027: \u0027foo\u0027, \u0027tunneled\u0027: False})\n         vif_b \u003d fake_network_cache_model.new_vif({\u0027network\u0027: network_b})\n-        network_c \u003d fake_network_cache_model.new_network({\n-            \u0027physical_network\u0027: \u0027bar\u0027, \u0027tunneled\u0027: False})\n+        network_c \u003d fake_network_cache_model.new_network()\n+        network_c[\u0027physical_network\u0027] \u003d \u0027bar\u0027\n+        network_c[\u0027tunneled\u0027] \u003d False\n         vif_c \u003d fake_network_cache_model.new_vif({\u0027network\u0027: network_c})\n         network_d \u003d fake_network_cache_model.new_network({\n             \u0027physical_network\u0027: None, \u0027tunneled\u0027: True})\nosboxes@osboxes:~/git/nova$ \n\n\nnova.tests.unit.objects.test_request_spec.TestRequestSpecObject.test_ensure_network_metadata\n--------------------------------------------------------------------------------------------\n\nCaptured pythonlogging:\n~~~~~~~~~~~~~~~~~~~~~~~\n    2018-07-24 14:28:02,867 INFO [248_add_expire_reservations_index] Skipped adding reservations_deleted_expire_idx because an equivalent index already exists.\n    \n\nCaptured traceback:\n~~~~~~~~~~~~~~~~~~~\n    Traceback (most recent call last):\n      File \"nova/tests/unit/objects/test_request_spec.py\", line 531, in test_ensure_network_metadata\n        self.assertEqual(spec.network_metadata.physnets, set([\u0027foo\u0027, \u0027bar\u0027]))\n      File \"/home/osboxes/git/nova/.tox/py27/local/lib/python2.7/site-packages/testtools/testcase.py\", line 411, in assertEqual\n        self.assertThat(observed, matcher, message)\n      File \"/home/osboxes/git/nova/.tox/py27/local/lib/python2.7/site-packages/testtools/testcase.py\", line 498, in assertThat\n        raise mismatch_error\n    testtools.matchers._impl.MismatchError: CoercedSet([u\u0027foo\u0027]) !\u003d set([\u0027bar\u0027, \u0027foo\u0027])","commit_id":"53c91f3d8528e60256b7f2bac1b9e6228d38ce0c"}],"nova/objects/request_spec.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3bc17e621dcc71cec6c12fb8fb2f1fbca789e354","unresolved":false,"context_lines":[{"line_number":470,"context_line":"            self.user_id \u003d instance.user_id"},{"line_number":471,"context_line":""},{"line_number":472,"context_line":"    def ensure_network_metadata(self, instance):"},{"line_number":473,"context_line":"        if not (instance.info_cache and instance.info_cache.network_info):"},{"line_number":474,"context_line":"            return"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"        physnets \u003d set([])"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_dcc74f3d","line":473,"updated":"2018-07-24 16:12:49.000000000","message":"You could just call instance.get_network_info() here and it will handle the null checks, but then you might not know if you should leave self.network_metadata unset, so I guess it\u0027s probably better to be explicit here.","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3bc17e621dcc71cec6c12fb8fb2f1fbca789e354","unresolved":false,"context_lines":[{"line_number":476,"context_line":"        physnets \u003d set([])"},{"line_number":477,"context_line":"        tunneled \u003d True"},{"line_number":478,"context_line":""},{"line_number":479,"context_line":"        for vif in instance.info_cache.network_info:"},{"line_number":480,"context_line":"            physnet \u003d vif.get(\u0027network\u0027, {}).get(\u0027meta\u0027, {}).get("},{"line_number":481,"context_line":"                \u0027physical_network\u0027, None)"},{"line_number":482,"context_line":"            if physnet:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_5c057f78","line":479,"updated":"2018-07-24 16:12:49.000000000","message":"Might be good to leave a comment here for maintainers that physical_network and tunneled might not be in the cache for old instances that haven\u0027t had their info_cache healed yet.","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e0e2677e025a5efc43cc14743ccc97684e040234","unresolved":false,"context_lines":[{"line_number":476,"context_line":"        physnets \u003d set([])"},{"line_number":477,"context_line":"        tunneled \u003d True"},{"line_number":478,"context_line":""},{"line_number":479,"context_line":"        for vif in instance.info_cache.network_info:"},{"line_number":480,"context_line":"            physnet \u003d vif.get(\u0027network\u0027, {}).get(\u0027meta\u0027, {}).get("},{"line_number":481,"context_line":"                \u0027physical_network\u0027, None)"},{"line_number":482,"context_line":"            if physnet:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_f3c59991","line":479,"in_reply_to":"5f7c97a3_5c057f78","updated":"2018-07-24 17:09:15.000000000","message":"Done","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"}],"nova/tests/unit/conductor/tasks/test_live_migrate.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4197bfdbf822a60c1c3de10a4c7db964fe79cc2e","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        self.heal_reqspec_is_bfv_mock \u003d _p.start()"},{"line_number":63,"context_line":"        self.addCleanup(_p.stop)"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        _p \u003d mock.patch(\u0027nova.objects.RequestSpec.ensure_network_metadata\u0027)"},{"line_number":66,"context_line":"        self.ensure_network_metadata_mock \u003d _p.start()"},{"line_number":67,"context_line":"        self.addCleanup(_p.stop)"},{"line_number":68,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_3bc1adfe","line":65,"updated":"2018-07-24 14:32:15.000000000","message":"This pretty much ensures that this whole file won\u0027t test the meat of what you\u0027re adding here.","commit_id":"cbe0aa2711bdd3c8013d1beb3af28674cc89fdde"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f0df337e87d1a31ac8ba8c981f128d1f86150030","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        self.heal_reqspec_is_bfv_mock \u003d _p.start()"},{"line_number":63,"context_line":"        self.addCleanup(_p.stop)"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        _p \u003d mock.patch(\u0027nova.objects.RequestSpec.ensure_network_metadata\u0027)"},{"line_number":66,"context_line":"        self.ensure_network_metadata_mock \u003d _p.start()"},{"line_number":67,"context_line":"        self.addCleanup(_p.stop)"},{"line_number":68,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_5c949fe7","line":65,"in_reply_to":"5f7c97a3_3bc1adfe","updated":"2018-07-24 16:14:45.000000000","message":"It\u0027s following the heal_reqspec_is_bfv_mock pattern above, and does assert when ensure_network_metadata is called and not called in two specific places which cover the change. Mocking it in *every* test it impacts would be overdoing it IMO which is why I went with this style for heal_reqspec_is_bfv_mock in the first place.","commit_id":"cbe0aa2711bdd3c8013d1beb3af28674cc89fdde"}],"nova/tests/unit/conductor/tasks/test_migrate.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4197bfdbf822a60c1c3de10a4c7db964fe79cc2e","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        self.heal_reqspec_is_bfv_mock \u003d _p.start()"},{"line_number":56,"context_line":"        self.addCleanup(_p.stop)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"        _p \u003d mock.patch(\u0027nova.objects.RequestSpec.ensure_network_metadata\u0027)"},{"line_number":59,"context_line":"        self.ensure_network_metadata_mock \u003d _p.start()"},{"line_number":60,"context_line":"        self.addCleanup(_p.stop)"},{"line_number":61,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_bbd4ddbd","line":58,"updated":"2018-07-24 14:32:15.000000000","message":"Same here.","commit_id":"cbe0aa2711bdd3c8013d1beb3af28674cc89fdde"}],"nova/tests/unit/objects/test_request_spec.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3bc17e621dcc71cec6c12fb8fb2f1fbca789e354","unresolved":false,"context_lines":[{"line_number":506,"context_line":"        network_a \u003d fake_network_cache_model.new_network({"},{"line_number":507,"context_line":"            \u0027physical_network\u0027: \u0027foo\u0027, \u0027tunneled\u0027: False})"},{"line_number":508,"context_line":"        vif_a \u003d fake_network_cache_model.new_vif({\u0027network\u0027: network_a})"},{"line_number":509,"context_line":"        network_b \u003d fake_network_cache_model.new_network({"},{"line_number":510,"context_line":"            \u0027physical_network\u0027: \u0027foo\u0027, \u0027tunneled\u0027: False})"},{"line_number":511,"context_line":"        vif_b \u003d fake_network_cache_model.new_vif({\u0027network\u0027: network_b})"},{"line_number":512,"context_line":"        network_c \u003d fake_network_cache_model.new_network({"},{"line_number":513,"context_line":"            \u0027physical_network\u0027: None, \u0027tunneled\u0027: True})"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_9c6d5731","line":510,"range":{"start_line":509,"start_character":8,"end_line":510,"end_character":58},"updated":"2018-07-24 16:12:49.000000000","message":"Did you mean to make this exactly the same as network_a? Why not use a different physnet name so we can see we have \u003e1 entry in the set?","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e0e2677e025a5efc43cc14743ccc97684e040234","unresolved":false,"context_lines":[{"line_number":506,"context_line":"        network_a \u003d fake_network_cache_model.new_network({"},{"line_number":507,"context_line":"            \u0027physical_network\u0027: \u0027foo\u0027, \u0027tunneled\u0027: False})"},{"line_number":508,"context_line":"        vif_a \u003d fake_network_cache_model.new_vif({\u0027network\u0027: network_a})"},{"line_number":509,"context_line":"        network_b \u003d fake_network_cache_model.new_network({"},{"line_number":510,"context_line":"            \u0027physical_network\u0027: \u0027foo\u0027, \u0027tunneled\u0027: False})"},{"line_number":511,"context_line":"        vif_b \u003d fake_network_cache_model.new_vif({\u0027network\u0027: network_b})"},{"line_number":512,"context_line":"        network_c \u003d fake_network_cache_model.new_network({"},{"line_number":513,"context_line":"            \u0027physical_network\u0027: None, \u0027tunneled\u0027: True})"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_70289f97","line":510,"range":{"start_line":509,"start_character":8,"end_line":510,"end_character":58},"in_reply_to":"5f7c97a3_9c6d5731","updated":"2018-07-24 17:09:15.000000000","message":"Yeah, I wanted to ensure things were getting normalized. Maybe I should add yet another one","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6ccc5bf7eb9cd08f291c828dc7d6d31bd684fca8","unresolved":false,"context_lines":[{"line_number":509,"context_line":"        network_b \u003d fake_network_cache_model.new_network({"},{"line_number":510,"context_line":"            \u0027physical_network\u0027: \u0027foo\u0027, \u0027tunneled\u0027: False})"},{"line_number":511,"context_line":"        vif_b \u003d fake_network_cache_model.new_vif({\u0027network\u0027: network_b})"},{"line_number":512,"context_line":"        network_c \u003d fake_network_cache_model.new_network({"},{"line_number":513,"context_line":"            \u0027physical_network\u0027: None, \u0027tunneled\u0027: True})"},{"line_number":514,"context_line":"        vif_c \u003d fake_network_cache_model.new_vif({\u0027network\u0027: network_c})"},{"line_number":515,"context_line":"        nw_info \u003d network_model.NetworkInfo([vif_a, vif_b, vif_c])"},{"line_number":516,"context_line":"        info_cache \u003d objects.InstanceInfoCache(network_info\u003dnw_info,"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_70b2df4c","line":513,"range":{"start_line":512,"start_character":20,"end_line":513,"end_character":56},"updated":"2018-07-24 16:23:39.000000000","message":"This isn\u0027t how the neutronv2 api code is actually setting the values in the Network model object and I think it\u0027s hiding the fact the code doesn\u0027t work.","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3bc17e621dcc71cec6c12fb8fb2f1fbca789e354","unresolved":false,"context_lines":[{"line_number":527,"context_line":"        self.assertEqual(spec.network_metadata.physnets, set([\u0027foo\u0027]))"},{"line_number":528,"context_line":"        self.assertTrue(spec.network_metadata.tunneled)"},{"line_number":529,"context_line":""},{"line_number":530,"context_line":"    def test_ensure_network_metadata__missing(self):"},{"line_number":531,"context_line":"        nw_info \u003d network_model.NetworkInfo([])"},{"line_number":532,"context_line":"        info_cache \u003d objects.InstanceInfoCache(network_info\u003dnw_info,"},{"line_number":533,"context_line":"                                               instance_uuid\u003duuids.instance)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_fc556b53","line":530,"range":{"start_line":530,"start_character":36,"end_line":530,"end_character":38},"updated":"2018-07-24 16:12:49.000000000","message":"_","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e0e2677e025a5efc43cc14743ccc97684e040234","unresolved":false,"context_lines":[{"line_number":527,"context_line":"        self.assertEqual(spec.network_metadata.physnets, set([\u0027foo\u0027]))"},{"line_number":528,"context_line":"        self.assertTrue(spec.network_metadata.tunneled)"},{"line_number":529,"context_line":""},{"line_number":530,"context_line":"    def test_ensure_network_metadata__missing(self):"},{"line_number":531,"context_line":"        nw_info \u003d network_model.NetworkInfo([])"},{"line_number":532,"context_line":"        info_cache \u003d objects.InstanceInfoCache(network_info\u003dnw_info,"},{"line_number":533,"context_line":"                                               instance_uuid\u003duuids.instance)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_b391619e","line":530,"range":{"start_line":530,"start_character":36,"end_line":530,"end_character":38},"in_reply_to":"5f7c97a3_fc556b53","updated":"2018-07-24 17:09:15.000000000","message":"Done","commit_id":"a9335d98fa3eaa9d23097a91174e3fde051f1470"}]}