From d7fcb3c5a16dce188728b0c2ab3a2a0df9bb5e2a Mon Sep 17 00:00:00 2001 From: Binbin Date: Fri, 4 Feb 2022 17:39:51 +0800 Subject: [PATCH] Fix SENTINEL SET config rewrite test (#10232) Change the sentinel config file to a directory in SENTINEL SET test. So it will now fail on the `rename` in `rewriteConfigOverwriteFile`. The test used to set the sentinel config file permissions to `000` to simulate failure. But it fails on centos7 / freebsd / alpine. (introduced in #10151) Other changes: 1. More error messages after the config rewrite failure. 2. Modify arg name `force_all` in `rewriteConfig` to `force_write`. (was rename in #9304) 3. Fix a typo in debug quicklist-packed-threshold, then -> than. (#9357) --- src/debug.c | 4 ++-- src/sentinel.c | 2 +- src/server.c | 2 +- src/server.h | 2 +- tests/sentinel/tests/03-runtime-reconf.tcl | 13 +++++++++---- 5 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/debug.c b/src/debug.c index 2da2c5d50..146460646 100644 --- a/src/debug.c +++ b/src/debug.c @@ -825,7 +825,7 @@ NULL int memerr; unsigned long long sz = memtoull((const char *)c->argv[2]->ptr, &memerr); if (memerr || !quicklistisSetPackedThreshold(sz)) { - addReplyError(c, "argument must be a memory value bigger then 1 and smaller than 4gb"); + addReplyError(c, "argument must be a memory value bigger than 1 and smaller than 4gb"); } else { addReply(c,shared.ok); } @@ -926,7 +926,7 @@ NULL } else if (!strcasecmp(c->argv[1]->ptr,"config-rewrite-force-all") && c->argc == 2) { if (rewriteConfig(server.configfile, 1) == -1) - addReplyError(c, "CONFIG-REWRITE-FORCE-ALL failed"); + addReplyErrorFormat(c, "CONFIG-REWRITE-FORCE-ALL failed: %s", strerror(errno)); else addReply(c, shared.ok); } else if(!strcasecmp(c->argv[1]->ptr,"client-eviction") && c->argc == 2) { diff --git a/src/sentinel.c b/src/sentinel.c index 60cba2e1b..937a5b86f 100644 --- a/src/sentinel.c +++ b/src/sentinel.c @@ -2286,7 +2286,7 @@ werr: */ static void sentinelFlushConfigAndReply(client *c) { if (sentinelFlushConfig() == C_ERR) - addReplyErrorFormat(c,"Failed to save config file"); + addReplyError(c, "Failed to save config file. Check server logs."); else addReply(c, shared.ok); } diff --git a/src/server.c b/src/server.c index 565cd9467..55c28885f 100644 --- a/src/server.c +++ b/src/server.c @@ -1891,7 +1891,7 @@ int restartServer(int flags, mstime_t delay) { rewriteConfig(server.configfile, 0) == -1) { serverLog(LL_WARNING,"Can't restart: configuration rewrite process " - "failed"); + "failed: %s", strerror(errno)); return C_ERR; } diff --git a/src/server.h b/src/server.h index 485c051fd..90cb99ad8 100644 --- a/src/server.h +++ b/src/server.h @@ -2932,7 +2932,7 @@ void resetServerSaveParams(void); struct rewriteConfigState; /* Forward declaration to export API. */ void rewriteConfigRewriteLine(struct rewriteConfigState *state, const char *option, sds line, int force); void rewriteConfigMarkAsProcessed(struct rewriteConfigState *state, const char *option); -int rewriteConfig(char *path, int force_all); +int rewriteConfig(char *path, int force_write); void initConfigValues(); sds getConfigDebugInfo(); int allowProtectedAction(int config, client *c); diff --git a/tests/sentinel/tests/03-runtime-reconf.tcl b/tests/sentinel/tests/03-runtime-reconf.tcl index 5d6e6b1a5..3e930646a 100644 --- a/tests/sentinel/tests/03-runtime-reconf.tcl +++ b/tests/sentinel/tests/03-runtime-reconf.tcl @@ -51,10 +51,15 @@ test "Sentinel Set with other error situations" { set update_quorum [expr $origin_quorum+1] set sentinel_id 0 set configfilename [file join "sentinel_$sentinel_id" "sentinel.conf"] - exec chmod 000 $configfilename + set configfilename_bak [file join "sentinel_$sentinel_id" "sentinel.conf.bak"] + + file rename $configfilename $configfilename_bak + file mkdir $configfilename catch {[S 0 SENTINEL SET mymaster quorum $update_quorum]} err - exec chmod 644 $configfilename - assert_equal "ERR Failed to save config file" $err -} + file delete $configfilename + file rename $configfilename_bak $configfilename + + assert_match "ERR Failed to save config file*" $err +}