Merge "use postgresql INET datatype for storing IPs"

4 files changed, 238 insertions, 36 deletions

diff --git a/nova/db/sqlalchemy/migrate_repo/versions/149_inet_datatype_for_postgres.py b/nova/db/sqlalchemy/migrate_repo/versions/149_inet_datatype_for_postgres.py
new file mode 100644
index 000000000..fe9889e35
--- /dev/null
+++ b/nova/db/sqlalchemy/migrate_repo/versions/149_inet_datatype_for_postgres.py
@@ -0,0 +1,70 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2012 OpenStack LLC.
+# All Rights Reserved.
+#
+#    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 sqlalchemy import MetaData, String, Table
+from sqlalchemy.dialects import postgresql
+
+
+TABLE_COLUMNS = [
+    # table name, column name
+    ('instances', 'access_ip_v4'),
+    ('instances', 'access_ip_v6'),
+    ('security_group_rules', 'cidr'),
+    ('provider_fw_rules', 'cidr'),
+    ('networks', 'cidr'),
+    ('networks', 'cidr_v6'),
+    ('networks', 'gateway'),
+    ('networks', 'gateway_v6'),
+    ('networks', 'netmask'),
+    ('networks', 'netmask_v6'),
+    ('networks', 'broadcast'),
+    ('networks', 'dns1'),
+    ('networks', 'dns2'),
+    ('networks', 'vpn_public_address'),
+    ('networks', 'vpn_private_address'),
+    ('networks', 'dhcp_start'),
+    ('fixed_ips', 'address'),
+    ('floating_ips', 'address'),
+    ('console_pools', 'address')]
+
+
+def upgrade(migrate_engine):
+    """Convert String columns holding IP addresses to INET for postgresql."""
+    meta = MetaData()
+    meta.bind = migrate_engine
+    dialect = migrate_engine.url.get_dialect()
+    if dialect is postgresql.dialect:
+        for table, column in TABLE_COLUMNS:
+            # can't use migrate's alter() because it does not support
+            # explicit casting
+            migrate_engine.execute(
+                "ALTER TABLE %(table)s "
+                "ALTER COLUMN %(column)s TYPE INET USING %(column)s::INET"
+                % locals())
+    else:
+        for table, column in TABLE_COLUMNS:
+            t = Table(table, meta, autoload=True)
+            getattr(t.c, column).alter(type=String(39))
+
+
+def downgrade(migrate_engine):
+    """Convert columns back to the larger String(255)."""
+    meta = MetaData()
+    meta.bind = migrate_engine
+    for table, column in TABLE_COLUMNS:
+        t = Table(table, meta, autoload=True)
+        getattr(t.c, column).alter(type=String(255))
diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index 52985a3eb..56a4d944a 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -27,6 +27,7 @@ from sqlalchemy import ForeignKey, DateTime, Boolean, Text, Float
 from sqlalchemy.orm import relationship, backref, object_mapper
 
 from nova.db.sqlalchemy.session import get_session
+from nova.db.sqlalchemy.types import IPAddress
 from nova.openstack.common import cfg
 from nova.openstack.common import timeutils
 
@@ -290,8 +291,8 @@ class Instance(BASE, NovaBase):
 
     # User editable field meant to represent what ip should be used
     # to connect to the instance
-    access_ip_v4 = Column(String(255))
-    access_ip_v6 = Column(String(255))
+    access_ip_v4 = Column(IPAddress())
+    access_ip_v6 = Column(IPAddress())
 
     auto_disk_config = Column(Boolean())
     progress = Column(Integer)
@@ -592,7 +593,7 @@ class SecurityGroupIngressRule(BASE, NovaBase):
     protocol = Column(String(5))  # "tcp", "udp", or "icmp"
     from_port = Column(Integer)
     to_port = Column(Integer)
-    cidr = Column(String(255))
+    cidr = Column(IPAddress())
 
     # Note: This is not the parent SecurityGroup. It's SecurityGroup we're
     # granting access for.
@@ -612,7 +613,7 @@ class ProviderFirewallRule(BASE, NovaBase):
     protocol = Column(String(5))  # "tcp", "udp", or "icmp"
     from_port = Column(Integer)
     to_port = Column(Integer)
