Changing DNS to pass string to driver

This commit also moves the "get_ip_address" functions out of
instance.views and into the models code under the SimpleInstance class,
where it's renamed get_visible_ip_addresses().
Also, the content field (the ip address) is now passed to the driver
instead of set on the Entry object by the manager, which allows more
flexibility for dealing with various drivers and will hopefully prevent
issues in the future.
Finally, integration tests were added to keep this from breaking
in the future.

Closes-Bug: 1273446

Change-Id: I70bf37838cc5cecfe579fe6001df79d7f6f5d53e
This commit is contained in:
Tim Simpson 2014-01-27 15:54:18 -06:00
parent c14553393f
commit 3cdb9ea817
15 changed files with 289 additions and 91 deletions

View File

@ -12,7 +12,8 @@
"trove_version":"v1.0",
"trove_api_updated":"2012-08-01T00:00:00Z",
"trove_dns_support":false,
"trove_dns_support":true,
"trove_dns_checker":"trove.tests.fakes.dns.FakeDnsChecker",
"trove_ip_support":false,
"nova_client": null,

View File

@ -9,6 +9,12 @@ remote_cinder_client = trove.tests.fakes.nova.fake_create_cinder_client
# Fake out the RPC implementation
rpc_backend = trove.common.rpc.impl_fake
# Fake out DNS.
trove_dns_support = True
dns_driver = trove.tests.fakes.dns.FakeDnsDriver
dns_instance_entry_factory = trove.tests.fakes.dns.FakeDnsInstanceEntryFactory
# This will remove some of the verbose logging when trying to diagnose tox issues
default_log_levels=routes.middleware=ERROR,trove.common.auth=WARN

View File

@ -75,7 +75,7 @@ class DesignateDriver(driver.DnsDriver):
self.default_dns_zone = DesignateDnsZone(id=DNS_DOMAIN_ID,
name=DNS_DOMAIN_NAME)
def create_entry(self, entry):
def create_entry(self, entry, content):
"""Creates the entry in the driver at the given dns zone."""
dns_zone = entry.dns_zone or self.default_dns_zone
if not dns_zone.id:
@ -86,7 +86,7 @@ class DesignateDriver(driver.DnsDriver):
# Record name has to end with a '.' by dns standard
record = Record(name=entry.name + '.',
type=entry.type,
data=entry.content,
data=content,
ttl=entry.ttl,
priority=entry.priority)
client.records.create(dns_zone.id, record)
@ -137,10 +137,10 @@ class DesignateDriver(driver.DnsDriver):
class DesignateInstanceEntryFactory(driver.DnsInstanceEntryFactory):
"""Defines how instance DNS entries are created for instances."""
def create_entry(self, instance):
def create_entry(self, instance_id):
zone = DesignateDnsZone(id=DNS_DOMAIN_ID, name=DNS_DOMAIN_NAME)
# Constructing the hostname by hashing the instance ID.
name = base64.b32encode(hashlib.md5(instance).digest())[:11]
name = base64.b32encode(hashlib.md5(instance_id).digest())[:11]
hostname = ("%s.%s" % (name, zone.name))
#Removing the leading dot if present
if hostname.endswith('.'):

View File

