Browse Source

[3465] Avoid race conditions when checking if server is down.

There is a new function used in keactrl tests which waits for the server
to shutdown, rather than check if it is down already. This is to avoid
race condition when the process may not completely terminate when we
check that it is.
Marcin Siodelski 10 years ago
parent
commit
54f1da5b66

+ 4 - 7
src/bin/d2/tests/d2_process_tests.sh.in

@@ -211,13 +211,10 @@ shutdown_test() {
         clean_exit 1
     fi
 
-    # Server should have shut down.
-    get_pids ${bin}
-    if [ ${_GET_PIDS_NUM} -ne 0 ]; then
-        printf "ERROR: D2 did not shut down after receiving signal.\n"\
- ${_GET_PIDS_NUM}
-        clean_exit 1
-    fi
+    # Make sure the server is down.
+    wait_for_server_down 5 ${bin}
+    assert_eq 1 ${_WAIT_FOR_SERVER_DOWN} \
+        "Expected wait_for_server_down return %d, returned %d"
 
     test_finish 0
 }

+ 4 - 7
src/bin/dhcp4/tests/dhcp4_process_tests.sh.in

@@ -215,13 +215,10 @@ shutdown_test() {
         clean_exit 1
     fi
 
-    # Server should have shut down.
-    get_pids ${bin}
-    if [ ${_GET_PIDS_NUM} -ne 0 ]; then
-        printf "ERROR: Kea did not shut down after receiving signal.\n"\
- ${_GET_PIDS_NUM}
-        clean_exit 1
-    fi
+    # Make sure the server is down.
+    wait_for_server_down 5 ${bin}
+    assert_eq 1 ${_WAIT_FOR_SERVER_DOWN} \
+        "Expected wait_for_server_down return %d, returned %d"
 
     test_finish 0
 }

+ 4 - 7
src/bin/dhcp6/tests/dhcp6_process_tests.sh.in

@@ -218,13 +218,10 @@ shutdown_test() {
         clean_exit 1
     fi
 
-    # Server should have shut down.
-    get_pids ${bin}
-    if [ ${_GET_PIDS_NUM} -ne 0 ]; then
-        printf "ERROR: Kea did not shut down after receiving signal.\n"\
- ${_GET_PIDS_NUM}
-        clean_exit 1
-    fi
+    # Make sure the server is down.
+    wait_for_server_down 5 ${bin}
+    assert_eq 1 ${_WAIT_FOR_SERVER_DOWN} \
+        "Expected wait_for_server_down return %d, returned %d"
 
     test_finish 0
 }

+ 40 - 86
src/bin/keactrl/tests/keactrl_tests.sh.in

@@ -27,9 +27,10 @@ KEACTRL_CFG_FILE=@abs_top_builddir@/src/bin/keactrl/tests/keactrl_test.conf
 # Path to the Kea log file.
 LOG_FILE=@abs_top_builddir@/src/bin/keactrl/tests/test.log
 # Binaries' names
-kea4_name="b10-dhcp4"
-kea6_name="b10-dhcp6"
-d2_name="b10-dhcp-ddns"
+wildcard_name="b10-dhcp"
+kea4_name="${wildcard_name}4"
+kea6_name="${wildcard_name}6"
+d2_name="${wildcard_name}-ddns"
 # Kea configuration
 config="{
     \"Dhcp4\":
@@ -180,18 +181,10 @@ Expected wait_for_message return %d, returned %d."
         "Timeout waiting for ${d2_name} to shutdown. \
 Expected wait_for_message return %d, returned %d."
 