-    cidr = Column(String(255))
+    cidr = Column(IPAddress())
 
 
 class KeyPair(BASE, NovaBase):
@@ -662,25 +663,25 @@ class Network(BASE, NovaBase):
     label = Column(String(255))
 
     injected = Column(Boolean, default=False)
-    cidr = Column(String(255), unique=True)
-    cidr_v6 = Column(String(255), unique=True)
+    cidr = Column(IPAddress(), unique=True)
+    cidr_v6 = Column(IPAddress(), unique=True)
     multi_host = Column(Boolean, default=False)
 
-    gateway_v6 = Column(String(255))
-    netmask_v6 = Column(String(255))
-    netmask = Column(String(255))
+    gateway_v6 = Column(IPAddress())
+    netmask_v6 = Column(IPAddress())
+    netmask = Column(IPAddress())
     bridge = Column(String(255))
     bridge_interface = Column(String(255))
-    gateway = Column(String(255))
-    broadcast = Column(String(255))
-    dns1 = Column(String(255))
-    dns2 = Column(String(255))
+    gateway = Column(IPAddress())
+    broadcast = Column(IPAddress())
+    dns1 = Column(IPAddress())
+    dns2 = Column(IPAddress())
 
     vlan = Column(Integer)
-    vpn_public_address = Column(String(255))
+    vpn_public_address = Column(IPAddress())
     vpn_public_port = Column(Integer)
-    vpn_private_address = Column(String(255))
-    dhcp_start = Column(String(255))
+    vpn_private_address = Column(IPAddress())
+    dhcp_start = Column(IPAddress())
 
     rxtx_base = Column(Integer)
 
@@ -705,7 +706,7 @@ class FixedIp(BASE, NovaBase):
     """Represents a fixed ip for an instance."""
     __tablename__ = 'fixed_ips'
     id = Column(Integer, primary_key=True)
-    address = Column(String(255))
+    address = Column(IPAddress())
     network_id = Column(Integer, nullable=True)
     virtual_interface_id = Column(Integer, nullable=True)
     instance_uuid = Column(String(36), nullable=True)
@@ -722,7 +723,7 @@ class FloatingIp(BASE, NovaBase):
     """Represents a floating ip that dynamically forwards to a fixed ip."""
     __tablename__ = 'floating_ips'
     id = Column(Integer, primary_key=True)
-    address = Column(String(255))
+    address = Column(IPAddress())
     fixed_ip_id = Column(Integer, nullable=True)
     project_id = Column(String(255))
     host = Column(String(255))  # , ForeignKey('hosts.id'))
@@ -744,7 +745,7 @@ class ConsolePool(BASE, NovaBase):
     """Represents pool of consoles on the same physical node."""
     __tablename__ = 'console_pools'
     id = Column(Integer, primary_key=True)
-    address = Column(String(255))
+    address = Column(IPAddress())
     username = Column(String(255))
     password = Column(String(255))
     console_type = Column(String(255))
diff --git a/nova/db/sqlalchemy/types.py b/nova/db/sqlalchemy/types.py
new file mode 100644
index 000000000..275e61a4c
--- /dev/null
+++ b/nova/db/sqlalchemy/types.py
@@ -0,0 +1,26 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 OpenStack LLC.
+# All Rights Reserved.
+#
+#    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.
+
+"""Custom SQLAlchemy types."""
+
+from sqlalchemy.dialects import postgresql
+from sqlalchemy import String
+
+
+def IPAddress():
+    """An SQLAlchemy type representing an IP-address."""
+    return String(39).with_variant(postgresql.INET(), 'postgresql')
diff --git a/nova/tests/test_migrations.py b/nova/tests/test_migrations.py
index 750326592..abd04a641 100644
--- a/nova/tests/test_migrations.py
+++ b/nova/tests/test_migrations.py
@@ -42,37 +42,48 @@ from nova import test
 LOG = logging.getLogger(__name__)
 
 
-def _mysql_get_connect_string(user="openstack_citest",
-                              passwd="openstack_citest",
-                              database="openstack_citest"):
+def _get_connect_string(backend,
+                        user="openstack_citest",
+                        passwd="openstack_citest",
+                        database="openstack_citest"):
     """
     Try to get a connection with a very specfic set of values, if we get
-    these then we'll run the mysql tests, otherwise they are skipped
+    these then we'll run the tests, otherwise they are skipped
     """
