On 6.6.2016 11:55, Martin Basti wrote:
> 
> 
> 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.

Of yes, we forgot to backport it. ACK.

-- 
Petr^2 Spacek

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