)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"040aa5cfa3d1958d5770d03770bb1856456eb66c","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Lucas Alvares Gomes \u003clucasagomes@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-07-22 10:10:22 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Lazy delete instances"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This blueprint adds support for deleting an instance at any stage of"},{"line_number":10,"context_line":"the deployment."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3a50d1a3_3ecd2c66","line":7,"updated":"2015-07-22 10:22:01.000000000","message":"commit message is a bit confusing, what about \"Allow \u0027deleted\u0027 provision verb in \u0027DEPLOYING\u0027 state\" or anything more specific?","commit_id":"655b284ee8906fbc57b0d7bfe20e16dd1cecc6eb"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"046e956d9ccb551c2a094d05b5c02a71bdd730a1","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Lucas Alvares Gomes \u003clucasagomes@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-07-22 10:10:22 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Lazy delete instances"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This blueprint adds support for deleting an instance at any stage of"},{"line_number":10,"context_line":"the deployment."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3a50d1a3_f9b7de37","line":7,"in_reply_to":"3a50d1a3_3ecd2c66","updated":"2015-07-22 10:41:42.000000000","message":"Done","commit_id":"655b284ee8906fbc57b0d7bfe20e16dd1cecc6eb"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"3739851aeea08a3d792885caa870e64f3e354826","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Lucas Alvares Gomes \u003clucasagomes@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-07-22 10:10:22 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Lazy delete instances"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This blueprint adds support for deleting an instance at any stage of"},{"line_number":10,"context_line":"the deployment."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3a50d1a3_99b9fa7b","line":7,"in_reply_to":"3a50d1a3_f9b7de37","updated":"2015-07-22 10:46:14.000000000","message":"Actually that\u0027s the title of the spec/blueprint. Hmm it\u0027s hard to synthesize it in few words.\n\nI will improve the commit message body","commit_id":"655b284ee8906fbc57b0d7bfe20e16dd1cecc6eb"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"6a7e2b4aa18a0909a1155a20e3ca1b90fbe1128a","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Lucas Alvares Gomes \u003clucasagomes@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-07-29 13:55:06 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Lazy delete instances"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Nova expects to be able to delete an instance even when it\u0027s spawning"},{"line_number":10,"context_line":"but in Ironic only nodes in DEPLOYWAIT state are allowed to be aborted"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"1a4dcd0f_503887d3","line":7,"updated":"2015-08-04 12:37:10.000000000","message":"nit: I still find the title very confusing... Why not call it \"Deleting instances on any stage of deployment\" like you have in summary?","commit_id":"b83ef587445321329e6777cb469156a016c42710"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"86cbb161c991d2fbb160af554fcf5759ca778535","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Lucas Alvares Gomes \u003clucasagomes@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-07-29 13:55:06 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Lazy delete instances"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Nova expects to be able to delete an instance even when it\u0027s spawning"},{"line_number":10,"context_line":"but in Ironic only nodes in DEPLOYWAIT state are allowed to be aborted"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"1a4dcd0f_70826302","line":7,"in_reply_to":"1a4dcd0f_503887d3","updated":"2015-08-05 11:26:08.000000000","message":"+1 \n\nYeah I wanted to summarize it in a word or so but it\u0027s hard. I will update it","commit_id":"b83ef587445321329e6777cb469156a016c42710"}],"specs/approved/lazy-delete-instances.rst":[{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"748b64dfddbad45e569244788b0cf42edfee19db","unresolved":false,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":"A new field ``delete_instance`` will be added to the Node table. This"},{"line_number":96,"context_line":"field will hold a boolean value indicating whether the deployment should"},{"line_number":97,"context_line":"be lazyly deleted or not."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"State Machine Impact"},{"line_number":100,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":9,"id":"1a4dcd0f_d292d2f5","line":97,"updated":"2015-08-06 16:46:56.000000000","message":"Nit s/lazyly/lazily/\n\nI assume this defaults to False, but should probably mention it if you push another patchset.","commit_id":"8d3d57f3327ec7fe738b989ab251b493425b7d19"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"2fae55609cfdaf2b4be79cbd657eb6843c05f9b9","unresolved":false,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":"A new field ``delete_instance`` will be added to the Node table. This"},{"line_number":96,"context_line":"field will hold a boolean value indicating whether the deployment should"},{"line_number":97,"context_line":"be lazyly deleted or not."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"State Machine Impact"},{"line_number":100,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":9,"id":"1a4dcd0f_fd8a3f63","line":97,"in_reply_to":"1a4dcd0f_d292d2f5","updated":"2015-08-06 16:53:01.000000000","message":"Done","commit_id":"8d3d57f3327ec7fe738b989ab251b493425b7d19"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"748b64dfddbad45e569244788b0cf42edfee19db","unresolved":false,"context_lines":[{"line_number":206,"context_line":"* Allow the verb ``deleted`` to nodes in ``DEPLOYING`` state."},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"* Create a periodic task that will be responsible for deleting the"},{"line_number":209,"context_line":"  instances asynchronously."},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"* Change the Nova driver to not wait on the provision state of the node"},{"line_number":212,"context_line":"  when deleting an instance."}],"source_content_type":"text/x-rst","patch_set":9,"id":"1a4dcd0f_b23706fe","line":209,"updated":"2015-08-06 16:46:56.000000000","message":"This should probably come before allowing deleted on nodes in DEPLOYING state. :)","commit_id":"8d3d57f3327ec7fe738b989ab251b493425b7d19"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"2fae55609cfdaf2b4be79cbd657eb6843c05f9b9","unresolved":false,"context_lines":[{"line_number":206,"context_line":"* Allow the verb ``deleted`` to nodes in ``DEPLOYING`` state."},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"* Create a periodic task that will be responsible for deleting the"},{"line_number":209,"context_line":"  instances asynchronously."},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"* Change the Nova driver to not wait on the provision state of the node"},{"line_number":212,"context_line":"  when deleting an instance."}],"source_content_type":"text/x-rst","patch_set":9,"id":"1a4dcd0f_bdefd71c","line":209,"in_reply_to":"1a4dcd0f_b23706fe","updated":"2015-08-06 16:53:01.000000000","message":"True! +1","commit_id":"8d3d57f3327ec7fe738b989ab251b493425b7d19"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"748b64dfddbad45e569244788b0cf42edfee19db","unresolved":false,"context_lines":[{"line_number":217,"context_line":"Dependencies"},{"line_number":218,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"* This patch depends on the blueprint `ipxe-dymic-config"},{"line_number":221,"context_line":"  \u003chttps://review.openstack.org/#/c/177726/\u003e`_ that makes the iPXE"},{"line_number":222,"context_line":"  configuration files to be dynamically generated."},{"line_number":223,"context_line":""}],"source_content_type":"text/x-rst","patch_set":9,"id":"1a4dcd0f_72b89e75","line":220,"updated":"2015-08-06 16:46:56.000000000","message":"Nit s/dymic/dynamic/\n\nAlso, this isn\u0027t actually a dependency, right? I assume this is copy/paste fail.","commit_id":"8d3d57f3327ec7fe738b989ab251b493425b7d19"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"2fae55609cfdaf2b4be79cbd657eb6843c05f9b9","unresolved":false,"context_lines":[{"line_number":217,"context_line":"Dependencies"},{"line_number":218,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"* This patch depends on the blueprint `ipxe-dymic-config"},{"line_number":221,"context_line":"  \u003chttps://review.openstack.org/#/c/177726/\u003e`_ that makes the iPXE"},{"line_number":222,"context_line":"  configuration files to be dynamically generated."},{"line_number":223,"context_line":""}],"source_content_type":"text/x-rst","patch_set":9,"id":"1a4dcd0f_bd3d9790","line":220,"in_reply_to":"1a4dcd0f_72b89e75","updated":"2015-08-06 16:53:01.000000000","message":"Oh yeah it doesn\u0027t. I used the spec that uses swift tempurls as base to write this one It came from that.\n\nThanks for noticing!","commit_id":"8d3d57f3327ec7fe738b989ab251b493425b7d19"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"748b64dfddbad45e569244788b0cf42edfee19db","unresolved":false,"context_lines":[{"line_number":238,"context_line":""},{"line_number":239,"context_line":"For new version of the Nova Ironic driver and old versions of Ironic,"},{"line_number":240,"context_line":"deleting a node in ``DEPLOYING`` state will fail making the ``nova"},{"line_number":241,"context_line":"delete`` command to fail."},{"line_number":242,"context_line":""},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"Documentation Impact"}],"source_content_type":"text/x-rst","patch_set":9,"id":"1a4dcd0f_5da49306","line":241,"updated":"2015-08-06 16:46:56.000000000","message":"So this is a change in behavior... perhaps we can try/except this so that the instance is still deleted in nova? We\u0027d have to call _cleanup_deploy, still, too.\n\nSo something like:\n\n    try:\n        _unprovision()\n    except Conflict:\n        # here to handle backwards compat with older ironic\n        if node.provision_state \u003d\u003d DEPLOYING:\n            self._cleanup_deploy()","commit_id":"8d3d57f3327ec7fe738b989ab251b493425b7d19"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"2fae55609cfdaf2b4be79cbd657eb6843c05f9b9","unresolved":false,"context_lines":[{"line_number":238,"context_line":""},{"line_number":239,"context_line":"For new version of the Nova Ironic driver and old versions of Ironic,"},{"line_number":240,"context_line":"deleting a node in ``DEPLOYING`` state will fail making the ``nova"},{"line_number":241,"context_line":"delete`` command to fail."},{"line_number":242,"context_line":""},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"Documentation Impact"}],"source_content_type":"text/x-rst","patch_set":9,"id":"1a4dcd0f_bd8ef72b","line":241,"in_reply_to":"1a4dcd0f_5da49306","updated":"2015-08-06 16:53:01.000000000","message":"This is if we use a new version of nova with an old version of Ironic? So it will raise conflict? \n\nTo be very honest with you, yes that\u0027s a behavior change but IMHO it would be better to raise Conflict and stop the instance deletion (marking the instance as deletion failed) instead of just call _cleanup_deploy().\n\nBecause if we don\u0027t tell Ironic to unprovision the instance and we clean up it in nova (delete instance_uuid, VIFs etc...) this will leave the ACTIVE nodes orphan in Ironic.","commit_id":"8d3d57f3327ec7fe738b989ab251b493425b7d19"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"260f1a61629f2fe9aa813bec105b8e68bcd146a0","unresolved":false,"context_lines":[{"line_number":238,"context_line":""},{"line_number":239,"context_line":"For new version of the Nova Ironic driver and old versions of Ironic,"},{"line_number":240,"context_line":"deleting a node in ``DEPLOYING`` state will fail making the ``nova"},{"line_number":241,"context_line":"delete`` command to fail."},{"line_number":242,"context_line":""},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"Documentation Impact"}],"source_content_type":"text/x-rst","patch_set":9,"id":"1a4dcd0f_fdf6bf5b","line":241,"in_reply_to":"1a4dcd0f_bd8ef72b","updated":"2015-08-06 16:57:45.000000000","message":"Discussed in IRC, this is going to be changed:\n\n    16:53:30     lucasagomes | jroll, it\u0027s all good... Commented there. Re that behavior change when it conflicts on _unprovision()\n    16:53:39     lucasagomes | IMHO I would prefer it to conflict and stop the instance deletion\n    16:53:49     lucasagomes | because that\u0027s exactly what would leave orphan nodes in Ironic \n    16:53:53     lucasagomes | it\u0027s a bug as I see it\n    16:54:10     lucasagomes | if we just ignore the conflict (no set-provision-state deleted was called) but we delete the instance in nova\n    16:54:29     lucasagomes | that will leave ACTIVE nodes in Ironic without any instance associated in Nova   \n    16:54:31           jroll | lucasagomes: right, so what I\u0027ve learned recently\n    16:54:46           jroll | is that most nova users, when they see an instance in ERROR, they panic and call support\n    16:54:52           jroll | don\u0027t read the error message or anything\n    16:54:56     lucasagomes | oh\n    16:55:35           jroll | also, an undeleted instance may trigger billing things, obviously depending on the cloud\n    16:55:55           jroll | so I\u0027d rather orphan it, I think, it\u0027s easily detectable for alerts or whatever\n    16:55:59           jroll | we actually alert on this\n    16:56:03     lucasagomes | right... so it\u0027s better to clean up the ondes in Ironic if it happens\n    16:56:16     lucasagomes | jroll, ok... right yeah I will keep the behavior\n    16:56:21           jroll | yeah, we just kick them manually\n    16:56:23           jroll | cool, thanks\n    16:56:26     lucasagomes | but probably fix it as a separeted issue\n    16:56:28               * | jroll pastes this in a comment\n    16:56:33     lucasagomes | cool thanks","commit_id":"8d3d57f3327ec7fe738b989ab251b493425b7d19"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"748b64dfddbad45e569244788b0cf42edfee19db","unresolved":false,"context_lines":[{"line_number":244,"context_line":"Documentation Impact"},{"line_number":245,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"The documentation will add explaining for deployers how the lazyly"},{"line_number":248,"context_line":"deletion of an instance works."},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"References"}],"source_content_type":"text/x-rst","patch_set":9,"id":"1a4dcd0f_ddb8a32c","line":247,"updated":"2015-08-06 16:46:56.000000000","message":"Again s/lazyly/lazily","commit_id":"8d3d57f3327ec7fe738b989ab251b493425b7d19"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"2fae55609cfdaf2b4be79cbd657eb6843c05f9b9","unresolved":false,"context_lines":[{"line_number":244,"context_line":"Documentation Impact"},{"line_number":245,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"The documentation will add explaining for deployers how the lazyly"},{"line_number":248,"context_line":"deletion of an instance works."},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"References"}],"source_content_type":"text/x-rst","patch_set":9,"id":"1a4dcd0f_3d718776","line":247,"in_reply_to":"1a4dcd0f_ddb8a32c","updated":"2015-08-06 16:53:01.000000000","message":"Done","commit_id":"8d3d57f3327ec7fe738b989ab251b493425b7d19"},{"author":{"_account_id":9315,"name":"ramesh","email":"rameshg87@gmail.com","username":"rameshg87"},"change_message_id":"d926103aaa334245873b4c5e1e54d4d1a24f85b4","unresolved":false,"context_lines":[{"line_number":38,"context_line":"lock prevents any race condition with the ramdisk talking back to Ironic"},{"line_number":39,"context_line":"to continue the deployment)."},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"For nodes in ``DEPLOYING`` state the deletion of the instance should"},{"line_number":42,"context_line":"happen asynchronously. We first need to mark the node in the database to"},{"line_number":43,"context_line":"say that the instance will be deleted and then have a periodic task that"},{"line_number":44,"context_line":"query for those nodes and perform the deletion of the instance once no"}],"source_content_type":"text/x-rst","patch_set":10,"id":"1a4dcd0f_3c9e0408","line":41,"updated":"2015-08-10 06:12:24.000000000","message":"There is a small window of gap when a lock is acquired for a node in DEPLOYWAIT as well (from the moment vendor passthru is initiated to the point we do task.process_event(\u0027resume\u0027)).  So, I think we need to do the same for nodes in DEPLOYWAIT as well if we failed to acquire the lock.","commit_id":"53ec8cdbe1ddced2f5938578e08d3b01ed70c8af"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"6e0e7cb0dfb831b9b43b5f2912ef109c710875b7","unresolved":false,"context_lines":[{"line_number":38,"context_line":"lock prevents any race condition with the ramdisk talking back to Ironic"},{"line_number":39,"context_line":"to continue the deployment)."},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"For nodes in ``DEPLOYING`` state the deletion of the instance should"},{"line_number":42,"context_line":"happen asynchronously. We first need to mark the node in the database to"},{"line_number":43,"context_line":"say that the instance will be deleted and then have a periodic task that"},{"line_number":44,"context_line":"query for those nodes and perform the deletion of the instance once no"}],"source_content_type":"text/x-rst","patch_set":10,"id":"1a4dcd0f_964b622b","line":41,"in_reply_to":"1a4dcd0f_3c9e0408","updated":"2015-08-10 13:31:36.000000000","message":"Yeah that\u0027s true probably we could say something like:\n\n\"If we can\u0027t acquire a lock to delete the node at the time the request to delete the instance comes it will be marked and deleted later\" i.e\n\n try:\n     node.acquire()\n except NodeLocked:\n    delete_instance  \u003d True\n    return\n\nI just used the states here DEPLOYING and DEPLOYWAIT because that\u0027s the states exposed in the API, where the lock is kinda an internal flag (tho it\u0027s leaked in the API \"reservation\" field).","commit_id":"53ec8cdbe1ddced2f5938578e08d3b01ed70c8af"},{"author":{"_account_id":9315,"name":"ramesh","email":"rameshg87@gmail.com","username":"rameshg87"},"change_message_id":"b6142a3250f4d49a002dda2f5f2e16b9da23f4dd","unresolved":false,"context_lines":[{"line_number":38,"context_line":"lock prevents any race condition with the ramdisk talking back to Ironic"},{"line_number":39,"context_line":"to continue the deployment)."},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"For nodes in ``DEPLOYING`` state the deletion of the instance should"},{"line_number":42,"context_line":"happen asynchronously. We first need to mark the node in the database to"},{"line_number":43,"context_line":"say that the instance will be deleted and then have a periodic task that"},{"line_number":44,"context_line":"query for those nodes and perform the deletion of the instance once no"}],"source_content_type":"text/x-rst","patch_set":10,"id":"1a4dcd0f_603b4c39","line":41,"in_reply_to":"1a4dcd0f_964b622b","updated":"2015-08-11 05:04:43.000000000","message":"+1","commit_id":"53ec8cdbe1ddced2f5938578e08d3b01ed70c8af"},{"author":{"_account_id":9315,"name":"ramesh","email":"rameshg87@gmail.com","username":"rameshg87"},"change_message_id":"d926103aaa334245873b4c5e1e54d4d1a24f85b4","unresolved":false,"context_lines":[{"line_number":41,"context_line":"For nodes in ``DEPLOYING`` state the deletion of the instance should"},{"line_number":42,"context_line":"happen asynchronously. We first need to mark the node in the database to"},{"line_number":43,"context_line":"say that the instance will be deleted and then have a periodic task that"},{"line_number":44,"context_line":"query for those nodes and perform the deletion of the instance once no"},{"line_number":45,"context_line":"locks are being held for that node. So if the node is in ``DEPLOYING``"},{"line_number":46,"context_line":"state (which already holds an exclusive lock) it\u0027s possible that the node"},{"line_number":47,"context_line":"become ``ACTIVE`` before the deletion happens. If the deployment of the"}],"source_content_type":"text/x-rst","patch_set":10,"id":"1a4dcd0f_1c7468cd","line":44,"updated":"2015-08-10 06:12:24.000000000","message":"I think it will be useful to mention the query filters here.  I think periodic task will query for nodes in states.ACTIVE, states.DEPLOYFAIL and states.DEPLOYWAIT with the flag node.delete_instance set to True. Right ?","commit_id":"53ec8cdbe1ddced2f5938578e08d3b01ed70c8af"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"6e0e7cb0dfb831b9b43b5f2912ef109c710875b7","unresolved":false,"context_lines":[{"line_number":41,"context_line":"For nodes in ``DEPLOYING`` state the deletion of the instance should"},{"line_number":42,"context_line":"happen asynchronously. We first need to mark the node in the database to"},{"line_number":43,"context_line":"say that the instance will be deleted and then have a periodic task that"},{"line_number":44,"context_line":"query for those nodes and perform the deletion of the instance once no"},{"line_number":45,"context_line":"locks are being held for that node. So if the node is in ``DEPLOYING``"},{"line_number":46,"context_line":"state (which already holds an exclusive lock) it\u0027s possible that the node"},{"line_number":47,"context_line":"become ``ACTIVE`` before the deletion happens. If the deployment of the"}],"source_content_type":"text/x-rst","patch_set":10,"id":"1a4dcd0f_3315c09a","line":44,"in_reply_to":"1a4dcd0f_1c7468cd","updated":"2015-08-10 13:31:36.000000000","message":"I was thinking about only filter by nodes that have \"reservation \u003d None\" and \"delete_instance \u003d True\". Cause that will cover all those states mentioned.","commit_id":"53ec8cdbe1ddced2f5938578e08d3b01ed70c8af"},{"author":{"_account_id":9315,"name":"ramesh","email":"rameshg87@gmail.com","username":"rameshg87"},"change_message_id":"b6142a3250f4d49a002dda2f5f2e16b9da23f4dd","unresolved":false,"context_lines":[{"line_number":41,"context_line":"For nodes in ``DEPLOYING`` state the deletion of the instance should"},{"line_number":42,"context_line":"happen asynchronously. We first need to mark the node in the database to"},{"line_number":43,"context_line":"say that the instance will be deleted and then have a periodic task that"},{"line_number":44,"context_line":"query for those nodes and perform the deletion of the instance once no"},{"line_number":45,"context_line":"locks are being held for that node. So if the node is in ``DEPLOYING``"},{"line_number":46,"context_line":"state (which already holds an exclusive lock) it\u0027s possible that the node"},{"line_number":47,"context_line":"become ``ACTIVE`` before the deletion happens. If the deployment of the"}],"source_content_type":"text/x-rst","patch_set":10,"id":"1a4dcd0f_60502c5f","line":44,"in_reply_to":"1a4dcd0f_3315c09a","updated":"2015-08-11 05:04:43.000000000","message":"Okay.","commit_id":"53ec8cdbe1ddced2f5938578e08d3b01ed70c8af"},{"author":{"_account_id":9315,"name":"ramesh","email":"rameshg87@gmail.com","username":"rameshg87"},"change_message_id":"d926103aaa334245873b4c5e1e54d4d1a24f85b4","unresolved":false,"context_lines":[{"line_number":55,"context_line":"whether the instance should be deleted or not. This field should also"},{"line_number":56,"context_line":"be updated regardless of the node be holding a lock or not (nodes in"},{"line_number":57,"context_line":"``DEPLOYING`` state does hold a lock). Also, to be safe this field will"},{"line_number":58,"context_line":"be cleaned up at node tear down, just like ``instance_info``."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"In the Nova perspective a successful call to delete whether it happens"},{"line_number":61,"context_line":"immediately (``DEPLOYWAIT``) or eventually (``DEPLOYING``) is enough to"}],"source_content_type":"text/x-rst","patch_set":10,"id":"1a4dcd0f_bc2e94a3","line":58,"updated":"2015-08-10 06:12:24.000000000","message":"What if we end up seeing delete_instance set to True for a node that is in states.AVAILABLE (some race condition that we missed).  Will the periodic task set to False or will it be set to False when the deployment is initiated ? I think the safer bet is the second one, to set this to False when deployment is initiated.","commit_id":"53ec8cdbe1ddced2f5938578e08d3b01ed70c8af"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"c2ddb2e0193adb9f46563dadac720d78c89ef903","unresolved":false,"context_lines":[{"line_number":55,"context_line":"whether the instance should be deleted or not. This field should also"},{"line_number":56,"context_line":"be updated regardless of the node be holding a lock or not (nodes in"},{"line_number":57,"context_line":"``DEPLOYING`` state does hold a lock). Also, to be safe this field will"},{"line_number":58,"context_line":"be cleaned up at node tear down, just like ``instance_info``."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"In the Nova perspective a successful call to delete whether it happens"},{"line_number":61,"context_line":"immediately (``DEPLOYWAIT``) or eventually (``DEPLOYING``) is enough to"}],"source_content_type":"text/x-rst","patch_set":10,"id":"1a4dcd0f_a52639b8","line":58,"in_reply_to":"1a4dcd0f_8b6eb9c7","updated":"2015-08-11 08:53:21.000000000","message":"Talked on IRC and agreed to have the periodic task to check if the node is not in the state it can be deleted and if so we clean up that flag (auto-healing even if the db was changed manually for e.g)","commit_id":"53ec8cdbe1ddced2f5938578e08d3b01ed70c8af"},{"author":{"_account_id":9315,"name":"ramesh","email":"rameshg87@gmail.com","username":"rameshg87"},"change_message_id":"b6142a3250f4d49a002dda2f5f2e16b9da23f4dd","unresolved":false,"context_lines":[{"line_number":55,"context_line":"whether the instance should be deleted or not. This field should also"},{"line_number":56,"context_line":"be updated regardless of the node be holding a lock or not (nodes in"},{"line_number":57,"context_line":"``DEPLOYING`` state does hold a lock). Also, to be safe this field will"},{"line_number":58,"context_line":"be cleaned up at node tear down, just like ``instance_info``."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"In the Nova perspective a successful call to delete whether it happens"},{"line_number":61,"context_line":"immediately (``DEPLOYWAIT``) or eventually (``DEPLOYING``) is enough to"}],"source_content_type":"text/x-rst","patch_set":10,"id":"1a4dcd0f_8b6eb9c7","line":58,"in_reply_to":"1a4dcd0f_b6353edc","updated":"2015-08-11 05:04:43.000000000","message":"Yeah, I agree. But since we are not acquiring (or cannot acquire) the lock on the node, we are not double sure about the state of the node.  \n\nFor example, image if the node is in DEPLOYWAIT and we make the same API request to set the target state to deleted.  One possibility is the following sequence of events:\n\nThread 1 - Finds node is in DEPLOYWAIT, acquires the lock to do the tear down, but hasn\u0027t changed the state of node in the DB, context switches at this point of time.\n\nThread 2- Find node is in DEPLOYWAIT, tries to acquire the lock, fails, so goes ahead to put delete_instances\u003dTrue, but doesn\u0027t complete it and context switches at this point of time.\n\nThread 1 - Does the tear down, clears up the instance info and move the node to available\n\nThread 2 - Sets delete_instance to True for the available node.\n\nEventhough our threading model with greenlet is co-operative and eventlet patching makes sure we should be context switching whenever we block on a system call, but I don\u0027t know if it\u0027s guaranteed that above flow cannot occur.  \n\nEven the above flow looks only like a distant possibility (or impossible), updating node without locks is dangerous and any measures to prevent any unintended behaviour should be guarded.\n\nThoughts ?","commit_id":"53ec8cdbe1ddced2f5938578e08d3b01ed70c8af"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"6e0e7cb0dfb831b9b43b5f2912ef109c710875b7","unresolved":false,"context_lines":[{"line_number":55,"context_line":"whether the instance should be deleted or not. This field should also"},{"line_number":56,"context_line":"be updated regardless of the node be holding a lock or not (nodes in"},{"line_number":57,"context_line":"``DEPLOYING`` state does hold a lock). Also, to be safe this field will"},{"line_number":58,"context_line":"be cleaned up at node tear down, just like ``instance_info``."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"In the Nova perspective a successful call to delete whether it happens"},{"line_number":61,"context_line":"immediately (``DEPLOYWAIT``) or eventually (``DEPLOYING``) is enough to"}],"source_content_type":"text/x-rst","patch_set":10,"id":"1a4dcd0f_b6353edc","line":58,"in_reply_to":"1a4dcd0f_bc2e94a3","updated":"2015-08-10 13:31:36.000000000","message":"Hmm I don\u0027t see how it would be possible tho.\n\nThe \"delete_instance\" field will be cleaned together with \"instance_info\" [1] and others. At the time no more request to delete the node is accepted by the API (node will be at DELETING provision state, so it won\u0027t have a race marking the node to be deleted later). \n\nThe only way I see it happening is if one actually changes the database. So I\u0027m not addressing this point here (not think we should).\n\nWDYT?\n\n[1] https://github.com/openstack/ironic/blob/master/ironic/conductor/manager.py#L805-L819","commit_id":"53ec8cdbe1ddced2f5938578e08d3b01ed70c8af"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"658819607b72f2df4036ebfad235539d1a85976f","unresolved":false,"context_lines":[{"line_number":8,"context_line":"Deleting instances at any stage of the deployment"},{"line_number":9,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"https://blueprints.launchpad.net/ironic/+spec/lazy-delete-instances"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This blueprint adds support for deleting instances at any stage of"},{"line_number":14,"context_line":"the deployment."}],"source_content_type":"text/x-rst","patch_set":11,"id":"3a7e1126_378aa4c0","line":11,"updated":"2015-12-17 12:11:14.000000000","message":"Hi, when you\u0027re going to update this, please change this link to RFE bug 1526481 and use it to track progress.","commit_id":"2d98d9617d95d4df5e408ab0bd939c5a8f2cd6c6"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"dad3c2b4e5d30b1b4068a8376314854e6a4c78b7","unresolved":false,"context_lines":[{"line_number":29,"context_line":"we allow it to be passed to a node in ``DEPLOYWAIT`` state."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"The way the instance is deleted depends on the provision state of the"},{"line_number":32,"context_line":"node. Ironic already supports deleting instances when nodes are is not"},{"line_number":33,"context_line":"holding a lock (``DEPLOYWAIT`` state), so that won\u0027t change. In this case,"},{"line_number":34,"context_line":"what it does is pretty straight forward. Since they hold no locks nor"},{"line_number":35,"context_line":"there\u0027s a worker thread running in the ``ironic-conductor`` executing"}],"source_content_type":"text/x-rst","patch_set":11,"id":"1a4dcd0f_3c155248","line":32,"updated":"2015-08-11 13:41:57.000000000","message":"nit: \"nodes are is not holding a lock\" -\u003e \"a node is not...\" or \"nodes are not...\"","commit_id":"2d98d9617d95d4df5e408ab0bd939c5a8f2cd6c6"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"450fb1836a28a7f4220b6d71d4416f1166fef76c","unresolved":false,"context_lines":[{"line_number":29,"context_line":"we allow it to be passed to a node in ``DEPLOYWAIT`` state."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"The way the instance is deleted depends on the provision state of the"},{"line_number":32,"context_line":"node. Ironic already supports deleting instances when nodes are is not"},{"line_number":33,"context_line":"holding a lock (``DEPLOYWAIT`` state), so that won\u0027t change. In this case,"},{"line_number":34,"context_line":"what it does is pretty straight forward. Since they hold no locks nor"},{"line_number":35,"context_line":"there\u0027s a worker thread running in the ``ironic-conductor`` executing"}],"source_content_type":"text/x-rst","patch_set":11,"id":"1a4dcd0f_549cb2bf","line":32,"in_reply_to":"1a4dcd0f_3c155248","updated":"2015-08-11 14:10:00.000000000","message":":-( typo...\n\nnodes are not","commit_id":"2d98d9617d95d4df5e408ab0bd939c5a8f2cd6c6"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"addb7083ecd5ca0213fc5fc6a56b4cf4b794eeea","unresolved":false,"context_lines":[{"line_number":53,"context_line":"this spec proposes adding a new field to the node object called"},{"line_number":54,"context_line":"``delete_instance``. The value of the field is a boolean, indicating"},{"line_number":55,"context_line":"whether the instance should be deleted or not. This field should also"},{"line_number":56,"context_line":"be updated regardless of the node be holding a lock or not (nodes in"},{"line_number":57,"context_line":"``DEPLOYING`` state does hold a lock). Also, to be safe this field will"},{"line_number":58,"context_line":"be cleaned up at node tear down, just like ``instance_info``."},{"line_number":59,"context_line":""}],"source_content_type":"text/x-rst","patch_set":11,"id":"1a4dcd0f_9d833156","line":56,"updated":"2015-08-11 17:56:32.000000000","message":"How about something like \u0027Note that this field will be updated even if the task doesn\u0027t have an exclusive lock on it (eg when another task has an exclusive lock on the node while the node is in ``DEPLOYING`` state.\u0027","commit_id":"2d98d9617d95d4df5e408ab0bd939c5a8f2cd6c6"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"eea6821bac03dc64b81c22756207dc819a13712f","unresolved":false,"context_lines":[{"line_number":53,"context_line":"this spec proposes adding a new field to the node object called"},{"line_number":54,"context_line":"``delete_instance``. The value of the field is a boolean, indicating"},{"line_number":55,"context_line":"whether the instance should be deleted or not. This field should also"},{"line_number":56,"context_line":"be updated regardless of the node be holding a lock or not (nodes in"},{"line_number":57,"context_line":"``DEPLOYING`` state does hold a lock). Also, to be safe this field will"},{"line_number":58,"context_line":"be cleaned up at node tear down, just like ``instance_info``."},{"line_number":59,"context_line":""}],"source_content_type":"text/x-rst","patch_set":11,"id":"fa1b9901_f79a17c5","line":56,"in_reply_to":"1a4dcd0f_9d833156","updated":"2015-08-17 14:10:23.000000000","message":"Done","commit_id":"2d98d9617d95d4df5e408ab0bd939c5a8f2cd6c6"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"bd2261338978e1481f7c0b384818b242a7220a30","unresolved":false,"context_lines":[{"line_number":111,"context_line":"REST API impact"},{"line_number":112,"context_line":"---------------"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"We should expose the new ``delete_instance`` field in the node resources"},{"line_number":115,"context_line":"of the REST API. This field will be read-only, no updates will be allowed"},{"line_number":116,"context_line":"to it via PATCH. This change will also require an API microversion bump."},{"line_number":117,"context_line":""}],"source_content_type":"text/x-rst","patch_set":11,"id":"1a4dcd0f_6cdbe24f","line":114,"updated":"2015-08-12 16:58:59.000000000","message":"I do not like that this changes the API semantics for how a user determines what state a node is in.\n\nCurrently, a user looks at provision-state and target_provision_state. This proposal adds a new field when we really want to change the target state.\n\nWhat about this: allow the target state \"DELETING\" while the current state is \"DEPLOYING\"? This will introduce more complexity in the state machine and conductor handling of exiting from the DEPLOYING state (ie, the \"done\" event) but it keeps the API semantics (same two fields, same set of allowed states) the same.","commit_id":"2d98d9617d95d4df5e408ab0bd939c5a8f2cd6c6"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"eea6821bac03dc64b81c22756207dc819a13712f","unresolved":false,"context_lines":[{"line_number":111,"context_line":"REST API impact"},{"line_number":112,"context_line":"---------------"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"We should expose the new ``delete_instance`` field in the node resources"},{"line_number":115,"context_line":"of the REST API. This field will be read-only, no updates will be allowed"},{"line_number":116,"context_line":"to it via PATCH. This change will also require an API microversion bump."},{"line_number":117,"context_line":""}],"source_content_type":"text/x-rst","patch_set":11,"id":"fa1b9901_9adae277","line":114,"in_reply_to":"1a4dcd0f_04e9b37e","updated":"2015-08-17 14:10:23.000000000","message":"Yeah I will have to think about how o do that since it\u0027s not straight forward because of the way the global lock works.\n\nBut even sematically I don\u0027t think the flag is all bad? Because it\u0027s not actually lying, the flag is about \"delete this instance when possible\", so if a node is in \"target_provision_state \u003d ACTIVE\" and \"provision_state \u003d DEPLOYING\" , it will actually go to that target state first prior to the deletion start (since we can\u0027t/don\u0027t interrupt the deployment in the middle).","commit_id":"2d98d9617d95d4df5e408ab0bd939c5a8f2cd6c6"},{"author":{"_account_id":9315,"name":"ramesh","email":"rameshg87@gmail.com","username":"rameshg87"},"change_message_id":"30d755f294c91ab78825b05bd1617fea0729334a","unresolved":false,"context_lines":[{"line_number":111,"context_line":"REST API impact"},{"line_number":112,"context_line":"---------------"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"We should expose the new ``delete_instance`` field in the node resources"},{"line_number":115,"context_line":"of the REST API. This field will be read-only, no updates will be allowed"},{"line_number":116,"context_line":"to it via PATCH. This change will also require an API microversion bump."},{"line_number":117,"context_line":""}],"source_content_type":"text/x-rst","patch_set":11,"id":"1a4dcd0f_04e9b37e","line":114,"in_reply_to":"1a4dcd0f_35e9967f","updated":"2015-08-13 10:08:37.000000000","message":"Oh, a new lock for node\u0027s target_provision_state seems to require more changes in the db to hold the lock.\n\nAnother alternative is just return node.target_provision_state as DELETED in the API (I think it should be DELETED instead of DELETING), when\n\n  (node.provision_state \u003d\u003d states.DEPLOYWAIT or node.provision_state \u003d\u003d states.DEPLOYING) \n    and node.delete_instance \u003d True\n\nIt is safe to do this in my opinion because delete_instance is irreversible in the API and node will goto DELETING whatever be the first outcome of transition from DEPLOYING -\u003e (DEPLOYWAIT, AVAILABLE or DEPLOYFAIL).","commit_id":"2d98d9617d95d4df5e408ab0bd939c5a8f2cd6c6"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"d5dc218659dafac30acc5868677f7ec3f9246b49","unresolved":false,"context_lines":[{"line_number":111,"context_line":"REST API impact"},{"line_number":112,"context_line":"---------------"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"We should expose the new ``delete_instance`` field in the node resources"},{"line_number":115,"context_line":"of the REST API. This field will be read-only, no updates will be allowed"},{"line_number":116,"context_line":"to it via PATCH. This change will also require an API microversion bump."},{"line_number":117,"context_line":""}],"source_content_type":"text/x-rst","patch_set":11,"id":"1a4dcd0f_35e9967f","line":114,"in_reply_to":"1a4dcd0f_6cdbe24f","updated":"2015-08-12 20:27:30.000000000","message":"I didn\u0027t mind having a node.delete_instance field that is updated by a task that didn\u0027t have an exclusive lock on the node, but it seems scary to have a task modify the target_provision_state while another task has an exclusive lock on the node.\n\nOtherwise, I like the idea of changing target_provision_state.\n\nMaybe some sort of event-queue might make it more explicit to the user, so they can see what other actions might/will be done on the node.\n\nOr introduce a new lock for a node\u0027s target_provision_state?","commit_id":"2d98d9617d95d4df5e408ab0bd939c5a8f2cd6c6"}],"specs/liberty/lazy-delete-instances.rst":[{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"874213a1529299c8a958a2894dc25e1ec569c6cc","unresolved":false,"context_lines":[{"line_number":25,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"The propose implementation consists in allowing the **deleted** verb"},{"line_number":28,"context_line":"for provison state to be passed to a node in in ``DEPLOYWAIT`` or"},{"line_number":29,"context_line":"``DEPLOYING`` provision state."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"The way the instance is deleted depends on the state of the node. For"}],"source_content_type":"text/x-rst","patch_set":1,"id":"3a50d1a3_f3270fc8","line":28,"updated":"2015-07-22 02:14:56.000000000","message":"nits: s/provison/provision","commit_id":"ccf1f85ed77a463abd9e499bddc7a25fdf6b0483"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"d0e4a9e9ed557abbfe055390d6f758abbb22d348","unresolved":false,"context_lines":[{"line_number":25,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"The propose implementation consists in allowing the **deleted** verb"},{"line_number":28,"context_line":"for provison state to be passed to a node in in ``DEPLOYWAIT`` or"},{"line_number":29,"context_line":"``DEPLOYING`` provision state."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"The way the instance is deleted depends on the state of the node. For"}],"source_content_type":"text/x-rst","patch_set":1,"id":"3a50d1a3_3895473e","line":28,"in_reply_to":"3a50d1a3_f3270fc8","updated":"2015-07-22 08:57:17.000000000","message":"Done","commit_id":"ccf1f85ed77a463abd9e499bddc7a25fdf6b0483"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"874213a1529299c8a958a2894dc25e1ec569c6cc","unresolved":false,"context_lines":[{"line_number":32,"context_line":"nodes in ``DEPLOYWAIT`` provison state its pretty straight forward"},{"line_number":33,"context_line":"since they don\u0027t hold any locks nor there\u0027s a thread running in the"},{"line_number":34,"context_line":"``ironic-conductor`` executing the code for the deployment. So for"},{"line_number":35,"context_line":"nodes in ``DEPLOYWAIT`` provison state tate we can simple move it to"},{"line_number":36,"context_line":"``DEPLOYFAIL`` state and proceed with cleaning (we acquire a lock"},{"line_number":37,"context_line":"before to prevent any race condition in case the ramdisk talks to the"},{"line_number":38,"context_line":"``ironic-conductor`` to continue the deployment)."}],"source_content_type":"text/x-rst","patch_set":1,"id":"3a50d1a3_1613895c","line":35,"updated":"2015-07-22 02:14:56.000000000","message":"nits: s/provison/provision","commit_id":"ccf1f85ed77a463abd9e499bddc7a25fdf6b0483"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"5c61c619607b13719877b36dcbef812ccafbcd48","unresolved":false,"context_lines":[{"line_number":32,"context_line":"nodes in ``DEPLOYWAIT`` provison state its pretty straight forward"},{"line_number":33,"context_line":"since they don\u0027t hold any locks nor there\u0027s a thread running in the"},{"line_number":34,"context_line":"``ironic-conductor`` executing the code for the deployment. So for"},{"line_number":35,"context_line":"nodes in ``DEPLOYWAIT`` provison state tate we can simple move it to"},{"line_number":36,"context_line":"``DEPLOYFAIL`` state and proceed with cleaning (we acquire a lock"},{"line_number":37,"context_line":"before to prevent any race condition in case the ramdisk talks to the"},{"line_number":38,"context_line":"``ironic-conductor`` to continue the deployment)."}],"source_content_type":"text/x-rst","patch_set":1,"id":"3a50d1a3_d3aea884","line":35,"updated":"2015-07-21 17:10:27.000000000","message":"s/tate// typo","commit_id":"ccf1f85ed77a463abd9e499bddc7a25fdf6b0483"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"d0e4a9e9ed557abbfe055390d6f758abbb22d348","unresolved":false,"context_lines":[{"line_number":32,"context_line":"nodes in ``DEPLOYWAIT`` provison state its pretty straight forward"},{"line_number":33,"context_line":"since they don\u0027t hold any locks nor there\u0027s a thread running in the"},{"line_number":34,"context_line":"``ironic-conductor`` executing the code for the deployment. So for"},{"line_number":35,"context_line":"nodes in ``DEPLOYWAIT`` provison state tate we can simple move it to"},{"line_number":36,"context_line":"``DEPLOYFAIL`` state and proceed with cleaning (we acquire a lock"},{"line_number":37,"context_line":"before to prevent any race condition in case the ramdisk talks to the"},{"line_number":38,"context_line":"``ironic-conductor`` to continue the deployment)."}],"source_content_type":"text/x-rst","patch_set":1,"id":"3a50d1a3_d89d4367","line":35,"in_reply_to":"3a50d1a3_1613895c","updated":"2015-07-22 08:57:17.000000000","message":"Done","commit_id":"ccf1f85ed77a463abd9e499bddc7a25fdf6b0483"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"c99223acf5b0400b998781b5601a06a7665e7afe","unresolved":false,"context_lines":[{"line_number":209,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"The documentation will add explaining for deployers how the lazyly"},{"line_number":212,"context_line":"deleation of an instance works."},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"References"},{"line_number":215,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"3a50d1a3_f67e1d18","line":212,"updated":"2015-07-22 02:36:00.000000000","message":"s/deleation/deletion","commit_id":"ccf1f85ed77a463abd9e499bddc7a25fdf6b0483"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"d0e4a9e9ed557abbfe055390d6f758abbb22d348","unresolved":false,"context_lines":[{"line_number":209,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"The documentation will add explaining for deployers how the lazyly"},{"line_number":212,"context_line":"deleation of an instance works."},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"References"},{"line_number":215,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"3a50d1a3_988a5b1a","line":212,"in_reply_to":"3a50d1a3_f67e1d18","updated":"2015-07-22 08:57:17.000000000","message":"Done","commit_id":"ccf1f85ed77a463abd9e499bddc7a25fdf6b0483"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"040aa5cfa3d1958d5770d03770bb1856456eb66c","unresolved":false,"context_lines":[{"line_number":46,"context_line":"become ``ACTIVE`` before the deletion happens. If the deployment of the"},{"line_number":47,"context_line":"node happens in multiple phases it\u0027s likely that once a node enters in"},{"line_number":48,"context_line":"``DEPLOYWAIT`` state (which doesn\u0027t hold a exclusive lock) the deletion"},{"line_number":49,"context_line":"will be performed on this stage before the node ever goes to ``ACTIVE``."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"To mark the node so the instance deletion happens asynchronously"},{"line_number":52,"context_line":"this spec proposes adding a new field to the node object called"}],"source_content_type":"text/x-rst","patch_set":2,"id":"3a50d1a3_7e33b462","line":49,"updated":"2015-07-22 10:22:01.000000000","message":"Won\u0027t it be easy to require all deploy implementations to respect this state? I think we might even just use DELETING state as indication that node should NOT be moved to active and that we should proceed with cleaning instead?","commit_id":"655b284ee8906fbc57b0d7bfe20e16dd1cecc6eb"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"046e956d9ccb551c2a094d05b5c02a71bdd730a1","unresolved":false,"context_lines":[{"line_number":46,"context_line":"become ``ACTIVE`` before the deletion happens. If the deployment of the"},{"line_number":47,"context_line":"node happens in multiple phases it\u0027s likely that once a node enters in"},{"line_number":48,"context_line":"``DEPLOYWAIT`` state (which doesn\u0027t hold a exclusive lock) the deletion"},{"line_number":49,"context_line":"will be performed on this stage before the node ever goes to ``ACTIVE``."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"To mark the node so the instance deletion happens asynchronously"},{"line_number":52,"context_line":"this spec proposes adding a new field to the node object called"}],"source_content_type":"text/x-rst","patch_set":2,"id":"3a50d1a3_99e01afc","line":49,"in_reply_to":"3a50d1a3_7e33b462","updated":"2015-07-22 10:41:42.000000000","message":"The problem with require it is that we have to account with out-of-tree drivers that may have a different deployment method. Therefor the periodic task is good because it makes sure that it works generically for all deploy methods by eventually deleting the instance.\n\nBut that said, for the in-tree drivers we could add checks in the deployment code to check for this flag and abort the deployment in case delete_instance \u003d\u003d True that will make things faster.\n\n...\n\nReusing target_provision_state I thought about it but it\u0027s racy since task.process_event() is in charge of changing it. That\u0027s why I added a flag like maintenance for this (which is not a state and can be set at any time). \n\nIt\u0027s important to note that to be safe the \"delete_instance\" get\u0027s cleaned up on every node tears down (just like the instance_info), maybe I should mention it on the spec too.","commit_id":"655b284ee8906fbc57b0d7bfe20e16dd1cecc6eb"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"046e956d9ccb551c2a094d05b5c02a71bdd730a1","unresolved":false,"context_lines":[{"line_number":89,"context_line":"REST API impact"},{"line_number":90,"context_line":"---------------"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"None"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"Client (CLI) impact"},{"line_number":95,"context_line":"-------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"3a50d1a3_79fc2e72","line":92,"updated":"2015-07-22 10:41:42.000000000","message":"We should expose this field in the API as read-only","commit_id":"655b284ee8906fbc57b0d7bfe20e16dd1cecc6eb"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"91840c1df5dd872710dab03c400e2839fe55ec46","unresolved":false,"context_lines":[{"line_number":19,"context_line":"Nova expects to be able to delete an instance even when it\u0027s spawning"},{"line_number":20,"context_line":"but in Ironic the deployment can only be stopped in a specific stage"},{"line_number":21,"context_line":"(when the node is in ``DEPLOYWAIT`` state). We should allow deleting"},{"line_number":22,"context_line":"the instance at any time"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Proposed change"},{"line_number":25,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_9aad17eb","line":22,"updated":"2015-07-23 15:15:07.000000000","message":"nit, missing period at the end here. s/time/time./","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"dfd51d6ab988b2e3005b940eb8de6a28b4fc930d","unresolved":false,"context_lines":[{"line_number":19,"context_line":"Nova expects to be able to delete an instance even when it\u0027s spawning"},{"line_number":20,"context_line":"but in Ironic the deployment can only be stopped in a specific stage"},{"line_number":21,"context_line":"(when the node is in ``DEPLOYWAIT`` state). We should allow deleting"},{"line_number":22,"context_line":"the instance at any time"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Proposed change"},{"line_number":25,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_4076c006","line":22,"in_reply_to":"3a50d1a3_9aad17eb","updated":"2015-07-23 15:31:59.000000000","message":"Done","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"91840c1df5dd872710dab03c400e2839fe55ec46","unresolved":false,"context_lines":[{"line_number":24,"context_line":"Proposed change"},{"line_number":25,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"The propose implementation consists in allowing the **deleted** verb for"},{"line_number":28,"context_line":"provision state to be passed to a node in ``DEPLOYING`` state, same as"},{"line_number":29,"context_line":"we allow it to be passed to a node in ``DEPLOYWAIT`` state."},{"line_number":30,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_3a8f2376","line":27,"updated":"2015-07-23 15:15:07.000000000","message":"s/The propose/This proposed/ ??\n\nI think it should be s/deleted/delete/ for the verb. Before the new state machine, we had \u0027deleted\u0027, but if you look at the new state machine, it is \u0027delete\u0027. (We made it backwards compatible though and still support \u0027deleted\u0027.)\n\nAnd/or use \u0027delete\u0027 and put something in parentheses, eg: \u0027(or \u0027deleted\u0027 verb used prior to the new state machine)\u0027.","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"dfd51d6ab988b2e3005b940eb8de6a28b4fc930d","unresolved":false,"context_lines":[{"line_number":24,"context_line":"Proposed change"},{"line_number":25,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"The propose implementation consists in allowing the **deleted** verb for"},{"line_number":28,"context_line":"provision state to be passed to a node in ``DEPLOYING`` state, same as"},{"line_number":29,"context_line":"we allow it to be passed to a node in ``DEPLOYWAIT`` state."},{"line_number":30,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_e0acaca0","line":27,"in_reply_to":"3a50d1a3_00ec3828","updated":"2015-07-23 15:31:59.000000000","message":"+1 for \"This proposed\"\n\nYeah for the verb we still use \"deleted\" (probably left for backwards compatibility). So I will just be reusing it as part of this spec.","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"df3aa88a228dc453c3b5f7bdd942dc5ca8477fb8","unresolved":false,"context_lines":[{"line_number":24,"context_line":"Proposed change"},{"line_number":25,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"The propose implementation consists in allowing the **deleted** verb for"},{"line_number":28,"context_line":"provision state to be passed to a node in ``DEPLOYING`` state, same as"},{"line_number":29,"context_line":"we allow it to be passed to a node in ``DEPLOYWAIT`` state."},{"line_number":30,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_00ec3828","line":27,"in_reply_to":"3a50d1a3_3a8f2376","updated":"2015-07-23 15:21:28.000000000","message":"Sigh. I looked at the code -- although we use \u0027delete\u0027 as the verb for the state machine, it doesn\u0027t look like we\u0027ve coded it so that we can use \u0027delete\u0027 as the verb/provision-state that the user can specify. I guess we can leave that for a rainy day. Or I can fix it.","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"91840c1df5dd872710dab03c400e2839fe55ec46","unresolved":false,"context_lines":[{"line_number":29,"context_line":"we allow it to be passed to a node in ``DEPLOYWAIT`` state."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"The way the instance is deleted depends on the provision state of"},{"line_number":32,"context_line":"the node. For nodes in ``DEPLOYWAIT`` state it\u0027s pretty straight"},{"line_number":33,"context_line":"forward. Since they hold no locks nor there\u0027s a worker thread running"},{"line_number":34,"context_line":"in the ``ironic-conductor`` executing the code for the deployment we"},{"line_number":35,"context_line":"can simple move it to ``DELETING`` state and proceed with the cleaning"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_bd7a4d75","line":32,"updated":"2015-07-23 15:15:07.000000000","message":"This description for nodes in DEPLOYWAIT is what the code is already doing, right? Or are you proposing to change it to what you are describing here?","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"aa90b67c2cb8e0317d2d56c7bda5e8199fb55a7a","unresolved":false,"context_lines":[{"line_number":29,"context_line":"we allow it to be passed to a node in ``DEPLOYWAIT`` state."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"The way the instance is deleted depends on the provision state of"},{"line_number":32,"context_line":"the node. For nodes in ``DEPLOYWAIT`` state it\u0027s pretty straight"},{"line_number":33,"context_line":"forward. Since they hold no locks nor there\u0027s a worker thread running"},{"line_number":34,"context_line":"in the ``ironic-conductor`` executing the code for the deployment we"},{"line_number":35,"context_line":"can simple move it to ``DELETING`` state and proceed with the cleaning"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_a3c8ee55","line":32,"in_reply_to":"3a50d1a3_807fe813","updated":"2015-07-23 16:44:59.000000000","message":"I would update the spec to mention that then. Eg, \"Ironic already supports deleting instances when nodes are in DEPLOYWAIT state, so that won\u0027t change. In this case, what it does is pretty straight forward...\"","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"8222927e0f2819fa44c6f1bf5d4073c572a0539c","unresolved":false,"context_lines":[{"line_number":29,"context_line":"we allow it to be passed to a node in ``DEPLOYWAIT`` state."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"The way the instance is deleted depends on the provision state of"},{"line_number":32,"context_line":"the node. For nodes in ``DEPLOYWAIT`` state it\u0027s pretty straight"},{"line_number":33,"context_line":"forward. Since they hold no locks nor there\u0027s a worker thread running"},{"line_number":34,"context_line":"in the ``ironic-conductor`` executing the code for the deployment we"},{"line_number":35,"context_line":"can simple move it to ``DELETING`` state and proceed with the cleaning"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_616bbbb7","line":32,"in_reply_to":"3a50d1a3_a3c8ee55","updated":"2015-07-23 17:00:06.000000000","message":"Done","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"dfd51d6ab988b2e3005b940eb8de6a28b4fc930d","unresolved":false,"context_lines":[{"line_number":29,"context_line":"we allow it to be passed to a node in ``DEPLOYWAIT`` state."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"The way the instance is deleted depends on the provision state of"},{"line_number":32,"context_line":"the node. For nodes in ``DEPLOYWAIT`` state it\u0027s pretty straight"},{"line_number":33,"context_line":"forward. Since they hold no locks nor there\u0027s a worker thread running"},{"line_number":34,"context_line":"in the ``ironic-conductor`` executing the code for the deployment we"},{"line_number":35,"context_line":"can simple move it to ``DELETING`` state and proceed with the cleaning"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_807fe813","line":32,"in_reply_to":"3a50d1a3_bd7a4d75","updated":"2015-07-23 15:31:59.000000000","message":"Yeah, that\u0027s what the code current does and won\u0027t be changed. I just described it here so folks can see that depending on the state that the node is current in the deletion of the instance will happens in different ways.","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"aa90b67c2cb8e0317d2d56c7bda5e8199fb55a7a","unresolved":false,"context_lines":[{"line_number":35,"context_line":"can simple move it to ``DELETING`` state and proceed with the cleaning"},{"line_number":36,"context_line":"(We will acquire an exclusive lock before changing the state to prevent"},{"line_number":37,"context_line":"any race condition with the ramdisk talking back to Ironic to continue"},{"line_number":38,"context_line":"the deployment)."},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"For nodes in ``DEPLOYING`` state the deletion of the instance should"},{"line_number":41,"context_line":"happen asynchronously. We first need to mark the node in the database to"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_e97b37f3","line":38,"updated":"2015-07-23 16:44:59.000000000","message":"How about rewording this like\n\n..executing the code for the deployment, we acquire an exclusive lock, change the node to DELETING provision state, and proceed with the deleting/cleaning. (The exclusive lock prevents any race condition...)","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"8222927e0f2819fa44c6f1bf5d4073c572a0539c","unresolved":false,"context_lines":[{"line_number":35,"context_line":"can simple move it to ``DELETING`` state and proceed with the cleaning"},{"line_number":36,"context_line":"(We will acquire an exclusive lock before changing the state to prevent"},{"line_number":37,"context_line":"any race condition with the ramdisk talking back to Ironic to continue"},{"line_number":38,"context_line":"the deployment)."},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"For nodes in ``DEPLOYING`` state the deletion of the instance should"},{"line_number":41,"context_line":"happen asynchronously. We first need to mark the node in the database to"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_216133d3","line":38,"in_reply_to":"3a50d1a3_e97b37f3","updated":"2015-07-23 17:00:06.000000000","message":"Done","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"aa90b67c2cb8e0317d2d56c7bda5e8199fb55a7a","unresolved":false,"context_lines":[{"line_number":46,"context_line":"become ``ACTIVE`` before the deletion happens. If the deployment of the"},{"line_number":47,"context_line":"node happens in multiple phases it\u0027s likely that once a node enters in"},{"line_number":48,"context_line":"``DEPLOYWAIT`` state (which doesn\u0027t hold a exclusive lock) the deletion"},{"line_number":49,"context_line":"will be performed on this stage before the node ever goes to ``ACTIVE``."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"To mark the node so the instance deletion happens asynchronously"},{"line_number":52,"context_line":"this spec proposes adding a new field to the node object called"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_cf83db01","line":49,"updated":"2015-07-23 16:44:59.000000000","message":"I looked quickly at the code. There are cases where DEPLOYWAIT-\u003eDEPLOYING (via resume)-\u003eACTIVE (via done). I think all the above is basically saying that even though the request to delete the instance happened when the node was in DEPLOYING, the actual deletion will happen whenever the periodic task manages to get an exclusive lock on the node -- at which point the node will not be in DEPLOYING but that is OK.\n\nInstead of a periodic task, an alternative might be to \u0027capture\u0027 the case of node in DEPLOYING, before the node transitions to another state (eg before or at the call to task_manager.process_event()). This would cause the deletion to occur sooner so that the node would be available sooner. (eg, if we looked at whether it needed to be deleted before rebooting the node to finish the deploy).\n\nBut the periodic task might be simpler to code/understand, if we don\u0027t care if it takes a bit longer for the node to be deleted/available again.\n\nDo we use a node\u0027s state eg ACTIVE for anything that might skew nova\u0027s stats when the node is transitioning from DEPLOY*-\u003eACTIVE-\u003eDELETING due to this lazy delete? (Being lazy and don\u0027t have time now to look at nova code.)","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"0f1efdad98fb0bc15fda5abe988a6627a17a49d4","unresolved":false,"context_lines":[{"line_number":46,"context_line":"become ``ACTIVE`` before the deletion happens. If the deployment of the"},{"line_number":47,"context_line":"node happens in multiple phases it\u0027s likely that once a node enters in"},{"line_number":48,"context_line":"``DEPLOYWAIT`` state (which doesn\u0027t hold a exclusive lock) the deletion"},{"line_number":49,"context_line":"will be performed on this stage before the node ever goes to ``ACTIVE``."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"To mark the node so the instance deletion happens asynchronously"},{"line_number":52,"context_line":"this spec proposes adding a new field to the node object called"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_ad07ff7f","line":49,"in_reply_to":"3a50d1a3_1bc43b5d","updated":"2015-07-27 14:17:22.000000000","message":"I was thinking that the code wouldn\u0027t actually transition to the intended new state. And it wouldn\u0027t need to do a refresh if that was an issue, it could get a new instance of the node. (Or if that flag was in a non-node table then it wouldn\u0027t be an issue.) So I don\u0027t think there would be a race condition.\n\nEg, conductor is done with deploying. Before transitioning to new state, it can check whether the node should be deleted, and if so, don\u0027t transition to new state, go to delete process instead.","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"f702e6646187e6a5dbd30970c9d774b175190749","unresolved":false,"context_lines":[{"line_number":46,"context_line":"become ``ACTIVE`` before the deletion happens. If the deployment of the"},{"line_number":47,"context_line":"node happens in multiple phases it\u0027s likely that once a node enters in"},{"line_number":48,"context_line":"``DEPLOYWAIT`` state (which doesn\u0027t hold a exclusive lock) the deletion"},{"line_number":49,"context_line":"will be performed on this stage before the node ever goes to ``ACTIVE``."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"To mark the node so the instance deletion happens asynchronously"},{"line_number":52,"context_line":"this spec proposes adding a new field to the node object called"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_10e886e1","line":49,"in_reply_to":"3a50d1a3_ad07ff7f","updated":"2015-07-27 14:23:09.000000000","message":"Yeah getting a new instance of the node will avoid refresh() but,\n\n\"Before transitioning to new state, it can check whether the node should be deleted, and if so, don\u0027t transition to new state, go to delete process instead\"\n\nThere\u0027s still a potential race there, what if the delete_instance\u003dTrue gets set after the check and before the change provision state ? I know that might be rare but still there\u0027s a window there.","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"8222927e0f2819fa44c6f1bf5d4073c572a0539c","unresolved":false,"context_lines":[{"line_number":46,"context_line":"become ``ACTIVE`` before the deletion happens. If the deployment of the"},{"line_number":47,"context_line":"node happens in multiple phases it\u0027s likely that once a node enters in"},{"line_number":48,"context_line":"``DEPLOYWAIT`` state (which doesn\u0027t hold a exclusive lock) the deletion"},{"line_number":49,"context_line":"will be performed on this stage before the node ever goes to ``ACTIVE``."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"To mark the node so the instance deletion happens asynchronously"},{"line_number":52,"context_line":"this spec proposes adding a new field to the node object called"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_e12eeb41","line":49,"in_reply_to":"3a50d1a3_cf83db01","updated":"2015-07-23 17:00:06.000000000","message":"Hi, yes definitely adding some checks in the thread that is doing the deployment (While the node is in DEPOYING state) can make the lazy deletion to happen faster (for e.g if delete_instance is set just skip the rest of the deployment), this can be done for the drivers in-tree as part of this spec too.\n\nBut since we have to think about out-of-tree drivers, the periodic task is a generic way to lazyly delete the deployments for all nodes (independent of the driver) that the conductor is managing.\n\n...\n\nre Nova, it only cares about the nodes in provision_state AVAILABLE or NOSTATE (for backwards compat) to calculate if the node amount of available resources[1], so for a node in provision_state !\u003d (AVAILABLE, NOSTATE) the nova will account it as non available for deployments.\n\n[1] https://github.com/openstack/nova/blob/master/nova/virt/ironic/driver.py#L199-L215","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"9ad08637ef058f3c556f7ba3648b6d5b1d36ebfe","unresolved":false,"context_lines":[{"line_number":46,"context_line":"become ``ACTIVE`` before the deletion happens. If the deployment of the"},{"line_number":47,"context_line":"node happens in multiple phases it\u0027s likely that once a node enters in"},{"line_number":48,"context_line":"``DEPLOYWAIT`` state (which doesn\u0027t hold a exclusive lock) the deletion"},{"line_number":49,"context_line":"will be performed on this stage before the node ever goes to ``ACTIVE``."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"To mark the node so the instance deletion happens asynchronously"},{"line_number":52,"context_line":"this spec proposes adding a new field to the node object called"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_f27f8564","line":49,"in_reply_to":"3a50d1a3_e12eeb41","updated":"2015-07-24 02:12:09.000000000","message":"I\u0027m not convinced that out-of-tree drivers are an issue. If we were to do an alternative to periodic tasks, anything we do would/should be via the conductor manager or task_manager.","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"dbc2d6b70ef48299b9ac4f309aed21c898b2a449","unresolved":false,"context_lines":[{"line_number":46,"context_line":"become ``ACTIVE`` before the deletion happens. If the deployment of the"},{"line_number":47,"context_line":"node happens in multiple phases it\u0027s likely that once a node enters in"},{"line_number":48,"context_line":"``DEPLOYWAIT`` state (which doesn\u0027t hold a exclusive lock) the deletion"},{"line_number":49,"context_line":"will be performed on this stage before the node ever goes to ``ACTIVE``."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"To mark the node so the instance deletion happens asynchronously"},{"line_number":52,"context_line":"this spec proposes adding a new field to the node object called"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_1bc43b5d","line":49,"in_reply_to":"3a50d1a3_f27f8564","updated":"2015-07-27 09:19:59.000000000","message":"@Ruby,\n\nI think the task_manager can be racy. Say we will verify that flag just before we move the node from DEPLOYING to ACTIVE there\u0027s still a race there in the moment it\u0027s checking the flag and actually changing the state.\n\nAlso, that would be needed to do a \u003cobject\u003e.refresh() inside the task_manager to always verify the latest version in the db and that can be tricky because it will overwrite previous changes not saved on the object yet.\n\nI think the periodic task is a better solution because it\u0027s race free, tho it could be a little slower. But that shouldn\u0027t be a problem.","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"91840c1df5dd872710dab03c400e2839fe55ec46","unresolved":false,"context_lines":[{"line_number":51,"context_line":"To mark the node so the instance deletion happens asynchronously"},{"line_number":52,"context_line":"this spec proposes adding a new field to the node object called"},{"line_number":53,"context_line":"``delete_instance``. The value of the field is a boolean, indicating"},{"line_number":54,"context_line":"whether the instance should be deleted or not. It\u0027s important to note"},{"line_number":55,"context_line":"that to be safe this field will be cleaned up on every node tear down,"},{"line_number":56,"context_line":"just like ``instance_info``."},{"line_number":57,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_fd8a151a","line":54,"updated":"2015-07-23 15:15:07.000000000","message":"My concern with this is that you want to mark/update the node, yet there is/could be an exclusive lock on the node at the time when you want to update \u0027delete_instance\u0027.","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"aa90b67c2cb8e0317d2d56c7bda5e8199fb55a7a","unresolved":false,"context_lines":[{"line_number":51,"context_line":"To mark the node so the instance deletion happens asynchronously"},{"line_number":52,"context_line":"this spec proposes adding a new field to the node object called"},{"line_number":53,"context_line":"``delete_instance``. The value of the field is a boolean, indicating"},{"line_number":54,"context_line":"whether the instance should be deleted or not. It\u0027s important to note"},{"line_number":55,"context_line":"that to be safe this field will be cleaned up on every node tear down,"},{"line_number":56,"context_line":"just like ``instance_info``."},{"line_number":57,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_147f96e0","line":54,"in_reply_to":"3a50d1a3_0015189a","updated":"2015-07-23 16:44:59.000000000","message":"I\u0027d prefer the to-be-deleted table just because it is cleaner. An exclusive lock to me means that nothing else should be touching it/the db. Although the way our objects are coded, I think if there were two simultaneous requests to update the DB for a node, that one request wouldn\u0027t touch/alter the other request\u0027s change to delete_instance, unless both were modifying it?","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"8222927e0f2819fa44c6f1bf5d4073c572a0539c","unresolved":false,"context_lines":[{"line_number":51,"context_line":"To mark the node so the instance deletion happens asynchronously"},{"line_number":52,"context_line":"this spec proposes adding a new field to the node object called"},{"line_number":53,"context_line":"``delete_instance``. The value of the field is a boolean, indicating"},{"line_number":54,"context_line":"whether the instance should be deleted or not. It\u0027s important to note"},{"line_number":55,"context_line":"that to be safe this field will be cleaned up on every node tear down,"},{"line_number":56,"context_line":"just like ``instance_info``."},{"line_number":57,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_a1c8e3a8","line":54,"in_reply_to":"3a50d1a3_147f96e0","updated":"2015-07-23 17:00:06.000000000","message":"exactly","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"dfd51d6ab988b2e3005b940eb8de6a28b4fc930d","unresolved":false,"context_lines":[{"line_number":51,"context_line":"To mark the node so the instance deletion happens asynchronously"},{"line_number":52,"context_line":"this spec proposes adding a new field to the node object called"},{"line_number":53,"context_line":"``delete_instance``. The value of the field is a boolean, indicating"},{"line_number":54,"context_line":"whether the instance should be deleted or not. It\u0027s important to note"},{"line_number":55,"context_line":"that to be safe this field will be cleaned up on every node tear down,"},{"line_number":56,"context_line":"just like ``instance_info``."},{"line_number":57,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_0015189a","line":54,"in_reply_to":"3a50d1a3_fd8a151a","updated":"2015-07-23 15:31:59.000000000","message":"Yeah the idea is to not require a lock to update that field, because it won\u0027t be a race there (we always will set it to delete_instance \u003d True). An alternative would be to create a table for the nodes that will be deleted later so we don\u0027t have to update the nodes table.","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":10380,"name":"Josh Gachnang","email":"josh@pcsforeducation.com","username":"JoshNang"},"change_message_id":"1e140963da2712036630902369c96e8912010971","unresolved":false,"context_lines":[{"line_number":54,"context_line":"whether the instance should be deleted or not. It\u0027s important to note"},{"line_number":55,"context_line":"that to be safe this field will be cleaned up on every node tear down,"},{"line_number":56,"context_line":"just like ``instance_info``."},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"In the Nova perspective a successful call to delete whether it happens"},{"line_number":59,"context_line":"immediately (``DEPLOYWAIT``) or eventually (``DEPLOYING``) is enough to"},{"line_number":60,"context_line":"indicate that the instance have been deleted successfully."}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_ac97ba46","line":57,"updated":"2015-07-22 19:20:42.000000000","message":"One thing missing is how the instance UUID will be cleared. Will we change Ironic to automatically remove it as part of tear down? Allow it to be removed from a node in DEPLOY*/ACTIVE? Right now Nova polls for a state change to DELETING/CLEANING, then sends an API request to remove the UUID.\n\nPersonally, I\u0027d say making Ironic remove the instance uuid when cleaning up instance info would be the way to go, just need to make sure we make it backwards compatible, so you don\u0027t have to do Nova/Ironic upgrades together.","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"b677df805c885d0916c922c29db77c182c1aef7f","unresolved":false,"context_lines":[{"line_number":54,"context_line":"whether the instance should be deleted or not. It\u0027s important to note"},{"line_number":55,"context_line":"that to be safe this field will be cleaned up on every node tear down,"},{"line_number":56,"context_line":"just like ``instance_info``."},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"In the Nova perspective a successful call to delete whether it happens"},{"line_number":59,"context_line":"immediately (``DEPLOYWAIT``) or eventually (``DEPLOYING``) is enough to"},{"line_number":60,"context_line":"indicate that the instance have been deleted successfully."}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_e23e298a","line":57,"in_reply_to":"3a50d1a3_ac97ba46","updated":"2015-07-22 19:52:04.000000000","message":"HI Josh,\n\nThe way I thought to make it work would be to allow it to be cleaned when node is in DEPLOY*, like we do for CLEAN* today. Ideally I would love to have the instance_uuid to be cleaned as part of the node\u0027s tear_down() just like it happens to the instance_info field, but that would be a non-backward compat change that I\u0027m not willing to do as part of this spec.\n\nFor nodes in ACTIVE no work is needed, because that\u0027s how the delete works today and will happen synchronously (since there are no locks being hold on a node in that state)","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"a1362f648f12e18cdd475a7a1be986a1b6c69da8","unresolved":false,"context_lines":[{"line_number":54,"context_line":"whether the instance should be deleted or not. It\u0027s important to note"},{"line_number":55,"context_line":"that to be safe this field will be cleaned up on every node tear down,"},{"line_number":56,"context_line":"just like ``instance_info``."},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"In the Nova perspective a successful call to delete whether it happens"},{"line_number":59,"context_line":"immediately (``DEPLOYWAIT``) or eventually (``DEPLOYING``) is enough to"},{"line_number":60,"context_line":"indicate that the instance have been deleted successfully."}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_87822cff","line":57,"in_reply_to":"3a50d1a3_af1e9e09","updated":"2015-07-27 10:30:02.000000000","message":"Josh,\n\nI have proposed patches separated from this spec for this problem:\n\n* Nova: https://review.openstack.org/204995\n* Ironic: https://review.openstack.org/205033","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"8806b1961ab98ec5702a105383f619ac9dac115a","unresolved":false,"context_lines":[{"line_number":54,"context_line":"whether the instance should be deleted or not. It\u0027s important to note"},{"line_number":55,"context_line":"that to be safe this field will be cleaned up on every node tear down,"},{"line_number":56,"context_line":"just like ``instance_info``."},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"In the Nova perspective a successful call to delete whether it happens"},{"line_number":59,"context_line":"immediately (``DEPLOYWAIT``) or eventually (``DEPLOYING``) is enough to"},{"line_number":60,"context_line":"indicate that the instance have been deleted successfully."}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_af1e9e09","line":57,"in_reply_to":"3a50d1a3_e23e298a","updated":"2015-07-23 13:09:04.000000000","message":"Hi Josh,\n\nSo I was looking this morning at this problem, you have opened a bug for it here [1].\n\nSo I\u0027m trying to fix this outside this spec (there are patches already proposed). But that said the bug fix should be a requirement for this spec. What do you think?\n\n[1] https://bugs.launchpad.net/ironic/+bug/1436568","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"f46f1024b62fcbd6e414541b570e05d7c0477cf7","unresolved":false,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"In the Nova perspective a successful call to delete whether it happens"},{"line_number":59,"context_line":"immediately (``DEPLOYWAIT``) or eventually (``DEPLOYING``) is enough to"},{"line_number":60,"context_line":"indicate that the instance have been deleted successfully."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"Alternatives"},{"line_number":63,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_34d1cb23","line":60,"updated":"2015-07-22 10:56:52.000000000","message":"Isn\u0027t Nova need to know the actual status of delete operation? What if \u0027delete\u0027 operation were to fail when it gets triggered asynchronously?","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"3607e713a71390120205ea4fa294eb5d10abd178","unresolved":false,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"In the Nova perspective a successful call to delete whether it happens"},{"line_number":59,"context_line":"immediately (``DEPLOYWAIT``) or eventually (``DEPLOYING``) is enough to"},{"line_number":60,"context_line":"indicate that the instance have been deleted successfully."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"Alternatives"},{"line_number":63,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_6ff1c000","line":60,"in_reply_to":"3a50d1a3_34d1cb23","updated":"2015-07-22 11:15:57.000000000","message":"Not really, nova should know that the request to delete the instance was accepted by Ironic and take care of cleaning up the things from the Nova\u0027s side (deleting the instance registry, removing the network it created etc...). For Ironic it will be like cleaning, Nova doesn\u0027t wait the node to be cleaned up and become available before returning from destroy(). If the node fail to be deleted or cleaned it will go to a *FAIL state and operators can then take a look at what happened and retry it etc...","commit_id":"448708b3023f1a5062adcf6700056a548f252ff0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"5b65266d91d70369d42b71d5f5fd84c9f5da2099","unresolved":false,"context_lines":[{"line_number":54,"context_line":"``delete_instance``. The value of the field is a boolean, indicating"},{"line_number":55,"context_line":"whether the instance should be deleted or not. It\u0027s important to note"},{"line_number":56,"context_line":"that to be safe this field will be cleaned up on every node tear down,"},{"line_number":57,"context_line":"just like ``instance_info``."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"In the Nova perspective a successful call to delete whether it happens"},{"line_number":60,"context_line":"immediately (``DEPLOYWAIT``) or eventually (``DEPLOYING``) is enough to"}],"source_content_type":"text/x-rst","patch_set":4,"id":"3a50d1a3_6d12570b","line":57,"updated":"2015-07-27 14:19:52.000000000","message":"I think this should mention that we will be updating this field/node even though there is an exclusive lock on it by some other task. But that will be OK because ...","commit_id":"c3b2e4bb460195643a86299741cf5d67e70f8b24"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"668e1a89918554bb8d1feeedd19062f72deadebf","unresolved":false,"context_lines":[{"line_number":54,"context_line":"``delete_instance``. The value of the field is a boolean, indicating"},{"line_number":55,"context_line":"whether the instance should be deleted or not. It\u0027s important to note"},{"line_number":56,"context_line":"that to be safe this field will be cleaned up on every node tear down,"},{"line_number":57,"context_line":"just like ``instance_info``."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"In the Nova perspective a successful call to delete whether it happens"},{"line_number":60,"context_line":"immediately (``DEPLOYWAIT``) or eventually (``DEPLOYING``) is enough to"}],"source_content_type":"text/x-rst","patch_set":4,"id":"3a50d1a3_90a1f6ab","line":57,"in_reply_to":"3a50d1a3_6d12570b","updated":"2015-07-27 14:25:28.000000000","message":"++","commit_id":"c3b2e4bb460195643a86299741cf5d67e70f8b24"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"5b65266d91d70369d42b71d5f5fd84c9f5da2099","unresolved":false,"context_lines":[{"line_number":76,"context_line":"  which is doing that work, even after a hash-ring rebalance (that\u0027s"},{"line_number":77,"context_line":"  possible just by routing the message to the conductor which is set in"},{"line_number":78,"context_line":"  the node\u0027s ``reservation`` field)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"Data model impact"},{"line_number":81,"context_line":"-----------------"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"3a50d1a3_0de873e1","line":79,"updated":"2015-07-27 14:19:52.000000000","message":"I think it is worth mentioning the alternative of the conductor checking to see if the node should be deleted, before it transitions to ACTIVE or DEPLOYWAIT. The adv being that the deletion occurs sooner. The disadv being (not totally sure, code is messier to write/understand?)","commit_id":"c3b2e4bb460195643a86299741cf5d67e70f8b24"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"668e1a89918554bb8d1feeedd19062f72deadebf","unresolved":false,"context_lines":[{"line_number":76,"context_line":"  which is doing that work, even after a hash-ring rebalance (that\u0027s"},{"line_number":77,"context_line":"  possible just by routing the message to the conductor which is set in"},{"line_number":78,"context_line":"  the node\u0027s ``reservation`` field)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"Data model impact"},{"line_number":81,"context_line":"-----------------"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"3a50d1a3_50992e6d","line":79,"in_reply_to":"3a50d1a3_0de873e1","updated":"2015-07-27 14:25:28.000000000","message":"I\u0027m not convinced that\u0027s an alternative to the periodic task. Because it\u0027s racy, there\u0027s a window between after the check and before the transition change starts.","commit_id":"c3b2e4bb460195643a86299741cf5d67e70f8b24"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"5b65266d91d70369d42b71d5f5fd84c9f5da2099","unresolved":false,"context_lines":[{"line_number":82,"context_line":""},{"line_number":83,"context_line":"A new field ``delete_instance`` will be added to the Node table. This"},{"line_number":84,"context_line":"field will hold a boolean value indicating whether the deployment should"},{"line_number":85,"context_line":"be lazyly deleted or not."},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"State Machine Impact"},{"line_number":88,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"3a50d1a3_4db67b8b","line":85,"updated":"2015-07-27 14:19:52.000000000","message":"I think we should mention the alternative of putting the new field in a new table and why we aren\u0027t doing this. Not sure if it belongs here or above in Alternatives.","commit_id":"c3b2e4bb460195643a86299741cf5d67e70f8b24"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"668e1a89918554bb8d1feeedd19062f72deadebf","unresolved":false,"context_lines":[{"line_number":82,"context_line":""},{"line_number":83,"context_line":"A new field ``delete_instance`` will be added to the Node table. This"},{"line_number":84,"context_line":"field will hold a boolean value indicating whether the deployment should"},{"line_number":85,"context_line":"be lazyly deleted or not."},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"State Machine Impact"},{"line_number":88,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"3a50d1a3_f0a87a79","line":85,"in_reply_to":"3a50d1a3_4db67b8b","updated":"2015-07-27 14:25:28.000000000","message":"++ probably on the alternatives","commit_id":"c3b2e4bb460195643a86299741cf5d67e70f8b24"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"5b65266d91d70369d42b71d5f5fd84c9f5da2099","unresolved":false,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":"We should expose the new ``delete_instance`` field in the node resources"},{"line_number":96,"context_line":"of the REST API. This field will be read-only, no updates will be allowed"},{"line_number":97,"context_line":"to it via PATCH."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"Client (CLI) impact"},{"line_number":100,"context_line":"-------------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"3a50d1a3_8d31e30d","line":97,"updated":"2015-07-27 14:19:52.000000000","message":"Is there a microversion bump? (Not only for exposing the new field, but for this lazy deletion feature during DEPLOYING?)","commit_id":"c3b2e4bb460195643a86299741cf5d67e70f8b24"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"668e1a89918554bb8d1feeedd19062f72deadebf","unresolved":false,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":"We should expose the new ``delete_instance`` field in the node resources"},{"line_number":96,"context_line":"of the REST API. This field will be read-only, no updates will be allowed"},{"line_number":97,"context_line":"to it via PATCH."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"Client (CLI) impact"},{"line_number":100,"context_line":"-------------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"3a50d1a3_d0dbfe19","line":97,"in_reply_to":"3a50d1a3_8d31e30d","updated":"2015-07-27 14:25:28.000000000","message":"Yes, we should bump the version","commit_id":"c3b2e4bb460195643a86299741cf5d67e70f8b24"},{"author":{"_account_id":9315,"name":"ramesh","email":"rameshg87@gmail.com","username":"rameshg87"},"change_message_id":"e6bb0ed5b16b28ab9c21591ba988c38455831d4b","unresolved":false,"context_lines":[{"line_number":21,"context_line":"(when the node is in ``DEPLOYWAIT`` state). We should allow deleting"},{"line_number":22,"context_line":"the instance at any time."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Proposed change"},{"line_number":25,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"This proposed implementation consists in allowing the **deleted** verb for"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3a50d1a3_3613de66","line":24,"updated":"2015-07-29 06:24:10.000000000","message":"IMHO, we can avoid periodic tasks here if possible because they are kicked off in every interval at every conductor and wakes up consuming resources which does atleast a DB query each time. \n\nAnd another point is we could have newer requirements around Ironic after this - a client would want to abort an ongoing INSPECTING or CLEANING node instead of waiting for node to reach INSPECTWAIT (future) or CLEANWAIT.   So can we generalize this to more of a queued event which needs to be applied once the node reaches the next stable state (I consider DEPLOYWAIT, CLEANWAIT as a stable state because Ironic will not act on the node unless there is a API request to do some action).  \n\nOne alternative is how about if we generalize the new field \u0027delete_instance\u0027 to a \u0027queued_event\u0027 which needs to be applied when node reaches a stable state.  Ironic has to make sure this event is triggered after the transition to next stable state.  If the node is currently not in a stable state, at some point of time it will reach a stable state by doing a task.process_event(\u003csome-verb\u003e).  At that point, we can trigger the queued event.  The queued event is executed only if a transition is possible (i.e \n\n* task.process_event(\u0027done\u0027) results in active from which \u0027fail\u0027 can be applied\n* task.process_event(\u0027wait\u0027) results in \"wait call-back\" from which \u0027fail\u0027 can be applied\n* task.process_event(\u0027fail\u0027) results in \"deploy failed\" on which trying to apply \u0027fail\u0027 again will result in failure, but this has be logged and ignored.\n\nThoughts ?","commit_id":"38f38f2c5b69c6f071da940b66cb9184c9acbe05"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"94dbb52b820ff5872dfe96fc939e30d55c0f4a98","unresolved":false,"context_lines":[{"line_number":21,"context_line":"(when the node is in ``DEPLOYWAIT`` state). We should allow deleting"},{"line_number":22,"context_line":"the instance at any time."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Proposed change"},{"line_number":25,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"This proposed implementation consists in allowing the **deleted** verb for"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3a50d1a3_901bd272","line":24,"in_reply_to":"3a50d1a3_3613de66","updated":"2015-07-29 08:41:06.000000000","message":"Hi there,\n\nRe Periodic task: How can we avoid the periodic task without making the code racy?I think the periodic task is the safer to do because even if we check if the flag is set on every transition change (task.process_event()) there\u0027s still a window between the check and the actual call to transition the state which will be racy.\n\nRe Generic queue: This patch is about conforming with the Nova interaction with the Ironic service which allows an instance to be deleted at any time it\u0027s spawning. Nova doesn\u0027t know about cleaning, inspecting or zapping. While I\u0027m OK making it a generic mechanism I think things are different in some points.\n\nFor example when nova asks for delete an instance the node should go to AVAILABLE state afterwards, so the idea of calling \u0027fail\u0027 is not ideal here because the node will be stuck on DEPLOYFAIL.\n\nOn the other hands for other states like CLEAN* should call \u0027fail\u0027 and stay at the CLEANFAIL if aborted.\n\n...\n\nWhat if we instead of calling the new field \"delete_instance\" we call it something more generic as you suggested \"abort_operation\" or something like that... And on the periodic task (or the other mechanism) we can figure out how to proceed with the abortion.","commit_id":"38f38f2c5b69c6f071da940b66cb9184c9acbe05"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"9dea8ddab6d2bb86e70dfd929b40806896e4a3bf","unresolved":false,"context_lines":[{"line_number":150,"context_line":".. note::"},{"line_number":151,"context_line":"   Other interactions between the Nova driver and Ironic should remains"},{"line_number":152,"context_line":"   the same. Things like removing the VIFs references from the Ironic"},{"line_number":153,"context_line":"   port resources are fine because the port objects are lock-free."},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"Security impact"},{"line_number":156,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3a50d1a3_97b3e84c","line":153,"updated":"2015-07-28 17:37:53.000000000","message":"Tested and this is not true. I will update the spec and code about this interaction.","commit_id":"38f38f2c5b69c6f071da940b66cb9184c9acbe05"}]}