@ -52,9 +52,8 @@ class DnsManager(object):
"""
entry = self.entry_factory.create_entry(instance_id)
if entry:
entry.content = content
LOG.debug("Creating entry address %s." % str(entry))
self.driver.create_entry(entry)
self.driver.create_entry(entry, content)
else:
LOG.debug("Entry address not found for instance %s" % instance_id)

View File

@ -117,7 +117,7 @@ class RsDnsDriver(object):
msg = "TTL value '--dns_ttl=%s' should be greater than 300"
raise Exception(msg % DNS_TTL)
def create_entry(self, entry):
def create_entry(self, entry, content):
dns_zone = entry.dns_zone or self.default_dns_zone
if dns_zone.id is None:
raise TypeError("The entry's dns_zone must have an ID specified.")
@ -127,7 +127,7 @@ class RsDnsDriver(object):
future = self.dns_client.records.create(
domain=dns_zone.id,
record_name=name,
record_data=entry.content,
record_data=content,
record_type=entry.type,
record_ttl=entry.ttl)
try:

View File

@ -17,6 +17,7 @@
"""Model classes that form the core of instances functionality."""
import re
from datetime import datetime
from novaclient import exceptions as nova_exceptions
from trove.common import cfg
@ -44,6 +45,11 @@ CONF = cfg.CONF
LOG = logging.getLogger(__name__)
def filter_ips(ips, regex):
"""Filter out IPs not matching regex."""
return [ip for ip in ips if re.search(regex, ip)]
def load_server(context, instance_id, server_id):
"""Loads a server or raises an exception."""
client = create_nova_client(context)
@ -116,26 +122,45 @@ class SimpleInstance(object):
"""
def __init__(self, context, db_info, service_status, root_password=None):
def __init__(self, context, db_info, service_status, root_password=None,
ds_version=None, ds=None):
self.context = context
self.db_info = db_info
self.service_status = service_status
self.root_pass = root_password
self.ds_version = (datastore_models.DatastoreVersion.
load_by_uuid(self.db_info.datastore_version_id))
self.ds = (datastore_models.Datastore.
load(self.ds_version.datastore_id))
if ds_version is None:
self.ds_version = (datastore_models.DatastoreVersion.
load_by_uuid(self.db_info.datastore_version_id))
if ds is None:
self.ds = (datastore_models.Datastore.
load(self.ds_version.datastore_id))
@property
def addresses(self):
#TODO(tim.simpson): Review whether we should keep this... its a mess.
#TODO(tim.simpson): This code attaches two parts of the Nova server to
# db_info: "status" and "addresses". The idea
# originally was to listen to events to update this
# data and store it in the Trove database.
# However, it may have been unwise as a year and a
# half later we still have to load the server anyway
# and this makes the code confusing.
if hasattr(self.db_info, 'addresses'):
return self.db_info.addresses
else:
return None
@property
def created(self):
return self.db_info.created
@property
def dns_ip_address(self):
"""Returns the IP address to be used with DNS."""
ips = self.get_visible_ip_addresses()
if ips is None or len(ips) < 1:
return None
return ips[0]
@property
def flavor_id(self):
# Flavor ID is a str in the 1.0 API.
@ -145,6 +170,21 @@ class SimpleInstance(object):
def hostname(self):
return self.db_info.hostname
def get_visible_ip_addresses(self):
"""Returns IPs that will be visible to the user."""
if self.addresses is None:
return None
IPs = []
for label in self.addresses:
if (re.search(CONF.network_label_regex, label) and
len(self.addresses[label]) > 0):
IPs.extend([addr.get('addr')
for addr in self.addresses[label]])
# Includes ip addresses that match the regexp pattern
if CONF.ip_regex:
IPs = filter_ips(IPs, CONF.ip_regex)
return IPs
@property
def id(self):
return self.db_info.id

View File

@ -15,35 +15,15 @@
# License for the specific language governing permissions and limitations
# under the License.
import re
from trove.openstack.common import log as logging
from trove.common import cfg
from trove.common.views import create_links
from trove.instance import models
LOG = logging.getLogger(__name__)
CONF = cfg.CONF
def get_ip_address(addresses):
if addresses is None:
return None
IPs = []
for label in addresses:
if (re.search(CONF.network_label_regex, label) and
len(addresses[label]) > 0):
IPs.extend([addr.get('addr') for addr in addresses[label]])
# Includes ip addresses that match the regexp pattern
if CONF.ip_regex:
IPs = filter_ips(IPs, CONF.ip_regex)
return IPs
def filter_ips(ips, regex):
return [ip for ip in ips if re.search(regex, ip)]
class InstanceView(object):
"""Uses a SimpleInstance."""
@ -98,7 +78,7 @@ class InstanceDetailView(InstanceView):
if self.instance.hostname:
result['instance']['hostname'] = self.instance.hostname
else:
ip = get_ip_address(self.instance.addresses)
ip = self.instance.get_visible_ip_addresses()
if ip is not None and len(ip) > 0:
result['instance']['ip'] = ip

View File

@ -43,7 +43,6 @@ from trove.instance.models import FreshInstance
from trove.instance.tasks import InstanceTasks
from trove.instance.models import InstanceStatus
from trove.instance.models import InstanceServiceStatus
from trove.instance.views import get_ip_address
from trove.openstack.common import log as logging
from trove.openstack.common.gettextutils import _
from trove.openstack.common.notifier import api as notifier
@ -586,11 +585,12 @@ class FreshInstanceTasks(FreshInstance, NotifyMixin, ConfigurationMixin):
sleep_time=1, time_out=DNS_TIME_OUT)
server = self.nova_client.servers.get(
self.db_info.compute_instance_id)
self.db_info.addresses = server.addresses
LOG.info(_("Creating dns entry..."))
ip = get_ip_address(server.addresses)
ip = self.dns_ip_address
if not ip:
raise TroveError('Error creating DNS. No IP available.')
dns_client.create_instance_entry(self.id, ip.pop)
dns_client.create_instance_entry(self.id, ip)
else:
LOG.debug(_("%(gt)s: DNS not enabled for instance: %(id)s") %
{'gt': greenthread.getcurrent(), 'id': self.id})

