)]}'
{"neutron/cmd/upgrade_checks/checks.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"2c841d84b72fecbd8c67a5144d0fb75f34de2789","unresolved":true,"context_lines":[{"line_number":124,"context_line":"            (_(\u0027Policy File JSON to YAML Migration\u0027),"},{"line_number":125,"context_line":"             (common_checks.check_policy_json, {\u0027conf\u0027: cfg.CONF})),"},{"line_number":126,"context_line":"            (_(\u0027Port MAC address sanity check\u0027),"},{"line_number":127,"context_line":"             self.port_mac_address_sanity),"},{"line_number":128,"context_line":"        ]"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":9,"id":"173bd19a_b129c1ec","line":127,"updated":"2021-10-06 14:02:27.000000000","message":"You need to add new check to that list too","commit_id":"0e0f78b8ede6fad8b4e8cc454c8f8aed4e0ec321"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2d8c601e64528f6e3efc19e406ce1470739cdfa2","unresolved":true,"context_lines":[{"line_number":102,"context_line":"        segment.NetworkSegment.network_type,"},{"line_number":103,"context_line":"        segment.NetworkSegment.physical_network"},{"line_number":104,"context_line":"    )"},{"line_number":105,"context_line":"    query \u003d query.having(func.count(segment.NetworkSegment.id) \u003e 2)"},{"line_number":106,"context_line":"    return query.count()"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"5f2fb74a_920aa32f","line":105,"range":{"start_line":105,"start_character":25,"end_line":105,"end_character":62},"updated":"2021-10-07 10:17:25.000000000","message":"1) Why counting a column you didn\u0027t request in the query? This should not work. It could if the count() is in the query itself\n2) This should be just func.count(), to count the number of registers matching this query and the group_by filter.","commit_id":"904f6c3f7ebb385b6c1b24a1fcde0bdf1c377615"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"754da4425be94dec794d3537c06228239847e575","unresolved":true,"context_lines":[{"line_number":102,"context_line":"        segment.NetworkSegment.network_type,"},{"line_number":103,"context_line":"        segment.NetworkSegment.physical_network"},{"line_number":104,"context_line":"    )"},{"line_number":105,"context_line":"    query \u003d query.having(func.count(segment.NetworkSegment.id) \u003e 2)"},{"line_number":106,"context_line":"    return query.count()"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"18ebb345_061ca877","line":105,"range":{"start_line":105,"start_character":63,"end_line":105,"end_character":67},"updated":"2021-10-07 10:20:24.000000000","message":"Why \"\u003e 2\"? We need to find all queries \"\u003e 1\", right?","commit_id":"904f6c3f7ebb385b6c1b24a1fcde0bdf1c377615"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"ae8c08874f2f3acd7bf795c7aaa4904de6c79ad5","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        segment.NetworkSegment.network_type,"},{"line_number":103,"context_line":"        segment.NetworkSegment.physical_network"},{"line_number":104,"context_line":"    )"},{"line_number":105,"context_line":"    query \u003d query.having(func.count(segment.NetworkSegment.id) \u003e 2)"},{"line_number":106,"context_line":"    return query.count()"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"100836d4_ee9f0cd1","line":105,"range":{"start_line":105,"start_character":63,"end_line":105,"end_character":67},"in_reply_to":"18ebb345_061ca877","updated":"2021-10-07 12:51:31.000000000","message":"Yes, this should definitely be 1.","commit_id":"904f6c3f7ebb385b6c1b24a1fcde0bdf1c377615"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"ae8c08874f2f3acd7bf795c7aaa4904de6c79ad5","unresolved":true,"context_lines":[{"line_number":102,"context_line":"        segment.NetworkSegment.network_type,"},{"line_number":103,"context_line":"        segment.NetworkSegment.physical_network"},{"line_number":104,"context_line":"    )"},{"line_number":105,"context_line":"    query \u003d query.having(func.count(segment.NetworkSegment.id) \u003e 2)"},{"line_number":106,"context_line":"    return query.count()"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"91c9d4bc_7b6d50a1","line":105,"range":{"start_line":105,"start_character":25,"end_line":105,"end_character":62},"in_reply_to":"30c76d51_c726a90e","updated":"2021-10-07 12:51:31.000000000","message":"Exactly, we group by the three fields, then look at all groups that we have found more than one segment for. I don\u0027t think it matters much if we count the NetworkSegment.id or * in this case. I\u0027ve tried out both queries in my qa landscape. For comparison, here\u0027s the SQL for them, starting from query \u003d query.group_by(...):\n\n\u003e\u003e\u003e print(query.having(func.count() \u003e 1))\nSELECT networksegments.network_id AS networksegments_network_id \nFROM networksegments \nWHERE networksegments.physical_network IS NOT NULL GROUP BY networksegments.network_id, networksegments.network_type, networksegments.physical_network \nHAVING count(*) \u003e %(count_1)s\n\n\u003e\u003e\u003e print(query.having(func.count(segment.NetworkSegment.id) \u003e 1))\nSELECT networksegments.network_id AS networksegments_network_id \nFROM networksegments \nWHERE networksegments.physical_network IS NOT NULL GROUP BY networksegments.network_id, networksegments.network_type, networksegments.physical_network \nHAVING count(networksegments.id) \u003e %(count_1)s\n\nFor patchset 11 I\u0027ve adopted your first recommendation, but I can switch back to segment.NetworkSegment.id, as both should work.","commit_id":"904f6c3f7ebb385b6c1b24a1fcde0bdf1c377615"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"754da4425be94dec794d3537c06228239847e575","unresolved":true,"context_lines":[{"line_number":102,"context_line":"        segment.NetworkSegment.network_type,"},{"line_number":103,"context_line":"        segment.NetworkSegment.physical_network"},{"line_number":104,"context_line":"    )"},{"line_number":105,"context_line":"    query \u003d query.having(func.count(segment.NetworkSegment.id) \u003e 2)"},{"line_number":106,"context_line":"    return query.count()"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"30c76d51_c726a90e","line":105,"range":{"start_line":105,"start_character":25,"end_line":105,"end_character":62},"in_reply_to":"5f2fb74a_920aa32f","updated":"2021-10-07 10:20:24.000000000","message":"Actually no because the network_id could be repeated. This should be:\n\n    query \u003d ctx.session.query(segment.NetworkSegment.network_id,\n                              func.count(segment.NetworkSegment.id))\n    # for a unique constraint it\u0027s always NULL !\u003d NULL --\u003e we filter them out\n    query \u003d query.filter(segment.NetworkSegment.physical_network.isnot(None))\n    query \u003d query.group_by(\n        segment.NetworkSegment.network_id,\n        segment.NetworkSegment.network_type,\n        segment.NetworkSegment.physical_network\n    )\n    query \u003d query.having(func.count(segment.NetworkSegment.id) \u003e 1)","commit_id":"904f6c3f7ebb385b6c1b24a1fcde0bdf1c377615"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ff30404b3a71a7305e8d2c14ce744d5a8e52eb14","unresolved":true,"context_lines":[{"line_number":102,"context_line":"        segment.NetworkSegment.network_type,"},{"line_number":103,"context_line":"        segment.NetworkSegment.physical_network"},{"line_number":104,"context_line":"    )"},{"line_number":105,"context_line":"    query \u003d query.having(func.count() \u003e 1)"},{"line_number":106,"context_line":"    return query.count()"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"84166a54_5b6d5fee","line":105,"range":{"start_line":105,"start_character":3,"end_line":105,"end_character":42},"updated":"2021-10-07 14:20:03.000000000","message":"I\u0027ve tested this manually and works fine.","commit_id":"b993ebb407208bf04a7957a3d15c9ac482a1dd5c"}],"neutron/db/migration/alembic_migrations/versions/xena/expand/e981acd076d3_add_networksegments_database_constraint.py":[{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"bf48039e54067443b50a049fa2f202b5bbed3958","unresolved":true,"context_lines":[{"line_number":33,"context_line":"    op.create_unique_constraint("},{"line_number":34,"context_line":"        \u0027uniq_networksegment0network_id0network_type0physical_network\u0027,"},{"line_number":35,"context_line":"        \u0027networksegments\u0027,"},{"line_number":36,"context_line":"        [\u0027network_id\u0027, \u0027network_type\u0027, \u0027physical_network\u0027]"},{"line_number":37,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":2,"id":"0076a341_d7273af1","line":36,"range":{"start_line":36,"start_character":9,"end_line":36,"end_character":57},"updated":"2021-04-30 07:23:02.000000000","message":"How could we have 2 records with same network_id but different network_type or physical_network? Can you please clarify? Seems just network_id should be enough.","commit_id":"e2fff3522564aa392c94bca9188f03e00e7f7624"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"5832730cd4736f5fd3e2fa5e824f10cc97776217","unresolved":true,"context_lines":[{"line_number":33,"context_line":"    op.create_unique_constraint("},{"line_number":34,"context_line":"        \u0027uniq_networksegment0network_id0network_type0physical_network\u0027,"},{"line_number":35,"context_line":"        \u0027networksegments\u0027,"},{"line_number":36,"context_line":"        [\u0027network_id\u0027, \u0027network_type\u0027, \u0027physical_network\u0027]"},{"line_number":37,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":2,"id":"855c437d_7ed650eb","line":36,"range":{"start_line":36,"start_character":9,"end_line":36,"end_character":57},"in_reply_to":"0076a341_d7273af1","updated":"2021-04-30 10:24:25.000000000","message":"This is quite common with hierarchical portbinding[0]. One could have a network_type VXLAN top-level segment and then a multitude of network_type VLAN segments under it. The table ml2_port_binding_levels (model neutron.plugins.ml2.models.PortBindingLevels) will then contain multiple entries per port binding, each line referencing a different segment.\n\nHaving only network_id in this constraint would break hierarchical portbinding. \n\n\n[0] https://specs.openstack.org/openstack/neutron-specs/specs/kilo/ml2-hierarchical-port-binding.html","commit_id":"e2fff3522564aa392c94bca9188f03e00e7f7624"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"27ec18a3b7f0a2f94177e1ec3da36e5ef3786f6d","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    op.create_unique_constraint("},{"line_number":34,"context_line":"        \u0027uniq_networksegment0network_id0network_type0physical_network\u0027,"},{"line_number":35,"context_line":"        \u0027networksegments\u0027,"},{"line_number":36,"context_line":"        [\u0027network_id\u0027, \u0027network_type\u0027, \u0027physical_network\u0027]"},{"line_number":37,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a611e9d_6e33b04c","line":36,"range":{"start_line":36,"start_character":9,"end_line":36,"end_character":57},"in_reply_to":"24ccc5f9_df773634","updated":"2021-09-01 14:20:48.000000000","message":"Done","commit_id":"e2fff3522564aa392c94bca9188f03e00e7f7624"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"626af1ddaf9288d9ac6d271b051c6376a764f188","unresolved":true,"context_lines":[{"line_number":33,"context_line":"    op.create_unique_constraint("},{"line_number":34,"context_line":"        \u0027uniq_networksegment0network_id0network_type0physical_network\u0027,"},{"line_number":35,"context_line":"        \u0027networksegments\u0027,"},{"line_number":36,"context_line":"        [\u0027network_id\u0027, \u0027network_type\u0027, \u0027physical_network\u0027]"},{"line_number":37,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":2,"id":"24ccc5f9_df773634","line":36,"range":{"start_line":36,"start_character":9,"end_line":36,"end_character":57},"in_reply_to":"66537424_84b05030","updated":"2021-07-07 17:34:36.000000000","message":"In hierarchical port binding, the network has one tunnelled network (VXLAN, geneve, GRE) and multiple VLAN type segments. Each VLAN network will be assigned to a different physical network.\n\nIn RPN [1], the segments are VLAN type. Each one is assigned to a different physical network. That is what will segregate the traffic between racks.\n\nReviewing the code and the current features we have in Neutron, I think this constraint is valid.\n\n[1]https://docs.openstack.org/neutron/pike/admin/config-routed-networks.html","commit_id":"e2fff3522564aa392c94bca9188f03e00e7f7624"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"8fce1d7ac641c04f4628d256e896cf310d55c2be","unresolved":true,"context_lines":[{"line_number":33,"context_line":"    op.create_unique_constraint("},{"line_number":34,"context_line":"        \u0027uniq_networksegment0network_id0network_type0physical_network\u0027,"},{"line_number":35,"context_line":"        \u0027networksegments\u0027,"},{"line_number":36,"context_line":"        [\u0027network_id\u0027, \u0027network_type\u0027, \u0027physical_network\u0027]"},{"line_number":37,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":2,"id":"66537424_84b05030","line":36,"range":{"start_line":36,"start_character":9,"end_line":36,"end_character":57},"in_reply_to":"855c437d_7ed650eb","updated":"2021-05-13 05:01:19.000000000","message":"Ack, thanks for clarification, I forgot about hierarchical portbinding.","commit_id":"e2fff3522564aa392c94bca9188f03e00e7f7624"}],"neutron/plugins/ml2/managers.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"5016b0a4568560bd9c4570204cbfcd4ff53e4c74","unresolved":true,"context_lines":[{"line_number":341,"context_line":"                      \"network type is not supported.\", segment)"},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"    @db_api.retry_db_errors"},{"line_number":344,"context_line":"    def allocate_dynamic_segment(self, context, network_id, segment):"},{"line_number":345,"context_line":"        \"\"\"Allocate a dynamic segment using a partial or full segment dict.\"\"\""},{"line_number":346,"context_line":"        dynamic_segment \u003d segments_db.get_dynamic_segment("},{"line_number":347,"context_line":"            context, network_id, segment.get(api.PHYSICAL_NETWORK),"}],"source_content_type":"text/x-python","patch_set":2,"id":"470edde4_9605ad15","line":344,"range":{"start_line":344,"start_character":8,"end_line":344,"end_character":32},"updated":"2021-05-12 07:37:22.000000000","message":"Checking now the code, where this method (and \"release_dynamic_segment\") is called?\n\nTo allocate a segment, we have \"create_network_segments\" L207, that is called when a network is created. This method will call \"reserve_provider_segment\" L293 and there, we\u0027ll call to each driver (vlan, vlxan, etc) to reserve a provider segment. In each type driver we allocate the segments.\n\nI must be wrong but I don\u0027t see when we call this method.","commit_id":"e2fff3522564aa392c94bca9188f03e00e7f7624"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"626af1ddaf9288d9ac6d271b051c6376a764f188","unresolved":true,"context_lines":[{"line_number":341,"context_line":"                      \"network type is not supported.\", segment)"},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"    @db_api.retry_db_errors"},{"line_number":344,"context_line":"    def allocate_dynamic_segment(self, context, network_id, segment):"},{"line_number":345,"context_line":"        \"\"\"Allocate a dynamic segment using a partial or full segment dict.\"\"\""},{"line_number":346,"context_line":"        dynamic_segment \u003d segments_db.get_dynamic_segment("},{"line_number":347,"context_line":"            context, network_id, segment.get(api.PHYSICAL_NETWORK),"}],"source_content_type":"text/x-python","patch_set":2,"id":"bcd4c46c_f0080bd7","line":344,"range":{"start_line":344,"start_character":8,"end_line":344,"end_character":32},"in_reply_to":"3974ba49_9851aad0","updated":"2021-07-07 17:34:36.000000000","message":"Right, I see that cisco, arista and cumulus are calling this API method.\n\nBut Neutron testing is not properly checking the DB context.\n\nAnyway, I would recommend you to add this retry decorator in \"release_dynamic_segment\" too because you\u0027ll need eventually to call it.","commit_id":"e2fff3522564aa392c94bca9188f03e00e7f7624"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"ecef58d3c4a0d27b8c0a929500c31cfb8024d71c","unresolved":true,"context_lines":[{"line_number":341,"context_line":"                      \"network type is not supported.\", segment)"},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"    @db_api.retry_db_errors"},{"line_number":344,"context_line":"    def allocate_dynamic_segment(self, context, network_id, segment):"},{"line_number":345,"context_line":"        \"\"\"Allocate a dynamic segment using a partial or full segment dict.\"\"\""},{"line_number":346,"context_line":"        dynamic_segment \u003d segments_db.get_dynamic_segment("},{"line_number":347,"context_line":"            context, network_id, segment.get(api.PHYSICAL_NETWORK),"}],"source_content_type":"text/x-python","patch_set":2,"id":"3974ba49_9851aad0","line":344,"range":{"start_line":344,"start_character":8,"end_line":344,"end_character":32},"in_reply_to":"470edde4_9605ad15","updated":"2021-05-25 12:53:11.000000000","message":"I think allocate_dynamic_segment() is only called by third party drivers that support hierarchical portbinding (HPB[0]). I know of at least networking-arista[1] using it.\n\nThe problem I\u0027m having (and which the bug is about) is occuring when the network is already created (including its provider segment) and the driver needs to create a new network segment for an incoming port binding. If two neutron-server instances try to allocate a segment for the same purpose (same network_id, network_type, physical_network) we might end up with two segments for these parameters which have a different segmentation_id. These duplicate segments can break a lot of stuff (configuring wrong vlans on devices, alternating between vlans depending on db query sort order, configuration errors because of \"only one VLAN allowed\", stuff like that).\n\nSo I (with my \"driver-dev\" hat on) need a way to dynamically allocate and release segments for existing networks without duplicates. allocate_dynamic_segment() tries to do that, but might fail due to the race condition addressed in this patch.\n\nRegarding create_network_segments() I think it will suffer the same problem. Although I\u0027m not yet sure how I\u0027d use it to create a segment for a specific physical_network (or if it\u0027s even possible) - if it could be used in such a way we would also have two calls to it trying to allocate the same segment, getting different segmentation_ids from the TypeDriver and then allocate a segment with this data. The VlanTypeDriver cannot help us as it does not know for which segment it is currently allocation an vlan_id. That\u0027s why I propose the unique constraint for the networksegments table.\n\nIn the networking-arista case[1] mentioned at the beginning they use oslo.lock to guarantee that only one process at a time can create a segment. This does not help if the neutron-server instances are run on different hosts (e.g. kubernetes pods) and can\u0027t synchronize via a shared filesystem lock.\n\n[0] https://specs.openstack.org/openstack/neutron-specs/specs/kilo/ml2-hierarchical-port-binding.html\n[1] https://opendev.org/x/networking-arista/src/commit/1327851e5a923a169090d75374e28c00839a854e/networking_arista/ml2/mechanism_arista.py#L358-L361","commit_id":"e2fff3522564aa392c94bca9188f03e00e7f7624"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"626af1ddaf9288d9ac6d271b051c6376a764f188","unresolved":true,"context_lines":[{"line_number":358,"context_line":"            else:"},{"line_number":359,"context_line":"                dynamic_segment \u003d driver.obj.reserve_provider_segment("},{"line_number":360,"context_line":"                    context, segment)"},{"line_number":361,"context_line":"            segments_db.add_network_segment(context, network_id,"},{"line_number":362,"context_line":"                                            dynamic_segment,"},{"line_number":363,"context_line":"                                            is_dynamic\u003dTrue)"},{"line_number":364,"context_line":"        return dynamic_segment"},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"    def release_dynamic_segment(self, context, segment_id):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ed6898dc_205c6a08","line":363,"range":{"start_line":361,"start_character":12,"end_line":363,"end_character":60},"updated":"2021-07-07 17:34:36.000000000","message":"(no action) I was going to say that \"add_network_segment\" has its own writer_context, but you are right on wrapping all this section together.","commit_id":"e2fff3522564aa392c94bca9188f03e00e7f7624"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"27ec18a3b7f0a2f94177e1ec3da36e5ef3786f6d","unresolved":false,"context_lines":[{"line_number":358,"context_line":"            else:"},{"line_number":359,"context_line":"                dynamic_segment \u003d driver.obj.reserve_provider_segment("},{"line_number":360,"context_line":"                    context, segment)"},{"line_number":361,"context_line":"            segments_db.add_network_segment(context, network_id,"},{"line_number":362,"context_line":"                                            dynamic_segment,"},{"line_number":363,"context_line":"                                            is_dynamic\u003dTrue)"},{"line_number":364,"context_line":"        return dynamic_segment"},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"    def release_dynamic_segment(self, context, segment_id):"}],"source_content_type":"text/x-python","patch_set":2,"id":"474e0e9a_2e65d4ac","line":363,"range":{"start_line":361,"start_character":12,"end_line":363,"end_character":60},"in_reply_to":"ed6898dc_205c6a08","updated":"2021-09-01 14:20:48.000000000","message":"Done","commit_id":"e2fff3522564aa392c94bca9188f03e00e7f7624"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"626af1ddaf9288d9ac6d271b051c6376a764f188","unresolved":true,"context_lines":[{"line_number":368,"context_line":"        segment \u003d segments_db.get_segment_by_id(context, segment_id)"},{"line_number":369,"context_line":"        if segment:"},{"line_number":370,"context_line":"            driver \u003d self.drivers.get(segment.get(api.NETWORK_TYPE))"},{"line_number":371,"context_line":"            if driver:"},{"line_number":372,"context_line":"                if isinstance(driver.obj, api.TypeDriver):"},{"line_number":373,"context_line":"                    driver.obj.release_segment(context.session, segment)"},{"line_number":374,"context_line":"                else:"},{"line_number":375,"context_line":"                    driver.obj.release_segment(context, segment)"},{"line_number":376,"context_line":"                segments_db.delete_network_segment(context, segment_id)"},{"line_number":377,"context_line":"            else:"},{"line_number":378,"context_line":"                LOG.error(\"Failed to release segment \u0027%s\u0027 because \""},{"line_number":379,"context_line":"                          \"network type is not supported.\", segment)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fd2de37f_087163e6","line":376,"range":{"start_line":371,"start_character":12,"end_line":376,"end_character":71},"updated":"2021-07-07 17:34:36.000000000","message":"You\u0027ll need to add a write context here too.","commit_id":"e2fff3522564aa392c94bca9188f03e00e7f7624"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"27ec18a3b7f0a2f94177e1ec3da36e5ef3786f6d","unresolved":false,"context_lines":[{"line_number":368,"context_line":"        segment \u003d segments_db.get_segment_by_id(context, segment_id)"},{"line_number":369,"context_line":"        if segment:"},{"line_number":370,"context_line":"            driver \u003d self.drivers.get(segment.get(api.NETWORK_TYPE))"},{"line_number":371,"context_line":"            if driver:"},{"line_number":372,"context_line":"                if isinstance(driver.obj, api.TypeDriver):"},{"line_number":373,"context_line":"                    driver.obj.release_segment(context.session, segment)"},{"line_number":374,"context_line":"                else:"},{"line_number":375,"context_line":"                    driver.obj.release_segment(context, segment)"},{"line_number":376,"context_line":"                segments_db.delete_network_segment(context, segment_id)"},{"line_number":377,"context_line":"            else:"},{"line_number":378,"context_line":"                LOG.error(\"Failed to release segment \u0027%s\u0027 because \""},{"line_number":379,"context_line":"                          \"network type is not supported.\", segment)"}],"source_content_type":"text/x-python","patch_set":2,"id":"538d354a_39c7d099","line":376,"range":{"start_line":371,"start_character":12,"end_line":376,"end_character":71},"in_reply_to":"fd2de37f_087163e6","updated":"2021-09-01 14:20:48.000000000","message":"Done","commit_id":"e2fff3522564aa392c94bca9188f03e00e7f7624"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"a5f9f14b1e77b6bc78ab8ceb479fcdfdc9774ac3","unresolved":true,"context_lines":[{"line_number":340,"context_line":"            LOG.error(\"Failed to release segment \u0027%s\u0027 because \""},{"line_number":341,"context_line":"                      \"network type is not supported.\", segment)"},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"    @db_api.retry_db_errors"},{"line_number":344,"context_line":"    def allocate_dynamic_segment(self, context, network_id, segment):"},{"line_number":345,"context_line":"        \"\"\"Allocate a dynamic segment using a partial or full segment dict.\"\"\""},{"line_number":346,"context_line":"        dynamic_segment \u003d segments_db.get_dynamic_segment("}],"source_content_type":"text/x-python","patch_set":3,"id":"a3155145_2246e755","line":343,"range":{"start_line":343,"start_character":12,"end_line":343,"end_character":27},"updated":"2021-09-01 07:43:52.000000000","message":"better use retry_if_session_inactive as this could be used inside active transaction and retry_db_errors will cause misleading error trace","commit_id":"5b58eaf6cbbcaf8954ce1f0ffd55561e1d2c6fbe"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"27ec18a3b7f0a2f94177e1ec3da36e5ef3786f6d","unresolved":false,"context_lines":[{"line_number":340,"context_line":"            LOG.error(\"Failed to release segment \u0027%s\u0027 because \""},{"line_number":341,"context_line":"                      \"network type is not supported.\", segment)"},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"    @db_api.retry_db_errors"},{"line_number":344,"context_line":"    def allocate_dynamic_segment(self, context, network_id, segment):"},{"line_number":345,"context_line":"        \"\"\"Allocate a dynamic segment using a partial or full segment dict.\"\"\""},{"line_number":346,"context_line":"        dynamic_segment \u003d segments_db.get_dynamic_segment("}],"source_content_type":"text/x-python","patch_set":3,"id":"073c25b3_e196e323","line":343,"range":{"start_line":343,"start_character":12,"end_line":343,"end_character":27},"in_reply_to":"a3155145_2246e755","updated":"2021-09-01 14:20:48.000000000","message":"Done","commit_id":"5b58eaf6cbbcaf8954ce1f0ffd55561e1d2c6fbe"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"a5f9f14b1e77b6bc78ab8ceb479fcdfdc9774ac3","unresolved":true,"context_lines":[{"line_number":363,"context_line":"                                            is_dynamic\u003dTrue)"},{"line_number":364,"context_line":"        return dynamic_segment"},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"    @db_api.retry_db_errors"},{"line_number":367,"context_line":"    def release_dynamic_segment(self, context, segment_id):"},{"line_number":368,"context_line":"        \"\"\"Delete a dynamic segment.\"\"\""},{"line_number":369,"context_line":"        segment \u003d segments_db.get_segment_by_id(context, segment_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"196f5978_10b92cd8","line":366,"range":{"start_line":366,"start_character":4,"end_line":366,"end_character":27},"updated":"2021-09-01 07:43:52.000000000","message":"ditto","commit_id":"5b58eaf6cbbcaf8954ce1f0ffd55561e1d2c6fbe"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"27ec18a3b7f0a2f94177e1ec3da36e5ef3786f6d","unresolved":false,"context_lines":[{"line_number":363,"context_line":"                                            is_dynamic\u003dTrue)"},{"line_number":364,"context_line":"        return dynamic_segment"},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"    @db_api.retry_db_errors"},{"line_number":367,"context_line":"    def release_dynamic_segment(self, context, segment_id):"},{"line_number":368,"context_line":"        \"\"\"Delete a dynamic segment.\"\"\""},{"line_number":369,"context_line":"        segment \u003d segments_db.get_segment_by_id(context, segment_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ddbd86aa_e8c28ff4","line":366,"range":{"start_line":366,"start_character":4,"end_line":366,"end_character":27},"in_reply_to":"196f5978_10b92cd8","updated":"2021-09-01 14:20:48.000000000","message":"Done","commit_id":"5b58eaf6cbbcaf8954ce1f0ffd55561e1d2c6fbe"}],"neutron/tests/unit/plugins/ml2/test_plugin.py":[{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"f5c6fb8fc1a7520ed21eb30504cbc49d509cacd5","unresolved":true,"context_lines":[{"line_number":2876,"context_line":"        res \u003d network_req.get_response(self.api)"},{"line_number":2877,"context_line":"        self.assertEqual(400, res.status_int)"},{"line_number":2878,"context_line":""},{"line_number":2879,"context_line":"    def test_create_network_duplicate_partial_segments_fail(self):"},{"line_number":2880,"context_line":"        data \u003d {\u0027network\u0027: {\u0027name\u0027: \u0027net1\u0027,"},{"line_number":2881,"context_line":"                            mpnet_apidef.SEGMENTS:"},{"line_number":2882,"context_line":"                            [{pnet.NETWORK_TYPE: \u0027vlan\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"53ae488b_dda38524","line":2879,"updated":"2021-08-31 18:13:34.000000000","message":"I updated most tests that they\u0027ll work with the new constraint, but with this test we have a functionality change in the tests (which I wanted to explictly point out).\n\nAdditionally, the test is now checking for 409 conflict, as now we get the NeutronError NeutronDbObjectDuplicateEntry as an entry:\n\n409 Conflict\n{\"NeutronError\": {\"type\": \"NeutronDbObjectDuplicateEntry\", \"message\": \"Failed to create a duplicate NetworkSegment: for attribute(s) [\u0027network_id\u0027, \u0027network_type\u0027, \u0027physical_network\u0027] with value(s) None\", \"detail\": \"\"}}\n\nOpen question is if this is acceptable behavior. The error shows exactly what\u0027s wrong, so I\u0027m fine with it, but do we maybe need a more \"userfriendly\" error message?","commit_id":"5b58eaf6cbbcaf8954ce1f0ffd55561e1d2c6fbe"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"27ec18a3b7f0a2f94177e1ec3da36e5ef3786f6d","unresolved":true,"context_lines":[{"line_number":2876,"context_line":"        res \u003d network_req.get_response(self.api)"},{"line_number":2877,"context_line":"        self.assertEqual(400, res.status_int)"},{"line_number":2878,"context_line":""},{"line_number":2879,"context_line":"    def test_create_network_duplicate_partial_segments_fail(self):"},{"line_number":2880,"context_line":"        data \u003d {\u0027network\u0027: {\u0027name\u0027: \u0027net1\u0027,"},{"line_number":2881,"context_line":"                            mpnet_apidef.SEGMENTS:"},{"line_number":2882,"context_line":"                            [{pnet.NETWORK_TYPE: \u0027vlan\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"c295ffb0_f7c822d3","line":2879,"in_reply_to":"53ae488b_dda38524","updated":"2021-09-01 14:20:48.000000000","message":"The lower constraint tests also fails because of this test. It returns 500 HTTPInternalServerError instead of a 409 NeutronDbObjectDuplicateEntry.\n\n\n{\"NeutronError\": {\"type\": \"HTTPInternalServerError\", \"message\": \"Request Failed: internal server error while processing your request.\", \"detail\": \"\"}}\n\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\nFailed 1 tests - output below:\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n\nneutron.tests.unit.plugins.ml2.test_plugin.TestMultiSegmentNetworks.test_create_network_duplicate_partial_segments_fail\n-----------------------------------------------------------------------------------------------------------------------\n\nCaptured traceback:\n~~~~~~~~~~~~~~~~~~~\n    b\u0027Traceback (most recent call last):\u0027\n    b\u0027  File \"/home/ccloud/dev/neutron/neutron/tests/base.py\", line 183, in func\u0027\n    b\u0027    return f(self, *args, **kwargs)\u0027\n    b\u0027  File \"/home/ccloud/dev/neutron/neutron/tests/unit/plugins/ml2/test_plugin.py\", line 2890, in test_create_network_duplicate_partial_segments_fail\u0027\n    b\u0027    self.assertEqual(409, res.status_int)\u0027\n    b\u0027  File \"/home/ccloud/dev/neutron/.tox/lower-constraints/lib/python3.8/site-packages/testtools/testcase.py\", line 411, in assertEqual\u0027\n    b\u0027    self.assertThat(observed, matcher, message)\u0027\n    b\u0027  File \"/home/ccloud/dev/neutron/.tox/lower-constraints/lib/python3.8/site-packages/testtools/testcase.py\", line 498, in assertThat\u0027\n    b\u0027    raise mismatch_error\u0027\n    b\u0027testtools.matchers._impl.MismatchError: 409 !\u003d 500\u0027\n    b\u0027\u0027","commit_id":"5b58eaf6cbbcaf8954ce1f0ffd55561e1d2c6fbe"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"dbf71615a97856081ce78a810d40c3a1476b5d48","unresolved":true,"context_lines":[{"line_number":2876,"context_line":"        res \u003d network_req.get_response(self.api)"},{"line_number":2877,"context_line":"        self.assertEqual(400, res.status_int)"},{"line_number":2878,"context_line":""},{"line_number":2879,"context_line":"    def test_create_network_duplicate_partial_segments_fail(self):"},{"line_number":2880,"context_line":"        data \u003d {\u0027network\u0027: {\u0027name\u0027: \u0027net1\u0027,"},{"line_number":2881,"context_line":"                            mpnet_apidef.SEGMENTS:"},{"line_number":2882,"context_line":"                            [{pnet.NETWORK_TYPE: \u0027vlan\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa2b696_46b2e5f4","line":2879,"in_reply_to":"c295ffb0_f7c822d3","updated":"2021-09-02 14:10:13.000000000","message":"This here is the problem:\n[testenv:lower-constraints]\nsetenv \u003d OS_TEST_TIMEOUT\u003d{env:OS_TEST_TIMEOUT:60}\n\nThe new failing test takes longer than 60 seconds and fails with lower-constraints. Raising it to 180 (the same value as in testenv:common) makes the test go green. I\u0027m not sure if raising it is the right solution here, or if we maybe can reduce the retries / time taken for a broken network create.","commit_id":"5b58eaf6cbbcaf8954ce1f0ffd55561e1d2c6fbe"}],"releasenotes/notes/add-networksegments-uniq-constraint-89e52b42ca2f7ec2.yaml":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"b179f45b86cffc80ed0599f7c7b1d3e40e7bc2ef","unresolved":true,"context_lines":[{"line_number":5,"context_line":"    added to the networksegments table. This was done to prevent race"},{"line_number":6,"context_line":"    conditions on dynamic segment allocation. Operators having networks with"},{"line_number":7,"context_line":"    multiple segments (e.g. when using hierarchical portbinding) should check"},{"line_number":8,"context_line":"    if this constraint is violated before upgrading."}],"source_content_type":"text/x-yaml","patch_set":8,"id":"5526f8eb_b4bfd37c","line":8,"updated":"2021-10-06 07:46:28.000000000","message":"we can propose upgrade check for that, please see https://docs.openstack.org/neutron/latest/contributor/upgrade_checks.html and existing checks which are in https://github.com/openstack/neutron/blob/master/neutron/cmd/upgrade_checks/checks.py","commit_id":"bd0973e8f2c01d6937846425d842066bb4df1d4b"}]}