)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"71982408cc55ffa3901c72c1f81a5eee954207e9","unresolved":true,"context_lines":[{"line_number":11,"context_line":"unplugging VIFs and removing volume targets. Prior to"},{"line_number":12,"context_line":"Iea337f73c231db2cb9d9f639b92475daaede6793, we also reset the"},{"line_number":13,"context_line":"instance_info and instance_uuid fields."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"When Ironic automated cleaning is enabled, these cleanup actions race"},{"line_number":16,"context_line":"with Ironic, and may result in an HTTP 409 Conflict and a number of"},{"line_number":17,"context_line":"retries, due to Ironic holding the node\u0027s lock or the node being in a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"2ae7bec7_501743c3","line":14,"updated":"2024-07-24 18:38:28.000000000","message":"all of thse actions need to be completed before the nova instance is marked as deleted and befor ethe ironic node is returned to an available state.\n\ni am not sure if that is what happens to do but doing that asynchronously is not really ok unless nova is configured for soft delete.\n\nit is only ok in that case because wehn soft delete is enabeld we have to support restroing the vm if asked with all data intact.\n\nallowing them to be cleared async would potentially be a security issue.\ni.e. if the volumes have delete on terminate set we cannot just skip this on the nova side and mark the instance as deleted we need to ensure the voluems are deleted before that.\n\nfor volumes without delete on terminate we shoudl still ensure the athey are detached proir to marking th instace as deleted.","commit_id":"d56b7e639866246b6a2f3b03804c1956e4e9175a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"71982408cc55ffa3901c72c1f81a5eee954207e9","unresolved":true,"context_lines":[{"line_number":23,"context_line":"unnecessary once a node has been deprovisioned, since they are performed"},{"line_number":24,"context_line":"as part of the node\u0027s tear_down routine [1][2]. This change removes"},{"line_number":25,"context_line":"these cleanup operations in that case, retaining them only when the"},{"line_number":26,"context_line":"provision state of the node was never changed (i.e. spawn failed early)."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"[1] https://opendev.org/openstack/ironic/src/commit/912dcbbdc9c0859168ac4ea6fa6b6f7ad4358e84/ironic/conductor/manager.py#L1117"},{"line_number":29,"context_line":"[2] https://opendev.org/openstack/ironic/src/commit/912dcbbdc9c0859168ac4ea6fa6b6f7ad4358e84/ironic/drivers/modules/agent_base.py#L685"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"f44adca8_0b3de665","line":26,"updated":"2024-07-24 18:38:28.000000000","message":"ack. illl keep this in mind when reviewing the actual code change.\n\nwhen i frist responed i had just going as far as the commit message and not the code change itsefl as in genreal this is touching some deep technial debth that we dont want to extend future by relying on ironic to do things its not ment to be doign today.\n\nin the context of resolving the current issue it might be accpable in a limited scope","commit_id":"d56b7e639866246b6a2f3b03804c1956e4e9175a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"93b5fabac34f7f57eb502fa2b6316f2ce97cde03","unresolved":true,"context_lines":[{"line_number":24,"context_line":"as part of the node\u0027s tear_down routine [1][2]. This change removes"},{"line_number":25,"context_line":"these cleanup operations in that case, retaining them only when the"},{"line_number":26,"context_line":"provision state of the node was never changed (i.e. spawn failed early)."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"[1] https://opendev.org/openstack/ironic/src/commit/912dcbbdc9c0859168ac4ea6fa6b6f7ad4358e84/ironic/conductor/manager.py#L1117"},{"line_number":29,"context_line":"[2] https://opendev.org/openstack/ironic/src/commit/912dcbbdc9c0859168ac4ea6fa6b6f7ad4358e84/ironic/drivers/modules/agent_base.py#L685"},{"line_number":30,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"0e633f58_1a5c00a8","line":27,"updated":"2024-07-24 09:13:31.000000000","message":"so im in two minds about this.\n\nwhen provisioning a nova instance ironic should not be doing any volume or neutron port management. its technically not allowed to update either however for historical reasons because of design choices in ironci driven by ironic standalone\nironic has been interacting with netron ports and volume attachments even though tis a security issue.\n\nso nova is really operating as it should in manageing the volume/neturon resouces in a driver indepentent maner as it was design too and this is a result of the fact the ironic driver is and ironic its self is breaking the virt driver/service contract by modifying resouce in neutron/cidner owned by a nova instnace.\n\nat some point we need to change ironci to not do that when an instnace is manged by nova. it is ok only when ironic is in standarone mode.","commit_id":"d56b7e639866246b6a2f3b03804c1956e4e9175a"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"4e105b531e609747d2a50309b01c8e340f45aad4","unresolved":true,"context_lines":[{"line_number":24,"context_line":"as part of the node\u0027s tear_down routine [1][2]. This change removes"},{"line_number":25,"context_line":"these cleanup operations in that case, retaining them only when the"},{"line_number":26,"context_line":"provision state of the node was never changed (i.e. spawn failed early)."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"[1] https://opendev.org/openstack/ironic/src/commit/912dcbbdc9c0859168ac4ea6fa6b6f7ad4358e84/ironic/conductor/manager.py#L1117"},{"line_number":29,"context_line":"[2] https://opendev.org/openstack/ironic/src/commit/912dcbbdc9c0859168ac4ea6fa6b6f7ad4358e84/ironic/drivers/modules/agent_base.py#L685"},{"line_number":30,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"915a14fe_b5c5f08f","line":27,"in_reply_to":"0e633f58_1a5c00a8","updated":"2024-07-24 09:18:33.000000000","message":"I think we need to separate fixing issues in the way it currently works from possible future architecture changes (which we know have a habit of taking a long time if they happen at all). There\u0027s a race here that can end up with instances in ERROR state. Let\u0027s fix it.","commit_id":"d56b7e639866246b6a2f3b03804c1956e4e9175a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"71982408cc55ffa3901c72c1f81a5eee954207e9","unresolved":true,"context_lines":[{"line_number":24,"context_line":"as part of the node\u0027s tear_down routine [1][2]. This change removes"},{"line_number":25,"context_line":"these cleanup operations in that case, retaining them only when the"},{"line_number":26,"context_line":"provision state of the node was never changed (i.e. spawn failed early)."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"[1] https://opendev.org/openstack/ironic/src/commit/912dcbbdc9c0859168ac4ea6fa6b6f7ad4358e84/ironic/conductor/manager.py#L1117"},{"line_number":29,"context_line":"[2] https://opendev.org/openstack/ironic/src/commit/912dcbbdc9c0859168ac4ea6fa6b6f7ad4358e84/ironic/drivers/modules/agent_base.py#L685"},{"line_number":30,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"97a322d1_8b7b82a1","line":27,"in_reply_to":"2ab26e84_e928b47d","updated":"2024-07-24 18:38:28.000000000","message":"jay and i talked about this separately on irc\n\nwe will likely fix things as they are and preoably even backport them.\nbut to be clear we can\u0027t continue to support the ironic driver long term if we\ndont start paryign down the technial debt\n\nwe discussed some of these topics during the zed and antolpoe cycle.\n\nat the antelope ptg we focused on the topics that were blocking the ironic sharding work https://etherpad.opendev.org/p/r.8ae4e0ef997aebfe626b2b272ff23f1b#L59\n\nto which you had crated https://review.opendev.org/c/openstack/nova-specs/+/842015/1/specs/zed/approved/ironic-fix-management-issues.rst to do some of the context seting.\n\nwhen https://security.openstack.org/ossa/OSSA-2023-003.html was discussed we had several conversations as a result about how it was not ok for any other service to delete/detach or attach ports or volumes from a nova instance.\n\nthat included ironic, when its used as a virt driver it shoudl not interact with cinder or neutron apis.\n\nit is not supported form a nova perspective to detach a volume or port from a nova instance in any other way than via the nova api which inlcuded manging the port bidnings or volume attachments.\n\nwe probably need to create a \"Ironic\u003c-\u003eNova compute inconsistencies take 2\" spec to capture the set of remaining issues related to networking and volumes.","commit_id":"d56b7e639866246b6a2f3b03804c1956e4e9175a"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"4c3db080ea6838692ad1fe8b0e617e9b825096e1","unresolved":true,"context_lines":[{"line_number":24,"context_line":"as part of the node\u0027s tear_down routine [1][2]. This change removes"},{"line_number":25,"context_line":"these cleanup operations in that case, retaining them only when the"},{"line_number":26,"context_line":"provision state of the node was never changed (i.e. spawn failed early)."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"[1] https://opendev.org/openstack/ironic/src/commit/912dcbbdc9c0859168ac4ea6fa6b6f7ad4358e84/ironic/conductor/manager.py#L1117"},{"line_number":29,"context_line":"[2] https://opendev.org/openstack/ironic/src/commit/912dcbbdc9c0859168ac4ea6fa6b6f7ad4358e84/ironic/drivers/modules/agent_base.py#L685"},{"line_number":30,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"2ab26e84_e928b47d","line":27,"in_reply_to":"915a14fe_b5c5f08f","updated":"2024-07-24 15:33:10.000000000","message":"What bug # is that Sean? Who is planning on doing this redesign? I\u0027ve not seen it mentioned in any Ironic PTG session or Nova PTG summary, and I\u0027m suspicious we have the people to assign to this work. Will that redesign be higher or lower priority than e.g. eventlet removal? \n\nI agree with Mark that we should fix the bug as it exists today and not chase after redesigns which, AFAICT, have not been planned in any meaningful manner.","commit_id":"d56b7e639866246b6a2f3b03804c1956e4e9175a"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"189f7f0ef94b9dd870a9f83e13bb474bdee7bf55","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"57a23c3f_003672f8","updated":"2023-05-18 08:22:21.000000000","message":"recheck\n\nnova-multi-cell failed 1 test. Looks ok in https://zuul.openstack.org/builds?job_name\u003dnova-multi-cell\u0026skip\u003d0","commit_id":"2bf0f888180e3e7c617f9d4054f2306f0833cffc"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"f5bf25548361da1aae050911b1c3a2728abae683","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"dd4f9262_93ad5884","updated":"2024-07-04 18:39:43.000000000","message":"recheck ensuring it still passes tests","commit_id":"2bf0f888180e3e7c617f9d4054f2306f0833cffc"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"93b5fabac34f7f57eb502fa2b6316f2ce97cde03","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"418b1908_943a51be","updated":"2024-07-24 09:13:31.000000000","message":"-1 because this is caused by somethign we have prvisouly discuss as incorrect for ironic to be doing for nova instances","commit_id":"d56b7e639866246b6a2f3b03804c1956e4e9175a"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"8c217426770c3e1a895b61c76a7d2f3f3a8ed95b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"adf2afeb_7a58c2d3","updated":"2024-07-24 09:02:54.000000000","message":"Fixed merge conflicts","commit_id":"d56b7e639866246b6a2f3b03804c1956e4e9175a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"71982408cc55ffa3901c72c1f81a5eee954207e9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"886fc03c_5628b7c4","updated":"2024-07-24 18:38:28.000000000","message":"ill review this when i have some time proably next week","commit_id":"d56b7e639866246b6a2f3b03804c1956e4e9175a"},{"author":{"_account_id":6799,"name":"Nicholas Kuechler","email":"nkuechler@gmail.com","username":"nicholaskuechler"},"change_message_id":"1060bcde91b9bf2f95a191249c159773810667d7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"2fd7d565_6c0ceae4","updated":"2025-01-21 18:32:25.000000000","message":"I\u0027m hitting the same issue and I\u0027d like to see this fixed. Thanks!","commit_id":"9acb7182bd2b2cc7aaa4093f8230f280d4f94a9e"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"329bd4e3c725c206692ace4e8c6cb3c892559e6d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"c0ee58b2_1aa6df65","updated":"2025-01-23 10:07:52.000000000","message":"Looks good to me, longer term goals aside. It\u0027s a useful bug fix.","commit_id":"9acb7182bd2b2cc7aaa4093f8230f280d4f94a9e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7e4a5e06420d19f2a30494a4e9623e834d2c604a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"a57c2b65_f19b3e51","updated":"2025-01-29 22:24:00.000000000","message":"Looks reasonable to me.","commit_id":"6ebd8a56d1c79fc15f385f232ce0e22937b2fdac"},{"author":{"_account_id":37620,"name":"suiong ng","display_name":"Suiong Ng","email":"YoungN@supermicro.com","username":"suiong_ng"},"change_message_id":"410bad19d34a5f683ce3ac39d0347c63df633c52","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f22821b8_26c2c58b","updated":"2025-02-03 05:52:20.000000000","message":"should we cherry pick this to 2024.1 and 2024.2 as well","commit_id":"6ebd8a56d1c79fc15f385f232ce0e22937b2fdac"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bc2a77ee363f14d1e7e543cec8f589d335163d42","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b25c1bde_1bd57720","updated":"2025-01-29 11:59:29.000000000","message":"the previous ci logs have long rotated but lets be optimistic and assume it will pass after this rebase.","commit_id":"6ebd8a56d1c79fc15f385f232ce0e22937b2fdac"},{"author":{"_account_id":5890,"name":"Doug Goldstein","email":"cardoe@cardoe.com","username":"cardoe"},"change_message_id":"84b5240a2e055f73ee399c60cfefad098d324164","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"6e59f63f_5947f67f","in_reply_to":"f22821b8_26c2c58b","updated":"2025-02-06 02:29:34.000000000","message":"I would definitely like to have it in at least 2024.2. I\u0027ll propose the cherry-pick.","commit_id":"6ebd8a56d1c79fc15f385f232ce0e22937b2fdac"}]}