[SSSD] [PATCH] DNS site support 1st wave - generic SRV lookup plugin

Pavel Březina pbrezina at redhat.com
Wed Mar 27 10:22:49 UTC 2013


On 03/27/2013 09:57 AM, Jakub Hrozek wrote:
> On Mon, Mar 25, 2013 at 05:18:27PM +0100, Jakub Hrozek wrote:
>> On Fri, Mar 22, 2013 at 02:47:31PM +0100, Pavel Březina wrote:
>>> Hi, this patchset takes the current SRV lookup code and converts
>>> it into a plugin. Next waves will add custom plugin for IPA and
>>> AD.
>>>
>>> Patch 1: Introduce new plugin interface.
>>>
>>> Patch 2: First SRV lookup plugin.
>>>
>>> Patch 3: Replaces current code with a plugin code. I originally
>>> wanted to remove resolve_srv_* functions completely and call the
>>> plugin in fo_resolve_service_send() directly but that proved to
>>> be harder and more error sensitive than I expected.
>>>
>>> This got me thinking, we should create a refactoring ticket for
>>> the failover code. It has become quite complex and hard to read
>>> and understand because it lacks a lot of comments. Also many
>>> things there seems very hackish to me.
>>>
>>
>> Feel free to create that ticket, I agree that the current failover
>> code has grown too much..but keep in mind that many of the "hacks"
>> are in fact special cases for all kinds of weird errors.
>>
>> My idea was to decouple the caching logic in the failover to a
>> separate layer. So resolver would always talk to DNS or whatever
>> back end there was, then there would be a resolver cache and
>> finally fail over that would only manage states of servers and
>> ports.
>>
>> Is there anything else that bothers you with the current failover
>> architecture?
>>
>>> Patch 4: Set SRV lookup plugin for all providers. This is done as
>>> a separate patch because it will be reverted once each provider
>>> has its own plugin.
>>>
>>> I also would like to discuss where we can set the plugin in the
>>> future. At the moment we have one failover context for the whole
>>> backend. But the backend can be configured to run several
>>> different providers at the same time. The plugin itself is made
>>> per provider - you have different plugin for IPA and different
>>> for LDAP.
>>>
>>> I think we should have failover context per provider. My idea is
>>> to create a new module constructor e.g. sssm_ipa_init(). This
>>> will be called only once before all other sssm_ipa_*_init()
>>> functions and it will initialize IPA-wise failover context.
>>>
>>
>> In theory there may be a case where we need this functionality,
>> yes. I can't think of any use-case right now, but this may be a
>> better architecture going forward, especially as we are adding more
>> providers like sudo or autofs that are only implemented with some
>> back ends.
>>
>>> We can also use it to initialize sdap_id_ctx instead of calling
>>> sssm_ipa_id_init from other places.
>>
>> Please see comments inline.
>>
>>> From 4391f7f47317f3c5664f577b833ef91d3f8e15e2 Mon Sep 17 00:00:00
>>> 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
>>> <pbrezina at redhat.com> Date: Tue, 19 Mar 2013 15:53:44 +0100
>>> Subject: [PATCH 1/4] DNS sites support - SRV lookup plugin
>>> interface
>>>
>>> https://fedorahosted.org/sssd/ticket/1032
>>>
>>> Introduces two new error codes: - ERR_DNS_SRV_NOT_FOUND -
>>> ERR_DNS_SRV_LOOKUP_ERROR --- src/providers/fail_over.c | 18
>>> +++++++++++++++++ src/providers/fail_over.h | 51
>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>> src/util/util_errors.c    |  2 ++ src/util/util_errors.h    |  2
>>> ++ 4 files changed, 73 insertions(+)
>>>
>>> diff --git a/src/providers/fail_over.c
>>> b/src/providers/fail_over.c index
>>> e7c44174ded773a8e3bb99dc436c45d4e8ca277d..660fe8261aa279735af2b8d9d31596acbad19810
>>> 100644 --- a/src/providers/fail_over.c +++
>>> b/src/providers/fail_over.c @@ -55,6 +55,10 @@ struct fo_ctx {
>>> struct server_common *server_common_list;
>>>
>>> struct fo_options *opts; + +    srv_lookup_plugin_send_t
>>> srv_send_fn; +    srv_lookup_plugin_recv_t srv_recv_fn; +    void
>>> *srv_pvt;
>>
>> The design page we agreed on describes that these callbacks should
>> have been in the resolv context. Any reason to move them to fo_ctx
>> ?
>>
>> The failover should be providing logic for moving between servers
>> and ports and just call "resolve a service". The logic on how to
>> resolve a service should be in the resolver I think. Also see the
>> fo_server_info structure -- this seems exactly like code that
>> belongs to resolver.
>
> We discussed the approach with Pavel a bit and I'd like to gather
> more opinions. I still prefer the basic SRV plugin to be placed in
> resolver directly to keep the current layered architecture:
>
> be_fo -> fo -> resolv
>
> I think it still makes sense because the failover (and the backend
> specific be_fo) not only always resolve, but also act as kind of a
> cache and in many cases would simply return the current server
> because it's still valid.
>
> The plugins, on the other hand, should have no knowledge of
> "servers", "ports" and other failover concepts and go straight to the
> DNS or AD server.
>
> The only thing with the current design[1] that introduced new concept
> to the resolver is that the plugin would return "primary" and
> "secondary" results, while previously the resolver just returned
> results and let the failover sort them out. I think we can either
> just accept this, or make the results returned from the resolver a
> bit more generic to allow storing private data (such as
> primary/backup) in the results.

Storing private data as part of a result - wag.

> But in general I still think the most generic SRV plugin belongs to
> the resolver and the back ends would be allowed to override it with
> some specific plugins of theirs.
>
> Are there any thoughts on placing the plugins to resolver versus
> failover?
>
> [1]
> https://fedorahosted.org/sssd/wiki/DesignDocs/ActiveDirectoryDNSSites

I semi-agree that it doesn't belong to fail over, but I don't want to
put it inside resolver either.

Resolver is something that I see as a separate library inside SSSD tree.
It should not have any knowledge of SSSD what so ever. It is a tevent
wrapper around c-ares and should contain only low level functions for
resolving names and DNS queries.

Plugin
1) returns list of primary and backup server
- it has to, the plugin is the only thing that can categorized servers
2) may or may not communicate with DNS (e.g. AD plugin will send CLDAP
ping)

The resolver part of the plugin does not belong to the fail over. Yes.
But in addition, concept of primary and backup server is something that
should be only known to the fail over. The way how is fail over done
now, I'm inclined to put it in fail over.

Later, we can place the plugin architecture inside a module that knows
about SSSD and is placed above resolver. You said you want to decouple
fail over to fail over -> cache -> resolver. This is nice idea.

The plugin architecture can be put inside the cache, because cache is
responsible for resolving expired uris and _srv_ and it does not really
say that is has to use resolver directly.



More information about the sssd-devel mailing list