On 19.2.2014 17:55, Martin Basti wrote:
On Wed, 2014-02-19 at 17:10 +0100, Petr Spacek wrote:
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.

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

Tests failed:
test_dns[92]: dnsrecord_add: Add A record to u'ns2' in zone
u'zone3.test' ... ok
   File "/usr/lib/python2.7/site-packages/nose/case.py", line 197, in
runTest
     self.test(*self.arg)
   File "/root/freeipa/ipatests/test_xmlrpc/xmlrpc_test.py", line 291, in
<lambda>
     func = lambda: self.check(nice, **test)
   File "/root/freeipa/ipatests/test_xmlrpc/xmlrpc_test.py", line 309, in
check
     self.check_output(nice, cmd, args, options, expected, extra_check)
   File "/root/freeipa/ipatests/test_xmlrpc/xmlrpc_test.py", line 348, in
check_output
     got = api.Command[cmd](*args, **options)
   File "/root/freeipa/ipalib/frontend.py", line 436, in __call__
     ret = self.run(*args, **options)
   File "/root/freeipa/ipalib/frontend.py", line 761, in run
     return self.forward(*args, **options)
   File "/root/freeipa/ipalib/frontend.py", line 782, in forward
     return self.Backend.rpcclient.forward(self.name, *args, **kw)
   File "/root/freeipa/ipalib/rpc.py", line 836, in forward
     return self._call_command(command, params)
   File "/root/freeipa/ipalib/rpc.py", line 813, in _call_command
     return command(*params)
   File "/root/freeipa/ipalib/rpc.py", line 951, in _call
     return self.__request(name, args)
   File "/root/freeipa/ipalib/rpc.py", line 945, in __request
     raise error_class(message=error['message'])
DNSTimeout: DNS query timeout: Expected {_kerberos.zone2.test. 86400 IN
TXT "IDM.LAB.ENG.BRQ.REDHAT.COM"} got {SERVFAIL}

======================================================================
ERROR: test_dns[51]: dnsrecord_add: Add NS+DNAME record to u'zone2.test'
zone record using dnsrecord_add
----------------------------------------------------------------------
Traceback (most recent call last):
   File "/usr/lib/python2.7/site-packages/nose/case.py", line 197, in
runTest
     self.test(*self.arg)
   File "/root/freeipa/ipatests/test_xmlrpc/xmlrpc_test.py", line 291, in
<lambda>
     func = lambda: self.check(nice, **test)
   File "/root/freeipa/ipatests/test_xmlrpc/xmlrpc_test.py", line 309, in
check
     self.check_output(nice, cmd, args, options, expected, extra_check)
   File "/root/freeipa/ipatests/test_xmlrpc/xmlrpc_test.py", line 348, in
check_output
     got = api.Command[cmd](*args, **options)
   File "/root/freeipa/ipalib/frontend.py", line 436, in __call__
     ret = self.run(*args, **options)
   File "/root/freeipa/ipalib/frontend.py", line 761, in run
     return self.forward(*args, **options)
   File "/root/freeipa/ipalib/frontend.py", line 782, in forward
     return self.Backend.rpcclient.forward(self.name, *args, **kw)
   File "/root/freeipa/ipalib/rpc.py", line 836, in forward
     return self._call_command(command, params)
   File "/root/freeipa/ipalib/rpc.py", line 813, in _call_command
     return command(*params)
   File "/root/freeipa/ipalib/rpc.py", line 951, in _call
     return self.__request(name, args)
   File "/root/freeipa/ipalib/rpc.py", line 945, in __request
     raise error_class(message=error['message'])
DNSTimeout: DNS query timeout: Expected {zone2.test. 86400 IN NS
ns1.dnszone.test.
zone2.test. 86400 IN NS ns1.zone2.test.} got {SERVFAIL}

configuration was: wait_for_dns=10

All tests passed without wait_for_dns option.

Sometimes at first run, I get only error and testing is interrupted.

I hope I covered all corner cases in this version.

I renamed DNSTimeout exception to DNSDataMismatch in hope that it will be less confusing.

--
Petr^2 Spacek
From 6d5c7c96e96d24b5611fbc4f0e0574cebed7a9a8 Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Thu, 20 Feb 2014 14:26:06 +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         | 151 ++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 169 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..f7d362a917107c70ad4deb846a2ffaad9d40e25e 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -1512,6 +1512,24 @@ class DatabaseTimeout(DatabaseError):
     format = _('LDAP timeout')
 
 
+class DNSDataMismatch(ExecutionError):
+    """
+    **4212** Raised when an DNS query didn't return expected answer
+    in expected time period
+
+    For example:
+
+    >>> raise DNSDataMismatch(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):
+      ...
+    DNS data mismatch: Expected {zone3.test. 86400 IN A 192.0.2.1} got {zone3.test. 86400 IN A 192.168.1.1}
+    """
+
+    errno = 4212
+    format = _('DNS data mismatch: 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..6f535db5fa6fe8fbd2d6268e806ac3807a8fd677 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,118 @@ 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  # RRset is empty
+
+                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] = dns.resolver.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.resolver.NoNameservers,
+                        dns.resolver.Timeout) as e:
+                    dns_rrset = e
+
+                # __eq__ doesn't work for dns.exception.DNSException instances
+                if ((isinstance(dns_rrset, dns.exception.DNSException)
+                     and type(dns_rrset) == type(ldap_rrset))
+                    or dns_rrset == ldap_rrset):
+                    logfn('DNS answer matches expectations')
+                    return
+
+                failures += 1
+                logfn('waiting for DNS answer {%r} - got %s: {%r}; '
+                      'waiting %s seconds before next try',
+                      str(ldap_rrset), type(dns_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 isinstance(dns_rrset, dns.resolver.NXDOMAIN))
+                or isinstance(dns_rrset, dns.resolver.NoNameservers)):
+                    self.log.warn('giving up after %s retries - '
+                                  'maybe the %s: {%r} is expected answer',
+                                  failures, type(dns_rrset), str(dns_rrset))
+            else:
+                got = str(type(dns_rrset))
+                if str(dns_rrset):
+                    got += ": %s" % dns_rrset
+                raise errors.DNSDataMismatch(expected=ldap_rrset, got=got)
+
 api.register(dnsrecord)
 
 
@@ -2558,6 +2671,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 +2702,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 +2779,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 +2804,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 +2955,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 +2970,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