On 30.05.2016 12:49, Petr Spacek wrote:
On 29.5.2016 14:45, Martin Basti wrote:

On 27.05.2016 14:12, Petr Spacek wrote:
On 25.5.2016 12:50, Martin Basti wrote:
On 20.05.2016 12:19, Petr Spacek wrote:
On 11.5.2016 12:08, Martin Basti wrote:
On 03.05.2016 14:59, Petr Spacek wrote:
Hello,

DNS upgrade: change forwarding policy to "only" if private IPs are used.

https://fedorahosted.org/freeipa/ticket/5710

This is the upgrade part. I will add one more patch to print a warning in
dnsforwardzone* commands to avoid surprises. Please do not close the ticket
yet.



1)
Upgrade failed with 'BindInstance' object has no attribute
'named_conf_get_directive'
IPA server upgrade failed: Inspect /var/log/ipaupgrade.log and run command
ipa-server-upgrade manually.
('IPA upgrade failed.', 1)
The ipa-server-upgrade command failed. See /var/log/ipaupgrade.log for more
information

2016-05-11T08:26:20Z ERROR Upgrade failed with 'BindInstance' object has no
attribute 'named_conf_get_directive'
2016-05-11T08:26:20Z DEBUG Traceback (most recent call last):
     File
"/usr/lib/python2.7/site-packages/ipaserver/install/upgradeinstance.py",
line
213, in __upgrade
       self.modified = (ld.update(self.files) or self.modified)
     File "/usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py",
line 917, in update
       self._run_updates(all_updates)
     File "/usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py",
line 889, in _run_updates
       self._run_update_plugin(update['plugin'])
     File "/usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py",
line 862, in _run_update_plugin
       restart_ds, updates = self.api.Updater[plugin_name]()
     File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line
1418, in
__call__
       return self.execute(**options)
     File
"/usr/lib/python2.7/site-packages/ipaserver/install/plugins/dns.py",
line 547, in execute
       self.update_global_named_conf_forwarder(bind)
     File
"/usr/lib/python2.7/site-packages/ipaserver/install/plugins/dns.py",
line 508, in update_global_named_conf_forwarder
       if bind.named_conf_get_directive(
AttributeError: 'BindInstance' object has no attribute
'named_conf_get_directive'

2016-05-11T08:26:20Z DEBUG Traceback (most recent call last):
     File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py",
line
447, in start_creation
       run_step(full_msg, method)
     File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py",
line
437, in run_step
       method()
     File
"/usr/lib/python2.7/site-packages/ipaserver/install/upgradeinstance.py",
line
221, in __upgrade
       raise RuntimeError(e)
RuntimeError: 'BindInstance' object has no attribute
'named_conf_get_directive'

PATCH * Add ipaDNSVersion option to dnsconfig* commands and use new
attribute *
2)
+        Int('ipadnsversion?',
+            label=_('IPA DNS version'),
+        ),

Shouldn't be this part of System: Read DNS Configuration permission?

3)
-    def postprocess_result(self, result):
+    def postprocess_result(self, result, show_version):
            if not any(param in result['result'] for param in self.params):
                result['summary'] = unicode(_('Global DNS configuration is
empty'))

show_version param was added but I don't see it used in this patch.

4)
+        Int('ipadnsversion?',
+            label=_('IPA DNS version'),
+        ),

Could we add comment here that this option is accessible only from
installers
and upgrade?

5)
+        for config_option in container_entry.get("ipaConfigString", []):
+            matched = re.match("^DNSVersion\s+(?P<version>\d+)$",
+                               config_option, flags=re.I)
+            if matched:
+                version = int(matched.group("version"))

Shouldn't we print error if version cannot be parsed?

PATCH  * DNS upgrade: separate backup logic to make it reusable *

LGTM

PATCH * Add function ipapython.dnsutil.related_to_auto_empty_zone() *

7)
I'm curious why do you need to check superdomains?

PATCH * DNS upgrade: change forwarding policy to = only for conflicting
forward zones*

8)
+            self.log.debug('Zone %s was sucessfully modified to use '
+                           'forward policy "only"', zone['idnsname'][0])
<---missing empty line---->
+    def execute(self, **options):

PATCH * DNS upgrade: change global forwarding policy in LDAP to "only" if
private IPs are used *
9)
- dnsutil.related_to_auto_empty_zone(zone.get('idnsname')[0])
+                dnsutil.related_to_auto_empty_zone(
+                    dnsutil.DNSName(zone.get('idnsname')[0]))

Should be in previous commit

10)
-            return
+            return False, []
This should be fixed in the previous commit

PATCH * DNS upgrade: change global forwarding policy in named.conf to "only"
if private IPs are used *
11)
IMO this is an upgrade of configuration and this should be in
ipaserver/install/server/upgrade.py, upgrade plugins are used only for
updating of LDAP values

Unless you really want to use this as precedence, but then it requires
broader
discussion.

12)

bind.named_conf_get_directive
should be
bindinstance.named_conf_get_directive

