On 11/23/2015 03:47 PM, Lukas Slebodnik wrote:
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.
Happy to help.
>>
>>
>>>>>> 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.fedorahos...
>>>>>
>>>>>> 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.
Was there? I don't think so.
>>
>>>
https://lists.fedorahosted.org/archives/list/sssd-devel%40lists.fedorahos...
>>>
>> 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. ...)
Sure it would be easier for you, but thread was a bit stalled at the moment when design
was written so it helped to pass the document there so context of discussion was obvious.
So I think it was better for relevant people in this case Jakub and
Simo. It's always a trade off.
>>
>> 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.
It was not a bug at the moment when document was written. It became a bug when we had to
call nsupdate for each transaction separately - before that it was rather optimization.
>
> LS
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
>