[SSSD] [PATCH] AD: fall back to LDAP if GC is not available.

Lukas Slebodnik lslebodn at redhat.com
Fri Oct 25 12:46:31 UTC 2013


On (25/10/13 11:54), Sumit Bose wrote:
>On Thu, Oct 24, 2013 at 11:54:30AM +0200, Lukas Slebodnik wrote:
>> ehlo,
>> 
>> Global catalog port was ignored from SRV record, hard coded value was used
>> every time.
>> 1st patch should fix it.
>
>good catch, but see my comment below.
>
>> 
>> 2nd patch add fall back to LDAP if GC is not available.
>> with this patch SSSD will not go offline if GC is not available,
>> but it may happen that SSSD will try to connect to GC often.
>> I am not sure, but we can add new option do disable GC lookup completely.
>
>The patch is working as expected, so ACK. I still can see some debug
>messages saying that SSD will go offline before the fallback to LDAP
>happens, but I think this is aceptable.
>
I forgot to move this debug message inside the if statement.
and debug message has high debug level SSSDBG_CRIT_FAILURE.
It make sense.

>I don't think that we need an option to disable the GC lookup
>completely. Since we are doing SRV lookups to find the GC server there
>should be always one wokring GC. If not, there is either a firewall
>issue or the AD domain has other problems.
>
Thank you for review.