see 1)
This new patchset completely obsoletes the old one. I had to reshuffle few
things to to make the split between server config & LDAP upgrade possible.

Hopefully I addressed all your comment.

commits
* Move IP address resolution from ipaserver.install.installutils to
ipapython.dnsutil *  and * Turn verify_host_resolvable() into a wrapper around
ipapython.dnsutil *

cause regression in case that dns.python resolver returns NoNameservers
exception, it is handled as 'Internal server error'

In original code every exception was caught and transformed to
DNSNotARecordError.

So we have following options:
* keep the old behavior in 'resolve_rrsets' and catch all exceptions there
* or catch all DNS errors in 'verify_host_resolvable' and raise it as new
PublicError (DNSGenericError (doesn't exist) for example)


E               InternalError: an internal error has occurred

../ipalib/rpc.py:1100: InternalError
   test_forwardzone_delegation_warnings.test_command[0017: dnsrecord_mod:
Delete
(using dnsrecord-mod) NS record which delegates zone
u'fw.sub2.sub.dnszone.test.' from zone u'dnszone.test' (expected warning for
u'fw.sub2.sub.dnszone.test.')]

[Wed May 25 12:17:00.172143 2016] [wsgi:error] [pid 62789] Traceback (most
recent call last):
[Wed May 25 12:17:00.172152 2016] [wsgi:error] [pid 62789]   File
"/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 350, in
wsgi_execute
[Wed May 25 12:17:00.172158 2016] [wsgi:error] [pid 62789] result =
self.Command[name](*args, **options)
[Wed May 25 12:17:00.172164 2016] [wsgi:error] [pid 62789]   File
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 434, in __call__
[Wed May 25 12:17:00.172168 2016] [wsgi:error] [pid 62789] return
self.__do_call(*args, **options)
[Wed May 25 12:17:00.172173 2016] [wsgi:error] [pid 62789]   File
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 460, in __do_call
[Wed May 25 12:17:00.172178 2016] [wsgi:error] [pid 62789]     ret =
self.run(*args, **options)
[Wed May 25 12:17:00.172183 2016] [wsgi:error] [pid 62789]   File
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 777, in run
[Wed May 25 12:17:00.172189 2016] [wsgi:error] [pid 62789] return
self.execute(*args, **options)
[Wed May 25 12:17:00.172194 2016] [wsgi:error] [pid 62789]   File
"/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 3774, in
execute
[Wed May 25 12:17:00.172199 2016] [wsgi:error] [pid 62789] result =
super(dnsrecord_add, self).execute(*keys, **options)
[Wed May 25 12:17:00.172204 2016] [wsgi:error] [pid 62789]   File
"/usr/lib/python2.7/site-packages/ipalib/plugins/baseldap.py", line 1230, in
execute
[Wed May 25 12:17:00.172209 2016] [wsgi:error] [pid 62789] *keys, **options)
[Wed May 25 12:17:00.172213 2016] [wsgi:error] [pid 62789]   File
"/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 3719, in
pre_callback
[Wed May 25 12:17:00.172229 2016] [wsgi:error] [pid 62789]
self.obj.run_precallback_validators(dn, entry_attrs, *keys, **options)
[Wed May 25 12:17:00.172237 2016] [wsgi:error] [pid 62789]   File
"/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 3135, in
run_precallback_validators
[Wed May 25 12:17:00.172242 2016] [wsgi:error] [pid 62789] rtype_cb(ldap, dn,
entry_attrs, *keys, **options)
[Wed May 25 12:17:00.172247 2016] [wsgi:error] [pid 62789]   File
"/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 3057, in
_nsrecord_pre_callback
[Wed May 25 12:17:00.172252 2016] [wsgi:error] [pid 62789]
check_ns_rec_resolvable(keys[0], DNSName(nsrecord), self.log)
[Wed May 25 12:17:00.172256 2016] [wsgi:error] [pid 62789]   File
"/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 1577, in
check_ns_rec_resolvable
[Wed May 25 12:17:00.172261 2016] [wsgi:error] [pid 62789]
verify_host_resolvable(name)
[Wed May 25 12:17:00.172265 2016] [wsgi:error] [pid 62789]   File
"/usr/lib/python2.7/site-packages/ipalib/util.py", line 70, in
verify_host_resolvable
[Wed May 25 12:17:00.172270 2016] [wsgi:error] [pid 62789]     if not
resolve_ip_addresses(fqdn):
[Wed May 25 12:17:00.172274 2016] [wsgi:error] [pid 62789]   File
"/usr/lib/python2.7/site-packages/ipapython/dnsutil.py", line 328, in
resolve_ip_addresses
[Wed May 25 12:17:00.172278 2016] [wsgi:error] [pid 62789] rrsets =
resolve_rrsets(fqdn, ['A', 'AAAA'])
[Wed May 25 12:17:00.172282 2016] [wsgi:error] [pid 62789]   File
"/usr/lib/python2.7/site-packages/ipapython/dnsutil.py", line 305, in
resolve_rrsets
[Wed May 25 12:17:00.172287 2016] [wsgi:error] [pid 62789] answer =
dns.resolver.query(fqdn, rdtype)
[Wed May 25 12:17:00.172292 2016] [wsgi:error] [pid 62789]   File
"/usr/lib/python2.7/site-packages/dns/resolver.py", line 1029, in query
[Wed May 25 12:17:00.172296 2016] [wsgi:error] [pid 62789] raise_on_no_answer,
source_port)
[Wed May 25 12:17:00.172301 2016] [wsgi:error] [pid 62789]   File
"/usr/lib/python2.7/site-packages/dns/resolver.py", line 856, in query
[Wed May 25 12:17:00.172328 2016] [wsgi:error] [pid 62789]     raise
NoNameservers(request=request, errors=errors)
Fixed patches are attached.


