)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"aca7a117d558ef263335a7e5d798f92c77cc4644","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"373b994a_ca8250fd","updated":"2022-05-13 10:00:42.000000000","message":"+1 because i was wondering how you feel about perhapse also adding functional tests for this\n\nwe have limited functional test that test the low level ovsdb operations.\nhttps://github.com/openstack/os-vif/blob/master/vif_plug_ovs/tests/functional/base.py\nhttps://github.com/openstack/os-vif/blob/master/vif_plug_ovs/tests/functional/ovsdb/test_ovsdb_lib.py\nits been on my todo list to add functional tests for the the plugin at a high level.\n\nwould you feel comfrotable creating a functional test that calls plug and unplug  on the plugink and asserts the trunk bridge is created and deleted. it basicly the same as the unit test you wrote\n\ne.g. vif_plug_ovs/tests/functional/test_pluging.py\n\nclass TestOVSPlugin(testscenarios.WithScenarios,\n                   base.VifPlugOvsBaseFunctionalTestCase):\n\n def setUp(self):\n        super(TestOVSPlugin, self).setUp()\n        run_privileged(\u0027ovs-vsctl\u0027, \u0027set-manager\u0027, \u0027ptcp:6640\u0027)\n        self.plugin \u003d ovs.OvsPlugin.load(constants.PLUGIN_NAME)\n        self.flags(ovsdb_interface\u003dself.interface, group\u003d\u0027os_vif_ovs\u0027)\n\n        self.ovs \u003d ovsdb_lib.BaseOVS(CONF.os_vif_ovs)\n        self._ovsdb \u003d self.ovs.ovsdb\n\n        self.profile_ovs \u003d objects.vif.VIFPortProfileOpenVSwitch(\n            interface_id\u003d\u0027e65867e0-9340-4a7f-a256-09af6eb7a3aa\u0027,\n            datapath_type\u003d\u0027netdev\u0027)\n\n        self.vif_vhostuser_trunk \u003d objects.vif.VIFVHostUser(\n            id\u003d\u0027b679325f-ca89-4ee0-a8be-6db1409b69ea\u0027,\n            address\u003d\u0027ca:fe:de:ad:be:ef\u0027,\n            network\u003dself.network_ovs_trunk,\n            path\u003d\u0027/var/run/openvswitch/vhub679325f-ca\u0027,\n            mode\u003d\u0027client\u0027,\n            port_profile\u003dself.profile_ovs)\n\n        self.instance \u003d objects.instance_info.InstanceInfo(\n            name\u003d\u0027demo\u0027,\n            uuid\u003d\u0027f0000000-0000-0000-0000-000000000001\u0027)\n\n    def _check_bridge(self, name):\n        return self._ovsdb.br_exists(name).execute()\n\n    def _add_bridge(self, name, may_exist\u003dTrue, datapath_type\u003dNone):\n        self._ovsdb.add_br(name, may_exist\u003dmay_exist,\n                           datapath_type\u003ddatapath_type).execute()\n        self.assertTrue(self._check_bridge(name))\n\n    def _del_bridge(self, name):\n        self._ovsdb.del_br(name).execute()\n\n    def test_unplug_ovs_vhostuser_trunk(self, delete_ovs_vif_port,\n                                        delete_ovs_bridge):\n        trunk_bridge \u003d \u0027%s01\u0027 % constants.TRUNK_BR_PREFIX\n        self.plugin.plug(self.vif_vhostuser_trunk, self.instance)\n        # this should not be requried but just in cases unplug brakes\n        self.addCleanup(self._del_bridge, trunk_bridge)\n\n        # assert that  trunk bride bridges are created here\n        self.assertTrue(self._check_bridge(trunk_bridge))\n\n        other_bridge \u003d \u0027br-%s\u0027 %  uuidutils.generate_uuid()\n        self._add_bridge(other_bridge)\n        self.addCleanup(self._del_bridge, other_bridge)\n        self.plugin.unplug(self.vif_vhostuser_trunk, self.instance)\n        self.assertTrue(self._check_bridge(other_bridge))\n        self.assertFalse(self._check_bridge(trunk_bridge))\n\nall of the helper fucniton i just copy pasted form the ovsdb func tests\nhttps://github.com/openstack/os-vif/blob/master/vif_plug_ovs/tests/functional/ovsdb/test_ovsdb_lib.py\n\nthey really shoudl be moved to base\n\nhttps://github.com/openstack/os-vif/blob/master/vif_plug_ovs/tests/functional/base.py\nbut thats not important right now.\n\ni would like use to start buildign out the functional tests more if we can but if you dont have time i wont block on this.","commit_id":"7493964577681b56ddb5ca64946dec9e49049638"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"ee99e5824ab680d4c5d29f4ae872e294157e0a5c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b3ab5a9c_547a62f7","updated":"2022-05-13 21:54:18.000000000","message":"Yes, I\u0027ll add a functional test for this. Thanks for the review","commit_id":"7493964577681b56ddb5ca64946dec9e49049638"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a5237b6e4bafa2fe57d146b1f219b3e038e82a7a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1f0f6c2b_02a23505","updated":"2022-05-13 10:33:12.000000000","message":"oh one thing to note is os-vif\u0027s functional test need sudo to run since its actully creating ovs bridges ectra.\n\nso you either need passwardless sudo for privesep to start or run them with sudo.","commit_id":"7493964577681b56ddb5ca64946dec9e49049638"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"0dde0db825af6cb287c83bd76d52e15023e1e17b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1070486c_77bf10f1","updated":"2022-05-12 23:17:18.000000000","message":"recheck failure with os-vif-ovs-iptables job seems to be an infra issue. 124 test cases failed, involving identity, image, compute, networking. Many of them failed with:\n\n  File \"/opt/stack/tempest/tempest/lib/services/identity/v3/token_client.py\", line 172, in request\n    raise exceptions.IdentityError(\ntempest.lib.exceptions.IdentityError: Got identity error\nDetails: Unexpected status code 500\n","commit_id":"7493964577681b56ddb5ca64946dec9e49049638"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"6e572efd72ba873666a0813a1587cac03f1382fe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ff4f2f03_d2890f83","in_reply_to":"373b994a_ca8250fd","updated":"2022-05-24 23:37:06.000000000","message":"Done","commit_id":"7493964577681b56ddb5ca64946dec9e49049638"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f72f7536e5984704f9c86752180845fb52dd5182","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"943f3d0f_65988758","updated":"2022-05-25 20:16:46.000000000","message":"+1 for now and ill check this again tomorrow once the ci has run\nbut i think im happy with this now.\nthanks for addign the functional tests.","commit_id":"7861bec9b72b096ce382833a7e83e6516468acfd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"563c620aa1c43397f7fb7462a38785fb5fd8c5bd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"e3efe194_b45c437c","updated":"2022-05-26 10:58:23.000000000","message":"the py36 failure is not related to your change we just have not update the ci sicne 36 support was drop so ill do that in a sperate patch.","commit_id":"2f41b0d9f4c1287a0a26cbfb40df0afec68b3fd4"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"9957b7511652ff45ccbe6ea3532d3fcf0ceb5822","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"74ffb432_fb304201","updated":"2022-05-31 14:08:39.000000000","message":"I\u0027m not a pure os-vif specialist but I trust you.","commit_id":"75b290fb2a8f706583e0c12c5c5a4c0fc80e6481"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dbb7d03239c2cb415bff6aa5479d5180cfc51414","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"70aa2a9e_19f50d6d","updated":"2022-05-30 15:55:47.000000000","message":"the job configuration issue is not resolved so +2","commit_id":"75b290fb2a8f706583e0c12c5c5a4c0fc80e6481"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"bb7df77435b0c5648198d3f3c9cda6a97ffe6606","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"2d78e446_24005ef4","in_reply_to":"74ffb432_fb304201","updated":"2022-05-31 14:09:36.000000000","message":"\u003e I\u0027m not a pure os-vif specialist but I trust you.\n\nAnd I trust the CI as the depending patch looks good to it.","commit_id":"75b290fb2a8f706583e0c12c5c5a4c0fc80e6481"}],"vif_plug_ovs/tests/functional/base.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f72f7536e5984704f9c86752180845fb52dd5182","unresolved":true,"context_lines":[{"line_number":23,"context_line":"    PRIVILEGED_GROUP \u003d \u0027vif_plug_ovs_privileged\u0027"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    def _check_bridge(self, name):"},{"line_number":26,"context_line":"        return self._ovsdb.br_exists(name).execute()"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    def _add_bridge(self, name, may_exist\u003dTrue, datapath_type\u003dNone):"},{"line_number":29,"context_line":"        self._ovsdb.add_br(name, may_exist\u003dmay_exist,"}],"source_content_type":"text/x-python","patch_set":3,"id":"251c4a63_489af5e1","line":26,"range":{"start_line":26,"start_character":15,"end_line":26,"end_character":26},"updated":"2022-05-25 20:16:46.000000000","message":"the only thing that is slightly odd is this is only define  in the class that inherits form this so this is accting more as a mixin the a normal base class with that said since this is never expected to be instanciated by its self i think im ok with this as is.","commit_id":"7861bec9b72b096ce382833a7e83e6516468acfd"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"e95ab6e545aa3165dc7a726b2411e0a85d073fd9","unresolved":true,"context_lines":[{"line_number":23,"context_line":"    PRIVILEGED_GROUP \u003d \u0027vif_plug_ovs_privileged\u0027"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    def _check_bridge(self, name):"},{"line_number":26,"context_line":"        return self._ovsdb.br_exists(name).execute()"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    def _add_bridge(self, name, may_exist\u003dTrue, datapath_type\u003dNone):"},{"line_number":29,"context_line":"        self._ovsdb.add_br(name, may_exist\u003dmay_exist,"}],"source_content_type":"text/x-python","patch_set":3,"id":"cf16e00b_797dafe7","line":26,"range":{"start_line":26,"start_character":15,"end_line":26,"end_character":26},"in_reply_to":"251c4a63_489af5e1","updated":"2022-05-25 21:32:25.000000000","message":"Yeah, I see what you you say. I was just trying to eliminate some redundant code. If you want I can move these methods back to the two descendant classes, even if we have some duplicated code. Let me know","commit_id":"7861bec9b72b096ce382833a7e83e6516468acfd"}]}