diff --git a/octodns/provider/googlecloud.py b/octodns/provider/googlecloud.py index 103c3f5..fb85744 100644 --- a/octodns/provider/googlecloud.py +++ b/octodns/provider/googlecloud.py @@ -53,6 +53,8 @@ class GoogleCloudProvider(BaseProvider): self.log = getLogger('GoogleCloudProvider[{}]'.format(id)) self.id = id + self._gcloud_zones = {} + super(GoogleCloudProvider, self).__init__(id, *args, **kwargs) def _apply(self, plan): @@ -70,7 +72,10 @@ class GoogleCloudProvider(BaseProvider): len(changes)) # Get gcloud zone, or create one if none existed before. - gcloud_zone = self._get_gcloud_zone(desired.name, create=True) + if desired.name not in self.gcloud_zones: + gcloud_zone = self._create_gcloud_zone(desired.name) + else: + gcloud_zone = self.gcloud_zones.get(desired.name) gcloud_changes = gcloud_zone.changes() @@ -117,13 +122,19 @@ class GoogleCloudProvider(BaseProvider): # and only contain lowercase letters, digits or dashes zone_name = re.sub("[^a-z0-9-]", "", dns_name[:-1].replace('.', "-")) - # make sure that the end result did not end up wo leading letter - if re.match('[^a-z]', zone_name[0]): - # I cannot think of a situation where a zone name derived from - # a domain name would'nt start with leading letter and thereby - # violate the constraint, however if such a situation is - # encountered, add a leading "a" here. - zone_name = "a%s" % zone_name + + # Check if there is another zone in google cloud which has the same + # name as the new one + while zone_name in [z.name for z in self.gcloud_zones.values()]: + # If there is a zone in google cloud alredy, then try suffixing the + # name with a -i where i is a number which keeps increasing until + # a free name has been reached. + m = re.match("^(.+)-([0-9]+$)", zone_name) + if m: + i = int(m.group(2)) + 1 + zone_name = "{}-{!s}".format(m.group(1), i) + else: + zone_name += "-2" gcloud_zone = self.gcloud_client.zone( name=zone_name, @@ -131,6 +142,9 @@ class GoogleCloudProvider(BaseProvider): ) gcloud_zone.create(client=self.gcloud_client) + # add this new zone to the list of zones. + self._gcloud_zones[gcloud_zone.dns_name] = gcloud_zone + self.log.info("Created zone %s. Fqdn %s." % (zone_name, dns_name)) @@ -159,39 +173,25 @@ class GoogleCloudProvider(BaseProvider): # yield from is in python 3 only. yield gcloud_record - def _get_gcloud_zone(self, dns_name, page_token=None, create=False): - """Return the ManagedZone which has has the matching dns_name, or - None if no such zone exist, unless create=True, then create a new - one and return it. + def _get_cloud_zones(self, page_token=None): + """Load all ManagedZones into the self._gcloud_zones dict which is + mapped with the dns_name as key. - :param dns_name: fqdn of dns name for zone to get. - :type dns_name: str - :param page_token: page token for the page to get - :type page_token: str - :param create: if true, create ManagedZone if it does not exist - already - - :type return: new google.cloud.dns.ManagedZone + :return: void """ - # Find the google name for the incoming zone + gcloud_zones = self.gcloud_client.list_zones(page_token=page_token) for gcloud_zone in gcloud_zones: - if gcloud_zone.dns_name == dns_name: - return gcloud_zone - else: - # Zone not found. Check if there are more results which could be - # retrieved by checking "next_page_token". - if gcloud_zones.next_page_token: - return self._get_gcloud_zone(dns_name, - gcloud_zones.next_page_token) - else: - # Nothing found, either return None or else create zone and - # return that one (if create=True) - self.log.debug('_get_gcloud_zone: zone name=%s, ' - 'was not found by %s.', - dns_name, self.gcloud_client) - if create: - return self._create_gcloud_zone(dns_name) + self._gcloud_zones[gcloud_zone.dns_name] = gcloud_zone + + if gcloud_zones.next_page_token: + self._get_cloud_zones(gcloud_zones.next_page_token) + + @property + def gcloud_zones(self): + if not self._gcloud_zones: + self._get_cloud_zones() + return self._gcloud_zones def populate(self, zone, target=False, lenient=False): """Required function of manager.py to collect records from zone. @@ -210,7 +210,7 @@ class GoogleCloudProvider(BaseProvider): target, lenient) before = len(zone.records) - gcloud_zone = self._get_gcloud_zone(zone.name) + gcloud_zone = self.gcloud_zones.get(zone.name) if gcloud_zone: for gcloud_record in self._get_gcloud_records(gcloud_zone): diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index b498d4c..fa667cc 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -164,8 +164,9 @@ class DummyResourceRecordSet: class DummyGoogleCloudZone: - def __init__(self, dns_name): + def __init__(self, dns_name, name=""): self.dns_name = dns_name + self.name = name def resource_record_set(self, *args): return DummyResourceRecordSet(*args) @@ -173,6 +174,9 @@ class DummyGoogleCloudZone: def list_resource_record_sets(self, *args): pass + def create(self, *args, **kwargs): + pass + class DummyIterator: """Returns a mock DummyIterator object to use in testing. @@ -239,7 +243,7 @@ class TestGoogleCloudProvider(TestCase): 'type': 'A', 'values': ['1.4.3.2']}) - gcloud_zone_mock = DummyGoogleCloudZone("unit.tests.") + gcloud_zone_mock = DummyGoogleCloudZone("unit.tests.", "unit-tests") status_mock = Mock() return_values_for_status = iter( ['', '', '', '', '', '', '', '', '', '', '', '', '', '', '', @@ -250,10 +254,9 @@ class TestGoogleCloudProvider(TestCase): provider = self._get_provider() provider.gcloud_client = Mock() - provider._get_gcloud_zone = Mock( - return_value=gcloud_zone_mock) + provider._gcloud_zones = {"unit.tests.": gcloud_zone_mock} desired = Mock() - desired.name = Mock(return_value="unit.tests.") + desired.name = "unit.tests." changes = [] changes.append(Create(create_r)) changes.append(Delete(delete_r)) @@ -343,7 +346,7 @@ class TestGoogleCloudProvider(TestCase): google_cloud_zone.list_resource_record_sets = Mock( side_effect=_get_mock_record_sets) - self.assertEqual(provider._get_gcloud_zone("unit.tests.").dns_name, + self.assertEqual(provider.gcloud_zones.get("unit.tests.").dns_name, "unit.tests.") test_zone = Zone('unit.tests.', []) @@ -374,8 +377,8 @@ class TestGoogleCloudProvider(TestCase): provider._get_gcloud_records = Mock( side_effect=[not_same_fqdn]) - provider._get_gcloud_zone = Mock(return_value=DummyGoogleCloudZone( - dns_name="unit.tests.")) + provider._gcloud_zones = { + "unit.tests.": DummyGoogleCloudZone("unit.tests.", "unit-tests")} provider.populate(test_zone) @@ -391,7 +394,7 @@ class TestGoogleCloudProvider(TestCase): provider.gcloud_client.list_zones = Mock( return_value=DummyIterator([])) - self.assertIsNone(provider._get_gcloud_zone("nonexistant.xone"), + self.assertIsNone(provider.gcloud_zones.get("nonexistant.xone"), msg="Check that nonexistant zones return None when" "there's no create=True flag") @@ -413,22 +416,33 @@ class TestGoogleCloudProvider(TestCase): provider.gcloud_client.list_zones = Mock( return_value=DummyIterator([])) - mock_zone = provider._get_gcloud_zone( - 'nonexistant.zone.mock', create=True) + mock_zone = provider._create_gcloud_zone("nonexistant.zone.mock") mock_zone.create.assert_called() provider.gcloud_client.zone.assert_called() provider.gcloud_client.zone.assert_called_once_with( dns_name=u'nonexistant.zone.mock', name=u'nonexistant-zone-moc') - def test__create_zone_with_numbers_in_name(self): + def test__create_zone_with_duplicate_names(self): + + def _create_dummy_zone(name, dns_name): + return DummyGoogleCloudZone(name=name, dns_name=dns_name) + provider = self._get_provider() provider.gcloud_client = Mock() + provider.gcloud_client.zone = Mock(side_effect=_create_dummy_zone) provider.gcloud_client.list_zones = Mock( return_value=DummyIterator([])) - provider._get_gcloud_zone( - '111.', create=True) - provider.gcloud_client.zone.assert_called_once_with( - dns_name=u'111.', name=u'a111') + _gcloud_zones = { + 'unit-tests': DummyGoogleCloudZone("a.unit-tests.", "unit-tests") + } + + provider._gcloud_zones = _gcloud_zones + + test_zone_1 = provider._create_gcloud_zone("unit.tests.") + self.assertEqual(test_zone_1.name, "unit-tests-2") + + test_zone_2 = provider._create_gcloud_zone("unit.tests.") + self.assertEqual(test_zone_2.name, "unit-tests-3")