View File

@ -12,7 +12,6 @@
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import hashlib
import os
import re
@ -53,6 +52,7 @@ from trove import tests
from trove.tests.config import CONFIG
from trove.tests.util import create_dbaas_client
from trove.tests.util.usage import create_usage_verifier
from trove.tests.util import dns_checker
from trove.tests.util import iso_time
from trove.tests.util.users import Requirements
from trove.common.utils import poll_until
@ -114,7 +114,10 @@ class InstanceTestInfo(object):
def get_address(self):
result = self.dbaas_admin.mgmt.instances.show(self.id)
return result.ip[0]
if not hasattr(result, 'hostname'):
return result.ip[0]
else:
return result.server['addresses']
def get_local_id(self):
mgmt_instance = self.dbaas_admin.management.show(self.id)
@ -811,6 +814,19 @@ class TestGuestProcess(object):
diagnostic_tests_helper(diagnostics)
@test(depends_on_classes=[WaitForGuestInstallationToFinish],
groups=[GROUP, GROUP_TEST, "dbaas.dns"])
class DnsTests(object):
@test
def test_dns_entries_are_found(self):
"""Talk to DNS system to ensure entries were created."""
print("Instance name=%s" % instance_info.name)
client = instance_info.dbaas_admin
mgmt_instance = client.mgmt.instances.show(instance_info.id)
dns_checker(mgmt_instance)
@test(depends_on_classes=[WaitForGuestInstallationToFinish],
groups=[GROUP, GROUP_TEST, "dbaas.guest.start.test"])
class TestAfterInstanceCreatedGuestData(object):
@ -887,14 +903,6 @@ class TestInstanceListing(object):
check.links(instance_dict['links'])
check.used_volume()
@test(enabled=CONFIG.trove_dns_support)
def test_instance_hostname(self):
instance = dbaas.instances.get(instance_info.id)
assert_equal(200, dbaas.last_http_code)
hostname_prefix = ("%s" % (hashlib.sha1(instance.id).hexdigest()))
instance_hostname_prefix = instance.hostname.split('.')[0]
assert_equal(hostname_prefix, instance_hostname_prefix)
@test
def test_get_instance_status(self):
result = dbaas.instances.get(instance_info.id)

View File

@ -18,11 +18,13 @@
import time
from proboscis import after_class
from proboscis import before_class
from proboscis import test
from proboscis import asserts
from proboscis.decorators import time_out
from trove.common import cfg
from troveclient.compat import exceptions
from trove.tests.util import create_dbaas_client
from trove.common.utils import poll_until
@ -31,6 +33,8 @@ from trove.tests.util.users import Requirements
from trove.tests.api.instances import instance_info
from trove.tests.api.instances import VOLUME_SUPPORT
CONF = cfg.CONF
class TestBase(object):
@ -51,12 +55,12 @@ class TestBase(object):
def wait_for_instance_status(self, instance_id, status="ACTIVE"):
poll_until(lambda: self.dbaas.instances.get(instance_id),
lambda instance: instance.status == status,
time_out=10)
time_out=3, sleep_time=1)
def wait_for_instance_task_status(self, instance_id, description):
poll_until(lambda: self.dbaas.management.show(instance_id),
lambda instance: instance.task_description == description,
time_out=10)
time_out=3, sleep_time=1)
def is_instance_deleted(self, instance_id):
while True:
@ -83,7 +87,7 @@ class TestBase(object):
self.delete_instance(instance_id)
@test(runs_after_groups=["services.initialize"],
@test(runs_after_groups=["services.initialize", "dbaas.guest.shutdown"],
groups=['dbaas.api.instances.delete'])
class ErroredInstanceDelete(TestBase):
"""
@ -92,8 +96,12 @@ class ErroredInstanceDelete(TestBase):
"""
@before_class
def set_up(self):
def set_up_err(self):
"""Create some flawed instances."""
from trove.taskmanager.models import CONF
self.old_dns_support = CONF.trove_dns_support
CONF.trove_dns_support = False
super(ErroredInstanceDelete, self).set_up()
# Create an instance that fails during server prov.
self.server_error = self.create_instance('test_SERVER_ERROR')
@ -108,6 +116,11 @@ class ErroredInstanceDelete(TestBase):
# Create an instance that fails while it's been deleted the first time.
self.delete_error = self.create_instance('test_ERROR_ON_DELETE')
@after_class(always_run=True)
def clean_up(self):
from trove.taskmanager.models import CONF
CONF.trove_dns_support = self.old_dns_support
@test
@time_out(30)
def delete_server_error(self):

View File

@ -180,11 +180,16 @@ class AllAccounts(object):
@test(groups=["fake.%s.broken" % GROUP],
depends_on_groups=["services.initialize"])
depends_on_groups=["services.initialize"],
runs_after_groups=["dbaas.guest.shutdown"])
class AccountWithBrokenInstance(object):
@before_class
def setUp(self):
def setUpACCR(self):
from trove.taskmanager.models import CONF
self.old_dns_support = CONF.trove_dns_support
CONF.trove_dns_support = False
self.user = test_config.users.find_user(Requirements(is_admin=True))
self.client = create_dbaas_client(self.user)
self.name = 'test_SERVER_ERROR'
@ -224,3 +229,8 @@ class AccountWithBrokenInstance(object):
@after_class
def tear_down(self):
self.client.instances.delete(self.response.id)
@after_class
def restore_dns(self):
from trove.taskmanager.models import CONF
CONF.trove_dns_support = self.old_dns_support