>bye,
>Sumit
>
>> 
>> LS
>
>> From abf7cfeaa9826fb52e9a18d3bd5812003c38f118 Mon Sep 17 00:00:00 2001
>> From: Lukas Slebodnik <lslebodn at redhat.com>
>> Date: Thu, 24 Oct 2013 11:03:32 +0200
>> Subject: [PATCH 1/3] AD: Prefer GC port from SRV record
>> 
>> We had a hard coded value of Global Catalog port (3268).
>> Informations from SRV record was ignored.
>> This patch prefer port number from SRV record and hard coded value is used only
>> as a fall back if port number was not initialized.
>> ---
>>  src/providers/ad/ad_common.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>> 
>> diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c
>> index ab62d64a943a726724698d1b7eb9f15bc40b389e..1c4bd088b30eed4f71a787ce4c3bd60366b72def 100644
>> --- a/src/providers/ad/ad_common.c
>> +++ b/src/providers/ad/ad_common.c
>> @@ -729,12 +729,15 @@ ad_resolve_callback(void *private_data, struct fo_server *server)
>>      talloc_zfree(service->gc->uri);
>>      talloc_zfree(service->gc->sockaddr);
>>      if (sdata && sdata->gc) {
>> +        int new_port = fo_get_server_port(server);
>
>Please move the declaration to the top of the function (see
>http://www.freeipa.org/page/Coding_Style)
>
Fixed.
(I thought that declaration at the beginning of block is allowed.)

>> +        new_port = (new_port == 0) ? AD_GC_PORT : new_port;
>> +
>>          service->gc->uri = talloc_asprintf(service->gc, "%s:%d",
>> -                                           new_uri, AD_GC_PORT);
>> +                                           new_uri, new_port);
>>  
>>          service->gc->sockaddr = resolv_get_sockaddr_address(service->gc,
>>                                                              srvaddr,
>> -                                                            AD_GC_PORT);
>> +                                                            new_port);
>>      } else {
>>          /* Make sure there always is an URI even if we know that this
>>           * server doesn't support GC. That way the lookup would go through
>> -- 
>> 1.8.3.1
>> 

Updated patches are attached.

LS
-------------- next part --------------
>From ef2b08ba838a9984a5e9b86c0ffb13a3230bafe6 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Thu, 24 Oct 2013 11:03:32 +0200
Subject: [PATCH 1/2] AD: Prefer GC port from SRV record

We had a hard coded value of Global Catalog port (3268).
Informations from SRV record was ignored.
This patch prefer port number from SRV record and hard coded value is used only
as a fall back if port number was not initialized.
---
 src/providers/ad/ad_common.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c
index ab62d64a943a726724698d1b7eb9f15bc40b389e..f679c11ad18078b454b778ef30e40cca716412cb 100644
--- a/src/providers/ad/ad_common.c
+++ b/src/providers/ad/ad_common.c
@@ -661,6 +661,7 @@ ad_resolve_callback(void *private_data, struct fo_server *server)
     char *address;
     const char *safe_address;
     char *new_uri;
+    int new_port;
     const char *srv_name;
     struct ad_server_data *sdata = NULL;
 
@@ -729,12 +730,15 @@ ad_resolve_callback(void *private_data, struct fo_server *server)
     talloc_zfree(service->gc->uri);
     talloc_zfree(service->gc->sockaddr);
     if (sdata && sdata->gc) {
+        new_port = fo_get_server_port(server);
+        new_port = (new_port == 0) ? AD_GC_PORT : new_port;
+
         service->gc->uri = talloc_asprintf(service->gc, "%s:%d",
-                                           new_uri, AD_GC_PORT);
+                                           new_uri, new_port);
 
         service->gc->sockaddr = resolv_get_sockaddr_address(service->gc,
                                                             srvaddr,
-                                                            AD_GC_PORT);
+                                                            new_port);
     } else {
         /* Make sure there always is an URI even if we know that this
          * server doesn't support GC. That way the lookup would go through
-- 
1.8.3.1

-------------- next part --------------
>From 6fc83bee1832c712c1a27b1ec726ccaabc9a52d0 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Wed, 23 Oct 2013 18:05:58 +0200
Subject: [PATCH 2/2] AD: fall back to LDAP if GC is not available.

AD provider went offline if the Global Catalog could not be connected although
there was also the LDAP port available. With this patch, AD provider will
fall back to the LDAP port before going offline.

New boolean flag ignore_mark_offline was added to structure  sdap_id_conn_ctx
If this flag is enabled function be_mark_offline will not be called.

Resolves:
https://fedorahosted.org/sssd/ticket/2104
---
 src/providers/ad/ad_id.c         | 11 +++++++++++
 src/providers/ldap/ldap_common.h |  2 ++
 src/providers/ldap/sdap_id_op.c  | 15 +++++++++++----
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/src/providers/ad/ad_id.c b/src/providers/ad/ad_id.c
index f09b9c6fe239b9b602f53e8afce641a4d07be6e9..20f9c23fa8d1d272f91c0b886bd1e2e3a2522320 100644
--- a/src/providers/ad/ad_id.c
+++ b/src/providers/ad/ad_id.c
@@ -118,6 +118,14 @@ ad_handle_acct_info_done(struct tevent_req *subreq)
                                             struct ad_handle_acct_info_state);
 
     ret = sdap_handle_acct_req_recv(subreq, &dp_error, &err, &sdap_err);
+    if (dp_error == DP_ERR_OFFLINE
+        && state->conn[state->cindex]->ignore_mark_offline) {
+         /* This is a special case: GC does not work.
+          *  We need to Fall back to ldap
+          */
+        ret = EOK;
+        sdap_err = ENOENT;
+    }
     talloc_zfree(subreq);
     if (ret != EOK) {
         tevent_req_error(req, ret);
@@ -192,9 +200,12 @@ get_conn_list(struct be_req *breq, struct ad_id_ctx *ad_ctx,
         /* Always try GC first */
         clist[0] = ad_ctx->gc_ctx;
         if (IS_SUBDOMAIN(dom) == true) {
+            clist[0]->ignore_mark_offline = false;
             /* Subdomain users are only present in GC. */
             break;
         }
+        /* fall back to ldap if gc is not available */
+        clist[0]->ignore_mark_offline = true;
 
         /* With root domain users we have the option to
          * fall back to LDAP in case ie POSIX attributes
diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h
index 0d565fc63b7e949e628ae5cf079e54c134464e77..fb9a34c6067521f8d28fb403b97ca6394918f303 100644
--- a/src/providers/ldap/ldap_common.h
+++ b/src/providers/ldap/ldap_common.h
@@ -52,6 +52,8 @@ struct sdap_id_conn_ctx {
     struct sdap_id_conn_cache *conn_cache;
     /* dlinklist pointers */
     struct sdap_id_conn_ctx *prev, *next;
+    /* do not go offline, try another connection */
+    bool ignore_mark_offline;
 };
 
 struct sdap_id_ctx {
diff --git a/src/providers/ldap/sdap_id_op.c b/src/providers/ldap/sdap_id_op.c
index 52cf78569d7176bd054b6cbeabf772721c9a1c98..5e166e19fa1c8a8f342803b9ed45cc4f50bbb2ab 100644
--- a/src/providers/ldap/sdap_id_op.c
+++ b/src/providers/ldap/sdap_id_op.c
@@ -553,10 +553,17 @@ static void sdap_id_op_connect_done(struct tevent_req *subreq)
     }
 
     if (ret != EOK && !can_retry) {
-        /* be is going offline as there is no more servers to try */
-        DEBUG(1, ("Failed to connect, going offline (%d [%s])\n",
-                  ret, strerror(ret)));
-        be_mark_offline(conn_cache->id_conn->id_ctx->be);
+        if (conn_cache->id_conn->ignore_mark_offline) {
+            DEBUG(SSSDBG_TRACE_FUNC,
+                  ("Failed to connect to server, but ignore mark offline "
+                   "is enabled.\n"));
+        } else {
+            /* be is going offline as there is no more servers to try */
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  ("Failed to connect, going offline (%d [%s])\n",
+                   ret, strerror(ret)));
+            be_mark_offline(conn_cache->id_conn->id_ctx->be);
+        }
         is_offline = true;
     }
 
-- 
1.8.3.1



More information about the sssd-devel mailing list