Merge pull request #667 from rreichel3/rreichel3/azure-dns-zones-improvement

Improve checking for and creating Azure DNS Zones

octodns/provider/azuredns.py

@@ -7,7 +7,6 @@ from __future__ import absolute_import, division, print_function, \
 
 from azure.common.credentials import ServicePrincipalCredentials
 from azure.mgmt.dns import DnsManagementClient
-from msrestazure.azure_exceptions import CloudError
 
 from azure.mgmt.dns.models import ARecord, AaaaRecord, CaaRecord, \
     CnameRecord, MxRecord, SrvRecord, NsRecord, PtrRecord, TxtRecord, Zone
@@ -341,7 +340,7 @@ class AzureProvider(BaseProvider):
         self.log.debug('azure_zones: loading')
         list_zones = self._dns_client.zones.list_by_resource_group
         for zone in list_zones(self._resource_group):
-            self._azure_zones.add(zone.name)
+            self._azure_zones.add(zone.name.rstrip('.'))
 
     def _check_zone(self, name, create=False):
         '''Checks whether a zone specified in a source exist in Azure server.
@@ -356,29 +355,20 @@ class AzureProvider(BaseProvider):
 
             :type return: str or None
         '''
-        self.log.debug('_check_zone: name=%s', name)
-        try:
-            if name in self._azure_zones:
-                return name
-            self._dns_client.zones.get(self._resource_group, name)
+        self.log.debug('_check_zone: name=%s create=%s', name, create)
+        # Check if the zone already exists in our set
+        if name in self._azure_zones:
+            return name
+        # If not, and its time to create, lets do it.
+        if create:
+            self.log.debug('_check_zone:no matching zone; creating %s', name)
+            create_zone = self._dns_client.zones.create_or_update
+            create_zone(self._resource_group, name, Zone(location='global'))
             self._azure_zones.add(name)
             return name
-        except CloudError as err:
-            msg = 'The Resource \'Microsoft.Network/dnszones/{}\''.format(name)
-            msg += ' under resource group \'{}\''.format(self._resource_group)
-            msg += ' was not found.'
-            if msg == err.message:
-                # Then the only error is that the zone doesn't currently exist
-                if create:
-                    self.log.debug('_check_zone:no matching zone; creating %s',
-                                   name)
-                    create_zone = self._dns_client.zones.create_or_update
-                    create_zone(self._resource_group, name,
-                                Zone(location='global'))
-                    return name
-                else:
-                    return
-            raise
+        else:
+            # Else return nothing (aka false)
+            return
 
     def populate(self, zone, target=False, lenient=False):
         '''Required function of manager.py to collect records from zone.

tests/test_octodns_provider_azuredns.py

@@ -498,32 +498,44 @@ class TestAzureDnsProvider(TestCase):
         record_list = provider._dns_client.record_sets.list_by_dns_zone
         record_list.return_value = rs
 
+        zone_list = provider._dns_client.zones.list_by_resource_group
+        zone_list.return_value = [zone]
+
         exists = provider.populate(zone)
-        self.assertTrue(exists)
+
         self.assertEquals(len(zone.records), 17)
+        self.assertTrue(exists)
 
     def test_populate_zone(self):
         provider = self._get_provider()
 
         zone_list = provider._dns_client.zones.list_by_resource_group
-        zone_list.return_value = [AzureZone(location='global'),
-                                  AzureZone(location='global')]
+        zone_1 = AzureZone(location='global')
+        # This is far from ideal but the
+        # zone constructor doesn't let me set it on creation
+        zone_1.name = "zone-1"
+        zone_2 = AzureZone(location='global')
+        # This is far from ideal but the
+        # zone constructor doesn't let me set it on creation
+        zone_2.name = "zone-2"
+        zone_list.return_value = [zone_1,
+                                  zone_2,
+                                  zone_1]
 
         provider._populate_zones()
 
-        self.assertEquals(len(provider._azure_zones), 1)
+        # This should be returning two zones since two zones are the same
+        self.assertEquals(len(provider._azure_zones), 2)
 
     def test_bad_zone_response(self):
         provider = self._get_provider()
 
         _get = provider._dns_client.zones.get
         _get.side_effect = CloudError(Mock(status=404), 'Azure Error')
-        trip = False
-        try:
-            provider._check_zone('unit.test', create=False)
-        except CloudError:
-            trip = True
-        self.assertEquals(trip, True)
+        self.assertEquals(
+            provider._check_zone('unit.test', create=False),
+            None
+        )
 
     def test_apply(self):
         provider = self._get_provider()