[Freeipa-devel] [PATCH 0063] client: Update DNS with all available local IP addresses.

Wed Aug 19 11:12:09 UTC 2015


On 08/19/2015 12:46 PM, David Kupka wrote:
> On 19/08/15 11:06, Jan Cholasta wrote:
>> On 19.8.2015 10:36, Martin Basti wrote:
>>>
>>>
>>> On 08/18/2015 10:53 PM, Martin Basti wrote:
>>>>
>>>>
>>>> On 08/18/2015 08:02 PM, David Kupka wrote:
>>>>> On 31/07/15 18:31, Martin Basti wrote:
>>>>>> On 28/07/15 09:52, David Kupka wrote:
>>>>>>> On 27/07/15 16:45, David Kupka wrote:
>>>>>>>> On 15/01/15 17:13, David Kupka wrote:
>>>>>>>>> On 01/15/2015 03:22 PM, David Kupka wrote:
>>>>>>>>>> On 01/15/2015 12:43 PM, David Kupka wrote:
>>>>>>>>>>> On 01/12/2015 06:34 PM, Martin Basti wrote:
>>>>>>>>>>>> On 09/01/15 14:43, David Kupka wrote:
>>>>>>>>>>>>> On 01/07/2015 04:15 PM, Martin Basti wrote:
>>>>>>>>>>>>>> On 07/01/15 12:27, David Kupka wrote:
>>>>>>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/4249
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thank you for patch:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 1)
>>>>>>>>>>>>>> -        root_logger.error("Cannot update DNS records! "
>>>>>>>>>>>>>> -                          "Failed to connect to server
>>>>>>>>>>>>>> '%s'.",
>>>>>>>>>>>>>> server)
>>>>>>>>>>>>>> +        ips = get_local_ipaddresses()
>>>>>>>>>>>>>> +    except CalledProcessError as e:
>>>>>>>>>>>>>> +        root_logger.error("Cannot update DNS records. %s"
>>>>>>>>>>>>>> % e)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> IMO the error message should be more specific, add there
>>>>>>>>>>>>>> something
>>>>>>>>>>>>>> like
>>>>>>>>>>>>>> "Unable to get local IP addresses". at least in log.debug()
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 2)
>>>>>>>>>>>>>> +    lines = ipresult[0].replace('\\', '').split('\n')
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> .replace() is not needed
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 3)
>>>>>>>>>>>>>> +    if len(ips) == 0:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> if not ips:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> is more pythonic by PEP8
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks for catching these. Updated patch attached.
>>>>>>>>>>>>>
>>>>>>>>>>>> merciful NACK
>>>>>>>>>>>>
>>>>>>>>>>>> Thank you for the patch, unfortunately I hit one issue which
>>>>>>>>>>>> needs
>>>>>>>>>>>> to be
>>>>>>>>>>>> resolved.
>>>>>>>>>>>>
>>>>>>>>>>>> If "sync PTR" is activated in zone settings, and reverse zone
>>>>>>>>>>>> doesn't
>>>>>>>>>>>> exists, nsupdate/BIND returns SERVFAIL and ipa-client-install
>>>>>>>>>>>> print
>>>>>>>>>>>> Error message, 'DNS update failed'. In fact, all A/AAAA
>>>>>>>>>>>> records was
>>>>>>>>>>>> succesfully updated, only PTR records failed.
>>>>>>>>>>>>
>>>>>>>>>>>> Bind log:
>>>>>>>>>>>> named-pkcs11[28652]: updating zone 'example.com/IN': adding an
>>>>>>>>>>>> RR at
>>>>>>>>>>>> 'vm-101.example.com' AAAA
>>>>>>>>>>>>
>>>>>>>>>>>> named-pkcs11[28652]: PTR record synchronization (addition) for
>>>>>>>>>>>> A/AAAA
>>>>>>>>>>>> 'vm-101.example.com.' refused: unable to find active reverse
>>>>>>>>>>>> zone
>>>>>>>>>>>> for IP
>>>>>>>>>>>> address '2620:52:0:104c:21a:4aff:fe10:4eaa': not found
>>>>>>>>>>>>
>>>>>>>>>>>> With IPv6 we have several addresses from different reverse
>>>>>>>>>>>> zones and
>>>>>>>>>>>> this situation may happen often.
>>>>>>>>>>>> I suggest following:
>>>>>>>>>>>> 1) Print list of addresses which will be updated. (Now if 
>>>>>>>>>>>> update
>>>>>>>>>>>> fails,
>>>>>>>>>>>> user needs to read log, which addresses installer tried to
>>>>>>>>>>>> update)
>>>>>>>>>>>> 2) Split nsupdates per A/AAAA record.
>>>>>>>>>>>> 3a) If failed, check with DNS query if A/AAAA and PTR 
>>>>>>>>>>>> record are
>>>>>>>>>>>> there
>>>>>>>>>>>> and print proper error message
>>>>>>>>>>>> 3b) Just print A/AAAA (or PTR) record may not be updated for
>>>>>>>>>>>> particular
>>>>>>>>>>>> IP address.
>>>>>>>>>>>>
>>>>>>>>>>>> Any other suggestions are welcome.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> After long discussion with DNS and UX guru I've implemented it
>>>>>>>>>>> this
>>>>>>>>>>> way:
>>>>>>>>>>> 1. Call nsupdate only once with all updates.
>>>>>>>>>>> 2. Verify that the expected records are resolvable.
>>>>>>>>>>> 3. If no print list of missing A/AAAA, list of missing PTR
>>>>>>>>>>> records
>>>>>>>>>>> and
>>>>>>>>>>> list to mismatched PTR record.
>>>>>>>>>>>
>>>>>>>>>>> As this is running inside client we can't much more and it's
>>>>>>>>>>> up to
>>>>>>>>>>> user
>>>>>>>>>>> to check what's rotten in his DNS setup.
>>>>>>>>>>>
>>>>>>>>>>> Updated patch attached.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> Freeipa-devel mailing list
>>>>>>>>>>> Freeipa-devel at redhat.com
>>>>>>>>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> One more change to behave well in -crazy- exotic environments 
>>>>>>>>>> that
>>>>>>>>>> resolves more PTR records for single IP.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> _______________________________________________
>>>>>>>>>> Freeipa-devel mailing list
>>>>>>>>>> Freeipa-devel at redhat.com
>>>>>>>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Yet another change to make language nerds and our UX guru 
>>>>>>>>> happy :-)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> _______________________________________________
>>>>>>>>> Freeipa-devel mailing list
>>>>>>>>> Freeipa-devel at redhat.com
>>>>>>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>>>>>>>
>>>>>>>>
>>>>>>>> Rebased patch attached.
>>>>>>>>
>>>>>>>>
>>>>>>> Updated patch attached.
>>>>>>>
>>>>>> Just for record this patch is for dualstack/IPv6 support.
>>>>>> IMO this ticket also requires to fix ipa-join to support IPv6.
>>>>>>
>>>>>> I still have doubts to have multihomed support as default, this 
>>>>>> may be
>>>>>> unexpected change of ipa-client-install behavior.
>>>>>> I know, is hard to detect which addresses user want to register 
>>>>>> in IPA
>>>>>> without crystal ball, but it should not be impossible :-) .
>>>>>>
>>>>>> I propose following solution:
>>>>>>
>>>>>> To add new options:
>>>>>> --multihomed or --all-ip-address - all IP addresses from client
>>>>>> will be
>>>>>> used
>>>>>> --ip-address  - adress which will be registered on (IPA) DNS server
>>>>>> --ip-address-interface - interface from which address will be
>>>>>> registered
>>>>>>
>>>>>>
>>>>>> 0) without any option specified, current behavior will be used + 
>>>>>> IPv6
>>>>>> * detect which address is used to communicate with IPA server
>>>>>> * detect interface where this address belongs
>>>>>> * use ipv4 and all ipv6 addresses of this interface
>>>>>> * if --enable-dns-updates=true: configure SSSD as is configured now:
>>>>>> automatically detect which address is used + patched SSSD will also
>>>>>> updates proper IPv6 address
>>>>>>
>>>>>> 1) --multihomed or --all-ip-addresses (this is multihomed ticket)
>>>>>> * all adresses will be used
>>>>>> * if --enable-dns-updates=true: SSSD will be configured to send all
>>>>>> ip_addresses
>>>>>>
>>>>>> 2) --ip-address option specified:
>>>>>> * only specified addresses will be used (+ check if this addresses
>>>>>> exist
>>>>>> locally)
>>>>>> * if --enable-dns-updates=true: ERROR dynamic updates may change 
>>>>>> this
>>>>>> address (user should choose static vs dynamic)
>>>>>>
>>>>>> 3) --ip-address-interface option specified:
>>>>>> * only addresses from specified interfaces will be used
>>>>>> * if --enable-dns-updates=true: SSSD will be configured to use these
>>>>>> interfaces to get addresses that will be dynamically updated on dns
>>>>>>
>>>>>> Modification of current patch should not be hard, we already have
>>>>>> almost
>>>>>> everything implemented:
>>>>>> * method get_local_addresses should return dict {interface:[list of
>>>>>> addresses]}, this can be used in all of 4 cases.
>>>>>> * restore original function to detect IP address used to communicate
>>>>>> with IPA server
>>>>>>
>>>>>> I insist on 0) and 1), others may be stretch goal (easy to 
>>>>>> implement)
>>>>>> (It would be shame to not implemented multihomed support together 
>>>>>> with
>>>>>> this ticket, as it requires max 5 extra lines of code)
>>>>>>
>>>>>> Seems my proposal reasonable?
>>>>>>
>>>>>> What is you opinion Martin? Should we just use all addresses to be
>>>>>> registered, or try to keep old behavior as much as possible?
>>>>>>
>>>>>> Martin^2
>>>>>>
>>>>>
>>>>> 0-2 implemented, IMO there is no real use-case for 3. It can be added
>>>>> later when/if there is need.
>>>>> Updated patch (+ rebase for ipa-4-2 branch) attached.
>>>>>
>>>>
>>>> ACK, I just modified typo in --ip-address help message before push.
>>>>
>>>> SSSD guys (Pavel CCed) will provide SSSD srpm that should go to our
>>>> freeipa-master copr. Then we will bump required SSSD version in
>>>> specfile.
>>>>
>>>> Pushed to ipa-4-2: ff34125bcaa99898859cb8ceefea88a4497959b3
>>>> Pushed to master: 8ba1392a3903894dda06c733bf37853c6cc3108c
>>>>
>>> Attached patch bumps required version of SSSD (available in
>>> freeipa-master copr)
>>
>> ACK.
>>
>> Pushed to:
>> master: 9fe67dcf2b6c10ca4eebab1c573d101316f481cd
>> ipa-4-2: 7924007a83a82674a495afe0e63a4bc85ab2a5ab
>>
>
> man page update.
ACK

Pushed to:
master: f160aa3d0a3f8714463c12dac1acc479d1e18a8d
ipa-4-2: d0c41bd2f2d125fa591b13c602c125a2a112a6c7