URL: https://github.com/SSSD/sssd/pull/561
Author: jhrozek
 Title: #561: DYNDNS: Retry also on timeouts
Action: opened

PR body:
"""
There is the dyndns_server option that is supposed to make it possible for
the admin to select a server to update DNS with if the server detected by
nsupdate does not work. The fallback works OK for the case where nsupdate
fails with a non-zero return code, but doesn't work for the case where
nsupdate times out.

This patch extends the retry condition to also fallback to the
dyndns_server directive if nsupdate return ERR_DYNDNS_TIMEOUT.
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/561/head:pr561
git checkout pr561
From db0ac156f34b24fa04e1543defcc514befbe4515 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Mon, 30 Apr 2018 11:16:25 +0200
Subject: [PATCH 1/2] DYNDNS: Move the retry logic into a separate function

Let's not repeat ourselves

Related to:
https://pagure.io/SSSD/sssd/issue/3725
---
 src/providers/ldap/sdap_dyndns.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/providers/ldap/sdap_dyndns.c b/src/providers/ldap/sdap_dyndns.c
index 9d28b5758..f791ba9f3 100644
--- a/src/providers/ldap/sdap_dyndns.c
+++ b/src/providers/ldap/sdap_dyndns.c
@@ -79,6 +79,16 @@ static struct sss_iface_addr*
 sdap_get_address_to_delete(struct sss_iface_addr *address_it,
                            uint8_t remove_af);
 
+static bool should_retry(int child_status)
+{
+    if (WIFEXITED(child_status)
+            && WEXITSTATUS(child_status) != 0) {
+        return true;
+    }
+
+    return false;
+}
+
 struct tevent_req *
 sdap_dyndns_update_send(TALLOC_CTX *mem_ctx,
                         struct tevent_context *ev,
@@ -371,8 +381,7 @@ sdap_dyndns_update_done(struct tevent_req *subreq)
     if (ret != EOK) {
         /* If the update didn't succeed, we can retry using the server name */
         if (state->fallback_mode == false
-                && WIFEXITED(child_status)
-                && WEXITSTATUS(child_status) != 0) {
+                && should_retry(child_status)) {
             state->fallback_mode = true;
             DEBUG(SSSDBG_MINOR_FAILURE,
                   "nsupdate failed, retrying.\n");
@@ -514,8 +523,7 @@ sdap_dyndns_update_ptr_done(struct tevent_req *subreq)
     if (ret != EOK) {
         /* If the update didn't succeed, we can retry using the server name */
         if (state->fallback_mode == false
-                && WIFEXITED(child_status)
-                && WEXITSTATUS(child_status) != 0) {
+                && should_retry(child_status)) {
             state->fallback_mode = true;
             DEBUG(SSSDBG_MINOR_FAILURE, "nsupdate failed, retrying\n");
             ret = sdap_dyndns_update_ptr_step(req);

From 05c139b47a469d9e6cf9424d31dec84dac0888a2 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Mon, 30 Apr 2018 11:18:49 +0200
Subject: [PATCH 2/2] DYNDNS: Retry also on timeouts

There is the dyndns_server option that is supposed to make it possible
for the admin to select a server to update DNS with if the server
detected by nsupdate does not work. The fallback works OK for the case
where nsupdate fails with a non-zero return code, but doesn't work
for the case where nsupdate times out.

This patch extends the retry condition to also fallback to the
dyndns_server directive if nsupdate return ERR_DYNDNS_TIMEOUT.

Resolves:
https://pagure.io/SSSD/sssd/issue/3725
---
 src/providers/ldap/sdap_dyndns.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/providers/ldap/sdap_dyndns.c b/src/providers/ldap/sdap_dyndns.c
index f791ba9f3..20d97ca41 100644
--- a/src/providers/ldap/sdap_dyndns.c
+++ b/src/providers/ldap/sdap_dyndns.c
@@ -79,10 +79,10 @@ static struct sss_iface_addr*
 sdap_get_address_to_delete(struct sss_iface_addr *address_it,
                            uint8_t remove_af);
 
-static bool should_retry(int child_status)
+static bool should_retry(int nsupdate_ret, int child_status)
 {
-    if (WIFEXITED(child_status)
-            && WEXITSTATUS(child_status) != 0) {
+    if ((WIFEXITED(child_status) && WEXITSTATUS(child_status) != 0)
+         || nsupdate_ret == ERR_DYNDNS_TIMEOUT) {
         return true;
     }
 
@@ -381,7 +381,7 @@ sdap_dyndns_update_done(struct tevent_req *subreq)
     if (ret != EOK) {
         /* If the update didn't succeed, we can retry using the server name */
         if (state->fallback_mode == false
-                && should_retry(child_status)) {
+                && should_retry(ret, child_status)) {
             state->fallback_mode = true;
             DEBUG(SSSDBG_MINOR_FAILURE,
                   "nsupdate failed, retrying.\n");
@@ -523,7 +523,7 @@ sdap_dyndns_update_ptr_done(struct tevent_req *subreq)
     if (ret != EOK) {
         /* If the update didn't succeed, we can retry using the server name */
         if (state->fallback_mode == false
-                && should_retry(child_status)) {
+                && should_retry(ret, child_status)) {
             state->fallback_mode = true;
             DEBUG(SSSDBG_MINOR_FAILURE, "nsupdate failed, retrying\n");
             ret = sdap_dyndns_update_ptr_step(req);
_______________________________________________
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