Add RouterPort bindings for all HA ports

Adding these bindings allows us to remove the special-cased HA
interface deletion in l3_hamode_db that had the potential for
leaving orphaned ports on failures. With the bindings present,
the interface deletion is handled before the router delete so
a failure to delete the interface will prevent router deletion.

Related-Bug: #1540271
Change-Id: I2de8503742661c18a2ec2c5ade7ec58ea380e749

neutron/db/l3_hamode_db.py

@@ -588,13 +588,6 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin,
             if ha_network:
                 self._delete_vr_id_allocation(
                     context, ha_network, router_db.extra_attributes.ha_vr_id)
-                # NOTE(kevinbenton): normally the ha interfaces should have
-                # been automatically removed by the super delete_router call.
-                # However, that only applies to interfaces created after fix
-                # Ifd3e007aaf2a2ed8123275aa3a9f540838e3c003 which added the
-                # RouterPort relationship to ha interfaces. Legacy interfaces
-                # will be cleaned up by this.
-                self._delete_ha_interfaces(context, router_db.id)
 
                 # always attempt to cleanup the network as the router is
                 # deleted. the core plugin will stop us if its in use

neutron/db/migration/alembic_migrations/versions/CONTRACT_HEAD

@@ -1 +1 @@
-7d9d8eeec6ad
+a8b517cff8ab

neutron/db/migration/alembic_migrations/versions/newton/contract/a8b517cff8ab_add_routerport_bindings_for_ha.py

@@ -0,0 +1,67 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""Add routerport bindings for L3 HA
+
+Revision ID: a8b517cff8ab
+Revises: a8b517cff8ab
+Create Date: 2016-07-18 14:31:45.725516
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'a8b517cff8ab'
+down_revision = '7d9d8eeec6ad'
+
+from alembic import op
+import sqlalchemy as sa
+
+from neutron.common import constants
+
+
+HA_AGENT_BINDINGS = 'ha_router_agent_port_bindings'
+ROUTER_PORTS = 'routerports'
+
+
+def upgrade():
+    ha_bindings = sa.Table(
+        HA_AGENT_BINDINGS,
+        sa.MetaData(),
+        sa.Column('port_id', sa.String(36)),
+        sa.Column('router_id', sa.String(36)),
+        sa.Column('l3_agent_id', sa.String(36)),
+        sa.Column('state', sa.Enum(constants.HA_ROUTER_STATE_ACTIVE,
+                                   constants.HA_ROUTER_STATE_STANDBY,
+                                   name='l3_ha_states'))
+    )
+    router_ports = sa.Table(ROUTER_PORTS,
+                            sa.MetaData(),
+                            sa.Column('router_id', sa.String(36)),
+                            sa.Column('port_id', sa.String(36)),
+                            sa.Column('port_type', sa.String(255)))
+    session = sa.orm.Session(bind=op.get_bind())
+    with session.begin(subtransactions=True):
+        router_port_tuples = set()
+        for ha_bind in session.query(ha_bindings):
+            router_port_tuples.add((ha_bind.router_id, ha_bind.port_id))
+        # we have to remove any from the bulk insert that may already exist
+        # as a result of Ifd3e007aaf2a2ed8123275aa3a9f540838e3c003 being
+        # back-ported
+        for router_port in session.query(router_ports).filter(
+            router_ports.c.port_type == constants.DEVICE_OWNER_ROUTER_HA_INTF):
+                router_port_tuples.discard((router_port.router_id,
+                                            router_port.port_id))
+        new_records = [dict(router_id=router_id, port_id=port_id,
+                            port_type=constants.DEVICE_OWNER_ROUTER_HA_INTF)
+                       for router_id, port_id in router_port_tuples]
+    op.bulk_insert(router_ports, new_records)
+    session.commit()

neutron/tests/functional/db/migrations/test_a8b517cff8ab_add_routerport_bindings_for_ha.py

