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

Jakub Hrozek jhrozek at redhat.com
Wed Mar 27 08:57:38 UTC 2013


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.

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



More information about the sssd-devel mailing list