From f7fefad2e68d23d512a4edcbab678a8358d307be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Tue, 25 Apr 2017 15:51:55 +0300 Subject: [PATCH] Improve persisted configuration handling When a persisted configuration file is read, the values in it are considered to be more up-to-date than the ones in the main configuration file. This allows all objects to be persisted in a more complete form making it easier to change configuration values at runtime. This change is intended to help make runtime alterations to services possible. --- server/core/config.cc | 26 +++++++++- server/core/config_runtime.cc | 17 +++--- server/core/maxscale/config.h | 10 ++++ server/core/maxscale/monitor.h | 16 ------ server/core/maxscale/service.h | 7 +-- server/core/monitor.cc | 94 +++++----------------------------- server/core/service.cc | 63 ++++++++++++++++++++--- 7 files changed, 116 insertions(+), 117 deletions(-) diff --git a/server/core/config.cc b/server/core/config.cc index 09dec74b1..9b3712fbd 100644 --- a/server/core/config.cc +++ b/server/core/config.cc @@ -474,7 +474,14 @@ ini_handler(void *userdata, const char *section, const char *name, const char *v if (config_get_param(ptr->parameters, name)) { - if (!config_append_param(ptr, name, value)) + /** The values in the persisted configurations are updated versions of + * the ones in the main configuration file. */ + if (is_persisted_config && !config_replace_param(ptr, name, value)) + { + return 0; + } + /** Multi-line parameter */ + else if (!config_append_param(ptr, name, value)) { return 0; } @@ -2424,6 +2431,23 @@ bool config_append_param(CONFIG_CONTEXT* obj, const char* key, const char* value return rval; } +bool config_replace_param(CONFIG_CONTEXT* obj, const char* key, const char* value) +{ + MXS_CONFIG_PARAMETER *param = config_get_param(obj->parameters, key); + ss_dassert(param); + char *new_value = MXS_STRDUP(value); + bool rval; + + if (new_value) + { + MXS_FREE(param->value); + param->value = new_value; + rval = true; + } + + return rval; +} + MXS_CONFIG* config_get_global_options() { return &gateway; diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc index 9e6170653..93aa62d81 100644 --- a/server/core/config_runtime.cc +++ b/server/core/config_runtime.cc @@ -49,7 +49,7 @@ bool runtime_link_server(SERVER *server, const char *target) { if (serviceAddBackend(service, server)) { - service_serialize_servers(service); + service_serialize(service); rval = true; } } @@ -57,7 +57,7 @@ bool runtime_link_server(SERVER *server, const char *target) { if (monitorAddServer(monitor, server)) { - monitor_serialize_servers(monitor); + monitor_serialize(monitor); rval = true; } } @@ -87,12 +87,12 @@ bool runtime_unlink_server(SERVER *server, const char *target) if (service) { serviceRemoveBackend(service, server); - service_serialize_servers(service); + service_serialize(service); } else if (monitor) { monitorRemoveServer(monitor, server); - monitor_serialize_servers(monitor); + monitor_serialize(monitor); } const char *type = service ? "service" : "monitor"; @@ -103,7 +103,6 @@ bool runtime_unlink_server(SERVER *server, const char *target) return rval; } - bool runtime_create_server(const char *name, const char *address, const char *port, const char *protocol, const char *authenticator, const char *authenticator_options) @@ -804,7 +803,7 @@ SERVER* runtime_create_server_from_json(json_t* json) /** The port needs to be in string format */ char port[200]; // Enough to store any port value int i = json_integer_value(json_object_get(json, CN_PORT)); - snprintf(port, sizeof (port), "%d", i); + snprintf(port, sizeof(port), "%d", i); /** Optional parameters */ const char* protocol = string_or_null(json, CN_PROTOCOL); @@ -870,7 +869,8 @@ bool runtime_alter_server_from_json(SERVER* server, json_t* new_json) if (server_to_object_relations(server, old_json.get(), new_json)) { json_t* parameters = json_object_get(new_json, CN_PARAMETERS); - json_t* old_parameters = json_object_get(old_json.get(), CN_PARAMETERS);; + json_t* old_parameters = json_object_get(old_json.get(), CN_PARAMETERS); + ss_dassert(old_parameters); if (parameters) @@ -1023,7 +1023,8 @@ bool runtime_alter_monitor_from_json(MXS_MONITOR* monitor, json_t* new_json) { bool changed = false; json_t* parameters = json_object_get(new_json, CN_PARAMETERS); - json_t* old_parameters = json_object_get(old_json.get(), CN_PARAMETERS);; + json_t* old_parameters = json_object_get(old_json.get(), CN_PARAMETERS); + ss_dassert(old_parameters); if (parameters) diff --git a/server/core/maxscale/config.h b/server/core/maxscale/config.h index 7d56cf9fe..8c86d9a39 100644 --- a/server/core/maxscale/config.h +++ b/server/core/maxscale/config.h @@ -92,6 +92,16 @@ bool config_add_param(CONFIG_CONTEXT* obj, const char* key, const char* value); */ bool config_append_param(CONFIG_CONTEXT* obj, const char* key, const char* value); +/** + * @brief Replace an existing parameter + * + * @param obj Configuration context + * @param key Parameter name + * @param value Parameter value + * @return True on success, false on memory allocation error + */ +bool config_replace_param(CONFIG_CONTEXT* obj, const char* key, const char* value); + /** * @brief Construct an SSL structure * diff --git a/server/core/maxscale/monitor.h b/server/core/maxscale/monitor.h index f74e84b14..938c03d5d 100644 --- a/server/core/maxscale/monitor.h +++ b/server/core/maxscale/monitor.h @@ -65,22 +65,6 @@ bool monitorRemoveParameter(MXS_MONITOR *monitor, const char *key); void monitorSetInterval (MXS_MONITOR *, unsigned long); bool monitorSetNetworkTimeout(MXS_MONITOR *, int, int); -/** - * @brief Serialize the servers of a monitor to a file - * - * This partially converts @c monitor into an INI format file. Only the servers - * of the monitor are serialized. This allows the monitor to keep monitoring - * the servers that were added at runtime even after a restart. - * - * NOTE: This does not persist the complete monitor configuration and requires - * that an existing monitor configuration is in the main configuration file. - * Changes to monitor parameters are not persisted. - * - * @param monitor Monitor to serialize - * @return False if the serialization of the monitor fails, true if it was successful - */ -bool monitor_serialize_servers(const MXS_MONITOR *monitor); - /** * @brief Serialize a monitor to a file * diff --git a/server/core/maxscale/service.h b/server/core/maxscale/service.h index f8288e2bb..b8347fb1b 100644 --- a/server/core/maxscale/service.h +++ b/server/core/maxscale/service.h @@ -82,18 +82,15 @@ void serviceRemoveBackend(SERVICE *service, const SERVER *server); /** * @brief Serialize a service to a file * - * This partially converts @c service into an INI format file. Only the servers - * of the service are serialized. This allows the service to keep using the servers - * added at runtime even after a restart. + * This converts @c service into an INI format file. * * NOTE: This does not persist the complete service configuration and requires * that an existing service configuration is in the main configuration file. - * Changes to service parameters are not persisted. * * @param service Service to serialize * @return False if the serialization of the service fails, true if it was successful */ -bool service_serialize_servers(const SERVICE *service); +bool service_serialize(const SERVICE *service); /** * Internal utility functions diff --git a/server/core/monitor.cc b/server/core/monitor.cc index bd8ff70da..3c58b3577 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -1273,51 +1273,6 @@ MXS_MONITOR* monitor_server_in_use(const SERVER *server) return rval; } -/** - * Creates a monitor configuration at the location pointed by @c filename - * - * @param monitor Monitor to serialize into a configuration - * @param filename Filename where configuration is written - * @return True on success, false on error - */ -static bool create_monitor_server_config(const MXS_MONITOR *monitor, const char *filename) -{ - int file = open(filename, O_EXCL | O_CREAT | O_WRONLY, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); - - if (file == -1) - { - MXS_ERROR("Failed to open file '%s' when serializing monitor '%s': %d, %s", - filename, monitor->name, errno, mxs_strerror(errno)); - return false; - } - - /** - * Only additional parameters are added to the configuration. This prevents - * duplication or addition of parameters that don't support it. - * - * TODO: Check for return values on all of the dprintf calls - */ - dprintf(file, "[%s]\n", monitor->name); - - if (monitor->databases) - { - dprintf(file, "%s=", CN_SERVERS); - for (MXS_MONITOR_SERVERS *db = monitor->databases; db; db = db->next) - { - if (db != monitor->databases) - { - dprintf(file, ","); - } - dprintf(file, "%s", db->server->unique_name); - } - dprintf(file, "\n"); - } - - close(file); - - return true; -} - static bool create_monitor_config(const MXS_MONITOR *monitor, const char *filename) { int file = open(filename, O_EXCL | O_CREAT | O_WRONLY, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); @@ -1346,46 +1301,25 @@ static bool create_monitor_config(const MXS_MONITOR *monitor, const char *filena dprintf(file, "%s=%d\n", CN_BACKEND_READ_TIMEOUT, monitor->read_timeout); dprintf(file, "%s=%d\n", CN_BACKEND_CONNECT_ATTEMPTS, monitor->connect_attempts); + if (monitor->databases) + { + dprintf(file, "%s=", CN_SERVERS); + for (MXS_MONITOR_SERVERS *db = monitor->databases; db; db = db->next) + { + if (db != monitor->databases) + { + dprintf(file, ","); + } + dprintf(file, "%s", db->server->unique_name); + } + dprintf(file, "\n"); + } + close(file); return true; } -bool monitor_serialize_servers(const MXS_MONITOR *monitor) -{ - bool rval = false; - char filename[PATH_MAX]; - snprintf(filename, sizeof(filename), "%s/%s.cnf.tmp", get_config_persistdir(), - monitor->name); - - if (unlink(filename) == -1 && errno != ENOENT) - { - MXS_ERROR("Failed to remove temporary monitor configuration at '%s': %d, %s", - filename, errno, mxs_strerror(errno)); - } - else if (create_monitor_server_config(monitor, filename)) - { - char final_filename[PATH_MAX]; - strcpy(final_filename, filename); - - char *dot = strrchr(final_filename, '.'); - ss_dassert(dot); - *dot = '\0'; - - if (rename(filename, final_filename) == 0) - { - rval = true; - } - else - { - MXS_ERROR("Failed to rename temporary monitor configuration at '%s': %d, %s", - filename, errno, mxs_strerror(errno)); - } - } - - return rval; -} - bool monitor_serialize(const MXS_MONITOR *monitor) { bool rval = false; diff --git a/server/core/service.cc b/server/core/service.cc index 6ae14b8a1..2d137e81d 100644 --- a/server/core/service.cc +++ b/server/core/service.cc @@ -2223,22 +2223,36 @@ static bool create_service_config(const SERVICE *service, const char *filename) } /** - * Only additional parameters are added to the configuration. This prevents - * duplication or addition of parameters that don't support it. - * * TODO: Check for return values on all of the dprintf calls */ dprintf(file, "[%s]\n", service->name); + dprintf(file, "%s=service\n", CN_TYPE); + dprintf(file, "%s=%s\n", CN_USER, service->credentials.name); + dprintf(file, "%s=%s\n", CN_PASSWORD, service->credentials.authdata); + dprintf(file, "%s=%s\n", CN_ENABLE_ROOT_USER, service->enable_root ? "true" : "false"); + dprintf(file, "%s=%d\n", CN_MAX_RETRY_INTERVAL, service->max_retry_interval); + dprintf(file, "%s=%d\n", CN_MAX_CONNECTIONS, service->max_connections); + dprintf(file, "%s=%ld\n", CN_CONNECTION_TIMEOUT, service->conn_idle_timeout); + dprintf(file, "%s=%s\n", CN_AUTH_ALL_SERVERS, service->users_from_all ? "true" : "false"); + dprintf(file, "%s=%s\n", CN_STRIP_DB_ESC, service->strip_db_esc ? "true" : "false"); + dprintf(file, "%s=%s\n", CN_LOCALHOST_MATCH_WILDCARD_HOST, service->localhost_match_wildcard_host ? "true" : "false"); + dprintf(file, "%s=%s\n", CN_VERSION_STRING, service->version_string); + dprintf(file, "%s=%s\n", CN_WEIGHTBY, service->weightby); + dprintf(file, "%s=%s\n", CN_LOG_AUTH_WARNINGS, service->log_auth_warnings ? "true" : "false"); + dprintf(file, "%s=%s\n", CN_RETRY_ON_FAILURE, service->retry_start ? "true" : "false"); + if (service->dbref) { - dprintf(file, "servers="); + dprintf(file, "%s=", CN_SERVERS); + const char *sep = ""; + for (SERVER_REF *db = service->dbref; db; db = db->next) { - if (db != service->dbref) + if (SERVER_REF_IS_ACTIVE(db)) { - dprintf(file, ","); + dprintf(file, "%s%s", sep, db->server->unique_name); + sep = ","; } - dprintf(file, "%s", db->server->unique_name); } dprintf(file, "\n"); } @@ -2248,6 +2262,41 @@ static bool create_service_config(const SERVICE *service, const char *filename) return true; } +bool service_serialize(const SERVICE *service) +{ + bool rval = false; + char filename[PATH_MAX]; + snprintf(filename, sizeof(filename), "%s/%s.cnf.tmp", get_config_persistdir(), + service->name); + + if (unlink(filename) == -1 && errno != ENOENT) + { + MXS_ERROR("Failed to remove temporary service configuration at '%s': %d, %s", + filename, errno, mxs_strerror(errno)); + } + else if (create_service_config(service, filename)) + { + char final_filename[PATH_MAX]; + strcpy(final_filename, filename); + + char *dot = strrchr(final_filename, '.'); + ss_dassert(dot); + *dot = '\0'; + + if (rename(filename, final_filename) == 0) + { + rval = true; + } + else + { + MXS_ERROR("Failed to rename temporary service configuration at '%s': %d, %s", + filename, errno, mxs_strerror(errno)); + } + } + + return rval; +} + bool service_serialize_servers(const SERVICE *service) { bool rval = false;