On (23/11/15 14:57), Pavel Reichl wrote:
On 11/23/2015 02:44 PM, Lukas Slebodnik wrote:
>On (23/11/15 14:18), Pavel Reichl wrote:
>>
>>
>>On 11/23/2015 02:06 PM, Lukas Slebodnik wrote:
>>>On (23/11/15 13:43), Pavel Reichl wrote:
>>>>
>>>>
>>>>On 11/23/2015 09:20 AM, Lukas Slebodnik wrote:
>>>>>On (20/11/15 15:57), Pavel Reichl wrote:
>>>>>>Hello,
>>>>>>
>>>>>>please see attached patch.
>>>>>>
>>>>>>Thanks!
>>>>>
>>>>>>From 5af54ded9173d336113c65ab1e83dec5d1ca77b5 Mon Sep 17 00:00:00
2001
>>>>>>From: Pavel Reichl <preichl(a)redhat.com>
>>>>>>Date: Fri, 20 Nov 2015 09:25:11 -0500
>>>>>>Subject: [PATCH] DDNS: Use nsupdate keywords for all transactions
>>>>>>
>>>>>>Keywords realm and server might be used in fallback attempt to
update
>>>>>>DDNS.
>>>>>>
>>>>>>This patch puts them explicitely to the nsupdate transaction.
>>>>>>
>>>>>>Resolves:
>>>>>>https://fedorahosted.org/sssd/ticket/2872
>>>>>The bug in ticket #2872 is cause by bad design document for ticket
#2495
>>>>>https://fedorahosted.org/sssd/wiki/DesignDocs/DDNSMessagesUpdate
>>>>
>>>>Could you please explain why the design is bad?
>>>>
>>>Because fallback for A records is correct but incorect for AAAA records.
>>>+ testing part should be extended. We try to prevent bad designs in review.
>>>But I was not able to find it on sssd-devel
>>>
>>>>Let me quote Petr Spacek's comment from ticket #2872
>>>>>Let me clarify this: server and realm commands affect nsupdate's
behavior. The workaround which executes nsupdate binary again for each DNS update
transaction forces us to start each transaction without hacks (explicit keyword usage) and
add the keywords again in second attempt for each transaction.
>>>>
>>>>The workaround Petr mentions is
https://fedorahosted.org/sssd/ticket/2783
and was filled several months after the design was finished.
>>>>
>>>I thought that ticket #2783 is related to another ticket #2873
>>>and not to this this one. Is it correct? If not could you explain what is the
>>>difference between #2872 #2873?
>>
>>One is about adding REALM and SERVER to nsupdate message and the other is about
grouping DELETE and UPDATE into one transaction.
>>
>>If you want to discuss difference in more detail lets do it off the list.
>>
>So the ticket #2873 is unrelated and workaround #2783 is also unrelated to
>REALM and SERVER.
>
>Thank you very much for quick explanation.
>This thread is only related to the old ticket #2495
>But could you explain why did you mention unrelated tickets here?
This is related to:
https://fedorahosted.org/sssd/ticket/2872
https://fedorahosted.org/sssd/ticket/2734
As stated in commit message.
I'm not aware to mentioning unrelated tickets. Again if you have problem with
understanding the DDNS related tickets and their relations feel free to discuss it off the
list to save some time.
The unrelated ticket #2783 was mentioned in mail
https://lists.fedorahosted.org/archives/list/sssd-devel%40lists.fedorahos...
But it is clear now. Thank you very much for clear explanation.
>
>
>>>>>So it would be good to keep design document in sync.
>>>>>I could not find review of that desing document on sssd-devel.
>>>>
>>>>https://lists.fedorahosted.org/archives/list/sssd-devel%40lists.fedorahosted.org/thread/WWLJEXNQDGXBZWR6RYEHTHVUFNTBGJHY/#WWLJEXNQDGXBZWR6RYEHTHVUFNTBGJHY
>>>>
>>>>>It might be root of the problem. We might also ask bind maintainer
>>>>>for yet another review.
>>>>>
>>>>>Fortunatelly, this RFE(and also bug) is not in rhel7.2.
>>>>>The patches were pushed only to sssd-1.13.1 but they might be in
another rhel.
>>>>>So it would be good to extend section with how to test.
>>>>>
>>>>>After the review of desing document we can tcontinue with review of
patches.
>>>>
>>>>I agree we could update the document, but I don't agree it's a
prerequisite for reviewing patches.
>>>>
>>>It's not only about updating desing document. If the design document was
not
>>>reviewed then it might contain bugs. Therefore implementation will be buggy
as
>>>well. And it would be good to prevent too many versions of patches.
>>
>>Again there is thread where the design was acked by pspacek and jhrozek.
>>
>and it didn't help. That's the reason why I proposed to review updated design
>by neutral person (bind maintainer or simo ...)
Help with what?
with finding a BUG in design document.
>
>>https://lists.fedorahosted.org/archives/list/sssd-devel%40lists.fedorahosted.org/thread/WWLJEXNQDGXBZWR6RYEHTHVUFNTBGJHY/#WWLJEXNQDGXBZWR6RYEHTHVUFNTBGJHY
>>
>The subject "[PATCH] DYNDNS: Add a new option dyndns_server" explains
>why I was not able to find it.
It was the main feature of the patch.
>
>FYI. It's best practice to send review of design document in separate thread
>and before sending a patch. So it can be easily find in a mail archive.
I continued in the related topic that was between Simo and Jakub and posted there the
solution. It was very relevant thread.
It's obvious what you did after a reading a thread. But it was almost
impossible to find review of desing without that knowlege.
It's much easier to find review of other desing documents.
e.g. (the recent one)
https://fedorahosted.org/sssd/wiki/DesignDocs/SUDOIPASchema
- has title "IPA sudo schema support"
and relared thread
https://lists.fedorahosted.org/archives/list/sssd-devel%40lists.fedorahos...
- has subject "Design Discussion: IPA sudo schema"
Subject of threads with review for design document usually start with string
"Design Discussion:" and contains title from wiki. It's very convenient for
people who do not have a time to read all patches but care about review
of designs (Simo, Dimitri, Martin K. ...)
>
>Would you be so kind to update the design document?
I can update the message for fallback attempt.
It would be also good to update/extend test section.
The curent version is very brief. It might be a reason why we noticed bug so
late.
LS