)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"3c3a1c027254f348b83802dc6ab9279ee69744c1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"e218f997_1767f47d","updated":"2024-03-01 15:07:56.000000000","message":"please fix all the linter errors first before I take a deeper look.\n\nIt might be beneficial to install flake8 locally directly into your favorite editor or IDE to avoid pushing code to gerrit that doesn\u0027t pass basic linter tests.\n\nThis also helps speed up the development cycle because you don\u0027t need to wait for gerrit to return CI results and helps preserve CI resources in our test infrastructure.\n\nI actually just installed flake8 myself into vs code, because it was somehow gone missing from my setup (I usually run tox -e pep8 manually before committing but this doesn\u0027t seem to catch all pep8 errors locally).\n\nHTH","commit_id":"4e8b9bc8ce98c37e75c7ecbbc8e2bea43e57fb2c"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"4c7d414c38132ecd927c11d0d17866ed59e163cb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"50b52aac_5e59bfe9","updated":"2024-04-17 16:03:50.000000000","message":"I need to look at this some more (just barely reviewed `kolla-docker_worker.py`).\n\nNotice: The build failure should be related to debian currently having no recent enough version of python-docker, afaik.","commit_id":"2bd864c2fe965b23727e3adcac715c188282c549"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"d6a39953337aa654d0256f4f8b0f50d43ef86f96","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"822bdc3d_3a0e92e5","updated":"2024-06-25 07:44:06.000000000","message":"Please address the open comments, thank you.","commit_id":"f3bb93216a75e3c4edba032e682a3c2548041a13"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"721c6f09510bfd190e3fee477fa51ea461698961","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"a85bebc4_881c6f15","updated":"2024-06-26 07:34:28.000000000","message":"thanks for the updated source code comments!","commit_id":"d22c0f57011be85c9d97090b6d293ec9697640b5"},{"author":{"_account_id":36701,"name":"Ivan Halomi","display_name":"Ivan Halomi","email":"ivan.halomi@gmail.com","username":"ivanhalomi"},"change_message_id":"6f1a3a08fcc7198dcc0d7f0324150631b20bde94","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":22,"id":"dcf1210b_4348ed65","updated":"2024-07-02 09:13:33.000000000","message":"recheck a-c-k patch merged","commit_id":"5a5393b16e6e1b1e5342b6b74eed786b1401b138"},{"author":{"_account_id":14200,"name":"Maksim Malchuk","email":"maksim.malchuk@gmail.com","username":"mmalchuk"},"change_message_id":"b28a776d98a40069c57e089d69c66823d8498bc9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"fc12e549_1a3bee10","updated":"2024-08-22 08:17:26.000000000","message":"lgtm, but shouldn\u0027t we add a releasenote about changes?","commit_id":"5804f3f89919b1b124dd96eddda4707cc6b3513a"},{"author":{"_account_id":34113,"name":"Martin Hiner","email":"martin.hiner@tietoevry.com","username":"hinermar"},"change_message_id":"4bd4c5f6651962b297115a57c586e984b492008e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"96da4190_aa6bf710","in_reply_to":"fc12e549_1a3bee10","updated":"2024-08-27 14:12:16.000000000","message":"Done plus updated commit message.","commit_id":"5804f3f89919b1b124dd96eddda4707cc6b3513a"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"50fe1aa99274a0960583e1a1efd6012f27dc8c9d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":30,"id":"a9295bd5_88284fac","updated":"2024-09-04 08:55:11.000000000","message":"still need to look at the changed tests for docker_worker.\nbesides the comments, kolla_docker_worker.py looks fine.","commit_id":"02b93f68250e5fba27b5aca63a666f8b93418efc"},{"author":{"_account_id":34113,"name":"Martin Hiner","email":"martin.hiner@tietoevry.com","username":"hinermar"},"change_message_id":"296f43511ad275df81c63faf51e0febf32307127","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":30,"id":"fa4def99_1c748e38","in_reply_to":"a9295bd5_88284fac","updated":"2024-09-04 08:58:36.000000000","message":"Postpone it a bit. I am looking at them myself and there will possibly be some changes.","commit_id":"02b93f68250e5fba27b5aca63a666f8b93418efc"},{"author":{"_account_id":34113,"name":"Martin Hiner","email":"martin.hiner@tietoevry.com","username":"hinermar"},"change_message_id":"1712a35ef094262dc6eaa4f765ee1da7f895a4a2","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":30,"id":"ebdc6c51_874de4e7","in_reply_to":"fa4def99_1c748e38","updated":"2024-09-18 15:10:50.000000000","message":"Tests are ready for review. There were also some minor changes to kolla_docker_worker.py\n\nSorry for taking so long but due to internal stuff I didn\u0027t have much time for this.","commit_id":"02b93f68250e5fba27b5aca63a666f8b93418efc"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"fee8a0545fb55e61a39444db7d38e54e2ee20f79","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":31,"id":"1f96f228_8a1330ec","updated":"2024-09-25 13:39:09.000000000","message":"What about kolla_toolbox module? do we need to move that as well?","commit_id":"a17d1fb4dcd859954b013623edb898573cf10f9f"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"9dfc530bf0893f580d532f0eeb47e102de8e1ac3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":31,"id":"fe740f3d_9846eeff","updated":"2024-11-01 11:49:52.000000000","message":"recheck test results are no longer available and local testing is not failing","commit_id":"a17d1fb4dcd859954b013623edb898573cf10f9f"},{"author":{"_account_id":34113,"name":"Martin Hiner","email":"martin.hiner@tietoevry.com","username":"hinermar"},"change_message_id":"2a9155ca0fbcd48ee6abe36b0a91f51cbb0b0573","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":31,"id":"89210375_f7699f15","in_reply_to":"1f96f228_8a1330ec","updated":"2024-09-25 14:05:39.000000000","message":"Yes, that would be the right thing to do. But I think it should get it\u0027s own patch.","commit_id":"a17d1fb4dcd859954b013623edb898573cf10f9f"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"d691f9717ec3e100d8b37552e4a6fa11329ccb66","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":31,"id":"301fe6de_e34c3a4e","in_reply_to":"89210375_f7699f15","updated":"2024-11-05 10:07:50.000000000","message":"https://review.opendev.org/c/openstack/kolla-ansible/+/933073","commit_id":"a17d1fb4dcd859954b013623edb898573cf10f9f"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"896e2958533e46ffd172eab7ea882cda37e0a7ea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"167b0b2e_b9804943","updated":"2024-11-11 10:03:33.000000000","message":"There (imo) problems with this patchset. rn I am working on addressing those.","commit_id":"f192e64420e2a73c44a8ac55395e2a803a8736ba"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"6d6d7724f93440e41aa17f7f9fdf4244ec0cd6dc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"f4a6b450_cde732da","updated":"2025-03-25 12:35:18.000000000","message":"Waiting for response from ubuntu team https://bugs.launchpad.net/ubuntu/+source/python-docker/+bug/2098863","commit_id":"f192e64420e2a73c44a8ac55395e2a803a8736ba"},{"author":{"_account_id":34113,"name":"Martin Hiner","email":"martin.hiner@tietoevry.com","username":"hinermar"},"change_message_id":"6a531453ed76210187c7dc96cac201583d7a9aca","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"bc1adf83_7dbef40f","updated":"2025-01-30 14:05:51.000000000","message":"recheck expired test results","commit_id":"f192e64420e2a73c44a8ac55395e2a803a8736ba"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"c04527ea8d568592352f4d7d426b23c372b54111","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"027f2096_d76a2865","updated":"2024-11-08 09:52:12.000000000","message":"recheck temporary failure with logging at rackspace","commit_id":"f192e64420e2a73c44a8ac55395e2a803a8736ba"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"8f2df602a8a9f8e39a94793fd8ce0f88330d76b1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"9596cea3_cca25c31","updated":"2024-11-07 15:14:55.000000000","message":"recheck unrelated failure in kolla-ansible-ubuntu during test-dashboards.sh with + curl --include --location --fail --cacert /etc/kolla/certificates/ca/root.crt https://192.0.2.10 returning 503 server error","commit_id":"f192e64420e2a73c44a8ac55395e2a803a8736ba"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"08bab0f9296410cea78e73352abe54cd46866df8","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":32,"id":"78ff786d_70161bbc","in_reply_to":"167b0b2e_b9804943","updated":"2024-11-25 13:06:43.000000000","message":"can you be specific about the problems, that you are seeing? Thank you.","commit_id":"f192e64420e2a73c44a8ac55395e2a803a8736ba"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"bc388d072aeccda90220cec46e2db457a826a9b3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"ba3b74dc_29faaf6f","in_reply_to":"53cddf5c_a3c55dac","updated":"2025-01-30 15:43:40.000000000","message":"After discussion with Martin Hiner, we have resolved what I though was not correct with the patch set. It was just a disagreement.","commit_id":"f192e64420e2a73c44a8ac55395e2a803a8736ba"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"7bf76eb037dbf6642f66daffd4c123192812ea63","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"53cddf5c_a3c55dac","in_reply_to":"78ff786d_70161bbc","updated":"2024-11-28 09:39:13.000000000","message":"I took over this patch set after Ivan left our company. My personal belief is that by upgrading to higher-level client, we are able to cut down on code duplication as the new clients are very very similar. So far, I was able to merge most of the functionality of kolla_container into one class that can handle both Docker and Podman. I think that this way, the code is cleaner and more maintainable. But since it is a big change, it will take some more time to finish completely.","commit_id":"f192e64420e2a73c44a8ac55395e2a803a8736ba"},{"author":{"_account_id":36702,"name":"Roman Krcek","display_name":"Roman Krček","email":"roman.krcek@tietoevry.com","username":"r-krcek"},"change_message_id":"68de3dedf411033366d46aa5802065501730362c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":32,"id":"019aac99_bcf92bac","in_reply_to":"f4a6b450_cde732da","updated":"2025-04-17 11:51:18.000000000","message":"Replies I got in the Ubuntu IRC:\n\n\"Packages in Ubuntu may not be the latest. Ubuntu aims for stability, so \"latest\" may not be a good idea. Post-release updates are only considered if they are fixes for security vulnerabilities, high impact bug fixes, or unintrusive bug fixes with substantial benefit.\"\n\n\"other than some rolling release distros, you\u0027ll find it\u0027s difficult to convince any distribution to include an updated version of software on an existing stable release, if it\u0027s not associated with patching a vulnerability\"\n\nLooks like the update might not happen just because we would like to. I guess I will focus on making virtual environment mandatory. I believe this way, we can eliminate this problem from occurring again and eliminate future problems of similar nature (and hopefully not generate new ones :)","commit_id":"f192e64420e2a73c44a8ac55395e2a803a8736ba"}],"ansible/module_utils/kolla_docker_worker.py":[{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"4c7d414c38132ecd927c11d0d17866ed59e163cb","unresolved":true,"context_lines":[{"line_number":465,"context_line":"        if not self.params.get(\u0027detach\u0027):"},{"line_number":466,"context_line":"            rc \u003d container.wait()"},{"line_number":467,"context_line":""},{"line_number":468,"context_line":"            stdout \u003d [line.decode() for line in container.logs(stdout\u003dTrue,"},{"line_number":469,"context_line":"                      stderr\u003dFalse, stream\u003dTrue)]"},{"line_number":470,"context_line":"            stderr \u003d [line.decode() for line in container.logs(stdout\u003dFalse,"},{"line_number":471,"context_line":"                      stderr\u003dTrue, stream\u003dTrue)]"}],"source_content_type":"text/x-python","patch_set":14,"id":"f56d25b9_bd0ae6db","line":468,"range":{"start_line":468,"start_character":22,"end_line":468,"end_character":35},"updated":"2024-04-17 16:03:50.000000000","message":"are you sure this is still in raw bytes and we need manually transcode to UTF-8 string? I had hoped we would get a string already from the container object? Not sure though.","commit_id":"2bd864c2fe965b23727e3adcac715c188282c549"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"ea5bf516b1db0e91388e80ceee397db49e08f6be","unresolved":false,"context_lines":[{"line_number":465,"context_line":"        if not self.params.get(\u0027detach\u0027):"},{"line_number":466,"context_line":"            rc \u003d container.wait()"},{"line_number":467,"context_line":""},{"line_number":468,"context_line":"            stdout \u003d [line.decode() for line in container.logs(stdout\u003dTrue,"},{"line_number":469,"context_line":"                      stderr\u003dFalse, stream\u003dTrue)]"},{"line_number":470,"context_line":"            stderr \u003d [line.decode() for line in container.logs(stdout\u003dFalse,"},{"line_number":471,"context_line":"                      stderr\u003dTrue, stream\u003dTrue)]"}],"source_content_type":"text/x-python","patch_set":14,"id":"61f000ba_59054b5d","line":468,"range":{"start_line":468,"start_character":22,"end_line":468,"end_character":35},"in_reply_to":"83148907_4ec12cd7","updated":"2024-06-10 13:25:12.000000000","message":"Acknowledged","commit_id":"2bd864c2fe965b23727e3adcac715c188282c549"},{"author":{"_account_id":36701,"name":"Ivan Halomi","display_name":"Ivan Halomi","email":"ivan.halomi@gmail.com","username":"ivanhalomi"},"change_message_id":"d3304b195c9eaa6d703cf40b05ec97b7ebf40c96","unresolved":true,"context_lines":[{"line_number":465,"context_line":"        if not self.params.get(\u0027detach\u0027):"},{"line_number":466,"context_line":"            rc \u003d container.wait()"},{"line_number":467,"context_line":""},{"line_number":468,"context_line":"            stdout \u003d [line.decode() for line in container.logs(stdout\u003dTrue,"},{"line_number":469,"context_line":"                      stderr\u003dFalse, stream\u003dTrue)]"},{"line_number":470,"context_line":"            stderr \u003d [line.decode() for line in container.logs(stdout\u003dFalse,"},{"line_number":471,"context_line":"                      stderr\u003dTrue, stream\u003dTrue)]"}],"source_content_type":"text/x-python","patch_set":14,"id":"83148907_4ec12cd7","line":468,"range":{"start_line":468,"start_character":22,"end_line":468,"end_character":35},"in_reply_to":"f56d25b9_bd0ae6db","updated":"2024-04-18 10:45:57.000000000","message":"you could, there is that stream flag for it. I chose it because it is same way as podman does it and plan is to merge as many functions as possible into container worker and not separate them for podman and docker.","commit_id":"2bd864c2fe965b23727e3adcac715c188282c549"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"ea5bf516b1db0e91388e80ceee397db49e08f6be","unresolved":true,"context_lines":[{"line_number":46,"context_line":"    \u0027detach\u0027,           # bool"},{"line_number":47,"context_line":"    \u0027entrypoint\u0027,       # string"},{"line_number":48,"context_line":"    \u0027environment\u0027,      # dict docker - environment - dictionary"},{"line_number":49,"context_line":"    \u0027healthcheck\u0027,      # same schema as docker -- healthcheck"},{"line_number":50,"context_line":"    \u0027image\u0027,            # string"},{"line_number":51,"context_line":"    \u0027ipc_mode\u0027,         # string only option is host"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"7f841e16_28e549a4","line":49,"range":{"start_line":49,"start_character":41,"end_line":49,"end_character":62},"updated":"2024-06-10 13:25:12.000000000","message":"is this a typo? not sure what is meant here exactly\n```suggestion\n    \u0027healthcheck\u0027,      # same schema as docker healthcheck\n```","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"},{"author":{"_account_id":36701,"name":"Ivan Halomi","display_name":"Ivan Halomi","email":"ivan.halomi@gmail.com","username":"ivanhalomi"},"change_message_id":"5edbdbbde7beaddc3c6e46c408a447a23120a145","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    \u0027detach\u0027,           # bool"},{"line_number":47,"context_line":"    \u0027entrypoint\u0027,       # string"},{"line_number":48,"context_line":"    \u0027environment\u0027,      # dict docker - environment - dictionary"},{"line_number":49,"context_line":"    \u0027healthcheck\u0027,      # same schema as docker -- healthcheck"},{"line_number":50,"context_line":"    \u0027image\u0027,            # string"},{"line_number":51,"context_line":"    \u0027ipc_mode\u0027,         # string only option is host"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"0eb0501a_0ef8a424","line":49,"range":{"start_line":49,"start_character":41,"end_line":49,"end_character":62},"in_reply_to":"7f841e16_28e549a4","updated":"2024-06-17 12:08:39.000000000","message":"Done","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"ea5bf516b1db0e91388e80ceee397db49e08f6be","unresolved":true,"context_lines":[{"line_number":51,"context_line":"    \u0027ipc_mode\u0027,         # string only option is host"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    \u0027labels\u0027,           # dict"},{"line_number":54,"context_line":"    \u0027netns\u0027,            # dict # TODO(i.halomi) - not sure how it works"},{"line_number":55,"context_line":"    \u0027network_options\u0027,  # string - none,bridge,host,container:id,"},{"line_number":56,"context_line":"                        # missing in docker but needs to be host"},{"line_number":57,"context_line":"    \u0027pid_mode\u0027,         # \"string\"  host, private or \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":18,"id":"1757c27e_162f9a99","line":54,"range":{"start_line":54,"start_character":0,"end_line":54,"end_character":2},"updated":"2024-06-10 13:25:12.000000000","message":"what exactly is missing here?","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"},{"author":{"_account_id":36701,"name":"Ivan Halomi","display_name":"Ivan Halomi","email":"ivan.halomi@gmail.com","username":"ivanhalomi"},"change_message_id":"5edbdbbde7beaddc3c6e46c408a447a23120a145","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    \u0027ipc_mode\u0027,         # string only option is host"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    \u0027labels\u0027,           # dict"},{"line_number":54,"context_line":"    \u0027netns\u0027,            # dict # TODO(i.halomi) - not sure how it works"},{"line_number":55,"context_line":"    \u0027network_options\u0027,  # string - none,bridge,host,container:id,"},{"line_number":56,"context_line":"                        # missing in docker but needs to be host"},{"line_number":57,"context_line":"    \u0027pid_mode\u0027,         # \"string\"  host, private or \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":18,"id":"f603b73a_09061905","line":54,"range":{"start_line":54,"start_character":0,"end_line":54,"end_character":2},"in_reply_to":"1757c27e_162f9a99","updated":"2024-06-17 12:08:39.000000000","message":"old comments, removed","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"d6a39953337aa654d0256f4f8b0f50d43ef86f96","unresolved":true,"context_lines":[{"line_number":62,"context_line":"    \u0027stop_timeout\u0027,     # int"},{"line_number":63,"context_line":"    \u0027tmpfs\u0027,            # dict"},{"line_number":64,"context_line":"    \u0027tty\u0027,              # bool"},{"line_number":65,"context_line":"    # VOLUMES NOT WORKING HAS TO BE DONE WITH MOUNTS"},{"line_number":66,"context_line":"    \u0027volumes\u0027,          # array of dict"},{"line_number":67,"context_line":"    \u0027volumes_from\u0027,     # array of strings"},{"line_number":68,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":18,"id":"7c8cbc8a_47b7db14","line":65,"range":{"start_line":65,"start_character":0,"end_line":65,"end_character":2},"updated":"2024-06-25 07:44:06.000000000","message":"does this work now? how?","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"},{"author":{"_account_id":36701,"name":"Ivan Halomi","display_name":"Ivan Halomi","email":"ivan.halomi@gmail.com","username":"ivanhalomi"},"change_message_id":"0c49ef222e79b9913d6a89bc74fe8aabe187695b","unresolved":true,"context_lines":[{"line_number":62,"context_line":"    \u0027stop_timeout\u0027,     # int"},{"line_number":63,"context_line":"    \u0027tmpfs\u0027,            # dict"},{"line_number":64,"context_line":"    \u0027tty\u0027,              # bool"},{"line_number":65,"context_line":"    # VOLUMES NOT WORKING HAS TO BE DONE WITH MOUNTS"},{"line_number":66,"context_line":"    \u0027volumes\u0027,          # array of dict"},{"line_number":67,"context_line":"    \u0027volumes_from\u0027,     # array of strings"},{"line_number":68,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":18,"id":"9facade0_b05628e8","line":65,"range":{"start_line":65,"start_character":0,"end_line":65,"end_character":2},"in_reply_to":"7c8cbc8a_47b7db14","updated":"2024-06-25 10:51:46.000000000","message":"we are using volumes and mounts at the same time but we have to manually parse them and set them right permissions, I added comment to the parsing method that explains it","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"721c6f09510bfd190e3fee477fa51ea461698961","unresolved":false,"context_lines":[{"line_number":62,"context_line":"    \u0027stop_timeout\u0027,     # int"},{"line_number":63,"context_line":"    \u0027tmpfs\u0027,            # dict"},{"line_number":64,"context_line":"    \u0027tty\u0027,              # bool"},{"line_number":65,"context_line":"    # VOLUMES NOT WORKING HAS TO BE DONE WITH MOUNTS"},{"line_number":66,"context_line":"    \u0027volumes\u0027,          # array of dict"},{"line_number":67,"context_line":"    \u0027volumes_from\u0027,     # array of strings"},{"line_number":68,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":18,"id":"49c6aa87_7e515083","line":65,"range":{"start_line":65,"start_character":0,"end_line":65,"end_character":2},"in_reply_to":"9facade0_b05628e8","updated":"2024-06-26 07:34:28.000000000","message":"Done","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"ea5bf516b1db0e91388e80ceee397db49e08f6be","unresolved":true,"context_lines":[{"line_number":203,"context_line":"            rc, raw_output \u003d container.exec_run(COMPARE_CONFIG_CMD,"},{"line_number":204,"context_line":"                                                user\u003d\u0027root\u0027)"},{"line_number":205,"context_line":"        except docker.errors.APIError:"},{"line_number":206,"context_line":"            return True"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"        # Exit codes:"},{"line_number":209,"context_line":"        # 0: not changed"}],"source_content_type":"text/x-python","patch_set":18,"id":"3356eb7b_6adcc132","line":206,"range":{"start_line":206,"start_character":19,"end_line":206,"end_character":23},"updated":"2024-06-10 13:25:12.000000000","message":"Why do we always return true, even if we get a docker APIError? This suggests to me that we either don\u0027t care about APIErrors - but why catch them then - or we should maybe return something else/handle the error somehow?","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"d6a39953337aa654d0256f4f8b0f50d43ef86f96","unresolved":true,"context_lines":[{"line_number":203,"context_line":"            rc, raw_output \u003d container.exec_run(COMPARE_CONFIG_CMD,"},{"line_number":204,"context_line":"                                                user\u003d\u0027root\u0027)"},{"line_number":205,"context_line":"        except docker.errors.APIError:"},{"line_number":206,"context_line":"            return True"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"        # Exit codes:"},{"line_number":209,"context_line":"        # 0: not changed"}],"source_content_type":"text/x-python","patch_set":18,"id":"44e5dc48_d1b427a3","line":206,"range":{"start_line":206,"start_character":19,"end_line":206,"end_character":23},"in_reply_to":"11bcda1a_033ffa52","updated":"2024-06-25 07:44:06.000000000","message":"yes, at least add the comment also here if this is copy and paste code, the comments need to go with it, because this is really hard to track down using git history only, if code is copied.\n\nThanks","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"},{"author":{"_account_id":36701,"name":"Ivan Halomi","display_name":"Ivan Halomi","email":"ivan.halomi@gmail.com","username":"ivanhalomi"},"change_message_id":"5edbdbbde7beaddc3c6e46c408a447a23120a145","unresolved":true,"context_lines":[{"line_number":203,"context_line":"            rc, raw_output \u003d container.exec_run(COMPARE_CONFIG_CMD,"},{"line_number":204,"context_line":"                                                user\u003d\u0027root\u0027)"},{"line_number":205,"context_line":"        except docker.errors.APIError:"},{"line_number":206,"context_line":"            return True"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"        # Exit codes:"},{"line_number":209,"context_line":"        # 0: not changed"}],"source_content_type":"text/x-python","patch_set":18,"id":"11bcda1a_033ffa52","line":206,"range":{"start_line":206,"start_character":19,"end_line":206,"end_character":23},"in_reply_to":"3356eb7b_6adcc132","updated":"2024-06-17 12:08:39.000000000","message":"there was note in the old code explaining it maybe I should add it also here. But basically you\u0027re correct we don\u0027t care about API errors, this means that container either doesn\u0027t exists or there was error during execution of command and the both cases we expect that the config is wrong and container should be recreated.","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"},{"author":{"_account_id":36701,"name":"Ivan Halomi","display_name":"Ivan Halomi","email":"ivan.halomi@gmail.com","username":"ivanhalomi"},"change_message_id":"0c49ef222e79b9913d6a89bc74fe8aabe187695b","unresolved":true,"context_lines":[{"line_number":203,"context_line":"            rc, raw_output \u003d container.exec_run(COMPARE_CONFIG_CMD,"},{"line_number":204,"context_line":"                                                user\u003d\u0027root\u0027)"},{"line_number":205,"context_line":"        except docker.errors.APIError:"},{"line_number":206,"context_line":"            return True"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"        # Exit codes:"},{"line_number":209,"context_line":"        # 0: not changed"}],"source_content_type":"text/x-python","patch_set":18,"id":"6a493971_caf00c6c","line":206,"range":{"start_line":206,"start_character":19,"end_line":206,"end_character":23},"in_reply_to":"44e5dc48_d1b427a3","updated":"2024-06-25 10:51:46.000000000","message":"code wasn\u0027t copied, it just uses same logic as code before for suppressing errors, added the comment to explain it. Added raise error in case it\u0027s not client error but I don\u0027t think this situation can occur.","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"721c6f09510bfd190e3fee477fa51ea461698961","unresolved":false,"context_lines":[{"line_number":203,"context_line":"            rc, raw_output \u003d container.exec_run(COMPARE_CONFIG_CMD,"},{"line_number":204,"context_line":"                                                user\u003d\u0027root\u0027)"},{"line_number":205,"context_line":"        except docker.errors.APIError:"},{"line_number":206,"context_line":"            return True"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"        # Exit codes:"},{"line_number":209,"context_line":"        # 0: not changed"}],"source_content_type":"text/x-python","patch_set":18,"id":"a2c3f17e_125f00f9","line":206,"range":{"start_line":206,"start_character":19,"end_line":206,"end_character":23},"in_reply_to":"6a493971_caf00c6c","updated":"2024-06-26 07:34:28.000000000","message":"Done","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"ea5bf516b1db0e91388e80ceee397db49e08f6be","unresolved":true,"context_lines":[{"line_number":393,"context_line":"        if volumes:"},{"line_number":394,"context_line":"            self.parse_volumes(volumes, mounts, filtered_volumes)"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"        tmpfs \u003d self.params.get(\u0027tmpfs\u0027)"},{"line_number":397,"context_line":"        if tmpfs:"},{"line_number":398,"context_line":"            tmpfs \u003d [t for t in tmpfs if t]"},{"line_number":399,"context_line":"            if tmpfs:"},{"line_number":400,"context_line":"                args[\u0027tmpfs\u0027] \u003d tmpfs"},{"line_number":401,"context_line":"            else:"},{"line_number":402,"context_line":"                args.pop(\u0027tmpfs\u0027, None)"},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"        args[\u0027mounts\u0027] \u003d mounts"},{"line_number":405,"context_line":"        args[\u0027volumes\u0027] \u003d filtered_volumes"}],"source_content_type":"text/x-python","patch_set":18,"id":"8b7b19ee_da157b2b","line":402,"range":{"start_line":396,"start_character":7,"end_line":402,"end_character":39},"updated":"2024-06-10 13:25:12.000000000","message":"could you explain the reasoning here a little bit? it seems rather complicated.","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"},{"author":{"_account_id":36701,"name":"Ivan Halomi","display_name":"Ivan Halomi","email":"ivan.halomi@gmail.com","username":"ivanhalomi"},"change_message_id":"5edbdbbde7beaddc3c6e46c408a447a23120a145","unresolved":true,"context_lines":[{"line_number":393,"context_line":"        if volumes:"},{"line_number":394,"context_line":"            self.parse_volumes(volumes, mounts, filtered_volumes)"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"        tmpfs \u003d self.params.get(\u0027tmpfs\u0027)"},{"line_number":397,"context_line":"        if tmpfs:"},{"line_number":398,"context_line":"            tmpfs \u003d [t for t in tmpfs if t]"},{"line_number":399,"context_line":"            if tmpfs:"},{"line_number":400,"context_line":"                args[\u0027tmpfs\u0027] \u003d tmpfs"},{"line_number":401,"context_line":"            else:"},{"line_number":402,"context_line":"                args.pop(\u0027tmpfs\u0027, None)"},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"        args[\u0027mounts\u0027] \u003d mounts"},{"line_number":405,"context_line":"        args[\u0027volumes\u0027] \u003d filtered_volumes"}],"source_content_type":"text/x-python","patch_set":18,"id":"91fefe0b_9921a8b3","line":402,"range":{"start_line":396,"start_character":7,"end_line":402,"end_character":39},"in_reply_to":"8b7b19ee_da157b2b","updated":"2024-06-17 12:08:39.000000000","message":"the problem is that we send empty string as a value ( https://github.com/openstack/kolla-ansible/blob/master/ansible/roles/cinder/defaults/main.yml#L205) so array looks like [ \"\"] which is True in python. So we need to remove empty strings from array and then check if array contains something afterwards","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"d6a39953337aa654d0256f4f8b0f50d43ef86f96","unresolved":true,"context_lines":[{"line_number":393,"context_line":"        if volumes:"},{"line_number":394,"context_line":"            self.parse_volumes(volumes, mounts, filtered_volumes)"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"        tmpfs \u003d self.params.get(\u0027tmpfs\u0027)"},{"line_number":397,"context_line":"        if tmpfs:"},{"line_number":398,"context_line":"            tmpfs \u003d [t for t in tmpfs if t]"},{"line_number":399,"context_line":"            if tmpfs:"},{"line_number":400,"context_line":"                args[\u0027tmpfs\u0027] \u003d tmpfs"},{"line_number":401,"context_line":"            else:"},{"line_number":402,"context_line":"                args.pop(\u0027tmpfs\u0027, None)"},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"        args[\u0027mounts\u0027] \u003d mounts"},{"line_number":405,"context_line":"        args[\u0027volumes\u0027] \u003d filtered_volumes"}],"source_content_type":"text/x-python","patch_set":18,"id":"baba5e8a_2b601d64","line":402,"range":{"start_line":396,"start_character":7,"end_line":402,"end_character":39},"in_reply_to":"91fefe0b_9921a8b3","updated":"2024-06-25 07:44:06.000000000","message":"Okay, I can at least understand that. I think that would be a good comment directly in the code, please add something like this (could be worded better maybe):\n\n\n```suggestion\n        # cinder by default sets this to an empty string inside an array\n        # which results in it always returning True in python.\n        # So we need to remove empty string from the array\n        # and check afterwards if it contains something.\n        tmpfs \u003d self.params.get(\u0027tmpfs\u0027)\n        if tmpfs:\n            tmpfs \u003d [t for t in tmpfs if t]\n            if tmpfs:\n                args[\u0027tmpfs\u0027] \u003d tmpfs\n            else:\n                args.pop(\u0027tmpfs\u0027, None)\n```","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"},{"author":{"_account_id":36701,"name":"Ivan Halomi","display_name":"Ivan Halomi","email":"ivan.halomi@gmail.com","username":"ivanhalomi"},"change_message_id":"0c49ef222e79b9913d6a89bc74fe8aabe187695b","unresolved":false,"context_lines":[{"line_number":393,"context_line":"        if volumes:"},{"line_number":394,"context_line":"            self.parse_volumes(volumes, mounts, filtered_volumes)"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"        tmpfs \u003d self.params.get(\u0027tmpfs\u0027)"},{"line_number":397,"context_line":"        if tmpfs:"},{"line_number":398,"context_line":"            tmpfs \u003d [t for t in tmpfs if t]"},{"line_number":399,"context_line":"            if tmpfs:"},{"line_number":400,"context_line":"                args[\u0027tmpfs\u0027] \u003d tmpfs"},{"line_number":401,"context_line":"            else:"},{"line_number":402,"context_line":"                args.pop(\u0027tmpfs\u0027, None)"},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"        args[\u0027mounts\u0027] \u003d mounts"},{"line_number":405,"context_line":"        args[\u0027volumes\u0027] \u003d filtered_volumes"}],"source_content_type":"text/x-python","patch_set":18,"id":"38e00313_4065cd07","line":402,"range":{"start_line":396,"start_character":7,"end_line":402,"end_character":39},"in_reply_to":"baba5e8a_2b601d64","updated":"2024-06-25 10:51:46.000000000","message":"Done","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"c8a092954bbbd325bf29742fb966b382b7b9b5a2","unresolved":true,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"uri \u003d \u0027http+unix:/var/run/docker.sock\u0027"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"CONTAINER_PARAMS \u003d ["},{"line_number":26,"context_line":"    \u0027name\u0027,             # string"},{"line_number":27,"context_line":"    \u0027cap_add\u0027,          # list"},{"line_number":28,"context_line":"    \u0027cgroupns\u0027,         # \u0027str\u0027,choices\u003d[\u0027private\u0027, \u0027host\u0027]"}],"source_content_type":"text/x-python","patch_set":22,"id":"c440fe13_402f0663","line":25,"updated":"2024-07-24 13:13:02.000000000","message":"Do we need to maintain it? Can\u0027t we get it from some function?","commit_id":"5a5393b16e6e1b1e5342b6b74eed786b1401b138"},{"author":{"_account_id":36701,"name":"Ivan Halomi","display_name":"Ivan Halomi","email":"ivan.halomi@gmail.com","username":"ivanhalomi"},"change_message_id":"194b3e3f5afc1a06d300cfbaed3fd3a9ee9750e7","unresolved":true,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"uri \u003d \u0027http+unix:/var/run/docker.sock\u0027"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"CONTAINER_PARAMS \u003d ["},{"line_number":26,"context_line":"    \u0027name\u0027,             # string"},{"line_number":27,"context_line":"    \u0027cap_add\u0027,          # list"},{"line_number":28,"context_line":"    \u0027cgroupns\u0027,         # \u0027str\u0027,choices\u003d[\u0027private\u0027, \u0027host\u0027]"}],"source_content_type":"text/x-python","patch_set":22,"id":"ccdbee35_5bfe8f8b","line":25,"in_reply_to":"c440fe13_402f0663","updated":"2024-07-29 07:05:39.000000000","message":"i tried to get it from their type defined here https://github.com/docker/docker-py/blob/main/docker/types/containers.py#L688 but its missing some parameters and there is inconsistency in their code anyways since they are using list of params defined in other part of code (https://github.com/docker/docker-py/blob/main/docker/models/containers.py#L1032) instead of this class so I don\u0027t think its possible without some changes to the list we get from function. So for clarity I would say this is the best solution.","commit_id":"5a5393b16e6e1b1e5342b6b74eed786b1401b138"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"51afefd2c68c4ffbcdc69865b764f90b81544729","unresolved":false,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"uri \u003d \u0027http+unix:/var/run/docker.sock\u0027"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"CONTAINER_PARAMS \u003d ["},{"line_number":26,"context_line":"    \u0027name\u0027,             # string"},{"line_number":27,"context_line":"    \u0027cap_add\u0027,          # list"},{"line_number":28,"context_line":"    \u0027cgroupns\u0027,         # \u0027str\u0027,choices\u003d[\u0027private\u0027, \u0027host\u0027]"}],"source_content_type":"text/x-python","patch_set":22,"id":"26641554_10f58056","line":25,"in_reply_to":"ccdbee35_5bfe8f8b","updated":"2024-08-22 06:56:47.000000000","message":"Acknowledged","commit_id":"5a5393b16e6e1b1e5342b6b74eed786b1401b138"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"82bd9428f284c0fb7d5fb6c954f011c9089cf0b7","unresolved":true,"context_lines":[{"line_number":21,"context_line":"uri \u003d \u0027http+unix:/var/run/docker.sock\u0027"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"CONTAINER_PARAMS \u003d ["},{"line_number":25,"context_line":"    \u0027name\u0027,             # string"},{"line_number":26,"context_line":"    \u0027cap_add\u0027,          # list"},{"line_number":27,"context_line":"    \u0027cgroupns\u0027,         # \u0027str\u0027,choices\u003d[\u0027private\u0027, \u0027host\u0027]"},{"line_number":28,"context_line":"    \u0027command\u0027,          # string"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    # host config"},{"line_number":31,"context_line":"    \u0027cpu_period\u0027,       # int"},{"line_number":32,"context_line":"    \u0027cpu_quota\u0027,        # int"},{"line_number":33,"context_line":"    \u0027cpuset_cpus\u0027,      # str"},{"line_number":34,"context_line":"    \u0027cpu_shares\u0027        # int"},{"line_number":35,"context_line":"    \u0027cpuset_mems\u0027,      # str"},{"line_number":36,"context_line":"    \u0027kernel_memory\u0027,    # int or string"},{"line_number":37,"context_line":"    \u0027mem_limit\u0027,        # (Union[int, str])"},{"line_number":38,"context_line":"    \u0027mem_reservation\u0027,  # (Union[int, str]): Memory soft limit."},{"line_number":39,"context_line":"    \u0027memswap_limit\u0027     # (Union[int, str]): Maximum amount of memory"},{"line_number":40,"context_line":"                        # + swap a container is allowed to consume."},{"line_number":41,"context_line":"    \u0027ulimits\u0027,          # List[Ulimit]"},{"line_number":42,"context_line":"    \u0027blkio_weight\u0027,     # int between 10 and 1000"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    \u0027detach\u0027,           # bool"},{"line_number":46,"context_line":"    \u0027entrypoint\u0027,       # string"},{"line_number":47,"context_line":"    \u0027environment\u0027,      # dict or array"},{"line_number":48,"context_line":"    \u0027healthcheck\u0027,      # schema as docker healthcheck"},{"line_number":49,"context_line":"    \u0027image\u0027,            # string"},{"line_number":50,"context_line":"    \u0027ipc_mode\u0027,         # string only option is host"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    \u0027labels\u0027,           # dict"},{"line_number":53,"context_line":"    \u0027netns\u0027,            # dict"},{"line_number":54,"context_line":"    \u0027network_options\u0027,  # string , choices\u003d[None, \u0027bridge\u0027, \u0027host\u0027]"},{"line_number":55,"context_line":"    \u0027pid_mode\u0027,         # string, choices\u003d[\u0027host\u0027, \u0027private\u0027,\u0027\u0027]"},{"line_number":56,"context_line":"    \u0027privileged\u0027,       # bool"},{"line_number":57,"context_line":"    \u0027restart_policy\u0027,   # set to none, handled by systemd"},{"line_number":58,"context_line":"    \u0027remove\u0027,           # bool"},{"line_number":59,"context_line":"    \u0027restart_tries\u0027,    # int doesn\u0027t matter done by systemd"},{"line_number":60,"context_line":"    \u0027stop_timeout\u0027,     # int"},{"line_number":61,"context_line":"    \u0027tmpfs\u0027,            # dict"},{"line_number":62,"context_line":"    \u0027tty\u0027,              # bool"},{"line_number":63,"context_line":"    # volumes need to be parsed, see parse_volumes() for more info"},{"line_number":64,"context_line":"    \u0027volumes\u0027,          # array of dict"},{"line_number":65,"context_line":"    \u0027volumes_from\u0027,     # array of strings"},{"line_number":66,"context_line":"]"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"class DockerWorker(ContainerWorker):"},{"line_number":70,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"75194582_d3d8dcf0","line":67,"range":{"start_line":24,"start_character":0,"end_line":67,"end_character":1},"updated":"2024-08-30 15:35:22.000000000","message":"shouldn\u0027t we follow the current logic here and define common values like name etc in `kolla_container_worker.py` instead and import those from there?\n\nWe avoid code duplication this way and it\u0027s also easier to track when debugging if there is an error in docker, podman or if it\u0027s common to both engines, don\u0027t you think?\n\nThe downside is of course if one engine changes a common value in an incompatible way in the future, but I would deem the risk of that rather low?","commit_id":"bd429fbcc2bfd757e9c619eb4d67d9a7596e5273"},{"author":{"_account_id":34113,"name":"Martin Hiner","email":"martin.hiner@tietoevry.com","username":"hinermar"},"change_message_id":"6463321f8cd9b3a317cb8ec12e57555a92d26dc3","unresolved":true,"context_lines":[{"line_number":21,"context_line":"uri \u003d \u0027http+unix:/var/run/docker.sock\u0027"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"CONTAINER_PARAMS \u003d ["},{"line_number":25,"context_line":"    \u0027name\u0027,             # string"},{"line_number":26,"context_line":"    \u0027cap_add\u0027,          # list"},{"line_number":27,"context_line":"    \u0027cgroupns\u0027,         # \u0027str\u0027,choices\u003d[\u0027private\u0027, \u0027host\u0027]"},{"line_number":28,"context_line":"    \u0027command\u0027,          # string"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    # host config"},{"line_number":31,"context_line":"    \u0027cpu_period\u0027,       # int"},{"line_number":32,"context_line":"    \u0027cpu_quota\u0027,        # int"},{"line_number":33,"context_line":"    \u0027cpuset_cpus\u0027,      # str"},{"line_number":34,"context_line":"    \u0027cpu_shares\u0027        # int"},{"line_number":35,"context_line":"    \u0027cpuset_mems\u0027,      # str"},{"line_number":36,"context_line":"    \u0027kernel_memory\u0027,    # int or string"},{"line_number":37,"context_line":"    \u0027mem_limit\u0027,        # (Union[int, str])"},{"line_number":38,"context_line":"    \u0027mem_reservation\u0027,  # (Union[int, str]): Memory soft limit."},{"line_number":39,"context_line":"    \u0027memswap_limit\u0027     # (Union[int, str]): Maximum amount of memory"},{"line_number":40,"context_line":"                        # + swap a container is allowed to consume."},{"line_number":41,"context_line":"    \u0027ulimits\u0027,          # List[Ulimit]"},{"line_number":42,"context_line":"    \u0027blkio_weight\u0027,     # int between 10 and 1000"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    \u0027detach\u0027,           # bool"},{"line_number":46,"context_line":"    \u0027entrypoint\u0027,       # string"},{"line_number":47,"context_line":"    \u0027environment\u0027,      # dict or array"},{"line_number":48,"context_line":"    \u0027healthcheck\u0027,      # schema as docker healthcheck"},{"line_number":49,"context_line":"    \u0027image\u0027,            # string"},{"line_number":50,"context_line":"    \u0027ipc_mode\u0027,         # string only option is host"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    \u0027labels\u0027,           # dict"},{"line_number":53,"context_line":"    \u0027netns\u0027,            # dict"},{"line_number":54,"context_line":"    \u0027network_options\u0027,  # string , choices\u003d[None, \u0027bridge\u0027, \u0027host\u0027]"},{"line_number":55,"context_line":"    \u0027pid_mode\u0027,         # string, choices\u003d[\u0027host\u0027, \u0027private\u0027,\u0027\u0027]"},{"line_number":56,"context_line":"    \u0027privileged\u0027,       # bool"},{"line_number":57,"context_line":"    \u0027restart_policy\u0027,   # set to none, handled by systemd"},{"line_number":58,"context_line":"    \u0027remove\u0027,           # bool"},{"line_number":59,"context_line":"    \u0027restart_tries\u0027,    # int doesn\u0027t matter done by systemd"},{"line_number":60,"context_line":"    \u0027stop_timeout\u0027,     # int"},{"line_number":61,"context_line":"    \u0027tmpfs\u0027,            # dict"},{"line_number":62,"context_line":"    \u0027tty\u0027,              # bool"},{"line_number":63,"context_line":"    # volumes need to be parsed, see parse_volumes() for more info"},{"line_number":64,"context_line":"    \u0027volumes\u0027,          # array of dict"},{"line_number":65,"context_line":"    \u0027volumes_from\u0027,     # array of strings"},{"line_number":66,"context_line":"]"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"class DockerWorker(ContainerWorker):"},{"line_number":70,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"e3ea38b1_89d6dfbc","line":67,"range":{"start_line":24,"start_character":0,"end_line":67,"end_character":1},"in_reply_to":"75194582_d3d8dcf0","updated":"2024-09-02 15:38:39.000000000","message":"Good idea and done. I also changed the order of actions in `prepare_container_args` to be more similar to Podman\u0027s so it will be easier to compare them in the future.\n\nBy the way, the list is same for both engines because it only contains params that we are able to send from our Ansible module (as per our argument_spec) so the more specialized and differing params are left out.\nAny tweaks to params such as renaming `cgroupns_mode` are done in each engines `prepare_container_args` method.","commit_id":"bd429fbcc2bfd757e9c619eb4d67d9a7596e5273"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"ba0e71aec4a758648ade5df3534cec573526e618","unresolved":false,"context_lines":[{"line_number":21,"context_line":"uri \u003d \u0027http+unix:/var/run/docker.sock\u0027"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"CONTAINER_PARAMS \u003d ["},{"line_number":25,"context_line":"    \u0027name\u0027,             # string"},{"line_number":26,"context_line":"    \u0027cap_add\u0027,          # list"},{"line_number":27,"context_line":"    \u0027cgroupns\u0027,         # \u0027str\u0027,choices\u003d[\u0027private\u0027, \u0027host\u0027]"},{"line_number":28,"context_line":"    \u0027command\u0027,          # string"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    # host config"},{"line_number":31,"context_line":"    \u0027cpu_period\u0027,       # int"},{"line_number":32,"context_line":"    \u0027cpu_quota\u0027,        # int"},{"line_number":33,"context_line":"    \u0027cpuset_cpus\u0027,      # str"},{"line_number":34,"context_line":"    \u0027cpu_shares\u0027        # int"},{"line_number":35,"context_line":"    \u0027cpuset_mems\u0027,      # str"},{"line_number":36,"context_line":"    \u0027kernel_memory\u0027,    # int or string"},{"line_number":37,"context_line":"    \u0027mem_limit\u0027,        # (Union[int, str])"},{"line_number":38,"context_line":"    \u0027mem_reservation\u0027,  # (Union[int, str]): Memory soft limit."},{"line_number":39,"context_line":"    \u0027memswap_limit\u0027     # (Union[int, str]): Maximum amount of memory"},{"line_number":40,"context_line":"                        # + swap a container is allowed to consume."},{"line_number":41,"context_line":"    \u0027ulimits\u0027,          # List[Ulimit]"},{"line_number":42,"context_line":"    \u0027blkio_weight\u0027,     # int between 10 and 1000"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    \u0027detach\u0027,           # bool"},{"line_number":46,"context_line":"    \u0027entrypoint\u0027,       # string"},{"line_number":47,"context_line":"    \u0027environment\u0027,      # dict or array"},{"line_number":48,"context_line":"    \u0027healthcheck\u0027,      # schema as docker healthcheck"},{"line_number":49,"context_line":"    \u0027image\u0027,            # string"},{"line_number":50,"context_line":"    \u0027ipc_mode\u0027,         # string only option is host"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    \u0027labels\u0027,           # dict"},{"line_number":53,"context_line":"    \u0027netns\u0027,            # dict"},{"line_number":54,"context_line":"    \u0027network_options\u0027,  # string , choices\u003d[None, \u0027bridge\u0027, \u0027host\u0027]"},{"line_number":55,"context_line":"    \u0027pid_mode\u0027,         # string, choices\u003d[\u0027host\u0027, \u0027private\u0027,\u0027\u0027]"},{"line_number":56,"context_line":"    \u0027privileged\u0027,       # bool"},{"line_number":57,"context_line":"    \u0027restart_policy\u0027,   # set to none, handled by systemd"},{"line_number":58,"context_line":"    \u0027remove\u0027,           # bool"},{"line_number":59,"context_line":"    \u0027restart_tries\u0027,    # int doesn\u0027t matter done by systemd"},{"line_number":60,"context_line":"    \u0027stop_timeout\u0027,     # int"},{"line_number":61,"context_line":"    \u0027tmpfs\u0027,            # dict"},{"line_number":62,"context_line":"    \u0027tty\u0027,              # bool"},{"line_number":63,"context_line":"    # volumes need to be parsed, see parse_volumes() for more info"},{"line_number":64,"context_line":"    \u0027volumes\u0027,          # array of dict"},{"line_number":65,"context_line":"    \u0027volumes_from\u0027,     # array of strings"},{"line_number":66,"context_line":"]"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"class DockerWorker(ContainerWorker):"},{"line_number":70,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"f671c827_49e4607b","line":67,"range":{"start_line":24,"start_character":0,"end_line":67,"end_character":1},"in_reply_to":"e3ea38b1_89d6dfbc","updated":"2024-09-03 15:40:53.000000000","message":"Done","commit_id":"bd429fbcc2bfd757e9c619eb4d67d9a7596e5273"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"82bd9428f284c0fb7d5fb6c954f011c9089cf0b7","unresolved":true,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"            rc, raw_output \u003d container.exec_run(COMPARE_CONFIG_CMD,"},{"line_number":196,"context_line":"                                                user\u003d\u0027root\u0027)"},{"line_number":197,"context_line":"        # APIError means either container doesn\u0027t exist or exec command"},{"line_number":198,"context_line":"        # failed, which means that container is in bad state and we can"},{"line_number":199,"context_line":"        # expect that config is stale so we return True and recreate container"},{"line_number":200,"context_line":"        except docker.errors.APIError as e:"},{"line_number":201,"context_line":"            if e.is_client_error():"},{"line_number":202,"context_line":"                return True"},{"line_number":203,"context_line":"            else:"},{"line_number":204,"context_line":"                raise"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"8fa624e5_71ba8aec","line":202,"range":{"start_line":197,"start_character":0,"end_line":202,"end_character":27},"updated":"2024-08-30 15:35:22.000000000","message":"note that this is afaik a slightly different behaviour then before:\n\npreviously, if we get an error, we first try `container.reload()` and only if that fails we return True.\n\nI\u0027m not sure if this matters in practice, but I found it worth it to mention it for completeness.","commit_id":"bd429fbcc2bfd757e9c619eb4d67d9a7596e5273"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"ba0e71aec4a758648ade5df3534cec573526e618","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"            rc, raw_output \u003d container.exec_run(COMPARE_CONFIG_CMD,"},{"line_number":196,"context_line":"                                                user\u003d\u0027root\u0027)"},{"line_number":197,"context_line":"        # APIError means either container doesn\u0027t exist or exec command"},{"line_number":198,"context_line":"        # failed, which means that container is in bad state and we can"},{"line_number":199,"context_line":"        # expect that config is stale so we return True and recreate container"},{"line_number":200,"context_line":"        except docker.errors.APIError as e:"},{"line_number":201,"context_line":"            if e.is_client_error():"},{"line_number":202,"context_line":"                return True"},{"line_number":203,"context_line":"            else:"},{"line_number":204,"context_line":"                raise"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"c87197b1_f7b6aecd","line":202,"range":{"start_line":197,"start_character":0,"end_line":202,"end_character":27},"in_reply_to":"4583f0ae_97c04c7a","updated":"2024-09-03 15:40:53.000000000","message":"Acknowledged","commit_id":"bd429fbcc2bfd757e9c619eb4d67d9a7596e5273"},{"author":{"_account_id":34113,"name":"Martin Hiner","email":"martin.hiner@tietoevry.com","username":"hinermar"},"change_message_id":"6463321f8cd9b3a317cb8ec12e57555a92d26dc3","unresolved":true,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"            rc, raw_output \u003d container.exec_run(COMPARE_CONFIG_CMD,"},{"line_number":196,"context_line":"                                                user\u003d\u0027root\u0027)"},{"line_number":197,"context_line":"        # APIError means either container doesn\u0027t exist or exec command"},{"line_number":198,"context_line":"        # failed, which means that container is in bad state and we can"},{"line_number":199,"context_line":"        # expect that config is stale so we return True and recreate container"},{"line_number":200,"context_line":"        except docker.errors.APIError as e:"},{"line_number":201,"context_line":"            if e.is_client_error():"},{"line_number":202,"context_line":"                return True"},{"line_number":203,"context_line":"            else:"},{"line_number":204,"context_line":"                raise"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"4583f0ae_97c04c7a","line":202,"range":{"start_line":197,"start_character":0,"end_line":202,"end_character":27},"in_reply_to":"8fa624e5_71ba8aec","updated":"2024-09-02 15:38:39.000000000","message":"Doesn\u0027t seem to me it was that way. I checked current Docker code and we just return True after encountering the exception. Podman does it the same way.\n\nAlso, `container.reload()` would just loads latest data to `container.attrs` dictionary. I don\u0027t see how it would help us here.","commit_id":"bd429fbcc2bfd757e9c619eb4d67d9a7596e5273"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"50fe1aa99274a0960583e1a1efd6012f27dc8c9d","unresolved":true,"context_lines":[{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    def remove_container(self):"},{"line_number":213,"context_line":"        self.changed |\u003d self.systemd.remove_unit_file()"},{"line_number":214,"context_line":"        container \u003d self.check_container()"},{"line_number":215,"context_line":"        if container:"},{"line_number":216,"context_line":"            try:"},{"line_number":217,"context_line":"                container.remove(force\u003dTrue)"},{"line_number":218,"context_line":"            except docker.errors.APIError:"},{"line_number":219,"context_line":"                if self.check_container():"},{"line_number":220,"context_line":"                    raise"}],"source_content_type":"text/x-python","patch_set":30,"id":"b98a821d_f3f56636","line":217,"range":{"start_line":214,"start_character":0,"end_line":217,"end_character":44},"updated":"2024-09-04 08:55:11.000000000","message":"I currently don\u0027t understand why it\u0027s no longer necessary here to remove the systemd unit file (line 238 in old patch). Could you please explain it?","commit_id":"02b93f68250e5fba27b5aca63a666f8b93418efc"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"3616926fd8cfc2721bd7d3d5130375aab590a9db","unresolved":false,"context_lines":[{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    def remove_container(self):"},{"line_number":213,"context_line":"        self.changed |\u003d self.systemd.remove_unit_file()"},{"line_number":214,"context_line":"        container \u003d self.check_container()"},{"line_number":215,"context_line":"        if container:"},{"line_number":216,"context_line":"            try:"},{"line_number":217,"context_line":"                container.remove(force\u003dTrue)"},{"line_number":218,"context_line":"            except docker.errors.APIError:"},{"line_number":219,"context_line":"                if self.check_container():"},{"line_number":220,"context_line":"                    raise"}],"source_content_type":"text/x-python","patch_set":30,"id":"0e2f75c4_74665b47","line":217,"range":{"start_line":214,"start_character":0,"end_line":217,"end_character":44},"in_reply_to":"b3e61061_ceb90a4a","updated":"2024-11-07 15:11:49.000000000","message":"Acknowledged","commit_id":"02b93f68250e5fba27b5aca63a666f8b93418efc"},{"author":{"_account_id":34113,"name":"Martin Hiner","email":"martin.hiner@tietoevry.com","username":"hinermar"},"change_message_id":"1712a35ef094262dc6eaa4f765ee1da7f895a4a2","unresolved":true,"context_lines":[{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    def remove_container(self):"},{"line_number":213,"context_line":"        self.changed |\u003d self.systemd.remove_unit_file()"},{"line_number":214,"context_line":"        container \u003d self.check_container()"},{"line_number":215,"context_line":"        if container:"},{"line_number":216,"context_line":"            try:"},{"line_number":217,"context_line":"                container.remove(force\u003dTrue)"},{"line_number":218,"context_line":"            except docker.errors.APIError:"},{"line_number":219,"context_line":"                if self.check_container():"},{"line_number":220,"context_line":"                    raise"}],"source_content_type":"text/x-python","patch_set":30,"id":"b3e61061_ceb90a4a","line":217,"range":{"start_line":214,"start_character":0,"end_line":217,"end_character":44},"in_reply_to":"b98a821d_f3f56636","updated":"2024-09-18 15:10:50.000000000","message":"It gets called beforehand - L213 now, L226 in old patch. Looks to me like the second removal from old patch was redundant.","commit_id":"02b93f68250e5fba27b5aca63a666f8b93418efc"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"50fe1aa99274a0960583e1a1efd6012f27dc8c9d","unresolved":true,"context_lines":[{"line_number":264,"context_line":"        env \u003d self._inject_env_var(self.params.get(\u0027environment\u0027))"},{"line_number":265,"context_line":"        return {k: \"\" if env[k] is None else env[k] for k in env}"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"    # Docker-py encounters issues parsing and setting permissions for"},{"line_number":268,"context_line":"    # a mix of volumes and binds when sent together. Therefore, we must"},{"line_number":269,"context_line":"    # parse them and set the permissions ourselves and send them separately."},{"line_number":270,"context_line":"    def parse_volumes(self, volumes, mounts, filtered_volumes):"},{"line_number":271,"context_line":"        # we can ignore empty strings"},{"line_number":272,"context_line":"        volumes \u003d [item for item in volumes if item.strip()]"}],"source_content_type":"text/x-python","patch_set":30,"id":"1acdf1d1_81a815fa","line":269,"range":{"start_line":267,"start_character":3,"end_line":269,"end_character":76},"updated":"2024-09-04 08:55:11.000000000","message":"what kind of issue is this? is there a bugreport about this upstream at docker-py we could link to here? is this a known and expected behaviour of docker-py, if yes is that documented upstream, so we could link the docs maybe?\n\nIt\u0027s currently not clear to me, from this comment alone, if this is meant as a workaround which we can remove once a bug upstream is fixed or if we need to carry this until the end of time - or docker-py. :)\n\nThanks.","commit_id":"02b93f68250e5fba27b5aca63a666f8b93418efc"},{"author":{"_account_id":34113,"name":"Martin Hiner","email":"martin.hiner@tietoevry.com","username":"hinermar"},"change_message_id":"1712a35ef094262dc6eaa4f765ee1da7f895a4a2","unresolved":true,"context_lines":[{"line_number":264,"context_line":"        env \u003d self._inject_env_var(self.params.get(\u0027environment\u0027))"},{"line_number":265,"context_line":"        return {k: \"\" if env[k] is None else env[k] for k in env}"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"    # Docker-py encounters issues parsing and setting permissions for"},{"line_number":268,"context_line":"    # a mix of volumes and binds when sent together. Therefore, we must"},{"line_number":269,"context_line":"    # parse them and set the permissions ourselves and send them separately."},{"line_number":270,"context_line":"    def parse_volumes(self, volumes, mounts, filtered_volumes):"},{"line_number":271,"context_line":"        # we can ignore empty strings"},{"line_number":272,"context_line":"        volumes \u003d [item for item in volumes if item.strip()]"}],"source_content_type":"text/x-python","patch_set":30,"id":"36213141_3f7167d2","line":269,"range":{"start_line":267,"start_character":3,"end_line":269,"end_character":76},"in_reply_to":"1acdf1d1_81a815fa","updated":"2024-09-18 15:10:50.000000000","message":"Seems to me like this isn\u0027t really a problem with docker-py but just an expected behavior. The \"problem\" is that our list of our mounts/volumes cannot just be fed into docker-py\u0027s mounts or volumes so it has to be parsed and sorted like this.\n\nI have removed the confusing comment.","commit_id":"02b93f68250e5fba27b5aca63a666f8b93418efc"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"3616926fd8cfc2721bd7d3d5130375aab590a9db","unresolved":false,"context_lines":[{"line_number":264,"context_line":"        env \u003d self._inject_env_var(self.params.get(\u0027environment\u0027))"},{"line_number":265,"context_line":"        return {k: \"\" if env[k] is None else env[k] for k in env}"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"    # Docker-py encounters issues parsing and setting permissions for"},{"line_number":268,"context_line":"    # a mix of volumes and binds when sent together. Therefore, we must"},{"line_number":269,"context_line":"    # parse them and set the permissions ourselves and send them separately."},{"line_number":270,"context_line":"    def parse_volumes(self, volumes, mounts, filtered_volumes):"},{"line_number":271,"context_line":"        # we can ignore empty strings"},{"line_number":272,"context_line":"        volumes \u003d [item for item in volumes if item.strip()]"}],"source_content_type":"text/x-python","patch_set":30,"id":"743a1454_b9d19d8d","line":269,"range":{"start_line":267,"start_character":3,"end_line":269,"end_character":76},"in_reply_to":"36213141_3f7167d2","updated":"2024-11-07 15:11:49.000000000","message":"Done","commit_id":"02b93f68250e5fba27b5aca63a666f8b93418efc"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"50fe1aa99274a0960583e1a1efd6012f27dc8c9d","unresolved":true,"context_lines":[{"line_number":273,"context_line":""},{"line_number":274,"context_line":"        for item in volumes:"},{"line_number":275,"context_line":"            # if it starts with / it is bind not volume"},{"line_number":276,"context_line":"            if item[0] \u003d\u003d \u0027/\u0027:"},{"line_number":277,"context_line":"                mode \u003d None"},{"line_number":278,"context_line":"                try:"},{"line_number":279,"context_line":"                    if item.count(\u0027:\u0027) \u003d\u003d 2:"}],"source_content_type":"text/x-python","patch_set":30,"id":"086db23b_9713851b","line":276,"range":{"start_line":276,"start_character":15,"end_line":276,"end_character":29},"updated":"2024-09-04 08:55:11.000000000","message":"imho easier to grok:\n\n```suggestion\n            if item.startswith(\u0027/\u0027):\n```","commit_id":"02b93f68250e5fba27b5aca63a666f8b93418efc"},{"author":{"_account_id":34113,"name":"Martin Hiner","email":"martin.hiner@tietoevry.com","username":"hinermar"},"change_message_id":"1712a35ef094262dc6eaa4f765ee1da7f895a4a2","unresolved":false,"context_lines":[{"line_number":273,"context_line":""},{"line_number":274,"context_line":"        for item in volumes:"},{"line_number":275,"context_line":"            # if it starts with / it is bind not volume"},{"line_number":276,"context_line":"            if item[0] \u003d\u003d \u0027/\u0027:"},{"line_number":277,"context_line":"                mode \u003d None"},{"line_number":278,"context_line":"                try:"},{"line_number":279,"context_line":"                    if item.count(\u0027:\u0027) \u003d\u003d 2:"}],"source_content_type":"text/x-python","patch_set":30,"id":"e1695188_1e8966bf","line":276,"range":{"start_line":276,"start_character":15,"end_line":276,"end_character":29},"in_reply_to":"086db23b_9713851b","updated":"2024-09-18 15:10:50.000000000","message":"Done","commit_id":"02b93f68250e5fba27b5aca63a666f8b93418efc"}],"ansible/roles/openvswitch/tasks/config-host.yml":[{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"ea5bf516b1db0e91388e80ceee397db49e08f6be","unresolved":true,"context_lines":[{"line_number":7,"context_line":"      - {\u0027name\u0027: openvswitch}"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"# NOTE(m.hiner): Podman considers non-existent mount directory"},{"line_number":10,"context_line":"# as a error, so it has to be created beforehand. There is same"},{"line_number":11,"context_line":"# issue with docker."},{"line_number":12,"context_line":"# See: https://github.com/containers/podman/issues/14781"},{"line_number":13,"context_line":"- name: Create /run/openvswitch directory on host"},{"line_number":14,"context_line":"  become: True"}],"source_content_type":"text/x-yaml","patch_set":18,"id":"48857f5b_a8ab5d65","line":11,"range":{"start_line":10,"start_character":50,"end_line":11,"end_character":20},"updated":"2024-06-10 13:25:12.000000000","message":"I don\u0027t see how that follows from any of the linked issues on github, it\u0027s rather the reverse: docker creates directories if they don\u0027t exist, while podman does not.\n\nhttps://docs.docker.com/reference/cli/docker/container/run/#volume\n\nDo you have a source, that this is also needed for docker?\nIt doesn\u0027t hurt though to precreate the directory for docker as well, because other stuff like permissions and owner are also important to be correct.\n\nSo it might be easier for us to manage this directory in a uniform way across different container runtimes.\n\nNotice that podmans behavior is also consistent with the linux \"mount\" command here, which requires that you precreate any needed directories beforehand.","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"721c6f09510bfd190e3fee477fa51ea461698961","unresolved":false,"context_lines":[{"line_number":7,"context_line":"      - {\u0027name\u0027: openvswitch}"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"# NOTE(m.hiner): Podman considers non-existent mount directory"},{"line_number":10,"context_line":"# as a error, so it has to be created beforehand. There is same"},{"line_number":11,"context_line":"# issue with docker."},{"line_number":12,"context_line":"# See: https://github.com/containers/podman/issues/14781"},{"line_number":13,"context_line":"- name: Create /run/openvswitch directory on host"},{"line_number":14,"context_line":"  become: True"}],"source_content_type":"text/x-yaml","patch_set":18,"id":"b4a69aca_143e1f9e","line":11,"range":{"start_line":10,"start_character":50,"end_line":11,"end_character":20},"in_reply_to":"16036a8d_26b07335","updated":"2024-06-26 07:34:28.000000000","message":"Done","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"},{"author":{"_account_id":36701,"name":"Ivan Halomi","display_name":"Ivan Halomi","email":"ivan.halomi@gmail.com","username":"ivanhalomi"},"change_message_id":"0c49ef222e79b9913d6a89bc74fe8aabe187695b","unresolved":true,"context_lines":[{"line_number":7,"context_line":"      - {\u0027name\u0027: openvswitch}"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"# NOTE(m.hiner): Podman considers non-existent mount directory"},{"line_number":10,"context_line":"# as a error, so it has to be created beforehand. There is same"},{"line_number":11,"context_line":"# issue with docker."},{"line_number":12,"context_line":"# See: https://github.com/containers/podman/issues/14781"},{"line_number":13,"context_line":"- name: Create /run/openvswitch directory on host"},{"line_number":14,"context_line":"  become: True"}],"source_content_type":"text/x-yaml","patch_set":18,"id":"16036a8d_26b07335","line":11,"range":{"start_line":10,"start_character":50,"end_line":11,"end_character":20},"in_reply_to":"48857f5b_a8ab5d65","updated":"2024-06-25 10:51:46.000000000","message":"There is exactly same problem in docker, it\u0027s not clear from podman issue since they talk about volumes which creates confusion. Docker volume will create non existing path for legacy reasons but we are not using volumes( issue mentioned in docker-worker) but we use mounts(specifically binds). Binds don\u0027t create path if it doesn\u0027t exist but will throw error ( see documentation: https://docs.docker.com/storage/bind-mounts/ ). I can\u0027t find doc page for podman explaining better why we need to create path before so I will just add docker documentation so it\u0027s more clear.","commit_id":"f0532f55df054f081d279fe08e81e558265bd48b"}],"tests/kolla_container_tests/test_docker_worker.py":[{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"466a80b614cdb17793609a5a8f3fe3f02f125ec1","unresolved":false,"context_lines":[{"line_number":373,"context_line":"        self.assertTrue(self.dw.changed)"},{"line_number":374,"context_line":"        docker_create_kwargs \u003d self.dw.dc.containers.create.call_args.kwargs.items()    # noqa"},{"line_number":375,"context_line":"        self.dw.dc.containers.create.assert_called_once()"},{"line_number":376,"context_line":"        self.assertIn((\u0027tmpfs\u0027, [\u0027/tmp\u0027]), docker_create_kwargs)  # nosec: B108"},{"line_number":377,"context_line":""},{"line_number":378,"context_line":"    def test_create_container_with_tmpfs_empty_string(self):"},{"line_number":379,"context_line":"        self.fake_data[\u0027params\u0027][\u0027tmpfs\u0027] \u003d [\u0027\u0027]  # nosec: B108"}],"source_content_type":"text/x-python","patch_set":13,"id":"2454b0c7_919da8f1","line":376,"range":{"start_line":376,"start_character":0,"end_line":376,"end_character":2},"updated":"2024-03-07 11:49:06.000000000","message":"just saying, we could as well just make usage of secure tmp file locations even in CI tests. not sure everyone agrees with me, so marking this as resolved for now.","commit_id":"f58b3e412ba79b962abe9caa16bd57c22f75c74d"}]}