85
trove/tests/fakes/dns.py Normal file
View File

@ -0,0 +1,85 @@
# Copyright 2014 Rackspace
#
# 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 trove.dns import driver
from proboscis.asserts import fail
from proboscis.asserts import assert_equal
from proboscis.asserts import assert_true
from trove.openstack.common import log as logging
LOG = logging.getLogger(__name__)
ENTRIES = {}
class FakeDnsDriver(driver.DnsDriver):
def create_entry(self, entry, content):
"""Pretend to create a DNS entry somewhere.
Since nothing else tests that this works, there's nothing more to do
here.
"""
entry.content = content
assert_true(entry.name not in ENTRIES)
LOG.debug("Adding fake DNS entry for hostname %s." % entry.name)
ENTRIES[entry.name] = entry
def delete_entry(self, name, type, dns_zone=None):
LOG.debug("Deleting fake DNS entry for hostname %s" % name)
ENTRIES.pop(name, None)
class FakeDnsInstanceEntryFactory(driver.DnsInstanceEntryFactory):
def create_entry(self, instance_id):
# Construct hostname using pig-latin.
hostname = "%s-lay" % instance_id
LOG.debug("Mapping instance_id %s to hostname %s"
% (instance_id, hostname))
return driver.DnsEntry(name=hostname, content=None,
type="A", ttl=42, dns_zone=None)
class FakeDnsChecker(object):
"""Used by tests to make sure a DNS record was written in fake mode."""
def __call__(self, mgmt_instance):
"""
Given an instance ID and ip address, confirm that the proper DNS
record was stored in Designate or some other DNS system.
"""
entry = FakeDnsInstanceEntryFactory().create_entry(mgmt_instance.id)
# Confirm DNS entry shown to user is what we expect.
assert_equal(entry.name, mgmt_instance.hostname)
hostname = entry.name
for i in ENTRIES:
print(i)
print("\t%s" % ENTRIES[i])
assert_true(hostname in ENTRIES,
"Hostname %s not found in DNS entries!" % hostname)
entry = ENTRIES[hostname]
# See if the ip address assigned to the record is what we expect.
# This isn't perfect, but for Fake Mode its good enough. If we
# really want to know exactly what it should be then we should restore
# the ability to return the IP from the API as well as a hostname,
# since that lines up to the DnsEntry's content field.
ip_addresses = mgmt_instance.server['addresses']
for network_name, ip_list in ip_addresses.items():
for ip in ip_list:
if entry.content == ip['addr']:
return
fail("Couldn't find IP address %s among these values: %s"
% (entry.content, ip_addresses))

View File

