On 19.2.2014 15:11, Petr Spacek wrote:
On 18.2.2014 17:34, Nathaniel McCallum wrote:
On Tue, 2014-02-18 at 17:06 +0100, Petr Viktorin wrote:
On 02/18/2014 04:45 PM, Petr Spacek wrote:
Hello,

Add wait_for_dns option to default.conf.

This option makes record changes in DNS tree synchronous.
IPA calls will wait until new data are visible over DNS protocol.

It is intended only for testing - it should prevent tests from
failing if there is bigger delay between change in LDAP and DNS.

I would recommend value like 10 seconds.

Here are a few Python nitpicks you requested.

Thank you very much. This new version solves problems you found + adds proper
handling for real DNS timeouts.

It seems to me like a more general TimeoutError would be useful in a
broader context. DNSTimeout seems overly narrow to me, unless I'm
missing something.

I would like to keep them separate. DNSTimeout shouldn't be handled at all
because it means that your DNS server or database is dead or broken in some
interesting way.

I assume that generic TimeoutError could be interpreted as 'try it
again'/'failover' or something like that.

Maybe the DNSTimeout is not the best name, I'm open to suggestions.

I have sent the old version with new name, gggrrr.

--
Petr^2 Spacek
From 39483c21c26bcc883161481d1987a6af216e1581 Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Wed, 19 Feb 2014 14:18:38 +0100
Subject: [PATCH] Add wait_for_dns option to default.conf.

This option makes record changes in DNS tree synchronous.
IPA calls will wait until new data are visible over DNS protocol.

It is intended only for testing - it should prevent tests from
failing if there is bigger delay between change in LDAP and DNS.
---
 ipa-client/man/default.conf.5 |   3 +
 ipalib/constants.py           |   1 +
 ipalib/errors.py              |  18 ++++++
 ipalib/plugins/dns.py         | 146 ++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 164 insertions(+), 4 deletions(-)

diff --git a/ipa-client/man/default.conf.5 b/ipa-client/man/default.conf.5
index 5d5a48db62cb97e7424b42b6cb70d0c872b2bc34..e8938cb5799c4fa41e5a1b75125e2a4063bf010f 100644
--- a/ipa-client/man/default.conf.5
+++ b/ipa-client/man/default.conf.5
@@ -178,6 +178,9 @@ Used internally in the IPA source package to verify that the API has not changed
 .B verbose <boolean>
 When True provides more information. Specifically this sets the global log level to "info".
 .TP
+.B wait_for_dns <number of attempts>
+Controls whether the IPA commands dnsrecord\-{add,mod,del} work synchronously or not. The DNS commands will repeat DNS queries (up to the specified number of attempts) until the DNS server returns an up\-to\-date answer to a query for modified records. Delay between retries is one second. The DNS commands will return a DNSTimeout exception if the answer doesn't match the expected value even after the specified number of attempts. The DNS queries will be sent to the resolver configured in /etc/resolv.conf on the IPA server. Do not enable this in production! It could cause problems if the resolver on IPA server uses a caching server instead of a local authoritative server. The default is disabled (the option is not present).
+.TP
 .B xmlrpc_uri <URI>
 Specifies the URI of the XML\-RPC server for a client. This may be used by IPA, and is used by some external tools, such as ipa\-getcert. Example: https://ipa.example.com/ipa/xml
 .TP
