)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"f84ab567b5bb9a6b3f652b0fb7db4bb971147c8a","unresolved":true,"context_lines":[{"line_number":11,"context_line":"be passed to a container at creation time via the container runtime,"},{"line_number":12,"context_line":"rather than requiring dynamic group manipulation inside the container."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This is needed to allow fluentd to read the systemd journal mounted"},{"line_number":15,"context_line":"at /var/log/journal. The journal directory is owned by a GID that is"},{"line_number":16,"context_line":"not statically assigned and varies per distro and image build."},{"line_number":17,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"f8e5a03f_07050d72","line":14,"updated":"2026-05-29 16:46:42.000000000","message":"It looks good generally, but please we can clarify what happens if the GID you try to add is already assigned in the container? This is the case for the systemd journal on Ubuntu. GID 999 gets taken by `input` IIRC.","commit_id":"f743076f5a469e04759e26d444155c269b46a2fc"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"69e89d6fbae6c4685de05d48317466bfbd37b90a","unresolved":true,"context_lines":[{"line_number":11,"context_line":"be passed to a container at creation time via the container runtime,"},{"line_number":12,"context_line":"rather than requiring dynamic group manipulation inside the container."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This is needed to allow fluentd to read the systemd journal mounted"},{"line_number":15,"context_line":"at /var/log/journal. The journal directory is owned by a GID that is"},{"line_number":16,"context_line":"not statically assigned and varies per distro and image build."},{"line_number":17,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"52402ece_9cf12cb0","line":14,"in_reply_to":"8f6d67d7_48d6a2a3","updated":"2026-06-01 11:53:13.000000000","message":"I see it doesn\u0027t seem to complain in CI at least.\n\nThere is a potential issue that you can grant the container user access to some random group that it wouldn\u0027t normally have access to. I.e for fluentd, the fluentd user is going to have access to stuff owned by the `input` group which could potentially create a security issue. FWIW, this is no different to the final version of my original patch.","commit_id":"f743076f5a469e04759e26d444155c269b46a2fc"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"568eb717e70a5fd1440f206a096b3a12f4ddefae","unresolved":true,"context_lines":[{"line_number":11,"context_line":"be passed to a container at creation time via the container runtime,"},{"line_number":12,"context_line":"rather than requiring dynamic group manipulation inside the container."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This is needed to allow fluentd to read the systemd journal mounted"},{"line_number":15,"context_line":"at /var/log/journal. The journal directory is owned by a GID that is"},{"line_number":16,"context_line":"not statically assigned and varies per distro and image build."},{"line_number":17,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"8f6d67d7_48d6a2a3","line":14,"in_reply_to":"f8e5a03f_07050d72","updated":"2026-06-01 08:36:04.000000000","message":"We translate group name to GID and that should be fine?","commit_id":"f743076f5a469e04759e26d444155c269b46a2fc"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"f84ab567b5bb9a6b3f652b0fb7db4bb971147c8a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"fbf89dae_c320962e","updated":"2026-05-29 16:46:42.000000000","message":"It\u0027s unclear to me what happens if this will solve","commit_id":"f743076f5a469e04759e26d444155c269b46a2fc"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"1fb66c014a93c0e45be12064c22ac665fc1c3f9a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9adb5def_ad4fa673","in_reply_to":"fbf89dae_c320962e","updated":"2026-05-29 16:47:05.000000000","message":"the journal issue","commit_id":"f743076f5a469e04759e26d444155c269b46a2fc"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"69e89d6fbae6c4685de05d48317466bfbd37b90a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"3d1d1620_89a1f55f","updated":"2026-06-01 11:53:13.000000000","message":"It looks good overall.","commit_id":"bc1e52437e8edb4bb6892e4bb151a9d9e70c29da"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"807fd8f1da466a1f6c13a033d21b1fdb353b2890","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"34290d1f_61ad8202","updated":"2026-06-01 08:35:23.000000000","message":"recheck podman unrelated issue","commit_id":"bc1e52437e8edb4bb6892e4bb151a9d9e70c29da"}],"ansible/module_utils/kolla_container_worker.py":[{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"69e89d6fbae6c4685de05d48317466bfbd37b90a","unresolved":true,"context_lines":[{"line_number":165,"context_line":"        try:"},{"line_number":166,"context_line":"            current \u003d container_info[\u0027HostConfig\u0027].get(\u0027GroupAdd\u0027) or []"},{"line_number":167,"context_line":"        except (KeyError, TypeError):"},{"line_number":168,"context_line":"            current \u003d []"},{"line_number":169,"context_line":"        return sorted(current), sorted(self._resolved_group_add)"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    def diff_security_opt(self, container_info):"}],"source_content_type":"text/x-python","patch_set":4,"id":"343d3632_6aa864fb","line":168,"updated":"2026-06-01 11:53:13.000000000","message":"nit: the try/except pattern could be factored out of this / diff_cap_add / diff_security_opt","commit_id":"bc1e52437e8edb4bb6892e4bb151a9d9e70c29da"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"69e89d6fbae6c4685de05d48317466bfbd37b90a","unresolved":true,"context_lines":[{"line_number":288,"context_line":"    def compare_group_add(self, container_info):"},{"line_number":289,"context_line":"        try:"},{"line_number":290,"context_line":"            current_group_add \u003d container_info[\u0027HostConfig\u0027].get(\u0027GroupAdd\u0027,"},{"line_number":291,"context_line":"                                                                 None)"},{"line_number":292,"context_line":"        except (KeyError, TypeError):"},{"line_number":293,"context_line":"            current_group_add \u003d None"},{"line_number":294,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"17ca2578_7f3d9f99","line":291,"updated":"2026-06-01 11:53:13.000000000","message":"there\u0027s no need for an exception catch here - you could just do something like:\n```\ncurrent_group_add \u003d container_info.get(\u0027HostConfig\u0027, {}).get(\u0027GroupAdd\u0027, [])\n```\nand then remove line 295/296 as well","commit_id":"bc1e52437e8edb4bb6892e4bb151a9d9e70c29da"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"69e89d6fbae6c4685de05d48317466bfbd37b90a","unresolved":true,"context_lines":[{"line_number":296,"context_line":"            current_group_add \u003d list()"},{"line_number":297,"context_line":"        if set(self._resolved_group_add).symmetric_difference("},{"line_number":298,"context_line":"                set(current_group_add)):"},{"line_number":299,"context_line":"            return True"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def compare_security_opt(self, container_info):"},{"line_number":302,"context_line":"        ipc_mode \u003d self.params.get(\u0027ipc_mode\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"df41f761_5802bd3d","line":299,"updated":"2026-06-01 11:53:13.000000000","message":"return False if there is no difference, not implict None","commit_id":"bc1e52437e8edb4bb6892e4bb151a9d9e70c29da"}],"tests/kolla_container_tests/test_docker_worker.py":[{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"69e89d6fbae6c4685de05d48317466bfbd37b90a","unresolved":true,"context_lines":[{"line_number":1181,"context_line":"    def test_compare_group_add_neg(self):"},{"line_number":1182,"context_line":"        container_info \u003d {\u0027HostConfig\u0027: dict(GroupAdd\u003d[\u00271000\u0027])}"},{"line_number":1183,"context_line":"        self.dw \u003d get_DockerWorker({\u0027group_add\u0027: [\u00271000\u0027]})"},{"line_number":1184,"context_line":"        self.assertFalse(self.dw.compare_group_add(container_info))"},{"line_number":1185,"context_line":""},{"line_number":1186,"context_line":"    def test_compare_group_add_pos(self):"},{"line_number":1187,"context_line":"        container_info \u003d {\u0027HostConfig\u0027: dict(GroupAdd\u003d[\u00271000\u0027])}"}],"source_content_type":"text/x-python","patch_set":4,"id":"a1400e02_11f1205b","line":1184,"updated":"2026-06-01 11:53:13.000000000","message":"nit: assertIs(False, self.dw.compare_group_add(container_info)) would catch the inconsistency of this method returning True or None\n(same with others)","commit_id":"bc1e52437e8edb4bb6892e4bb151a9d9e70c29da"}]}
