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.


ACK

master:
* 0c75df4bf3784eae08f41c176bbaab44c6d510a7 Move check_zone_overlap() from ipapython.ipautil to ipapython.dnsutil * ec49130b94d2aa195c6b704a30fe6c3137fabdbf Use root_logger for verify_host_resolvable() * dc405005f537cf278fd6ddfe6b87060bd13d9a67 Move IP address resolution from ipaserver.install.installutils to ipapython.dnsutil * 70794c7b1d001ce331d4a64c77d23abcc02c541e Turn verify_host_resolvable() into a wrapper around ipapython.dnsutil * 321a2ba9185e4a21d5b2f9949cd3bec32a1fd60a Add ipaDNSVersion option to dnsconfig* commands and use new attribute * a4da9a23788d9f09f562c12d80353fca42f73441 DNS upgrade: separate backup logic to make it reusable * c978ad5b425a564b6bd3b97fb7a5e25219000e52 Add function ipapython.dnsutil.related_to_auto_empty_zone() * f750d42b6f2d7f792ce56b6832d2bd1ae1f333a0 DNS upgrade: change forwarding policy to = only for conflicting forward zones * e45a80308c947a58c0fb5266d75eedc1d9aef321 DNS upgrade: change global forwarding policy in LDAP to "only" if private IPs are used * 6eb00561c0f85085d86f7be936b632ba017fc4f1 DNS upgrade: change global forwarding policy in named.conf to "only" if private IPs are used

ipa-4-3:
* b18f848bed6ace5fd63fe1c6559ebc68997ccbe2 Move check_zone_overlap() from ipapython.ipautil to ipapython.dnsutil * a54b8222dc2f3ec7d22e67f00b8c30b3893c0ced Use root_logger for verify_host_resolvable() * f170f155b9a0921b6f4ab6b24713a3c1bf0148bc Move IP address resolution from ipaserver.install.installutils to ipapython.dnsutil * da119a620f36bac24f43098d2f0713bb46cf4329 Turn verify_host_resolvable() into a wrapper around ipapython.dnsutil * d75998c55d9a961c1cbd74b3ef00a5921e9bde0a Add ipaDNSVersion option to dnsconfig* commands and use new attribute * 12590597320c7d4ef4506a018097dad629113fdc DNS upgrade: separate backup logic to make it reusable * e69254b253903aa8fd76022df7f5e5505a819ebf Add function ipapython.dnsutil.related_to_auto_empty_zone() * f8a39898bbd4c17d2976515c9d73f0e27a984709 DNS upgrade: change forwarding policy to = only for conflicting forward zones * 700246174c8c9cfbbbee5b0101acfa4227995cc7 DNS upgrade: change global forwarding policy in LDAP to "only" if private IPs are used * 233550ab1dab55d518cfdc104b521672babdde7f DNS upgrade: change global forwarding policy in named.conf to "only" if private IPs are used


--
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