)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15197,"name":"Pierre Riteau","email":"pierre@stackhpc.com","username":"priteau","status":"StackHPC"},"change_message_id":"488e08a7bf026c6d23e6547064927b33d2dd5909","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"34b0db33_eca648db","updated":"2024-06-26 20:24:53.000000000","message":"I have wanted this feature for a long time, thank you 😊","commit_id":"b41dc7ce1fe325f7aa2e27b4beec9867d458b232"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"21d9b128a44203d3b626257d382a336699b3119a","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"cf62cb7a_5b3b5509","updated":"2024-06-26 16:16:41.000000000","message":"need to maybe move this around a bit, originally I had this check in config.yml, maybe add it in config-validate.yml, even if we normally just use that for oslo-config stuff.","commit_id":"b41dc7ce1fe325f7aa2e27b4beec9867d458b232"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"f5cc7439cec3ee0b3c3d493600b6dc92cc9f23f2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"da8819e8_0a0f86b7","in_reply_to":"45bfe580_4f51131a","updated":"2024-06-27 12:23:51.000000000","message":"Done","commit_id":"b41dc7ce1fe325f7aa2e27b4beec9867d458b232"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"aa1cfe6a12bf3ac97a51a5a346cc2057284333b6","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"45bfe580_4f51131a","in_reply_to":"cf62cb7a_5b3b5509","updated":"2024-06-26 16:17:24.000000000","message":"currently fails in: https://zuul.opendev.org/t/openstack/build/bb3ad4b86ebd44079e1717d091a5369c/log/primary/logs/ansible/deploy-prechecks\n\nbecause container-fact haproxy is not defined","commit_id":"b41dc7ce1fe325f7aa2e27b4beec9867d458b232"},{"author":{"_account_id":32398,"name":"Gaël THEROND","display_name":"Fl1nt","email":"gael.therond@bitswalk.com","username":"Fl1nt"},"change_message_id":"97cb1ba7ac246ab73aaa6094fd322bd3501a54f1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"4c22e965_d9edcf5f","updated":"2024-07-16 14:26:22.000000000","message":"LGTM","commit_id":"6435dee26538853428ba70c7ff071d371f06c8e1"},{"author":{"_account_id":35263,"name":"Matt Crees","email":"mattc@stackhpc.com","username":"mattcrees"},"change_message_id":"52ab1d62eb00e4f439368843799ec93e456f683f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"12a0e1b0_d3038901","updated":"2024-06-28 08:34:19.000000000","message":"Looks like a great addition. \nI like that you\u0027ve included an assertion on the config being valid, we could consider adding that to the oslo config validations in a future patch.","commit_id":"6435dee26538853428ba70c7ff071d371f06c8e1"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"29e624b53725ec60847bdca8c8a01840cabfce64","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"886cbad1_7f9bdd89","updated":"2024-06-27 12:23:03.000000000","message":"example output from the validate job:\n\n```\n\nTASK [loadbalancer : Validating haproxy config files] **************************\n\ntask path: /home/zuul/kolla-ansible-venv/share/kolla-ansible/ansible/roles/loadbalancer/tasks/config_validate.yml:2\n\nThursday 27 June 2024  11:29:26 +0000 (0:00:00.121)       0:00:04.429 ********* \n\nUsing module file /home/zuul/kolla-ansible-venv/lib/python3.11/site-packages/ansible/modules/command.py\n\nPipelining is enabled.\n\n\u003c172.99.67.210\u003e ESTABLISH SSH CONNECTION FOR USER: None\n\n\u003c172.99.67.210\u003e SSH: EXEC ssh -C -o ControlMaster\u003dauto -o ControlPersist\u003d300 -o StrictHostKeyChecking\u003dno -o KbdInteractiveAuthentication\u003dno -o PreferredAuthentications\u003dgssapi-with-mic,gssapi-keyex,hostbased,publickey -o PasswordAuthentication\u003dno -o ConnectTimeout\u003d10 -o \u0027ControlPath\u003d\"/home/zuul/.ansible/cp/7a97ed0cbe\"\u0027 172.99.67.210 \u0027/bin/sh -c \u0027\"\u0027\"\u0027sudo -H -S -n  -u root /bin/sh -c \u0027\"\u0027\"\u0027\"\u0027\"\u0027\"\u0027\"\u0027\"\u0027\"\u0027echo BECOME-SUCCESS-yiwvcgevmpstobvczrwscnbkrsrroind ; /usr/bin/python3\u0027\"\u0027\"\u0027\"\u0027\"\u0027\"\u0027\"\u0027\"\u0027\"\u0027 \u0026\u0026 sleep 0\u0027\"\u0027\"\u0027\u0027\n\nEscalation succeeded\n\n\u003c172.99.67.210\u003e (0, b\u0027\\n{\"changed\": true, \"stdout\": \"Warnings were found.\\\\nConfiguration file is valid\", \"stderr\": \"[NOTICE]   (1277) : haproxy version is 2.6.12-1+deb12u1\\\\n[NOTICE]   (1277) : path to executable is /usr/sbin/haproxy\\\\n[WARNING]  (1277) : config : parsing [/etc/haproxy/services.d//horizon.cfg:7] : a \\\u0027http-request\\\u0027 rule placed after a \\\u0027use_backend\\\u0027 rule will still be processed before.\", \"rc\": 0, \"cmd\": \"docker exec -i haproxy haproxy -c -f /etc/haproxy/haproxy.cfg -f /etc/haproxy/services.d/\", \"start\": \"2024-06-27 11:29:27.052215\", \"end\": \"2024-06-27 11:29:27.389740\", \"delta\": \"0:00:00.337525\", \"msg\": \"\", \"invocation\": {\"module_args\": {\"_raw_params\": \"docker exec -i haproxy haproxy -c -f /etc/haproxy/haproxy.cfg -f /etc/haproxy/services.d/\", \"_uses_shell\": true, \"stdin_add_newline\": true, \"strip_empty_ends\": true, \"argv\": null, \"chdir\": null, \"executable\": null, \"creates\": null, \"removes\": null, \"stdin\": null}}}\\n\u0027, b\u0027\u0027)\n\nchanged: [primary] \u003d\u003e {\n\n    \"changed\": true,\n\n    \"cmd\": \"docker exec -i haproxy haproxy -c -f /etc/haproxy/haproxy.cfg -f /etc/haproxy/services.d/\",\n\n    \"delta\": \"0:00:00.337525\",\n\n    \"end\": \"2024-06-27 11:29:27.389740\",\n\n    \"invocation\": {\n\n        \"module_args\": {\n\n            \"_raw_params\": \"docker exec -i haproxy haproxy -c -f /etc/haproxy/haproxy.cfg -f /etc/haproxy/services.d/\",\n\n            \"_uses_shell\": true,\n\n            \"argv\": null,\n\n            \"chdir\": null,\n\n            \"creates\": null,\n\n            \"executable\": null,\n\n            \"removes\": null,\n\n            \"stdin\": null,\n\n            \"stdin_add_newline\": true,\n\n            \"strip_empty_ends\": true\n\n        }\n\n    },\n\n    \"msg\": \"\",\n\n    \"rc\": 0,\n\n    \"start\": \"2024-06-27 11:29:27.052215\",\n\n    \"stderr\": \"[NOTICE]   (1277) : haproxy version is 2.6.12-1+deb12u1\\n[NOTICE]   (1277) : path to executable is /usr/sbin/haproxy\\n[WARNING]  (1277) : config : parsing [/etc/haproxy/services.d//horizon.cfg:7] : a \u0027http-request\u0027 rule placed after a \u0027use_backend\u0027 rule will still be processed before.\",\n\n    \"stderr_lines\": [\n\n        \"[NOTICE]   (1277) : haproxy version is 2.6.12-1+deb12u1\",\n\n        \"[NOTICE]   (1277) : path to executable is /usr/sbin/haproxy\",\n\n        \"[WARNING]  (1277) : config : parsing [/etc/haproxy/services.d//horizon.cfg:7] : a \u0027http-request\u0027 rule placed after a \u0027use_backend\u0027 rule will still be processed before.\"\n\n    ],\n\n    \"stdout\": \"Warnings were found.\\nConfiguration file is valid\",\n\n    \"stdout_lines\": [\n\n        \"Warnings were found.\",\n\n        \"Configuration file is valid\"\n\n    ]\n\n}\n\n\nTASK [loadbalancer : Assert haproxy config is valid] ***************************\n\ntask path: /home/zuul/kolla-ansible-venv/share/kolla-ansible/ansible/roles/loadbalancer/tasks/config_validate.yml:15\n\nThursday 27 June 2024  11:29:27 +0000 (0:00:01.198)       0:00:05.628 ********* \n\nok: [primary] \u003d\u003e {\n\n    \"changed\": false,\n\n    \"msg\": \"haproxy config is valid\"\n\n}\n```\n\nwe could later even decide if warnings are errors.","commit_id":"6435dee26538853428ba70c7ff071d371f06c8e1"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"90200ac43e617867125341d237704f72b1058c16","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"b93e943e_b44ed587","in_reply_to":"12a0e1b0_d3038901","updated":"2024-06-28 09:04:57.000000000","message":"Thank you. Ideally I would  - later - like to move our whole config validation tasks into it\u0027s own CI job, where it can run before we spawn actual dozens of infrastructure VMs. But for that to work we need to be able to generate the config without actually deploying it, which might prove rather difficult.\n\nBut this at least spots mistakes in configs during CI runs so we can have hopefully increased confidence in changes to haproxy configs.","commit_id":"6435dee26538853428ba70c7ff071d371f06c8e1"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"62d20076ef599d04a55151b6523977de0e7d382a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"ea4a7033_87d3c86c","in_reply_to":"886cbad1_7f9bdd89","updated":"2024-06-27 12:23:36.000000000","message":"link to actual log: https://zuul.opendev.org/t/openstack/build/ebd49d4bb8844ac9a1bf97c3d9328f3c/log/primary/logs/ansible/validate-config","commit_id":"6435dee26538853428ba70c7ff071d371f06c8e1"}],"ansible/roles/loadbalancer/tasks/config_validate.yml":[{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"1fa639710fe34dfb141a30d60c6bf825ccf6da87","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"- name: Validating haproxy config files"},{"line_number":3,"context_line":"  vars:"},{"line_number":4,"context_line":"    service: \"{{ loadbalancer_services[\u0027haproxy\u0027] }}\""},{"line_number":5,"context_line":"  shell: \u003e-"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"76d6bfa3_ea6b4b33","line":2,"updated":"2024-07-03 13:24:07.000000000","message":"And where is that file run? missing import_* in some other playbooks?","commit_id":"6435dee26538853428ba70c7ff071d371f06c8e1"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"bfa8d8a09500664d605862d6061dae7adc52d00d","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"- name: Validating haproxy config files"},{"line_number":3,"context_line":"  vars:"},{"line_number":4,"context_line":"    service: \"{{ loadbalancer_services[\u0027haproxy\u0027] }}\""},{"line_number":5,"context_line":"  shell: \u003e-"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"7781f560_a57cf387","line":2,"in_reply_to":"03814d2f_07887e26","updated":"2024-07-24 13:40:31.000000000","message":"Done","commit_id":"6435dee26538853428ba70c7ff071d371f06c8e1"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"2a74a4d2ec0544f2f21d09967046e8470587bba6","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"- name: Validating haproxy config files"},{"line_number":3,"context_line":"  vars:"},{"line_number":4,"context_line":"    service: \"{{ loadbalancer_services[\u0027haproxy\u0027] }}\""},{"line_number":5,"context_line":"  shell: \u003e-"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"03814d2f_07887e26","line":2,"in_reply_to":"3775d49b_b2c2b83d","updated":"2024-07-17 14:37:22.000000000","message":"is that sufficient explanation @mnasiadka@gmail.com? Thanks for taking a look.","commit_id":"6435dee26538853428ba70c7ff071d371f06c8e1"},{"author":{"_account_id":32553,"name":"Sven Kieske","email":"sven_oss@posteo.de","username":"skieske"},"change_message_id":"42f0dae529007f8278339df465bc2440624378e7","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"- name: Validating haproxy config files"},{"line_number":3,"context_line":"  vars:"},{"line_number":4,"context_line":"    service: \"{{ loadbalancer_services[\u0027haproxy\u0027] }}\""},{"line_number":5,"context_line":"  shell: \u003e-"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"3775d49b_b2c2b83d","line":2,"in_reply_to":"76d6bfa3_ea6b4b33","updated":"2024-07-10 07:32:12.000000000","message":"currently all `config_validate` tasks are only run as a special kolla-ansible command \"validate-config\" (so the naming is reversed which makes searching for it harder).\n\nThis is currently executed in `tests/deploy.sh` line 73:\n\nhttps://opendev.org/openstack/kolla-ansible/src/branch/master/tests/deploy.sh#L73\n\nYou can see the result from this e.g. here:\n\nhttps://zuul.opendev.org/t/openstack/build/d8b62e20fb2f451aa4075bad02a26a72/log/primary/logs/ansible/validate-config\n\nCTRL-F for \"Validating haproxy config files\", we get:\n\n```\nTASK [loadbalancer : Validating haproxy config files] **************************\n[...]\n    \"start\": \"2024-06-27 11:26:15.414573\",\n\n    \"stderr\": \"[WARNING]  (232) : parsing [/etc/haproxy/services.d//horizon.cfg:7] : a \u0027http-request\u0027 rule placed after a \u0027use_backend\u0027 rule will still be processed before.\",\n\n    \"stderr_lines\": [\n\n        \"[WARNING]  (232) : parsing [/etc/haproxy/services.d//horizon.cfg:7] : a \u0027http-request\u0027 rule placed after a \u0027use_backend\u0027 rule will still be processed before.\"\n\n    ],\n\n    \"stdout\": \"Warnings were found.\\nConfiguration file is valid\",\n\n    \"stdout_lines\": [\n\n        \"Warnings were found.\",\n\n        \"Configuration file is valid\"\n\n    ]\n```\n\nNotice, that the check succeeds, even if we have warnings, it only fails when hard errors are present, which is exactly what we want imho.\n\nI agree it would be nicer to use standard ansible mechanics for this task (so call it via an explicit task instead of arcane bash commands, but I wanted to keep it in line with current config_validate tasks of other plays.\n\nAlso it\u0027s necessary to call this relatively late, because all configs need to be finally rendered before validation can take place.\n\nIn an ideal world, I would like to have a separate CI job that doesn\u0027t need to spin up a complete openstack env, just to validate the configuration, but I guess that is outside the scope of this change.\n\nMarking this as resolved for now.\n\nHTH","commit_id":"6435dee26538853428ba70c7ff071d371f06c8e1"}]}