Fix bad config defaults

* Remove strings used for testing from config.yaml defaults
* neutron-* and nova-* config options are no longer compulsary. These
  are only needed for automatic generation of records when guests are
  booted or floating-ips associated. Manual management of records is
  a valid option.
* Add assess status to catch the situation where neutron-* or nova-*
  variables have been set but nameservers has not. In thios scenraio
  the domains cannot be created as there is no server defined.
* Extend amulet tests to create a server as well as domains
* 'domains' no longer resolves in sink file regex and has been
  replaces with zones
* Add missing relation for auto-generated nova records
* Rename dns-server-record config option to nameservers which is more
  consistant with upstream docs. Unused old 'nameservers' property
  needed to be removed to allow this

Change-Id: Ibd58aa8ffb3931a2fb359fad6292a9d33775a0f8

src/config.yaml

@@ -8,30 +8,33 @@ options:
       only be used if DNS servers are outside of Juju control. Using the
       designate-bind charm is the prefered approach.
   nova-domain:
-    default: 'www.example.com.'
+    default:
     type: string
     description: Domain to add records for new instances to
   nova-domain-email:
-    default: 'email@example.com'
+    default:
     type: string
     description: Email address of the person responsible for the domain.
-  dns-server-record:
+  nameservers:
-    default: 'ns1.www.example.com.'
+    default:
     type: string
-    description: DNS server record
+    description: |
+      Space delimited list of nameservers. These are the nameservers that have
+      been provided to the domain registrar in order to delegate the domain to
+      Designate. e.g. "ns1.example.com ns2.example.com"
   neutron-domain:
-    default: 'www.bob.com.'
+    default:
     type: string
     description: Domain to add  floating ip records to
   neutron-domain-email:
-    default: 'email@example.com'
+    default:
     type: string
     description: Email address of the person responsible for the domain.
   neutron-record-format:
-    default: '%(octet0)s-%(octet1)s-%(octet2)s-%(octet3)s.%(domain)s'
+    default: '%(octet0)s-%(octet1)s-%(octet2)s-%(octet3)s.%(zone)s'
     type: string
     description: Format of floating ip global records
   nova-record-format:
-    default: '%(hostname)s.%(tenant_id)s.%(domain)s'
+    default: '%(hostname)s.%(tenant_id)s.%(zone)s'
     type: string
     description: Format of floating ip global records

src/lib/charm/openstack/designate.py

@@ -198,16 +198,16 @@ class BindRNDCRelationAdapter(openstack_adapters.OpenStackRelationAdapter):
 
     @property
     def slave_ips(self):
-        '''List of DNS slave address infoprmation
+        """List of DNS slave address infoprmation
 
         @returns: list [{'unit': unitname, 'address': 'address'},
                         ...]
-        '''
+        """
         return self.relation.slave_ips()
 
     @property
     def pool_config(self):
-        '''List of DNS slave information from Juju attached DNS slaves
+        """List of DNS slave information from Juju attached DNS slaves
 
         Creates a dict for each backends and returns a list of those dicts.
         The designate config file has a section per backend. The template uses
@@ -217,7 +217,7 @@ class BindRNDCRelationAdapter(openstack_adapters.OpenStackRelationAdapter):
         @returns: list [{'nameserver': name, 'pool_target': name,
                          'address': slave_ip_addr},
                         ...]
-        '''
+        """
         pconfig = []
         for slave in self.slave_ips:
             unit_name = slave['unit'].replace('/', '_').replace('-', '_')
@@ -228,38 +228,30 @@ class BindRNDCRelationAdapter(openstack_adapters.OpenStackRelationAdapter):
             })
         return pconfig
 
-    @property
-    def nameservers(self):
-        '''List of nameserver section names
-
-        @returns: str Comma delimited list of nameserver section names
-        '''
-        return ', '.join([s['nameserver'] for s in self.pool_config])
-
     @property
     def pool_targets(self):
-        '''List of pool_target section names
+        """List of pool_target section names
 
         @returns: str Comma delimited list of pool_target section names
-        '''
+        """
         return ', '.join([s['pool_target'] for s in self.pool_config])
 
     @property
     def slave_addresses(self):
-        '''List of slave IP addresses
+        """List of slave IP addresses
 
         @returns: str Comma delimited list of slave IP addresses
-        '''
+        """
         return ', '.join(['{}:53'.format(s['address'])
                          for s in self.pool_config])
 
     @property
     def rndc_info(self):
-        '''Rndc key and algorith in formation.
+        """Rndc key and algorith in formation.
 
         @returns: dict {'algorithm': rndc_algorithm,
                         'secret': rndc_secret_digest}
