)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8ab19463d40a2d32621aa9f8f3215b9b1605f6f7","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Julia Kreger \u003cjuliaashleykreger@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-04-25 12:54:21 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"ironic: add instance_uuid before any other spawn activity"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In the baremetal use case, it is necessary to allow a remote"},{"line_number":10,"context_line":"source of truth assert information about a physical node,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9f6a8fd7_d9120f0a","line":7,"range":{"start_line":7,"start_character":8,"end_line":7,"end_character":57},"updated":"2018-04-27 16:19:40.000000000","message":"I\u0027d just call this \"reserve node before spawn\"","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8ab19463d40a2d32621aa9f8f3215b9b1605f6f7","unresolved":false,"context_lines":[{"line_number":7,"context_line":"ironic: add instance_uuid before any other spawn activity"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In the baremetal use case, it is necessary to allow a remote"},{"line_number":10,"context_line":"source of truth assert information about a physical node,"},{"line_number":11,"context_line":"as opposed to a client asserting desired configuration."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"As such, the prepare_networks_before_block_device_mapping virt"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9f6a8fd7_37db197e","line":10,"range":{"start_line":10,"start_character":15,"end_line":10,"end_character":22},"updated":"2018-04-27 16:19:40.000000000","message":"to assert","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8ab19463d40a2d32621aa9f8f3215b9b1605f6f7","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"As such, the prepare_networks_before_block_device_mapping virt"},{"line_number":14,"context_line":"driver method was added, however in doing so network VIF attaching"},{"line_number":15,"context_line":"was moved before actual hardware reservation. As ironic only"},{"line_number":16,"context_line":"supports users to have a number of VIFs limited by the number"},{"line_number":17,"context_line":"of network interfaces in the physical node, VIF attach actions"},{"line_number":18,"context_line":"cannot be performed without first asserting control over the node."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Adding an \"instance_uuid\" upfront allows other ironic API consumers"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9f6a8fd7_d7089dda","line":17,"range":{"start_line":15,"start_character":46,"end_line":17,"end_character":42},"updated":"2018-04-27 16:19:40.000000000","message":"\"As ironic nodes only support as many VIFs as network interfaces on the physical node, \"","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"}],"nova/compute/manager.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"113893ce8394fd02da38929006bdbcec90763d65","unresolved":false,"context_lines":[{"line_number":1994,"context_line":"                                                        request_spec)"},{"line_number":1995,"context_line":"            rt \u003d self._get_resource_tracker()"},{"line_number":1996,"context_line":"            with rt.instance_claim(context, instance, node, limits):"},{"line_number":1997,"context_line":"                self.driver.prepare_for_spawn(instance)"},{"line_number":1998,"context_line":""},{"line_number":1999,"context_line":"                # NOTE(russellb) It\u0027s important that this validation be done"},{"line_number":2000,"context_line":"                # *after* the resource tracker instance claim, as that is where"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f6a8fd7_99f57786","line":1997,"updated":"2018-04-27 16:20:26.000000000","message":"Also, I note that there is no unit test coverage of this added virt driver API method call.","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"884ff480b5dae09675e8fcd448b39929a04eec3c","unresolved":false,"context_lines":[{"line_number":1994,"context_line":"                                                        request_spec)"},{"line_number":1995,"context_line":"            rt \u003d self._get_resource_tracker()"},{"line_number":1996,"context_line":"            with rt.instance_claim(context, instance, node, limits):"},{"line_number":1997,"context_line":"                self.driver.prepare_for_spawn(instance)"},{"line_number":1998,"context_line":""},{"line_number":1999,"context_line":"                # NOTE(russellb) It\u0027s important that this validation be done"},{"line_number":2000,"context_line":"                # *after* the resource tracker instance claim, as that is where"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f6a8fd7_0dfe0105","line":1997,"in_reply_to":"9f6a8fd7_07758298","updated":"2018-04-27 19:00:35.000000000","message":"We could do it in build resources, we would still need a call out to the ironic virt driver to perform clean-up, which is something we\u0027ve forgotten about in this patch... atlhough the only likely case of that happeneing moving forward with some of the cleanup code we\u0027ve added to ironic is if someone were to request 3 neworks when only 2 nics exist in the physical node.","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"89309306d396318538d5606ceabfe73e04e24608","unresolved":false,"context_lines":[{"line_number":1994,"context_line":"                                                        request_spec)"},{"line_number":1995,"context_line":"            rt \u003d self._get_resource_tracker()"},{"line_number":1996,"context_line":"            with rt.instance_claim(context, instance, node, limits):"},{"line_number":1997,"context_line":"                self.driver.prepare_for_spawn(instance)"},{"line_number":1998,"context_line":""},{"line_number":1999,"context_line":"                # NOTE(russellb) It\u0027s important that this validation be done"},{"line_number":2000,"context_line":"                # *after* the resource tracker instance claim, as that is where"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f6a8fd7_4d9dd964","line":1997,"in_reply_to":"9f6a8fd7_0dfe0105","updated":"2018-04-27 19:09:34.000000000","message":"Right, that was my comment on L2269 below.","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1c4a17125163ab7d887ddf422b13ffc068d18ba7","unresolved":false,"context_lines":[{"line_number":1994,"context_line":"                                                        request_spec)"},{"line_number":1995,"context_line":"            rt \u003d self._get_resource_tracker()"},{"line_number":1996,"context_line":"            with rt.instance_claim(context, instance, node, limits):"},{"line_number":1997,"context_line":"                self.driver.prepare_for_spawn(instance)"},{"line_number":1998,"context_line":""},{"line_number":1999,"context_line":"                # NOTE(russellb) It\u0027s important that this validation be done"},{"line_number":2000,"context_line":"                # *after* the resource tracker instance claim, as that is where"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f6a8fd7_07758298","line":1997,"in_reply_to":"9f6a8fd7_19672728","updated":"2018-04-27 17:23:43.000000000","message":"That would make this:\n\n with rt.instance_claim():\n     with self.driver.prepare_for_spawn():\n         with self._build_resources():\n             self.driver.spawn()\n\nWhich is...a bit much for my taste. It also kinda puts the driver in the middle of compute manager business (not that this prepare concept in general doesn\u0027t already, but..). I also feel like right after we do this, someone is going to expect to be able to yield driver-specific context and have us keep track of it for them.\n\nWhy don\u0027t we just integrate this into _build_resources()? We\u0027re already collecting a set of things to use in the spawn, which we roll back if we fail.\n\nWhat if ... (see below).","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"5e2b1e1f91d1922b3c6b18136057aac7d2068f37","unresolved":false,"context_lines":[{"line_number":1994,"context_line":"                                                        request_spec)"},{"line_number":1995,"context_line":"            rt \u003d self._get_resource_tracker()"},{"line_number":1996,"context_line":"            with rt.instance_claim(context, instance, node, limits):"},{"line_number":1997,"context_line":"                self.driver.prepare_for_spawn(instance)"},{"line_number":1998,"context_line":""},{"line_number":1999,"context_line":"                # NOTE(russellb) It\u0027s important that this validation be done"},{"line_number":2000,"context_line":"                # *after* the resource tracker instance claim, as that is where"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f6a8fd7_be5fd8e3","line":1997,"in_reply_to":"9f6a8fd7_4d9dd964","updated":"2018-05-01 01:44:47.000000000","message":"Thinking about it, I\u0027m leaning more towards keeping it out of build_resources if we can help it. For two reasons, it feels more explicit not in and around the yeild. Two, we need to claim the node as quickly as possible and it feels like.. .that we could eventually fail with build in several different places. Of course, I\u0027m only thinking that because I spotted an issue elsewhere we fail to save some additional information to ironic.","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"32f5c3ace255ee9a26c25cc06b075704ce53c337","unresolved":false,"context_lines":[{"line_number":1994,"context_line":"                                                        request_spec)"},{"line_number":1995,"context_line":"            rt \u003d self._get_resource_tracker()"},{"line_number":1996,"context_line":"            with rt.instance_claim(context, instance, node, limits):"},{"line_number":1997,"context_line":"                self.driver.prepare_for_spawn(instance)"},{"line_number":1998,"context_line":""},{"line_number":1999,"context_line":"                # NOTE(russellb) It\u0027s important that this validation be done"},{"line_number":2000,"context_line":"                # *after* the resource tracker instance claim, as that is where"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f6a8fd7_19672728","line":1997,"in_reply_to":"9f6a8fd7_99f57786","updated":"2018-04-27 16:32:33.000000000","message":"Thinking about the cleanup case more, I\u0027m thinking it might be best to do this inside a context manager, because if anything fails along the way during the build, we\u0027re going to want to undo what prepare_for_spawn did. Without a context manager, we\u0027d be adding a whole lot of cleanup_* calls to the various except blocks below.","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1c4a17125163ab7d887ddf422b13ffc068d18ba7","unresolved":false,"context_lines":[{"line_number":2236,"context_line":"            msg \u003d _(\u0027Failure retrieving placement allocations\u0027)"},{"line_number":2237,"context_line":"            raise exception.BuildAbortException(instance_uuid\u003dinstance.uuid,"},{"line_number":2238,"context_line":"                                                reason\u003dmsg)"},{"line_number":2239,"context_line":""},{"line_number":2240,"context_line":"        try:"},{"line_number":2241,"context_line":"            yield resources"},{"line_number":2242,"context_line":"        except Exception as exc:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f6a8fd7_a765b6e3","line":2239,"updated":"2018-04-27 17:23:43.000000000","message":"We could:\n\n resources[\u0027driver_stuff\u0027] \u003d self.driver.prepare_for_spawn()","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"5e2b1e1f91d1922b3c6b18136057aac7d2068f37","unresolved":false,"context_lines":[{"line_number":2236,"context_line":"            msg \u003d _(\u0027Failure retrieving placement allocations\u0027)"},{"line_number":2237,"context_line":"            raise exception.BuildAbortException(instance_uuid\u003dinstance.uuid,"},{"line_number":2238,"context_line":"                                                reason\u003dmsg)"},{"line_number":2239,"context_line":""},{"line_number":2240,"context_line":"        try:"},{"line_number":2241,"context_line":"            yield resources"},{"line_number":2242,"context_line":"        except Exception as exc:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f6a8fd7_be8ef824","line":2239,"in_reply_to":"9f6a8fd7_a765b6e3","updated":"2018-05-01 01:44:47.000000000","message":"Except we need to do it before line 2182... not understanding why you wish to track it in resources. I\u0027m guessing something about nova design at this point.","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"93bd36610d8ebb6a9c1fa34ab3ad3ae64416b94a","unresolved":false,"context_lines":[{"line_number":2236,"context_line":"            msg \u003d _(\u0027Failure retrieving placement allocations\u0027)"},{"line_number":2237,"context_line":"            raise exception.BuildAbortException(instance_uuid\u003dinstance.uuid,"},{"line_number":2238,"context_line":"                                                reason\u003dmsg)"},{"line_number":2239,"context_line":""},{"line_number":2240,"context_line":"        try:"},{"line_number":2241,"context_line":"            yield resources"},{"line_number":2242,"context_line":"        except Exception as exc:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_7206fad0","line":2239,"in_reply_to":"9f6a8fd7_be8ef824","updated":"2018-05-11 15:14:13.000000000","message":"So put it before 2182. I just put it at the end of this series because I thought it was more clear. If it needs to happen before the network thing then do it before the network thing.\n\nDoing it in build_resources means we have one place to call the cleanup if we fail the spawn. As opposed to the many places you have it in the latest version.","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1c4a17125163ab7d887ddf422b13ffc068d18ba7","unresolved":false,"context_lines":[{"line_number":2266,"context_line":"                    raise exception.BuildAbortException("},{"line_number":2267,"context_line":"                            instance_uuid\u003dinstance.uuid,"},{"line_number":2268,"context_line":"                            reason\u003dsix.text_type(exc))"},{"line_number":2269,"context_line":""},{"line_number":2270,"context_line":"    def _cleanup_allocated_networks(self, context, instance,"},{"line_number":2271,"context_line":"            requested_networks):"},{"line_number":2272,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f6a8fd7_47b7fa3b","line":2269,"updated":"2018-04-27 17:23:43.000000000","message":"Here we would:\n\n self.driver.cleanup_your_mess_homey(resources[\u0027driver_stuff\u0027]","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"5e2b1e1f91d1922b3c6b18136057aac7d2068f37","unresolved":false,"context_lines":[{"line_number":2266,"context_line":"                    raise exception.BuildAbortException("},{"line_number":2267,"context_line":"                            instance_uuid\u003dinstance.uuid,"},{"line_number":2268,"context_line":"                            reason\u003dsix.text_type(exc))"},{"line_number":2269,"context_line":""},{"line_number":2270,"context_line":"    def _cleanup_allocated_networks(self, context, instance,"},{"line_number":2271,"context_line":"            requested_networks):"},{"line_number":2272,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f6a8fd7_9e9bf464","line":2269,"in_reply_to":"9f6a8fd7_47b7fa3b","updated":"2018-05-01 01:44:47.000000000","message":"Tacking into network cleanup?","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ce17c48b1c4dc45f0b388514d790a1d74a7cdc3d","unresolved":false,"context_lines":[{"line_number":2027,"context_line":"                             \u0027the hypervisor.\u0027, timer.elapsed(),"},{"line_number":2028,"context_line":"                             instance\u003dinstance)"},{"line_number":2029,"context_line":"        except (exception.InstanceNotFound,"},{"line_number":2030,"context_line":"                exception.UnexpectedDeletingTaskStateError) as e:"},{"line_number":2031,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":2032,"context_line":"                self._notify_about_instance_usage(context, instance,"},{"line_number":2033,"context_line":"                    \u0027create.error\u0027, fault\u003de)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_f721f8fc","line":2030,"updated":"2018-05-07 23:19:57.000000000","message":"I think you would have to failed_spawn_cleanup here in case the instance.save at L2014 fails during a delete race (instance delete lands while instance is building).","commit_id":"43d8506887b89909fb330539248169c4b4df0517"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ce17c48b1c4dc45f0b388514d790a1d74a7cdc3d","unresolved":false,"context_lines":[{"line_number":2035,"context_line":"                    context, instance, self.host,"},{"line_number":2036,"context_line":"                    phase\u003dfields.NotificationPhase.ERROR, exception\u003de,"},{"line_number":2037,"context_line":"                    bdms\u003dblock_device_mapping)"},{"line_number":2038,"context_line":"        except exception.ComputeResourcesUnavailable as e:"},{"line_number":2039,"context_line":"            LOG.debug(e.format_message(), instance\u003dinstance)"},{"line_number":2040,"context_line":"            self._notify_about_instance_usage(context, instance,"},{"line_number":2041,"context_line":"                    \u0027create.error\u0027, fault\u003de)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_978684db","line":2038,"updated":"2018-05-07 23:19:57.000000000","message":"I think it\u0027s correct you don\u0027t need one here because instance_claim is the thing that raises ComputeResourcesUnavailable and prepare_for_spawn is called after that.","commit_id":"43d8506887b89909fb330539248169c4b4df0517"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"93bd36610d8ebb6a9c1fa34ab3ad3ae64416b94a","unresolved":false,"context_lines":[{"line_number":2112,"context_line":"                    context, instance, self.host,"},{"line_number":2113,"context_line":"                    phase\u003dfields.NotificationPhase.ERROR, exception\u003de,"},{"line_number":2114,"context_line":"                    bdms\u003dblock_device_mapping)"},{"line_number":2115,"context_line":"            self.driver.failed_spawn_cleanup(instance)"},{"line_number":2116,"context_line":"            raise exception.RescheduledException("},{"line_number":2117,"context_line":"                    instance_uuid\u003dinstance.uuid, reason\u003dsix.text_type(e))"},{"line_number":2118,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_9234f694","line":2115,"updated":"2018-05-11 15:14:13.000000000","message":"The six places we have to call driver cleanup is exactly why I think this should be in build_resources.","commit_id":"406c5acff56420e5d60fd5837f045aa7323c6580"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f07e3d234e6a9f0168d04e7ccc246a4bc6a0c983","unresolved":false,"context_lines":[{"line_number":2234,"context_line":"                          instance\u003dinstance)"},{"line_number":2235,"context_line":"            # Make sure the async call finishes"},{"line_number":2236,"context_line":"            if network_info is not None:"},{"line_number":2237,"context_line":"                network_info.wait(do_raise\u003dFalse)"},{"line_number":2238,"context_line":"            self.driver.failed_spawn_cleanup(instance)"},{"line_number":2239,"context_line":"            msg \u003d _(\u0027Failure retrieving placement allocations\u0027)"},{"line_number":2240,"context_line":"            raise exception.BuildAbortException(instance_uuid\u003dinstance.uuid,"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_364ff193","line":2237,"updated":"2018-05-14 17:26:56.000000000","message":"Unrelated to this patch: Huh. This looks like a bug that we don\u0027t call self.driver.clean_networks_preparation(instance, network_info) here.","commit_id":"b1ddb4e454e9661f50187f6f39fc5fe075dd1203"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"366b8ef4f83f633e4eb8f343876b679a260991b0","unresolved":false,"context_lines":[{"line_number":2234,"context_line":"                          instance\u003dinstance)"},{"line_number":2235,"context_line":"            # Make sure the async call finishes"},{"line_number":2236,"context_line":"            if network_info is not None:"},{"line_number":2237,"context_line":"                network_info.wait(do_raise\u003dFalse)"},{"line_number":2238,"context_line":"            self.driver.failed_spawn_cleanup(instance)"},{"line_number":2239,"context_line":"            msg \u003d _(\u0027Failure retrieving placement allocations\u0027)"},{"line_number":2240,"context_line":"            raise exception.BuildAbortException(instance_uuid\u003dinstance.uuid,"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_b4fbcc44","line":2237,"in_reply_to":"5f7c97a3_364ff193","updated":"2018-05-16 12:18:40.000000000","message":"Likely a good idea to fix in another or follow-up patch, seems like it might cause issues if resources run out.","commit_id":"b1ddb4e454e9661f50187f6f39fc5fe075dd1203"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ca47bb6e34d436bfb7c7ea81c77a1b1f89d53279","unresolved":false,"context_lines":[{"line_number":2004,"context_line":"                    # instance from _allocate_network_async, as they aren\u0027t"},{"line_number":2005,"context_line":"                    # saved in that function to prevent races."},{"line_number":2006,"context_line":"                    instance.save(expected_task_state\u003d"},{"line_number":2007,"context_line":"                            task_states.BLOCK_DEVICE_MAPPING)"},{"line_number":2008,"context_line":"                    block_device_info \u003d resources[\u0027block_device_info\u0027]"},{"line_number":2009,"context_line":"                    network_info \u003d resources[\u0027network_info\u0027]"},{"line_number":2010,"context_line":"                    allocs \u003d resources[\u0027allocations\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_e0e27e02","line":2007,"updated":"2018-07-02 09:12:46.000000000","message":"If this fails with ironic, does that mean we leave the node assigned to the user now?","commit_id":"f568d6da41f1f2089883aa4e5378585a9662394d"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"0fedc754bea132448bf5b3812d493cd945bbb318","unresolved":false,"context_lines":[{"line_number":2004,"context_line":"                    # instance from _allocate_network_async, as they aren\u0027t"},{"line_number":2005,"context_line":"                    # saved in that function to prevent races."},{"line_number":2006,"context_line":"                    instance.save(expected_task_state\u003d"},{"line_number":2007,"context_line":"                            task_states.BLOCK_DEVICE_MAPPING)"},{"line_number":2008,"context_line":"                    block_device_info \u003d resources[\u0027block_device_info\u0027]"},{"line_number":2009,"context_line":"                    network_info \u003d resources[\u0027network_info\u0027]"},{"line_number":2010,"context_line":"                    allocs \u003d resources[\u0027allocations\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_ca03dc0f","line":2007,"in_reply_to":"5f7c97a3_e0e27e02","updated":"2018-07-02 12:32:05.000000000","message":"Well, it would fail _build_resources, which would mean we would never save the task bdm. This shouldn\u0027t fail, at least from ironic\u0027s standpoint unless the instance save action also invokes the virt driver.","commit_id":"f568d6da41f1f2089883aa4e5378585a9662394d"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ca47bb6e34d436bfb7c7ea81c77a1b1f89d53279","unresolved":false,"context_lines":[{"line_number":2203,"context_line":"                    network_info.wait(do_raise\u003dFalse)"},{"line_number":2204,"context_line":"                    self.driver.clean_networks_preparation(instance,"},{"line_number":2205,"context_line":"                                                           network_info)"},{"line_number":2206,"context_line":"                self.driver.failed_spawn_cleanup(instance)"},{"line_number":2207,"context_line":"        except (exception.UnexpectedTaskStateError,"},{"line_number":2208,"context_line":"                exception.OverQuota, exception.InvalidBDM) as e:"},{"line_number":2209,"context_line":"            # Make sure the async call finishes"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_40f25207","line":2206,"updated":"2018-07-02 09:12:46.000000000","message":"I am getting worried about leaving nodes with the uuid present, and not tidying things up at some point. It does feel like you have covered all the cases there though. Just noting my concern.","commit_id":"f568d6da41f1f2089883aa4e5378585a9662394d"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"0fedc754bea132448bf5b3812d493cd945bbb318","unresolved":false,"context_lines":[{"line_number":2203,"context_line":"                    network_info.wait(do_raise\u003dFalse)"},{"line_number":2204,"context_line":"                    self.driver.clean_networks_preparation(instance,"},{"line_number":2205,"context_line":"                                                           network_info)"},{"line_number":2206,"context_line":"                self.driver.failed_spawn_cleanup(instance)"},{"line_number":2207,"context_line":"        except (exception.UnexpectedTaskStateError,"},{"line_number":2208,"context_line":"                exception.OverQuota, exception.InvalidBDM) as e:"},{"line_number":2209,"context_line":"            # Make sure the async call finishes"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_6d08a226","line":2206,"in_reply_to":"5f7c97a3_40f25207","updated":"2018-07-02 12:32:05.000000000","message":"Noted, thanks!","commit_id":"f568d6da41f1f2089883aa4e5378585a9662394d"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"0fedc754bea132448bf5b3812d493cd945bbb318","unresolved":false,"context_lines":[{"line_number":2258,"context_line":"                # the chance of orphaned ports in neutron"},{"line_number":2259,"context_line":"                deallocate_networks \u003d False if network_info else True"},{"line_number":2260,"context_line":"                try:"},{"line_number":2261,"context_line":"                    self._shutdown_instance(context, instance,"},{"line_number":2262,"context_line":"                            block_device_mapping, requested_networks,"},{"line_number":2263,"context_line":"                            try_deallocate_networks\u003ddeallocate_networks)"},{"line_number":2264,"context_line":"                except Exception as exc2:"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_ad4c7a75","line":2261,"updated":"2018-07-02 12:32:05.000000000","message":"Re John\u0027s concern, I believe since _shutdown_instance gets called, that should take care of the remaining cleanup.","commit_id":"f568d6da41f1f2089883aa4e5378585a9662394d"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d067699ff841772d975a18cd1262cd235f7ffb1","unresolved":false,"context_lines":[{"line_number":5485,"context_line":"                    \u0027_shutdown_instance\u0027),"},{"line_number":5486,"context_line":"                mock.patch.object(self.compute,"},{"line_number":5487,"context_line":"                    \u0027_validate_instance_group_policy\u0027),"},{"line_number":5488,"context_line":"                mock.patch(\u0027nova.compute.utils.notify_about_instance_create\u0027),"},{"line_number":5489,"context_line":"        ) as (spawn, save,"},{"line_number":5490,"context_line":"                _build_networks_for_instance, _notify_about_instance_usage,"},{"line_number":5491,"context_line":"                _shutdown_instance, _validate_instance_group_policy,"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_592e5e92","line":5488,"updated":"2018-05-30 17:22:22.000000000","message":"Unrelated change","commit_id":"b1ddb4e454e9661f50187f6f39fc5fe075dd1203"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d067699ff841772d975a18cd1262cd235f7ffb1","unresolved":false,"context_lines":[{"line_number":5844,"context_line":"        mock_prepspawn.assert_called_once_with(self.instance)"},{"line_number":5845,"context_line":"        # Complete should have occured with _shutdown_instance"},{"line_number":5846,"context_line":"        # so calling after the fact is not necessary."},{"line_number":5847,"context_line":"        mock_failedspawn.assert_not_called()"},{"line_number":5848,"context_line":""},{"line_number":5849,"context_line":"    @mock.patch.object(virt_driver.ComputeDriver, \u0027failed_spawn_cleanup\u0027)"},{"line_number":5850,"context_line":"    @mock.patch.object(virt_driver.ComputeDriver, \u0027prepare_for_spawn\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_7cdf8d65","line":5847,"updated":"2018-05-30 17:22:22.000000000","message":"Note to self: _shutdown_instance calls driver.destroy() and the ironic driver does the _cleanup_deploy there, which is the same thing that failed_spawn_cleanup does.","commit_id":"b1ddb4e454e9661f50187f6f39fc5fe075dd1203"}],"nova/tests/unit/virt/ironic/test_driver.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d067699ff841772d975a18cd1262cd235f7ffb1","unresolved":false,"context_lines":[{"line_number":2341,"context_line":"        expected_patch \u003d [{\u0027path\u0027: \u0027/instance_uuid\u0027, \u0027op\u0027: \u0027add\u0027,"},{"line_number":2342,"context_line":"                           \u0027value\u0027: instance.uuid}]"},{"line_number":2343,"context_line":"        mock_call.has_calls("},{"line_number":2344,"context_line":"            [mock.call(\u0027node.get\u0027, node.uuid, mock.ANY),"},{"line_number":2345,"context_line":"             mock.call(\u0027node.update\u0027, node.uuid,"},{"line_number":2346,"context_line":"                      expected_patch, retry_on_conflict\u003dFalse)])"},{"line_number":2347,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_7c146de9","line":2344,"range":{"start_line":2344,"start_character":46,"end_line":2344,"end_character":54},"updated":"2018-05-30 17:22:22.000000000","message":"Note to self: this is fields\u003dironic_driver._NODE_FIELDS\n\nIf there were a test__get_node, could just mock.patch _get_node instead.","commit_id":"b1ddb4e454e9661f50187f6f39fc5fe075dd1203"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d067699ff841772d975a18cd1262cd235f7ffb1","unresolved":false,"context_lines":[{"line_number":2343,"context_line":"        mock_call.has_calls("},{"line_number":2344,"context_line":"            [mock.call(\u0027node.get\u0027, node.uuid, mock.ANY),"},{"line_number":2345,"context_line":"             mock.call(\u0027node.update\u0027, node.uuid,"},{"line_number":2346,"context_line":"                      expected_patch, retry_on_conflict\u003dFalse)])"},{"line_number":2347,"context_line":""},{"line_number":2348,"context_line":"    @mock.patch.object(cw.IronicClientWrapper, \u0027call\u0027)"},{"line_number":2349,"context_line":"    def test__set_instance_uuid(self, mock_call):"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_fcab9d84","line":2346,"updated":"2018-05-30 17:22:22.000000000","message":"Note: since test__set_instance_uuid already tests this level of detail, this test could just mock.patch _set_instance_uuid instead.","commit_id":"b1ddb4e454e9661f50187f6f39fc5fe075dd1203"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d067699ff841772d975a18cd1262cd235f7ffb1","unresolved":false,"context_lines":[{"line_number":2383,"context_line":"        self.driver.failed_spawn_cleanup(instance)"},{"line_number":2384,"context_line":"        mock_call.assert_called_once_with(\u0027node.get_by_instance_uuid\u0027,"},{"line_number":2385,"context_line":"                                          instance.uuid,"},{"line_number":2386,"context_line":"                                          fields\u003dironic_driver._NODE_FIELDS)"},{"line_number":2387,"context_line":"        self.assertEqual(1, mock_cleanup.call_count)"},{"line_number":2388,"context_line":""},{"line_number":2389,"context_line":"    @mock.patch.object(ironic_driver.IronicDriver, \u0027_stop_firewall\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_1cc9b924","line":2386,"updated":"2018-05-30 17:22:22.000000000","message":"Note to self: this is for the call of _validate_instance_and_node","commit_id":"b1ddb4e454e9661f50187f6f39fc5fe075dd1203"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d067699ff841772d975a18cd1262cd235f7ffb1","unresolved":false,"context_lines":[{"line_number":2404,"context_line":"        mock_stop_fw.assert_called_once_with(instance, None)"},{"line_number":2405,"context_line":"        expected_patch \u003d [{\u0027path\u0027: \u0027/instance_uuid\u0027, \u0027op\u0027: \u0027remove\u0027}]"},{"line_number":2406,"context_line":"        mock_call.has_calls("},{"line_number":2407,"context_line":"            [mock.call(\u0027node.update\u0027, node.uuid, expected_patch)])"},{"line_number":2408,"context_line":""},{"line_number":2409,"context_line":""},{"line_number":2410,"context_line":"class IronicDriverSyncTestCase(IronicDriverTestCase):"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_3cf59557","line":2407,"updated":"2018-05-30 17:22:22.000000000","message":"Note to self: this is for the call of _remove_instance_info_from_node","commit_id":"b1ddb4e454e9661f50187f6f39fc5fe075dd1203"}],"nova/virt/driver.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8ab19463d40a2d32621aa9f8f3215b9b1605f6f7","unresolved":false,"context_lines":[{"line_number":276,"context_line":"        \"\"\"Prepare to spawn instance."},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        Perform any pre-flight checks, tagging, etc. that the virt driver"},{"line_number":279,"context_line":"        must execute before executing the spawn process for a new instance."},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"        :param instance: nova.objects.instance.Instance"},{"line_number":282,"context_line":"                         This function should use the data there to guide"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f6a8fd7_39c8eb72","line":279,"range":{"start_line":279,"start_character":8,"end_line":279,"end_character":37},"updated":"2018-04-27 16:19:40.000000000","message":"maybe reword to \"must perform before executing\"?","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"77847ecfedb2949b44672b0c956916b132f7cd30","unresolved":false,"context_lines":[{"line_number":282,"context_line":"                         This function should use the data there to guide"},{"line_number":283,"context_line":"                         the creation of the new instance."},{"line_number":284,"context_line":"        \"\"\""},{"line_number":285,"context_line":"        pass"},{"line_number":286,"context_line":""},{"line_number":287,"context_line":"    def spawn(self, context, instance, image_meta, injected_files,"},{"line_number":288,"context_line":"              admin_password, allocations, network_info\u003dNone,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f6a8fd7_b9971b3b","line":285,"updated":"2018-04-27 16:21:24.000000000","message":"I think we also need something like, \"cleanup_prepare_for_spawn\" right? Where in the case of the ironic driver, it would unreserve the node and be called if something during spawn fails?","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"884ff480b5dae09675e8fcd448b39929a04eec3c","unresolved":false,"context_lines":[{"line_number":282,"context_line":"                         This function should use the data there to guide"},{"line_number":283,"context_line":"                         the creation of the new instance."},{"line_number":284,"context_line":"        \"\"\""},{"line_number":285,"context_line":"        pass"},{"line_number":286,"context_line":""},{"line_number":287,"context_line":"    def spawn(self, context, instance, image_meta, injected_files,"},{"line_number":288,"context_line":"              admin_password, allocations, network_info\u003dNone,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f6a8fd7_6d5f7d0e","line":285,"in_reply_to":"9f6a8fd7_b9971b3b","updated":"2018-04-27 19:00:35.000000000","message":"Great catch. This is true, we do need this else were going to prhan nodes that fails with vif binding before spawn(). It seems like that would only be the case with telling Ironic of the vifs. This will always fail if someone requests 3 networks but only has 2 network interfaces in the baremetal node.","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"c7e81ce73cdcfcb0c23bfebc6f9517c0fede1bbd","unresolved":false,"context_lines":[{"line_number":289,"context_line":""},{"line_number":290,"context_line":"        Perform any hypervisor clean-up required should the spawn operation"},{"line_number":291,"context_line":"        fail, such as the removal of tags that were added during the"},{"line_number":292,"context_line":"        prepare_for_spawn method."},{"line_number":293,"context_line":""},{"line_number":294,"context_line":"        :param instance: nova.objects.instance.Instance"},{"line_number":295,"context_line":"                         This function should use the data there to guide"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f6a8fd7_4db82f29","line":292,"updated":"2018-05-02 16:12:15.000000000","message":"Add a note that it should be idempotent?","commit_id":"48ceee1bbdde679d8ff087f0cb6eca2b67bb54fd"}],"nova/virt/ironic/driver.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8ab19463d40a2d32621aa9f8f3215b9b1605f6f7","unresolved":false,"context_lines":[{"line_number":343,"context_line":"    def _stop_firewall(self, instance, network_info):"},{"line_number":344,"context_line":"        self.firewall_driver.unfilter_instance(instance, network_info)"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"    def prepare_for_spawn(self, instance):"},{"line_number":347,"context_line":"        node_uuid \u003d instance.get(\u0027node\u0027)"},{"line_number":348,"context_line":"        if not node_uuid:"},{"line_number":349,"context_line":"            raise ironic.exc.BadRequest("}],"source_content_type":"text/x-python","patch_set":2,"id":"9f6a8fd7_19bc07ba","line":346,"updated":"2018-04-27 16:19:40.000000000","message":"maybe add a quick:\n\n LOG.debug(\"Preparing to spawn instance.\", instance\u003dinstance)\n\nhere.","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8ab19463d40a2d32621aa9f8f3215b9b1605f6f7","unresolved":false,"context_lines":[{"line_number":349,"context_line":"            raise ironic.exc.BadRequest("},{"line_number":350,"context_line":"                _(\"Ironic node uuid not supplied to \""},{"line_number":351,"context_line":"                  \"driver for instance %s.\") % instance.uuid)"},{"line_number":352,"context_line":"        node \u003d self._get_node(node_uuid)"},{"line_number":353,"context_line":"        patch \u003d [{\u0027path\u0027: \u0027/instance_uuid\u0027, \u0027op\u0027: \u0027add\u0027,"},{"line_number":354,"context_line":"                  \u0027value\u0027: instance.uuid}]"},{"line_number":355,"context_line":"        try:"},{"line_number":356,"context_line":"            # NOTE(TheJulia): Assert an instance UUID to lock the node"},{"line_number":357,"context_line":"            # from other deployment attempts while configuration is"},{"line_number":358,"context_line":"            # being set."},{"line_number":359,"context_line":"            self.ironicclient.call(\u0027node.update\u0027, node.uuid, patch,"},{"line_number":360,"context_line":"                                   retry_on_conflict\u003dFalse)"},{"line_number":361,"context_line":"        except ironic.exc.BadRequest:"},{"line_number":362,"context_line":"            msg \u003d (_(\"Failed to reserve node %(node)s \""},{"line_number":363,"context_line":"                     \"when provisioning the instance %(instance)s\")"},{"line_number":364,"context_line":"                   % {\u0027node\u0027: node.uuid, \u0027instance\u0027: instance.uuid})"},{"line_number":365,"context_line":"            LOG.error(msg)"},{"line_number":366,"context_line":"            raise exception.InstanceDeployFailure(msg)"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"    def _add_instance_info_to_node(self, node, instance, image_meta, flavor,"},{"line_number":369,"context_line":"                                   preserve_ephemeral\u003dNone,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f6a8fd7_b982bbff","line":366,"range":{"start_line":352,"start_character":8,"end_line":366,"end_character":54},"updated":"2018-04-27 16:19:40.000000000","message":"recommend pulling this into a separate IronicDriver._node_reserve() method.\n\nAnd consider adding a:\n\n LOG.debug(\"Reserving node %(node)s for instance.\", node, instance\u003dinstance)\n\nto the _node_reserve() method.","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"5e2b1e1f91d1922b3c6b18136057aac7d2068f37","unresolved":false,"context_lines":[{"line_number":349,"context_line":"            raise ironic.exc.BadRequest("},{"line_number":350,"context_line":"                _(\"Ironic node uuid not supplied to \""},{"line_number":351,"context_line":"                  \"driver for instance %s.\") % instance.uuid)"},{"line_number":352,"context_line":"        node \u003d self._get_node(node_uuid)"},{"line_number":353,"context_line":"        patch \u003d [{\u0027path\u0027: \u0027/instance_uuid\u0027, \u0027op\u0027: \u0027add\u0027,"},{"line_number":354,"context_line":"                  \u0027value\u0027: instance.uuid}]"},{"line_number":355,"context_line":"        try:"},{"line_number":356,"context_line":"            # NOTE(TheJulia): Assert an instance UUID to lock the node"},{"line_number":357,"context_line":"            # from other deployment attempts while configuration is"},{"line_number":358,"context_line":"            # being set."},{"line_number":359,"context_line":"            self.ironicclient.call(\u0027node.update\u0027, node.uuid, patch,"},{"line_number":360,"context_line":"                                   retry_on_conflict\u003dFalse)"},{"line_number":361,"context_line":"        except ironic.exc.BadRequest:"},{"line_number":362,"context_line":"            msg \u003d (_(\"Failed to reserve node %(node)s \""},{"line_number":363,"context_line":"                     \"when provisioning the instance %(instance)s\")"},{"line_number":364,"context_line":"                   % {\u0027node\u0027: node.uuid, \u0027instance\u0027: instance.uuid})"},{"line_number":365,"context_line":"            LOG.error(msg)"},{"line_number":366,"context_line":"            raise exception.InstanceDeployFailure(msg)"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"    def _add_instance_info_to_node(self, node, instance, image_meta, flavor,"},{"line_number":369,"context_line":"                                   preserve_ephemeral\u003dNone,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f6a8fd7_bedcb818","line":366,"range":{"start_line":352,"start_character":8,"end_line":366,"end_character":54},"in_reply_to":"9f6a8fd7_b982bbff","updated":"2018-05-01 01:44:47.000000000","message":"agreed, I\u0027m going to push a wip up and likely refactor towards this when I work on the tests.","commit_id":"f78e131d98728a105c84b2c0bdc23b93ad4aa4b0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"c7e81ce73cdcfcb0c23bfebc6f9517c0fede1bbd","unresolved":false,"context_lines":[{"line_number":374,"context_line":"    def failed_spawn_cleanup(self, instance):"},{"line_number":375,"context_line":"        node_uuid \u003d instance.get(\u0027node\u0027)"},{"line_number":376,"context_line":"        if not node_uuid:"},{"line_number":377,"context_line":"            raise ironic.exc.BadRequest("},{"line_number":378,"context_line":"                _(\"Ironic node uuid not supplied to \""},{"line_number":379,"context_line":"                  \"driver for instance %s.\") % instance.uuid)"},{"line_number":380,"context_line":"        node \u003d self._get_node(node_uuid)"},{"line_number":381,"context_line":"        # TODO(TheJulia): We should pull a list of set vifs, although"},{"line_number":382,"context_line":"        # that would be redundant."}],"source_content_type":"text/x-python","patch_set":5,"id":"9f6a8fd7_4d910f99","line":379,"range":{"start_line":377,"start_character":0,"end_line":379,"end_character":61},"updated":"2018-05-02 16:12:15.000000000","message":"I don\u0027t think we should raise exceptions in the failed_spawn method. I\u0027d leave a warning and just return.","commit_id":"48ceee1bbdde679d8ff087f0cb6eca2b67bb54fd"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"dc0ee94ff9f5a4e089ab1e2c1dbef67f1f08e096","unresolved":false,"context_lines":[{"line_number":374,"context_line":"    def failed_spawn_cleanup(self, instance):"},{"line_number":375,"context_line":"        node_uuid \u003d instance.get(\u0027node\u0027)"},{"line_number":376,"context_line":"        if not node_uuid:"},{"line_number":377,"context_line":"            raise ironic.exc.BadRequest("},{"line_number":378,"context_line":"                _(\"Ironic node uuid not supplied to \""},{"line_number":379,"context_line":"                  \"driver for instance %s.\") % instance.uuid)"},{"line_number":380,"context_line":"        node \u003d self._get_node(node_uuid)"},{"line_number":381,"context_line":"        # TODO(TheJulia): We should pull a list of set vifs, although"},{"line_number":382,"context_line":"        # that would be redundant."}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_32cc9088","line":379,"range":{"start_line":377,"start_character":0,"end_line":379,"end_character":61},"in_reply_to":"9f6a8fd7_4d910f99","updated":"2018-05-03 21:11:24.000000000","message":"agreed, updating.","commit_id":"48ceee1bbdde679d8ff087f0cb6eca2b67bb54fd"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"3ce91e092d58a5faff4fb76dfa675628012e4903","unresolved":false,"context_lines":[{"line_number":482,"context_line":"        self._cleanup_volume_target_info(instance)"},{"line_number":483,"context_line":"        self._unplug_vifs(node, instance, network_info)"},{"line_number":484,"context_line":"        self._stop_firewall(instance, network_info)"},{"line_number":485,"context_line":"        self._remove_instance_info_from_node(node, instance)"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":"    def _wait_for_active(self, instance):"},{"line_number":488,"context_line":"        \"\"\"Wait for the node to be marked as ACTIVE in Ironic.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f6a8fd7_ae2466c7","line":485,"updated":"2018-05-02 15:33:31.000000000","message":"Will this succeed if called twice? I think it will, but we may want to check. It will now be called twice here: https://github.com/openstack/nova/blob/master/nova/virt/ironic/driver.py#L1202-L1204","commit_id":"48ceee1bbdde679d8ff087f0cb6eca2b67bb54fd"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"dc0ee94ff9f5a4e089ab1e2c1dbef67f1f08e096","unresolved":false,"context_lines":[{"line_number":482,"context_line":"        self._cleanup_volume_target_info(instance)"},{"line_number":483,"context_line":"        self._unplug_vifs(node, instance, network_info)"},{"line_number":484,"context_line":"        self._stop_firewall(instance, network_info)"},{"line_number":485,"context_line":"        self._remove_instance_info_from_node(node, instance)"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":"    def _wait_for_active(self, instance):"},{"line_number":488,"context_line":"        \"\"\"Wait for the node to be marked as ACTIVE in Ironic.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_32f570cc","line":485,"in_reply_to":"9f6a8fd7_ae2466c7","updated":"2018-05-03 21:11:24.000000000","message":"Just validated that we don\u0027t raise an exception if we ship the same path twice and and no values are actually set.","commit_id":"48ceee1bbdde679d8ff087f0cb6eca2b67bb54fd"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d067699ff841772d975a18cd1262cd235f7ffb1","unresolved":false,"context_lines":[{"line_number":341,"context_line":"        self.firewall_driver.apply_instance_filter(instance, network_info)"},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"    def _stop_firewall(self, instance, network_info):"},{"line_number":344,"context_line":"        self.firewall_driver.unfilter_instance(instance, network_info)"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"    def _set_instance_uuid(self, node, instance):"},{"line_number":347,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_c7704a75","line":344,"updated":"2018-05-30 17:22:22.000000000","message":"Note to self: I checked unfilter_instance and it doesn\u0027t use network_info and thus handles network_info\u003dNone","commit_id":"b1ddb4e454e9661f50187f6f39fc5fe075dd1203"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d067699ff841772d975a18cd1262cd235f7ffb1","unresolved":false,"context_lines":[{"line_number":362,"context_line":""},{"line_number":363,"context_line":"    def prepare_for_spawn(self, instance):"},{"line_number":364,"context_line":"        LOG.debug(\u0027Preparing to spawn instance %(instance)s.\u0027,"},{"line_number":365,"context_line":"                  instance\u003dinstance.uuid)"},{"line_number":366,"context_line":"        node_uuid \u003d instance.get(\u0027node\u0027)"},{"line_number":367,"context_line":"        if not node_uuid:"},{"line_number":368,"context_line":"            raise ironic.exc.BadRequest("}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_5c75f130","line":365,"range":{"start_line":365,"start_character":18,"end_line":365,"end_character":40},"updated":"2018-05-30 17:22:22.000000000","message":"I don\u0027t think this will match to %(instance)s, you need dict(instance\u003dinstance.uuid) or {\u0027instance\u0027: instance.uuid} instead.\n\nThe instance\u003dinstance.uuid would prepend the log message with e.g. \"[instance: b6ada5ab-6e0c-4bf7-8d4d-6f484184d236]\" if that\u0027s what you want.\n\nSo you either want to remove the \"%(instance)s\" or change to a dictionary or add a dictionary if you want both the prepended tag and the message \"Preparing to spawn instance \u003cuuid\u003e\"","commit_id":"b1ddb4e454e9661f50187f6f39fc5fe075dd1203"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"fac8a23f578be846c4119493286d061428ba58d6","unresolved":false,"context_lines":[{"line_number":362,"context_line":""},{"line_number":363,"context_line":"    def prepare_for_spawn(self, instance):"},{"line_number":364,"context_line":"        LOG.debug(\u0027Preparing to spawn instance %(instance)s.\u0027,"},{"line_number":365,"context_line":"                  instance\u003dinstance.uuid)"},{"line_number":366,"context_line":"        node_uuid \u003d instance.get(\u0027node\u0027)"},{"line_number":367,"context_line":"        if not node_uuid:"},{"line_number":368,"context_line":"            raise ironic.exc.BadRequest("}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_f50ac287","line":365,"range":{"start_line":365,"start_character":18,"end_line":365,"end_character":40},"in_reply_to":"5f7c97a3_5c75f130","updated":"2018-05-31 21:16:26.000000000","message":"Doh, great catch, will fix in next revision. Thanks!","commit_id":"b1ddb4e454e9661f50187f6f39fc5fe075dd1203"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d067699ff841772d975a18cd1262cd235f7ffb1","unresolved":false,"context_lines":[{"line_number":382,"context_line":"                        \u0027present on the node.\u0027, instance.uuid)"},{"line_number":383,"context_line":"            return"},{"line_number":384,"context_line":"        # TODO(TheJulia): We should pull a list of set vifs, although"},{"line_number":385,"context_line":"        # that would be redundant."},{"line_number":386,"context_line":"        self._cleanup_deploy(node, instance)"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":"    def _add_instance_info_to_node(self, node, instance, image_meta, flavor,"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_e73ea689","line":385,"updated":"2018-05-30 17:22:22.000000000","message":"What do you mean by this comment? Looking in _unplug_vifs, it will iterate across the vifs in network_info and detach them, but this is going to use network_info\u003dNone, so vifs won\u0027t be unplugged. Is this going to orphan vifs then? Do we need to take care of getting a list of set vifs now instead of later?","commit_id":"b1ddb4e454e9661f50187f6f39fc5fe075dd1203"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c2b0ffec91f438ffe22e72ec9451f74eea9f0a65","unresolved":false,"context_lines":[{"line_number":382,"context_line":"                        \u0027present on the node.\u0027, instance.uuid)"},{"line_number":383,"context_line":"            return"},{"line_number":384,"context_line":"        # TODO(TheJulia): We should pull a list of set vifs, although"},{"line_number":385,"context_line":"        # that would be redundant."},{"line_number":386,"context_line":"        self._cleanup_deploy(node, instance)"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":"    def _add_instance_info_to_node(self, node, instance, image_meta, flavor,"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_d19f90b1","line":385,"in_reply_to":"5f7c97a3_7524720d","updated":"2018-05-31 21:26:38.000000000","message":"Ah, right, the clean_networks_preparation calls (that are already existing) will take care of unplugging vifs. Thanks.","commit_id":"b1ddb4e454e9661f50187f6f39fc5fe075dd1203"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"79bbc78f75d88d2e506e50e972e41f46ce1baac5","unresolved":false,"context_lines":[{"line_number":382,"context_line":"                        \u0027present on the node.\u0027, instance.uuid)"},{"line_number":383,"context_line":"            return"},{"line_number":384,"context_line":"        # TODO(TheJulia): We should pull a list of set vifs, although"},{"line_number":385,"context_line":"        # that would be redundant."},{"line_number":386,"context_line":"        self._cleanup_deploy(node, instance)"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":"    def _add_instance_info_to_node(self, node, instance, image_meta, flavor,"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_91f35809","line":385,"in_reply_to":"5f7c97a3_7524720d","updated":"2018-05-31 21:29:31.000000000","message":"Removing, since it doesn\u0027t make sense to do.","commit_id":"b1ddb4e454e9661f50187f6f39fc5fe075dd1203"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"fac8a23f578be846c4119493286d061428ba58d6","unresolved":false,"context_lines":[{"line_number":382,"context_line":"                        \u0027present on the node.\u0027, instance.uuid)"},{"line_number":383,"context_line":"            return"},{"line_number":384,"context_line":"        # TODO(TheJulia): We should pull a list of set vifs, although"},{"line_number":385,"context_line":"        # that would be redundant."},{"line_number":386,"context_line":"        self._cleanup_deploy(node, instance)"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":"    def _add_instance_info_to_node(self, node, instance, image_meta, flavor,"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_7524720d","line":385,"in_reply_to":"5f7c97a3_e73ea689","updated":"2018-05-31 21:16:26.000000000","message":"I was thinking of going an extra step and just telling the driver to make sure it is leaving the node with no vifs. Not really necessary since the cleanup should have taken care of the ones with network_info in the clean netwrok preparation step. More an idea of leaving the ironic node in what should be a good clean state. tl;dr me being over concerned.","commit_id":"b1ddb4e454e9661f50187f6f39fc5fe075dd1203"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d067699ff841772d975a18cd1262cd235f7ffb1","unresolved":false,"context_lines":[{"line_number":481,"context_line":"        self._cleanup_volume_target_info(instance)"},{"line_number":482,"context_line":"        self._unplug_vifs(node, instance, network_info)"},{"line_number":483,"context_line":"        self._stop_firewall(instance, network_info)"},{"line_number":484,"context_line":"        self._remove_instance_info_from_node(node, instance)"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"    def _wait_for_active(self, instance):"},{"line_number":487,"context_line":"        \"\"\"Wait for the node to be marked as ACTIVE in Ironic.\"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_470afad1","line":484,"updated":"2018-05-30 17:22:22.000000000","message":"Note to self: makes sense that this is last as it releases the node \"lock\"","commit_id":"b1ddb4e454e9661f50187f6f39fc5fe075dd1203"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d067699ff841772d975a18cd1262cd235f7ffb1","unresolved":false,"context_lines":[{"line_number":1459,"context_line":"                  {\u0027uuid\u0027: instance.uuid,"},{"line_number":1460,"context_line":"                   \u0027network_info\u0027: network_info_str})"},{"line_number":1461,"context_line":"        if not network_info:"},{"line_number":1462,"context_line":"            return"},{"line_number":1463,"context_line":"        for vif in network_info:"},{"line_number":1464,"context_line":"            port_id \u003d six.text_type(vif[\u0027id\u0027])"},{"line_number":1465,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_9c7869c4","line":1462,"updated":"2018-05-30 17:22:22.000000000","message":"Note to self: this handles network_info\u003dNone","commit_id":"b1ddb4e454e9661f50187f6f39fc5fe075dd1203"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"08d36eff27038d459455c5d59597c2a25fd772bc","unresolved":false,"context_lines":[{"line_number":1232,"context_line":"                # NOTE(hshiina): if spawn() fails before ironic starts"},{"line_number":1233,"context_line":"                #                provisioning, instance information should be"},{"line_number":1234,"context_line":"                #                removed from ironic node."},{"line_number":1235,"context_line":"                self._remove_instance_info_from_node(node, instance)"},{"line_number":1236,"context_line":"        finally:"},{"line_number":1237,"context_line":"            self._cleanup_deploy(node, instance, network_info)"},{"line_number":1238,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_f7b00543","line":1235,"updated":"2018-06-19 13:49:26.000000000","message":"this one can be removed now?","commit_id":"f568d6da41f1f2089883aa4e5378585a9662394d"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"0fedc754bea132448bf5b3812d493cd945bbb318","unresolved":false,"context_lines":[{"line_number":1232,"context_line":"                # NOTE(hshiina): if spawn() fails before ironic starts"},{"line_number":1233,"context_line":"                #                provisioning, instance information should be"},{"line_number":1234,"context_line":"                #                removed from ironic node."},{"line_number":1235,"context_line":"                self._remove_instance_info_from_node(node, instance)"},{"line_number":1236,"context_line":"        finally:"},{"line_number":1237,"context_line":"            self._cleanup_deploy(node, instance, network_info)"},{"line_number":1238,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_61fdcbb0","line":1235,"in_reply_to":"5f7c97a3_f7b00543","updated":"2018-07-02 12:32:05.000000000","message":"I guess it can be, we can also just do that in a follow-up.","commit_id":"f568d6da41f1f2089883aa4e5378585a9662394d"}]}
