)]}'
{"neutron/agent/l3/dvr_fip_ns.py":[{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"d8642114d38de6b5ba218abcd0a4bf0b4d3e0cfc","unresolved":true,"context_lines":[{"line_number":192,"context_line":"        self.agent_gateway_port \u003d ex_gw_port"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"        cmd \u003d [\u0027sysctl\u0027, \u0027-w\u0027, \u0027net.ipv4.conf.%s.proxy_arp\u003d1\u0027 % interface_name]"},{"line_number":195,"context_line":"        ip_wrapper.netns.execute(cmd, check_exit_code\u003dFalse, privsep_exec\u003dTrue)"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"    def create(self):"},{"line_number":198,"context_line":"        LOG.debug(\"DVR: add fip namespace: %s\", self.name)"}],"source_content_type":"text/x-python","patch_set":8,"id":"e16a11de_f2846503","line":195,"range":{"start_line":195,"start_character":61,"end_line":195,"end_character":78},"updated":"2021-01-13 09:05:00.000000000","message":"This param will be removed once all rootwraps are replaced in code, right? Are there some rootwrap calls that prevent us from switching to prisep in one go (as was originally in this patch) and hence should stay for some time? Just wondering what obstructs a one go approach.\n\n(Sorry if missed that in one of the meetings)","commit_id":"2729b286d53a499c1b8a323dac0c38d33a2b2586"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"077f48baceff9bdb69b0c2e5ab8dea355e815f9d","unresolved":true,"context_lines":[{"line_number":192,"context_line":"        self.agent_gateway_port \u003d ex_gw_port"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"        cmd \u003d [\u0027sysctl\u0027, \u0027-w\u0027, \u0027net.ipv4.conf.%s.proxy_arp\u003d1\u0027 % interface_name]"},{"line_number":195,"context_line":"        ip_wrapper.netns.execute(cmd, check_exit_code\u003dFalse, privsep_exec\u003dTrue)"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"    def create(self):"},{"line_number":198,"context_line":"        LOG.debug(\"DVR: add fip namespace: %s\", self.name)"}],"source_content_type":"text/x-python","patch_set":8,"id":"4089a2d3_1a848263","line":195,"range":{"start_line":195,"start_character":61,"end_line":195,"end_character":78},"in_reply_to":"e16a11de_f2846503","updated":"2021-01-19 14:59:42.000000000","message":"Yes, this param should be dropped once we migrate all \"execute\" calls to privsep.\n\nBut I found that some calls are long term processes. For example, when calling this method from ProcessManager. That\u0027s a problem because privsep calls should be short-lived processes, just a system call and return, instead of a deamon kind process.\n\nBut we\u0027ll be able to address this problem later.\n\nI also had some problems with the CI and I preferred a step by step approach, to have a more detailed git history and ensure that what is migrated to privsep is actually working.\n\n[1]https://github.com/openstack/neutron/blob/2d4b3aa119c4dcee73c8cbb4f964bb092ae0775b/neutron/agent/linux/external_process.py#L88-L89","commit_id":"2729b286d53a499c1b8a323dac0c38d33a2b2586"}],"neutron/agent/l3/namespaces.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"0dff80db38ff2cc5042bf15e3dd5c34a164879bb","unresolved":true,"context_lines":[{"line_number":94,"context_line":"        # these sysctl values."},{"line_number":95,"context_line":"        ip_wrapper \u003d self.ip_wrapper_root.ensure_namespace(self.name)"},{"line_number":96,"context_line":"        cmd \u003d [\u0027sysctl\u0027, \u0027-w\u0027, \u0027net.ipv4.ip_forward\u003d1\u0027]"},{"line_number":97,"context_line":"        ip_wrapper.netns.execute(cmd, privsep_exec\u003dTrue)"},{"line_number":98,"context_line":"        # 1. Reply only if the target IP address is local address configured"},{"line_number":99,"context_line":"        #    on the incoming interface; and"},{"line_number":100,"context_line":"        # 2. Always use the best local address"}],"source_content_type":"text/x-python","patch_set":10,"id":"10e21600_23b80d1c","line":97,"updated":"2021-02-03 08:19:30.000000000","message":"just a note, no action required in that patch: I\u0027m not sure if it\u0027s just my mistake or is it true but in the e.g. L3 agent logs I now can\u0027t find clear log that for example that cmd was run (and when). It\u0027s not for this patch but maybe we will need to somehow improve logging of the privsep calls to have logged what was executed exactly by agents.","commit_id":"1cf45cded9d439155be31fbbc9c41e3b55f798a8"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"cbfa8ec7bfe85fc4f4c1dc416cc829a13f33d044","unresolved":true,"context_lines":[{"line_number":94,"context_line":"        # these sysctl values."},{"line_number":95,"context_line":"        ip_wrapper \u003d self.ip_wrapper_root.ensure_namespace(self.name)"},{"line_number":96,"context_line":"        cmd \u003d [\u0027sysctl\u0027, \u0027-w\u0027, \u0027net.ipv4.ip_forward\u003d1\u0027]"},{"line_number":97,"context_line":"        ip_wrapper.netns.execute(cmd, privsep_exec\u003dTrue)"},{"line_number":98,"context_line":"        # 1. Reply only if the target IP address is local address configured"},{"line_number":99,"context_line":"        #    on the incoming interface; and"},{"line_number":100,"context_line":"        # 2. Always use the best local address"}],"source_content_type":"text/x-python","patch_set":10,"id":"cbdb7993_b0f8e59b","line":97,"in_reply_to":"10e21600_23b80d1c","updated":"2021-02-03 08:54:53.000000000","message":"You are right, the commands are now executed under the privsep context and not logged in the agent logs. This is something we need to improve for sure.","commit_id":"1cf45cded9d439155be31fbbc9c41e3b55f798a8"}],"neutron/agent/linux/external_process.py":[{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"89756dea83a8b36764fcbd8171ffb0c9e6179489","unresolved":true,"context_lines":[{"line_number":98,"context_line":"        else:"},{"line_number":99,"context_line":"            self.disable(\u0027HUP\u0027)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def disable(self, sig\u003d\u00279\u0027, get_stop_command\u003dNone, privsep_exec\u003dFalse):"},{"line_number":102,"context_line":"        pid \u003d self.pid"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        if self.active:"}],"source_content_type":"text/x-python","patch_set":9,"id":"953fe70a_72ac9a44","line":101,"range":{"start_line":101,"start_character":54,"end_line":101,"end_character":72},"updated":"2021-01-25 13:04:46.000000000","message":"Is it called with privsep_exec\u003dTrue somewhere? Can\u0027t find in the patch.","commit_id":"36bfdf8862698b57cd604837603393388f1c0dff"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"cf64f1acdd9b8edb7f2b982686f59106ee21e87d","unresolved":true,"context_lines":[{"line_number":98,"context_line":"        else:"},{"line_number":99,"context_line":"            self.disable(\u0027HUP\u0027)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def disable(self, sig\u003d\u00279\u0027, get_stop_command\u003dNone, privsep_exec\u003dFalse):"},{"line_number":102,"context_line":"        pid \u003d self.pid"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        if self.active:"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a9961b1_34efc906","line":101,"range":{"start_line":101,"start_character":54,"end_line":101,"end_character":72},"updated":"2021-02-05 07:16:34.000000000","message":"Can you please point where disable() is called with privsep_exec \u003d\u003d True?","commit_id":"3466127d08359dbdfbca2c42f4545e0c205e0ee2"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"b767476db941ded750226159df48ceda8a20a722","unresolved":true,"context_lines":[{"line_number":98,"context_line":"        else:"},{"line_number":99,"context_line":"            self.disable(\u0027HUP\u0027)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def disable(self, sig\u003d\u00279\u0027, get_stop_command\u003dNone, privsep_exec\u003dFalse):"},{"line_number":102,"context_line":"        pid \u003d self.pid"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        if self.active:"}],"source_content_type":"text/x-python","patch_set":11,"id":"bb1a5016_8de90dee","line":101,"range":{"start_line":101,"start_character":54,"end_line":101,"end_character":72},"in_reply_to":"5b5b8dd1_568b3cf2","updated":"2021-02-05 12:54:49.000000000","message":"Ok, it just confused me, why not do the change in the patch where it\u0027s actually used?\nWill \"privsep_exec\u003dTrue\" in 2 cases below lead to errors?","commit_id":"3466127d08359dbdfbca2c42f4545e0c205e0ee2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"672abd445d334c2d64fa2d1d4db95aa9c421dd0b","unresolved":true,"context_lines":[{"line_number":98,"context_line":"        else:"},{"line_number":99,"context_line":"            self.disable(\u0027HUP\u0027)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def disable(self, sig\u003d\u00279\u0027, get_stop_command\u003dNone, privsep_exec\u003dFalse):"},{"line_number":102,"context_line":"        pid \u003d self.pid"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        if self.active:"}],"source_content_type":"text/x-python","patch_set":11,"id":"5b5b8dd1_568b3cf2","line":101,"range":{"start_line":101,"start_character":54,"end_line":101,"end_character":72},"in_reply_to":"9a9961b1_34efc906","updated":"2021-02-05 11:52:15.000000000","message":"Still nowhere but will be in future patches.","commit_id":"3466127d08359dbdfbca2c42f4545e0c205e0ee2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"328028c021f61a047d0a71c1e1dcb6960dc26c03","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        else:"},{"line_number":99,"context_line":"            self.disable(\u0027HUP\u0027)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def disable(self, sig\u003d\u00279\u0027, get_stop_command\u003dNone, privsep_exec\u003dFalse):"},{"line_number":102,"context_line":"        pid \u003d self.pid"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        if self.active:"}],"source_content_type":"text/x-python","patch_set":11,"id":"187e3556_e25ff70e","line":101,"range":{"start_line":101,"start_character":54,"end_line":101,"end_character":72},"in_reply_to":"bb1a5016_8de90dee","updated":"2021-02-05 16:39:24.000000000","message":"Done","commit_id":"3466127d08359dbdfbca2c42f4545e0c205e0ee2"}],"neutron/agent/linux/utils.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"28af36a7922bdfea90b5faffc2e9c8e789ded490","unresolved":true,"context_lines":[{"line_number":111,"context_line":"    try:"},{"line_number":112,"context_line":"        returncode, _stdout, _stderr \u003d client.execute(cmd, process_input)"},{"line_number":113,"context_line":"    except Exception:"},{"line_number":114,"context_line":"        LOG.error(\"Rootwrap error running command: %s\", cmd)"},{"line_number":115,"context_line":"        raise"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    _stdout \u003d helpers.safe_decode_utf8(_stdout)"},{"line_number":118,"context_line":"    _stderr \u003d helpers.safe_decode_utf8(_stderr)"},{"line_number":119,"context_line":"    return _stdout, _stderr, returncode"}],"source_content_type":"text/x-python","patch_set":10,"id":"bbd33f25_64fd3069","line":116,"range":{"start_line":114,"start_character":8,"end_line":116,"end_character":0},"updated":"2021-02-04 05:08:00.000000000","message":"Why do we need to change it from excutils.save_and_reraise_exception()? In my understanding, it was introduced as reraise sometimes does not work expectedly when I/O like logging is done inside the except clause.","commit_id":"1cf45cded9d439155be31fbbc9c41e3b55f798a8"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"bc64293e8bf891e35421dd8a6a63564c65d421b6","unresolved":true,"context_lines":[{"line_number":111,"context_line":"    try:"},{"line_number":112,"context_line":"        returncode, _stdout, _stderr \u003d client.execute(cmd, process_input)"},{"line_number":113,"context_line":"    except Exception:"},{"line_number":114,"context_line":"        LOG.error(\"Rootwrap error running command: %s\", cmd)"},{"line_number":115,"context_line":"        raise"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    _stdout \u003d helpers.safe_decode_utf8(_stdout)"},{"line_number":118,"context_line":"    _stderr \u003d helpers.safe_decode_utf8(_stderr)"},{"line_number":119,"context_line":"    return _stdout, _stderr, returncode"}],"source_content_type":"text/x-python","patch_set":10,"id":"c570efb0_42c5a00b","line":116,"range":{"start_line":114,"start_character":8,"end_line":116,"end_character":0},"in_reply_to":"bbd33f25_64fd3069","updated":"2021-02-04 12:48:14.000000000","message":"I didn\u0027t know that. I\u0027ll restore it.","commit_id":"1cf45cded9d439155be31fbbc9c41e3b55f798a8"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"33555dd1e51578b4559b95b95456f4c15baaccf7","unresolved":true,"context_lines":[{"line_number":128,"context_line":"        else:"},{"line_number":129,"context_line":"            _process_input \u003d None"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"        if run_as_root and privsep_exec:"},{"line_number":132,"context_line":"            _stdout, _stderr, returncode \u003d priv_utils.execute_process("},{"line_number":133,"context_line":"                cmd, _process_input, addl_env)"},{"line_number":134,"context_line":"        elif run_as_root and cfg.CONF.AGENT.root_helper_daemon:"}],"source_content_type":"text/x-python","patch_set":10,"id":"63bfdd00_a2110935","line":131,"range":{"start_line":131,"start_character":8,"end_line":131,"end_character":40},"updated":"2021-02-03 14:34:03.000000000","message":"do we need both run_as_root and privsep_exec?\nI would go for a simplified signature and rename run_as_root if that is not fully correct anymore with privsep","commit_id":"1cf45cded9d439155be31fbbc9c41e3b55f798a8"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"cd7691c6d77587d300ed4609e48859112bf89d63","unresolved":false,"context_lines":[{"line_number":128,"context_line":"        else:"},{"line_number":129,"context_line":"            _process_input \u003d None"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"        if run_as_root and privsep_exec:"},{"line_number":132,"context_line":"            _stdout, _stderr, returncode \u003d priv_utils.execute_process("},{"line_number":133,"context_line":"                cmd, _process_input, addl_env)"},{"line_number":134,"context_line":"        elif run_as_root and cfg.CONF.AGENT.root_helper_daemon:"}],"source_content_type":"text/x-python","patch_set":10,"id":"e99145bb_997b1975","line":131,"range":{"start_line":131,"start_character":8,"end_line":131,"end_character":40},"in_reply_to":"63bfdd00_a2110935","updated":"2021-02-03 14:37:09.000000000","message":"I think I found an answer in this comment:\nhttps://review.opendev.org/c/openstack/neutron/+/764015/8/neutron/agent/l3/dvr_fip_ns.py#195","commit_id":"1cf45cded9d439155be31fbbc9c41e3b55f798a8"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"443c600307c508e9e337b21036f4656cfdf3462a","unresolved":true,"context_lines":[{"line_number":82,"context_line":"        # execution is done."},{"line_number":83,"context_line":"        cmd \u003d shlex.split(config.get_root_helper(cfg.CONF)) + cmd"},{"line_number":84,"context_line":"    LOG.debug(\"Running command: %s\", cmd)"},{"line_number":85,"context_line":"    obj \u003d subprocess.Popen(cmd, shell\u003dFalse, stdin\u003dsubprocess.PIPE,"},{"line_number":86,"context_line":"                           stdout\u003dsubprocess.PIPE, stderr\u003dsubprocess.PIPE)"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"    return obj, cmd"}],"source_content_type":"text/x-python","patch_set":12,"id":"8d40f695_a8b2e7f0","line":85,"range":{"start_line":85,"start_character":4,"end_line":85,"end_character":26},"updated":"2021-02-08 13:59:45.000000000","message":"+1","commit_id":"c89c1f53db288143ef95362971d8db86b8f978ff"}],"neutron/privileged/agent/linux/utils.py":[{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"89756dea83a8b36764fcbd8171ffb0c9e6179489","unresolved":true,"context_lines":[{"line_number":69,"context_line":"def _addl_env_args(addl_env):"},{"line_number":70,"context_line":"    \"\"\"Build arguments for adding additional environment vars with env\"\"\""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    # NOTE (twilson) If using rootwrap, an EnvFilter should be set up for the"},{"line_number":73,"context_line":"    # command instead of a CommandFilter."},{"line_number":74,"context_line":"    if addl_env is None:"},{"line_number":75,"context_line":"        return []"},{"line_number":76,"context_line":"    return [\u0027env\u0027] + [\u0027%s\u003d%s\u0027 % pair for pair in addl_env.items()]"}],"source_content_type":"text/x-python","patch_set":9,"id":"391137ae_5e663acf","line":73,"range":{"start_line":72,"start_character":4,"end_line":73,"end_character":41},"updated":"2021-01-25 13:04:46.000000000","message":"Is it needed?","commit_id":"36bfdf8862698b57cd604837603393388f1c0dff"},{"author":{"_account_id":21798,"name":"Bernard Cafarelli","email":"bcafarel@redhat.com","username":"bcafarel"},"change_message_id":"d41bd455bd8a63e3a25bc286dce1fbe36ddd268a","unresolved":true,"context_lines":[{"line_number":69,"context_line":"def _addl_env_args(addl_env):"},{"line_number":70,"context_line":"    \"\"\"Build arguments for adding additional environment vars with env\"\"\""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    # NOTE (twilson) If using rootwrap, an EnvFilter should be set up for the"},{"line_number":73,"context_line":"    # command instead of a CommandFilter."},{"line_number":74,"context_line":"    if addl_env is None:"},{"line_number":75,"context_line":"        return []"},{"line_number":76,"context_line":"    return [\u0027env\u0027] + [\u0027%s\u003d%s\u0027 % pair for pair in addl_env.items()]"}],"source_content_type":"text/x-python","patch_set":9,"id":"9448a998_2416e6ad","line":73,"range":{"start_line":72,"start_character":4,"end_line":73,"end_character":41},"in_reply_to":"391137ae_5e663acf","updated":"2021-01-27 14:32:59.000000000","message":"Probably not for this new wrapper, just that code came straight from old rootwrap helper in neutron/agent/linux/utils.py","commit_id":"36bfdf8862698b57cd604837603393388f1c0dff"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"28af36a7922bdfea90b5faffc2e9c8e789ded490","unresolved":true,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"@privileged.default.entrypoint"},{"line_number":55,"context_line":"def execute_process(cmd, _process_input, addl_env):"},{"line_number":56,"context_line":"    return _execute_process(cmd, _process_input, addl_env)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"def _execute_process(cmd, _process_input, addl_env):"}],"source_content_type":"text/x-python","patch_set":10,"id":"8ceb3cd8_10cd7ac3","line":56,"updated":"2021-02-04 05:08:00.000000000","message":"just question: This function just calls _execute_process with the same arguments. Why do we need this function and cannot we decorate _execute_process directly?","commit_id":"1cf45cded9d439155be31fbbc9c41e3b55f798a8"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"bc64293e8bf891e35421dd8a6a63564c65d421b6","unresolved":true,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"@privileged.default.entrypoint"},{"line_number":55,"context_line":"def execute_process(cmd, _process_input, addl_env):"},{"line_number":56,"context_line":"    return _execute_process(cmd, _process_input, addl_env)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"def _execute_process(cmd, _process_input, addl_env):"}],"source_content_type":"text/x-python","patch_set":10,"id":"d0cf1bc2_848b3480","line":56,"in_reply_to":"8ceb3cd8_10cd7ac3","updated":"2021-02-04 12:48:14.000000000","message":"You are right. I just copy/pasted the agent.linux.utils method and then wrapped it inside this other function.","commit_id":"1cf45cded9d439155be31fbbc9c41e3b55f798a8"}]}