@ -0,0 +1,77 @@
# Copyright 2014 Rackspace Hosting
# Copyright 2014 Hewlett-Packard Development Company, L.P.
# 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 mock import Mock
from testtools import TestCase
from trove.common import cfg
from trove.instance.models import filter_ips
from trove.instance.models import DBInstance
from trove.instance.models import SimpleInstance
from trove.instance.tasks import InstanceTasks
CONF = cfg.CONF
class SimpleInstanceTest(TestCase):
def setUp(self):
super(SimpleInstanceTest, self).setUp()
db_info = DBInstance(InstanceTasks.BUILDING, name="TestInstance")
self.instance = SimpleInstance(None, db_info, "BUILD",
ds_version=Mock(), ds=Mock())
db_info.addresses = {"private": [{"addr": "123.123.123.123"}],
"internal": [{"addr": "10.123.123.123"}],
"public": [{"addr": "15.123.123.123"}]}
self.orig_conf = CONF.network_label_regex
self.orig_ip_regex = CONF.ip_regex
def tearDown(self):
super(SimpleInstanceTest, self).tearDown()
CONF.network_label_regex = self.orig_conf
CONF.ip_start = None
CONF.ip_regex = self.orig_ip_regex
def test_filter_ips(self):
CONF.network_label_regex = '.*'
CONF.ip_regex = '^(15.|123.)'
ip = self.instance.get_visible_ip_addresses()
ip = filter_ips(ip, CONF.ip_regex)
self.assertTrue(len(ip) == 2)
self.assertTrue('123.123.123.123' in ip)
self.assertTrue('15.123.123.123' in ip)
def test_one_network_label_exact(self):
CONF.network_label_regex = '^internal$'
ip = self.instance.get_visible_ip_addresses()
self.assertEqual(['10.123.123.123'], ip)
def test_one_network_label(self):
CONF.network_label_regex = 'public'
ip = self.instance.get_visible_ip_addresses()
self.assertEqual(['15.123.123.123'], ip)
def test_two_network_labels(self):
CONF.network_label_regex = '^(private|public)$'
ip = self.instance.get_visible_ip_addresses()
self.assertTrue(len(ip) == 2)
self.assertTrue('123.123.123.123' in ip)
self.assertTrue('15.123.123.123' in ip)
def test_all_network_labels(self):
CONF.network_label_regex = '.*'
ip = self.instance.get_visible_ip_addresses()
self.assertTrue(len(ip) == 3)
self.assertTrue('10.123.123.123' in ip)
self.assertTrue('123.123.123.123' in ip)
self.assertTrue('15.123.123.123' in ip)

View File

@ -16,8 +16,6 @@
from mock import Mock
from testtools import TestCase
from trove.common import cfg
from trove.instance.views import get_ip_address
from trove.instance.views import filter_ips
from trove.instance.views import InstanceView
from trove.instance.views import InstanceDetailView
@ -38,40 +36,6 @@ class InstanceViewsTest(TestCase):
CONF.network_label_regex = self.orig_conf
CONF.ip_start = None
def test_one_network_label_exact(self):
CONF.network_label_regex = '^internal$'
ip = get_ip_address(self.addresses)
self.assertEqual(['10.123.123.123'], ip)
def test_one_network_label(self):
CONF.network_label_regex = 'public'
ip = get_ip_address(self.addresses)
self.assertEqual(['15.123.123.123'], ip)
def test_two_network_labels(self):
CONF.network_label_regex = '^(private|public)$'
ip = get_ip_address(self.addresses)
self.assertTrue(len(ip) == 2)
self.assertTrue('123.123.123.123' in ip)
self.assertTrue('15.123.123.123' in ip)
def test_all_network_labels(self):
CONF.network_label_regex = '.*'
ip = get_ip_address(self.addresses)
self.assertTrue(len(ip) == 3)
self.assertTrue('10.123.123.123' in ip)
self.assertTrue('123.123.123.123' in ip)
self.assertTrue('15.123.123.123' in ip)
def test_filter_ips(self):
CONF.network_label_regex = '.*'
CONF.ip_regex = '^(15.|123.)'
ip = get_ip_address(self.addresses)
ip = filter_ips(ip, CONF.ip_regex)
self.assertTrue(len(ip) == 2)
self.assertTrue('123.123.123.123' in ip)
self.assertTrue('15.123.123.123' in ip)
class InstanceDetailViewTest(TestCase):
@ -91,6 +55,7 @@ class InstanceDetailViewTest(TestCase):
self.instance.addresses = {"private": [{"addr": self.ip}]}
self.instance.volume_used = '3'
self.instance.root_password = 'iloveyou'
self.instance.get_visible_ip_addresses = lambda: ["1.2.3.4"]
def tearDown(self):
super(InstanceDetailViewTest, self).tearDown()

View File

@ -177,6 +177,20 @@ def create_nova_client(user, service_type=None):
return TestClient(openstack)
def dns_checker(mgmt_instance):
"""Given a MGMT instance, ensures DNS provisioning worked.
Uses a helper class which, given a mgmt instance (returned by the mgmt
API) can confirm that the DNS record provisioned correctly.
"""
skip_if_xml() # The mgmt instance won't look the same, so skip this.
if CONFIG.values.get('trove_dns_checker') is not None:
checker = import_class(CONFIG.trove_dns_checker)
checker()(mgmt_instance)
else:
raise SkipTest("Can't access DNS system to check if DNS provisioned.")
def process(cmd):
process = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE,
stderr=subprocess.PIPE)