)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"9766aa2cef5693d914e022cd023814a8e2cff805","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0521095f_e4048b74","updated":"2021-10-14 10:37:57.000000000","message":"\u003e Maybe a temporary quota reduction is less of an issue for baremetal given how often the user will want to redeploy with the exact same node.\n\nI\u0027m not sure I get what you\u0027re saying. If cleaning counts towards the quota, the users will end up in situation where their quota disappears for tens of minutes or even hours (in case of shredding).\n\nThis will provide another motivation for operators to disable cleaning, and I\u0027m absolutely against doing that.","commit_id":"5557baa12ceeb27bac1d127f7af94bd2ccc571eb"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"10d748f332e9b5287a2c9359258d0d53aa37306c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"26b4ab54_2ecc121f","updated":"2021-11-05 11:25:08.000000000","message":"\u003e during volume deletion in Cinder (unless you enable trash)\n\nDoes it also take from 15 minutes to many hours?","commit_id":"5557baa12ceeb27bac1d127f7af94bd2ccc571eb"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"e56106ba840f94f242d1cbb27959066ee49110fd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b4ba9238_1e7e66fa","updated":"2021-11-22 20:52:08.000000000","message":"I\u0027ll propose switching this behavior based on a new nova.conf parameter ","commit_id":"5557baa12ceeb27bac1d127f7af94bd2ccc571eb"},{"author":{"_account_id":11292,"name":"Arne Wiebalck","email":"Arne.Wiebalck@cern.ch","username":"wiebalck"},"change_message_id":"eabacbf41ebb6688eb18c22933638d36b9ad6868","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6f188855_1a45c0a0","updated":"2021-10-13 06:14:47.000000000","message":"Thanks, Steve!","commit_id":"5557baa12ceeb27bac1d127f7af94bd2ccc571eb"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"cd1bf82f163580c1c1fb9040de1c1e6fcb074201","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ab3df512_1e3c7871","updated":"2021-10-13 08:51:09.000000000","message":"The reason we did not do it previously was IIRC because an instance in DELETING would count against the user\u0027s quota. Is it not the case?","commit_id":"5557baa12ceeb27bac1d127f7af94bd2ccc571eb"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"1cd41bab953d3817a544dc1c1e66b1de8363e208","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4fb578d8_5011da8a","updated":"2021-11-15 14:25:32.000000000","message":"Then I\u0027d say that the new behavior should be optional. There may be completely opposite operation requirements here.","commit_id":"5557baa12ceeb27bac1d127f7af94bd2ccc571eb"},{"author":{"_account_id":11292,"name":"Arne Wiebalck","email":"Arne.Wiebalck@cern.ch","username":"wiebalck"},"change_message_id":"376f95595f899a2086cd1e8f3977e7bb2a77f8bd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6126a940_173b3d5e","in_reply_to":"0521095f_e4048b74","updated":"2021-11-04 19:11:10.000000000","message":"I think counting resources during deletion against the quota is happening in other projects as well, e.g. during volume deletion in Cinder (unless you enable trash).\n\nI agree, though, that blocking users may increase the pressure on operators to disable cleaning. OTOH, freeing up the quota but then letting the instance creation fail without any visible explanation for the user is not much better. It depends on the deployment: if there are many free instances of the same type, the quota should be returned. If this is the only node to instantiate the user should not be given the impression his formerly used resources are available again.","commit_id":"5557baa12ceeb27bac1d127f7af94bd2ccc571eb"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"249955634f9336e2632f28fd752ca848c4c19e6b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"d37fa1a6_5b64d568","in_reply_to":"245e183f_6e431326","updated":"2021-10-13 20:56:38.000000000","message":"Maybe a temporary quota reduction is less of an issue for baremetal given how often the user will want to redeploy with the exact same node.","commit_id":"5557baa12ceeb27bac1d127f7af94bd2ccc571eb"},{"author":{"_account_id":11292,"name":"Arne Wiebalck","email":"Arne.Wiebalck@cern.ch","username":"wiebalck"},"change_message_id":"2b5fd0c88821f4781bbf6480a85dd6cb4f4fbde8","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5da4a387_854628a2","in_reply_to":"26b4ab54_2ecc121f","updated":"2021-11-09 08:04:01.000000000","message":"Yes, absolutely. This is why I added the trash feature to Cinder to make the deletion asynchronous ... and here I am arguing in the opposite direction :) ... well, basically the same: make it flexible so you can adapt to the situation.","commit_id":"5557baa12ceeb27bac1d127f7af94bd2ccc571eb"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"f0814203f87797db001ceecc2eb0ddc93ce3139d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"245e183f_6e431326","in_reply_to":"ab3df512_1e3c7871","updated":"2021-10-13 18:23:58.000000000","message":"Maybe it should?","commit_id":"5557baa12ceeb27bac1d127f7af94bd2ccc571eb"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"37e0457fad73c41f2c1a81a344d9bb9ad7d07b08","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7ccdfa75_638d3418","in_reply_to":"b4ba9238_1e7e66fa","updated":"2021-11-23 15:04:39.000000000","message":"++ That is what I think makes the most sense, since some operators will want/need this, others could care less in the grand scheme of the universe.","commit_id":"5557baa12ceeb27bac1d127f7af94bd2ccc571eb"},{"author":{"_account_id":2033,"name":"Belmiro Moreira","email":"moreira.belmiro.email.lists@gmail.com","username":"moreira-belmiro-email-lists"},"change_message_id":"b0a3f16036acfafad108047a989891cabb335cef","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"76bca29a_daac22f8","updated":"2021-12-02 13:31:27.000000000","message":"LGTM.\nClearly there are 2 different use cases.\nThe config option gives the required flexibility to the operator.\n\nThanks Steve.","commit_id":"6996c52979991ee12e32df191627dad4971aef1e"},{"author":{"_account_id":11292,"name":"Arne Wiebalck","email":"Arne.Wiebalck@cern.ch","username":"wiebalck"},"change_message_id":"1a2aafec749bdd52a87fcc7dedc28cdd001a7c4c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f640774b_5cde2c2e","updated":"2021-12-02 07:51:16.000000000","message":"This would cover our use case and provides the flexibility for others operators to chose the behavior which they think is best for their environment.","commit_id":"6996c52979991ee12e32df191627dad4971aef1e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d091744d92b8922c02a5eee3bef262275559188d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f48e77d6_91a80523","updated":"2022-01-15 01:11:07.000000000","message":"I like the overall concept here, the use case makes a lot of sense. But as gibi noted on the bug, this approach could potentially hold the user\u0027s neutron, cinder, and nova resources for hours (if cleaning can take hours).","commit_id":"a69d09888a8a797543d8073e6a1ffb621af3fbcc"}],"nova/virt/ironic/driver.py":[{"author":{"_account_id":11292,"name":"Arne Wiebalck","email":"Arne.Wiebalck@cern.ch","username":"wiebalck"},"change_message_id":"eabacbf41ebb6688eb18c22933638d36b9ad6868","unresolved":true,"context_lines":[{"line_number":1330,"context_line":"                raise loopingcall.LoopingCallDone()"},{"line_number":1331,"context_line":""},{"line_number":1332,"context_line":"            if node.provision_state \u003d\u003d ironic_states.CLEANFAIL:"},{"line_number":1333,"context_line":"                # ironic failed to deploy"},{"line_number":1334,"context_line":"                msg \u003d (_(\"Failed to unprovision instance %(inst)s: %(reason)s\")"},{"line_number":1335,"context_line":"                    % {\u0027inst\u0027: instance.uuid, \u0027reason\u0027: node.last_error})"},{"line_number":1336,"context_line":"                raise exception.InstanceTerminationFailure(msg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"86215426_3a650025","line":1333,"range":{"start_line":1333,"start_character":35,"end_line":1333,"end_character":41},"updated":"2021-10-13 06:14:47.000000000","message":"Nit: \"# ironic failed to delete/clean\"","commit_id":"5557baa12ceeb27bac1d127f7af94bd2ccc571eb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d091744d92b8922c02a5eee3bef262275559188d","unresolved":true,"context_lines":[{"line_number":1317,"context_line":"            # point since we will have already done it. The destroy will only"},{"line_number":1318,"context_line":"            # succeed if this method returns without error, so we will end up"},{"line_number":1319,"context_line":"            # removing the instance info eventually."},{"line_number":1320,"context_line":"            self._cleanup_deploy(node, instance, network_info,"},{"line_number":1321,"context_line":"                                 remove_instance_info\u003dFalse)"},{"line_number":1322,"context_line":""},{"line_number":1323,"context_line":"        def _wait_for_cleaning():"},{"line_number":1324,"context_line":"            instance.refresh()"}],"source_content_type":"text/x-python","patch_set":3,"id":"b133c5df_49debd87","line":1321,"range":{"start_line":1320,"start_character":12,"end_line":1321,"end_character":60},"updated":"2022-01-15 01:11:07.000000000","message":"In comment 9 on the bug [1], the ordering of network and volume teardown is mentioned with the concern being if waiting for cleaning would hold up release of network and volume resources. This ^ is where the volume targets are removed and the vifs unplugged per comment 9.\n\nFrom the compute manager perspective, as mentioned in comment 7 on the bug [2], the wait for cleaning would occur before [3] releasing resources in neutron [4] and cinder [5] and also before the instance record is deleted [6]. And quota for the respective resources won\u0027t be released until after cleaning is finished.\n\n[1] https://bugs.launchpad.net/nova/+bug/1884217/comments/9\n[2] https://bugs.launchpad.net/nova/+bug/1884217/comments/7\n[3] https://github.com/openstack/nova/blob/d5b6412ef52b1e5ad797a49850c9c6701b0405db/nova/compute/manager.py#L2903\n[4] https://github.com/openstack/nova/blob/d5b6412ef52b1e5ad797a49850c9c6701b0405db/nova/compute/manager.py#L2920\n[5] https://github.com/openstack/nova/blob/d5b6412ef52b1e5ad797a49850c9c6701b0405db/nova/compute/manager.py#L2923-L2936\n[6] https://github.com/openstack/nova/blob/d5b6412ef52b1e5ad797a49850c9c6701b0405db/nova/compute/manager.py#L3057","commit_id":"a69d09888a8a797543d8073e6a1ffb621af3fbcc"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d091744d92b8922c02a5eee3bef262275559188d","unresolved":true,"context_lines":[{"line_number":1340,"context_line":""},{"line_number":1341,"context_line":"        if CONF.ironic.delete_wait_for_cleaning:"},{"line_number":1342,"context_line":"            timer \u003d loopingcall.FixedIntervalLoopingCall(_wait_for_cleaning)"},{"line_number":1343,"context_line":"            timer.start(interval\u003dCONF.ironic.api_retry_interval).wait()"},{"line_number":1344,"context_line":""},{"line_number":1345,"context_line":"        LOG.info(\u0027Successfully unprovisioned Ironic node %s\u0027,"},{"line_number":1346,"context_line":"                 node.uuid, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":3,"id":"0cd95562_52093706","line":1343,"updated":"2022-01-15 01:11:07.000000000","message":"AFAICT if the node never changes to AVAILABLE or CLEANFAIL, this will loop indefinitely and hold up the nova-compute delete path indefinitely. I understand that cleaning can take quite a long time, but maybe a configurable timeout for this retry should be added? If for some reason we don\u0027t want to put a timeout here, I think we should add a note or warning to the config option help text to say that the wait for cleaning will never timeout.","commit_id":"a69d09888a8a797543d8073e6a1ffb621af3fbcc"}]}