)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"32896c12da06d1ca552c2745129a739d86fc1ff9","unresolved":true,"context_lines":[{"line_number":27,"context_line":"that when you don\u0027t have automatic cleaning, we wait a bit"},{"line_number":28,"context_line":"longer to notice the node is available again."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Fixes-Bug: #1974070"},{"line_number":31,"context_line":"Change-Id: Iab92124b5776a799c7f90d07281d28fcf191c8fe"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"82196138_afc8c4b4","line":30,"range":{"start_line":30,"start_character":0,"end_line":30,"end_character":5},"updated":"2022-11-17 11:29:46.000000000","message":"nit: Closes-Bug: although Fixes-Bug might also work with the launchpad integration.\n\n\non a related note since this is actully fixing it and i think large ironic deployments will care about this can you add a release note to let them know.","commit_id":"c098888ada56770513aebb4bc1942d1ff37dcd65"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f873ed23c93afe12e2563b636c6a3d0937dd79a0","unresolved":false,"context_lines":[{"line_number":27,"context_line":"that when you don\u0027t have automatic cleaning, we wait a bit"},{"line_number":28,"context_line":"longer to notice the node is available again."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Fixes-Bug: #1974070"},{"line_number":31,"context_line":"Change-Id: Iab92124b5776a799c7f90d07281d28fcf191c8fe"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3bb90b1d_7ae8ea94","line":30,"range":{"start_line":30,"start_character":0,"end_line":30,"end_character":5},"in_reply_to":"82196138_afc8c4b4","updated":"2022-11-17 12:38:25.000000000","message":"yeah, good idea","commit_id":"c098888ada56770513aebb4bc1942d1ff37dcd65"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ac821aeb0763734c9737986367bde1036fd474ca","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e3c8047a_c252b5e1","updated":"2022-11-16 17:40:38.000000000","message":"This is clever. But it is OK from placement perspective. We intentionally allow reserving allocated inventories but not allow allocating reserved inventories. So this is a valid usage of placement.\n\nMy only concerns is having an ironic deployment without cleaning configured but the periodic update_provider_tree set to run infrequently do to nova-compute performance reasons. In this case nova will hold the node for a long time without good reason. \n\n","commit_id":"c098888ada56770513aebb4bc1942d1ff37dcd65"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"32896c12da06d1ca552c2745129a739d86fc1ff9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6280748a_be0fb0f2","updated":"2022-11-17 11:29:46.000000000","message":"in terms of the placement usage while a little unortodox when compared to a vm which has trivial clean up time there is nothing wrong about setting the reserved value after the allocation is made.\n\nit will solve multiple case of the same race condtion as you noted on irc and with the ablity to opt out via a workaround i think it would be backportable.\n\n","commit_id":"c098888ada56770513aebb4bc1942d1ff37dcd65"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"b746d699a9f8a7dc516f6a4559ceb14aac8d3c62","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"09990247_40f7085f","in_reply_to":"e3c8047a_c252b5e1","updated":"2022-11-17 09:13:59.000000000","message":"I am glad its a valid use of placement, I had my doubts 😊\n\nThat is a good point, on those without cleaning. But at least this is \"correct\" in both cases, in that the only problem is slightly reduced capacity until the next periodic.\n\nThis seems more backportable without adding another workaround config for the old behaviour in the case of no automatic cleaning. But that is certainly an option.","commit_id":"c098888ada56770513aebb4bc1942d1ff37dcd65"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"e710fc9dedbad484be99bf2a0203fba177a8acea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f8f43fe6_8c9dfdbd","updated":"2022-11-17 16:57:10.000000000","message":"One meaningful question; but also some spelling/grammar nits around exposed-to-operator strings (e.g. change logs and config descriptions)... we should probably fix those?","commit_id":"3c022e968375c1b2eadf3c2dd7190b9434c6d4c1"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3fac78a369bb1ae33c37a11e77003833ebcb0a37","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"17040ec5_9694f30b","updated":"2022-12-14 12:25:20.000000000","message":"This looks good to me now. The inline nits can be all handled in a follow up","commit_id":"3c022e968375c1b2eadf3c2dd7190b9434c6d4c1"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d923423760c610df280bb0f66a273254cc4ff4ff","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8d3e0c9e_6e9eacfd","updated":"2022-11-17 14:23:59.000000000","message":"im ok with this current version however im happy to re reivew if you feel like adressing the nit.","commit_id":"3c022e968375c1b2eadf3c2dd7190b9434c6d4c1"}],"nova/conf/workarounds.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d4a15646930bb2e772713d7339c5a34be7a2b5a6","unresolved":true,"context_lines":[{"line_number":421,"context_line":"        \u0027skip_reserve_in_use_ironic_nodes\u0027,"},{"line_number":422,"context_line":"        default\u003dFalse,"},{"line_number":423,"context_line":"        help\u003d\"\"\""},{"line_number":424,"context_line":"If you don\u0027t have automatic cleaning on Ironic, by default it"},{"line_number":425,"context_line":"after a previous Nova instance is deleted. If that is a problem you"},{"line_number":426,"context_line":"can stop marking in use ironic nodes as reserved, at this risk of"},{"line_number":427,"context_line":"placement suggesting un-available nodes as a valid candidate."},{"line_number":428,"context_line":"\"\"\"),"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fc80a12_cf785395","line":425,"range":{"start_line":424,"start_character":0,"end_line":425,"end_character":41},"updated":"2022-11-17 12:47:58.000000000","message":"I failed to parse this sentence.","commit_id":"1942cc0659307d50f56c6f817d3beee58b8f3d2b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d923423760c610df280bb0f66a273254cc4ff4ff","unresolved":false,"context_lines":[{"line_number":421,"context_line":"        \u0027skip_reserve_in_use_ironic_nodes\u0027,"},{"line_number":422,"context_line":"        default\u003dFalse,"},{"line_number":423,"context_line":"        help\u003d\"\"\""},{"line_number":424,"context_line":"If you don\u0027t have automatic cleaning on Ironic, by default it"},{"line_number":425,"context_line":"after a previous Nova instance is deleted. If that is a problem you"},{"line_number":426,"context_line":"can stop marking in use ironic nodes as reserved, at this risk of"},{"line_number":427,"context_line":"placement suggesting un-available nodes as a valid candidate."},{"line_number":428,"context_line":"\"\"\"),"}],"source_content_type":"text/x-python","patch_set":2,"id":"5e91a28f_2e4c9ce4","line":425,"range":{"start_line":424,"start_character":0,"end_line":425,"end_character":41},"in_reply_to":"45a2b909_1c4f573b","updated":"2022-11-17 14:23:59.000000000","message":"Done","commit_id":"1942cc0659307d50f56c6f817d3beee58b8f3d2b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"43bf8975d23c4650f8d28235415d1e726e1641d2","unresolved":true,"context_lines":[{"line_number":421,"context_line":"        \u0027skip_reserve_in_use_ironic_nodes\u0027,"},{"line_number":422,"context_line":"        default\u003dFalse,"},{"line_number":423,"context_line":"        help\u003d\"\"\""},{"line_number":424,"context_line":"If you don\u0027t have automatic cleaning on Ironic, by default it"},{"line_number":425,"context_line":"after a previous Nova instance is deleted. If that is a problem you"},{"line_number":426,"context_line":"can stop marking in use ironic nodes as reserved, at this risk of"},{"line_number":427,"context_line":"placement suggesting un-available nodes as a valid candidate."},{"line_number":428,"context_line":"\"\"\"),"}],"source_content_type":"text/x-python","patch_set":2,"id":"45a2b909_1c4f573b","line":425,"range":{"start_line":424,"start_character":0,"end_line":425,"end_character":41},"in_reply_to":"9fc80a12_cf785395","updated":"2022-11-17 14:08:55.000000000","message":"Actually, this is all a bit confusing, I will try write this again.","commit_id":"1942cc0659307d50f56c6f817d3beee58b8f3d2b"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"e710fc9dedbad484be99bf2a0203fba177a8acea","unresolved":true,"context_lines":[{"line_number":424,"context_line":"This may be useful if you use the Ironic driver, but don\u0027t have"},{"line_number":425,"context_line":"automatic cleaning enabled in Ironic. Nova, by default, will mark"},{"line_number":426,"context_line":"Ironic nodes as reserved as soon as they are in use. When you free"},{"line_number":427,"context_line":"the Ironic node (by deleting the nova instance) it takes a while"},{"line_number":428,"context_line":"for Nova to un-reserve that Ironic node in placement. Usually this"},{"line_number":429,"context_line":"is a good idea, because it avoids placement providing an Ironic"},{"line_number":430,"context_line":"as a valid candidate when it is still being cleaned."}],"source_content_type":"text/x-python","patch_set":3,"id":"a30d2f30_0c3b7787","line":427,"updated":"2022-11-17 16:57:10.000000000","message":"nit: consistently capitalize \"Nova\"","commit_id":"3c022e968375c1b2eadf3c2dd7190b9434c6d4c1"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"e710fc9dedbad484be99bf2a0203fba177a8acea","unresolved":true,"context_lines":[{"line_number":428,"context_line":"for Nova to un-reserve that Ironic node in placement. Usually this"},{"line_number":429,"context_line":"is a good idea, because it avoids placement providing an Ironic"},{"line_number":430,"context_line":"as a valid candidate when it is still being cleaned."},{"line_number":431,"context_line":"Howerver, if you don\u0027t use automatic cleaning, it can cause an"},{"line_number":432,"context_line":"extra delay before and Ironic node is available for building a"},{"line_number":433,"context_line":"new Nova instance."},{"line_number":434,"context_line":"\"\"\"),"}],"source_content_type":"text/x-python","patch_set":3,"id":"508105aa_b4a73f11","line":431,"updated":"2022-11-17 16:57:10.000000000","message":"Spelling: However","commit_id":"3c022e968375c1b2eadf3c2dd7190b9434c6d4c1"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d923423760c610df280bb0f66a273254cc4ff4ff","unresolved":false,"context_lines":[{"line_number":430,"context_line":"as a valid candidate when it is still being cleaned."},{"line_number":431,"context_line":"Howerver, if you don\u0027t use automatic cleaning, it can cause an"},{"line_number":432,"context_line":"extra delay before and Ironic node is available for building a"},{"line_number":433,"context_line":"new Nova instance."},{"line_number":434,"context_line":"\"\"\"),"},{"line_number":435,"context_line":"]"},{"line_number":436,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"cd583021_d06b6b62","line":433,"updated":"2022-11-17 14:23:59.000000000","message":"yep this reads much better +1","commit_id":"3c022e968375c1b2eadf3c2dd7190b9434c6d4c1"}],"nova/virt/ironic/driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"32896c12da06d1ca552c2745129a739d86fc1ff9","unresolved":true,"context_lines":[{"line_number":875,"context_line":"        # nodename is the ironic node\u0027s UUID."},{"line_number":876,"context_line":"        node \u003d self._node_from_cache(nodename)"},{"line_number":877,"context_line":"        reserved \u003d False"},{"line_number":878,"context_line":"        if (self._node_resources_used(node) or"},{"line_number":879,"context_line":"            self._node_resources_unavailable(node)):"},{"line_number":880,"context_line":"            # Make resources as reserved once we have"},{"line_number":881,"context_line":"            # and instance here."}],"source_content_type":"text/x-python","patch_set":1,"id":"3373782b_6407758c","line":878,"range":{"start_line":878,"start_character":12,"end_line":878,"end_character":43},"updated":"2022-11-17 11:29:46.000000000","message":"So gibi raised a concern about large clouds with large intervals and cleaning disabled.\n\nAs such, this change could extend the time before a compute node becomes available again which might be undesirable depending on your config and usage patterns.\n\nTo address this I think we likely should add a workaround config option to opt-out of this behaviour.","commit_id":"c098888ada56770513aebb4bc1942d1ff37dcd65"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f873ed23c93afe12e2563b636c6a3d0937dd79a0","unresolved":false,"context_lines":[{"line_number":875,"context_line":"        # nodename is the ironic node\u0027s UUID."},{"line_number":876,"context_line":"        node \u003d self._node_from_cache(nodename)"},{"line_number":877,"context_line":"        reserved \u003d False"},{"line_number":878,"context_line":"        if (self._node_resources_used(node) or"},{"line_number":879,"context_line":"            self._node_resources_unavailable(node)):"},{"line_number":880,"context_line":"            # Make resources as reserved once we have"},{"line_number":881,"context_line":"            # and instance here."}],"source_content_type":"text/x-python","patch_set":1,"id":"0f2dcc48_5a4bb158","line":878,"range":{"start_line":878,"start_character":12,"end_line":878,"end_character":43},"in_reply_to":"3373782b_6407758c","updated":"2022-11-17 12:38:25.000000000","message":"OK, I tried to add one.","commit_id":"c098888ada56770513aebb4bc1942d1ff37dcd65"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"e710fc9dedbad484be99bf2a0203fba177a8acea","unresolved":true,"context_lines":[{"line_number":880,"context_line":"            # Operators might mark a node as in maintainance,"},{"line_number":881,"context_line":"            # even when an instance is on the node,"},{"line_number":882,"context_line":"            # either way lets mark this as reserved"},{"line_number":883,"context_line":"            reserved \u003d True"},{"line_number":884,"context_line":""},{"line_number":885,"context_line":"        if (self._node_resources_used(node) and"},{"line_number":886,"context_line":"            not CONF.workarounds.skip_reserve_in_use_ironic_nodes):"}],"source_content_type":"text/x-python","patch_set":3,"id":"c5fa2bc7_b388b003","line":883,"updated":"2022-11-17 16:57:10.000000000","message":"Does this have any side effects? e.g. could the instance still be deleted?","commit_id":"3c022e968375c1b2eadf3c2dd7190b9434c6d4c1"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6dc16c72e2eb7eb5f6d495b5a568e680391ae56d","unresolved":true,"context_lines":[{"line_number":880,"context_line":"            # Operators might mark a node as in maintainance,"},{"line_number":881,"context_line":"            # even when an instance is on the node,"},{"line_number":882,"context_line":"            # either way lets mark this as reserved"},{"line_number":883,"context_line":"            reserved \u003d True"},{"line_number":884,"context_line":""},{"line_number":885,"context_line":"        if (self._node_resources_used(node) and"},{"line_number":886,"context_line":"            not CONF.workarounds.skip_reserve_in_use_ironic_nodes):"}],"source_content_type":"text/x-python","patch_set":3,"id":"abec2c31_651e8c57","line":883,"in_reply_to":"be6db88d_ae281cc2","updated":"2022-11-17 18:38:09.000000000","message":"yes we designed placment to allow you to increase the reservation if if that would overcommit and the behavior is that no new allcotion will be allowed while in this state but you can remove the old ones.\n\nthere is a bug that this will expose that would be bad for vms but not ironci\n\nnamely because we currently use one allcoation for both the source and destionat host when eveacuatating  it will faile to make that update if we are over commited.\n\n\nhowever ironic does not support evacuate so that wont be a problem here.\n\ni cant find the exact bug for evacuate but this is basicly the same problem\n\nhttps://bugs.launchpad.net/nova/+bug/1924123\n\nbecause ironic does not support move operations its ok.\nif it did that would be a problem if the operation did not use seperate allcoations for the source host and the destinations.\n\n\nwe use a seperate allcoation for cold migrate where one allcotion is held by the instanfce the the second one by the migration uuid. in that toplogy this is not an issue is only an issue if we use a singel allcoation for both which is what we do in evacuate so its a know issue that we shoudl eventually fix.\n\nso its good that you asked but for this specific case there will be no negitive side effect.","commit_id":"3c022e968375c1b2eadf3c2dd7190b9434c6d4c1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"c5a1edf651eb78c99946757c913b684ad28e978d","unresolved":true,"context_lines":[{"line_number":880,"context_line":"            # Operators might mark a node as in maintainance,"},{"line_number":881,"context_line":"            # even when an instance is on the node,"},{"line_number":882,"context_line":"            # either way lets mark this as reserved"},{"line_number":883,"context_line":"            reserved \u003d True"},{"line_number":884,"context_line":""},{"line_number":885,"context_line":"        if (self._node_resources_used(node) and"},{"line_number":886,"context_line":"            not CONF.workarounds.skip_reserve_in_use_ironic_nodes):"}],"source_content_type":"text/x-python","patch_set":3,"id":"be6db88d_ae281cc2","line":883,"in_reply_to":"c5fa2bc7_b388b003","updated":"2022-11-17 17:03:00.000000000","message":"It still allows allocations to be deleted, and apparently, placement was designed such that this kind of thing should work, which is handy, and was news to me.","commit_id":"3c022e968375c1b2eadf3c2dd7190b9434c6d4c1"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d923423760c610df280bb0f66a273254cc4ff4ff","unresolved":true,"context_lines":[{"line_number":882,"context_line":"            # either way lets mark this as reserved"},{"line_number":883,"context_line":"            reserved \u003d True"},{"line_number":884,"context_line":""},{"line_number":885,"context_line":"        if (self._node_resources_used(node) and"},{"line_number":886,"context_line":"            not CONF.workarounds.skip_reserve_in_use_ironic_nodes):"},{"line_number":887,"context_line":"            # Make resources as reserved once we have"},{"line_number":888,"context_line":"            # and instance here."}],"source_content_type":"text/x-python","patch_set":3,"id":"b3b8ebc5_6023e518","line":885,"range":{"start_line":885,"start_character":6,"end_line":885,"end_character":10},"updated":"2022-11-17 14:23:59.000000000","message":"nit: this could be an elif but its areguabel simpler if not as efficent to keep these seperate.","commit_id":"3c022e968375c1b2eadf3c2dd7190b9434c6d4c1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"b7424c5f103721b080f346a900d079cf34c5da1c","unresolved":true,"context_lines":[{"line_number":882,"context_line":"            # either way lets mark this as reserved"},{"line_number":883,"context_line":"            reserved \u003d True"},{"line_number":884,"context_line":""},{"line_number":885,"context_line":"        if (self._node_resources_used(node) and"},{"line_number":886,"context_line":"            not CONF.workarounds.skip_reserve_in_use_ironic_nodes):"},{"line_number":887,"context_line":"            # Make resources as reserved once we have"},{"line_number":888,"context_line":"            # and instance here."}],"source_content_type":"text/x-python","patch_set":3,"id":"c2f0cf68_15847a66","line":885,"range":{"start_line":885,"start_character":6,"end_line":885,"end_character":10},"in_reply_to":"b3b8ebc5_6023e518","updated":"2022-11-17 14:30:20.000000000","message":"Hmm, I hadn\u0027t thought about elif somehow, that is a good idea. Will do that if there is a need to respin.","commit_id":"3c022e968375c1b2eadf3c2dd7190b9434c6d4c1"}],"releasenotes/notes/fix-ironic-scheduler-race-08cf8aba0365f512.yaml":[{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"e710fc9dedbad484be99bf2a0203fba177a8acea","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Fixed when placement returns ironic nodes that have just started automatic"},{"line_number":5,"context_line":"    cleaning as possible valid candidates. This is done by marking all ironic"},{"line_number":6,"context_line":"    nodes with an instance on them as reserved, such that nova only makes them"},{"line_number":7,"context_line":"    available once we have double checked Ironic reports the node as available."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"61ea6534_467719c1","line":4,"updated":"2022-11-17 16:57:10.000000000","message":"nit: consistently capitalize Ironic","commit_id":"3c022e968375c1b2eadf3c2dd7190b9434c6d4c1"}]}