)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"da1d6000f702b0b4246dab8698930d44945ad9af","unresolved":false,"context_lines":[{"line_number":1688,"context_line":"                sys_meta[\u0027network_allocated\u0027] \u003d \u0027True\u0027"},{"line_number":1689,"context_line":"                self._instance_update(context, instance.uuid,"},{"line_number":1690,"context_line":"                        system_metadata\u003dsys_meta)"},{"line_number":1691,"context_line":"                return nwinfo"},{"line_number":1692,"context_line":"            except Exception:"},{"line_number":1693,"context_line":"                exc_info \u003d sys.exc_info()"},{"line_number":1694,"context_line":"                log_info \u003d {\u0027attempt\u0027: attempt,"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba7be1f8_908fa329","line":1691,"updated":"2015-02-25 13:27:56.000000000","message":"If I understand things correctly, the issue is that we\u0027re failing here but the port has actually been created?  I think a fix should attempt to reduce the chance of failure here, and in the extreme case of failure cause the build to fail here because Nova doesn\u0027t know the state of things.","commit_id":"5a1b8b35f7bd056ed13fd79cacdf3bdff035c10f"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"00b9febe8bec8c69ce695ffacfe46d3a126e6cca","unresolved":false,"context_lines":[{"line_number":1688,"context_line":"                sys_meta[\u0027network_allocated\u0027] \u003d \u0027True\u0027"},{"line_number":1689,"context_line":"                self._instance_update(context, instance.uuid,"},{"line_number":1690,"context_line":"                        system_metadata\u003dsys_meta)"},{"line_number":1691,"context_line":"                return nwinfo"},{"line_number":1692,"context_line":"            except Exception:"},{"line_number":1693,"context_line":"                exc_info \u003d sys.exc_info()"},{"line_number":1694,"context_line":"                log_info \u003d {\u0027attempt\u0027: attempt,"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba7be1f8_48d3895f","line":1691,"in_reply_to":"ba7be1f8_908fa329","updated":"2015-02-26 16:01:20.000000000","message":"right, it\u0027s allocate_for_instance() call at #1679 which is raising so sys_meta[\u0027network_allocated\u0027] was not set to True. However we don\u0027t know if ports were created or not. Not sure I understand how to reduce the chance of failure here. Please see below my proposal for the fix.","commit_id":"5a1b8b35f7bd056ed13fd79cacdf3bdff035c10f"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"da1d6000f702b0b4246dab8698930d44945ad9af","unresolved":false,"context_lines":[{"line_number":1712,"context_line":""},{"line_number":1713,"context_line":"        # If we\u0027re here from a reschedule the network may already be allocated."},{"line_number":1714,"context_line":"        if strutils.bool_from_string("},{"line_number":1715,"context_line":"                instance.system_metadata.get(\u0027network_allocated\u0027, \u0027False\u0027)):"},{"line_number":1716,"context_line":"            # NOTE(alex_xu): The network_allocated is True means the network"},{"line_number":1717,"context_line":"            # resource already allocated at previous scheduling, and the"},{"line_number":1718,"context_line":"            # network setup is cleanup at previous. After rescheduling, the"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba7be1f8_90d7a34a","line":1715,"updated":"2015-02-25 13:27:56.000000000","message":"This is where the re-use happens on a reschedule.","commit_id":"5a1b8b35f7bd056ed13fd79cacdf3bdff035c10f"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"00b9febe8bec8c69ce695ffacfe46d3a126e6cca","unresolved":false,"context_lines":[{"line_number":1712,"context_line":""},{"line_number":1713,"context_line":"        # If we\u0027re here from a reschedule the network may already be allocated."},{"line_number":1714,"context_line":"        if strutils.bool_from_string("},{"line_number":1715,"context_line":"                instance.system_metadata.get(\u0027network_allocated\u0027, \u0027False\u0027)):"},{"line_number":1716,"context_line":"            # NOTE(alex_xu): The network_allocated is True means the network"},{"line_number":1717,"context_line":"            # resource already allocated at previous scheduling, and the"},{"line_number":1718,"context_line":"            # network setup is cleanup at previous. After rescheduling, the"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba7be1f8_085431c8","line":1715,"in_reply_to":"ba7be1f8_90d7a34a","updated":"2015-02-26 16:01:20.000000000","message":"Yeah, thanks. So with this bug we get False here and do network allocation again on new compute.","commit_id":"5a1b8b35f7bd056ed13fd79cacdf3bdff035c10f"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"9a9a95710dc325a79587cf26e1ae2f136d8c8a26","unresolved":false,"context_lines":[{"line_number":2085,"context_line":"                    decoded_files, admin_password, requested_networks,"},{"line_number":2086,"context_line":"                    security_groups, block_device_mapping, node, limits,"},{"line_number":2087,"context_line":"                    filter_properties)"},{"line_number":2088,"context_line":"            return build_results.ACTIVE"},{"line_number":2089,"context_line":"        except exception.RescheduledException as e:"},{"line_number":2090,"context_line":"            LOG.debug(e.format_message(), instance\u003dinstance)"},{"line_number":2091,"context_line":"            retry \u003d filter_properties.get(\u0027retry\u0027, None)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba7be1f8_9f1ef5b9","line":2088,"updated":"2015-02-24 20:00:12.000000000","message":"There are 4 exception cases here, and three of them deallocate the network.  This one does not because the instance is going to be rescheduled and can potentially use the network that has already been allocated.  I\u0027m not sure where the port leak is here.","commit_id":"5a1b8b35f7bd056ed13fd79cacdf3bdff035c10f"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"467baf82e5b221d064c988ba757c24db3376acc7","unresolved":false,"context_lines":[{"line_number":2085,"context_line":"                    decoded_files, admin_password, requested_networks,"},{"line_number":2086,"context_line":"                    security_groups, block_device_mapping, node, limits,"},{"line_number":2087,"context_line":"                    filter_properties)"},{"line_number":2088,"context_line":"            return build_results.ACTIVE"},{"line_number":2089,"context_line":"        except exception.RescheduledException as e:"},{"line_number":2090,"context_line":"            LOG.debug(e.format_message(), instance\u003dinstance)"},{"line_number":2091,"context_line":"            retry \u003d filter_properties.get(\u0027retry\u0027, None)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba7be1f8_69f7b759","line":2088,"in_reply_to":"ba7be1f8_6fe5feda","updated":"2015-02-25 10:26:49.000000000","message":"Now I see how reusing allocated networks works. The reason for port leak is that network allocation was not considered successful - ports were created but failed to get them from neutron.","commit_id":"5a1b8b35f7bd056ed13fd79cacdf3bdff035c10f"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"d0edc49150dfaa70d221473f953660b7a652f6ca","unresolved":false,"context_lines":[{"line_number":2085,"context_line":"                    decoded_files, admin_password, requested_networks,"},{"line_number":2086,"context_line":"                    security_groups, block_device_mapping, node, limits,"},{"line_number":2087,"context_line":"                    filter_properties)"},{"line_number":2088,"context_line":"            return build_results.ACTIVE"},{"line_number":2089,"context_line":"        except exception.RescheduledException as e:"},{"line_number":2090,"context_line":"            LOG.debug(e.format_message(), instance\u003dinstance)"},{"line_number":2091,"context_line":"            retry \u003d filter_properties.get(\u0027retry\u0027, None)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba7be1f8_6fe5feda","line":2088,"in_reply_to":"ba7be1f8_9f1ef5b9","updated":"2015-02-25 08:32:49.000000000","message":"In neutron api I don\u0027t see the mechanism for reusing allocated networks on rescheduling (neither in nova-net api) - another compute will allocate network resources from scratch which leads to orphaned ports in neutron.\nI don\u0027t know why self.driver.deallocate_networks_on_reschedule() returns False by default, probably it should return True. I might be missing something. What do you think?","commit_id":"5a1b8b35f7bd056ed13fd79cacdf3bdff035c10f"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"00b9febe8bec8c69ce695ffacfe46d3a126e6cca","unresolved":false,"context_lines":[{"line_number":2321,"context_line":"                        LOG.exception(_LE(\u0027Instance failed to spawn\u0027),"},{"line_number":2322,"context_line":"                                instance\u003dinstance)"},{"line_number":2323,"context_line":"                # Make sure the async call finishes"},{"line_number":2324,"context_line":"                if network_info is not None:"},{"line_number":2325,"context_line":"                    network_info.wait(do_raise\u003dFalse)"},{"line_number":2326,"context_line":"                try:"},{"line_number":2327,"context_line":"                    self._shutdown_instance(context, instance,"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba7be1f8_c8a5b9a2","line":2324,"updated":"2015-02-26 16:01:20.000000000","message":"In case network allocation failed network_info will be empty. Due to allocate_for_instance nature here we can\u0027t say for sure if ports were created or not. So the proposal is to deallocate networks if network_info is empty to be on the safe side. If it\u0027s not empty it will be deleted later in this code path or reused in case of rescheduling.","commit_id":"5a1b8b35f7bd056ed13fd79cacdf3bdff035c10f"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"9a9a95710dc325a79587cf26e1ae2f136d8c8a26","unresolved":false,"context_lines":[{"line_number":2453,"context_line":"                self._try_deallocate_network(context, instance,"},{"line_number":2454,"context_line":"                                             requested_networks)"},{"line_number":2455,"context_line":""},{"line_number":2456,"context_line":"        self._try_deallocate_network(context, instance, requested_networks)"},{"line_number":2457,"context_line":""},{"line_number":2458,"context_line":"        for bdm in vol_bdms:"},{"line_number":2459,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba7be1f8_5fdd5ddf","line":2456,"updated":"2015-02-24 20:00:12.000000000","message":"If the network is deallocated here then it won\u0027t be available if the instance is rescheduled.  At the very least you would need to set the \u0027network_allocated\u0027 system metadata like cleanup_allocated_network does.  But I\u0027m not yet convinced that this is fixing anything.","commit_id":"5a1b8b35f7bd056ed13fd79cacdf3bdff035c10f"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"d0edc49150dfaa70d221473f953660b7a652f6ca","unresolved":false,"context_lines":[{"line_number":2453,"context_line":"                self._try_deallocate_network(context, instance,"},{"line_number":2454,"context_line":"                                             requested_networks)"},{"line_number":2455,"context_line":""},{"line_number":2456,"context_line":"        self._try_deallocate_network(context, instance, requested_networks)"},{"line_number":2457,"context_line":""},{"line_number":2458,"context_line":"        for bdm in vol_bdms:"},{"line_number":2459,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba7be1f8_4f383aef","line":2456,"in_reply_to":"ba7be1f8_5fdd5ddf","updated":"2015-02-25 08:32:49.000000000","message":"I\u0027m not sure how reusing allocated network works on rescheduling, I was unable to find it. However in this case network allocation wasn\u0027t successful (in fact was partially successful - ports were created but timeout when trying to fetch them to build nw info) and \u0027network_allocated\u0027 was not set to True at #1688 so no need to reset it I guess.","commit_id":"5a1b8b35f7bd056ed13fd79cacdf3bdff035c10f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"02dd5ee54080c6f3aadfa52fb30edf714769bfe6","unresolved":false,"context_lines":[{"line_number":2326,"context_line":"                # if network_info is empty we\u0027re likely here because of"},{"line_number":2327,"context_line":"                # network allocation failure. Since nothing can be reused on"},{"line_number":2328,"context_line":"                # rescheduling it\u0027s better to deallocate network to eliminate"},{"line_number":2329,"context_line":"                # the chance of orphaned ports in neutron. Bug 1423845."},{"line_number":2330,"context_line":"                deallocate_networks \u003d False if network_info else True"},{"line_number":2331,"context_line":"                try:"},{"line_number":2332,"context_line":"                    self._shutdown_instance(context, instance,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_12ac128c","line":2329,"updated":"2015-03-17 19:10:02.000000000","message":"We don\u0027t need the bug reference in the comment, that\u0027s tracked with git history.","commit_id":"5f784067c03103c60b7d23a158c5971400666b0c"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"d39ff56707f29d1859f310da62d32e076e3ad1f7","unresolved":false,"context_lines":[{"line_number":2326,"context_line":"                # if network_info is empty we\u0027re likely here because of"},{"line_number":2327,"context_line":"                # network allocation failure. Since nothing can be reused on"},{"line_number":2328,"context_line":"                # rescheduling it\u0027s better to deallocate network to eliminate"},{"line_number":2329,"context_line":"                # the chance of orphaned ports in neutron. Bug 1423845."},{"line_number":2330,"context_line":"                deallocate_networks \u003d False if network_info else True"},{"line_number":2331,"context_line":"                try:"},{"line_number":2332,"context_line":"                    self._shutdown_instance(context, instance,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_8114a944","line":2329,"in_reply_to":"9a80dd14_12ac128c","updated":"2015-03-20 08:10:39.000000000","message":"Done","commit_id":"5f784067c03103c60b7d23a158c5971400666b0c"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5dad541bbfea7523d0b96e5b914fd5776decef69","unresolved":false,"context_lines":[{"line_number":2327,"context_line":"                # network allocation failure. Since nothing can be reused on"},{"line_number":2328,"context_line":"                # rescheduling it\u0027s better to deallocate network to eliminate"},{"line_number":2329,"context_line":"                # the chance of orphaned ports in neutron. Bug 1423845."},{"line_number":2330,"context_line":"                deallocate_networks \u003d False if network_info else True"},{"line_number":2331,"context_line":"                try:"},{"line_number":2332,"context_line":"                    self._shutdown_instance(context, instance,"},{"line_number":2333,"context_line":"                            block_device_mapping, requested_networks,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_6300dd34","line":2330,"updated":"2015-03-12 21:59:13.000000000","message":"I really don\u0027t understand this.\n\nIf network_info is empty, you are setting deallocate_networks to True. How do you know the networks have been allocated in the first place? Since the call is asynchronous, and wait() returns None (when raise\u003dFalse), there\u0027s no way to know if there\u0027s anything to deallocate at all...","commit_id":"5f784067c03103c60b7d23a158c5971400666b0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7b76ea8775f876413ca564756dada23a575df112","unresolved":false,"context_lines":[{"line_number":2327,"context_line":"                # network allocation failure. Since nothing can be reused on"},{"line_number":2328,"context_line":"                # rescheduling it\u0027s better to deallocate network to eliminate"},{"line_number":2329,"context_line":"                # the chance of orphaned ports in neutron. Bug 1423845."},{"line_number":2330,"context_line":"                deallocate_networks \u003d False if network_info else True"},{"line_number":2331,"context_line":"                try:"},{"line_number":2332,"context_line":"                    self._shutdown_instance(context, instance,"},{"line_number":2333,"context_line":"                            block_device_mapping, requested_networks,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_b6298d8b","line":2330,"in_reply_to":"9a80dd14_157bc5f5","updated":"2015-03-17 13:53:44.000000000","message":"Couldn\u0027t we just call self._get_instance_nw_info and then check based on that?  Also, looks like you need to rebase since _get_instance_nw_info is not the latest.\n\nI\u0027m also not totally sure we have all cases covered in the now deprecated _build_instance error paths, we do call _deallocate_network in quite a few places there, but not all where we call \"network_info.wait(do_raise\u003dFalse)\".  But given that\u0027s a deprecated path I\u0027m not sure that it matters?  Probably need to ask alaski.","commit_id":"5f784067c03103c60b7d23a158c5971400666b0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"70349b4922c7a3198cbf4afe5259c564c494cfc7","unresolved":false,"context_lines":[{"line_number":2327,"context_line":"                # network allocation failure. Since nothing can be reused on"},{"line_number":2328,"context_line":"                # rescheduling it\u0027s better to deallocate network to eliminate"},{"line_number":2329,"context_line":"                # the chance of orphaned ports in neutron. Bug 1423845."},{"line_number":2330,"context_line":"                deallocate_networks \u003d False if network_info else True"},{"line_number":2331,"context_line":"                try:"},{"line_number":2332,"context_line":"                    self._shutdown_instance(context, instance,"},{"line_number":2333,"context_line":"                            block_device_mapping, requested_networks,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_0ea0f88e","line":2330,"in_reply_to":"9a80dd14_27c1764e","updated":"2015-03-19 20:17:16.000000000","message":"Andrew, the only thing I could think of for a safety net is this:\n\n\"We could catch NotFound in _try_deallocate_network and log a warning/ignore\"\n\nI don\u0027t really want to put a huge try/except in allocate_for_instance right now that keeps track of everything it created and makes sure to roll them back on failure (in this patch at least), basically like what a tempest test does - it adds an addCleanup to the stack for every resource it creates during the test run.\n\nOleg, the only other complaint I have with this change is the bug reference in the code comment, we should drop that.","commit_id":"5f784067c03103c60b7d23a158c5971400666b0c"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"93061315ddeb96958f92617498f9a7b773ac63f1","unresolved":false,"context_lines":[{"line_number":2327,"context_line":"                # network allocation failure. Since nothing can be reused on"},{"line_number":2328,"context_line":"                # rescheduling it\u0027s better to deallocate network to eliminate"},{"line_number":2329,"context_line":"                # the chance of orphaned ports in neutron. Bug 1423845."},{"line_number":2330,"context_line":"                deallocate_networks \u003d False if network_info else True"},{"line_number":2331,"context_line":"                try:"},{"line_number":2332,"context_line":"                    self._shutdown_instance(context, instance,"},{"line_number":2333,"context_line":"                            block_device_mapping, requested_networks,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_f20863db","line":2330,"in_reply_to":"9a80dd14_316da79a","updated":"2015-03-17 17:20:17.000000000","message":"The deprecated code path is a bit different because it always deallocates and reallocates networks on a reschedule.  So this fix isn\u0027t necessary there.\n\nThe issue I see with just using deallocate_networks is that there is no protection in Nova against an error if the network isn\u0027t found.  Neutron may not respond with an error in that case, but if another network system did it would fail the build.","commit_id":"5f784067c03103c60b7d23a158c5971400666b0c"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"4d01e21682b1f72d32ce224dff133771c892ced7","unresolved":false,"context_lines":[{"line_number":2327,"context_line":"                # network allocation failure. Since nothing can be reused on"},{"line_number":2328,"context_line":"                # rescheduling it\u0027s better to deallocate network to eliminate"},{"line_number":2329,"context_line":"                # the chance of orphaned ports in neutron. Bug 1423845."},{"line_number":2330,"context_line":"                deallocate_networks \u003d False if network_info else True"},{"line_number":2331,"context_line":"                try:"},{"line_number":2332,"context_line":"                    self._shutdown_instance(context, instance,"},{"line_number":2333,"context_line":"                            block_device_mapping, requested_networks,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_b2721448","line":2330,"in_reply_to":"9a80dd14_55dcb418","updated":"2015-03-19 08:46:10.000000000","message":"Agree with Matt here. If instance network (ports) is not found Neutron won\u0027t raise so I believe it should be safe.\n\n \u003e\u003e The other thing we could do is make sure that allocate_for_instance cleans up after itself on failure \n \u003e\u003e by rolling back any resources it created/associated to the instance.\n\nwe can though current fix seems more generic to me. Thoughts?","commit_id":"5f784067c03103c60b7d23a158c5971400666b0c"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"2ec12aed037beb0ea710a52fb942c50138edb388","unresolved":false,"context_lines":[{"line_number":2327,"context_line":"                # network allocation failure. Since nothing can be reused on"},{"line_number":2328,"context_line":"                # rescheduling it\u0027s better to deallocate network to eliminate"},{"line_number":2329,"context_line":"                # the chance of orphaned ports in neutron. Bug 1423845."},{"line_number":2330,"context_line":"                deallocate_networks \u003d False if network_info else True"},{"line_number":2331,"context_line":"                try:"},{"line_number":2332,"context_line":"                    self._shutdown_instance(context, instance,"},{"line_number":2333,"context_line":"                            block_device_mapping, requested_networks,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_157bc5f5","line":2330,"in_reply_to":"9a80dd14_6300dd34","updated":"2015-03-13 06:37:13.000000000","message":"So, at this point we don\u0027t know if ports we created or not, but we also can\u0027t say for sure that nothing was created in neutron. Since nothing can be reused after rescheduling next hypervisor will allocate net info from scratch - bug 1423845 is such a case where orphaned ports are left. It\u0027s better to try to deallocate all ports here to eliminate the chance of orphaned ports in neutron. It\u0027s a harmless call - if no vm ports found then nothing happens.","commit_id":"5f784067c03103c60b7d23a158c5971400666b0c"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"2516cdb76d1592d3f1f5badd53f0772b6ed89c99","unresolved":false,"context_lines":[{"line_number":2327,"context_line":"                # network allocation failure. Since nothing can be reused on"},{"line_number":2328,"context_line":"                # rescheduling it\u0027s better to deallocate network to eliminate"},{"line_number":2329,"context_line":"                # the chance of orphaned ports in neutron. Bug 1423845."},{"line_number":2330,"context_line":"                deallocate_networks \u003d False if network_info else True"},{"line_number":2331,"context_line":"                try:"},{"line_number":2332,"context_line":"                    self._shutdown_instance(context, instance,"},{"line_number":2333,"context_line":"                            block_device_mapping, requested_networks,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_27c1764e","line":2330,"in_reply_to":"9a80dd14_b2721448","updated":"2015-03-19 14:07:45.000000000","message":"I\u0027m okay with moving forward with _try_deallocate..., my point was just that despite nova-network and Neutron gracefully handling the NotFound case now, it\u0027s handled outside of Novas control and could change at any point.  I just think it would be nice to have our own safety net.  But as I said, this is fine for now.","commit_id":"5f784067c03103c60b7d23a158c5971400666b0c"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"26ec8992bab1376bcc2b398298d727555ffaf91d","unresolved":false,"context_lines":[{"line_number":2327,"context_line":"                # network allocation failure. Since nothing can be reused on"},{"line_number":2328,"context_line":"                # rescheduling it\u0027s better to deallocate network to eliminate"},{"line_number":2329,"context_line":"                # the chance of orphaned ports in neutron. Bug 1423845."},{"line_number":2330,"context_line":"                deallocate_networks \u003d False if network_info else True"},{"line_number":2331,"context_line":"                try:"},{"line_number":2332,"context_line":"                    self._shutdown_instance(context, instance,"},{"line_number":2333,"context_line":"                            block_device_mapping, requested_networks,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_316da79a","line":2330,"in_reply_to":"9a80dd14_b6298d8b","updated":"2015-03-17 14:16:11.000000000","message":"Why would we call self._get_instance_nw_info? It\u0027s just an extra call which is not needed: deallocate_networks() itself will look for corresponding ports in neutron (just like _get_instance_nw_info does) and delete them if exist or do nothing.\n\nRegarding covering all the cases - I\u0027m trying to fix a particular bug found in fields with this patch. I not sure if a requirement to cover all possible cases is fair, probably it should be a separate task.","commit_id":"5f784067c03103c60b7d23a158c5971400666b0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"02dd5ee54080c6f3aadfa52fb30edf714769bfe6","unresolved":false,"context_lines":[{"line_number":2327,"context_line":"                # network allocation failure. Since nothing can be reused on"},{"line_number":2328,"context_line":"                # rescheduling it\u0027s better to deallocate network to eliminate"},{"line_number":2329,"context_line":"                # the chance of orphaned ports in neutron. Bug 1423845."},{"line_number":2330,"context_line":"                deallocate_networks \u003d False if network_info else True"},{"line_number":2331,"context_line":"                try:"},{"line_number":2332,"context_line":"                    self._shutdown_instance(context, instance,"},{"line_number":2333,"context_line":"                            block_device_mapping, requested_networks,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_55dcb418","line":2330,"in_reply_to":"9a80dd14_f20863db","updated":"2015-03-17 19:10:02.000000000","message":"Hmm, _try_deallocate_network will set the instance vm_state to ERROR if there is a problem deallocating the network, so we wouldn\u0027t reschedule in that case anyway, right?  The instance is dead, @reverts_task_state should set task_state\u003dNone so the user can delete the instance and try again.\n\nWe could catch NotFound in _try_deallocate_network and log a warning/ignore....but that\u0027s a bit hacky.\n\nThe only other network API we have in tree is nova-network and at a quick glance that does a pretty decent job of handling NotFounds itself and gracefully working around those, i.e. if fixed_ips aren\u0027t found it doesn\u0027t try to deallocate them.\n\nIn the case of neutron, deallocate_for_instance gets all of the ports currently owned by the instance in neutron and filters out any that were passed in on the initial boot request (pre-existing ports in neutron we don\u0027t want to delete), and then tries to delete those ports that nova created - which wouldn\u0027t exist unless neutronv2 API created them on initial create since the device_id is associated.\n\nSo looking at the network APIs we have in tree and the code in them today, this seems safe enough and cleaning up after ourselves is the right thing to do.\n\nThe other thing we could do is make sure that allocate_for_instance cleans up after itself on failure by rolling back any resources it created/associated to the instance.","commit_id":"5f784067c03103c60b7d23a158c5971400666b0c"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"b3acdba75bc35bdb8d10b0e4f290ca7eba3cadbc","unresolved":false,"context_lines":[{"line_number":2413,"context_line":"                try:"},{"line_number":2414,"context_line":"                    self._shutdown_instance(context, instance,"},{"line_number":2415,"context_line":"                            block_device_mapping, requested_networks,"},{"line_number":2416,"context_line":"                            try_deallocate_networks\u003ddeallocate_networks)"},{"line_number":2417,"context_line":"                except Exception:"},{"line_number":2418,"context_line":"                    ctxt.reraise \u003d False"},{"line_number":2419,"context_line":"                    msg \u003d _(\u0027Could not clean up failed build,\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a6ced46_29a71751","line":2416,"updated":"2015-03-23 09:28:42.000000000","message":"according to the traceback from the LP issue (https://launchpadlibrarian.net/198249821/traceback.txt) I think this must be correct: ports are not deallocated only when get_instance_nw_info() fails and network_info.wait() won\u0027t raise due to do_raise\u003dFalse, so we need to handle port deallocation here","commit_id":"43dfc401abca3ffbd28ee7737fbe172b6ae1b439"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5dad541bbfea7523d0b96e5b914fd5776decef69","unresolved":false,"context_lines":[{"line_number":2820,"context_line":"                    side_effect\u003d[self.instance, self.instance]),"},{"line_number":2821,"context_line":"                mock.patch.object(self.compute,"},{"line_number":2822,"context_line":"                    \u0027_build_networks_for_instance\u0027,"},{"line_number":2823,"context_line":"                    return_value\u003dnetwork_model.NetworkInfo()),"},{"line_number":2824,"context_line":"                mock.patch.object(self.compute,"},{"line_number":2825,"context_line":"                    \u0027_notify_about_instance_usage\u0027),"},{"line_number":2826,"context_line":"                mock.patch.object(self.compute,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_2370a5db","line":2823,"updated":"2015-03-12 21:59:13.000000000","message":"But _build_networks_for_instance() does *not* return a network_moedl.NetworkInfo(). It returns a network_model.NetworkInfoAsyncWrapper() object... (i.e. self.network_info -- yes it\u0027s terribly convoluted test code...)","commit_id":"5f784067c03103c60b7d23a158c5971400666b0c"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"2ec12aed037beb0ea710a52fb942c50138edb388","unresolved":false,"context_lines":[{"line_number":2820,"context_line":"                    side_effect\u003d[self.instance, self.instance]),"},{"line_number":2821,"context_line":"                mock.patch.object(self.compute,"},{"line_number":2822,"context_line":"                    \u0027_build_networks_for_instance\u0027,"},{"line_number":2823,"context_line":"                    return_value\u003dnetwork_model.NetworkInfo()),"},{"line_number":2824,"context_line":"                mock.patch.object(self.compute,"},{"line_number":2825,"context_line":"                    \u0027_notify_about_instance_usage\u0027),"},{"line_number":2826,"context_line":"                mock.patch.object(self.compute,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_c81a746e","line":2823,"in_reply_to":"9a80dd14_2370a5db","updated":"2015-03-13 06:37:13.000000000","message":"Right, but I think these two were designed to de interchangeable [1], at least for tests. Also It\u0027s not the only place where network_model.NetworkInfo is used as return for _build_networks_for_instance. I believe it should not affects the test correctness. Am I missing something?\n\n[1] https://github.com/openstack/nova/blob/master/nova/network/model.py#L422-L427","commit_id":"5f784067c03103c60b7d23a158c5971400666b0c"}]}