From ce76dbab021a7137e2afef370476b755446117fa Mon Sep 17 00:00:00 2001
From: asarfaty <asarfaty@vmware.com>
Date: Wed, 13 Nov 2019 11:21:49 +0200
Subject: [PATCH] NSX|V: Fix error handling for distributed router interface

1. No need to rollback the interface creation in teh distributed router
driver. It is rolled back on the plugin level. the Double rollback causes
a new error to be raised.
2. In the plugin level - do not alert on the rollback faliure.
It may be legit
2. In the plugin level raise a proper error to neutron, instead of the
internal one.

Change-Id: I129f595d6cd17cd0af62fc9e2855451b97e73ff0
---
 .../drivers/distributed_router_driver.py      | 56 ++++++++-----------
 vmware_nsx/plugins/nsx_v/plugin.py            | 17 ++++--
 2 files changed, 34 insertions(+), 39 deletions(-)

diff --git a/vmware_nsx/plugins/nsx_v/drivers/distributed_router_driver.py b/vmware_nsx/plugins/nsx_v/drivers/distributed_router_driver.py
index a15a1ed794..bd2ebc98cf 100644
--- a/vmware_nsx/plugins/nsx_v/drivers/distributed_router_driver.py
+++ b/vmware_nsx/plugins/nsx_v/drivers/distributed_router_driver.py
@@ -14,7 +14,6 @@
 
 import netaddr
 from oslo_log import log as logging
-from oslo_utils import excutils
 
 from neutron.db import l3_db
 
@@ -265,41 +264,32 @@ class RouterDistributedDriver(router_driver.RouterBaseDriver):
         address_groups = self.plugin._get_address_groups(
             context, router_id, network_id)
         edge_id = self._get_edge_id(context, router_id)
-        interface_created = False
-        try:
-            with locking.LockManager.get_lock(str(edge_id)):
-                edge_utils.add_vdr_internal_interface(self.nsx_v, context,
-                                                      router_id, network_id,
-                                                      address_groups,
-                                                      router_db.admin_state_up)
-                interface_created = True
-                # Update edge's firewall rules to accept subnets flows.
-                self.plugin._update_subnets_and_dnat_firewall(context,
-                                                              router_db)
+        with locking.LockManager.get_lock(str(edge_id)):
+            edge_utils.add_vdr_internal_interface(self.nsx_v, context,
+                                                  router_id, network_id,
+                                                  address_groups,
+                                                  router_db.admin_state_up)
+            # Update edge's firewall rules to accept subnets flows.
+            self.plugin._update_subnets_and_dnat_firewall(context,
+                                                          router_db)
 
-                if router_db.gw_port:
-                    plr_id = self.edge_manager.get_plr_by_tlr_id(context,
-                                                                 router_id)
-                    if router_db.enable_snat:
-                        self.plugin._update_nat_rules(context,
-                                                      router_db, plr_id)
+            if router_db.gw_port:
+                plr_id = self.edge_manager.get_plr_by_tlr_id(context,
+                                                             router_id)
+                if router_db.enable_snat:
+                    self.plugin._update_nat_rules(context,
+                                                  router_db, plr_id)
 
-                    # Open firewall flows on plr
-                    self.plugin._update_subnets_and_dnat_firewall(
-                        context, router_db, router_id=plr_id)
-                    # Update static routes of plr
-                    nexthop = self.plugin._get_external_attachment_info(
-                        context, router_db)[2]
+                # Open firewall flows on plr
+                self.plugin._update_subnets_and_dnat_firewall(
+                    context, router_db, router_id=plr_id)
+                # Update static routes of plr
+                nexthop = self.plugin._get_external_attachment_info(
+                    context, router_db)[2]
 
-                    self._update_routes(context, router_id,
-                                        nexthop)
-
-        except Exception:
-            with excutils.save_and_reraise_exception():
-                if not interface_created:
-                    super(nsx_v.NsxVPluginV2,
-                          self.plugin).remove_router_interface(
-                              context, router_id, interface_info)
+                self._update_routes(context, router_id,
+                                    nexthop)
+        # In case of failure, rollbak will be done in teh plugin level
         return info
 
     def remove_router_interface(self, context, router_id, interface_info):
diff --git a/vmware_nsx/plugins/nsx_v/plugin.py b/vmware_nsx/plugins/nsx_v/plugin.py
index 0b518ff3ef..887efbc560 100644
--- a/vmware_nsx/plugins/nsx_v/plugin.py
+++ b/vmware_nsx/plugins/nsx_v/plugin.py
@@ -4098,14 +4098,19 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
         try:
             return router_driver.add_router_interface(
                 context, router_id, interface_info)
-        except vsh_exc.VcnsApiException:
-            with excutils.save_and_reraise_exception():
-                LOG.error("Failed to add interface_info %(info)s on "
-                          "router %(router_id)s",
-                          {'info': str(interface_info),
-                           'router_id': router_id})
+        except vsh_exc.VcnsApiException as e:
+            LOG.error("Failed to add interface_info %(info)s on "
+                      "router %(router_id)s: %(e)s",
+                      {'info': str(interface_info),
+                       'router_id': router_id,
+                       'e': e.message})
+            try:
                 router_driver.remove_router_interface(
                     context, router_id, interface_info)
+            except Exception:
+                # Rollback may fail if creation failed too early
+                pass
+            raise nsx_exc.NsxPluginException(err_msg=e.message)
 
     def remove_router_interface(self, context, router_id, interface_info):
         # Get the router interface port id