Tests: validate CONFIG REWRITE for all params. (#7764)

This is a catch-all test to confirm that that rewrite produces a valid
output for all parameters and that this process does not introduce
undesired configuration changes.
This commit is contained in:
Yossi Gottlieb 2020-09-09 15:43:11 +03:00 committed by GitHub
parent 1461f02deb
commit a8b7268911
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 43 additions and 6 deletions

View File

@ -1064,6 +1064,8 @@ struct rewriteConfigState {
sds *lines; /* Current lines as an array of sds strings */ sds *lines; /* Current lines as an array of sds strings */
int has_tail; /* True if we already added directives that were int has_tail; /* True if we already added directives that were
not present in the original config file. */ not present in the original config file. */
int force_all; /* True if we want all keywords to be force
written. Currently only used for testing. */
}; };
/* Append the new line to the current configuration state. */ /* Append the new line to the current configuration state. */
@ -1110,6 +1112,7 @@ struct rewriteConfigState *rewriteConfigReadOldFile(char *path) {
state->numlines = 0; state->numlines = 0;
state->lines = NULL; state->lines = NULL;
state->has_tail = 0; state->has_tail = 0;
state->force_all = 0;
if (fp == NULL) return state; if (fp == NULL) return state;
/* Read the old file line by line, populate the state. */ /* Read the old file line by line, populate the state. */
@ -1188,7 +1191,7 @@ void rewriteConfigRewriteLine(struct rewriteConfigState *state, const char *opti
rewriteConfigMarkAsProcessed(state,option); rewriteConfigMarkAsProcessed(state,option);
if (!l && !force) { if (!l && !force && !state->force_all) {
/* Option not used previously, and we are not forced to use it. */ /* Option not used previously, and we are not forced to use it. */
sdsfree(line); sdsfree(line);
sdsfree(o); sdsfree(o);
@ -1612,15 +1615,18 @@ cleanup:
* *
* Configuration parameters that are at their default value, unless already * Configuration parameters that are at their default value, unless already
* explicitly included in the old configuration file, are not rewritten. * explicitly included in the old configuration file, are not rewritten.
* The force_all flag overrides this behavior and forces everything to be
* written. This is currently only used for testing purposes.
* *
* On error -1 is returned and errno is set accordingly, otherwise 0. */ * On error -1 is returned and errno is set accordingly, otherwise 0. */
int rewriteConfig(char *path) { int rewriteConfig(char *path, int force_all) {
struct rewriteConfigState *state; struct rewriteConfigState *state;
sds newcontent; sds newcontent;
int retval; int retval;
/* Step 1: read the old config into our rewrite state. */ /* Step 1: read the old config into our rewrite state. */
if ((state = rewriteConfigReadOldFile(path)) == NULL) return -1; if ((state = rewriteConfigReadOldFile(path)) == NULL) return -1;
if (force_all) state->force_all = 1;
/* Step 2: rewrite every single option, replacing or appending it inside /* Step 2: rewrite every single option, replacing or appending it inside
* the rewrite state. */ * the rewrite state. */
@ -2427,7 +2433,7 @@ NULL
addReplyError(c,"The server is running without a config file"); addReplyError(c,"The server is running without a config file");
return; return;
} }
if (rewriteConfig(server.configfile) == -1) { if (rewriteConfig(server.configfile, 0) == -1) {
serverLog(LL_WARNING,"CONFIG REWRITE failed: %s", strerror(errno)); serverLog(LL_WARNING,"CONFIG REWRITE failed: %s", strerror(errno));
addReplyErrorFormat(c,"Rewriting config file: %s", strerror(errno)); addReplyErrorFormat(c,"Rewriting config file: %s", strerror(errno));
} else { } else {

View File

@ -408,6 +408,7 @@ void debugCommand(client *c) {
"STRUCTSIZE -- Return the size of different Redis core C structures.", "STRUCTSIZE -- Return the size of different Redis core C structures.",
"ZIPLIST <key> -- Show low level info about the ziplist encoding.", "ZIPLIST <key> -- Show low level info about the ziplist encoding.",
"STRINGMATCH-TEST -- Run a fuzz tester against the stringmatchlen() function.", "STRINGMATCH-TEST -- Run a fuzz tester against the stringmatchlen() function.",
"CONFIG-REWRITE-FORCE-ALL -- Like CONFIG REWRITE but writes all configuration options, including keywords not listed in original configuration file or default values.",
#ifdef USE_JEMALLOC #ifdef USE_JEMALLOC
"MALLCTL <key> [<val>] -- Get or set a malloc tunning integer.", "MALLCTL <key> [<val>] -- Get or set a malloc tunning integer.",
"MALLCTL-STR <key> [<val>] -- Get or set a malloc tunning string.", "MALLCTL-STR <key> [<val>] -- Get or set a malloc tunning string.",
@ -805,6 +806,12 @@ NULL
{ {
stringmatchlen_fuzz_test(); stringmatchlen_fuzz_test();
addReplyStatus(c,"Apparently Redis did not crash: test passed"); addReplyStatus(c,"Apparently Redis did not crash: test passed");
} 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");
else
addReply(c, shared.ok);
#ifdef USE_JEMALLOC #ifdef USE_JEMALLOC
} else if(!strcasecmp(c->argv[1]->ptr,"mallctl") && c->argc >= 3) { } else if(!strcasecmp(c->argv[1]->ptr,"mallctl") && c->argc >= 3) {
mallctl_int(c, c->argv+2, c->argc-2); mallctl_int(c, c->argv+2, c->argc-2);

View File

@ -1954,7 +1954,7 @@ void sentinelFlushConfig(void) {
int rewrite_status; int rewrite_status;
server.hz = CONFIG_DEFAULT_HZ; server.hz = CONFIG_DEFAULT_HZ;
rewrite_status = rewriteConfig(server.configfile); rewrite_status = rewriteConfig(server.configfile, 0);
server.hz = saved_hz; server.hz = saved_hz;
if (rewrite_status == -1) goto werr; if (rewrite_status == -1) goto werr;

View File

@ -2552,7 +2552,7 @@ int restartServer(int flags, mstime_t delay) {
/* Config rewriting. */ /* Config rewriting. */
if (flags & RESTART_SERVER_CONFIG_REWRITE && if (flags & RESTART_SERVER_CONFIG_REWRITE &&
server.configfile && server.configfile &&
rewriteConfig(server.configfile) == -1) rewriteConfig(server.configfile, 0) == -1)
{ {
serverLog(LL_WARNING,"Can't restart: configuration rewrite process " serverLog(LL_WARNING,"Can't restart: configuration rewrite process "
"failed"); "failed");

View File

@ -2101,7 +2101,7 @@ void appendServerSaveParams(time_t seconds, int changes);
void resetServerSaveParams(void); void resetServerSaveParams(void);
struct rewriteConfigState; /* Forward declaration to export API. */ struct rewriteConfigState; /* Forward declaration to export API. */
void rewriteConfigRewriteLine(struct rewriteConfigState *state, const char *option, sds line, int force); void rewriteConfigRewriteLine(struct rewriteConfigState *state, const char *option, sds line, int force);
int rewriteConfig(char *path); int rewriteConfig(char *path, int force_all);
void initConfigValues(); void initConfigValues();
/* db.c -- Keyspace access API */ /* db.c -- Keyspace access API */

View File

@ -147,4 +147,28 @@ start_server {tags {"introspection"}} {
} }
} }
# Do a force-all config rewrite and make sure we're able to parse
# it.
test {CONFIG REWRITE sanity} {
# Capture state of config before
set configs {}
foreach {k v} [r config get *] {
dict set configs $k $v
}
# Rewrite entire configuration, restart and confirm the
# server is able to parse it and start.
assert_equal [r debug config-rewrite-force-all] "OK"
restart_server 0 0
assert_equal [r ping] "PONG"
# Verify no changes were introduced
dict for {k v} $configs {
assert_equal $v [lindex [r config get $k] 1]
}
}
# Config file at this point is at a wierd state, and includes all
# known keywords. Might be a good idea to avoid adding tests here.
} }