-        '''
+        """
         return self.relation.rndc_info
 
 
@@ -273,7 +265,7 @@ class DesignateConfigurationAdapter(
 
     @property
     def pool_config(self):
-        '''List of DNS slave information from user defined config
+        """List of DNS slave information from user defined config
 
         Creates a dict for each backends and returns a list of those dicts.
         The designate config file has a section per backend. The template uses
@@ -285,7 +277,7 @@ class DesignateConfigurationAdapter(
                          'address': slave_ip_addr,
                          'rndc_key_file': rndc_key_file},
                         ...]
-        '''
+        """
         pconfig = []
         for entry in self.dns_slaves.split():
             address, port, key = entry.split(':')
@@ -299,28 +291,20 @@ class DesignateConfigurationAdapter(
             })
         return pconfig
 
-    @property
-    def nameservers(self):
-        '''List of nameserver section names
-
-        @returns: str Comma delimited list of nameserver section names
-        '''
-        return ', '.join([s['nameserver'] for s in self.pool_config])
-
     @property
     def pool_targets(self):
-        '''List of pool_target section names
+        """List of pool_target section names
 
         @returns: str Comma delimited list of pool_target section names
-        '''
+        """
         return ', '.join([s['pool_target'] for s in self.pool_config])
 
     @property
     def slave_addresses(self):
-        '''List of slave IP addresses
+        """List of slave IP addresses
 
         @returns: str Comma delimited list of slave IP addresses