-    return "mysql://%(user)s:%(passwd)s@localhost/%(database)s" % locals()
+    if backend == "postgres":
+        backend = "postgresql+psycopg2"
 
+    return ("%(backend)s://%(user)s:%(passwd)s@localhost/%(database)s"
+            % locals())
 
-def _is_mysql_avail(user="openstack_citest",
-                    passwd="openstack_citest",
-                    database="openstack_citest"):
+
+def _is_backend_avail(backend,
+                      user="openstack_citest",
+                      passwd="openstack_citest",
+                      database="openstack_citest"):
     try:
-        connect_uri = _mysql_get_connect_string(
-            user=user, passwd=passwd, database=database)
+        if backend == "mysql":
+            connect_uri = _get_connect_string("mysql",
+                user=user, passwd=passwd, database=database)
+        elif backend == "postgres":
+            connect_uri = _get_connect_string("postgres",
+                user=user, passwd=passwd, database=database)
         engine = sqlalchemy.create_engine(connect_uri)
         connection = engine.connect()
     except Exception:
         # intentionally catch all to handle exceptions even if we don't
-        # have mysql code loaded at all.
+        # have any backend code loaded.
         return False
     else:
         connection.close()
+        engine.dispose()
         return True
 
 
 def _have_mysql():
     present = os.environ.get('NOVA_TEST_MYSQL_PRESENT')
     if present is None:
-        return _is_mysql_avail()
+        return _is_backend_avail('mysql')
     return present.lower() in ('', 'true')
 
 
@@ -121,7 +132,6 @@ class TestMigrations(test.TestCase):
         self._reset_databases()
 
     def tearDown(self):
-
         # We destroy the test data store between each test case,
         # and recreate it, which ensures that we have no side-effects
         # from the tests
@@ -142,6 +152,7 @@ class TestMigrations(test.TestCase):
         for key, engine in self.engines.items():
             conn_string = self.test_databases[key]
             conn_pieces = urlparse.urlparse(conn_string)
+            engine.dispose()
             if conn_string.startswith('sqlite'):
                 # We can just delete the SQLite database, which is
                 # the easiest and cleanest solution
@@ -172,6 +183,7 @@ class TestMigrations(test.TestCase):
                 database = conn_pieces.path.strip('/')
                 loc_pieces = conn_pieces.netloc.split('@')
                 host = loc_pieces[1]
+
                 auth_pieces = loc_pieces[0].split(':')
                 user = auth_pieces[0]
                 password = ""
