URL: https://github.com/SSSD/sssd/pull/29
Author: jhrozek
 Title: #29: tests: Add a regression test for upstream ticket #3131
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/29/head:pr29
git checkout pr29
From e9b1ee2540b7f2c69494c0434340c3ed709235e3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Wed, 24 Aug 2016 14:21:12 +0200
Subject: [PATCH 1/2] failover: proceed normally when no new server is found

Multiple failover requests come in same time, the first one will
result in collapsing the meta server but multiple resolution of
SRV records are triggered. The first one finishes normally but the
others won't find any new server thus ends with an error.

This patch makes failover to proceed normally even in such case.

Resolves:
https://fedorahosted.org/sssd/ticket/3131
---
 src/providers/fail_over.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/src/providers/fail_over.c b/src/providers/fail_over.c
index 8ab39f2..7708409 100644
--- a/src/providers/fail_over.c
+++ b/src/providers/fail_over.c
@@ -1112,7 +1112,9 @@ fo_resolve_service_cont(struct tevent_req *subreq)
     ret = resolve_srv_recv(subreq, &state->server);
     talloc_zfree(subreq);
 
-    if (ret) {
+    /* We will proceed normally on ERR_SRV_DUPLICATES and if the server
+     * is already being resolved, we hook to that request. */
+    if (ret != EOK && ret != ERR_SRV_DUPLICATES) {
         tevent_req_error(req, ret);
         return;
     }
@@ -1398,11 +1400,23 @@ resolve_srv_done(struct tevent_req *subreq)
         }
 
         if (last_server == state->meta) {
-            /* SRV lookup returned only those servers
-             * that are already present. */
+            /* SRV lookup returned only those servers that are already present.
+             * This may happen only when an ongoing SRV resolution already
+             * exist. We will return server, but won't set any state. */
             DEBUG(SSSDBG_TRACE_FUNC, "SRV lookup did not return "
                                       "any new server.\n");
             ret = ERR_SRV_DUPLICATES;
+
+            /* Since no new server is returned, state->meta->next is NULL.
+             * We return last tried server if possible which is server
+             * from previous resolution of SRV record, and first server
+             * otherwise. */
+            if (state->service->last_tried_server != NULL) {
+                state->out = state->service->last_tried_server;
+                goto done;
+            }
+
+            state->out = state->service->server_list;
             goto done;
         }
 
@@ -1438,7 +1452,10 @@ resolve_srv_done(struct tevent_req *subreq)
     }
 
 done:
-    if (ret != EOK) {
+    if (ret == ERR_SRV_DUPLICATES) {
+        tevent_req_error(req, ret);
+        return;
+    } else if (ret != EOK) {
         state->out = state->meta;
         set_srv_data_status(state->meta->srv_data, SRV_RESOLVE_ERROR);
         tevent_req_error(req, ret);

From 9fefcc1a3b8eca504f99e058188f9e28e35165e3 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Wed, 21 Sep 2016 10:44:36 +0200
Subject: [PATCH 2/2] tests: Add a regression test for upstream ticket #3131

Tests that running two duplicate SRV resolution queries succeeds
and returns a valid host name.
---
 src/tests/cmocka/test_fo_srv.c | 66 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/src/tests/cmocka/test_fo_srv.c b/src/tests/cmocka/test_fo_srv.c
index a84ce43..197f8de 100644
--- a/src/tests/cmocka/test_fo_srv.c
+++ b/src/tests/cmocka/test_fo_srv.c
@@ -203,6 +203,8 @@ struct test_fo_ctx {
     int ttl;
 
     struct fo_server *srv;
+
+    int num_done;
 };
 
 int test_fo_srv_data_cmp(void *ud1, void *ud2)
@@ -691,6 +693,67 @@ static void test_fo_hostlist(void **state)
     assert_int_equal(ret, ERR_OK);
 }
 
+static void test_fo_srv_dup_done(struct tevent_req *req);
+
+/* Test that running two parallel SRV queries doesn't return an error.
+ * This is a regression test for https://fedorahosted.org/sssd/ticket/3131
+ */
+void test_fo_srv_duplicates(void **state)
+{
+    errno_t ret;
+    struct tevent_req *req;
+    struct test_fo_ctx *test_ctx =
+        talloc_get_type(*state, struct test_fo_ctx);
+
+    test_fo_srv_mock_dns(test_ctx, test_ctx->ttl);
+    test_fo_srv_mock_dns(test_ctx, test_ctx->ttl);
+
+    ret = fo_add_srv_server(test_ctx->fo_svc, "_ldap", "sssd.com",
+                            "sssd.local", "tcp", test_ctx);
+    assert_int_equal(ret, ERR_OK);
+
+    ret = fo_add_server(test_ctx->fo_svc, "ldap1.sssd.com",
+                        389, (void *) discard_const("ldap://ldap1.sssd.com";),
+                        true);
+    assert_int_equal(ret, ERR_OK);
+
+    req = fo_resolve_service_send(test_ctx, test_ctx->ctx->ev,
+                                  test_ctx->resolv, test_ctx->fo_ctx,
+                                  test_ctx->fo_svc);
+    assert_non_null(req);
+    tevent_req_set_callback(req, test_fo_srv_dup_done, test_ctx);
+
+    req = fo_resolve_service_send(test_ctx, test_ctx->ctx->ev,
+                                  test_ctx->resolv, test_ctx->fo_ctx,
+                                  test_ctx->fo_svc);
+    assert_non_null(req);
+    tevent_req_set_callback(req, test_fo_srv_dup_done, test_ctx);
+
+    ret = test_ev_loop(test_ctx->ctx);
+    assert_int_equal(ret, ERR_OK);
+}
+
+static void test_fo_srv_dup_done(struct tevent_req *req)
+{
+    struct test_fo_ctx *test_ctx = \
+        tevent_req_callback_data(req, struct test_fo_ctx);
+    errno_t ret;
+    const char *name;
+
+    ret = fo_resolve_service_recv(req, test_ctx, &test_ctx->srv);
+    talloc_zfree(req);
+    assert_int_equal(ret, EOK);
+
+    name = fo_get_server_name(test_ctx->srv);
+    assert_string_equal(name, "ldap1.sssd.com");
+
+    test_ctx->num_done++;
+    if (test_ctx->num_done == 2) {
+        test_ctx->ctx->error = ERR_OK;
+        test_ctx->ctx->done = true;
+    }
+}
+
 int main(int argc, const char *argv[])
 {
     int rv;
@@ -715,6 +778,9 @@ int main(int argc, const char *argv[])
         cmocka_unit_test_setup_teardown(test_fo_srv_ttl_zero,
                                         test_fo_srv_setup,
                                         test_fo_srv_teardown),
+        cmocka_unit_test_setup_teardown(test_fo_srv_duplicates,
+                                        test_fo_srv_setup,
+                                        test_fo_srv_teardown),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Reply via email to