)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"376aa36ccd5a4f25b3f77f2ce75a40fb02c45468","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"73acee46_129b0c14","updated":"2025-02-06 05:54:30.000000000","message":"I have seen the same issue when deploying ironic, so it would be good to have a solution, but I don\u0027t like having to give the ironic user essentially full root access. can\u0027t we ensure proper permissions otherwise, like from kolla-ansible maybe?","commit_id":"40377aec9e28d018e4bd01cb52ee8bb7afea5ead"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"b96e3c4af9e731a6b8e7b7cf7f9c3e21fca5535b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"55ddf6d6_5c3199ce","updated":"2025-02-06 22:39:58.000000000","message":"can we merhe this ? any comments ?","commit_id":"679e8b0f75c3d747cc80039ad1889be3f4e1aad1"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"cb823624c34187cec0cc4e7a81feecb634258fdb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"209f136d_74c7cc4f","updated":"2025-02-07 11:46:44.000000000","message":"@bartosz@stackhpc.com do you think you can approve also https://review.opendev.org/c/openstack/kolla/+/940605 as previous commmit in this relation chain ? Otherwise i think it will not be merged ? There is already +2 from frickler.","commit_id":"ed7c8399c4a2a43e9a4165db416b98f5be8d538c"}],"docker/ironic/ironic-base/extend_start.sh":[{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"376aa36ccd5a4f25b3f77f2ce75a40fb02c45468","unresolved":true,"context_lines":[{"line_number":4,"context_line":"METRICS_PATH\u003d/var/lib/ironic-metrics"},{"line_number":5,"context_line":""},{"line_number":6,"context_line":"if [[ ! -d \"${LOG_PATH}\" ]]; then"},{"line_number":7,"context_line":"    sudo mkdir -p \"${LOG_PATH}\""},{"line_number":8,"context_line":"fi"},{"line_number":9,"context_line":"if [ ! -d ${METRICS_PATH} ]; then"},{"line_number":10,"context_line":"    sudo mkdir -p ${METRICS_PATH}"}],"source_content_type":"text/x-sh","patch_set":8,"id":"2ebc3253_bc0bbd0f","line":7,"range":{"start_line":7,"start_character":4,"end_line":7,"end_character":8},"updated":"2025-02-06 05:54:30.000000000","message":"this change looks unrelated, is it an accident or why is it needed? /var/log/kolla should already be owned by kolla?","commit_id":"40377aec9e28d018e4bd01cb52ee8bb7afea5ead"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"36a8402a96146c6d66255c291639223fe2a1e411","unresolved":true,"context_lines":[{"line_number":4,"context_line":"METRICS_PATH\u003d/var/lib/ironic-metrics"},{"line_number":5,"context_line":""},{"line_number":6,"context_line":"if [[ ! -d \"${LOG_PATH}\" ]]; then"},{"line_number":7,"context_line":"    sudo mkdir -p \"${LOG_PATH}\""},{"line_number":8,"context_line":"fi"},{"line_number":9,"context_line":"if [ ! -d ${METRICS_PATH} ]; then"},{"line_number":10,"context_line":"    sudo mkdir -p ${METRICS_PATH}"}],"source_content_type":"text/x-sh","patch_set":8,"id":"5014c344_ea83e524","line":7,"range":{"start_line":7,"start_character":4,"end_line":7,"end_character":8},"in_reply_to":"2ebc3253_bc0bbd0f","updated":"2025-02-06 06:17:03.000000000","message":"Yes, you are right, but at the same time, you are not. Essentially, if mkdir -p /var/log/kolla is supposed to remain here, it should be run with sudo. The reason is that the image runs under the ironic user, and in kolla_start, kolla_extend_start is executed, which is essentially this script.\n\nThis means that the ironic user is currently trying to create /var/log/kolla using mkdir (without sudo). This works only because if the path already exists (which, as you said, it does—since it is mounted as a volume from kolla-ansible), mkdir simply completes successfully.\n\nNow that I think about it, mkdir could be removed entirely from all images. The directory is owned by kolla, but that’s only because the Fluentd container changes it. However, what if the user does not use Fluentd in kolla-ansible? In that case, the necessary sudo permissions come from this Fluentd sudoers file, if user is not using fluentd from kolla-ansible /var/log/kolla don\u0027t exist anywhere.\n\nSo yes, you are correct—sudo does not have to be there, but neither should mkdir itself. And if mkdir should remain everywhere, then chown should also be included. It actually seems like we should come up with a more general solution.\n\nSimply said I\u0027ve just added sudo to follow the same pattern in a script.\n\nSo what should I do now ?","commit_id":"40377aec9e28d018e4bd01cb52ee8bb7afea5ead"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"0c16e360cf575c2f1c1b3581e74a7b712a3ab2a9","unresolved":false,"context_lines":[{"line_number":4,"context_line":"METRICS_PATH\u003d/var/lib/ironic-metrics"},{"line_number":5,"context_line":""},{"line_number":6,"context_line":"if [[ ! -d \"${LOG_PATH}\" ]]; then"},{"line_number":7,"context_line":"    sudo mkdir -p \"${LOG_PATH}\""},{"line_number":8,"context_line":"fi"},{"line_number":9,"context_line":"if [ ! -d ${METRICS_PATH} ]; then"},{"line_number":10,"context_line":"    sudo mkdir -p ${METRICS_PATH}"}],"source_content_type":"text/x-sh","patch_set":8,"id":"a0a0dc4e_f59bbe8d","line":7,"range":{"start_line":7,"start_character":4,"end_line":7,"end_character":8},"in_reply_to":"5014c344_ea83e524","updated":"2025-02-06 06:30:07.000000000","message":"Aaaaa, sorry, my fault, now i realized that it\u0027s /var/log/kolla/IRONIC ...and not /var/log/kolla 😊. Sorry for my mistake 😊","commit_id":"40377aec9e28d018e4bd01cb52ee8bb7afea5ead"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"376aa36ccd5a4f25b3f77f2ce75a40fb02c45468","unresolved":true,"context_lines":[{"line_number":6,"context_line":"if [[ ! -d \"${LOG_PATH}\" ]]; then"},{"line_number":7,"context_line":"    sudo mkdir -p \"${LOG_PATH}\""},{"line_number":8,"context_line":"fi"},{"line_number":9,"context_line":"if [ ! -d ${METRICS_PATH} ]; then"},{"line_number":10,"context_line":"    sudo mkdir -p ${METRICS_PATH}"},{"line_number":11,"context_line":"fi"},{"line_number":12,"context_line":"if [[ $(stat -c %a \"${LOG_PATH}\") !\u003d \"755\" ]]; then"}],"source_content_type":"text/x-sh","patch_set":8,"id":"bde251a4_6775cc7f","line":9,"updated":"2025-02-06 05:54:30.000000000","message":"can we keep things consistent here? e.g. use `[[` and have `\"` around the variable like for LOG_PATH above?","commit_id":"40377aec9e28d018e4bd01cb52ee8bb7afea5ead"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"36a8402a96146c6d66255c291639223fe2a1e411","unresolved":false,"context_lines":[{"line_number":6,"context_line":"if [[ ! -d \"${LOG_PATH}\" ]]; then"},{"line_number":7,"context_line":"    sudo mkdir -p \"${LOG_PATH}\""},{"line_number":8,"context_line":"fi"},{"line_number":9,"context_line":"if [ ! -d ${METRICS_PATH} ]; then"},{"line_number":10,"context_line":"    sudo mkdir -p ${METRICS_PATH}"},{"line_number":11,"context_line":"fi"},{"line_number":12,"context_line":"if [[ $(stat -c %a \"${LOG_PATH}\") !\u003d \"755\" ]]; then"}],"source_content_type":"text/x-sh","patch_set":8,"id":"147d8b3b_d3400d29","line":9,"in_reply_to":"bde251a4_6775cc7f","updated":"2025-02-06 06:17:03.000000000","message":"Of course - done.","commit_id":"40377aec9e28d018e4bd01cb52ee8bb7afea5ead"}],"docker/ironic/ironic-base/ironic_sudoers":[{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"376aa36ccd5a4f25b3f77f2ce75a40fb02c45468","unresolved":true,"context_lines":[{"line_number":1,"context_line":"ironic ALL \u003d (root) NOPASSWD: /var/lib/kolla/venv/bin/ironic-rootwrap /etc/ironic/rootwrap.conf *, /bin/chown, /bin/chmod, /bin/mkdir"}],"source_content_type":"application/octet-stream","patch_set":8,"id":"976f43e2_2fdf84ab","line":1,"updated":"2025-02-06 05:54:30.000000000","message":"I\u0027m sceptical about this, as essentially it makes the ironic user root","commit_id":"40377aec9e28d018e4bd01cb52ee8bb7afea5ead"},{"author":{"_account_id":27339,"name":"Michal Arbet","email":"michal.arbet@ultimum.io","username":"michalarbet"},"change_message_id":"3516ad19037823a0a12e1d329ccea74c0978d550","unresolved":false,"context_lines":[{"line_number":1,"context_line":"ironic ALL \u003d (root) NOPASSWD: /var/lib/kolla/venv/bin/ironic-rootwrap /etc/ironic/rootwrap.conf *, /bin/chown, /bin/chmod, /bin/mkdir"}],"source_content_type":"application/octet-stream","patch_set":8,"id":"30d15dc6_9495f4bd","line":1,"in_reply_to":"976f43e2_2fdf84ab","updated":"2025-02-06 06:25:42.000000000","message":"I understand, restricted very strictly only to the commands called by extend_start.","commit_id":"40377aec9e28d018e4bd01cb52ee8bb7afea5ead"}]}