@@ -207,16 +219,16 @@ class TestMigrations(test.TestCase):
         Test that we can trigger a mysql connection failure and we fail
         gracefully to ensure we don't break people without mysql
         """
-        if _is_mysql_avail(user="openstack_cifail"):
+        if _is_backend_avail('mysql', user="openstack_cifail"):
             self.fail("Shouldn't have connected")
 
     def test_mysql_innodb(self):
         # Test that table creation on mysql only builds InnoDB tables
-        if not _have_mysql():
+        if not _is_backend_avail('mysql'):
             self.skipTest("mysql not available")
         # add this to the global lists to make reset work with it, it's removed
         # automatically in tearDown so no need to clean it up here.
-        connect_string = _mysql_get_connect_string()
+        connect_string = _get_connect_string("mysql")
         engine = sqlalchemy.create_engine(connect_string)
         self.engines["mysqlcitest"] = engine
         self.test_databases["mysqlcitest"] = connect_string
@@ -225,7 +237,7 @@ class TestMigrations(test.TestCase):
         self._reset_databases()
         self._walk_versions(engine, False, False)
 
-        uri = _mysql_get_connect_string(database="information_schema")
+        uri = _get_connect_string("mysql", database="information_schema")
         connection = sqlalchemy.create_engine(uri).connect()
 
         # sanity check
@@ -242,6 +254,99 @@ class TestMigrations(test.TestCase):
         count = noninnodb.scalar()
         self.assertEqual(count, 0, "%d non InnoDB tables created" % count)
 
+    def test_migration_149_postgres(self):
+        """Test updating a table with IPAddress columns."""
+        if not _is_backend_avail('postgres'):
+            self.skipTest("postgres not available")
+
+        connect_string = _get_connect_string("postgres")
+        engine = sqlalchemy.create_engine(connect_string)
+
+        self.engines["postgrescitest"] = engine
+        self.test_databases["postgrescitest"] = connect_string
+
+        self._reset_databases()
+        migration_api.version_control(engine, TestMigrations.REPOSITORY,
+                                      migration.INIT_VERSION)
+
+        connection = engine.connect()
+
+        self._migrate_up(engine, 148)
+        IPS = ("127.0.0.1", "255.255.255.255", "2001:db8::1:2", "::1")
+        connection.execute("INSERT INTO provider_fw_rules "
+                           "            (protocol, from_port, to_port, cidr)"
+                           "VALUES ('tcp', 1234, 1234, '%s'), "
+                           "       ('tcp', 1234, 1234, '%s'), "
+                           "       ('tcp', 1234, 1234, '%s'), "
+                           "       ('tcp', 1234, 1234, '%s')" % IPS)
+        self.assertEqual('character varying',
+                         connection.execute(
+                "SELECT data_type FROM INFORMATION_SCHEMA.COLUMNS "
+                "WHERE table_name='provider_fw_rules' "
+                "AND table_catalog='openstack_citest' "
+                "AND column_name='cidr'").scalar())
+
+        self._migrate_up(engine, 149)
+        self.assertEqual(IPS,
+                         tuple(tup[0] for tup in connection.execute(
+                    "SELECT cidr from provider_fw_rules").fetchall()))
+        self.assertEqual('inet',
+                         connection.execute(
+                "SELECT data_type FROM INFORMATION_SCHEMA.COLUMNS "
+                "WHERE table_name='provider_fw_rules' "
+                "AND table_catalog='openstack_citest' "
+                "AND column_name='cidr'").scalar())
+        connection.close()
+
+    def test_migration_149_mysql(self):
+        """Test updating a table with IPAddress columns."""
+        if not _have_mysql():
+            self.skipTest("mysql not available")
+
+        connect_string = _get_connect_string("mysql")
+        engine = sqlalchemy.create_engine(connect_string)
+        self.engines["mysqlcitest"] = engine
+        self.test_databases["mysqlcitest"] = connect_string
+
+        self._reset_databases()
+        migration_api.version_control(engine, TestMigrations.REPOSITORY,
+                                      migration.INIT_VERSION)
+
+        uri = _get_connect_string("mysql", database="openstack_citest")
+        connection = sqlalchemy.create_engine(uri).connect()
+
+        self._migrate_up(engine, 148)
+
+        IPS = ("127.0.0.1", "255.255.255.255", "2001:db8::1:2", "::1")
+        connection.execute("INSERT INTO provider_fw_rules "
+                           "            (protocol, from_port, to_port, cidr)"
+                           "VALUES ('tcp', 1234, 1234, '%s'), "
+                           "       ('tcp', 1234, 1234, '%s'), "
+                           "       ('tcp', 1234, 1234, '%s'), "
+                           "       ('tcp', 1234, 1234, '%s')" % IPS)
+        self.assertEqual('varchar(255)',
+                         connection.execute(
+                "SELECT column_type FROM INFORMATION_SCHEMA.COLUMNS "
+                "WHERE table_name='provider_fw_rules' "
+                "AND table_schema='openstack_citest' "
+                "AND column_name='cidr'").scalar())
+
+        connection.close()
+
+        self._migrate_up(engine, 149)
+
+        connection = sqlalchemy.create_engine(uri).connect()
+
+        self.assertEqual(IPS,
+                         tuple(tup[0] for tup in connection.execute(
+                    "SELECT cidr from provider_fw_rules").fetchall()))
+        self.assertEqual('varchar(39)',
+                         connection.execute(
+                "SELECT column_type FROM INFORMATION_SCHEMA.COLUMNS "
+                "WHERE table_name='provider_fw_rules' "
+                "AND table_schema='openstack_citest' "
+                "AND column_name='cidr'").scalar())
+
     def _walk_versions(self, engine=None, snake_walk=False, downgrade=True):
         # Determine latest version script from the repo, then
         # upgrade from 1 through to the latest, with no data