-    # Make sure that the servers are down.
-    get_pids ${kea4_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${kea4_name} processes running, found %d processes running"
-
-    get_pids ${kea6_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${kea6_name} processes running, found %d processes running"
-
-    get_pids ${d2_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${d2_name} processes running, found %d processes running"
+    # Make sure that all servers are down.
+    wait_for_server_down 5 ${wildcard_name}
+    assert_eq 1 ${_WAIT_FOR_SERVER_DOWN} \
+        "Expected wait_for_server_down return %d, returned %d"
 
     test_finish 0
 }
@@ -292,19 +285,10 @@ Expected wait_for_message return %d, returned %d."
         "Timeout waiting for ${d2_name} to shutdown. \
 Expected wait_for_message return %d, returned %d."
 
-
-    # Make sure that the servers are down.
-    get_pids ${kea4_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${kea4_name} processes running, found %d processes running"
-
-    get_pids ${kea6_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${kea6_name} processes running, found %d processes running"
-
-    get_pids ${d2_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${d2_name} processes running, found %d processes running"
+    # Make sure that all servers are down.
+    wait_for_server_down 5 ${wildcard_name}
+    assert_eq 1 ${_WAIT_FOR_SERVER_DOWN} \
+        "Expected wait_for_server_down return %d, returned %d"
 
     test_finish 0
 }
@@ -383,19 +367,10 @@ Expected wait_for_message return %d, returned %d."
         "Timeout waiting for ${kea4_name} to shutdown. \
 Expected wait_for_message return %d, returned %d."
 
-    # Make sure that the servers are down.
-    get_pids ${kea4_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${kea4_name} processes running, found %d processes running"
-
-    get_pids ${kea6_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${kea6_name} processes running, found %d processes running"
-
-    get_pids ${d2_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${d2_name} processes running, found %d processes running"
-
+    # Make sure that all servers are down.
+    wait_for_server_down 5 ${wildcard_name}
+    assert_eq 1 ${_WAIT_FOR_SERVER_DOWN} \
+        "Expected wait_for_server_down return %d, returned %d"
 
     test_finish 0
 }
@@ -473,18 +448,10 @@ Expected wait_for_message return %d, returned %d."
         "Timeout waiting for ${kea6_name} to shutdown. \
 Expected wait_for_message return %d, returned %d."
 
-    # Make sure that the servers are down.
-    get_pids ${kea4_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${kea4_name} processes running, found %d processes running"
-
-    get_pids ${kea6_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${kea6_name} processes running, found %d processes running"
-
-    get_pids ${d2_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${d2_name} processes running, found %d processes running"
+    # Make sure that all servers are down.
+    wait_for_server_down 5 ${wildcard_name}
+    assert_eq 1 ${_WAIT_FOR_SERVER_DOWN} \
+        "Expected wait_for_server_down return %d, returned %d"
 
     test_finish 0
 }
@@ -630,19 +597,10 @@ Expected wait_for_message return %d, returned %d."
         "Timeout waiting for ${d2_name} to shutdown. \
 Expected wait_for_message return %d, returned %d."
 
-
-    # Make sure that the servers are down.
-    get_pids ${kea4_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${kea4_name} processes running, found %d processes running"
-
-    get_pids ${kea6_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${kea6_name} processes running, found %d processes running"
-
-    get_pids ${d2_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${d2_name} processes running, found %d processes running"
+    # Make sure that all servers are down.
+    wait_for_server_down 5 ${wildcard_name}
+    assert_eq 1 ${_WAIT_FOR_SERVER_DOWN} \
+        "Expected wait_for_server_down return %d, returned %d"
 
     test_finish 0
 }
@@ -793,19 +751,10 @@ Expected wait_for_message return %d, returned %d."
         "Timeout waiting for ${d2_name} to shutdown. \
 Expected wait_for_message return %d, returned %d."
 
-
-    # Make sure that the servers are down.
-    get_pids ${kea4_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${kea4_name} processes running, found %d processes running"
-
-    get_pids ${kea6_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${kea6_name} processes running, found %d processes running"
-
-    get_pids ${d2_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${kea6_name} processes running, found %d processes running"
+    # Make sure that all servers are down.
+    wait_for_server_down 5 ${wildcard_name}
+    assert_eq 1 ${_WAIT_FOR_SERVER_DOWN} \
+        "Expected wait_for_server_down return %d, returned %d"
 
     test_finish 0
 }
@@ -892,6 +841,11 @@ Expected wait_for_message return %d, returned %d."
         "Timeout waiting for ${kea4_name} to shutdown. \
 Expected wait_for_message return %d, returned %d."
 
+    # Make sure that the DHCPv4 server is down.
+    wait_for_server_down 5 ${kea4_name}
+    assert_eq 1 ${_WAIT_FOR_SERVER_DOWN} \
+        "Expected wait_for_server_down return %d, returned %d"
+
     # Make sure DHCPv6 server is still running
     get_pids ${kea6_name}
     assert_eq 1 ${_GET_PIDS_NUM} \
@@ -915,9 +869,9 @@ Expected wait_for_message return %d, returned %d."
 Expected wait_for_message return %d, returned %d."
 
     # Make sure that the DHCPv6 server is down.
-    get_pids ${kea6_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${kea6_name} processes running, found %d processes running"
+    wait_for_server_down 5 ${kea6_name}
+    assert_eq 1 ${_WAIT_FOR_SERVER_DOWN} \
+        "Expected wait_for_server_down return %d, returned %d"
 
     # Make sure D2 server is still running
     get_pids ${d2_name}
@@ -925,7 +879,7 @@ Expected wait_for_message return %d, returned %d."
         "Expected %d ${d2_name} process running, found %d processes running"
 
     # Use keactrl stop to shutdown D2 server.
-    printf "Stopping DHCPv6 server: ${keactrl} stop -s dhcp_ddns -c ${KEACTRL_CFG_FILE}\n"
+    printf "Stopping DHCP DDNS server: ${keactrl} stop -s dhcp_ddns -c ${KEACTRL_CFG_FILE}\n"
     ${keactrl} stop -s dhcp_ddns -c ${KEACTRL_CFG_FILE}
     ret=${?}
     assert_eq 0 ${ret} "Expected keactrl to return %d, returned value was %d."
@@ -937,9 +891,9 @@ Expected wait_for_message return %d, returned %d."
 Expected wait_for_message return %d, returned %d."
 
     # Make sure that the D2 server is down.
-    get_pids ${d2_name}
-    assert_eq 0 ${_GET_PIDS_NUM} \
-        "Expected %d ${kea6_name} processes running, found %d processes running"
+    wait_for_server_down 5 ${d2_name}
+    assert_eq 1 ${_WAIT_FOR_SERVER_DOWN} \
+        "Expected wait_for_server_down return %d, returned %d"
 
     test_finish 0
 }

+ 47 - 2
src/lib/testutils/dhcp_test_lib.sh.in

@@ -19,12 +19,25 @@ KEA_PROCS="b10-dhcp4 b10-dhcp6 b10-dhcp-ddns"
 
 # Prints error message.
 test_lib_error() {
-    printf "ERROR/test_lib: %s\n" "${1}"
+    local s=${1}            # Error message.
+    local no_new_line=${2}  # If specified, the message not terminated with
+                            # new line.
+    printf "ERROR/test_lib: %s" "${s}"
+    if [ -z ${no_new_line} ]; then
+        printf "%s" "\n"
+    fi
+
 }
 
 # Prints info message.
 test_lib_info() {
-    printf "INFO/test_lib: %s\n" "${1}"
+    local s=${1}            # Info message.
+    local no_new_line=${2}  # If specified, the message is not terminated with
+                            # new line.
+    printf "INFO/test_lib: %s" "${s}"
+    if [ -z ${no_new_line} ]; then
+        printf "%s" "\n"
+    fi
 }
 
 ### Assertions ###
@@ -259,6 +272,7 @@ wait_for_kea() {
     esac
     local loops=0 # Loops counter
     _WAIT_FOR_KEA=0
+    test_lib_info "wait_for_kea " "skip-new-line"
     while [ ! -s ${LOG_FILE} ] && [ ${loops} -le ${timeout} ]; do
         printf "."
         sleep 1
@@ -310,6 +324,7 @@ must be a number"
 
     local loops=0          # Number of loops performed so far.
     _WAIT_FOR_MESSAGE=0
+    test_lib_info "wait_for_message ${message}: " "skip-new-line"
     # Check if log file exists and if we reached timeout.
     while [ ${loops} -le ${timeout} ]; do
         printf "."
@@ -328,6 +343,36 @@ must be a number"
     # Timeout.
 }
 
+# Waits for server to be down.
+# Return value:
+#    _WAIT_FOR_SERVER_DOWN: 1 if server is down, 0 if timeout occured and the
+#                             server is still running.
+wait_for_server_down() {
+    local timeout=${1}    # Timeout specified in seconds.
+    local proc_name=${2}  # Server process name.
+
+    case ${timeout} in
+        ''|*[!0-9]*)
+            test_lib_error "argument passed to wait_for_server_down must be a number"
+            clean_exit 1 ;;
+    esac
+    local loops=0 # Loops counter
+    _WAIT_FOR_SERVER_DOWN=0
+    test_lib_info "wait_for_server_down ${proc_name}: " "skip-new-line"
+    while [ ${loops} -le ${timeout} ]; do
+        printf "."
+        get_pids ${proc_name}
+        if [ ${_GET_PIDS_NUM} -eq 0 ]; then
+            printf "\n"
+            _WAIT_FOR_SERVER_DOWN=1
+            return
+        fi
+        sleep 1
+        loops=$( expr $loops + 1 )
+    done
+    printf "\n"
+}
+
 # Sends specified signal to the Kea process.
 send_signal() {
     local sig=${1}       # Signal number.