@@ -0,0 +1,102 @@
+# Copyright 2016 Business Cat is Very Serious
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+#
+
+from neutron_lib import constants
+from oslo_db.sqlalchemy import utils as db_utils
+from oslo_utils import uuidutils
+
+from neutron.tests.functional.db import test_migrations
+
+
+class HARouterPortMigrationMixin(object):
+    """Validates HA port to router port migration."""
+
+    def _create_so(self, o_type, values):
+        """create standard attr object."""
+        stan = db_utils.get_table(self.engine, 'standardattributes')
+        # find next available id taking into account existing records
+        rec_ids = [r.id for r in self.engine.execute(stan.select()).fetchall()]
+        next_id = max([0] + rec_ids) + 1
+        self.engine.execute(stan.insert().values({'id': next_id,
+                                                  'resource_type': o_type}))
+        values['standard_attr_id'] = next_id
+        return self._create_rec(o_type, values)
+
+    def _create_rec(self, o_type, values):
+        otable = db_utils.get_table(self.engine, o_type)
+        self.engine.execute(otable.insert().values(values))
+
+    def _make_router_agents_and_ports(self, router_id, network_id,
+                                      add_binding):
+        self._create_so('routers', {'id': router_id})
+        # each router gets a couple of agents
+        for _ in range(2):
+            port_id = uuidutils.generate_uuid()
+            self._create_so('ports', {'id': port_id, 'network_id': network_id,
+                                      'mac_address': port_id[0:31],
+                                      'admin_state_up': True,
+                                      'device_id': router_id,
+                                      'device_owner': 'network',
+                                      'status': 'ACTIVE'})
+            agent_id = uuidutils.generate_uuid()
+            timestamp = '2000-04-06T14:34:23'
+            self._create_rec('agents', {'id': agent_id, 'topic': 'x',
+                                        'agent_type': 'L3',
+                                        'binary': 'x',
+                                        'host': agent_id,
+                                        'created_at': timestamp,
+                                        'started_at': timestamp,
+                                        'heartbeat_timestamp': timestamp,
+                                        'configurations': ''})
+            self._create_rec('ha_router_agent_port_bindings',
+                             {'port_id': port_id, 'router_id': router_id,
+                              'l3_agent_id': agent_id})
+            if add_binding:
+                ptype = constants.DEVICE_OWNER_ROUTER_HA_INTF
+                self._create_rec('routerports',
+                                 {'router_id': router_id, 'port_id': port_id,
+                                  'port_type': ptype})
+
+    def _create_ha_routers_with_ports(self, engine):
+        network_id = uuidutils.generate_uuid()
+        self._create_so('networks', {'id': network_id})
+        unpatched_router_ids = [uuidutils.generate_uuid() for i in range(10)]
+        for rid in unpatched_router_ids:
+            self._make_router_agents_and_ports(rid, network_id, False)
+        # make half of the routers already have routerport bindings to simulate
+        # a back-port of Ifd3e007aaf2a2ed8123275aa3a9f540838e3c003
+        patched_router_ids = [uuidutils.generate_uuid() for i in range(10)]
+        for rid in patched_router_ids:
+            self._make_router_agents_and_ports(rid, network_id, True)
+
+    def _pre_upgrade_a8b517cff8ab(self, engine):
+        self._create_ha_routers_with_ports(engine)
+        return True  # return True so check function is invoked after migrate
+
+    def _check_a8b517cff8ab(self, engine, data):
+        rp = db_utils.get_table(engine, 'routerports')
+        # just ensuring the correct count of routerport records is enough.
+        # 20 routers * 2 ports per router
+        self.assertEqual(40, len(engine.execute(rp.select()).fetchall()))
+
+
+class TestHARouterPortMigrationMysql(HARouterPortMigrationMixin,
+                                     test_migrations.TestWalkMigrationsMysql):
+    pass
+
+
+class TestHARouterPortMigrationPsql(HARouterPortMigrationMixin,
+                                    test_migrations.TestWalkMigrationsPsql):
+    pass

neutron/tests/unit/db/test_l3_hamode_db.py

@@ -843,15 +843,6 @@ class L3HATestCase(L3HATestFramework):
                           self.plugin.get_number_of_agents_for_scheduling,
                           self.admin_ctx)
 
-    def test_ha_ports_deleted_in_parent_router_removal(self):
-        router1 = self._create_router()
-        # router cleanup should no longer depend on this function for
-        # newly created routers.
-        self.plugin._delete_ha_interfaces = mock.Mock()
-        self.plugin.delete_router(self.admin_ctx, router1['id'])
-        self.assertEqual([], self.core_plugin.get_ports(
-              self.admin_ctx, filters={'device_id': [router1['id']]}))
-
     def test_ha_network_deleted_if_no_ha_router_present_two_tenants(self):
         # Create two routers in different tenants.
         router1 = self._create_router()