diff --git a/server/core/config.cc b/server/core/config.cc index 98a8f8fee..b6a1e08ea 100644 --- a/server/core/config.cc +++ b/server/core/config.cc @@ -212,7 +212,7 @@ static int config_get_release_string(char* release); bool config_has_duplicate_sections(const char* config, DUPLICATE_CONTEXT* context); int create_new_service(CONFIG_CONTEXT *obj); int create_new_server(CONFIG_CONTEXT *obj); -int create_new_monitor(CONFIG_CONTEXT *context, CONFIG_CONTEXT *obj, std::set& monitored_servers); +int create_new_monitor(CONFIG_CONTEXT *obj, std::set& monitored_servers); int create_new_listener(CONFIG_CONTEXT *obj); int create_new_filter(CONFIG_CONTEXT *obj); int configure_new_service(CONFIG_CONTEXT *context, CONFIG_CONTEXT *obj); @@ -285,7 +285,7 @@ const MXS_MODULE_PARAM config_monitor_params[] = {CN_BACKEND_WRITE_TIMEOUT, MXS_MODULE_PARAM_COUNT, "2"}, {CN_BACKEND_CONNECT_ATTEMPTS, MXS_MODULE_PARAM_COUNT, "1"}, {CN_DISK_SPACE_THRESHOLD, MXS_MODULE_PARAM_STRING}, - {CN_DISK_SPACE_CHECK_INTERVAL, MXS_MODULE_PARAM_COUNT}, + {CN_DISK_SPACE_CHECK_INTERVAL, MXS_MODULE_PARAM_COUNT, "0"}, {NULL} }; @@ -1144,7 +1144,7 @@ process_config_context(CONFIG_CONTEXT *context) } else if (!strcmp(type, CN_MONITOR)) { - error_count += create_new_monitor(context, obj, monitored_servers); + error_count += create_new_monitor(obj, monitored_servers); } else if (strcmp(type, CN_SERVER) != 0 && strcmp(type, CN_FILTER) != 0) { @@ -3287,239 +3287,74 @@ int configure_new_service(CONFIG_CONTEXT *context, CONFIG_CONTEXT *obj) /** * Create a new monitor - * @param context The complete configuration context - * @param obj Monitor configuration context + * + * @param obj Monitor configuration context * @param monitored_servers Set containing the servers that are already monitored + * * @return Number of errors */ -int create_new_monitor(CONFIG_CONTEXT *context, CONFIG_CONTEXT *obj, std::set& monitored_servers) +int create_new_monitor(CONFIG_CONTEXT *obj, std::set& monitored_servers) { - int error_count = 0; + bool err = false; - char *module = config_get_value(obj->parameters, CN_MODULE); - if (module) + // TODO: Use server list parameter type for this + for (auto&& s: mxs::strtok(config_get_string(obj->parameters, CN_SERVERS), ", \t")) { - if ((obj->element = monitor_create(obj->object, module)) == NULL) + SERVER* server = server_find_by_unique_name(s.c_str()); + + if (!server) { - MXS_ERROR("Failed to create monitor '%s'.", obj->object); - error_count++; + MXS_ERROR("Unable to find server '%s' that is configured in " + "the monitor '%s'.", s.c_str(), obj->object); + err = true; } + else if (monitored_servers.insert(s.c_str()).second == false) + { + MXS_WARNING("Multiple monitors are monitoring server [%s]. " + "This will cause undefined behavior.", s.c_str()); + } + } + + if (err) + { + return 1; + } + + const char* module = config_get_string(obj->parameters, CN_MODULE); + ss_dassert(module); + + if (const MXS_MODULE* mod = get_module(module, MODULE_MONITOR)) + { + config_add_defaults(obj, config_monitor_params); + config_add_defaults(obj, mod->parameters); } else { - obj->element = NULL; - MXS_ERROR("Monitor '%s' is missing the required 'module' parameter.", obj->object); - error_count++; + MXS_ERROR("Unable to load monitor module '%s'.", module); + return 1; } - char *servers = config_get_value(obj->parameters, CN_SERVERS); + MXS_MONITOR* monitor = monitor_create(obj->object, module, obj->parameters); - if (error_count == 0) + if (monitor == NULL) { - MXS_MONITOR* monitor = (MXS_MONITOR*)obj->element; - const MXS_MODULE *mod = get_module(module, MODULE_MONITOR); + MXS_ERROR("Failed to create monitor '%s'.", obj->object); + return 1; + } - if (mod) - { - config_add_defaults(obj, mod->parameters); - monitor_add_parameters(monitor, obj->parameters); - } - else - { - error_count++; - } + obj->element = monitor; - char *interval_str = config_get_value(obj->parameters, CN_MONITOR_INTERVAL); - if (interval_str) - { - char *endptr; - long interval = strtol(interval_str, &endptr, 0); - /* The interval must be >0 because it is used as a divisor. - Perhaps a greater minimum value should be added? */ - if (*endptr == '\0' && interval > 0) - { - monitor_set_interval(monitor, (unsigned long)interval); - } - else - { - MXS_NOTICE("Invalid '%s' parameter for monitor '%s', " - "using default value of %d milliseconds.", - CN_MONITOR_INTERVAL, obj->object, DEFAULT_MONITOR_INTERVAL); - } - } - else - { - MXS_NOTICE("Monitor '%s' is missing the '%s' parameter, " - "using default value of %d milliseconds.", - obj->object, CN_MONITOR_INTERVAL, DEFAULT_MONITOR_INTERVAL); - } + int error_count = 0; - char *journal_age = config_get_value(obj->parameters, CN_JOURNAL_MAX_AGE); - if (journal_age) - { - char *endptr; - long interval = strtol(journal_age, &endptr, 0); + // TODO: Parse this in the configuration + const char* dst = config_get_value(obj->parameters, CN_DISK_SPACE_THRESHOLD); - if (*endptr == '\0' && interval > 0) - { - monitor_set_journal_max_age(monitor, (time_t)interval); - } - else - { - error_count++; - MXS_NOTICE("Invalid '%s' parameter for monitor '%s'", - CN_JOURNAL_MAX_AGE, obj->object); - } - } - else + if (dst) + { + if (!monitor_set_disk_space_threshold(monitor, dst)) { - MXS_NOTICE("Monitor '%s' is missing the '%s' parameter, " - "using default value of %d seconds.", - obj->object, CN_JOURNAL_MAX_AGE, DEFAULT_JOURNAL_MAX_AGE); - } - - char *script_timeout = config_get_value(obj->parameters, CN_SCRIPT_TIMEOUT); - if (script_timeout) - { - char *endptr; - long interval = strtol(script_timeout, &endptr, 0); - - if (*endptr == '\0' && interval > 0) - { - monitor_set_script_timeout(monitor, (uint32_t)interval); - } - else - { - error_count++; - MXS_NOTICE("Invalid '%s' parameter for monitor '%s'", - CN_SCRIPT_TIMEOUT, obj->object); - } - } - else - { - MXS_NOTICE("Monitor '%s' is missing the '%s' parameter, " - "using default value of %d seconds.", - obj->object, CN_SCRIPT_TIMEOUT, DEFAULT_SCRIPT_TIMEOUT); - } - - char *connect_timeout = config_get_value(obj->parameters, CN_BACKEND_CONNECT_TIMEOUT); - if (connect_timeout) - { - if (!monitor_set_network_timeout(monitor, MONITOR_CONNECT_TIMEOUT, - atoi(connect_timeout), CN_BACKEND_CONNECT_TIMEOUT)) - { - MXS_ERROR("Failed to set '%s'", CN_BACKEND_CONNECT_TIMEOUT); - error_count++; - } - } - - char *read_timeout = config_get_value(obj->parameters, CN_BACKEND_READ_TIMEOUT); - if (read_timeout) - { - if (!monitor_set_network_timeout(monitor, MONITOR_READ_TIMEOUT, - atoi(read_timeout), CN_BACKEND_READ_TIMEOUT)) - { - MXS_ERROR("Failed to set '%s'", CN_BACKEND_READ_TIMEOUT); - error_count++; - } - } - - char *write_timeout = config_get_value(obj->parameters, CN_BACKEND_WRITE_TIMEOUT); - if (write_timeout) - { - if (!monitor_set_network_timeout(monitor, MONITOR_WRITE_TIMEOUT, - atoi(write_timeout), CN_BACKEND_WRITE_TIMEOUT)) - { - MXS_ERROR("Failed to set '%s'", CN_BACKEND_WRITE_TIMEOUT); - error_count++; - } - } - - char *connect_attempts = config_get_value(obj->parameters, CN_BACKEND_CONNECT_ATTEMPTS); - if (connect_attempts) - { - if (!monitor_set_network_timeout(monitor, MONITOR_CONNECT_ATTEMPTS, - atoi(connect_attempts), CN_BACKEND_CONNECT_ATTEMPTS)) - { - MXS_ERROR("Failed to set '%s'", CN_BACKEND_CONNECT_ATTEMPTS); - error_count++; - } - } - - const char* disk_space_threshold = config_get_value(obj->parameters, CN_DISK_SPACE_THRESHOLD); - if (disk_space_threshold) - { - if (!monitor_set_disk_space_threshold(monitor, disk_space_threshold)) - { - MXS_ERROR("Invalid value for '%s' for monitor %s: %s", - CN_DISK_SPACE_THRESHOLD, monitor->name, disk_space_threshold); - error_count++; - } - } - - const char* disk_space_check_interval = - config_get_value(obj->parameters, CN_DISK_SPACE_CHECK_INTERVAL); - if (disk_space_check_interval) - { - char* endptr; - long int value = strtoll(disk_space_check_interval, &endptr, 0); - if (*endptr == 0 && value >= 0) - { - monitor->disk_space_check_interval = value; - } - else - { - MXS_ERROR("Invalid value for '%s': %s", - CN_DISK_SPACE_CHECK_INTERVAL, disk_space_check_interval); - ++error_count; - } - } - - if (servers) - { - /* get the servers to monitor */ - char *s, *lasts; - s = strtok_r(servers, ",", &lasts); - while (s) - { - CONFIG_CONTEXT *obj1 = context; - int found = 0; - while (obj1) - { - if (strcmp(trim(s), obj1->object) == 0 && obj->element && obj1->element) - { - found = 1; - if (monitored_servers.insert(obj1->object).second == false) - { - MXS_WARNING("Multiple monitors are monitoring server [%s]. " - "This will cause undefined behavior.", - obj1->object); - } - monitor_add_server(monitor, (SERVER*)obj1->element); - } - obj1 = obj1->next; - } - if (!found) - { - MXS_ERROR("Unable to find server '%s' that is " - "configured in the monitor '%s'.", s, obj->object); - error_count++; - } - - s = strtok_r(NULL, ",", &lasts); - } - } - - char *user = config_get_value(obj->parameters, CN_USER); - char *passwd = config_get_password(obj->parameters); - if (user && passwd) - { - monitor_add_user(monitor, user, passwd); - } - else if (user) - { - MXS_ERROR("Monitor '%s' defines a username but does not define a password.", - obj->object); + MXS_ERROR("Invalid value for '%s' for monitor %s: %s", + CN_DISK_SPACE_THRESHOLD, monitor->name, dst); error_count++; } } diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc index 63f033ec7..9bcc81bbc 100644 --- a/server/core/config_runtime.cc +++ b/server/core/config_runtime.cc @@ -418,30 +418,54 @@ bool runtime_alter_server(SERVER *server, const char *key, const char *value) return valid; } +static const MXS_MODULE_PARAM* get_type_parameters(const char* type) +{ + if (strcmp(type, CN_SERVICE) == 0) + { + return config_service_params; + } + else if (strcmp(type, CN_LISTENER) == 0) + { + return config_listener_params; + } + else if (strcmp(type, CN_MONITOR) == 0) + { + return config_monitor_params; + } + else if (strcmp(type, CN_FILTER) == 0) + { + return config_filter_params; + } + + MXS_NOTICE("Module type with no default parameters used: %s", type); + ss_info_dassert(!true, "Module type with no default parameters used"); + return NULL; +} + /** * @brief Add default parameters to a monitor * * @param monitor Monitor to modify */ -static void add_monitor_defaults(MXS_MONITOR *monitor) +static MXS_CONFIG_PARAMETER* load_defaults(const char* name, const char* module_type, + const char* object_type) { - /** Inject the default module parameters in case we only deleted - * a parameter */ - CONFIG_CONTEXT ctx = {}; - ctx.object = (char*)""; - const MXS_MODULE *mod = get_module(monitor->module_name, MODULE_MONITOR); + MXS_CONFIG_PARAMETER* rval = NULL; + CONFIG_CONTEXT ctx = {(char*)""}; - if (mod) + if (const MXS_MODULE* mod = get_module(name, module_type)) { + config_add_defaults(&ctx, get_type_parameters(object_type)); config_add_defaults(&ctx, mod->parameters); - monitor_add_parameters(monitor, ctx.parameters); - config_parameter_free(ctx.parameters); + rval = ctx.parameters; } else { - MXS_ERROR("Failed to load module '%s'. See previous error messages for more details.", - monitor->module_name); + MXS_ERROR("Failed to load module '%s'. See previous error messages for " + "more details.", name); } + + return rval; } bool runtime_alter_monitor(MXS_MONITOR *monitor, const char *key, const char *value) @@ -975,15 +999,18 @@ bool runtime_create_monitor(const char *name, const char *module) { MXS_DEBUG("Repurposed monitor '%s'", name); } - else if ((monitor = monitor_create(name, module)) == NULL) + else if (MXS_CONFIG_PARAMETER* params = load_defaults(module, MODULE_MONITOR, CN_MONITOR)) { - runtime_error("Could not create monitor '%s' with module '%s'", name, module); + if ((monitor = monitor_create(name, module, params)) == NULL) + { + runtime_error("Could not create monitor '%s' with module '%s'", name, module); + } + + config_parameter_free(params); } if (monitor) { - add_monitor_defaults(monitor); - if (monitor_serialize(monitor)) { MXS_NOTICE("Created monitor '%s'", name); diff --git a/server/core/internal/monitor.h b/server/core/internal/monitor.h index 35d5940af..0d41405d5 100644 --- a/server/core/internal/monitor.h +++ b/server/core/internal/monitor.h @@ -47,7 +47,7 @@ typedef enum MONITOR_CONNECT_ATTEMPTS = 3 } monitor_timeouts_t; -MXS_MONITOR *monitor_create(const char *, const char *); +MXS_MONITOR *monitor_create(const char *, const char *, MXS_CONFIG_PARAMETER* params); void monitor_destroy(MXS_MONITOR *); void monitor_start(MXS_MONITOR *, const MXS_CONFIG_PARAMETER*); diff --git a/server/core/monitor.cc b/server/core/monitor.cc index 86852eed6..6957b78dc 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -99,8 +99,16 @@ static uint64_t all_server_bits = SERVER_RUNNING | SERVER_MAINT | SERVER_MASTER * @param module The module to load * @return The newly created monitor */ -MXS_MONITOR* monitor_create(const char *name, const char *module) +MXS_MONITOR* monitor_create(const char *name, const char *module, MXS_CONFIG_PARAMETER* params) { + MXS_MONITOR_API* api = (MXS_MONITOR_API*)load_module(module, MODULE_MONITOR); + + if (api == NULL) + { + MXS_ERROR("Unable to load monitor module '%s'.", name); + return NULL; + } + char* my_name = MXS_STRDUP(name); char *my_module = MXS_STRDUP(module); MXS_MONITOR *mon = (MXS_MONITOR *)MXS_MALLOC(sizeof(MXS_MONITOR)); @@ -113,37 +121,37 @@ MXS_MONITOR* monitor_create(const char *name, const char *module) return NULL; } - if ((mon->api = (MXS_MONITOR_API*)load_module(module, MODULE_MONITOR)) == NULL) - { - MXS_ERROR("Unable to load monitor module '%s'.", my_name); - MXS_FREE(mon); - MXS_FREE(my_module); - MXS_FREE(my_name); - return NULL; - } + mon->api = api; mon->active = true; mon->state = MONITOR_STATE_ALLOC; mon->name = my_name; mon->module_name = my_module; - mon->instance = NULL; mon->monitored_servers = NULL; - *mon->password = '\0'; - *mon->user = '\0'; - mon->read_timeout = DEFAULT_READ_TIMEOUT; - mon->write_timeout = DEFAULT_WRITE_TIMEOUT; - mon->connect_timeout = DEFAULT_CONNECT_TIMEOUT; - mon->connect_attempts = DEFAULT_CONNECTION_ATTEMPTS; - mon->interval = DEFAULT_MONITOR_INTERVAL; - mon->journal_max_age = DEFAULT_JOURNAL_MAX_AGE; - mon->script_timeout = DEFAULT_SCRIPT_TIMEOUT; - mon->parameters = NULL; + mon->read_timeout = config_get_integer(params, CN_BACKEND_READ_TIMEOUT); + mon->write_timeout = config_get_integer(params, CN_BACKEND_WRITE_TIMEOUT); + mon->connect_timeout = config_get_integer(params, CN_BACKEND_CONNECT_TIMEOUT); + mon->connect_attempts = config_get_integer(params, CN_BACKEND_CONNECT_ATTEMPTS); + mon->interval = config_get_integer(params, CN_MONITOR_INTERVAL); + mon->journal_max_age = config_get_integer(params, CN_JOURNAL_MAX_AGE); + mon->script_timeout = config_get_integer(params, CN_SCRIPT_TIMEOUT); mon->check_maintenance_flag = MAINTENANCE_FLAG_NOCHECK; mon->ticks = 0; + mon->parameters = NULL; memset(mon->journal_hash, 0, sizeof(mon->journal_hash)); mon->disk_space_threshold = NULL; - mon->disk_space_check_interval = 0; + mon->disk_space_check_interval = config_get_integer(params, CN_DISK_SPACE_CHECK_INTERVAL); spinlock_init(&mon->lock); + for (auto&& s: mxs::strtok(config_get_string(params, CN_SERVERS), ", \t")) + { + monitor_add_server(mon, server_find_by_unique_name(s.c_str())); + } + + monitor_add_user(mon, config_get_string(params, CN_USER), + config_get_string(params, CN_PASSWORD)); + + monitor_add_parameters(mon, params); + if ((mon->instance = mon->api->createInstance(mon)) == NULL) { MXS_ERROR("Unable to create monitor instance for '%s', using module '%s'.", @@ -151,6 +159,7 @@ MXS_MONITOR* monitor_create(const char *name, const char *module) MXS_FREE(mon); MXS_FREE(my_module); MXS_FREE(my_name); + return NULL; } spinlock_acquire(&monLock); @@ -338,6 +347,7 @@ monitor_stop_all() */ bool monitor_add_server(MXS_MONITOR *mon, SERVER *server) { + ss_dassert(mon && server); bool rval = false; if (monitor_server_in_use(server)) diff --git a/server/core/test/test_modulecmd.cc b/server/core/test/test_modulecmd.cc index 3f808863b..a8cb869f7 100644 --- a/server/core/test/test_modulecmd.cc +++ b/server/core/test/test_modulecmd.cc @@ -407,7 +407,7 @@ int test_domain_matching(const char* actual_module, /** Create a monitor */ char *libdir = MXS_STRDUP_A("../../modules/monitor/mariadbmon/"); set_libdir(libdir); - monitor_create(name, actual_module); + monitor_create(name, actual_module, NULL); const MODULECMD *cmd;