)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":21,"context_line":"  inventories need to be moved, whereupon the resource tracker will"},{"line_number":22,"context_line":"  populate the `allocations` parameter and reinvoke update_provider_tree"},{"line_number":23,"context_line":"  with it."},{"line_number":24,"context_line":"- A new ReshapeNeeded exception is introduced. It is used as a signal by"},{"line_number":25,"context_line":"  the virt driver that inventories need to be moved. It should *only* be"},{"line_number":26,"context_line":"  raised by the virt driver a) on startup, and b) when the `allocations`"},{"line_number":27,"context_line":"  parameter is None."},{"line_number":28,"context_line":"- The compute manager\u0027s _update_available_resource_for_node gets special"},{"line_number":29,"context_line":"  exception handling clauses for ReshapeNeeded and ReshapeFailed, both"},{"line_number":30,"context_line":"  of which blow up the compute service on startup. (On periodic, where"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":36,"id":"3f79a3b5_3e0b067e","line":27,"range":{"start_line":24,"start_character":0,"end_line":27,"end_character":20},"updated":"2018-08-28 20:05:59.000000000","message":"So thinking out loud about the \"what do we do if a driver raises ReshapeNeeded in the periodic task\", one way that could happen via external forces is if let\u0027s say the libvirt driver reshaped and moved VGPU inventory from the parent root compute node provider onto a child provider for VGPU. Then something wasn\u0027t working as the operator expected with VGPU and they started dorking around with osc-placement, and put some VGPU inventory back on the root compute node provider - that would trigger a ReshapeNeeded from the virt driver even though there could be a child provider in the tree that already has VGPU inventory, and the operator messed up the inventory for the tree. I still think in that case that logging the exception but not auto-disabling the compute is good enough for now. If this turns out to be a systemic problem with how operators deal with resource providers when trying to fix problems, we\u0027ll have to reconsider the tool we use when we detect a problem.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e85dc8dbdcf46c7a246089a50a623a6a554dfe67","unresolved":false,"context_lines":[{"line_number":21,"context_line":"  inventories need to be moved, whereupon the resource tracker will"},{"line_number":22,"context_line":"  populate the `allocations` parameter and reinvoke update_provider_tree"},{"line_number":23,"context_line":"  with it."},{"line_number":24,"context_line":"- A new ReshapeNeeded exception is introduced. It is used as a signal by"},{"line_number":25,"context_line":"  the virt driver that inventories need to be moved. It should *only* be"},{"line_number":26,"context_line":"  raised by the virt driver a) on startup, and b) when the `allocations`"},{"line_number":27,"context_line":"  parameter is None."},{"line_number":28,"context_line":"- The compute manager\u0027s _update_available_resource_for_node gets special"},{"line_number":29,"context_line":"  exception handling clauses for ReshapeNeeded and ReshapeFailed, both"},{"line_number":30,"context_line":"  of which blow up the compute service on startup. (On periodic, where"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":36,"id":"3f79a3b5_949799ec","line":27,"range":{"start_line":24,"start_character":0,"end_line":27,"end_character":20},"in_reply_to":"3f79a3b5_3e0b067e","updated":"2018-08-28 21:28:42.000000000","message":"ack","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"}],"nova/compute/manager.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"70c6b7ba7d55b3add50114568b35afb3c082ac70","unresolved":false,"context_lines":[{"line_number":7359,"context_line":"            # anyway."},{"line_number":7360,"context_line":"            self._resource_tracker \u003d None"},{"line_number":7361,"context_line":"            return"},{"line_number":7362,"context_line":"        except exception.ResourceProviderUpdateFailed:"},{"line_number":7363,"context_line":"            with excutils.save_and_reraise_exception() as sare:"},{"line_number":7364,"context_line":"                LOG.exception(\"Error updating resources for node %(node)s.\","},{"line_number":7365,"context_line":"                              {\u0027node\u0027: nodename})"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_80606aa6","line":7362,"range":{"start_line":7362,"start_character":25,"end_line":7362,"end_character":53},"updated":"2018-06-19 12:47:34.000000000","message":"We don\u0027t want to blow up on startup for non-reshape errors from update_from_provider_tree, which is what this is doing.  Need to rework update_from_provider_tree to raise a special exception (or flag ResourceProviderUpdateFailed somehow) for a failed reshape - but just the reshape part, not the self-healable bits - and use that here instead.\n\nAlso this needs to blow up on ReshapeNeeded coming up from update_provider_tree, which happens whenever it\u0027s not caught in _update, i.e. on periodic when we done f\u0027ed up.","commit_id":"0250304d8d7c66f44e428cb19d3b5da95d56c562"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e1ca09191b329a681f12b5617f119d0568cbae72","unresolved":false,"context_lines":[{"line_number":7359,"context_line":"            # anyway."},{"line_number":7360,"context_line":"            self._resource_tracker \u003d None"},{"line_number":7361,"context_line":"            return"},{"line_number":7362,"context_line":"        except exception.ResourceProviderUpdateFailed:"},{"line_number":7363,"context_line":"            with excutils.save_and_reraise_exception() as sare:"},{"line_number":7364,"context_line":"                LOG.exception(\"Error updating resources for node %(node)s.\","},{"line_number":7365,"context_line":"                              {\u0027node\u0027: nodename})"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_10e5b0a4","line":7362,"range":{"start_line":7362,"start_character":25,"end_line":7362,"end_character":53},"in_reply_to":"5f7c97a3_80606aa6","updated":"2018-07-12 15:55:30.000000000","message":"Done.\n\nI created a new exception, ReshapeFailed, which will be raised from update_from_provider_tree only on failure of the POST /reshaper call, which will only be used if we\u0027re actually reshaping, which should only happen on startup. So I\u0027m trapping that exception here and blowing up the service.\n\nAlso trapping ReshapeNeeded here, which isn\u0027t supposed to happen ever, so blowing up the service on that one too.","commit_id":"0250304d8d7c66f44e428cb19d3b5da95d56c562"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"874e2e7cc8d126c078f98945b1f9e004c0e5ae71","unresolved":false,"context_lines":[{"line_number":7751,"context_line":"            # reinvoked update_provider_tree with allocations (whereupon the"},{"line_number":7752,"context_line":"            # driver is just supposed to *do* the reshape). We want to kill the"},{"line_number":7753,"context_line":"            # service."},{"line_number":7754,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":7755,"context_line":"                LOG.exception(\"ReshapeNeeded exception is unexpected here!\")"},{"line_number":7756,"context_line":"        except Exception:"},{"line_number":7757,"context_line":"            LOG.exception(\"Error updating resources for node %(node)s.\","},{"line_number":7758,"context_line":"                          {\u0027node\u0027: nodename})"}],"source_content_type":"text/x-python","patch_set":33,"id":"3f79a3b5_902b73e9","line":7755,"range":{"start_line":7754,"start_character":1,"end_line":7755,"end_character":76},"updated":"2018-08-24 12:37:02.000000000","message":"If we are reaching this point from the periodic task then simply raising an exception will not kill the service. So either the commit message is misleading or this code doesn\u0027t do what we want.","commit_id":"c932ef657b6a906a2c7cb700a717c7c8d1706a93"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a74c6f90a345866102bdb35d0f16bf2e0c7fb294","unresolved":false,"context_lines":[{"line_number":7751,"context_line":"            # reinvoked update_provider_tree with allocations (whereupon the"},{"line_number":7752,"context_line":"            # driver is just supposed to *do* the reshape). We want to kill the"},{"line_number":7753,"context_line":"            # service."},{"line_number":7754,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":7755,"context_line":"                LOG.exception(\"ReshapeNeeded exception is unexpected here!\")"},{"line_number":7756,"context_line":"        except Exception:"},{"line_number":7757,"context_line":"            LOG.exception(\"Error updating resources for node %(node)s.\","},{"line_number":7758,"context_line":"                          {\u0027node\u0027: nodename})"}],"source_content_type":"text/x-python","patch_set":33,"id":"3f79a3b5_ecb3e808","line":7755,"range":{"start_line":7754,"start_character":1,"end_line":7755,"end_character":76},"in_reply_to":"3f79a3b5_902b73e9","updated":"2018-08-24 20:53:07.000000000","message":"We\u0027re in the manager, not the resource tracker, here. re-raising ReshapeNeeded (which will happen with save_and_reraise_exception()) should appropriately kill the manager and thus the service.","commit_id":"c932ef657b6a906a2c7cb700a717c7c8d1706a93"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eabca40bdcdfc2af5c4b211a78f39f6bab0e54c0","unresolved":false,"context_lines":[{"line_number":7751,"context_line":"            # reinvoked update_provider_tree with allocations (whereupon the"},{"line_number":7752,"context_line":"            # driver is just supposed to *do* the reshape). We want to kill the"},{"line_number":7753,"context_line":"            # service."},{"line_number":7754,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":7755,"context_line":"                LOG.exception(\"ReshapeNeeded exception is unexpected here!\")"},{"line_number":7756,"context_line":"        except Exception:"},{"line_number":7757,"context_line":"            LOG.exception(\"Error updating resources for node %(node)s.\","},{"line_number":7758,"context_line":"                          {\u0027node\u0027: nodename})"}],"source_content_type":"text/x-python","patch_set":33,"id":"3f79a3b5_2eb3db0c","line":7755,"range":{"start_line":7754,"start_character":1,"end_line":7755,"end_character":76},"in_reply_to":"3f79a3b5_e3941c8f","updated":"2018-08-28 16:45:09.000000000","message":"I thought we agreed in the spec that we wouldn\u0027t be doing reshapes during periodics, and only on startup? Which is why the startup parameter is passed through.","commit_id":"c932ef657b6a906a2c7cb700a717c7c8d1706a93"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d683e58f4292a4a5eaff0cd0e0807d021ebca262","unresolved":false,"context_lines":[{"line_number":7751,"context_line":"            # reinvoked update_provider_tree with allocations (whereupon the"},{"line_number":7752,"context_line":"            # driver is just supposed to *do* the reshape). We want to kill the"},{"line_number":7753,"context_line":"            # service."},{"line_number":7754,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":7755,"context_line":"                LOG.exception(\"ReshapeNeeded exception is unexpected here!\")"},{"line_number":7756,"context_line":"        except Exception:"},{"line_number":7757,"context_line":"            LOG.exception(\"Error updating resources for node %(node)s.\","},{"line_number":7758,"context_line":"                          {\u0027node\u0027: nodename})"}],"source_content_type":"text/x-python","patch_set":33,"id":"3f79a3b5_e3941c8f","line":7755,"range":{"start_line":7754,"start_character":1,"end_line":7755,"end_character":76},"in_reply_to":"3f79a3b5_ecb3e808","updated":"2018-08-28 16:20:02.000000000","message":"Still this function is called from a periodic task update_available_resource() if we an exception bubble up from a periodic task it does not kill the whole service running the periodic task.\n\nI added some code (see the paste [1]) to trigger the ReshapeNeeded exception from a periodic call and I see in the logs that the periodic call stops due to the exception but the service happily runs forward and starts the periodic task again after the given spacing time passed.\n\n[1] http://paste.openstack.org/show/728980/","commit_id":"c932ef657b6a906a2c7cb700a717c7c8d1706a93"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ea1fbcf07cc58354f46a5e1f6b92a73181f47311","unresolved":false,"context_lines":[{"line_number":7749,"context_line":"            # rather than startup (we only allow reshapes to happen on startup)"},{"line_number":7750,"context_line":"            # or b) after the resource tracker already caught it once and"},{"line_number":7751,"context_line":"            # reinvoked update_provider_tree with allocations (whereupon the"},{"line_number":7752,"context_line":"            # driver is just supposed to *do* the reshape). We want to kill the"},{"line_number":7753,"context_line":"            # service."},{"line_number":7754,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":7755,"context_line":"                LOG.exception(\"ReshapeNeeded exception is unexpected here!\")"},{"line_number":7756,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":35,"id":"3f79a3b5_ae74cb37","line":7753,"range":{"start_line":7752,"start_character":60,"end_line":7753,"end_character":22},"updated":"2018-08-28 16:51:45.000000000","message":"You won\u0027t kill the service from a periodic, it\u0027s running on its own thread. If there is a bug in a driver raising this, that\u0027s on the driver and the bug should be reported, we don\u0027t know what would actually be the side effect of it - so we don\u0027t need to try and re-architect how periodics work to actually kill the service. If inventory is totally wonked on this host, presumably server builds are going to fail which will increment our failed build counter in the compute_node.stats which will make the scheduler weigh this host lower than other non-wonkified hosts.","commit_id":"bc03dc7237cb666362f7c54c8cd27b4ab999beaa"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"aea17c8e65da29e27b24abf9599380e9224c1ac5","unresolved":false,"context_lines":[{"line_number":7749,"context_line":"            # rather than startup (we only allow reshapes to happen on startup)"},{"line_number":7750,"context_line":"            # or b) after the resource tracker already caught it once and"},{"line_number":7751,"context_line":"            # reinvoked update_provider_tree with allocations (whereupon the"},{"line_number":7752,"context_line":"            # driver is just supposed to *do* the reshape). We want to kill the"},{"line_number":7753,"context_line":"            # service."},{"line_number":7754,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":7755,"context_line":"                LOG.exception(\"ReshapeNeeded exception is unexpected here!\")"},{"line_number":7756,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":35,"id":"3f79a3b5_1ef3aae9","line":7753,"range":{"start_line":7752,"start_character":60,"end_line":7753,"end_character":22},"in_reply_to":"3f79a3b5_ae74cb37","updated":"2018-08-28 19:05:09.000000000","message":"Reworded, see if you like.","commit_id":"bc03dc7237cb666362f7c54c8cd27b4ab999beaa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":7742,"context_line":"            # needed, was attempted, and failed. We want to kill the service."},{"line_number":7743,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":7744,"context_line":"                LOG.critical(\"Resource provider data migration failed \""},{"line_number":7745,"context_line":"                             \"fatally during startup for node %s.\", nodename)"},{"line_number":7746,"context_line":"        except exception.ReshapeNeeded:"},{"line_number":7747,"context_line":"            # This exception should only find its way here if the virt driver\u0027s"},{"line_number":7748,"context_line":"            # update_provider_tree raised it incorrectly: either"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_be739616","line":7745,"range":{"start_line":7745,"start_character":38,"end_line":7745,"end_character":52},"updated":"2018-08-28 20:05:59.000000000","message":"Could easily check the actual startup flag to make sure we are hitting this when we think we are. If we got this in some non-startup case, we\u0027d likely just have to log something about getting an unexpected error. Up to you if you want to do anything about this here or in a follow up (or never).","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"98f3905af65ab8c15fcd7f3dc9db9221fb055bf5","unresolved":false,"context_lines":[{"line_number":7742,"context_line":"            # needed, was attempted, and failed. We want to kill the service."},{"line_number":7743,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":7744,"context_line":"                LOG.critical(\"Resource provider data migration failed \""},{"line_number":7745,"context_line":"                             \"fatally during startup for node %s.\", nodename)"},{"line_number":7746,"context_line":"        except exception.ReshapeNeeded:"},{"line_number":7747,"context_line":"            # This exception should only find its way here if the virt driver\u0027s"},{"line_number":7748,"context_line":"            # update_provider_tree raised it incorrectly: either"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_8f2b0632","line":7745,"range":{"start_line":7745,"start_character":38,"end_line":7745,"end_character":52},"in_reply_to":"3f79a3b5_546fc1fb","updated":"2018-08-28 21:40:22.000000000","message":"\u003e Which is excessively paranoid IMO. Thoughts?\n\nThat\u0027s why I hedged with \"or never\". Fine to leave this as-is.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e85dc8dbdcf46c7a246089a50a623a6a554dfe67","unresolved":false,"context_lines":[{"line_number":7742,"context_line":"            # needed, was attempted, and failed. We want to kill the service."},{"line_number":7743,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":7744,"context_line":"                LOG.critical(\"Resource provider data migration failed \""},{"line_number":7745,"context_line":"                             \"fatally during startup for node %s.\", nodename)"},{"line_number":7746,"context_line":"        except exception.ReshapeNeeded:"},{"line_number":7747,"context_line":"            # This exception should only find its way here if the virt driver\u0027s"},{"line_number":7748,"context_line":"            # update_provider_tree raised it incorrectly: either"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_546fc1fb","line":7745,"range":{"start_line":7745,"start_character":38,"end_line":7745,"end_character":52},"in_reply_to":"3f79a3b5_be739616","updated":"2018-08-28 21:28:42.000000000","message":"The only time we should get ReshapeFailed here is when we actually tried to do reshape, and the only way that happens is if we get into update_from_provider_tree with allocations, and the only way that happens is if we passed allocations to update_provider_tree, and the only way that happens is if we responded to ReshapeNeeded from update_provider_tree by retrieving allocations and reinvoking it, and the only way that happens is if the (same) startup flag is True.\n\nSo the only reason to check it again here would be to future-proof against someone mucking something in that sequence, or introducing a different place that uses reshaper or otherwise raises ReshapeFailed. Which is excessively paranoid IMO. Thoughts?","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":7757,"context_line":"            # or may not happen again. Depending on the previous and intended"},{"line_number":7758,"context_line":"            # shape of the providers/inventories, this may not actually cause"},{"line_number":7759,"context_line":"            # any immediately visible symptoms (in terms of scheduling, etc.)"},{"line_number":7760,"context_line":"            # TODO(efried): Make it pop immediately, e.g. disable the service."},{"line_number":7761,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":7762,"context_line":"                LOG.exception(\"ReshapeNeeded exception is unexpected here!\")"},{"line_number":7763,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_be4cf6c8","line":7760,"range":{"start_line":7760,"start_character":12,"end_line":7760,"end_character":78},"updated":"2018-08-28 20:05:59.000000000","message":"Probably shouldn\u0027t have a TODO on this unless we plan on doing it - which we don\u0027t at this time. Probably better to make a NOTE saying what we *could* do if it becomes a problem.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e85dc8dbdcf46c7a246089a50a623a6a554dfe67","unresolved":false,"context_lines":[{"line_number":7757,"context_line":"            # or may not happen again. Depending on the previous and intended"},{"line_number":7758,"context_line":"            # shape of the providers/inventories, this may not actually cause"},{"line_number":7759,"context_line":"            # any immediately visible symptoms (in terms of scheduling, etc.)"},{"line_number":7760,"context_line":"            # TODO(efried): Make it pop immediately, e.g. disable the service."},{"line_number":7761,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":7762,"context_line":"                LOG.exception(\"ReshapeNeeded exception is unexpected here!\")"},{"line_number":7763,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_f4736de7","line":7760,"range":{"start_line":7760,"start_character":12,"end_line":7760,"end_character":78},"in_reply_to":"3f79a3b5_be4cf6c8","updated":"2018-08-28 21:28:42.000000000","message":"Ight. Will fup.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0911e8e64ee4aaf649021252d460d98b19a9a335","unresolved":false,"context_lines":[{"line_number":7757,"context_line":"            # or may not happen again. Depending on the previous and intended"},{"line_number":7758,"context_line":"            # shape of the providers/inventories, this may not actually cause"},{"line_number":7759,"context_line":"            # any immediately visible symptoms (in terms of scheduling, etc.)"},{"line_number":7760,"context_line":"            # TODO(efried): Make it pop immediately, e.g. disable the service."},{"line_number":7761,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":7762,"context_line":"                LOG.exception(\"ReshapeNeeded exception is unexpected here!\")"},{"line_number":7763,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_b09948d1","line":7760,"range":{"start_line":7760,"start_character":12,"end_line":7760,"end_character":78},"in_reply_to":"3f79a3b5_f4736de7","updated":"2018-08-30 22:38:03.000000000","message":"Done","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"}],"nova/compute/resource_tracker.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9affc2793777b9800a626dff3064b9682cd3ba0d","unresolved":false,"context_lines":[{"line_number":891,"context_line":"                self.driver.update_provider_tree(prov_tree, nodename)"},{"line_number":892,"context_line":"            except exception.ReshapeNeeded:"},{"line_number":893,"context_line":"                if not startup:"},{"line_number":894,"context_line":"                    # Treat this like a regular exception during periodic"},{"line_number":895,"context_line":"                    raise"},{"line_number":896,"context_line":"                LOG.info(\"Performing resource provider inventory and \""},{"line_number":897,"context_line":"                         \"allocation data migration during compute service \""}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_85f07c68","line":894,"range":{"start_line":894,"start_character":22,"end_line":894,"end_character":73},"updated":"2018-06-19 11:49:49.000000000","message":"Log a special message before reraising.\n\nhttps://review.openstack.org/#/c/572583/11/specs/rocky/approved/reshape-provider-tree.rst@261","commit_id":"0250304d8d7c66f44e428cb19d3b5da95d56c562"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e1ca09191b329a681f12b5617f119d0568cbae72","unresolved":false,"context_lines":[{"line_number":891,"context_line":"                self.driver.update_provider_tree(prov_tree, nodename)"},{"line_number":892,"context_line":"            except exception.ReshapeNeeded:"},{"line_number":893,"context_line":"                if not startup:"},{"line_number":894,"context_line":"                    # Treat this like a regular exception during periodic"},{"line_number":895,"context_line":"                    raise"},{"line_number":896,"context_line":"                LOG.info(\"Performing resource provider inventory and \""},{"line_number":897,"context_line":"                         \"allocation data migration during compute service \""}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_d0fe5838","line":894,"range":{"start_line":894,"start_character":22,"end_line":894,"end_character":73},"in_reply_to":"5f7c97a3_85f07c68","updated":"2018-07-12 15:55:30.000000000","message":"Done. (Actually logging it in the compute manager catch-all.)","commit_id":"0250304d8d7c66f44e428cb19d3b5da95d56c562"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1c7a599f5f43267e859e5a0f916a9d5eb6781426","unresolved":false,"context_lines":[{"line_number":892,"context_line":"            except exception.ReshapeNeeded:"},{"line_number":893,"context_line":"                if not startup:"},{"line_number":894,"context_line":"                    # This isn\u0027t supposed to happen during periodic, so raise"},{"line_number":895,"context_line":"                    # it up; the compute manager will treat it specially,"},{"line_number":896,"context_line":"                    # killing the compute service."},{"line_number":897,"context_line":"                    raise"},{"line_number":898,"context_line":"                LOG.info(\"Performing resource provider inventory and \""}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_f7f06194","line":895,"range":{"start_line":895,"start_character":49,"end_line":895,"end_character":73},"updated":"2018-08-02 12:20:15.000000000","message":"As far as I see the compute manager just let it bubble up and assumes that will kill the starting service.","commit_id":"a46daed64b9ca92ea5755ce422e6063320b8c466"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"4ee8f90ac3bef7a33da4a88ef9065bc39ef407be","unresolved":false,"context_lines":[{"line_number":892,"context_line":"            except exception.ReshapeNeeded:"},{"line_number":893,"context_line":"                if not startup:"},{"line_number":894,"context_line":"                    # This isn\u0027t supposed to happen during periodic, so raise"},{"line_number":895,"context_line":"                    # it up; the compute manager will treat it specially,"},{"line_number":896,"context_line":"                    # killing the compute service."},{"line_number":897,"context_line":"                    raise"},{"line_number":898,"context_line":"                LOG.info(\"Performing resource provider inventory and \""}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_f2638f43","line":895,"range":{"start_line":895,"start_character":49,"end_line":895,"end_character":73},"in_reply_to":"5f7c97a3_f7f06194","updated":"2018-08-02 13:06:10.000000000","message":"Right, which is treating it specially; everything else it traps and ignores.","commit_id":"a46daed64b9ca92ea5755ce422e6063320b8c466"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"874e2e7cc8d126c078f98945b1f9e004c0e5ae71","unresolved":false,"context_lines":[{"line_number":902,"context_line":"                if not startup:"},{"line_number":903,"context_line":"                    # This isn\u0027t supposed to happen during periodic, so raise"},{"line_number":904,"context_line":"                    # it up; the compute manager will treat it specially,"},{"line_number":905,"context_line":"                    # killing the compute service."},{"line_number":906,"context_line":"                    raise"},{"line_number":907,"context_line":"                LOG.info(\"Performing resource provider inventory and \""},{"line_number":908,"context_line":"                         \"allocation data migration during compute service \""}],"source_content_type":"text/x-python","patch_set":33,"id":"3f79a3b5_509b9b15","line":905,"range":{"start_line":905,"start_character":22,"end_line":905,"end_character":50},"updated":"2018-08-24 12:37:02.000000000","message":"it won\u0027t kill the service if we are in a periodic task.","commit_id":"c932ef657b6a906a2c7cb700a717c7c8d1706a93"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a74c6f90a345866102bdb35d0f16bf2e0c7fb294","unresolved":false,"context_lines":[{"line_number":902,"context_line":"                if not startup:"},{"line_number":903,"context_line":"                    # This isn\u0027t supposed to happen during periodic, so raise"},{"line_number":904,"context_line":"                    # it up; the compute manager will treat it specially,"},{"line_number":905,"context_line":"                    # killing the compute service."},{"line_number":906,"context_line":"                    raise"},{"line_number":907,"context_line":"                LOG.info(\"Performing resource provider inventory and \""},{"line_number":908,"context_line":"                         \"allocation data migration during compute service \""}],"source_content_type":"text/x-python","patch_set":33,"id":"3f79a3b5_6c759831","line":905,"range":{"start_line":905,"start_character":22,"end_line":905,"end_character":50},"in_reply_to":"3f79a3b5_509b9b15","updated":"2018-08-24 20:53:07.000000000","message":"why not? I believe the only reason the existing periodic update_available_resource() call doesn\u0027t kill the manager is because of the:\n\n except Exception:\n    LOG.error(...)\n\nbehaviour, but as noted in the manager.py file, Eric is re-raising ReshapeNeeded out of that block, meaning the:\n\n except Exception:\n\nwon\u0027t be triggered.","commit_id":"c932ef657b6a906a2c7cb700a717c7c8d1706a93"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"aea17c8e65da29e27b24abf9599380e9224c1ac5","unresolved":false,"context_lines":[{"line_number":902,"context_line":"                if not startup:"},{"line_number":903,"context_line":"                    # This isn\u0027t supposed to happen during periodic, so raise"},{"line_number":904,"context_line":"                    # it up; the compute manager will treat it specially,"},{"line_number":905,"context_line":"                    # killing the compute service."},{"line_number":906,"context_line":"                    raise"},{"line_number":907,"context_line":"                LOG.info(\"Performing resource provider inventory and \""},{"line_number":908,"context_line":"                         \"allocation data migration during compute service \""}],"source_content_type":"text/x-python","patch_set":33,"id":"3f79a3b5_3eb3060d","line":905,"range":{"start_line":905,"start_character":22,"end_line":905,"end_character":50},"in_reply_to":"3f79a3b5_63c48c0f","updated":"2018-08-28 19:05:09.000000000","message":"okay, I took out this line and reworded the comment in the trapping clause.","commit_id":"c932ef657b6a906a2c7cb700a717c7c8d1706a93"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d683e58f4292a4a5eaff0cd0e0807d021ebca262","unresolved":false,"context_lines":[{"line_number":902,"context_line":"                if not startup:"},{"line_number":903,"context_line":"                    # This isn\u0027t supposed to happen during periodic, so raise"},{"line_number":904,"context_line":"                    # it up; the compute manager will treat it specially,"},{"line_number":905,"context_line":"                    # killing the compute service."},{"line_number":906,"context_line":"                    raise"},{"line_number":907,"context_line":"                LOG.info(\"Performing resource provider inventory and \""},{"line_number":908,"context_line":"                         \"allocation data migration during compute service \""}],"source_content_type":"text/x-python","patch_set":33,"id":"3f79a3b5_63c48c0f","line":905,"range":{"start_line":905,"start_character":22,"end_line":905,"end_character":50},"in_reply_to":"3f79a3b5_6c759831","updated":"2018-08-28 16:20:02.000000000","message":"I tried and I see that the service runs forward after the exception is bubbled up. See my reply with details on reproducing the situation in the comment in the manager.py\n\nI think the reason is that the oslo.service periodic task handler catches all exceptions [1] by default [2].\n\n[1] https://github.com/openstack/oslo.service/blob/eb70bf64708a722989545a78c4b1a9f3e49f6221/oslo_service/periodic_task.py#L222\n\n[2] https://github.com/openstack/oslo.service/blob/eb70bf64708a722989545a78c4b1a9f3e49f6221/oslo_service/periodic_task.py#L193","commit_id":"c932ef657b6a906a2c7cb700a717c7c8d1706a93"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":899,"context_line":"            try:"},{"line_number":900,"context_line":"                self.driver.update_provider_tree(prov_tree, nodename)"},{"line_number":901,"context_line":"            except exception.ReshapeNeeded:"},{"line_number":902,"context_line":"                if not startup:"},{"line_number":903,"context_line":"                    # This isn\u0027t supposed to happen during periodic, so raise"},{"line_number":904,"context_line":"                    # it up; the compute manager will treat it specially."},{"line_number":905,"context_line":"                    raise"},{"line_number":906,"context_line":"                LOG.info(\"Performing resource provider inventory and \""},{"line_number":907,"context_line":"                         \"allocation data migration during compute service \""},{"line_number":908,"context_line":"                         \"startup or FFU.\")"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_def77259","line":905,"range":{"start_line":902,"start_character":16,"end_line":905,"end_character":25},"updated":"2018-08-28 20:05:59.000000000","message":"nit: another case for excutils.save_and_reraise_exception here, if not startup you can change the context to no reraise.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e85dc8dbdcf46c7a246089a50a623a6a554dfe67","unresolved":false,"context_lines":[{"line_number":899,"context_line":"            try:"},{"line_number":900,"context_line":"                self.driver.update_provider_tree(prov_tree, nodename)"},{"line_number":901,"context_line":"            except exception.ReshapeNeeded:"},{"line_number":902,"context_line":"                if not startup:"},{"line_number":903,"context_line":"                    # This isn\u0027t supposed to happen during periodic, so raise"},{"line_number":904,"context_line":"                    # it up; the compute manager will treat it specially."},{"line_number":905,"context_line":"                    raise"},{"line_number":906,"context_line":"                LOG.info(\"Performing resource provider inventory and \""},{"line_number":907,"context_line":"                         \"allocation data migration during compute service \""},{"line_number":908,"context_line":"                         \"startup or FFU.\")"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_74499d8c","line":905,"range":{"start_line":902,"start_character":16,"end_line":905,"end_character":25},"in_reply_to":"3f79a3b5_def77259","updated":"2018-08-28 21:28:42.000000000","message":"Thought it was overkill here, since the point of save_and_reraise_exception is to guard against exception context getting swizzled, and a trivial boolean check won\u0027t do that.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":905,"context_line":"                    raise"},{"line_number":906,"context_line":"                LOG.info(\"Performing resource provider inventory and \""},{"line_number":907,"context_line":"                         \"allocation data migration during compute service \""},{"line_number":908,"context_line":"                         \"startup or FFU.\")"},{"line_number":909,"context_line":"                allocs \u003d reportclient.get_allocations_for_provider_tree("},{"line_number":910,"context_line":"                    context, nodename)"},{"line_number":911,"context_line":"                self.driver.update_provider_tree(prov_tree, nodename,"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_9e97baec","line":908,"range":{"start_line":908,"start_character":37,"end_line":908,"end_character":40},"updated":"2018-08-28 20:05:59.000000000","message":"nit: let\u0027s spell this out: fast-forward upgrade","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0911e8e64ee4aaf649021252d460d98b19a9a335","unresolved":false,"context_lines":[{"line_number":905,"context_line":"                    raise"},{"line_number":906,"context_line":"                LOG.info(\"Performing resource provider inventory and \""},{"line_number":907,"context_line":"                         \"allocation data migration during compute service \""},{"line_number":908,"context_line":"                         \"startup or FFU.\")"},{"line_number":909,"context_line":"                allocs \u003d reportclient.get_allocations_for_provider_tree("},{"line_number":910,"context_line":"                    context, nodename)"},{"line_number":911,"context_line":"                self.driver.update_provider_tree(prov_tree, nodename,"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_30ad38ea","line":908,"range":{"start_line":908,"start_character":37,"end_line":908,"end_character":40},"in_reply_to":"3f79a3b5_3443256f","updated":"2018-08-30 22:38:03.000000000","message":"Done","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e85dc8dbdcf46c7a246089a50a623a6a554dfe67","unresolved":false,"context_lines":[{"line_number":905,"context_line":"                    raise"},{"line_number":906,"context_line":"                LOG.info(\"Performing resource provider inventory and \""},{"line_number":907,"context_line":"                         \"allocation data migration during compute service \""},{"line_number":908,"context_line":"                         \"startup or FFU.\")"},{"line_number":909,"context_line":"                allocs \u003d reportclient.get_allocations_for_provider_tree("},{"line_number":910,"context_line":"                    context, nodename)"},{"line_number":911,"context_line":"                self.driver.update_provider_tree(prov_tree, nodename,"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_3443256f","line":908,"range":{"start_line":908,"start_character":37,"end_line":908,"end_character":40},"in_reply_to":"3f79a3b5_9e97baec","updated":"2018-08-28 21:28:42.000000000","message":"ack","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":918,"context_line":"            inv_data \u003d prov_tree.data(nodename).inventory"},{"line_number":919,"context_line":"            _normalize_inventory_from_cn_obj(inv_data, compute_node)"},{"line_number":920,"context_line":"            prov_tree.update_inventory(nodename, inv_data)"},{"line_number":921,"context_line":"            # Flush any changes."},{"line_number":922,"context_line":"            reportclient.update_from_provider_tree(context, prov_tree,"},{"line_number":923,"context_line":"                                                   allocations\u003dallocs)"},{"line_number":924,"context_line":"        except NotImplementedError:"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_feaeee2d","line":921,"updated":"2018-08-28 20:05:59.000000000","message":"nit: could be a good reminder comment to add that if allocs is not None, this is where update_from_provider_tree is going to actually hit the reshape API in Placement.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0911e8e64ee4aaf649021252d460d98b19a9a335","unresolved":false,"context_lines":[{"line_number":918,"context_line":"            inv_data \u003d prov_tree.data(nodename).inventory"},{"line_number":919,"context_line":"            _normalize_inventory_from_cn_obj(inv_data, compute_node)"},{"line_number":920,"context_line":"            prov_tree.update_inventory(nodename, inv_data)"},{"line_number":921,"context_line":"            # Flush any changes."},{"line_number":922,"context_line":"            reportclient.update_from_provider_tree(context, prov_tree,"},{"line_number":923,"context_line":"                                                   allocations\u003dallocs)"},{"line_number":924,"context_line":"        except NotImplementedError:"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_5091d499","line":921,"in_reply_to":"3f79a3b5_54482186","updated":"2018-08-30 22:38:03.000000000","message":"Done","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e85dc8dbdcf46c7a246089a50a623a6a554dfe67","unresolved":false,"context_lines":[{"line_number":918,"context_line":"            inv_data \u003d prov_tree.data(nodename).inventory"},{"line_number":919,"context_line":"            _normalize_inventory_from_cn_obj(inv_data, compute_node)"},{"line_number":920,"context_line":"            prov_tree.update_inventory(nodename, inv_data)"},{"line_number":921,"context_line":"            # Flush any changes."},{"line_number":922,"context_line":"            reportclient.update_from_provider_tree(context, prov_tree,"},{"line_number":923,"context_line":"                                                   allocations\u003dallocs)"},{"line_number":924,"context_line":"        except NotImplementedError:"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_54482186","line":921,"in_reply_to":"3f79a3b5_feaeee2d","updated":"2018-08-28 21:28:42.000000000","message":"okay.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":939,"context_line":"                self.scheduler_client.update_compute_node(context,"},{"line_number":940,"context_line":"                                                          compute_node)"},{"line_number":941,"context_line":""},{"line_number":942,"context_line":"    @retrying.retry(stop_max_attempt_number\u003d4,"},{"line_number":943,"context_line":"                    retry_on_exception\u003dlambda e: isinstance("},{"line_number":944,"context_line":"                        e, exception.ResourceProviderUpdateConflict))"},{"line_number":945,"context_line":"    def _update(self, context, compute_node, startup\u003dFalse):"},{"line_number":946,"context_line":"        \"\"\"Update partial stats locally and populate them to Scheduler.\"\"\""},{"line_number":947,"context_line":"        if self._resource_change(compute_node):"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_9ef25a6f","line":944,"range":{"start_line":942,"start_character":4,"end_line":944,"end_character":69},"updated":"2018-08-28 20:05:59.000000000","message":"This shouldn\u0027t interfere in any weird ways with reshaping right? We would only retry if we get ResourceProviderUpdateConflict, which I\u0027m assuming could happen during a reshape, but maybe doesn\u0027t hurt?","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"98f3905af65ab8c15fcd7f3dc9db9221fb055bf5","unresolved":false,"context_lines":[{"line_number":939,"context_line":"                self.scheduler_client.update_compute_node(context,"},{"line_number":940,"context_line":"                                                          compute_node)"},{"line_number":941,"context_line":""},{"line_number":942,"context_line":"    @retrying.retry(stop_max_attempt_number\u003d4,"},{"line_number":943,"context_line":"                    retry_on_exception\u003dlambda e: isinstance("},{"line_number":944,"context_line":"                        e, exception.ResourceProviderUpdateConflict))"},{"line_number":945,"context_line":"    def _update(self, context, compute_node, startup\u003dFalse):"},{"line_number":946,"context_line":"        \"\"\"Update partial stats locally and populate them to Scheduler.\"\"\""},{"line_number":947,"context_line":"        if self._resource_change(compute_node):"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_8fd0e62d","line":944,"range":{"start_line":942,"start_character":4,"end_line":944,"end_character":69},"in_reply_to":"3f79a3b5_2f0d1235","updated":"2018-08-28 21:40:22.000000000","message":"Cool, well I\u0027m glad I asked. :)","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e85dc8dbdcf46c7a246089a50a623a6a554dfe67","unresolved":false,"context_lines":[{"line_number":939,"context_line":"                self.scheduler_client.update_compute_node(context,"},{"line_number":940,"context_line":"                                                          compute_node)"},{"line_number":941,"context_line":""},{"line_number":942,"context_line":"    @retrying.retry(stop_max_attempt_number\u003d4,"},{"line_number":943,"context_line":"                    retry_on_exception\u003dlambda e: isinstance("},{"line_number":944,"context_line":"                        e, exception.ResourceProviderUpdateConflict))"},{"line_number":945,"context_line":"    def _update(self, context, compute_node, startup\u003dFalse):"},{"line_number":946,"context_line":"        \"\"\"Update partial stats locally and populate them to Scheduler.\"\"\""},{"line_number":947,"context_line":"        if self._resource_change(compute_node):"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_2f0d1235","line":944,"range":{"start_line":942,"start_character":4,"end_line":944,"end_character":69},"in_reply_to":"3f79a3b5_9ef25a6f","updated":"2018-08-28 21:28:42.000000000","message":"Right, in fact it helps. I did actually think this through, so thanks for giving me the opportunity to expound:\n\nResourceProviderUpdateConflict can only happen in the ufpt code that precedes or follows the actual reshaper call. doesn\u0027t happen from the actual reshaper call. Only ReshapeFailed can happen from there. And that\u0027ll blow past this like we want it to.\n\nSo the retry gives us the opportunity to re-GET the world and redrive the whole upt/ufpt sequence. If the failure was *before* the reshaper call, presumably upt will request the reshape again, which is fine. If the failure was *after* the reshaper call, presumably upt will see that a reshape isn\u0027t needed, but still update the bits of the provider tree that didn\u0027t get finished up afterwards.\n\nIn either case, the outcome is supposed to be that we tried a number of times to make the right thing happen to the provider tree in placement, and only truly failed if a) ReshapeFailed, or b) we ran out of retries.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"}],"nova/exception.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":2386,"context_line":""},{"line_number":2387,"context_line":""},{"line_number":2388,"context_line":"class ReshapeNeeded(NovaException):"},{"line_number":2389,"context_line":"    msg_fmt \u003d _(\"Virt driver indicates that provider inventories need to be \""},{"line_number":2390,"context_line":"                \"moved.\")"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_bea87620","line":2389,"range":{"start_line":2389,"start_character":53,"end_line":2389,"end_character":64},"updated":"2018-08-28 20:05:59.000000000","message":"nit: inventories/allocations or \"inventories and/or allocations\"","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0911e8e64ee4aaf649021252d460d98b19a9a335","unresolved":false,"context_lines":[{"line_number":2386,"context_line":""},{"line_number":2387,"context_line":""},{"line_number":2388,"context_line":"class ReshapeNeeded(NovaException):"},{"line_number":2389,"context_line":"    msg_fmt \u003d _(\"Virt driver indicates that provider inventories need to be \""},{"line_number":2390,"context_line":"                \"moved.\")"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_f0e4a03e","line":2389,"range":{"start_line":2389,"start_character":53,"end_line":2389,"end_character":64},"in_reply_to":"3f79a3b5_4fdace4b","updated":"2018-08-30 22:38:03.000000000","message":"Leaving it alone","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e85dc8dbdcf46c7a246089a50a623a6a554dfe67","unresolved":false,"context_lines":[{"line_number":2386,"context_line":""},{"line_number":2387,"context_line":""},{"line_number":2388,"context_line":"class ReshapeNeeded(NovaException):"},{"line_number":2389,"context_line":"    msg_fmt \u003d _(\"Virt driver indicates that provider inventories need to be \""},{"line_number":2390,"context_line":"                \"moved.\")"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_cf325ef3","line":2389,"range":{"start_line":2389,"start_character":53,"end_line":2389,"end_character":64},"in_reply_to":"3f79a3b5_bea87620","updated":"2018-08-28 21:28:42.000000000","message":"Considered that, but a) it\u0027s always inventories, and only sometimes allocations; b) allocations are implied by inventories; but mainly c) the virt driver can\u0027t know by the time it raises this exception whether there are allocations to be moved, so it could be lying if it said that was the case.\n\nYou\u0027re right, and/or covers it, more or less, but I didn\u0027t think it was necessary. Thoughts?","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"98f3905af65ab8c15fcd7f3dc9db9221fb055bf5","unresolved":false,"context_lines":[{"line_number":2386,"context_line":""},{"line_number":2387,"context_line":""},{"line_number":2388,"context_line":"class ReshapeNeeded(NovaException):"},{"line_number":2389,"context_line":"    msg_fmt \u003d _(\"Virt driver indicates that provider inventories need to be \""},{"line_number":2390,"context_line":"                \"moved.\")"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_4fdace4b","line":2389,"range":{"start_line":2389,"start_character":53,"end_line":2389,"end_character":64},"in_reply_to":"3f79a3b5_cf325ef3","updated":"2018-08-28 21:40:22.000000000","message":"Big ol meh from me on changing this.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"}],"nova/scheduler/client/report.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"5909002d4fa5b8a08b9c275074695e7860dff011","unresolved":false,"context_lines":[{"line_number":1361,"context_line":"        :raises: ResourceProviderSyncFailed if any errors were encountered"},{"line_number":1362,"context_line":"                 attempting to perform the necessary API operations."},{"line_number":1363,"context_line":"        \"\"\""},{"line_number":1364,"context_line":"        # NOTE(efried): We currently do not handle the \"rename\" case.  This is"},{"line_number":1365,"context_line":"        # where new_tree contains a provider named Y whose UUID already exists"},{"line_number":1366,"context_line":"        # but is named X.  Today the only way the consumer could accomplish"},{"line_number":1367,"context_line":"        # this is by deleting the provider and recreating it with the new name."},{"line_number":1368,"context_line":""},{"line_number":1369,"context_line":"        @contextlib.contextmanager"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_06a57103","side":"PARENT","line":1366,"range":{"start_line":1364,"start_character":0,"end_line":1366,"end_character":26},"updated":"2018-07-12 20:26:04.000000000","message":"This is still true; restore this comment.","commit_id":"da16690f4db6172659ffa4f804296100b5ba24d6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d3ff53d368f9bca97fce9c4916c1a9b85406e266","unresolved":false,"context_lines":[{"line_number":1361,"context_line":"        :raises: ResourceProviderSyncFailed if any errors were encountered"},{"line_number":1362,"context_line":"                 attempting to perform the necessary API operations."},{"line_number":1363,"context_line":"        \"\"\""},{"line_number":1364,"context_line":"        # NOTE(efried): We currently do not handle the \"rename\" case.  This is"},{"line_number":1365,"context_line":"        # where new_tree contains a provider named Y whose UUID already exists"},{"line_number":1366,"context_line":"        # but is named X.  Today the only way the consumer could accomplish"},{"line_number":1367,"context_line":"        # this is by deleting the provider and recreating it with the new name."},{"line_number":1368,"context_line":""},{"line_number":1369,"context_line":"        @contextlib.contextmanager"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_c9330af7","side":"PARENT","line":1366,"range":{"start_line":1364,"start_character":0,"end_line":1366,"end_character":26},"in_reply_to":"5f7c97a3_06a57103","updated":"2018-07-12 20:28:57.000000000","message":"Done","commit_id":"da16690f4db6172659ffa4f804296100b5ba24d6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"5909002d4fa5b8a08b9c275074695e7860dff011","unresolved":false,"context_lines":[{"line_number":1363,"context_line":"        \"\"\""},{"line_number":1364,"context_line":"        # NOTE(efried): We currently do not handle the \"rename\" case.  This is"},{"line_number":1365,"context_line":"        # where new_tree contains a provider named Y whose UUID already exists"},{"line_number":1366,"context_line":"        # but is named X.  Today the only way the consumer could accomplish"},{"line_number":1367,"context_line":"        # this is by deleting the provider and recreating it with the new name."},{"line_number":1368,"context_line":""},{"line_number":1369,"context_line":"        @contextlib.contextmanager"},{"line_number":1370,"context_line":"        def catch_all(rp_uuid):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_a6a91ddf","side":"PARENT","line":1367,"range":{"start_line":1366,"start_character":27,"end_line":1367,"end_character":79},"updated":"2018-07-12 20:26:04.000000000","message":"But this isn\u0027t really true; remove this sentence.","commit_id":"da16690f4db6172659ffa4f804296100b5ba24d6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d3ff53d368f9bca97fce9c4916c1a9b85406e266","unresolved":false,"context_lines":[{"line_number":1363,"context_line":"        \"\"\""},{"line_number":1364,"context_line":"        # NOTE(efried): We currently do not handle the \"rename\" case.  This is"},{"line_number":1365,"context_line":"        # where new_tree contains a provider named Y whose UUID already exists"},{"line_number":1366,"context_line":"        # but is named X.  Today the only way the consumer could accomplish"},{"line_number":1367,"context_line":"        # this is by deleting the provider and recreating it with the new name."},{"line_number":1368,"context_line":""},{"line_number":1369,"context_line":"        @contextlib.contextmanager"},{"line_number":1370,"context_line":"        def catch_all(rp_uuid):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_6930b6ea","side":"PARENT","line":1367,"range":{"start_line":1366,"start_character":27,"end_line":1367,"end_character":79},"in_reply_to":"5f7c97a3_a6a91ddf","updated":"2018-07-12 20:28:57.000000000","message":"Done","commit_id":"da16690f4db6172659ffa4f804296100b5ba24d6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8baec175720a38ccd545fbd8386fc477db7f714d","unresolved":false,"context_lines":[{"line_number":1487,"context_line":"                        old_tree.data(rp_uuid).generation"},{"line_number":1488,"context_line":"                }"},{"line_number":1489,"context_line":"            # Now we\u0027re ready to POST /reshaper. This can raise ReshapeFailed."},{"line_number":1490,"context_line":"            self._reshape(context, inventories, allocations)"},{"line_number":1491,"context_line":""},{"line_number":1492,"context_line":"        # Now we can do provider deletions, because we should have moved any"},{"line_number":1493,"context_line":"        # allocations off of them via reshape."}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_f762efd8","line":1490,"range":{"start_line":1490,"start_character":12,"end_line":1490,"end_character":60},"updated":"2018-07-18 12:27:38.000000000","message":"Need to re-GET all the RPs in new_uuids after this call. See below.","commit_id":"15c3861634a78eefaf53c1abbd054145ff119653"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2547e94927f38b8b0d06438dc158d8f166960006","unresolved":false,"context_lines":[{"line_number":1487,"context_line":"                        old_tree.data(rp_uuid).generation"},{"line_number":1488,"context_line":"                }"},{"line_number":1489,"context_line":"            # Now we\u0027re ready to POST /reshaper. This can raise ReshapeFailed."},{"line_number":1490,"context_line":"            self._reshape(context, inventories, allocations)"},{"line_number":1491,"context_line":""},{"line_number":1492,"context_line":"        # Now we can do provider deletions, because we should have moved any"},{"line_number":1493,"context_line":"        # allocations off of them via reshape."}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_5c4b5527","line":1490,"range":{"start_line":1490,"start_character":12,"end_line":1490,"end_character":60},"in_reply_to":"5f7c97a3_f762efd8","updated":"2018-07-23 22:14:43.000000000","message":"Done","commit_id":"15c3861634a78eefaf53c1abbd054145ff119653"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8baec175720a38ccd545fbd8386fc477db7f714d","unresolved":false,"context_lines":[{"line_number":1500,"context_line":"                self._delete_provider(uuid)"},{"line_number":1501,"context_line":"            success \u003d success and status.success"},{"line_number":1502,"context_line":""},{"line_number":1503,"context_line":"        # At this point the local cache should have all the same providers as"},{"line_number":1504,"context_line":"        # new_tree.  Whether we added them or not, walk through and diff/flush"},{"line_number":1505,"context_line":"        # inventories, traits, and aggregates as necessary. Note that, if we"},{"line_number":1506,"context_line":"        # reshaped above, any inventory changes have already been done. But the"},{"line_number":1507,"context_line":"        # helper methods are set up to check and short out when the relevant"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_5772c3b1","line":1504,"range":{"start_line":1503,"start_character":10,"end_line":1504,"end_character":19},"updated":"2018-07-18 12:27:38.000000000","message":"Yeah, but if we reshaped, they have the wrong generations. See above.","commit_id":"15c3861634a78eefaf53c1abbd054145ff119653"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2547e94927f38b8b0d06438dc158d8f166960006","unresolved":false,"context_lines":[{"line_number":1500,"context_line":"                self._delete_provider(uuid)"},{"line_number":1501,"context_line":"            success \u003d success and status.success"},{"line_number":1502,"context_line":""},{"line_number":1503,"context_line":"        # At this point the local cache should have all the same providers as"},{"line_number":1504,"context_line":"        # new_tree.  Whether we added them or not, walk through and diff/flush"},{"line_number":1505,"context_line":"        # inventories, traits, and aggregates as necessary. Note that, if we"},{"line_number":1506,"context_line":"        # reshaped above, any inventory changes have already been done. But the"},{"line_number":1507,"context_line":"        # helper methods are set up to check and short out when the relevant"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_fc590100","line":1504,"range":{"start_line":1503,"start_character":10,"end_line":1504,"end_character":19},"in_reply_to":"5f7c97a3_5772c3b1","updated":"2018-07-23 22:14:43.000000000","message":"Done","commit_id":"15c3861634a78eefaf53c1abbd054145ff119653"}],"nova/tests/functional/test_servers.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"dcd1771a1fce49ee9540dc8e5547c4fd40b970f6","unresolved":false,"context_lines":[{"line_number":1807,"context_line":"            mock.call(mock.ANY, \u0027host1\u0027, allocations\u003d{}),"},{"line_number":1808,"context_line":"        ])"},{"line_number":1809,"context_line":""},{"line_number":1810,"context_line":"    def test_reshape_smoke(self):"},{"line_number":1811,"context_line":"        \"\"\"On startup, the virt driver signals it needs to reshape."},{"line_number":1812,"context_line":""},{"line_number":1813,"context_line":"        Okay, not really on actual startup. That still happens via setUp. We"}],"source_content_type":"text/x-python","patch_set":19,"id":"5f7c97a3_151adec5","line":1810,"range":{"start_line":1810,"start_character":8,"end_line":1810,"end_character":26},"updated":"2018-07-31 22:40:06.000000000","message":"YOU ARE HERE.\n\n- s/_smoke//\n- Nix the mock side effect and _run_update_available_resource. Instead...\n- Set up allocations on the compute node (preferably by creating \"real\" instances)\n- Run _update_provider_tree_multiple_providers\n- Validate the final allocations picture","commit_id":"2f5dfe8ab5b7df5e05fe5691a2399b9608dfbb2f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"033ea638d15865c67c5aa98c82b872375a76771b","unresolved":false,"context_lines":[{"line_number":1451,"context_line":""},{"line_number":1452,"context_line":""},{"line_number":1453,"context_line":"class ProviderTreeTests(integrated_helpers.ProviderUsageBaseTestCase):"},{"line_number":1454,"context_line":"    compute_driver \u003d \u0027fake.MediumFakeDriver\u0027"},{"line_number":1455,"context_line":""},{"line_number":1456,"context_line":"    def setUp(self):"},{"line_number":1457,"context_line":"        super(ProviderTreeTests, self).setUp()"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_1ffbc147","line":1454,"range":{"start_line":1454,"start_character":27,"end_line":1454,"end_character":33},"updated":"2018-08-01 21:48:50.000000000","message":"Note to reviewers: Needed something with space to create actual instances.","commit_id":"a46daed64b9ca92ea5755ce422e6063320b8c466"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"033ea638d15865c67c5aa98c82b872375a76771b","unresolved":false,"context_lines":[{"line_number":1616,"context_line":"                    \u0027MEMORY_MB\u0027: {"},{"line_number":1617,"context_line":"                         \u0027total\u0027: 1048576 * n,"},{"line_number":1618,"context_line":"                         \u0027reserved\u0027: 2048,"},{"line_number":1619,"context_line":"                         \u0027min_unit\u0027: 512,"},{"line_number":1620,"context_line":"                         \u0027max_unit\u0027: 131072,"},{"line_number":1621,"context_line":"                         \u0027step_size\u0027: 512,"},{"line_number":1622,"context_line":"                         \u0027allocation_ratio\u0027: 1.0,"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_1f80a19b","line":1619,"range":{"start_line":1619,"start_character":37,"end_line":1619,"end_character":40},"updated":"2018-08-01 21:48:50.000000000","message":"Note to reviewers: Had to bump this down for flavors[0] to work.","commit_id":"a46daed64b9ca92ea5755ce422e6063320b8c466"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1c7a599f5f43267e859e5a0f916a9d5eb6781426","unresolved":false,"context_lines":[{"line_number":1844,"context_line":"    def test_reshape(self):"},{"line_number":1845,"context_line":"        \"\"\"On startup, virt driver signals it needs to reshape, then does so."},{"line_number":1846,"context_line":""},{"line_number":1847,"context_line":"        Okay, not really on actual startup. That still happens via setUp. We"},{"line_number":1848,"context_line":"        simulate startup after the fact by passing startup\u003dTrue to"},{"line_number":1849,"context_line":"        update_available_resource."},{"line_number":1850,"context_line":""},{"line_number":1851,"context_line":"        This test creates a couple of instances so there are allocations to be"},{"line_number":1852,"context_line":"        moved by the reshape operation. Then we do the reshape and make sure"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_b742e991","line":1849,"range":{"start_line":1847,"start_character":8,"end_line":1849,"end_character":34},"updated":"2018-08-02 12:20:15.000000000","message":"Would it be hard to do this in a real startup? I see that to have something to move we need to compute to run first normally and report some PRs/inventories. Then we would need to stop the compute, mock the fake driver to ask for reshape, and start the compute again.","commit_id":"a46daed64b9ca92ea5755ce422e6063320b8c466"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a942bb78fdbe7895f8814d5f4776cdde715598a1","unresolved":false,"context_lines":[{"line_number":1844,"context_line":"    def test_reshape(self):"},{"line_number":1845,"context_line":"        \"\"\"On startup, virt driver signals it needs to reshape, then does so."},{"line_number":1846,"context_line":""},{"line_number":1847,"context_line":"        Okay, not really on actual startup. That still happens via setUp. We"},{"line_number":1848,"context_line":"        simulate startup after the fact by passing startup\u003dTrue to"},{"line_number":1849,"context_line":"        update_available_resource."},{"line_number":1850,"context_line":""},{"line_number":1851,"context_line":"        This test creates a couple of instances so there are allocations to be"},{"line_number":1852,"context_line":"        moved by the reshape operation. Then we do the reshape and make sure"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_b8bad5b7","line":1849,"range":{"start_line":1847,"start_character":8,"end_line":1849,"end_character":34},"in_reply_to":"5f7c97a3_0d1aaa69","updated":"2018-08-02 14:26:04.000000000","message":"Confirmed that self.restart_compute_service(self.compute) ought to work (_update gets called with startup\u003dTrue).\n\nGonna need to refactor _update_provider_tree_multiple_providers big time: extract the update_provider_tree method, as well as the verification, so they can be used from the existing test cases as well as this new one.","commit_id":"a46daed64b9ca92ea5755ce422e6063320b8c466"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"4ee8f90ac3bef7a33da4a88ef9065bc39ef407be","unresolved":false,"context_lines":[{"line_number":1844,"context_line":"    def test_reshape(self):"},{"line_number":1845,"context_line":"        \"\"\"On startup, virt driver signals it needs to reshape, then does so."},{"line_number":1846,"context_line":""},{"line_number":1847,"context_line":"        Okay, not really on actual startup. That still happens via setUp. We"},{"line_number":1848,"context_line":"        simulate startup after the fact by passing startup\u003dTrue to"},{"line_number":1849,"context_line":"        update_available_resource."},{"line_number":1850,"context_line":""},{"line_number":1851,"context_line":"        This test creates a couple of instances so there are allocations to be"},{"line_number":1852,"context_line":"        moved by the reshape operation. Then we do the reshape and make sure"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_0d1aaa69","line":1849,"range":{"start_line":1847,"start_character":8,"end_line":1849,"end_character":34},"in_reply_to":"5f7c97a3_b742e991","updated":"2018-08-02 13:06:10.000000000","message":"Why you gotta be reasonable?\n\nInvestigating...","commit_id":"a46daed64b9ca92ea5755ce422e6063320b8c466"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"567ad1b8bdef726016f7ea729d3f7d929fe46a28","unresolved":false,"context_lines":[{"line_number":1844,"context_line":"    def test_reshape(self):"},{"line_number":1845,"context_line":"        \"\"\"On startup, virt driver signals it needs to reshape, then does so."},{"line_number":1846,"context_line":""},{"line_number":1847,"context_line":"        Okay, not really on actual startup. That still happens via setUp. We"},{"line_number":1848,"context_line":"        simulate startup after the fact by passing startup\u003dTrue to"},{"line_number":1849,"context_line":"        update_available_resource."},{"line_number":1850,"context_line":""},{"line_number":1851,"context_line":"        This test creates a couple of instances so there are allocations to be"},{"line_number":1852,"context_line":"        moved by the reshape operation. Then we do the reshape and make sure"}],"source_content_type":"text/x-python","patch_set":21,"id":"3f79a3b5_be7dd6b3","line":1849,"range":{"start_line":1847,"start_character":8,"end_line":1849,"end_character":34},"in_reply_to":"5f7c97a3_b8bad5b7","updated":"2018-08-28 19:35:10.000000000","message":"https://review.openstack.org/597218\n\n \u003e Confirmed that self.restart_compute_service(self.compute) ought to\n \u003e work (_update gets called with startup\u003dTrue).\n\nIt did.\n\n \u003e Gonna need to refactor _update_provider_tree_multiple_providers big\n \u003e time: extract the update_provider_tree method, as well as the\n \u003e verification, so they can be used from the existing test cases as\n \u003e well as this new one.\n\nNope, turned out to be way easier than that.","commit_id":"a46daed64b9ca92ea5755ce422e6063320b8c466"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2316b5518cb1efac80b3d087c56f00abdb56f078","unresolved":false,"context_lines":[{"line_number":1576,"context_line":"        compute node\u0027s tree."},{"line_number":1577,"context_line":"        \"\"\""},{"line_number":1578,"context_line":"        def update_provider_tree(prov_tree, nodename, allocations\u003dNone):"},{"line_number":1579,"context_line":"            # This thing gets run multiple times (see"},{"line_number":1580,"context_line":"            # http://lists.openstack.org/pipermail/openstack-dev/2018-July/132531.html)  # noqa"},{"line_number":1581,"context_line":"            # so let\u0027s short out if we\u0027ve already been here."},{"line_number":1582,"context_line":"            if prov_tree.exists(uuids.ssp):"},{"line_number":1583,"context_line":"                return"},{"line_number":1584,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"3f79a3b5_fe7e71e2","line":1581,"range":{"start_line":1579,"start_character":12,"end_line":1581,"end_character":60},"updated":"2018-08-22 15:49:22.000000000","message":"Isn\u0027t this old now given https://review.openstack.org/#/c/520024/ is merged?","commit_id":"936f806900fea90d4c0cf4a787fb257b8221e538"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0ad7beac1ceafcfc2e1f90a97e61e4fb40cd39ef","unresolved":false,"context_lines":[{"line_number":1576,"context_line":"        compute node\u0027s tree."},{"line_number":1577,"context_line":"        \"\"\""},{"line_number":1578,"context_line":"        def update_provider_tree(prov_tree, nodename, allocations\u003dNone):"},{"line_number":1579,"context_line":"            # This thing gets run multiple times (see"},{"line_number":1580,"context_line":"            # http://lists.openstack.org/pipermail/openstack-dev/2018-July/132531.html)  # noqa"},{"line_number":1581,"context_line":"            # so let\u0027s short out if we\u0027ve already been here."},{"line_number":1582,"context_line":"            if prov_tree.exists(uuids.ssp):"},{"line_number":1583,"context_line":"                return"},{"line_number":1584,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"3f79a3b5_9e9bbd15","line":1581,"range":{"start_line":1579,"start_character":12,"end_line":1581,"end_character":60},"in_reply_to":"3f79a3b5_fe7e71e2","updated":"2018-08-22 16:03:01.000000000","message":"Oh, yeah, probably. I fixed other stuff up for that, but missed this one. Let me try it out and see if it can go away...\n\n[Later] Yup. Removed.","commit_id":"936f806900fea90d4c0cf4a787fb257b8221e538"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2316b5518cb1efac80b3d087c56f00abdb56f078","unresolved":false,"context_lines":[{"line_number":1853,"context_line":"        inst1 \u003d self._create_instance(flavors[0])"},{"line_number":1854,"context_line":"        inst2 \u003d self._create_instance(flavors[1])"},{"line_number":1855,"context_line":""},{"line_number":1856,"context_line":"        # Instance create calls _update, which calls update_provider_tree,"},{"line_number":1857,"context_line":"        # which is currently mocked to a no-op."},{"line_number":1858,"context_line":"        self.assertEqual(2, self.mock_upt.call_count)"},{"line_number":1859,"context_line":"        self.mock_upt.reset_mock()"},{"line_number":1860,"context_line":""},{"line_number":1861,"context_line":"        # Hit the reshape."},{"line_number":1862,"context_line":"        self._update_provider_tree_multiple_providers(startup\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":30,"id":"3f79a3b5_3e706908","line":1859,"range":{"start_line":1856,"start_character":8,"end_line":1859,"end_character":34},"updated":"2018-08-22 15:49:22.000000000","message":"Could we have created a fake virt driver that does the reshape stuff so we wouldn\u0027t have to mock this out?","commit_id":"936f806900fea90d4c0cf4a787fb257b8221e538"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0ad7beac1ceafcfc2e1f90a97e61e4fb40cd39ef","unresolved":false,"context_lines":[{"line_number":1853,"context_line":"        inst1 \u003d self._create_instance(flavors[0])"},{"line_number":1854,"context_line":"        inst2 \u003d self._create_instance(flavors[1])"},{"line_number":1855,"context_line":""},{"line_number":1856,"context_line":"        # Instance create calls _update, which calls update_provider_tree,"},{"line_number":1857,"context_line":"        # which is currently mocked to a no-op."},{"line_number":1858,"context_line":"        self.assertEqual(2, self.mock_upt.call_count)"},{"line_number":1859,"context_line":"        self.mock_upt.reset_mock()"},{"line_number":1860,"context_line":""},{"line_number":1861,"context_line":"        # Hit the reshape."},{"line_number":1862,"context_line":"        self._update_provider_tree_multiple_providers(startup\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":30,"id":"3f79a3b5_f92d2b95","line":1859,"range":{"start_line":1856,"start_character":8,"end_line":1859,"end_character":34},"in_reply_to":"3f79a3b5_1efa4d42","updated":"2018-08-22 16:03:01.000000000","message":"Okay, but update_provider_tree will get run in basically every functional test. If we put this implementation in the fake driver itself, the provider tree would need to settle into a realistic shape that all those other tests expect. Which (even when the reshape section isn\u0027t run) this isn\u0027t necessarily doing. And if it did, it wouldn\u0027t give us the test coverage we want out of it.\n\nAt most, I could buy refactoring this into a subclass of the fake virt driver; though I\u0027m not sure I see the point since it would only get used here.","commit_id":"936f806900fea90d4c0cf4a787fb257b8221e538"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a87a2e1c36ee59559ea7ca8307b8b3eff3cc4a46","unresolved":false,"context_lines":[{"line_number":1853,"context_line":"        inst1 \u003d self._create_instance(flavors[0])"},{"line_number":1854,"context_line":"        inst2 \u003d self._create_instance(flavors[1])"},{"line_number":1855,"context_line":""},{"line_number":1856,"context_line":"        # Instance create calls _update, which calls update_provider_tree,"},{"line_number":1857,"context_line":"        # which is currently mocked to a no-op."},{"line_number":1858,"context_line":"        self.assertEqual(2, self.mock_upt.call_count)"},{"line_number":1859,"context_line":"        self.mock_upt.reset_mock()"},{"line_number":1860,"context_line":""},{"line_number":1861,"context_line":"        # Hit the reshape."},{"line_number":1862,"context_line":"        self._update_provider_tree_multiple_providers(startup\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":30,"id":"3f79a3b5_1efa4d42","line":1859,"range":{"start_line":1856,"start_character":8,"end_line":1859,"end_character":34},"in_reply_to":"3f79a3b5_3e706908","updated":"2018-08-22 15:52:08.000000000","message":"I see now that we mock update_provider_tree (mock_upt) on the fake virt driver that this test is using. So we could break this out into a separate fake virt driver, but that could also just be done in a refactor followup if we care. Generally I like to keep the fake virt driver stuff as much in the fake virt drivers as possible rather than do mocks within functional tests.","commit_id":"936f806900fea90d4c0cf4a787fb257b8221e538"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a74c6f90a345866102bdb35d0f16bf2e0c7fb294","unresolved":false,"context_lines":[{"line_number":1447,"context_line":""},{"line_number":1448,"context_line":""},{"line_number":1449,"context_line":"class ProviderTreeTests(integrated_helpers.ProviderUsageBaseTestCase):"},{"line_number":1450,"context_line":"    compute_driver \u003d \u0027fake.MediumFakeDriver\u0027"},{"line_number":1451,"context_line":""},{"line_number":1452,"context_line":"    def setUp(self):"},{"line_number":1453,"context_line":"        super(ProviderTreeTests, self).setUp()"}],"source_content_type":"text/x-python","patch_set":33,"id":"3f79a3b5_6c7ef8af","line":1450,"updated":"2018-08-24 20:53:07.000000000","message":"I love me some medium fake.","commit_id":"c932ef657b6a906a2c7cb700a717c7c8d1706a93"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a74c6f90a345866102bdb35d0f16bf2e0c7fb294","unresolved":false,"context_lines":[{"line_number":1670,"context_line":""},{"line_number":1671,"context_line":"        self.mock_upt.side_effect \u003d update_provider_tree"},{"line_number":1672,"context_line":""},{"line_number":1673,"context_line":"        self._run_update_available_resource(startup)"},{"line_number":1674,"context_line":""},{"line_number":1675,"context_line":"        # Create a dict, keyed by provider UUID, of all the providers"},{"line_number":1676,"context_line":"        rps_by_uuid \u003d {}"}],"source_content_type":"text/x-python","patch_set":33,"id":"3f79a3b5_47d43548","line":1673,"updated":"2018-08-24 20:53:07.000000000","message":"to say these tests are confusing is an understatement...\n\nthis is more unit-testy than functional test-y in my opinion, with all the mocking and passing of exception and assertion expectation for the mocks.\n\nthat said, I realize that this patch actually doesn\u0027t implement the changes to update_provider_tree() in any of the virt drivers (just the signature is changed with a default kwarg for allocations). so, this is really just testing the integration points between the manager, resource tracker and virt driver\u0027s update_provider_tree() call. so, I suppose I\u0027m OK with it. I certainly wouldn\u0027t call it a functional test though, and I\u0027m not sure why it\u0027s in ProviderTreeTests since none of these test changes really have anything to do with provider trees :)","commit_id":"c932ef657b6a906a2c7cb700a717c7c8d1706a93"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9ed488c905beabaccc98a2d464703a5040c4cbae","unresolved":false,"context_lines":[{"line_number":1670,"context_line":""},{"line_number":1671,"context_line":"        self.mock_upt.side_effect \u003d update_provider_tree"},{"line_number":1672,"context_line":""},{"line_number":1673,"context_line":"        self._run_update_available_resource(startup)"},{"line_number":1674,"context_line":""},{"line_number":1675,"context_line":"        # Create a dict, keyed by provider UUID, of all the providers"},{"line_number":1676,"context_line":"        rps_by_uuid \u003d {}"}],"source_content_type":"text/x-python","patch_set":33,"id":"3f79a3b5_c7612507","line":1673,"in_reply_to":"3f79a3b5_47d43548","updated":"2018-08-24 21:13:30.000000000","message":"\u003e to say these tests are confusing is an understatement...\n\nDude, no argument here. If you can think of a way to make them less confusing, I\u0027m all ears.\n\n \u003e this is more unit-testy than functional test-y in my opinion, with\n \u003e all the mocking and passing of exception and assertion expectation\n \u003e for the mocks.\n\nThe mocking is just of the driver\u0027s update_provider_tree method, right? Matt had a similar/related comment earlier [1], suggesting that we put the actual upt impl into the FakeDriver instead. I could do that, but it would be a refactor without a real difference; it would put a big gap between different crucial pieces of the test code (making it even *tougher* to follow); and it would only be usable for this one test case (since all other normal/sane functional tests through the fake driver wouldn\u0027t want to do this craziness).\n\n \u003e so,\n \u003e this is really just testing the integration points between the\n \u003e manager, resource tracker and virt driver\u0027s update_provider_tree()\n \u003e call.\n\nWell, no, it\u0027s testing an end-to-end flow with a \"real\" reshaping update_provider_tree method in it.\n\n \u003e I\u0027m not sure why it\u0027s in\n \u003e ProviderTreeTests since none of these test changes really have\n \u003e anything to do with provider trees :)\n\nEh? It\u0027s testing the things that update_provider_tree might do, which is to... update provider trees in various ways.\n\n[1] https://review.openstack.org/#/c/576236/30/nova/tests/functional/test_servers.py@1859","commit_id":"c932ef657b6a906a2c7cb700a717c7c8d1706a93"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"874e2e7cc8d126c078f98945b1f9e004c0e5ae71","unresolved":false,"context_lines":[{"line_number":1963,"context_line":"            mock.call(mock.ANY, \u0027host1\u0027),"},{"line_number":1964,"context_line":"            mock.call(mock.ANY, \u0027host1\u0027, allocations\u003dexp_allocs),"},{"line_number":1965,"context_line":"        ])"},{"line_number":1966,"context_line":""},{"line_number":1967,"context_line":""},{"line_number":1968,"context_line":"class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase):"},{"line_number":1969,"context_line":"    \"\"\"Tests moving servers while checking the resource allocations and usages"}],"source_content_type":"text/x-python","patch_set":33,"id":"3f79a3b5_0b417839","line":1966,"updated":"2018-08-24 12:37:02.000000000","message":"These tests doesn\u0027t really feel functional due to the need of so much added code to the fake virt driver to make things triggered. I hope that as soon as libvirt driver implements the first reshape we can replace this with a test where the real libvirt driver code will do the reshape.","commit_id":"c932ef657b6a906a2c7cb700a717c7c8d1706a93"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9ed488c905beabaccc98a2d464703a5040c4cbae","unresolved":false,"context_lines":[{"line_number":1963,"context_line":"            mock.call(mock.ANY, \u0027host1\u0027),"},{"line_number":1964,"context_line":"            mock.call(mock.ANY, \u0027host1\u0027, allocations\u003dexp_allocs),"},{"line_number":1965,"context_line":"        ])"},{"line_number":1966,"context_line":""},{"line_number":1967,"context_line":""},{"line_number":1968,"context_line":"class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase):"},{"line_number":1969,"context_line":"    \"\"\"Tests moving servers while checking the resource allocations and usages"}],"source_content_type":"text/x-python","patch_set":33,"id":"3f79a3b5_e70a2124","line":1966,"in_reply_to":"3f79a3b5_0b417839","updated":"2018-08-24 21:13:30.000000000","message":"So I disagree with both of you about this not being a functional test. Unless we\u0027re just arguing about the definition of \"functional\", which I see as meaning \"end to end with minimal mocking\". In this case, yes, I\u0027m using a mock rather than a subclass of FakeVirtDriver to compose a virt driver that \"implements\" a reshaping update_provider_tree; but the effect is identical. Other than that, this is still a deep-stack test of a major chunk of the compute manager code, in line with the other (functional) tests in this module.","commit_id":"c932ef657b6a906a2c7cb700a717c7c8d1706a93"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5f49d8fe22ba26646fd2795503612397938d6e9d","unresolved":false,"context_lines":[{"line_number":1645,"context_line":"                    # All allocations should belong to the compute node."},{"line_number":1646,"context_line":"                    self.assertEqual([self.host_uuid], list(allocs))"},{"line_number":1647,"context_line":"                    new_allocs \u003d {}"},{"line_number":1648,"context_line":"                    for rc, amt in allocs[self.host_uuid][\u0027resources\u0027].items():"},{"line_number":1649,"context_line":"                        # Move VCPU to NUMA1 and MEMORY_MB to NUMA2. Bogus, but"},{"line_number":1650,"context_line":"                        # lets us prove stuff ends up where we tell it to go."},{"line_number":1651,"context_line":"                        if rc \u003d\u003d \u0027VCPU\u0027:"},{"line_number":1652,"context_line":"                            rp_uuid \u003d uuids.numa1"},{"line_number":1653,"context_line":"                        elif rc \u003d\u003d \u0027MEMORY_MB\u0027:"},{"line_number":1654,"context_line":"                            rp_uuid \u003d uuids.numa2"},{"line_number":1655,"context_line":"                        elif rc \u003d\u003d \u0027DISK_GB\u0027:"},{"line_number":1656,"context_line":"                            rp_uuid \u003d uuids.ssp"},{"line_number":1657,"context_line":"                        else:"},{"line_number":1658,"context_line":"                            self.fail(\"Unexpected resource on compute node: \""},{"line_number":1659,"context_line":"                                      \"%s\u003d%d\" % (rc, amt))"},{"line_number":1660,"context_line":"                        new_allocs[rp_uuid] \u003d {"},{"line_number":1661,"context_line":"                            \u0027resources\u0027: {rc: amt},"},{"line_number":1662,"context_line":"                        }"}],"source_content_type":"text/x-python","patch_set":35,"id":"3f79a3b5_ae47f2a2","line":1659,"range":{"start_line":1648,"start_character":0,"end_line":1659,"end_character":58},"updated":"2018-08-28 12:47:45.000000000","message":"this kind of code is going to end up in the virt drivers... I pity the reviewer that is going to need to review that mess. Oh wait... *I\u0027m* one of those reviewers. Crap.\n\nMakes me think we made a mistake by not having this kind of code quarantined to a nova-manage command or external script that can be thrown away after use...","commit_id":"bc03dc7237cb666362f7c54c8cd27b4ab999beaa"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a68d988967c3da2601b86ab2caf48eb789008457","unresolved":false,"context_lines":[{"line_number":1645,"context_line":"                    # All allocations should belong to the compute node."},{"line_number":1646,"context_line":"                    self.assertEqual([self.host_uuid], list(allocs))"},{"line_number":1647,"context_line":"                    new_allocs \u003d {}"},{"line_number":1648,"context_line":"                    for rc, amt in allocs[self.host_uuid][\u0027resources\u0027].items():"},{"line_number":1649,"context_line":"                        # Move VCPU to NUMA1 and MEMORY_MB to NUMA2. Bogus, but"},{"line_number":1650,"context_line":"                        # lets us prove stuff ends up where we tell it to go."},{"line_number":1651,"context_line":"                        if rc \u003d\u003d \u0027VCPU\u0027:"},{"line_number":1652,"context_line":"                            rp_uuid \u003d uuids.numa1"},{"line_number":1653,"context_line":"                        elif rc \u003d\u003d \u0027MEMORY_MB\u0027:"},{"line_number":1654,"context_line":"                            rp_uuid \u003d uuids.numa2"},{"line_number":1655,"context_line":"                        elif rc \u003d\u003d \u0027DISK_GB\u0027:"},{"line_number":1656,"context_line":"                            rp_uuid \u003d uuids.ssp"},{"line_number":1657,"context_line":"                        else:"},{"line_number":1658,"context_line":"                            self.fail(\"Unexpected resource on compute node: \""},{"line_number":1659,"context_line":"                                      \"%s\u003d%d\" % (rc, amt))"},{"line_number":1660,"context_line":"                        new_allocs[rp_uuid] \u003d {"},{"line_number":1661,"context_line":"                            \u0027resources\u0027: {rc: amt},"},{"line_number":1662,"context_line":"                        }"}],"source_content_type":"text/x-python","patch_set":35,"id":"3f79a3b5_b4166049","line":1659,"range":{"start_line":1648,"start_character":0,"end_line":1659,"end_character":58},"in_reply_to":"3f79a3b5_ae47f2a2","updated":"2018-08-28 12:58:09.000000000","message":"Hm, it\u0027s not out of the question for the virt driver impl to do it with some kind of dynamic import...","commit_id":"bc03dc7237cb666362f7c54c8cd27b4ab999beaa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":1454,"context_line":"        # Before starting compute, placement has no providers registered"},{"line_number":1455,"context_line":"        self.assertEqual([], self._get_all_providers())"},{"line_number":1456,"context_line":""},{"line_number":1457,"context_line":"        # Start compute without mocking update_provider_tree. The fake driver"},{"line_number":1458,"context_line":"        # doesn\u0027t implement the method, so this will cause us to start with the"},{"line_number":1459,"context_line":"        # legacy get_available_resource()-based inventory discovery and"},{"line_number":1460,"context_line":"        # boostrapping of placement data."},{"line_number":1461,"context_line":"        self.compute \u003d self._start_compute(host\u003d\u0027host1\u0027)"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_317b377c","line":1458,"range":{"start_line":1457,"start_character":62,"end_line":1458,"end_character":38},"updated":"2018-08-28 20:05:59.000000000","message":"As noted before, I think long-term we should have a ReshaperFakeDriver that needs a reshape and does it successfully, and likely a ReshapeFailsFakeDriver that needs a reshape and fails to do it, then we don\u0027t need the mocks in the functional test.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e85dc8dbdcf46c7a246089a50a623a6a554dfe67","unresolved":false,"context_lines":[{"line_number":1454,"context_line":"        # Before starting compute, placement has no providers registered"},{"line_number":1455,"context_line":"        self.assertEqual([], self._get_all_providers())"},{"line_number":1456,"context_line":""},{"line_number":1457,"context_line":"        # Start compute without mocking update_provider_tree. The fake driver"},{"line_number":1458,"context_line":"        # doesn\u0027t implement the method, so this will cause us to start with the"},{"line_number":1459,"context_line":"        # legacy get_available_resource()-based inventory discovery and"},{"line_number":1460,"context_line":"        # boostrapping of placement data."},{"line_number":1461,"context_line":"        self.compute \u003d self._start_compute(host\u003d\u0027host1\u0027)"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_6f59ca33","line":1458,"range":{"start_line":1457,"start_character":62,"end_line":1458,"end_character":38},"in_reply_to":"3f79a3b5_317b377c","updated":"2018-08-28 21:28:42.000000000","message":"I actually looked pretty hard at doing that this afternoon, because making you happy is what I live for. I decided (again) to punt on it because a) it would actually entail more than one fake virt driver implementation, since the tests want to do slightly different things in certain cases (I could signal those conditions some other how, but I thought that was even more of a hack); and b) that would *still* entail putting the fake upt implementation(s) even further away from the code that uses them, which also ties into c) this test class is the only place it makes even a little bit of sense to run this crazy reshape, specifically because it\u0027s doing crazy things to excercise edge cases.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":1571,"context_line":""},{"line_number":1572,"context_line":"    def _update_provider_tree_multiple_providers(self, startup\u003dFalse,"},{"line_number":1573,"context_line":"                                                 do_reshape\u003dFalse):"},{"line_number":1574,"context_line":"        \"\"\"Make update_provider_tree create multiple providers, including an"},{"line_number":1575,"context_line":"        additional root as a sharing provider; and some descendants in the"},{"line_number":1576,"context_line":"        compute node\u0027s tree."},{"line_number":1577,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_f1d93f84","line":1574,"updated":"2018-08-28 20:05:59.000000000","message":"nit: same request for ascii diagrams of the tree created here would be nice.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0911e8e64ee4aaf649021252d460d98b19a9a335","unresolved":false,"context_lines":[{"line_number":1571,"context_line":""},{"line_number":1572,"context_line":"    def _update_provider_tree_multiple_providers(self, startup\u003dFalse,"},{"line_number":1573,"context_line":"                                                 do_reshape\u003dFalse):"},{"line_number":1574,"context_line":"        \"\"\"Make update_provider_tree create multiple providers, including an"},{"line_number":1575,"context_line":"        additional root as a sharing provider; and some descendants in the"},{"line_number":1576,"context_line":"        compute node\u0027s tree."},{"line_number":1577,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_b0ee2889","line":1574,"in_reply_to":"3f79a3b5_2f3bd2c8","updated":"2018-08-30 22:38:03.000000000","message":"Done","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e85dc8dbdcf46c7a246089a50a623a6a554dfe67","unresolved":false,"context_lines":[{"line_number":1571,"context_line":""},{"line_number":1572,"context_line":"    def _update_provider_tree_multiple_providers(self, startup\u003dFalse,"},{"line_number":1573,"context_line":"                                                 do_reshape\u003dFalse):"},{"line_number":1574,"context_line":"        \"\"\"Make update_provider_tree create multiple providers, including an"},{"line_number":1575,"context_line":"        additional root as a sharing provider; and some descendants in the"},{"line_number":1576,"context_line":"        compute node\u0027s tree."},{"line_number":1577,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_2f3bd2c8","line":1574,"in_reply_to":"3f79a3b5_f1d93f84","updated":"2018-08-28 21:28:42.000000000","message":"ack","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":1584,"context_line":"            prov_tree.update_traits("},{"line_number":1585,"context_line":"                \u0027ssp\u0027, [\u0027MISC_SHARES_VIA_AGGREGATE\u0027, \u0027STORAGE_DISK_SSD\u0027])"},{"line_number":1586,"context_line":"            prov_tree.update_aggregates(\u0027ssp\u0027, [uuids.agg])"},{"line_number":1587,"context_line":"            prov_tree.update_inventory(\u0027ssp\u0027, {\u0027DISK_GB\u0027: {\u0027total\u0027: 500}})"},{"line_number":1588,"context_line":"            # Compute node is in the same aggregate"},{"line_number":1589,"context_line":"            prov_tree.update_aggregates(self.compute.host, [uuids.agg])"},{"line_number":1590,"context_line":"            # Create two NUMA nodes as children"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_91eb2b11","line":1587,"range":{"start_line":1587,"start_character":12,"end_line":1587,"end_character":74},"updated":"2018-08-28 20:05:59.000000000","message":"I guess this was just missing before or not exercised in existing tests?","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e85dc8dbdcf46c7a246089a50a623a6a554dfe67","unresolved":false,"context_lines":[{"line_number":1584,"context_line":"            prov_tree.update_traits("},{"line_number":1585,"context_line":"                \u0027ssp\u0027, [\u0027MISC_SHARES_VIA_AGGREGATE\u0027, \u0027STORAGE_DISK_SSD\u0027])"},{"line_number":1586,"context_line":"            prov_tree.update_aggregates(\u0027ssp\u0027, [uuids.agg])"},{"line_number":1587,"context_line":"            prov_tree.update_inventory(\u0027ssp\u0027, {\u0027DISK_GB\u0027: {\u0027total\u0027: 500}})"},{"line_number":1588,"context_line":"            # Compute node is in the same aggregate"},{"line_number":1589,"context_line":"            prov_tree.update_aggregates(self.compute.host, [uuids.agg])"},{"line_number":1590,"context_line":"            # Create two NUMA nodes as children"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_af9da2bc","line":1587,"range":{"start_line":1587,"start_character":12,"end_line":1587,"end_character":74},"in_reply_to":"3f79a3b5_91eb2b11","updated":"2018-08-28 21:28:42.000000000","message":"Yeah, before this test code was just concerned with moving around providers, aggregates, and traits. We didn\u0027t care as much with inventories. Now we do - specifically here wanting to make sure we can reshape inventories and allocations (esp. that we *get* the allocations) when they\u0027re on sharing providers.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":1657,"context_line":"                        else:"},{"line_number":1658,"context_line":"                            self.fail(\"Unexpected resource on compute node: \""},{"line_number":1659,"context_line":"                                      \"%s\u003d%d\" % (rc, amt))"},{"line_number":1660,"context_line":"                        new_allocs[rp_uuid] \u003d {"},{"line_number":1661,"context_line":"                            \u0027resources\u0027: {rc: amt},"},{"line_number":1662,"context_line":"                        }"},{"line_number":1663,"context_line":"                    # Add a VF for the heck of it. Again bogus, but see above."}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_d16763b7","line":1660,"range":{"start_line":1660,"start_character":24,"end_line":1660,"end_character":34},"updated":"2018-08-28 20:05:59.000000000","message":"Where are the allocations asserted in the test?\n\n(later)\n\nOK I guess in test_reshape.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":1793,"context_line":"        self.assertIn(\u0027HW_CPU_X86_AVX\u0027, traits)"},{"line_number":1794,"context_line":"        self.assertNotIn(\u0027FOO\u0027, traits)"},{"line_number":1795,"context_line":""},{"line_number":1796,"context_line":"    def test_reshape_needed_non_startup(self):"},{"line_number":1797,"context_line":"        \"\"\"ReshapeNeeded is reraised when not starting up.\"\"\""},{"line_number":1798,"context_line":"        self.mock_upt.side_effect \u003d exception.ReshapeNeeded()"},{"line_number":1799,"context_line":""}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_b1ff47aa","line":1796,"updated":"2018-08-28 20:05:59.000000000","message":"I think Jay already mentioned this before, but these are really unit tests and should live elsewhere in the nova/tests/unit/ tree.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0911e8e64ee4aaf649021252d460d98b19a9a335","unresolved":false,"context_lines":[{"line_number":1793,"context_line":"        self.assertIn(\u0027HW_CPU_X86_AVX\u0027, traits)"},{"line_number":1794,"context_line":"        self.assertNotIn(\u0027FOO\u0027, traits)"},{"line_number":1795,"context_line":""},{"line_number":1796,"context_line":"    def test_reshape_needed_non_startup(self):"},{"line_number":1797,"context_line":"        \"\"\"ReshapeNeeded is reraised when not starting up.\"\"\""},{"line_number":1798,"context_line":"        self.mock_upt.side_effect \u003d exception.ReshapeNeeded()"},{"line_number":1799,"context_line":""}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_cb29b989","line":1796,"in_reply_to":"3f79a3b5_4fb3ae0b","updated":"2018-08-30 22:38:03.000000000","message":"Done","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"98f3905af65ab8c15fcd7f3dc9db9221fb055bf5","unresolved":false,"context_lines":[{"line_number":1793,"context_line":"        self.assertIn(\u0027HW_CPU_X86_AVX\u0027, traits)"},{"line_number":1794,"context_line":"        self.assertNotIn(\u0027FOO\u0027, traits)"},{"line_number":1795,"context_line":""},{"line_number":1796,"context_line":"    def test_reshape_needed_non_startup(self):"},{"line_number":1797,"context_line":"        \"\"\"ReshapeNeeded is reraised when not starting up.\"\"\""},{"line_number":1798,"context_line":"        self.mock_upt.side_effect \u003d exception.ReshapeNeeded()"},{"line_number":1799,"context_line":""}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_4fb3ae0b","line":1796,"in_reply_to":"3f79a3b5_af860265","updated":"2018-08-28 21:40:22.000000000","message":"\u003e If you agree, and these bother you, I can just delete them in a fup.\n\nSounds good. We don\u0027t need the redundant coverage for this.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e85dc8dbdcf46c7a246089a50a623a6a554dfe67","unresolved":false,"context_lines":[{"line_number":1793,"context_line":"        self.assertIn(\u0027HW_CPU_X86_AVX\u0027, traits)"},{"line_number":1794,"context_line":"        self.assertNotIn(\u0027FOO\u0027, traits)"},{"line_number":1795,"context_line":""},{"line_number":1796,"context_line":"    def test_reshape_needed_non_startup(self):"},{"line_number":1797,"context_line":"        \"\"\"ReshapeNeeded is reraised when not starting up.\"\"\""},{"line_number":1798,"context_line":"        self.mock_upt.side_effect \u003d exception.ReshapeNeeded()"},{"line_number":1799,"context_line":""}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_af860265","line":1796,"in_reply_to":"3f79a3b5_b1ff47aa","updated":"2018-08-28 21:28:42.000000000","message":"Okay. I was just adding these to get more full-stack coverage; the units in question are already covered in the unit tests. If you agree, and these bother you, I can just delete them in a fup.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":1831,"context_line":"        inst \u003d self.api.post_server({\u0027server\u0027: server_req})"},{"line_number":1832,"context_line":"        return self._wait_for_state_change(self.admin_api, inst, \u0027ACTIVE\u0027)"},{"line_number":1833,"context_line":""},{"line_number":1834,"context_line":"    def test_reshape(self):"},{"line_number":1835,"context_line":"        \"\"\"On startup, virt driver signals it needs to reshape, then does so."},{"line_number":1836,"context_line":""},{"line_number":1837,"context_line":"        Okay, not really on actual startup. That still happens via setUp. We"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_b1c66768","line":1834,"updated":"2018-08-28 20:05:59.000000000","message":"Per the discussion here:\n\nhttps://review.openstack.org/#/c/584648/20/nova/scheduler/client/report.py@2056\n\nIt would be cool to have a wrinkle in this test (or a new test) where we:\n\n1. create an instance on host1\n2. stop nova-compute on host1 and force it down\n3. evacuate the instance to host2\n4. restart host2 and trigger a reshape\n5. assert the instance allocations for host2 are reshaped properly but any allocations against host1 are left untouched.\n\nHaving said that, I\u0027m not entirely sure how valuable it is given it\u0027s completely dependent on the virt driver implementation of update_provider_tree right?","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"98f3905af65ab8c15fcd7f3dc9db9221fb055bf5","unresolved":false,"context_lines":[{"line_number":1831,"context_line":"        inst \u003d self.api.post_server({\u0027server\u0027: server_req})"},{"line_number":1832,"context_line":"        return self._wait_for_state_change(self.admin_api, inst, \u0027ACTIVE\u0027)"},{"line_number":1833,"context_line":""},{"line_number":1834,"context_line":"    def test_reshape(self):"},{"line_number":1835,"context_line":"        \"\"\"On startup, virt driver signals it needs to reshape, then does so."},{"line_number":1836,"context_line":""},{"line_number":1837,"context_line":"        Okay, not really on actual startup. That still happens via setUp. We"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_afa362b1","line":1834,"in_reply_to":"3f79a3b5_8fe6a64a","updated":"2018-08-28 21:40:22.000000000","message":"\u003e I suppose we could add a unit test to verify that get_allocations_for_provider_tree will return \"the other host\u0027s\" allocations when they exist, but that would be pretty contrived...\n\nI think gibi already asked that the docstring on that method mention the scenario we know about so at least it\u0027s a thing users of that interface have to be conscious of - which is probably good enough for a start.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e85dc8dbdcf46c7a246089a50a623a6a554dfe67","unresolved":false,"context_lines":[{"line_number":1831,"context_line":"        inst \u003d self.api.post_server({\u0027server\u0027: server_req})"},{"line_number":1832,"context_line":"        return self._wait_for_state_change(self.admin_api, inst, \u0027ACTIVE\u0027)"},{"line_number":1833,"context_line":""},{"line_number":1834,"context_line":"    def test_reshape(self):"},{"line_number":1835,"context_line":"        \"\"\"On startup, virt driver signals it needs to reshape, then does so."},{"line_number":1836,"context_line":""},{"line_number":1837,"context_line":"        Okay, not really on actual startup. That still happens via setUp. We"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_8fe6a64a","line":1834,"in_reply_to":"3f79a3b5_b1c66768","updated":"2018-08-28 21:28:42.000000000","message":"Yes, completely, since that impl would need to do the right thing (whatever that is) with the other host\u0027s allocations. I suppose we could add a unit test to verify that get_allocations_for_provider_tree will return \"the other host\u0027s\" allocations when they exist, but that would be pretty contrived...","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":1847,"context_line":"        inst1 \u003d self._create_instance(flavors[0])"},{"line_number":1848,"context_line":"        inst2 \u003d self._create_instance(flavors[1])"},{"line_number":1849,"context_line":""},{"line_number":1850,"context_line":"        # Instance create calls _update, which calls update_provider_tree,"},{"line_number":1851,"context_line":"        # which is currently mocked to a no-op."},{"line_number":1852,"context_line":"        self.assertEqual(2, self.mock_upt.call_count)"},{"line_number":1853,"context_line":"        self.mock_upt.reset_mock()"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_111cfb11","line":1850,"range":{"start_line":1850,"start_character":32,"end_line":1850,"end_character":39},"updated":"2018-08-28 20:05:59.000000000","message":"nit: RT._update","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":1847,"context_line":"        inst1 \u003d self._create_instance(flavors[0])"},{"line_number":1848,"context_line":"        inst2 \u003d self._create_instance(flavors[1])"},{"line_number":1849,"context_line":""},{"line_number":1850,"context_line":"        # Instance create calls _update, which calls update_provider_tree,"},{"line_number":1851,"context_line":"        # which is currently mocked to a no-op."},{"line_number":1852,"context_line":"        self.assertEqual(2, self.mock_upt.call_count)"},{"line_number":1853,"context_line":"        self.mock_upt.reset_mock()"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_b118a71e","line":1850,"range":{"start_line":1850,"start_character":53,"end_line":1850,"end_character":73},"updated":"2018-08-28 20:05:59.000000000","message":"nit: driver.update_provider_tree","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0911e8e64ee4aaf649021252d460d98b19a9a335","unresolved":false,"context_lines":[{"line_number":1847,"context_line":"        inst1 \u003d self._create_instance(flavors[0])"},{"line_number":1848,"context_line":"        inst2 \u003d self._create_instance(flavors[1])"},{"line_number":1849,"context_line":""},{"line_number":1850,"context_line":"        # Instance create calls _update, which calls update_provider_tree,"},{"line_number":1851,"context_line":"        # which is currently mocked to a no-op."},{"line_number":1852,"context_line":"        self.assertEqual(2, self.mock_upt.call_count)"},{"line_number":1853,"context_line":"        self.mock_upt.reset_mock()"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_cb4e996d","line":1850,"range":{"start_line":1850,"start_character":53,"end_line":1850,"end_character":73},"in_reply_to":"3f79a3b5_4ff08efe","updated":"2018-08-30 22:38:03.000000000","message":"Done","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e85dc8dbdcf46c7a246089a50a623a6a554dfe67","unresolved":false,"context_lines":[{"line_number":1847,"context_line":"        inst1 \u003d self._create_instance(flavors[0])"},{"line_number":1848,"context_line":"        inst2 \u003d self._create_instance(flavors[1])"},{"line_number":1849,"context_line":""},{"line_number":1850,"context_line":"        # Instance create calls _update, which calls update_provider_tree,"},{"line_number":1851,"context_line":"        # which is currently mocked to a no-op."},{"line_number":1852,"context_line":"        self.assertEqual(2, self.mock_upt.call_count)"},{"line_number":1853,"context_line":"        self.mock_upt.reset_mock()"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_4ff08efe","line":1850,"range":{"start_line":1850,"start_character":53,"end_line":1850,"end_character":73},"in_reply_to":"3f79a3b5_b118a71e","updated":"2018-08-28 21:28:42.000000000","message":"ight","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":310,"context_line":"    @mock.patch.object(manager.ComputeManager, \u0027_get_resource_tracker\u0027)"},{"line_number":311,"context_line":"    def test_update_available_resource_for_node_reshape_needed(self, get_rt,"},{"line_number":312,"context_line":"                                                               log_mock):"},{"line_number":313,"context_line":"        \"\"\"ReshapeFailed logs and reraises.\"\"\""},{"line_number":314,"context_line":"        rt \u003d mock.Mock(spec_set\u003d[\u0027update_available_resource\u0027])"},{"line_number":315,"context_line":"        get_rt.return_value \u003d rt"},{"line_number":316,"context_line":"        rt.update_available_resource.side_effect \u003d exception.ReshapeNeeded()"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_d171a346","line":313,"range":{"start_line":313,"start_character":11,"end_line":313,"end_character":24},"updated":"2018-08-28 20:05:59.000000000","message":"ReshapeNeeded","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0911e8e64ee4aaf649021252d460d98b19a9a335","unresolved":false,"context_lines":[{"line_number":310,"context_line":"    @mock.patch.object(manager.ComputeManager, \u0027_get_resource_tracker\u0027)"},{"line_number":311,"context_line":"    def test_update_available_resource_for_node_reshape_needed(self, get_rt,"},{"line_number":312,"context_line":"                                                               log_mock):"},{"line_number":313,"context_line":"        \"\"\"ReshapeFailed logs and reraises.\"\"\""},{"line_number":314,"context_line":"        rt \u003d mock.Mock(spec_set\u003d[\u0027update_available_resource\u0027])"},{"line_number":315,"context_line":"        get_rt.return_value \u003d rt"},{"line_number":316,"context_line":"        rt.update_available_resource.side_effect \u003d exception.ReshapeNeeded()"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_8b7181ab","line":313,"range":{"start_line":313,"start_character":11,"end_line":313,"end_character":24},"in_reply_to":"3f79a3b5_0fda9678","updated":"2018-08-30 22:38:03.000000000","message":"Done","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e85dc8dbdcf46c7a246089a50a623a6a554dfe67","unresolved":false,"context_lines":[{"line_number":310,"context_line":"    @mock.patch.object(manager.ComputeManager, \u0027_get_resource_tracker\u0027)"},{"line_number":311,"context_line":"    def test_update_available_resource_for_node_reshape_needed(self, get_rt,"},{"line_number":312,"context_line":"                                                               log_mock):"},{"line_number":313,"context_line":"        \"\"\"ReshapeFailed logs and reraises.\"\"\""},{"line_number":314,"context_line":"        rt \u003d mock.Mock(spec_set\u003d[\u0027update_available_resource\u0027])"},{"line_number":315,"context_line":"        get_rt.return_value \u003d rt"},{"line_number":316,"context_line":"        rt.update_available_resource.side_effect \u003d exception.ReshapeNeeded()"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_0fda9678","line":313,"range":{"start_line":313,"start_character":11,"end_line":313,"end_character":24},"in_reply_to":"3f79a3b5_d171a346","updated":"2018-08-28 21:28:42.000000000","message":"ack","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"}],"nova/virt/driver.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5f49d8fe22ba26646fd2795503612397938d6e9d","unresolved":false,"context_lines":[{"line_number":932,"context_line":"            operation."},{"line_number":933,"context_line":"        :raises ReshapeNeeded: If allocations is None and any inventory needs"},{"line_number":934,"context_line":"            to be moved from one provider to another and/or to a different"},{"line_number":935,"context_line":"            resource class."},{"line_number":936,"context_line":"        \"\"\""},{"line_number":937,"context_line":"        raise NotImplementedError()"},{"line_number":938,"context_line":""}],"source_content_type":"text/x-python","patch_set":35,"id":"3f79a3b5_8ee1d69d","line":935,"updated":"2018-08-28 12:47:45.000000000","message":"Good docstring. ++","commit_id":"bc03dc7237cb666362f7c54c8cd27b4ab999beaa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":923,"context_line":"                },"},{"line_number":924,"context_line":"                ..."},{"line_number":925,"context_line":"              }"},{"line_number":926,"context_line":"            If None, and the method determines that any inventory needs to be"},{"line_number":927,"context_line":"            moved (from one provider to another and/or to a different resource"},{"line_number":928,"context_line":"            class), the ReshapeNeeded exception must be raised. Otherwise, this"},{"line_number":929,"context_line":"            dict must be edited in place to indicate the desired final state of"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_b1d747ef","line":926,"range":{"start_line":926,"start_character":56,"end_line":926,"end_character":65},"updated":"2018-08-28 20:05:59.000000000","message":"and allocations?","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e85dc8dbdcf46c7a246089a50a623a6a554dfe67","unresolved":false,"context_lines":[{"line_number":923,"context_line":"                },"},{"line_number":924,"context_line":"                ..."},{"line_number":925,"context_line":"              }"},{"line_number":926,"context_line":"            If None, and the method determines that any inventory needs to be"},{"line_number":927,"context_line":"            moved (from one provider to another and/or to a different resource"},{"line_number":928,"context_line":"            class), the ReshapeNeeded exception must be raised. Otherwise, this"},{"line_number":929,"context_line":"            dict must be edited in place to indicate the desired final state of"}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_efe07a29","line":926,"range":{"start_line":926,"start_character":56,"end_line":926,"end_character":65},"in_reply_to":"3f79a3b5_b1d747ef","updated":"2018-08-28 21:28:42.000000000","message":"Same response. TL;DR: the method *can\u0027t* determine whether allocations need to be moved, yet.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"713ccb331889526b0eb965c3b346cd12f0fdeaca","unresolved":false,"context_lines":[{"line_number":928,"context_line":"            class), the ReshapeNeeded exception must be raised. Otherwise, this"},{"line_number":929,"context_line":"            dict must be edited in place to indicate the desired final state of"},{"line_number":930,"context_line":"            allocations. Drivers should *only* edit allocation records for"},{"line_number":931,"context_line":"            providers whose inventories are being affected by the reshape"},{"line_number":932,"context_line":"            operation."},{"line_number":933,"context_line":"        :raises ReshapeNeeded: If allocations is None and any inventory needs"},{"line_number":934,"context_line":"            to be moved from one provider to another and/or to a different"},{"line_number":935,"context_line":"            resource class."}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_51027371","line":932,"range":{"start_line":931,"start_character":59,"end_line":932,"end_character":22},"updated":"2018-08-28 20:05:59.000000000","message":"This doesn\u0027t really explain what that is or when it should happen - given the worry about drivers f\u0027ing this up and raising in a periodic, I was thinking we should say something assertive like it is expected for this to only happen on upgrade and compute restart, not dynamically / periodically during normal operation. In other words, it\u0027s not supported outside of rare upgrade scenarios.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0911e8e64ee4aaf649021252d460d98b19a9a335","unresolved":false,"context_lines":[{"line_number":928,"context_line":"            class), the ReshapeNeeded exception must be raised. Otherwise, this"},{"line_number":929,"context_line":"            dict must be edited in place to indicate the desired final state of"},{"line_number":930,"context_line":"            allocations. Drivers should *only* edit allocation records for"},{"line_number":931,"context_line":"            providers whose inventories are being affected by the reshape"},{"line_number":932,"context_line":"            operation."},{"line_number":933,"context_line":"        :raises ReshapeNeeded: If allocations is None and any inventory needs"},{"line_number":934,"context_line":"            to be moved from one provider to another and/or to a different"},{"line_number":935,"context_line":"            resource class."}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_76a3a801","line":932,"range":{"start_line":931,"start_character":59,"end_line":932,"end_character":22},"in_reply_to":"3f79a3b5_0f3cd62e","updated":"2018-08-30 22:38:03.000000000","message":"For now I\u0027ve linked to the spec with a TODO to make real content in the upt doc.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e85dc8dbdcf46c7a246089a50a623a6a554dfe67","unresolved":false,"context_lines":[{"line_number":928,"context_line":"            class), the ReshapeNeeded exception must be raised. Otherwise, this"},{"line_number":929,"context_line":"            dict must be edited in place to indicate the desired final state of"},{"line_number":930,"context_line":"            allocations. Drivers should *only* edit allocation records for"},{"line_number":931,"context_line":"            providers whose inventories are being affected by the reshape"},{"line_number":932,"context_line":"            operation."},{"line_number":933,"context_line":"        :raises ReshapeNeeded: If allocations is None and any inventory needs"},{"line_number":934,"context_line":"            to be moved from one provider to another and/or to a different"},{"line_number":935,"context_line":"            resource class."}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_cfbc9e08","line":932,"range":{"start_line":931,"start_character":59,"end_line":932,"end_character":22},"in_reply_to":"3f79a3b5_51027371","updated":"2018-08-28 21:28:42.000000000","message":"IMO we should only go so far adding words to this docstring. How about we add a link to the spec and/or write a doc and refer to that, like on L877-9?","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"98f3905af65ab8c15fcd7f3dc9db9221fb055bf5","unresolved":false,"context_lines":[{"line_number":928,"context_line":"            class), the ReshapeNeeded exception must be raised. Otherwise, this"},{"line_number":929,"context_line":"            dict must be edited in place to indicate the desired final state of"},{"line_number":930,"context_line":"            allocations. Drivers should *only* edit allocation records for"},{"line_number":931,"context_line":"            providers whose inventories are being affected by the reshape"},{"line_number":932,"context_line":"            operation."},{"line_number":933,"context_line":"        :raises ReshapeNeeded: If allocations is None and any inventory needs"},{"line_number":934,"context_line":"            to be moved from one provider to another and/or to a different"},{"line_number":935,"context_line":"            resource class."}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_0f3cd62e","line":932,"range":{"start_line":931,"start_character":59,"end_line":932,"end_character":22},"in_reply_to":"3f79a3b5_cfbc9e08","updated":"2018-08-28 21:40:22.000000000","message":"\u003e IMO we should only go so far adding words to this docstring. How\n \u003e about we add a link to the spec and/or write a doc and refer to\n \u003e that, like on L877-9?\n\nI agree adding more words to the docstring could be overly confusing. Linking to something in the upt reference docs related to reshaper at a higher level than the spec might be good - something like, reshape is a thing, your driver might need it, here are some guidelines and known issues, and for more details refer to the spec.","commit_id":"232117ecfc37d8538c432594a6ad8a2eed0c224f"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"61e9df598b1277f034e40db921dda9d391785d05","unresolved":false,"context_lines":[{"line_number":6318,"context_line":"    def refresh_instance_security_rules(self, instance):"},{"line_number":6319,"context_line":"        self.firewall_driver.refresh_instance_security_rules(instance)"},{"line_number":6320,"context_line":""},{"line_number":6321,"context_line":"    def update_provider_tree(self, provider_tree, nodename, allocations\u003dNone):"},{"line_number":6322,"context_line":"        \"\"\"Update a ProviderTree object with current resource provider and"},{"line_number":6323,"context_line":"        inventory information."},{"line_number":6324,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_d8a53b4a","line":6321,"updated":"2018-07-11 16:22:19.000000000","message":"ironic has this now too","commit_id":"0250304d8d7c66f44e428cb19d3b5da95d56c562"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e1ca09191b329a681f12b5617f119d0568cbae72","unresolved":false,"context_lines":[{"line_number":6318,"context_line":"    def refresh_instance_security_rules(self, instance):"},{"line_number":6319,"context_line":"        self.firewall_driver.refresh_instance_security_rules(instance)"},{"line_number":6320,"context_line":""},{"line_number":6321,"context_line":"    def update_provider_tree(self, provider_tree, nodename, allocations\u003dNone):"},{"line_number":6322,"context_line":"        \"\"\"Update a ProviderTree object with current resource provider and"},{"line_number":6323,"context_line":"        inventory information."},{"line_number":6324,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_ed6e1bfd","line":6321,"in_reply_to":"5f7c97a3_d8a53b4a","updated":"2018-07-12 15:55:30.000000000","message":"Done","commit_id":"0250304d8d7c66f44e428cb19d3b5da95d56c562"}]}