Please note that I've renumbered the patches because the naming does not match
the original set anymore.

NACK

# ipa-run-tests test_xmlrpc/test_host_plugin.py
===============================================================================================
test session starts
===============================================================================================

platform linux2 -- Python 2.7.11, pytest-2.9.1, py-1.4.31, pluggy-0.3.1
rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: pytest.ini
plugins: sourceorder-0.5, multihost-1.0
collected 41 items

test_xmlrpc/test_host_plugin.py ...................F...........s.........

====================================================================================================
FAILURES
=====================================================================================================

________________________________________________________________________________________
TestCRUD.test_try_add_not_in_dns
_________________________________________________________________________________________


self = <ipatests.test_xmlrpc.test_host_plugin.TestCRUD object at
0x7efc061e4110>, host = <ipatests.test_xmlrpc.tracker.host_plugin.HostTracker
object at 0x7efc0623bb90>

     def test_try_add_not_in_dns(self, host):
         host.ensure_missing()
         command = host.make_create_command(force=False)
         with raises_exact(errors.DNSNotARecordError(
               reason=u'Host does not have corresponding DNS A/AAAA record')):
test_xmlrpc/test_host_plugin.py:315:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../ipalib/errors.py:264: in __init__
     messages.process_message_arguments(self, format, message, **kw)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

obj = DNSNotARecordError(), format = None, message = None, kw = {'reason':
'Host does not have corresponding DNS A/AAAA record'}, key = 'reason', value =
'Host does not have corresponding DNS A/AAAA record'
name = 'DNSNotARecordError'

     def process_message_arguments(obj, format=None, message=None, **kw):
         for key, value in kw.items():
             if not isinstance(value, six.integer_types):
                 try:
                     kw[key] = unicode(value)
                 except UnicodeError:
                     pass
         obj.kw = kw
         name = obj.__class__.__name__
         if obj.format is not None and format is not None:
             raise ValueError(
                 'non-generic %r needs format=None; got format=%r' % (
                     name, format)
             )
         if message is None:
             if obj.format is None:
                 if format is None:
                     raise ValueError(
                         '%s.format is None yet format=None, message=None' % 
name
                     )
                 obj.format = format
             obj.forwarded = False
           obj.msg = obj.format % kw
E           KeyError: 'hostname'
Sorry, this got lost in all the 'normal' failures :-)

I do not 100% understand this test logic so I hope that attached patch fixes
it correctly.


We broke IPA 4.3 tests, patch that fixes them is attached.

From 41d384529f01a280129f8d01757f69fba8e3fbad Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Mon, 6 Jun 2016 10:56:22 +0200
Subject: [PATCH] Fix: exceptions in DNS tests should not have data attribute

This was accidentally backported from master branch and should be removed

https://fedorahosted.org/freeipa/ticket/5710
---
 ipatests/test_xmlrpc/test_dns_plugin.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_dns_plugin.py b/ipatests/test_xmlrpc/test_dns_plugin.py
index d698050006e5be07c9b686ece73073e406657f65..7c2450fa438528e27e70a8ac20fa821a51f2b99e 100644
--- a/ipatests/test_xmlrpc/test_dns_plugin.py
+++ b/ipatests/test_xmlrpc/test_dns_plugin.py
@@ -1762,7 +1762,7 @@ class test_dns(Declarative):
                      u'code': 13021,
                      u'type': u'warning',
                      u'name': u'DNSForwardPolicyConflictWithEmptyZone',
-                     u'data': {}},
+                    },
                     {u'message': lambda x: x.startswith(
                         u"DNS server %s: query '. SOA':" % fwd_ip),
                      u'code': 13006,
@@ -3424,7 +3424,7 @@ class test_forward_zones(Declarative):
                      u'code': 13021,
                      u'type': u'warning',
                      u'name': u'DNSForwardPolicyConflictWithEmptyZone',
-                     u'data': {}},
+                    },
                     {u'message': lambda x: x.startswith(
                         u"DNS server %s: query '%s SOA':" %
                         (forwarder1, fwzone2)),
-- 
2.5.5

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to