-        '''
+        """
         return ', '.join(['{}:53'.format(s['address'])
                          for s in self.pool_config])
 
@@ -332,7 +316,9 @@ class DesignateConfigurationAdapter(
         @returns nova domain id
         """
         domain = hookenv.config('nova-domain')
-        return DesignateCharm.get_domain_id(domain)
+        if domain:
+            return DesignateCharm.get_domain_id(domain)
+        return None
 
     @property
     def neutron_domain_id(self):
@@ -342,7 +328,9 @@ class DesignateConfigurationAdapter(
         @returns neutron domain id
         """
         domain = hookenv.config('neutron-domain')
-        return DesignateCharm.get_domain_id(domain)
+        if domain:
+            return DesignateCharm.get_domain_id(domain)
+        return None
 
     @property
     def nova_conf_args(self):
@@ -539,20 +527,23 @@ class DesignateCharm(openstack_charm.HAOpenStackCharm):
         subprocess.check_call(create_cmd)
 
     def domain_init_done(self):
+        """Query leader db to see if domain creation is donei
+
+        @returns boolean"""
         return hookenv.leader_get(attribute='domain-init-done')
 
     @classmethod
-    def ensure_api_responding(self):
+    @decorators.retry_on_exception(
+        10, base_delay=5, exc_type=subprocess.CalledProcessError)
+    def ensure_api_responding(cls):
+        """Check that the api service is responding.
 
-        @decorators.retry_on_exception(
+        The retry_on_exception decorator will cause this method to be called
-            30, base_delay=5, exc_type=subprocess.CalledProcessError)
+        until it succeeds or retry limit is exceeded"""
-        def check_designate_api():
+        hookenv.log('Checking API service is responding',
-            print("Checking API service is responding")
+                    level=hookenv.WARNING)
-            check_cmd = ['reactive/designate_utils.py', 'server-list']
+        check_cmd = ['reactive/designate_utils.py', 'server-list']
-            subprocess.check_call(check_cmd)
+        subprocess.check_call(check_cmd)
-
-        check_designate_api()
-        return True
 
     @classmethod
     def create_initial_servers_and_domains(cls):
@@ -562,13 +553,21 @@ class DesignateCharm(openstack_charm.HAOpenStackCharm):
         @returns None
         """
         if hookenv.is_leader() and cls.ensure_api_responding():
-            cls.create_server(hookenv.config('dns-server-record'))
+            if hookenv.config('nameservers'):
-            cls.create_domain(
+                for ns in hookenv.config('nameservers').split():
-                hookenv.config('nova-domain'),
+                    cls.create_server(ns)
-                hookenv.config('nova-domain-email'))
+            else:
-            cls.create_domain(
+                hookenv.log('No nameserver specified, skipping creation of'
-                hookenv.config('neutron-domain'),
+                            'nova and neutron domains', level=hookenv.WARNING)
-                hookenv.config('neutron-domain-email'))
+                return
+            if hookenv.config('nova-domain'):
+                cls.create_domain(
+                    hookenv.config('nova-domain'),
+                    hookenv.config('nova-domain-email'))
+            if hookenv.config('neutron-domain'):
+                cls.create_domain(
+                    hookenv.config('neutron-domain'),
+                    hookenv.config('neutron-domain-email'))
             hookenv.leader_set({'domain-init-done': True})
 
     def update_pools(self):
@@ -577,3 +576,11 @@ class DesignateCharm(openstack_charm.HAOpenStackCharm):
         if hookenv.is_leader():
             cmd = ['designate-manage', 'pool', 'update']
             subprocess.check_call(cmd)
+
+    def custom_assess_status_check(self):
+        if (not hookenv.config('nameservers') and
+                (hookenv.config('nova-domain') or
+                 hookenv.config('neutron-domain'))):
+            return 'blocked', ('nameservers must be set when specifying'
+                               ' nova-domain or neutron-domain')
+        return None, None

src/reactive/designate_handlers.py

@@ -90,6 +90,7 @@ def create_servers_and_domains(*args):
     designate.create_initial_servers_and_domains()
     if designate.domain_init_done():
         reactive.set_state('domains.created')
+        designate.render_full_config(args)
 
 
 @reactive.when('cluster.available')
@@ -99,7 +100,7 @@ def update_peers(cluster):
 
 
 @reactive.when('cluster.available')
-@reactive.when('domains.created')
+@reactive.when('db.synched')
 @reactive.when(*COMPLETE_INTERFACE_STATES)
 def render_all_configs(*args):
     '''Write out all designate config include bootstrap domain info'''
@@ -108,7 +109,7 @@ def render_all_configs(*args):
 
 
 @reactive.when_not('cluster.available')
-@reactive.when('domains.created')
+@reactive.when('db.synched')
 @reactive.when(*COMPLETE_INTERFACE_STATES)
 def render_all_configs_single_node(*args):
     '''Write out all designate config include bootstrap domain info'''

src/templates/mitaka/designate.conf

@@ -364,17 +364,6 @@ connection = {{ shared_db.designate_uri }}
 #-----------------------
 #format = '%(hostname)s.%(domain)s'
 
-#------------------------
-# Neutron Floating Handler
-#------------------------
-[handler:neutron_floatingip]
-# Domain ID of domain to create records in. Should be pre-created
-domain_id = a2cd66c5-a1ec-47ae-b0dc-718ebb024a45
-notification_topics = notifications_designate
-control_exchange = 'neutron'
-format = '%(octet0)s-%(octet1)s-%(octet2)s-%(octet3)s.%(domain)s'
-#format = '%(hostname)s.%(domain)s'
-
 #############################
 ## Agent Backend Configuration
 #############################

src/templates/mitaka/neutron_sink.cfg

@@ -1,5 +1,7 @@
+{% if options.neutron_domain_id %}
 [handler:neutron_floatingip]
-domain_id = {{ options.neutron_domain_id }}
+zone_id = {{ options.neutron_domain_id }}
 notification_topics = notifications_designate
 control_exchange = 'neutron'
 format = '{{ options.neutron_record_format }}'
+{% endif %}

src/templates/mitaka/nova_sink.cfg

@@ -1,5 +1,7 @@
+{% if options.nova_domain_id %}
 [handler:nova_fixed]
-domain_id = {{ options.nova_domain_id }}
+zone_id = {{ options.nova_domain_id }}
 notification_topics = notifications_designate
 control_exchange = 'nova'
 format = '{{ options.nova_record_format }}'
+{% endif %}

src/templates/mitaka/pools.yaml

@@ -2,9 +2,11 @@
   name: default
   description: Pool genergated by Juju
 
+{% if options.dns_server_record %}
   ns_records:
     - hostname: {{ options.dns_server_record }}
       priority: 10
+{% endif %}
 
   nameservers:
 {% if dns_backend.pool_config %}

src/tests/basic_deployment.py

@@ -22,6 +22,7 @@ import time
 import designateclient.client as designate_client
 import designateclient.v1.domains as domains
 import designateclient.v1.records as records
+import designateclient.v1.servers as servers
 
 import charmhelpers.contrib.openstack.amulet.deployment as amulet_deployment
 import charmhelpers.contrib.openstack.amulet.utils as os_amulet_utils
@@ -36,6 +37,8 @@ class DesignateBasicDeployment(amulet_deployment.OpenStackAmuletDeployment):
     TEST_DOMAIN = 'amuletexample.com.'
     TEST_WWW_RECORD = "www.{}".format(TEST_DOMAIN)
     TEST_RECORD = {TEST_WWW_RECORD: '10.0.0.23'}
+    TEST_NS1_RECORD = 'ns1.amuletexample.com.'
+    TEST_NS2_RECORD = 'ns2.amuletexample.com.'
 
     def __init__(self, series, openstack=None, source=None, stable=False):
         """Deploy the entire test environment."""
@@ -348,11 +351,47 @@ class DesignateBasicDeployment(amulet_deployment.OpenStackAmuletDeployment):
             message = u.relation_error('designate dns-backend', ret)
             amulet.raise_status(amulet.FAIL, msg=message)
 
+    def get_server_id(self, server_name):
+        server_id = None
+        for server in self.designate.servers.list():
+            if server.name == server_name:
+                server_id = server.id
+                break
+        return server_id
+
+    def get_test_server_id(self):
+        return self.get_server_id(self.TEST_NS2_RECORD)
+
+    def check_test_server_gone(self):
+        return not self.get_test_server_id()
+
+    def test_400_server_creation(self):
+        """Simple api calls to create domain"""
+        # Designate does not allow the last server to be delete so ensure ns1
+        # always present
+        if not self.get_server_id(self.TEST_NS1_RECORD):
+            server = servers.Server(name=self.TEST_NS1_RECORD)
+            new_server = self.designate.servers.create(server)
+
+        u.log.debug('Checking if server exists before trying to create it')
+        old_server_id = self.get_test_server_id()
+        if old_server_id:
+            u.log.debug('Deleting old server')
+            self.designate.servers.delete(old_server_id)
+        self.check_and_wait(
+            self.check_test_server_gone,
+            desc='Waiting for server to disappear')
+        u.log.debug('Creating new server')
+        server = servers.Server(name=self.TEST_NS2_RECORD)
+        new_server = self.designate.servers.create(server)
+        assert(new_server is not None)
+
     def get_domain_id(self, domain_name):
         domain_id = None
         for dom in self.designate.domains.list():
             if dom.name == domain_name:
                 domain_id = dom.id
+                break
         return domain_id
 
     def get_test_domain_id(self):
@@ -368,7 +407,7 @@ class DesignateBasicDeployment(amulet_deployment.OpenStackAmuletDeployment):
         cmd_out = subprocess.check_output(lookup_cmd).rstrip('\r\n')
         return self.TEST_RECORD[self.TEST_WWW_RECORD] == cmd_out
 
-    def test_400_domain_creation(self):
+    def test_410_domain_creation(self):
         """Simple api calls to create domain"""
         u.log.debug('Checking if domain exists before trying to create it')
         old_dom_id = self.get_test_domain_id()

unit_tests/test_designate_handlers.py

@@ -106,12 +106,12 @@ class TestDesignateHandlers(unittest.TestCase):
             ],
             'render_all_configs': [
                 all_interfaces,
-                ('domains.created', ),
+                ('db.synched', ),
                 ('cluster.available', ),
             ],
             'render_all_configs_single_node': [
                 all_interfaces,
-                ('domains.created', ),
+                ('db.synched', ),
             ],
             'run_db_migration': [
                 all_interfaces,
@@ -211,6 +211,7 @@ class TestDesignateHandlers(unittest.TestCase):
         self.patch(handlers.reactive, 'set_state')
         self.patch(handlers.designate, 'create_initial_servers_and_domains')
         self.patch(handlers.designate, 'domain_init_done')
+        self.patch(handlers.designate, 'render_full_config')
         self.domain_init_done.return_value = False
         handlers.create_servers_and_domains('arg1', 'arg2')
         self.create_initial_servers_and_domains.assert_called_once_with()
@@ -220,6 +221,7 @@ class TestDesignateHandlers(unittest.TestCase):
         handlers.create_servers_and_domains('arg1', 'arg2')
         self.create_initial_servers_and_domains.assert_called_once_with()
         self.set_state.assert_called_once_with('domains.created')
+        self.render_full_config.assert_called_once_with(('arg1', 'arg2'))
 
     def test_update_peers(self):
         cluster = mock.MagicMock()

unit_tests/test_lib_charm_openstack_designate.py

@@ -193,9 +193,6 @@ class TestBindRNDCRelationAdapter(Helper):
                        'nameserver': 'nameserver_unit_2',
                        'pool_target': 'nameserver_unit_2'}]
             self.assertEqual(a.pool_config, expect)
-            self.assertEqual(
-                a.nameservers,
-                'nameserver_unit_1, nameserver_unit_2')
             self.assertEqual(
                 a.pool_targets,
                 'nameserver_unit_1, nameserver_unit_2')
@@ -230,7 +227,6 @@ class TestDesignateConfigurationAdapter(Helper):
                        'pool_target': 'nameserver_ip2',
                        'rndc_key_file': '/etc/designate/rndc_ip2.key'}]
             self.assertEqual(a.pool_config, expect)
-            self.assertEqual(a.nameservers, 'nameserver_ip1, nameserver_ip2')
             self.assertEqual(a.pool_targets, 'nameserver_ip1, nameserver_ip2')
             self.assertEqual(a.slave_addresses, 'ip1:53, ip2:53')
 
@@ -408,7 +404,7 @@ class TestDesignateCharm(Helper):
 
     def test_create_initial_servers_and_domains(self):
         test_config = {
-            'dns-server-record': 'dnsserverrec1',
+            'nameservers': 'dnsserverrec1',
             'nova-domain': 'novadomain',
             'nova-domain-email': 'novaemail',
             'neutron-domain': 'neutrondomain',