diff --git a/ipalib/constants.py b/ipalib/constants.py
index ae0827729764983675d5ae59bbd16bad1c0805ce..d6955f8cb62822d123f6debcefa4a2f35e40aa96 100644
--- a/ipalib/constants.py
+++ b/ipalib/constants.py
@@ -139,6 +139,7 @@ DEFAULT_CONFIG = (
     ('debug', False),
     ('startup_traceback', False),
     ('mode', 'production'),
+    ('wait_for_dns', False),
 
     # CA plugin:
     ('ca_host', FQDN),  # Set in Env._finalize_core()
diff --git a/ipalib/errors.py b/ipalib/errors.py
index 716decb2b41baf5470a1dc23c0cfb5d1c995e5ff..e6563c0a0ea0f65457b942426e45283dd237ed6e 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -1512,6 +1512,24 @@ class DatabaseTimeout(DatabaseError):
     format = _('LDAP timeout')
 
 
+class DNSTimeout(ExecutionError):
+    """
+    **4212** Raised when an DNS query didn't return expected answer
+    in expected time period
+
+    For example:
+
+    >>> raise DNSTimeout(expected="zone3.test. 86400 IN A 192.0.2.1", \
+                         got="zone3.test. 86400 IN A 192.168.1.1")
+    Traceback (most recent call last):
+      ...
+    DNSTimeout: DNS query timeout: Expected {zone3.test. 86400 IN A 192.0.2.1} got {zone3.test. 86400 IN A 192.168.1.1}
+    """
+
+    errno = 4212
+    format = _('DNS query timeout: Expected {%(expected)s} got {%(got)s}')
+
+
 class CertificateError(ExecutionError):
     """
     **4300** Base class for Certificate execution errors (*4300 - 4399*).
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index e7301a9f78466e9a790d26f03bfab757de501ed6..61a2346904ffcd73e289fc207bf189e90525efa0 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -24,6 +24,7 @@ import netaddr
 import time
 import re
 import dns.name
+import dns.resolver
 
 from ipalib.request import context
 from ipalib import api, errors, output
@@ -2396,6 +2397,113 @@ class dnsrecord(LDAPObject):
                                   'NS record except when located in a zone root '
                                   'record (RFC 6672, section 2.3)'))
 
+    def wait_for_modified_entries(self, entries):
+        '''Call wait_for_modified_attrs for all entries in given dict.
+
+        :param entries:
+            Dict {(dns_domain, dns_name): entry_for_wait_for_modified_attrs}
+        '''
+        for entry_name, entry in entries.iteritems():
+            dns_domain = dns.name.from_text(entry_name[0])
+            dns_name = dns.name.from_text(entry_name[1], origin=dns_domain)
+            self.wait_for_modified_attrs(entry, dns_name,
+                                         dns_domain)
+
+    def wait_for_modified_attrs(self, entry_attrs, dns_name, dns_domain):
+        '''Wait until DNS resolver returns up-to-date answer for given entry.
+
+        :param entry_attrs:
+            None if the entry was deleted from LDAP or
+            LDAPEntry instance containing at least all modified attributes.
+        :param dns_name: FQDN as dns.name.Name instance or string
+        '''
+        record_attr_suf = 'record'
+        max_retries = int(self.api.env['wait_for_dns'])
+        warn_retries = max_retries / 2
+        period = 1  # second
+
+        # represent data in LDAP as dictionary rdtype => rrset
+        ldap_rrsets = {}
+        if entry_attrs:
+            for attr, value in entry_attrs.iteritems():
+                if not attr.endswith(record_attr_suf):
+                    continue
+
+                rdtype = dns.rdatatype.from_text(attr[0:-len(record_attr_suf)])
+                if value:
+                    try:
+                        # TTL here can be arbitrary value because it is ignored
+                        # during comparison
+                        ldap_rrset = dns.rrset.from_text(
+                            dns_name, 86400, dns.rdataclass.IN, rdtype,
+                            *map(str, value))
+
+                        # make sure that all names are absolute so RRset
+                        # comparison will work
+                        for ldap_rr in ldap_rrset:
+                            ldap_rr.choose_relativity(origin=dns_domain,
+                                                      relativize=False)
+                    except dns.exception.SyntaxError as e:
+                        self.log.error(
+                            'DNS syntax error: %s %s %s: %s',
+                            dns_name, dns.rdatatype.to_text(rdtype),
+                            value, e)
+                        raise
+                else:
+                    ldap_rrset = None
+
+                ldap_rrsets[rdtype] = ldap_rrset
+        else:
+            # all records were deleted => name should not exist in DNS
+            rdtype = dns.rdatatype.from_text('A')
+            ldap_rrsets[rdtype] = 'NXDOMAIN'
+
+        for rdtype, ldap_rrset in ldap_rrsets.iteritems():
+            failures = 0
+            logfn = self.log.debug
+            # compare ldap_rrsets with data in DNS
+            logfn('querying DNS server - expecting answer {%r}',
+                  str(ldap_rrset))
+
+            while failures < max_retries:
+                if failures >= warn_retries:
+                    logfn = self.log.warn
+                try:
+                    dns_answer = dns.resolver.query(dns_name, rdtype,
+                                                    dns.rdataclass.IN,
+                                                    raise_on_no_answer=False)
+                    dns_rrset = dns_answer.rrset
+                except dns.resolver.NXDOMAIN:
+                    dns_rrset = 'NXDOMAIN'
+                except dns.resolver.NoNameservers:
+                    dns_rrset = 'SERVFAIL'
+                except dns.resolver.Timeout:
+                    dns_rrset = 'Timeout'
+
+                if dns_rrset == ldap_rrset:
+                    logfn('DNS answer matches expectations')
+                    return
+
+                failures += 1
+                logfn('waiting for DNS answer {%r} - got {%r}; '
+                      'waiting %s seconds before next try',
+                      str(ldap_rrset), str(dns_rrset), period)
+
+                time.sleep(period)
+
+            # Maximum number of retries was reached:
+            # Do not raise exception only if we have got n successive SERVFAILs
+            # or if we are adding NS record and answer has been NXDOMAIN.
+            # Maybe the user has intentionally created an invalid zone
+            # or the NS record is configured only on parent (this) server.
+            if ((rdtype == dns.rdatatype.NS and dns_rrset == 'NXDOMAIN')
+               or dns_rrset == 'SERVFAIL/timeout'):
+                    self.log.warn('giving up after %s retries - '
+                                  'may be that {%r} is expected answer',
+                                  failures, str(dns_rrset))
+            else:
+                    raise errors.DNSTimeout(expected=ldap_rrset, got=dns_rrset)
+
 api.register(dnsrecord)
 
 
@@ -2558,6 +2666,10 @@ class dnsrecord_add(LDAPCreate):
                 entry_attrs[attr] = list(set(old_entry.get(attr, []) + vals))
 
         self.obj.check_record_type_collisions(keys, old_entry, entry_attrs)
+        context.dnsrecord_entry_mods = getattr(context, 'dnsrecord_entry_mods',
+                                               {})
+        context.dnsrecord_entry_mods[(keys[0], keys[1])] = entry_attrs.copy()
+
         return dn
 
     def exc_callback(self, keys, options, exc, call_func, *call_args, **call_kwargs):
@@ -2585,6 +2697,8 @@ class dnsrecord_add(LDAPCreate):
 
         self.obj.postprocess_record(entry_attrs, **options)
 
+        if self.api.env['wait_for_dns']:
+            self.obj.wait_for_modified_entries(context.dnsrecord_entry_mods)
         return dn
 
 api.register(dnsrecord_add)
@@ -2660,6 +2774,10 @@ class dnsrecord_mod(LDAPUpdate):
                 entry_attrs[attr] = list(set(old_entry[attr] + new_dnsvalue))
 
         self.obj.check_record_type_collisions(keys, old_entry, entry_attrs)
+
+        context.dnsrecord_entry_mods = getattr(context, 'dnsrecord_entry_mods',
+                                               {})
+        context.dnsrecord_entry_mods[(keys[0], keys[1])] = entry_attrs.copy()
         return dn
 
     def execute(self, *keys, **options):
@@ -2681,7 +2799,14 @@ class dnsrecord_mod(LDAPUpdate):
                     break
 
             if del_all:
-                return self.obj.methods.delentry(*keys, version=options['version'])
+                result = self.obj.methods.delentry(*keys,
+                                                   version=options['version'])
+                # indicate that entry was deleted
+                context.dnsrecord_entry_mods[(keys[0], keys[1])] = None
+
+        if self.api.env['wait_for_dns']:
+            self.obj.wait_for_modified_entries(context.dnsrecord_entry_mods)
+
         return result
 
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
@@ -2825,7 +2950,10 @@ class dnsrecord_del(LDAPUpdate):
         # set del_all flag in context
         # when the flag is enabled, the entire DNS record object is deleted
         # in a post callback
-        setattr(context, 'del_all', del_all)
+        context.del_all = del_all
+        context.dnsrecord_entry_mods = getattr(context, 'dnsrecord_entry_mods',
+                                               {})
+        context.dnsrecord_entry_mods[(keys[0], keys[1])] = entry_attrs.copy()
 
         return dn
 
@@ -2837,13 +2965,23 @@ class dnsrecord_del(LDAPUpdate):
                         error=_('Zone record \'%s\' cannot be deleted') \
                                 % _dns_zone_record
                       )
-            return self.obj.methods.delentry(*keys, version=options['version'])
+            result = self.obj.methods.delentry(*keys,
+                                               version=options['version'])
+            if self.api.env['wait_for_dns']:
+                entries = {(keys[0], keys[1]): None}
+                self.obj.wait_for_modified_entries(entries)
+            return result
 
         result = super(dnsrecord_del, self).execute(*keys, **options)
 
         if getattr(context, 'del_all', False) and not \
                 self.obj.is_pkey_zone_record(*keys):
-            return self.obj.methods.delentry(*keys, version=options['version'])
+            result = self.obj.methods.delentry(*keys,
+                                               version=options['version'])
+            context.dnsrecord_entry_mods[(keys[0], keys[1])] = None
+
+        if self.api.env['wait_for_dns']:
+            self.obj.wait_for_modified_entries(context.dnsrecord_entry_mods)
         return result
 
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
-- 
1.8.5.3

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to