)]}'
{"etc/neutron/rootwrap.d/l3.filters":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"df90fff8c24174a29512cd476e28333abc808588","unresolved":false,"context_lines":[{"line_number":47,"context_line":"# l3 agent to delete floatingip\u0027s conntrack state"},{"line_number":48,"context_line":"conntrack: CommandFilter, conntrack, root"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"# sleep to simulate a busy process inside netns"},{"line_number":51,"context_line":"sleep: CommandFilter, sleep, root"}],"source_content_type":"application/octet-stream","patch_set":16,"id":"3a961159_226968f8","line":50,"updated":"2015-01-14 23:25:29.000000000","message":"I think this filter should be provided in a separate filter file (e.g. sanity.filters) so that it can be optionally deployed.","commit_id":"9a7a610928e09d9e6e927b9668d1dace0c391390"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"6d2050151b9b252c9a1990c289efd7bc33732c75","unresolved":false,"context_lines":[{"line_number":47,"context_line":"# l3 agent to delete floatingip\u0027s conntrack state"},{"line_number":48,"context_line":"conntrack: CommandFilter, conntrack, root"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"# sleep to simulate a busy process inside netns"},{"line_number":51,"context_line":"sleep: CommandFilter, sleep, root"}],"source_content_type":"application/octet-stream","patch_set":16,"id":"3a961159_0f53a53a","line":50,"in_reply_to":"3a961159_226968f8","updated":"2015-01-15 10:17:52.000000000","message":"Done","commit_id":"9a7a610928e09d9e6e927b9668d1dace0c391390"}],"neutron/cmd/sanity/checks.py":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"13e9269f2211609417ca1dd3adc02545ce104cc6","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    except ImportError:"},{"line_number":48,"context_line":"        return False"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"def del_netns_supported(root_helper\u003dNone):"},{"line_number":52,"context_line":"    \"\"\"check if delete netns is safe supported by the current iproute version,"},{"line_number":53,"context_line":"    by trying to remove a test netns when another netns is mounted (busy), and"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_6a9b15c9","line":50,"updated":"2014-07-20 16:39:49.000000000","message":"Just a note. Last time I checked on RHEL 7 router_delete_namespaces caused issues / didn\u0027t work smoothly. I don\u0027t have access to a RHEL machine right now but I\u0027d be surprised if this test would fail. Have you tested this on different distributions and iproute2 versions?","commit_id":"32eb8a513edd77dbc780cab6a8d6cbe0adb51cf3"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"c3479e6e8a5d940f8f64b78a6ddc46f44ce62af6","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    except ImportError:"},{"line_number":48,"context_line":"        return False"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"def del_netns_supported(root_helper\u003dNone):"},{"line_number":52,"context_line":"    \"\"\"check if delete netns is safe supported by the current iproute version,"},{"line_number":53,"context_line":"    by trying to remove a test netns when another netns is mounted (busy), and"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_6464ec20","line":50,"in_reply_to":"baada198_6a9b15c9","updated":"2014-08-12 15:34:10.000000000","message":"The issue with removing netns is related and fixed by iproute only [1]. If the distro includes the patched iproute package, it will work. Both Red Hat and Ubuntu reported testing steps to reproduce the issue in [2] and [3] repectively. Here the sanity check followed these steps 1,2,7,8 reported by Red Hat in [2], and worked on our Ubuntu environment:\n\n1. ip netns add netns_one\n\n2. ip netns add netns_two\n\n7. ip netns exec netns_one bash\n\n# in other terminal:\n\n8. ip netns delete netns_two\n\nSorry, I will add this info in the commit message to help code review.\n\n[1] https://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/commit/?id\u003d58a3e8270fe72f8ed92687d3a3132c2a708582dd\n\n[2] https://bugzilla.redhat.com/show_bug.cgi?id\u003d1062685#c11\n\n[3] https://bugs.launchpad.net/ubuntu/+source/iproute/+bug/1238981","commit_id":"32eb8a513edd77dbc780cab6a8d6cbe0adb51cf3"},{"author":{"_account_id":6598,"name":"Berezovsky Irena","email":"irenab.dev@gmail.com","username":"irenab"},"change_message_id":"d33968650b9768dafdccabec017b0d84dbd8092f","unresolved":false,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"def del_netns_supported(root_helper\u003dNone):"},{"line_number":97,"context_line":"    \"\"\"check if delete netns is safe supported by the current iproute version,"},{"line_number":98,"context_line":"    by trying to remove a test netns when another netns is mounted (busy), and"},{"line_number":99,"context_line":"    clean up the test netns afterwards. Useful if config dhcp/router_delete_"},{"line_number":100,"context_line":"    namespaces is enabled."}],"source_content_type":"text/x-python","patch_set":6,"id":"1abeadc6_468889e7","line":97,"updated":"2014-08-26 10:33:22.000000000","message":"docstring is not according to Hacking guide:\nhttp://docs.openstack.org/developer/hacking/#docstrings","commit_id":"214299d5e2dfd273d397429acba1590b6c2370fa"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"8294c5d2d6061ec17bea836daac2fa92a56350bb","unresolved":false,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"def del_netns_supported(root_helper\u003dNone):"},{"line_number":97,"context_line":"    \"\"\"check if delete netns is safe supported by the current iproute version,"},{"line_number":98,"context_line":"    by trying to remove a test netns when another netns is mounted (busy), and"},{"line_number":99,"context_line":"    clean up the test netns afterwards. Useful if config dhcp/router_delete_"},{"line_number":100,"context_line":"    namespaces is enabled."}],"source_content_type":"text/x-python","patch_set":6,"id":"1abeadc6_bd44fa74","line":97,"in_reply_to":"1abeadc6_468889e7","updated":"2014-08-26 16:24:31.000000000","message":"ok. sometimes forgot","commit_id":"214299d5e2dfd273d397429acba1590b6c2370fa"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"c4b33f2b10023c831558fe069b6c26584475b3b0","unresolved":false,"context_lines":[{"line_number":122,"context_line":"            sleep_proc.terminate()"},{"line_number":123,"context_line":"            # if not terminate immediately, wait for it"},{"line_number":124,"context_line":"            sleep_proc.communicate()"},{"line_number":125,"context_line":"        # netns not busy, can delete now"},{"line_number":126,"context_line":"        ip.netns.delete(testns2)"},{"line_number":127,"context_line":"        supported \u003d False"},{"line_number":128,"context_line":"    finally:"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_2cfd6f09","line":125,"updated":"2014-09-16 13:36:59.000000000","message":"For deleting the namespaces, you can use:\n\nself.addCleanup(ip.netns.delete, testns1)\n\nself.addCleanup(ip.netns.delete, testns2)\n\nAnd remove this delete as well as the finally block.","commit_id":"33f85234836e90deac6a0c98458e4a9998c37d45"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"40b9ca3fc4bdba22ae59717937e58a54f4afcecc","unresolved":false,"context_lines":[{"line_number":122,"context_line":"            sleep_proc.terminate()"},{"line_number":123,"context_line":"            # if not terminate immediately, wait for it"},{"line_number":124,"context_line":"            sleep_proc.communicate()"},{"line_number":125,"context_line":"        # netns not busy, can delete now"},{"line_number":126,"context_line":"        ip.netns.delete(testns2)"},{"line_number":127,"context_line":"        supported \u003d False"},{"line_number":128,"context_line":"    finally:"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_8818e71f","line":125,"in_reply_to":"fa98f980_2cfd6f09","updated":"2014-09-17 12:17:35.000000000","message":"you meant addCleanup() method from unittest???\nBut this is not a unittest. Could you please view the whole file?","commit_id":"33f85234836e90deac6a0c98458e4a9998c37d45"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"86f0fdad13c86d18a84e35e3f20f751dc5296248","unresolved":false,"context_lines":[{"line_number":122,"context_line":"            sleep_proc.terminate()"},{"line_number":123,"context_line":"            # if not terminate immediately, wait for it"},{"line_number":124,"context_line":"            sleep_proc.communicate()"},{"line_number":125,"context_line":"        # netns not busy, can delete now"},{"line_number":126,"context_line":"        ip.netns.delete(testns2)"},{"line_number":127,"context_line":"        supported \u003d False"},{"line_number":128,"context_line":"    finally:"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_03dcd845","line":125,"in_reply_to":"fa98f980_8818e71f","updated":"2014-09-17 12:57:36.000000000","message":"Oops, you\u0027re right, I\u0027m not used to the idea that this code is not running as a test. Other functional tests run as a test and eventually from a base test class that implements addCleanup. Maybe we should reconsider our approach with the sanity checks, but that\u0027s completely out of scope for this patch.","commit_id":"33f85234836e90deac6a0c98458e4a9998c37d45"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f701346aaac989ca5295fe0093cf7eb5cd4e109f","unresolved":false,"context_lines":[{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    :returns: a boolean if delete netns is safe"},{"line_number":121,"context_line":"    \"\"\""},{"line_number":122,"context_line":"    supported \u003d True"},{"line_number":123,"context_line":"    testns1 \u003d \"netnstest-\" + utils.get_random_string(6)"},{"line_number":124,"context_line":"    testns2 \u003d \"netnstest-\" + utils.get_random_string(6)"},{"line_number":125,"context_line":"    sleep_proc \u003d None"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_98471d1d","line":122,"updated":"2014-11-13 04:16:31.000000000","message":"Nit:  supported is not needed.  Just return True or False on lines 147 and 144 respectively","commit_id":"65bbe541f6d85703441a786f4e1c93ea9a18195a"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"b81b7344008d8d27a6752e8a87ff961266f9c8a2","unresolved":false,"context_lines":[{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    :returns: a boolean if delete netns is safe"},{"line_number":121,"context_line":"    \"\"\""},{"line_number":122,"context_line":"    supported \u003d True"},{"line_number":123,"context_line":"    testns1 \u003d \"netnstest-\" + utils.get_random_string(6)"},{"line_number":124,"context_line":"    testns2 \u003d \"netnstest-\" + utils.get_random_string(6)"},{"line_number":125,"context_line":"    sleep_proc \u003d None"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_26551a50","line":122,"in_reply_to":"5a890539_98471d1d","updated":"2014-11-13 14:06:24.000000000","message":"Done","commit_id":"65bbe541f6d85703441a786f4e1c93ea9a18195a"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"a00c3ab4ec35c46afef8e4209ce03db829ededb4","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        # add latency after sleep"},{"line_number":134,"context_line":"        time.sleep(0.5)"},{"line_number":135,"context_line":"        # try deleting the other (not busy) one"},{"line_number":136,"context_line":"        ip.netns.delete(testns2)"},{"line_number":137,"context_line":"    except Exception:"},{"line_number":138,"context_line":"        if sleep_proc and sleep_proc.poll() is None:"},{"line_number":139,"context_line":"            sleep_proc.terminate()"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_ad223dff","line":136,"updated":"2014-11-07 17:01:54.000000000","message":"According to the docstring, del_netns is unsupported if the 1st ip.netns.delete(testns2) fails ... so an error before its call should be reraised:\n\n  ip \u003d ip_lib.IPWrapper(root_helper)\n  try:\n    ip.ensure_namespace(testns1)\n    ip.ensure_namespace(testns2)\n    sleep_proc ...\n    time.sleep(.5)\n    \n    try:\n      ip.netns.delete(testns2)\n      return True\n    except Exception:\n      ...\n      return False\n   \n  finally:\n    ip.netns.delete(testns1)","commit_id":"65bbe541f6d85703441a786f4e1c93ea9a18195a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f701346aaac989ca5295fe0093cf7eb5cd4e109f","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        # add latency after sleep"},{"line_number":134,"context_line":"        time.sleep(0.5)"},{"line_number":135,"context_line":"        # try deleting the other (not busy) one"},{"line_number":136,"context_line":"        ip.netns.delete(testns2)"},{"line_number":137,"context_line":"    except Exception:"},{"line_number":138,"context_line":"        if sleep_proc and sleep_proc.poll() is None:"},{"line_number":139,"context_line":"            sleep_proc.terminate()"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_f8bbc1e2","line":136,"in_reply_to":"5a890539_58d89e1d","updated":"2014-11-13 04:16:31.000000000","message":"+1 to @Cedric\u0027s proposal.  I\u0027d prefer to tighten up the try/except to known expected exceptions rather than catching a swallowing exceptions more broadly.","commit_id":"65bbe541f6d85703441a786f4e1c93ea9a18195a"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"e63f79ffa2a6a5ab5d9fb53dbc5eeb6f2dac3dc4","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        # add latency after sleep"},{"line_number":134,"context_line":"        time.sleep(0.5)"},{"line_number":135,"context_line":"        # try deleting the other (not busy) one"},{"line_number":136,"context_line":"        ip.netns.delete(testns2)"},{"line_number":137,"context_line":"    except Exception:"},{"line_number":138,"context_line":"        if sleep_proc and sleep_proc.poll() is None:"},{"line_number":139,"context_line":"            sleep_proc.terminate()"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_58d89e1d","line":136,"in_reply_to":"5a890539_ad223dff","updated":"2014-11-10 18:11:25.000000000","message":"I consider the whole block from line 127 to 136 (ensure_namespace, sleep_proc) as one single business transaction, whereby anything wrong in between results only \u0027test failed\u0027, and have to rollback by following all the steps in Exception to cleanup the test environment, but not just catch the delete(testns2) only.\n\nFor examples:\n- if sleep_proc() goes wrong, try to terminate it, if such process exists.\n- if ensure_namespace(testns2) goes wrong (weather a loop device inside the netns can not brought up etc...), try deleting testns2 before testns1.\n\nThis sanity check probes the whole test scenario, makes excuse by ensuring a set orders of cleanup and logs.","commit_id":"65bbe541f6d85703441a786f4e1c93ea9a18195a"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"b81b7344008d8d27a6752e8a87ff961266f9c8a2","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        # add latency after sleep"},{"line_number":134,"context_line":"        time.sleep(0.5)"},{"line_number":135,"context_line":"        # try deleting the other (not busy) one"},{"line_number":136,"context_line":"        ip.netns.delete(testns2)"},{"line_number":137,"context_line":"    except Exception:"},{"line_number":138,"context_line":"        if sleep_proc and sleep_proc.poll() is None:"},{"line_number":139,"context_line":"            sleep_proc.terminate()"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_664f22be","line":136,"in_reply_to":"5a890539_f8bbc1e2","updated":"2014-11-13 14:06:24.000000000","message":"Done","commit_id":"65bbe541f6d85703441a786f4e1c93ea9a18195a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f701346aaac989ca5295fe0093cf7eb5cd4e109f","unresolved":false,"context_lines":[{"line_number":142,"context_line":"        # netns not busy, can delete now"},{"line_number":143,"context_line":"        ip.netns.delete(testns2)"},{"line_number":144,"context_line":"        supported \u003d False"},{"line_number":145,"context_line":"    finally:"},{"line_number":146,"context_line":"        ip.netns.delete(testns1)"},{"line_number":147,"context_line":"    return supported"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_98e39d04","line":145,"updated":"2014-11-13 04:16:31.000000000","message":"Do you need to terminate sleep_proc here before attempting to delete the namespace?","commit_id":"65bbe541f6d85703441a786f4e1c93ea9a18195a"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"b81b7344008d8d27a6752e8a87ff961266f9c8a2","unresolved":false,"context_lines":[{"line_number":142,"context_line":"        # netns not busy, can delete now"},{"line_number":143,"context_line":"        ip.netns.delete(testns2)"},{"line_number":144,"context_line":"        supported \u003d False"},{"line_number":145,"context_line":"    finally:"},{"line_number":146,"context_line":"        ip.netns.delete(testns1)"},{"line_number":147,"context_line":"    return supported"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_e6a07245","line":145,"in_reply_to":"5a890539_98e39d04","updated":"2014-11-13 14:06:24.000000000","message":"no, here is not the case","commit_id":"65bbe541f6d85703441a786f4e1c93ea9a18195a"},{"author":{"_account_id":2592,"name":"Mark McClain","email":"mark@mcclain.xyz","username":"markmcclain"},"change_message_id":"c56a619bb4d1b9b6ee6874e6bab290cd1068f193","unresolved":false,"context_lines":[{"line_number":110,"context_line":"    return True"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"def del_netns_supported(root_helper\u003dNone):"},{"line_number":114,"context_line":"    \"\"\"check if delete netns is safe supported by the current iproute version."},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    Try removing a test netns when another netns is mounted (busy), and"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a890539_30ed22b8","line":113,"updated":"2014-11-14 16:08:40.000000000","message":"Where\u0027s the unit test to validate the logic in this check?","commit_id":"d4d0dcbfda9a7f5942be03fee7596f20e48aa839"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"a1339b61416198d384e72b32ee64587e33e5e4cf","unresolved":false,"context_lines":[{"line_number":142,"context_line":"        # netns not busy, can delete now"},{"line_number":143,"context_line":"        ip.netns.delete(testns2)"},{"line_number":144,"context_line":"        return False"},{"line_number":145,"context_line":"    finally:"},{"line_number":146,"context_line":"        ip.netns.delete(testns1)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a890539_a09c8dc1","line":145,"updated":"2014-11-13 14:50:30.000000000","message":"it seems that Carl comment in PS8 should be take into account as nothing ensures if L135 succeed that the sleep ends before testns1 delete","commit_id":"d4d0dcbfda9a7f5942be03fee7596f20e48aa839"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"8b469207ccf8bab057ad0c3d264689a164873175","unresolved":false,"context_lines":[{"line_number":142,"context_line":"        # netns not busy, can delete now"},{"line_number":143,"context_line":"        ip.netns.delete(testns2)"},{"line_number":144,"context_line":"        return False"},{"line_number":145,"context_line":"    finally:"},{"line_number":146,"context_line":"        ip.netns.delete(testns1)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a890539_b2a3f6b0","line":145,"in_reply_to":"5a890539_a09c8dc1","updated":"2014-11-13 21:41:41.000000000","message":"please read the bug report referenced in my commit message to understand the real issue. The deletion of ns1 does not depend on the status if it is busy or not. So weather the sleep ends before the deletion of ns1 or not, does not matter.","commit_id":"d4d0dcbfda9a7f5942be03fee7596f20e48aa839"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d30087f06dd5a3b433d544243917bafae75cb0f7","unresolved":false,"context_lines":[{"line_number":142,"context_line":"        # netns not busy, can delete now"},{"line_number":143,"context_line":"        ip.netns.delete(testns2)"},{"line_number":144,"context_line":"        return False"},{"line_number":145,"context_line":"    finally:"},{"line_number":146,"context_line":"        ip.netns.delete(testns1)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a890539_d5d2275b","line":145,"in_reply_to":"5a890539_b2a3f6b0","updated":"2014-11-14 00:30:55.000000000","message":"I think I\u0027m okay with @Tri Hoang Vo\u0027s response here.","commit_id":"d4d0dcbfda9a7f5942be03fee7596f20e48aa839"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"9699a1ac03256ce72696001cc69abfed0baaa11b","unresolved":false,"context_lines":[{"line_number":142,"context_line":"        # netns not busy, can delete now"},{"line_number":143,"context_line":"        ip.netns.delete(testns2)"},{"line_number":144,"context_line":"        return False"},{"line_number":145,"context_line":"    finally:"},{"line_number":146,"context_line":"        ip.netns.delete(testns1)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a890539_242fa889","line":145,"in_reply_to":"5a890539_d5d2275b","updated":"2014-11-14 14:25:10.000000000","message":"me too","commit_id":"d4d0dcbfda9a7f5942be03fee7596f20e48aa839"},{"author":{"_account_id":105,"name":"Kyle Mestery","email":"mestery@mestery.com","username":"mestery"},"change_message_id":"e8596a411b41200e876eeacea11fb31007a135bc","unresolved":false,"context_lines":[{"line_number":111,"context_line":""},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"def del_netns_supported(root_helper\u003dNone):"},{"line_number":114,"context_line":"    \"\"\"check if delete netns is safe supported by the current iproute version."},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    Try removing a test netns when another netns is mounted (busy), and"},{"line_number":117,"context_line":"    clean up the test netns afterwards. Useful if config dhcp/router_delete_"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_96a777a6","line":114,"updated":"2014-11-17 21:08:26.000000000","message":"nit: s/safe/safely/","commit_id":"69c105901f3ef1f5962597b1c4d6e4d5f49569b2"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"d7834b03961ea697b23ca477026b18199d4d9e03","unresolved":false,"context_lines":[{"line_number":111,"context_line":""},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"def del_netns_supported(root_helper\u003dNone):"},{"line_number":114,"context_line":"    \"\"\"check if delete netns is safe supported by the current iproute version."},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    Try removing a test netns when another netns is mounted (busy), and"},{"line_number":117,"context_line":"    clean up the test netns afterwards. Useful if config dhcp/router_delete_"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_2c6bc285","line":114,"in_reply_to":"5a890539_96a777a6","updated":"2014-11-18 09:58:22.000000000","message":"Done","commit_id":"69c105901f3ef1f5962597b1c4d6e4d5f49569b2"},{"author":{"_account_id":105,"name":"Kyle Mestery","email":"mestery@mestery.com","username":"mestery"},"change_message_id":"e8596a411b41200e876eeacea11fb31007a135bc","unresolved":false,"context_lines":[{"line_number":114,"context_line":"    \"\"\"check if delete netns is safe supported by the current iproute version."},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    Try removing a test netns when another netns is mounted (busy), and"},{"line_number":117,"context_line":"    clean up the test netns afterwards. Useful if config dhcp/router_delete_"},{"line_number":118,"context_line":"    namespaces is enabled."},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    :returns: a boolean if delete netns is safe"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_36cde366","line":117,"updated":"2014-11-17 21:08:26.000000000","message":"nit: s/config/config of/","commit_id":"69c105901f3ef1f5962597b1c4d6e4d5f49569b2"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"d7834b03961ea697b23ca477026b18199d4d9e03","unresolved":false,"context_lines":[{"line_number":114,"context_line":"    \"\"\"check if delete netns is safe supported by the current iproute version."},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    Try removing a test netns when another netns is mounted (busy), and"},{"line_number":117,"context_line":"    clean up the test netns afterwards. Useful if config dhcp/router_delete_"},{"line_number":118,"context_line":"    namespaces is enabled."},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    :returns: a boolean if delete netns is safe"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_4c664ebc","line":117,"in_reply_to":"5a890539_36cde366","updated":"2014-11-18 09:58:22.000000000","message":"Done","commit_id":"69c105901f3ef1f5962597b1c4d6e4d5f49569b2"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f1e59716f7539175372bee63456a5f8321b0c992","unresolved":false,"context_lines":[{"line_number":129,"context_line":"    # async call, make one netns busy"},{"line_number":130,"context_line":"    sleep_proc, cmd \u003d agent_utils.create_process("},{"line_number":131,"context_line":"        [\u0027ip\u0027, \u0027netns\u0027, \u0027exec\u0027, testns1, \u0027sleep\u0027, \u00275\u0027], root_helper)"},{"line_number":132,"context_line":"    # add latency after sleep"},{"line_number":133,"context_line":"    time.sleep(0.5)"},{"line_number":134,"context_line":"    try:"},{"line_number":135,"context_line":"        # try deleting the other (not busy) one"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_160f5c89","line":132,"updated":"2014-12-19 00:10:09.000000000","message":"Why is this latency required?  I would assume that create_process() returning indicates that \u0027sleep\u0027 was running and has rendered the namespace busy.\n\nAlso, there would appear to be a potential timing issue here.  If 5s elapses before the deletion is attempted, True might be returned in error.  Do you think it might be worth adding a watchdog timer that raises an exception if deletion is not attempted during the expected interval of the sleep invocation?","commit_id":"3d01a51aac79dca18e0e611c6fa61c9980d7cd2a"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"d81108be99e80cb9874ccef3b5f32fee3fe4932b","unresolved":false,"context_lines":[{"line_number":129,"context_line":"    # async call, make one netns busy"},{"line_number":130,"context_line":"    sleep_proc, cmd \u003d agent_utils.create_process("},{"line_number":131,"context_line":"        [\u0027ip\u0027, \u0027netns\u0027, \u0027exec\u0027, testns1, \u0027sleep\u0027, \u00275\u0027], root_helper)"},{"line_number":132,"context_line":"    # add latency after sleep"},{"line_number":133,"context_line":"    time.sleep(0.5)"},{"line_number":134,"context_line":"    try:"},{"line_number":135,"context_line":"        # try deleting the other (not busy) one"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_f53c23c1","line":132,"in_reply_to":"3a961159_160f5c89","updated":"2015-01-08 18:37:55.000000000","message":"create_process() is async call and returns immediately with a future object. Neither the return of the call nor the future obj can indicate sleep is running inside the netns. 0,5s is very big, enough for sleep to start. And the deletion comes right after that.","commit_id":"3d01a51aac79dca18e0e611c6fa61c9980d7cd2a"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f1e59716f7539175372bee63456a5f8321b0c992","unresolved":false,"context_lines":[{"line_number":135,"context_line":"        # try deleting the other (not busy) one"},{"line_number":136,"context_line":"        ip.netns.delete(testns2)"},{"line_number":137,"context_line":"        return True"},{"line_number":138,"context_line":"    except Exception:"},{"line_number":139,"context_line":"        if sleep_proc and sleep_proc.poll() is None:"},{"line_number":140,"context_line":"            sleep_proc.terminate()"},{"line_number":141,"context_line":"            # if not terminate immediately, wait for it"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_76f9804d","line":138,"updated":"2014-12-19 00:10:09.000000000","message":"Please trap for a more explicit exception here.  A catch-all like this is likely to mask unrelated errors and make them difficult to debug.","commit_id":"3d01a51aac79dca18e0e611c6fa61c9980d7cd2a"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"d81108be99e80cb9874ccef3b5f32fee3fe4932b","unresolved":false,"context_lines":[{"line_number":135,"context_line":"        # try deleting the other (not busy) one"},{"line_number":136,"context_line":"        ip.netns.delete(testns2)"},{"line_number":137,"context_line":"        return True"},{"line_number":138,"context_line":"    except Exception:"},{"line_number":139,"context_line":"        if sleep_proc and sleep_proc.poll() is None:"},{"line_number":140,"context_line":"            sleep_proc.terminate()"},{"line_number":141,"context_line":"            # if not terminate immediately, wait for it"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_d5e72710","line":138,"in_reply_to":"3a961159_76f9804d","updated":"2015-01-08 18:37:55.000000000","message":"ok, explicit to RuntimeError","commit_id":"3d01a51aac79dca18e0e611c6fa61c9980d7cd2a"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"35765b6348bead7cdaa1c3478d7acd153aaa75c8","unresolved":false,"context_lines":[{"line_number":152,"context_line":"    sleep_proc, cmd \u003d agent_utils.create_process("},{"line_number":153,"context_line":"        [\u0027ip\u0027, \u0027netns\u0027, \u0027exec\u0027, testns1, \u0027sleep\u0027, \u00275\u0027], root_helper)"},{"line_number":154,"context_line":"    # add latency after sleep"},{"line_number":155,"context_line":"    time.sleep(0.5)"},{"line_number":156,"context_line":"    try:"},{"line_number":157,"context_line":"        # try deleting the other (not busy) one"},{"line_number":158,"context_line":"        ip.netns.delete(testns2)"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_fe307bea","line":155,"updated":"2015-01-08 23:53:15.000000000","message":"Relying on sleep in this way opens us up to timing issues.  Maybe half a second is plenty on your development host, but what if a deployment tool uses this check on a cpu-loaded box?  And what if for some crazy reason the sleep process exits before namespace deletion is attempted?\n\nIt\u0027s possible that I\u0027m being entirely too strict here, and I would welcome that feedback, but in an ideal world I think the process in the namespace would persist rather than being time-limited and its launch would be confirmed rather than expected within a certain timeframe.","commit_id":"3f9fdefd384bfa17db63b1725be5c2de565ac24a"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"ecfd87d3024ea49edb995fc03fd808062c9368ad","unresolved":false,"context_lines":[{"line_number":152,"context_line":"    sleep_proc, cmd \u003d agent_utils.create_process("},{"line_number":153,"context_line":"        [\u0027ip\u0027, \u0027netns\u0027, \u0027exec\u0027, testns1, \u0027sleep\u0027, \u00275\u0027], root_helper)"},{"line_number":154,"context_line":"    # add latency after sleep"},{"line_number":155,"context_line":"    time.sleep(0.5)"},{"line_number":156,"context_line":"    try:"},{"line_number":157,"context_line":"        # try deleting the other (not busy) one"},{"line_number":158,"context_line":"        ip.netns.delete(testns2)"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_be14174e","line":155,"in_reply_to":"3a961159_402fe5f5","updated":"2015-01-10 01:25:25.000000000","message":"There is lots of code in the functional test suite that deals with detecting when system state has been achieved (see: wait_until_true) and there is also support for running and managing simple daemon processes.  I would encourage you to examine these and solve the problem properly rather than implementing work-arounds.","commit_id":"3f9fdefd384bfa17db63b1725be5c2de565ac24a"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"66b0e79cde0a79fb8f89832d32c3eedc0818b2b6","unresolved":false,"context_lines":[{"line_number":152,"context_line":"    sleep_proc, cmd \u003d agent_utils.create_process("},{"line_number":153,"context_line":"        [\u0027ip\u0027, \u0027netns\u0027, \u0027exec\u0027, testns1, \u0027sleep\u0027, \u00275\u0027], root_helper)"},{"line_number":154,"context_line":"    # add latency after sleep"},{"line_number":155,"context_line":"    time.sleep(0.5)"},{"line_number":156,"context_line":"    try:"},{"line_number":157,"context_line":"        # try deleting the other (not busy) one"},{"line_number":158,"context_line":"        ip.netns.delete(testns2)"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_bc9de2bd","line":155,"in_reply_to":"3a961159_be14174e","updated":"2015-01-12 15:13:08.000000000","message":"Good. Use \u0027RootHelperProcess\u0027 to make sure sleep is being executed on return","commit_id":"3f9fdefd384bfa17db63b1725be5c2de565ac24a"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"0834b6ec367fd15e22257239741e99b4ce6eefaa","unresolved":false,"context_lines":[{"line_number":152,"context_line":"    sleep_proc, cmd \u003d agent_utils.create_process("},{"line_number":153,"context_line":"        [\u0027ip\u0027, \u0027netns\u0027, \u0027exec\u0027, testns1, \u0027sleep\u0027, \u00275\u0027], root_helper)"},{"line_number":154,"context_line":"    # add latency after sleep"},{"line_number":155,"context_line":"    time.sleep(0.5)"},{"line_number":156,"context_line":"    try:"},{"line_number":157,"context_line":"        # try deleting the other (not busy) one"},{"line_number":158,"context_line":"        ip.netns.delete(testns2)"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_402fe5f5","line":155,"in_reply_to":"3a961159_fe307bea","updated":"2015-01-09 12:25:18.000000000","message":"after create_process() was called and returned, a child process is started and returned even a \"pid\" immediately. But the sleep inside seems to have some latency to effectively lock the deletion of other netns. The reason is unknown. I understand 0,5s is a guess, not ideal, but I do not have any better ideas.\n\nsleep actively exist after a limit of time inside netns, and not persist here, because we are not 100% sure to be able to force the sleep inside netns to terminate by using terminate(). In some of my tests, terminate() executed successfully but  sleep did not terminate. It\u0027s the reason why I put a communicate() after it to wait, since we can catch when the child process ends.","commit_id":"3f9fdefd384bfa17db63b1725be5c2de565ac24a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"3950648faf023664602c7255c47e4488c4ffd9cb","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        # try deleting the other (not busy) one"},{"line_number":158,"context_line":"        ip.netns.delete(testns2)"},{"line_number":159,"context_line":"        return True"},{"line_number":160,"context_line":"    except RuntimeError:"},{"line_number":161,"context_line":"        if sleep_proc and sleep_proc.poll() is None:"},{"line_number":162,"context_line":"            sleep_proc.terminate()"},{"line_number":163,"context_line":"            # if not terminate immediately, wait for it"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_e40c7344","line":160,"updated":"2015-01-08 21:12:18.000000000","message":"In this case, I don\u0027t think you can catch a more specific exception without changing how ip_lib (more specifically utils.execute) works.","commit_id":"3f9fdefd384bfa17db63b1725be5c2de565ac24a"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"df90fff8c24174a29512cd476e28333abc808588","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from neutron.openstack.common import uuidutils"},{"line_number":26,"context_line":"from neutron.plugins.common import constants as const"},{"line_number":27,"context_line":"from neutron.plugins.openvswitch.common import constants as ovs_const"},{"line_number":28,"context_line":"from neutron.tests.functional.agent.linux import helpers"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_9d8b5d68","line":28,"updated":"2015-01-14 23:25:29.000000000","message":"My intention was to point you to how to perform a safer check, not to encourage you to import test code into the non-test tree.  Doing so complicates detection of what ci jobs need to run in response to a change.  Please refactor the code you want to use into the non-test tree or otherwise ensure that this module does not depend on the test tree.","commit_id":"9a7a610928e09d9e6e927b9668d1dace0c391390"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"952a92e57b79c23f16b9345c5f2d7e1b86342367","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from neutron.openstack.common import uuidutils"},{"line_number":26,"context_line":"from neutron.plugins.common import constants as const"},{"line_number":27,"context_line":"from neutron.plugins.openvswitch.common import constants as ovs_const"},{"line_number":28,"context_line":"from neutron.tests.functional.agent.linux import helpers"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"1a930d6b_620fc037","line":28,"in_reply_to":"3a961159_4963bfdc","updated":"2015-01-27 00:28:13.000000000","message":"I don\u0027t care how you go about it, but there is no way a patch introducing a dependency on the testing hierarchy is going to merge.  Either stop using the helpers module entirely, or relocate the functionality you are depending on available outside of the test tree.  It would probably be better if it was a parent change, but if the modifications were clean enough they could be included in this change as well.","commit_id":"9a7a610928e09d9e6e927b9668d1dace0c391390"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"a09abba5c535a9c1f3c2166fc8a492c6dca4175c","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from neutron.openstack.common import uuidutils"},{"line_number":26,"context_line":"from neutron.plugins.common import constants as const"},{"line_number":27,"context_line":"from neutron.plugins.openvswitch.common import constants as ovs_const"},{"line_number":28,"context_line":"from neutron.tests.functional.agent.linux import helpers"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_e8a1da95","line":28,"in_reply_to":"3a961159_5ab569ce","updated":"2015-01-15 20:06:53.000000000","message":"My intention in asking you to \u0027refactor the code you want to use\u0027 was to extract part or all of the code you are using in this patch out of the functional tree.  The fact that the helpers module is currently part of the testing tree is because no non-test code was using it, and in your refactor I would ask you to ensure that you extract only what you\u0027ll actually be using.","commit_id":"9a7a610928e09d9e6e927b9668d1dace0c391390"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"6d2050151b9b252c9a1990c289efd7bc33732c75","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from neutron.openstack.common import uuidutils"},{"line_number":26,"context_line":"from neutron.plugins.common import constants as const"},{"line_number":27,"context_line":"from neutron.plugins.openvswitch.common import constants as ovs_const"},{"line_number":28,"context_line":"from neutron.tests.functional.agent.linux import helpers"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_5ab569ce","line":28,"in_reply_to":"3a961159_9d8b5d68","updated":"2015-01-15 10:17:52.000000000","message":"Please take a look into RootHelperProcess, to see how many codes it implemented to wait for a cmd of child process to execute.  There are no points to reinvent the implementation here. Indeed this helpers is very useful and they should be move in a lib, not tests.functional.","commit_id":"9a7a610928e09d9e6e927b9668d1dace0c391390"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"8e524f6eeb67c3f96528eb0fb171c0586458af5f","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from neutron.openstack.common import uuidutils"},{"line_number":26,"context_line":"from neutron.plugins.common import constants as const"},{"line_number":27,"context_line":"from neutron.plugins.openvswitch.common import constants as ovs_const"},{"line_number":28,"context_line":"from neutron.tests.functional.agent.linux import helpers"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_4963bfdc","line":28,"in_reply_to":"3a961159_e8a1da95","updated":"2015-01-16 10:07:48.000000000","message":"Do you think it is the scope of this ticket to refactor RootHelperProcess? I can ask the author of RootHelperProcess to do so or create another ticket. But I don\u0027t think it is the scope here.","commit_id":"9a7a610928e09d9e6e927b9668d1dace0c391390"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"df90fff8c24174a29512cd476e28333abc808588","unresolved":false,"context_lines":[{"line_number":149,"context_line":"    ip.ensure_namespace(testns1)"},{"line_number":150,"context_line":"    ip.ensure_namespace(testns2)"},{"line_number":151,"context_line":"    # make one netns busy, make sure sleep is being executed on return"},{"line_number":152,"context_line":"    sleep_proc \u003d helpers.RootHelperProcess([\u0027sleep\u0027, \u00275\u0027],"},{"line_number":153,"context_line":"                                           namespace\u003dtestns1,"},{"line_number":154,"context_line":"                                           root_helper\u003droot_helper)"},{"line_number":155,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_fd30e19f","line":152,"updated":"2015-01-14 23:25:29.000000000","message":"I\u0027m still not convinced of the wisdom of relying on an auto-expiring process.  Wouldn\u0027t it make more sense to set a longer timeout and have to kill the process?  The process returned by RootHelperProcess has a kill() method that invokes kill -9 (which would require a KillFilter for the name of the target) that is likely to be more effective than the method currently employed.","commit_id":"9a7a610928e09d9e6e927b9668d1dace0c391390"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"8e524f6eeb67c3f96528eb0fb171c0586458af5f","unresolved":false,"context_lines":[{"line_number":149,"context_line":"    ip.ensure_namespace(testns1)"},{"line_number":150,"context_line":"    ip.ensure_namespace(testns2)"},{"line_number":151,"context_line":"    # make one netns busy, make sure sleep is being executed on return"},{"line_number":152,"context_line":"    sleep_proc \u003d helpers.RootHelperProcess([\u0027sleep\u0027, \u00275\u0027],"},{"line_number":153,"context_line":"                                           namespace\u003dtestns1,"},{"line_number":154,"context_line":"                                           root_helper\u003droot_helper)"},{"line_number":155,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_a64a9080","line":152,"in_reply_to":"3a961159_4dfe8007","updated":"2015-01-16 10:07:48.000000000","message":"in compare to SIGKILL, SIGTERM allows the process to perform nice termination releasing resources and saving state if appropriate [1].\n\nI want the process to have a chance to clean up itself in the test, otherwise it may i.e. leave corrupted files (or other state) that we cannot understand once restarted.\n\n[1] http://en.wikipedia.org/wiki/Unix_signal#SIGKILL","commit_id":"9a7a610928e09d9e6e927b9668d1dace0c391390"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"952a92e57b79c23f16b9345c5f2d7e1b86342367","unresolved":false,"context_lines":[{"line_number":149,"context_line":"    ip.ensure_namespace(testns1)"},{"line_number":150,"context_line":"    ip.ensure_namespace(testns2)"},{"line_number":151,"context_line":"    # make one netns busy, make sure sleep is being executed on return"},{"line_number":152,"context_line":"    sleep_proc \u003d helpers.RootHelperProcess([\u0027sleep\u0027, \u00275\u0027],"},{"line_number":153,"context_line":"                                           namespace\u003dtestns1,"},{"line_number":154,"context_line":"                                           root_helper\u003droot_helper)"},{"line_number":155,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":16,"id":"1a930d6b_223bf8eb","line":152,"in_reply_to":"3a961159_a64a9080","updated":"2015-01-27 00:28:13.000000000","message":"You\u0027re talking about \u0027sleep\u0027 here.  I\u0027m not sure I see the danger of invoking SIGKILL here to make process cleanup more reliable.","commit_id":"9a7a610928e09d9e6e927b9668d1dace0c391390"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"a09abba5c535a9c1f3c2166fc8a492c6dca4175c","unresolved":false,"context_lines":[{"line_number":149,"context_line":"    ip.ensure_namespace(testns1)"},{"line_number":150,"context_line":"    ip.ensure_namespace(testns2)"},{"line_number":151,"context_line":"    # make one netns busy, make sure sleep is being executed on return"},{"line_number":152,"context_line":"    sleep_proc \u003d helpers.RootHelperProcess([\u0027sleep\u0027, \u00275\u0027],"},{"line_number":153,"context_line":"                                           namespace\u003dtestns1,"},{"line_number":154,"context_line":"                                           root_helper\u003droot_helper)"},{"line_number":155,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_4dfe8007","line":152,"in_reply_to":"3a961159_daadd9eb","updated":"2015-01-15 20:06:53.000000000","message":"If you\u0027re going to use RootHelperProcess, which has an expanded capability to kill immediately with SIGKILL, why does it make sense to continue using terminate and communicate?","commit_id":"9a7a610928e09d9e6e927b9668d1dace0c391390"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"6d2050151b9b252c9a1990c289efd7bc33732c75","unresolved":false,"context_lines":[{"line_number":149,"context_line":"    ip.ensure_namespace(testns1)"},{"line_number":150,"context_line":"    ip.ensure_namespace(testns2)"},{"line_number":151,"context_line":"    # make one netns busy, make sure sleep is being executed on return"},{"line_number":152,"context_line":"    sleep_proc \u003d helpers.RootHelperProcess([\u0027sleep\u0027, \u00275\u0027],"},{"line_number":153,"context_line":"                                           namespace\u003dtestns1,"},{"line_number":154,"context_line":"                                           root_helper\u003droot_helper)"},{"line_number":155,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_daadd9eb","line":152,"in_reply_to":"3a961159_fd30e19f","updated":"2015-01-15 10:17:52.000000000","message":"I used terminate() from subprocess below. Reference to the doc of subprocess if you have any doubts.","commit_id":"9a7a610928e09d9e6e927b9668d1dace0c391390"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"5d0f59ee40bfde0426547f19359023a5ccc26777","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from neutron.openstack.common import uuidutils"},{"line_number":26,"context_line":"from neutron.plugins.common import constants as const"},{"line_number":27,"context_line":"from neutron.plugins.openvswitch.common import constants as ovs_const"},{"line_number":28,"context_line":"from neutron.tests.functional.agent.linux import helpers"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"da86d52c_e191a153","line":28,"updated":"2015-01-31 07:13:11.000000000","message":"In total agreement with Maru PS16 comments here. Please extract out the\nfunctionality you want to use here to a common lib. Either in this patch\nor in a patch before this one.","commit_id":"4ca7037f7e365e762f95300211382dcff427baca"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"5231bf006726589f48c5c931093dff8a5fb6d2b7","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from neutron.openstack.common import uuidutils"},{"line_number":26,"context_line":"from neutron.plugins.common import constants as const"},{"line_number":27,"context_line":"from neutron.plugins.openvswitch.common import constants as ovs_const"},{"line_number":28,"context_line":"from neutron.tests.functional.agent.linux import helpers"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"da86d52c_066162d3","line":28,"in_reply_to":"da86d52c_7c7c7873","updated":"2015-02-07 14:47:10.000000000","message":"Thanks Miguel, I add a parent commit to refactor RootHelperProcess. Please take a look if you have time.\n\nhttps://review.openstack.org/#/c/153785/","commit_id":"4ca7037f7e365e762f95300211382dcff427baca"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"446449b2f1984e2346e04cd70c6113d9363d730d","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from neutron.openstack.common import uuidutils"},{"line_number":26,"context_line":"from neutron.plugins.common import constants as const"},{"line_number":27,"context_line":"from neutron.plugins.openvswitch.common import constants as ovs_const"},{"line_number":28,"context_line":"from neutron.tests.functional.agent.linux import helpers"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"da86d52c_7c7c7873","line":28,"in_reply_to":"da86d52c_e191a153","updated":"2015-01-31 09:08:27.000000000","message":"Also, there is also a very strong point against this.\n\nSome distributions cut out the test directory in packaging. At least that\u0027s what happens for Red Hat, not sure about other distros.","commit_id":"4ca7037f7e365e762f95300211382dcff427baca"}],"neutron/cmd/sanity_check.py":[{"author":{"_account_id":6598,"name":"Berezovsky Irena","email":"irenab.dev@gmail.com","username":"irenab"},"change_message_id":"ea2161899032f381e05be9a1bc80cb29f8d20a52","unresolved":false,"context_lines":[{"line_number":68,"context_line":"    result \u003d checks.del_netns_supported(root_helper\u003dcfg.CONF.AGENT.root_helper)"},{"line_number":69,"context_line":"    if not result:"},{"line_number":70,"context_line":"        LOG.error(_(\u0027Check for deleting mounted netns failed. \u0027"},{"line_number":71,"context_line":"                    \u0027Please ensure that the version of iproute being used can \u0027"},{"line_number":72,"context_line":"                    \u0027safely delete mounted netns or disable config \u0027"},{"line_number":73,"context_line":"                    \u0027router_delete_namespaces and dhcp_delete_namespaces\u0027))"},{"line_number":74,"context_line":"    return result"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_70af6de3","line":71,"updated":"2014-07-27 06:11:35.000000000","message":"Can the error log be more specific for what version of iproute to use?","commit_id":"32eb8a513edd77dbc780cab6a8d6cbe0adb51cf3"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"c3479e6e8a5d940f8f64b78a6ddc46f44ce62af6","unresolved":false,"context_lines":[{"line_number":68,"context_line":"    result \u003d checks.del_netns_supported(root_helper\u003dcfg.CONF.AGENT.root_helper)"},{"line_number":69,"context_line":"    if not result:"},{"line_number":70,"context_line":"        LOG.error(_(\u0027Check for deleting mounted netns failed. \u0027"},{"line_number":71,"context_line":"                    \u0027Please ensure that the version of iproute being used can \u0027"},{"line_number":72,"context_line":"                    \u0027safely delete mounted netns or disable config \u0027"},{"line_number":73,"context_line":"                    \u0027router_delete_namespaces and dhcp_delete_namespaces\u0027))"},{"line_number":74,"context_line":"    return result"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_c4c77844","line":71,"in_reply_to":"baada198_70af6de3","updated":"2014-08-12 15:34:10.000000000","message":"the version is different for linux distros. For example it\u0027s 20111117-1ubuntu2.3 and iproute-2.6.32-130 for Ubuntu and Red Hat respectively. I will give this as a hint and a reference to the iproute patch for more info.","commit_id":"32eb8a513edd77dbc780cab6a8d6cbe0adb51cf3"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"13e9269f2211609417ca1dd3adc02545ce104cc6","unresolved":false,"context_lines":[{"line_number":81,"context_line":"    BoolOptCallback(\u0027ovs_patch\u0027, check_ovs_patch, default\u003dFalse,"},{"line_number":82,"context_line":"                    help\u003d_(\u0027Check for patch port support\u0027)),"},{"line_number":83,"context_line":"    BoolOptCallback(\u0027nova_notify\u0027, check_nova_notify, default\u003dFalse,"},{"line_number":84,"context_line":"                    help\u003d_(\u0027Check for nova notification support\u0027)),"},{"line_number":85,"context_line":"    BoolOptCallback(\u0027iproute_patch\u0027, check_delete_netns, default\u003dFalse,"},{"line_number":86,"context_line":"                    help\u003d_(\u0027Check for deleting netns support\u0027)),"},{"line_number":87,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_6aff9501","line":84,"updated":"2014-07-20 16:39:49.000000000","message":"The naming convention is according to capability, so this should be named something like \"delete_ip_namespace\".","commit_id":"32eb8a513edd77dbc780cab6a8d6cbe0adb51cf3"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"c3479e6e8a5d940f8f64b78a6ddc46f44ce62af6","unresolved":false,"context_lines":[{"line_number":81,"context_line":"    BoolOptCallback(\u0027ovs_patch\u0027, check_ovs_patch, default\u003dFalse,"},{"line_number":82,"context_line":"                    help\u003d_(\u0027Check for patch port support\u0027)),"},{"line_number":83,"context_line":"    BoolOptCallback(\u0027nova_notify\u0027, check_nova_notify, default\u003dFalse,"},{"line_number":84,"context_line":"                    help\u003d_(\u0027Check for nova notification support\u0027)),"},{"line_number":85,"context_line":"    BoolOptCallback(\u0027iproute_patch\u0027, check_delete_netns, default\u003dFalse,"},{"line_number":86,"context_line":"                    help\u003d_(\u0027Check for deleting netns support\u0027)),"},{"line_number":87,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_04d5f0e4","line":84,"in_reply_to":"baada198_6aff9501","updated":"2014-08-12 15:34:10.000000000","message":"alright, will change to delete_ip_namespace","commit_id":"32eb8a513edd77dbc780cab6a8d6cbe0adb51cf3"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"13e9269f2211609417ca1dd3adc02545ce104cc6","unresolved":false,"context_lines":[{"line_number":103,"context_line":"            cfg.CONF.notify_nova_on_port_data_changes):"},{"line_number":104,"context_line":"        cfg.CONF.set_override(\u0027nova_notify\u0027, True)"},{"line_number":105,"context_line":"    if(cfg.CONF.router_delete_namespaces or"},{"line_number":106,"context_line":"            cfg.CONF.dhcp_delete_namespaces):"},{"line_number":107,"context_line":"        cfg.CONF.set_override(\u0027iproute_patch\u0027, True)"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_ca0a611f","line":106,"updated":"2014-07-20 16:39:49.000000000","message":"Same comment about name.","commit_id":"32eb8a513edd77dbc780cab6a8d6cbe0adb51cf3"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"c3479e6e8a5d940f8f64b78a6ddc46f44ce62af6","unresolved":false,"context_lines":[{"line_number":103,"context_line":"            cfg.CONF.notify_nova_on_port_data_changes):"},{"line_number":104,"context_line":"        cfg.CONF.set_override(\u0027nova_notify\u0027, True)"},{"line_number":105,"context_line":"    if(cfg.CONF.router_delete_namespaces or"},{"line_number":106,"context_line":"            cfg.CONF.dhcp_delete_namespaces):"},{"line_number":107,"context_line":"        cfg.CONF.set_override(\u0027iproute_patch\u0027, True)"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_4429a8f0","line":106,"in_reply_to":"baada198_ca0a611f","updated":"2014-08-12 15:34:10.000000000","message":"alright, will change to delete_ip_namespace","commit_id":"32eb8a513edd77dbc780cab6a8d6cbe0adb51cf3"},{"author":{"_account_id":6598,"name":"Berezovsky Irena","email":"irenab.dev@gmail.com","username":"irenab"},"change_message_id":"d33968650b9768dafdccabec017b0d84dbd8092f","unresolved":false,"context_lines":[{"line_number":78,"context_line":"def check_delete_netns():"},{"line_number":79,"context_line":"    result \u003d checks.del_netns_supported(root_helper\u003dcfg.CONF.AGENT.root_helper)"},{"line_number":80,"context_line":"    if not result:"},{"line_number":81,"context_line":"        LOG.error(_(\u0027Check for deleting mounted netns failed. \u0027"},{"line_number":82,"context_line":"                    \u0027Please ensure that the version of iproute being used can \u0027"},{"line_number":83,"context_line":"                    \u0027safely delete mounted netns or disable config \u0027"},{"line_number":84,"context_line":"                    \u0027router_delete_namespaces and dhcp_delete_namespaces, \u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"1abeadc6_86ac6137","line":81,"updated":"2014-08-26 10:33:22.000000000","message":"please use _LE to log error messages","commit_id":"214299d5e2dfd273d397429acba1590b6c2370fa"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"8294c5d2d6061ec17bea836daac2fa92a56350bb","unresolved":false,"context_lines":[{"line_number":78,"context_line":"def check_delete_netns():"},{"line_number":79,"context_line":"    result \u003d checks.del_netns_supported(root_helper\u003dcfg.CONF.AGENT.root_helper)"},{"line_number":80,"context_line":"    if not result:"},{"line_number":81,"context_line":"        LOG.error(_(\u0027Check for deleting mounted netns failed. \u0027"},{"line_number":82,"context_line":"                    \u0027Please ensure that the version of iproute being used can \u0027"},{"line_number":83,"context_line":"                    \u0027safely delete mounted netns or disable config \u0027"},{"line_number":84,"context_line":"                    \u0027router_delete_namespaces and dhcp_delete_namespaces, \u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"1abeadc6_1d724ee3","line":81,"in_reply_to":"1abeadc6_86ac6137","updated":"2014-08-26 16:24:31.000000000","message":"ok","commit_id":"214299d5e2dfd273d397429acba1590b6c2370fa"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"97ebba202a317d8ea4fafb84162b39a73f87101c","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                    \u0027Please ensure that the version of iproute being used can \u0027"},{"line_number":83,"context_line":"                    \u0027safely delete mounted netns or disable config \u0027"},{"line_number":84,"context_line":"                    \u0027router_delete_namespaces and dhcp_delete_namespaces, \u0027"},{"line_number":85,"context_line":"                    \u0027for instances 20111117-1ubuntu2.3 and iproute-2.6.32-130 \u0027"},{"line_number":86,"context_line":"                    \u0027for Ubuntu and Red Hat respectively. Bug report go to \u0027"},{"line_number":87,"context_line":"                    \u0027Red Hat Bugzilla - Bug 1062685\u0027))"},{"line_number":88,"context_line":"    return result"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_cdc6b399","line":85,"updated":"2014-09-05 18:28:17.000000000","message":"Not sure you should provide such details as they are not exhaustive (what about debian version?)","commit_id":"33f85234836e90deac6a0c98458e4a9998c37d45"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"c4b33f2b10023c831558fe069b6c26584475b3b0","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                    \u0027Please ensure that the version of iproute being used can \u0027"},{"line_number":83,"context_line":"                    \u0027safely delete mounted netns or disable config \u0027"},{"line_number":84,"context_line":"                    \u0027router_delete_namespaces and dhcp_delete_namespaces, \u0027"},{"line_number":85,"context_line":"                    \u0027for instances 20111117-1ubuntu2.3 and iproute-2.6.32-130 \u0027"},{"line_number":86,"context_line":"                    \u0027for Ubuntu and Red Hat respectively. Bug report go to \u0027"},{"line_number":87,"context_line":"                    \u0027Red Hat Bugzilla - Bug 1062685\u0027))"},{"line_number":88,"context_line":"    return result"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_a1c7a845","line":85,"in_reply_to":"fa98f980_cdc6b399","updated":"2014-09-16 13:36:59.000000000","message":"Just because the information isn\u0027t exhaustive doesn\u0027t mean you shouldn\u0027t give it. I find it helpful.","commit_id":"33f85234836e90deac6a0c98458e4a9998c37d45"},{"author":{"_account_id":105,"name":"Kyle Mestery","email":"mestery@mestery.com","username":"mestery"},"change_message_id":"e8596a411b41200e876eeacea11fb31007a135bc","unresolved":false,"context_lines":[{"line_number":90,"context_line":"def check_delete_netns():"},{"line_number":91,"context_line":"    result \u003d checks.del_netns_supported(root_helper\u003dcfg.CONF.AGENT.root_helper)"},{"line_number":92,"context_line":"    if not result:"},{"line_number":93,"context_line":"        LOG.error(_LE(\u0027Check for deleting mounted netns failed. \u0027"},{"line_number":94,"context_line":"                    \u0027Please ensure that the version of iproute being used can \u0027"},{"line_number":95,"context_line":"                    \u0027safely delete mounted netns or disable config \u0027"},{"line_number":96,"context_line":"                    \u0027router_delete_namespaces and dhcp_delete_namespaces, \u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_f6e6fbe6","line":93,"updated":"2014-11-17 21:08:26.000000000","message":"Nit: How about this instead:\n\n\"Check if deleting a mounted nets fails.\"","commit_id":"69c105901f3ef1f5962597b1c4d6e4d5f49569b2"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"18a368c9969aa37fe355357255ffa13afd722019","unresolved":false,"context_lines":[{"line_number":90,"context_line":"def check_delete_netns():"},{"line_number":91,"context_line":"    result \u003d checks.del_netns_supported(root_helper\u003dcfg.CONF.AGENT.root_helper)"},{"line_number":92,"context_line":"    if not result:"},{"line_number":93,"context_line":"        LOG.error(_LE(\u0027Check for deleting mounted netns failed. \u0027"},{"line_number":94,"context_line":"                    \u0027Please ensure that the version of iproute being used can \u0027"},{"line_number":95,"context_line":"                    \u0027safely delete mounted netns or disable config \u0027"},{"line_number":96,"context_line":"                    \u0027router_delete_namespaces and dhcp_delete_namespaces, \u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_624320af","line":93,"in_reply_to":"5a890539_f6e6fbe6","updated":"2014-11-18 00:27:49.000000000","message":"@Kyle:  *netns","commit_id":"69c105901f3ef1f5962597b1c4d6e4d5f49569b2"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"d7834b03961ea697b23ca477026b18199d4d9e03","unresolved":false,"context_lines":[{"line_number":90,"context_line":"def check_delete_netns():"},{"line_number":91,"context_line":"    result \u003d checks.del_netns_supported(root_helper\u003dcfg.CONF.AGENT.root_helper)"},{"line_number":92,"context_line":"    if not result:"},{"line_number":93,"context_line":"        LOG.error(_LE(\u0027Check for deleting mounted netns failed. \u0027"},{"line_number":94,"context_line":"                    \u0027Please ensure that the version of iproute being used can \u0027"},{"line_number":95,"context_line":"                    \u0027safely delete mounted netns or disable config \u0027"},{"line_number":96,"context_line":"                    \u0027router_delete_namespaces and dhcp_delete_namespaces, \u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_4cfd6eb0","line":93,"in_reply_to":"5a890539_f6e6fbe6","updated":"2014-11-18 09:58:22.000000000","message":"Done","commit_id":"69c105901f3ef1f5962597b1c4d6e4d5f49569b2"},{"author":{"_account_id":4656,"name":"Sean M. Collins","email":"sean@coreitpro.com","username":"scollins"},"change_message_id":"48eaf5bd46f65bb95073a3a8744fa8a63d3c809b","unresolved":false,"context_lines":[{"line_number":93,"context_line":"        LOG.error(_LE(\u0027Check if deleting mounted netns fails. \u0027"},{"line_number":94,"context_line":"                    \u0027Please ensure that the version of iproute being used can \u0027"},{"line_number":95,"context_line":"                    \u0027safely delete mounted netns or disable config \u0027"},{"line_number":96,"context_line":"                    \u0027router_delete_namespaces and dhcp_delete_namespaces, \u0027"},{"line_number":97,"context_line":"                    \u0027for instances 20111117-1ubuntu2.3 and iproute-2.6.32-130 \u0027"},{"line_number":98,"context_line":"                    \u0027for Ubuntu and Red Hat respectively. Bug report go to \u0027"},{"line_number":99,"context_line":"                    \u0027Red Hat Bugzilla - Bug 1062685\u0027))"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_22b17a8f","line":96,"updated":"2014-12-18 21:37:50.000000000","message":"Nit - \"or disable the router_delete_namespaces and dhcp_delete namespaces config variables\"","commit_id":"3d01a51aac79dca18e0e611c6fa61c9980d7cd2a"},{"author":{"_account_id":4656,"name":"Sean M. Collins","email":"sean@coreitpro.com","username":"scollins"},"change_message_id":"48eaf5bd46f65bb95073a3a8744fa8a63d3c809b","unresolved":false,"context_lines":[{"line_number":95,"context_line":"                    \u0027safely delete mounted netns or disable config \u0027"},{"line_number":96,"context_line":"                    \u0027router_delete_namespaces and dhcp_delete_namespaces, \u0027"},{"line_number":97,"context_line":"                    \u0027for instances 20111117-1ubuntu2.3 and iproute-2.6.32-130 \u0027"},{"line_number":98,"context_line":"                    \u0027for Ubuntu and Red Hat respectively. Bug report go to \u0027"},{"line_number":99,"context_line":"                    \u0027Red Hat Bugzilla - Bug 1062685\u0027))"},{"line_number":100,"context_line":"    return result"},{"line_number":101,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_42ae862f","line":98,"updated":"2014-12-18 21:37:50.000000000","message":"Nit - \"Documented in Red Hat Bugzilla - Bug 1062685\"","commit_id":"3d01a51aac79dca18e0e611c6fa61c9980d7cd2a"}],"neutron/tests/functional/sanity/test_sanity.py":[{"author":{"_account_id":2592,"name":"Mark McClain","email":"mark@mcclain.xyz","username":"markmcclain"},"change_message_id":"c56a619bb4d1b9b6ee6874e6bab290cd1068f193","unresolved":false,"context_lines":[{"line_number":56,"context_line":"    def test_vf_management_runs(self):"},{"line_number":57,"context_line":"        checks.vf_management_supported(self.root_helper)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def test_delete_netns_supported(self):"},{"line_number":60,"context_line":"        checks.del_netns_supported(self.root_helper)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a890539_f0cf1a54","line":59,"updated":"2014-11-14 16:08:40.000000000","message":"I know the rest of the functional tests in this file are of the same style, but seems like a really weak validation that the method actually works.  Not raising an exception does not actually mean the check validates functionality.","commit_id":"d4d0dcbfda9a7f5942be03fee7596f20e48aa839"}],"neutron/tests/unit/test_sanity_check.py":[{"author":{"_account_id":6598,"name":"Berezovsky Irena","email":"irenab.dev@gmail.com","username":"irenab"},"change_message_id":"2e72539ee10de5bd2fb62460e5e02f2f25a99170","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":"#"},{"line_number":16,"context_line":"# @author: Tri Hoang Vo, Deutsche Telekom"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"\"\"\"Test Sanity Checks For Neutron\"\"\""},{"line_number":19,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1abeadc6_01194922","line":16,"updated":"2014-08-25 10:29:12.000000000","message":"I think there is a directive to remove @author from files","commit_id":"e4f997190b59432bd8dc0802cd1c757a1e64b1f0"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"213b685ad7757b73685039709dcc684e21aefaa4","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":"#"},{"line_number":16,"context_line":"# @author: Tri Hoang Vo, Deutsche Telekom"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"\"\"\"Test Sanity Checks For Neutron\"\"\""},{"line_number":19,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1abeadc6_d9dbc17b","line":16,"in_reply_to":"1abeadc6_01194922","updated":"2014-08-25 13:08:18.000000000","message":"i\u0027m not sured.. but will remove it in patch 6","commit_id":"e4f997190b59432bd8dc0802cd1c757a1e64b1f0"},{"author":{"_account_id":6598,"name":"Berezovsky Irena","email":"irenab.dev@gmail.com","username":"irenab"},"change_message_id":"7d263ed123dc0f5c14cf0027199a7306b9517b34","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":"#"},{"line_number":16,"context_line":"# @author: Tri Hoang Vo, Deutsche Telekom"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"\"\"\"Test Sanity Checks For Neutron\"\"\""},{"line_number":19,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1abeadc6_394595b3","line":16,"in_reply_to":"1abeadc6_d9dbc17b","updated":"2014-08-25 13:20:43.000000000","message":"please see https://review.openstack.org/#/c/112329/","commit_id":"e4f997190b59432bd8dc0802cd1c757a1e64b1f0"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"c4b33f2b10023c831558fe069b6c26584475b3b0","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from neutron.tests import base"},{"line_number":24,"context_line":"from neutron.tests import tools"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"class SanityTestCase(base.BaseTestCase):"},{"line_number":28,"context_line":"    def setUp(self):"},{"line_number":29,"context_line":"        super(SanityTestCase, self).setUp()"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_4c29d388","line":26,"updated":"2014-09-16 13:36:59.000000000","message":"I personally think that these unit tests can be removed but I won\u0027t stress the issue. Heavily mocked tests for system interaction are problematic...","commit_id":"33f85234836e90deac6a0c98458e4a9998c37d45"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"86f0fdad13c86d18a84e35e3f20f751dc5296248","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from neutron.tests import base"},{"line_number":24,"context_line":"from neutron.tests import tools"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"class SanityTestCase(base.BaseTestCase):"},{"line_number":28,"context_line":"    def setUp(self):"},{"line_number":29,"context_line":"        super(SanityTestCase, self).setUp()"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_63e20403","line":26,"in_reply_to":"fa98f980_48935f5c","updated":"2014-09-17 12:57:36.000000000","message":"Yeah, I get that.","commit_id":"33f85234836e90deac6a0c98458e4a9998c37d45"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"40b9ca3fc4bdba22ae59717937e58a54f4afcecc","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from neutron.tests import base"},{"line_number":24,"context_line":"from neutron.tests import tools"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"class SanityTestCase(base.BaseTestCase):"},{"line_number":28,"context_line":"    def setUp(self):"},{"line_number":29,"context_line":"        super(SanityTestCase, self).setUp()"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa98f980_48935f5c","line":26,"in_reply_to":"fa98f980_4c29d388","updated":"2014-09-17 12:17:35.000000000","message":"hm yes I also did not see any unittests for sanity. But this satisfies the request from mark mcclain. He comment in patch 4. I try my best but still cannot satisfy all :)","commit_id":"33f85234836e90deac6a0c98458e4a9998c37d45"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"c8d6af5cd63c3738b04fabb144ad0cf02ede4d8c","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from neutron.tests import base"},{"line_number":24,"context_line":"from neutron.tests import tools"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"class SanityTestCase(base.BaseTestCase):"},{"line_number":28,"context_line":"    def setUp(self):"},{"line_number":29,"context_line":"        super(SanityTestCase, self).setUp()"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a8c0949_44b607de","line":26,"in_reply_to":"fa98f980_63e20403","updated":"2014-10-30 11:03:04.000000000","message":"I agree with Assaf on this. ;], functional tests may be enough, and this only adds maintenance to our backs...","commit_id":"33f85234836e90deac6a0c98458e4a9998c37d45"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"18a368c9969aa37fe355357255ffa13afd722019","unresolved":false,"context_lines":[{"line_number":28,"context_line":"    def setUp(self):"},{"line_number":29,"context_line":"        super(SanityTestCase, self).setUp()"},{"line_number":30,"context_line":"        self.root_helper \u003d \u0027sudo\u0027"},{"line_number":31,"context_line":"        ramdom_string_p \u003d mock.patch(\u0027neutron.common.utils.get_random_string\u0027)"},{"line_number":32,"context_line":"        self.random_string \u003d ramdom_string_p.start()"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    @mock.patch.object(linux_utils, \u0027create_process\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_c2e694e7","line":31,"updated":"2014-11-18 00:27:49.000000000","message":"*random","commit_id":"69c105901f3ef1f5962597b1c4d6e4d5f49569b2"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"d7834b03961ea697b23ca477026b18199d4d9e03","unresolved":false,"context_lines":[{"line_number":28,"context_line":"    def setUp(self):"},{"line_number":29,"context_line":"        super(SanityTestCase, self).setUp()"},{"line_number":30,"context_line":"        self.root_helper \u003d \u0027sudo\u0027"},{"line_number":31,"context_line":"        ramdom_string_p \u003d mock.patch(\u0027neutron.common.utils.get_random_string\u0027)"},{"line_number":32,"context_line":"        self.random_string \u003d ramdom_string_p.start()"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    @mock.patch.object(linux_utils, \u0027create_process\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_cc287e35","line":31,"in_reply_to":"5a890539_c2e694e7","updated":"2014-11-18 09:58:22.000000000","message":"Done","commit_id":"69c105901f3ef1f5962597b1c4d6e4d5f49569b2"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"18a368c9969aa37fe355357255ffa13afd722019","unresolved":false,"context_lines":[{"line_number":29,"context_line":"        super(SanityTestCase, self).setUp()"},{"line_number":30,"context_line":"        self.root_helper \u003d \u0027sudo\u0027"},{"line_number":31,"context_line":"        ramdom_string_p \u003d mock.patch(\u0027neutron.common.utils.get_random_string\u0027)"},{"line_number":32,"context_line":"        self.random_string \u003d ramdom_string_p.start()"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    @mock.patch.object(linux_utils, \u0027create_process\u0027)"},{"line_number":35,"context_line":"    @mock.patch.object(ip_lib.IpNetnsCommand, \u0027delete\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_82072c81","line":32,"updated":"2014-11-18 00:27:49.000000000","message":"Where will .stop() be called?","commit_id":"69c105901f3ef1f5962597b1c4d6e4d5f49569b2"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d77a798310cb632946388fe6eceb6ec808e0f6fd","unresolved":false,"context_lines":[{"line_number":29,"context_line":"        super(SanityTestCase, self).setUp()"},{"line_number":30,"context_line":"        self.root_helper \u003d \u0027sudo\u0027"},{"line_number":31,"context_line":"        ramdom_string_p \u003d mock.patch(\u0027neutron.common.utils.get_random_string\u0027)"},{"line_number":32,"context_line":"        self.random_string \u003d ramdom_string_p.start()"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    @mock.patch.object(linux_utils, \u0027create_process\u0027)"},{"line_number":35,"context_line":"    @mock.patch.object(ip_lib.IpNetnsCommand, \u0027delete\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_9c6d6335","line":32,"in_reply_to":"5a890539_2c190222","updated":"2014-12-09 19:04:09.000000000","message":"Okay.  I have never relied on that.","commit_id":"69c105901f3ef1f5962597b1c4d6e4d5f49569b2"},{"author":{"_account_id":10536,"name":"Tri Hoang Vo","email":"t.vo_hoang@telekom.de","username":"tri.hoang.vo"},"change_message_id":"d7834b03961ea697b23ca477026b18199d4d9e03","unresolved":false,"context_lines":[{"line_number":29,"context_line":"        super(SanityTestCase, self).setUp()"},{"line_number":30,"context_line":"        self.root_helper \u003d \u0027sudo\u0027"},{"line_number":31,"context_line":"        ramdom_string_p \u003d mock.patch(\u0027neutron.common.utils.get_random_string\u0027)"},{"line_number":32,"context_line":"        self.random_string \u003d ramdom_string_p.start()"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    @mock.patch.object(linux_utils, \u0027create_process\u0027)"},{"line_number":35,"context_line":"    @mock.patch.object(ip_lib.IpNetnsCommand, \u0027delete\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_2c190222","line":32,"in_reply_to":"5a890539_82072c81","updated":"2014-11-18 09:58:22.000000000","message":"addCleanup(mock.patch.stopall) in super.setUp() does the job","commit_id":"69c105901f3ef1f5962597b1c4d6e4d5f49569b2"}]}