* [tarantool-patches] [PATCH v3 0/4] Syslog destination @ 2018-07-24 17:11 Olga Arkhangelskaia 2018-07-24 17:11 ` [tarantool-patches] [PATCH v3 1/4] Configurable syslog destination Olga Arkhangelskaia ` (3 more replies) 0 siblings, 4 replies; 8+ messages in thread From: Olga Arkhangelskaia @ 2018-07-24 17:11 UTC (permalink / raw) To: tarantool-patches; +Cc: Olga Arkhangelskaia This series adds support of configurable destionation for syslog. The option is called server. Possible options are ip4 and unix socket: syslog:server=unix:/path/to/socket,identity=myinstance syslog:server=ip4:port,identity=tarantool_myinstance If server option is not set, but syslog is used in log configuration - default sockets for syslogd are used: /dev/log or /var/run/syslog. Olga Arkhangelskaia (4): Configurable syslog destination box-tap: test valid log configuration box-tap: syslog destination test unix socket box-tap:syslog remote destination test src/say.c | 107 +++++++++++++++++++++++++++++++++++++++++++--- src/say.h | 13 +++++- test/box-tap/cfg.test.lua | 85 +++++++++++++++++++++++++++++++++++- 3 files changed, 197 insertions(+), 8 deletions(-) -- https://github.com/tarantool/tarantool/issues/3487 https://github.com/tarantool/tarantool/tree/OKriw/gh-3487-syslog-conf-dest v1: https://www.freelists.org/post/tarantool-patches/PATCH-03-Syslog-destination v2: https://www.freelists.org/post/tarantool-patches/PATCH-v2-03-Syslog-destination Changes in v2: - Changed tests. Use random port, no sever is started, used current dir - Changed basic structs - now we have emum for syslog dst, reuse log->pathi Changes in v3: - moved test to box-tap - used pcall in case of stopped/unconfigured syslogd - added syslog check syslog configuration test 2.14.3 (Apple Git-98) ^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] [PATCH v3 1/4] Configurable syslog destination 2018-07-24 17:11 [tarantool-patches] [PATCH v3 0/4] Syslog destination Olga Arkhangelskaia @ 2018-07-24 17:11 ` Olga Arkhangelskaia 2018-07-25 16:27 ` Vladimir Davydov 2018-07-24 17:11 ` [tarantool-patches] [PATCH v3 2/4] box-tap: test valid log configuration Olga Arkhangelskaia ` (2 subsequent siblings) 3 siblings, 1 reply; 8+ messages in thread From: Olga Arkhangelskaia @ 2018-07-24 17:11 UTC (permalink / raw) To: tarantool-patches; +Cc: Olga Arkhangelskaia Added server option to syslog configuration. Server option is responsible for log destination. At the momemt there is two ways of usage:server=unix:/path/to/socket or server=ipv4:port. If port is not set default udp port 514 is used. If logging to syslog is set, however there is no sever options - default location is used: Linux /dev/log and Mac /var/run/syslog. Closes #3487 --- src/say.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- src/say.h | 13 +++++++- 2 files changed, 113 insertions(+), 7 deletions(-) diff --git a/src/say.c b/src/say.c index ac221dd19..b4836d9ee 100644 --- a/src/say.c +++ b/src/say.c @@ -36,6 +36,7 @@ #include <stdarg.h> #include <stdio.h> #include <string.h> +#include <netdb.h> #include <sys/types.h> #include <unistd.h> #include <fcntl.h> @@ -473,16 +474,86 @@ syslog_connect_unix(const char *path) return fd; } +/** + * Connect to remote syslogd using server:port. + * @param ip:port address string of remote host. + * @retval not 0 Socket descriptor. + * @retval -1 Socket error. + */ +static int +syslog_connect_remote(const char *server_address) +{ + struct addrinfo *inf, hints, *ptr; + const char *remote; + char *portnum, *copy; + int fd = -1; + + copy = strdup(server_address); + if (copy == NULL) { + diag_set(OutOfMemory, strlen(server_address), "malloc", + "stslog server address"); + return fd; + } + portnum = copy; + remote = strsep(&portnum, ":"); + if (remote == NULL) { + remote = strdup("514"); + if (remote == NULL) { + diag_set(OutOfMemory, strlen(remote), "strdup", + "port"); + free(copy); + return fd; + } + } + memset(&hints, 0, sizeof(hints)); + hints.ai_family = AF_INET; + hints.ai_socktype = SOCK_DGRAM; + hints.ai_protocol = IPPROTO_UDP; + hints.ai_flags = 0; + + if (getaddrinfo(remote, portnum, NULL, &inf) < 0) { + free(copy); + return -1; + } + for (ptr = inf; ptr; ptr = ptr->ai_next) { + fd = socket(ptr->ai_family, ptr->ai_socktype, ptr->ai_protocol); + if (fd < 0) { + continue; + } + + if (connect(fd, inf->ai_addr, inf->ai_addrlen) != 0) { + close(fd); + fd = -1; + continue; + } + break; + } + freeaddrinfo(inf); + free(copy); + return fd; +} + static inline int log_syslog_connect(struct log *log) { + /* - * Try two locations: '/dev/log' for Linux and + * If server option is not set use '/dev/log' for Linux and * '/var/run/syslog' for Mac. */ - log->fd = syslog_connect_unix("/dev/log"); - if (log->fd < 0) - log->fd = syslog_connect_unix("/var/run/syslog"); + switch (log->server_type) { + case SAY_SYSLOG_UNIX: + log->fd = syslog_connect_unix(log->path); + break; + case SAY_SYSLOG_REMOTE: + log->fd = syslog_connect_remote(log->path); + break; + default: + log->fd = syslog_connect_unix("/dev/log"); + if (log->fd < 0) + log->fd = syslog_connect_unix("/var/run/syslog"); + + } return log->fd; } @@ -498,6 +569,17 @@ log_syslog_init(struct log *log, const char *init_str) if (say_parse_syslog_opts(init_str, &opts) < 0) return -1; + log->server_type = opts.syslog_server; + if (log->server_type == SAY_SYSLOG_DEFAULT) + log->path = NULL; + else { + log->path = strdup(opts.server); + if (log->path == NULL) { + diag_set(OutOfMemory, strlen(opts.server), + "malloc", "server address"); + return -1; + } + } if (opts.identity == NULL) log->syslog_ident = strdup("tarantool"); else @@ -1031,6 +1113,8 @@ say_syslog_facility_by_name(const char *facility) int say_parse_syslog_opts(const char *init_str, struct say_syslog_opts *opts) { + opts->server = NULL; + opts->syslog_server = SAY_SYSLOG_DEFAULT; opts->identity = NULL; opts->facility = syslog_facility_MAX; opts->copy = strdup(init_str); @@ -1038,7 +1122,7 @@ say_parse_syslog_opts(const char *init_str, struct say_syslog_opts *opts) diag_set(OutOfMemory, strlen(init_str), "malloc", "opts->copy"); return -1; } - char *ptr = opts->copy; + char *ptr = opts->copy; const char *option, *value; /* strsep() overwrites the separator with '\0' */ @@ -1047,7 +1131,18 @@ say_parse_syslog_opts(const char *init_str, struct say_syslog_opts *opts) break; value = option; - if (say_parse_prefix(&value, "identity=")) { + if (say_parse_prefix(&value, "server=")) { + if (opts->server != NULL || + opts->syslog_server != SAY_SYSLOG_DEFAULT) + goto duplicate; + if (say_parse_prefix(&value, "unix:")) { + opts->syslog_server = SAY_SYSLOG_UNIX; + opts->server = value; + } else { + opts->syslog_server = SAY_SYSLOG_REMOTE; + opts->server = value; + } + } else if (say_parse_prefix(&value, "identity=")) { if (opts->identity != NULL) goto duplicate; opts->identity = value; diff --git a/src/say.h b/src/say.h index 2c2395fe0..6681aae98 100644 --- a/src/say.h +++ b/src/say.h @@ -79,6 +79,12 @@ say_log_level_is_enabled(int level) extern enum say_format log_format; +enum say_logger_syslog_server { + SAY_SYSLOG_DEFAULT, + SAY_SYSLOG_UNIX, + SAY_SYSLOG_REMOTE +}; + enum say_logger_type { /** * Before the app server core is initialized, we do not @@ -141,7 +147,10 @@ struct log { /** The current log level. */ int level; enum say_logger_type type; - /** path to file if logging to file. */ + enum say_logger_syslog_server server_type; + /** Path to file if logging to file, socket + * or server address in case of syslog. + */ char *path; bool nonblock; log_format_func_t format_func; @@ -384,6 +393,8 @@ say_parse_logger_type(const char **str, enum say_logger_type *type); /** Syslog logger initialization params */ struct say_syslog_opts { + enum say_logger_syslog_server syslog_server; + const char *server; const char *identity; enum syslog_facility facility; /* Input copy (content unspecified). */ -- 2.14.3 (Apple Git-98) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [tarantool-patches] [PATCH v3 1/4] Configurable syslog destination 2018-07-24 17:11 ` [tarantool-patches] [PATCH v3 1/4] Configurable syslog destination Olga Arkhangelskaia @ 2018-07-25 16:27 ` Vladimir Davydov 0 siblings, 0 replies; 8+ messages in thread From: Vladimir Davydov @ 2018-07-25 16:27 UTC (permalink / raw) To: Olga Arkhangelskaia; +Cc: tarantool-patches On Tue, Jul 24, 2018 at 08:11:49PM +0300, Olga Arkhangelskaia wrote: > Added server option to syslog configuration. > Server option is responsible for log destination. At the momemt > there is two ways of usage:server=unix:/path/to/socket or > server=ipv4:port. If port is not set default udp port 514 is used. > If logging to syslog is set, however there is no sever options - > default location is used: Linux /dev/log and Mac /var/run/syslog. > > Closes #3487 > --- > src/say.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- > src/say.h | 13 +++++++- > 2 files changed, 113 insertions(+), 7 deletions(-) > > diff --git a/src/say.c b/src/say.c > index ac221dd19..b4836d9ee 100644 > --- a/src/say.c > +++ b/src/say.c > @@ -36,6 +36,7 @@ > #include <stdarg.h> > #include <stdio.h> > #include <string.h> > +#include <netdb.h> > #include <sys/types.h> > #include <unistd.h> > #include <fcntl.h> > @@ -473,16 +474,86 @@ syslog_connect_unix(const char *path) > return fd; > } > > +/** > + * Connect to remote syslogd using server:port. > + * @param ip:port address string of remote host. This function now takes a single argument, 'server_address'. Please fix the comment. > + * @retval not 0 Socket descriptor. > + * @retval -1 Socket error. > + */ > +static int > +syslog_connect_remote(const char *server_address) > +{ > + struct addrinfo *inf, hints, *ptr; > + const char *remote; > + char *portnum, *copy; > + int fd = -1; > + > + copy = strdup(server_address); > + if (copy == NULL) { > + diag_set(OutOfMemory, strlen(server_address), "malloc", > + "stslog server address"); > + return fd; > + } > + portnum = copy; > + remote = strsep(&portnum, ":"); > + if (remote == NULL) { > + remote = strdup("514"); The default port should be defined as a numeric constant and converted to a string here with snprintf(). Also, shouldn't you use 'portnum' instead of 'remote' here? Also, it looks like you leak 'remote' in this function. Why don't you use a buffer on stack for storing port number? > + if (remote == NULL) { > + diag_set(OutOfMemory, strlen(remote), "strdup", > + "port"); > + free(copy); > + return fd; Please use labels to free 'copy' (like 'goto out', 'out' points to the 'free(copy); return fd;' at the end of this function). That would look neater IMHO. > + } > + } > + memset(&hints, 0, sizeof(hints)); > + hints.ai_family = AF_INET; > + hints.ai_socktype = SOCK_DGRAM; > + hints.ai_protocol = IPPROTO_UDP; > + hints.ai_flags = 0; Nit: this line is pointless as you zero the whole struct anyway. > + > + if (getaddrinfo(remote, portnum, NULL, &inf) < 0) { You should set diag here. Take a look at SystemError. > + free(copy); > + return -1; > + } > + for (ptr = inf; ptr; ptr = ptr->ai_next) { > + fd = socket(ptr->ai_family, ptr->ai_socktype, ptr->ai_protocol); > + if (fd < 0) { ... and here > + continue; > + } > + > + if (connect(fd, inf->ai_addr, inf->ai_addrlen) != 0) { ... and here, too > + close(fd); > + fd = -1; > + continue; > + } > + break; > + } > + freeaddrinfo(inf); > + free(copy); > + return fd; > +} > + > static inline int > log_syslog_connect(struct log *log) > { > + Nit: please don't add an empty line here. > /* > - * Try two locations: '/dev/log' for Linux and > + * If server option is not set use '/dev/log' for Linux and > * '/var/run/syslog' for Mac. > */ > - log->fd = syslog_connect_unix("/dev/log"); > - if (log->fd < 0) > - log->fd = syslog_connect_unix("/var/run/syslog"); > + switch (log->server_type) { > + case SAY_SYSLOG_UNIX: Malformed indentation: 'switch' and 'case' should be at the same level. > + log->fd = syslog_connect_unix(log->path); > + break; > + case SAY_SYSLOG_REMOTE: > + log->fd = syslog_connect_remote(log->path); > + break; > + default: > + log->fd = syslog_connect_unix("/dev/log"); > + if (log->fd < 0) > + log->fd = syslog_connect_unix("/var/run/syslog"); > + > + } > return log->fd; > } > > @@ -498,6 +569,17 @@ log_syslog_init(struct log *log, const char *init_str) > if (say_parse_syslog_opts(init_str, &opts) < 0) > return -1; > > + log->server_type = opts.syslog_server; > + if (log->server_type == SAY_SYSLOG_DEFAULT) > + log->path = NULL; Nit: no need to set 'log->path' to NULL - it's already been done by 'log_create'. > + else { > + log->path = strdup(opts.server); > + if (log->path == NULL) { Malformed indentation. > + diag_set(OutOfMemory, strlen(opts.server), > + "malloc", "server address"); > + return -1; > + } > + } > if (opts.identity == NULL) > log->syslog_ident = strdup("tarantool"); > else > @@ -1031,6 +1113,8 @@ say_syslog_facility_by_name(const char *facility) > int > say_parse_syslog_opts(const char *init_str, struct say_syslog_opts *opts) > { > + opts->server = NULL; > + opts->syslog_server = SAY_SYSLOG_DEFAULT; Hmm, 'server', 'syslog_server', this looks weird... May be, 'server_path' and 'server_type' or simply 'path' and 'type'? > opts->identity = NULL; > opts->facility = syslog_facility_MAX; > opts->copy = strdup(init_str); > @@ -1038,7 +1122,7 @@ say_parse_syslog_opts(const char *init_str, struct say_syslog_opts *opts) > diag_set(OutOfMemory, strlen(init_str), "malloc", "opts->copy"); > return -1; > } > - char *ptr = opts->copy; > + char *ptr = opts->copy; What's this doing? Adding a space at the end of the line? Please remove. > const char *option, *value; > > /* strsep() overwrites the separator with '\0' */ > @@ -1047,7 +1131,18 @@ say_parse_syslog_opts(const char *init_str, struct say_syslog_opts *opts) > break; > > value = option; > - if (say_parse_prefix(&value, "identity=")) { > + if (say_parse_prefix(&value, "server=")) { > + if (opts->server != NULL || > + opts->syslog_server != SAY_SYSLOG_DEFAULT) > + goto duplicate; > + if (say_parse_prefix(&value, "unix:")) { > + opts->syslog_server = SAY_SYSLOG_UNIX; > + opts->server = value; > + } else { > + opts->syslog_server = SAY_SYSLOG_REMOTE; > + opts->server = value; > + } > + } else if (say_parse_prefix(&value, "identity=")) { > if (opts->identity != NULL) > goto duplicate; > opts->identity = value; > diff --git a/src/say.h b/src/say.h > index 2c2395fe0..6681aae98 100644 > --- a/src/say.h > +++ b/src/say.h > @@ -79,6 +79,12 @@ say_log_level_is_enabled(int level) > > extern enum say_format log_format; > > +enum say_logger_syslog_server { This sounds awkward. May be, say_syslog_server or say_syslog_server_type or simply say_syslog_type? Whatever name you choose, please be consistent throughout the code. That is, if you choose to call the enum say_syslog_type, then other names should be log->syslog_type and say_syslog_opts->type. > + SAY_SYSLOG_DEFAULT, > + SAY_SYSLOG_UNIX, > + SAY_SYSLOG_REMOTE > +}; > + > enum say_logger_type { > /** > * Before the app server core is initialized, we do not > @@ -141,7 +147,10 @@ struct log { > /** The current log level. */ > int level; > enum say_logger_type type; > - /** path to file if logging to file. */ > + enum say_logger_syslog_server server_type; Let's prefix this with syslog_, like other syslog-related options (syslog_identity, syslog_facility). > + /** Path to file if logging to file, socket > + * or server address in case of syslog. > + */ Malformed comment style. > char *path; > bool nonblock; > log_format_func_t format_func; > @@ -384,6 +393,8 @@ say_parse_logger_type(const char **str, enum say_logger_type *type); > > /** Syslog logger initialization params */ > struct say_syslog_opts { > + enum say_logger_syslog_server syslog_server; Here you don't need syslog_ prefix, because the struct is already named after syslog. > + const char *server; > const char *identity; > enum syslog_facility facility; > /* Input copy (content unspecified). */ ^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] [PATCH v3 2/4] box-tap: test valid log configuration 2018-07-24 17:11 [tarantool-patches] [PATCH v3 0/4] Syslog destination Olga Arkhangelskaia 2018-07-24 17:11 ` [tarantool-patches] [PATCH v3 1/4] Configurable syslog destination Olga Arkhangelskaia @ 2018-07-24 17:11 ` Olga Arkhangelskaia 2018-07-24 17:11 ` [tarantool-patches] [PATCH v3 3/4] box-tap: syslog destination test unix socket Olga Arkhangelskaia 2018-07-24 17:11 ` [tarantool-patches] [PATCH v3 4/4] box-tap:syslog remote destination test Olga Arkhangelskaia 3 siblings, 0 replies; 8+ messages in thread From: Olga Arkhangelskaia @ 2018-07-24 17:11 UTC (permalink / raw) To: tarantool-patches; +Cc: Olga Arkhangelskaia Test case checks that valid log configuration is not broken, as it happened in #3487 --- test/box-tap/cfg.test.lua | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/box-tap/cfg.test.lua b/test/box-tap/cfg.test.lua index ffafdbe42..67e4c3af9 100755 --- a/test/box-tap/cfg.test.lua +++ b/test/box-tap/cfg.test.lua @@ -6,7 +6,7 @@ local socket = require('socket') local fio = require('fio') local uuid = require('uuid') local msgpack = require('msgpack') -test:plan(95) +test:plan(96) -------------------------------------------------------------------------------- -- Invalid values @@ -464,5 +464,13 @@ code = string.format(code_fmt, dir, instance_uuid, uuid.new()) test:is(run_script(code), PANIC, "replicaset_uuid mismatch") fio.rmdir(dir) +-- +-- Check syslog configuration +-- (syslog daemon may be not started hence use pcall) +-- +code = [[pcall(box.cfg, {log = 'syslog:identity=tarantool'}) +]] +test:is(run_script(code), 0, "syslog log configuration") + test:check() os.exit(0) -- 2.14.3 (Apple Git-98) ^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] [PATCH v3 3/4] box-tap: syslog destination test unix socket 2018-07-24 17:11 [tarantool-patches] [PATCH v3 0/4] Syslog destination Olga Arkhangelskaia 2018-07-24 17:11 ` [tarantool-patches] [PATCH v3 1/4] Configurable syslog destination Olga Arkhangelskaia 2018-07-24 17:11 ` [tarantool-patches] [PATCH v3 2/4] box-tap: test valid log configuration Olga Arkhangelskaia @ 2018-07-24 17:11 ` Olga Arkhangelskaia 2018-07-25 16:40 ` Vladimir Davydov 2018-07-24 17:11 ` [tarantool-patches] [PATCH v3 4/4] box-tap:syslog remote destination test Olga Arkhangelskaia 3 siblings, 1 reply; 8+ messages in thread From: Olga Arkhangelskaia @ 2018-07-24 17:11 UTC (permalink / raw) To: tarantool-patches; +Cc: Olga Arkhangelskaia Adds syslog destination unix test in box-tap. Test redirects logs to newly created unix socket and sets appropriate log level. If log receives the same level - test passes. In case if syslog is not configured we use pcall. Closes #3487 --- test/box-tap/cfg.test.lua | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/test/box-tap/cfg.test.lua b/test/box-tap/cfg.test.lua index 67e4c3af9..d1e595b1c 100755 --- a/test/box-tap/cfg.test.lua +++ b/test/box-tap/cfg.test.lua @@ -6,7 +6,7 @@ local socket = require('socket') local fio = require('fio') local uuid = require('uuid') local msgpack = require('msgpack') -test:plan(96) +test:plan(97) -------------------------------------------------------------------------------- -- Invalid values @@ -472,5 +472,40 @@ code = [[pcall(box.cfg, {log = 'syslog:identity=tarantool'}) ]] test:is(run_script(code), 0, "syslog log configuration") +-- +-- Check syslog socket configuration +-- +code = [[ +local tap = require('tap') +local socket = require('socket') +local os = require('os') +local test = tap.test("syslog_unix") +local log = require('log') +local fio = require('fio') + +path = fio.pathjoin(fio.cwd(), 'log_unix_socket_test.sock') +unix_socket = socket('AF_UNIX', 'SOCK_DGRAM',0) +unix_socket:bind('unix/', path) +socket.tcp_connect('unix', path) + +opt = string.format("syslog:server=unix:%s,identity=tarantool", path) +res = 1 +if pcall (box.cfg, {log = opt, log_level = 5}) then + log.info("Test socket syslog destination") + buf = unix_socket:read("Test socket syslog destination", 30) + if buf ~= nil then + if buf:match('Test socket syslog destination') then res = 0 + else res = 1 end + else + res = 1 + end +end + +unix_socket:close() +os.remove(path) +os.exit(res) +]] +test:is(run_script(code), 0, "unix socket syslog log configuration") + test:check() os.exit(0) -- 2.14.3 (Apple Git-98) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [tarantool-patches] [PATCH v3 3/4] box-tap: syslog destination test unix socket 2018-07-24 17:11 ` [tarantool-patches] [PATCH v3 3/4] box-tap: syslog destination test unix socket Olga Arkhangelskaia @ 2018-07-25 16:40 ` Vladimir Davydov 0 siblings, 0 replies; 8+ messages in thread From: Vladimir Davydov @ 2018-07-25 16:40 UTC (permalink / raw) To: Olga Arkhangelskaia; +Cc: tarantool-patches We fold tests in the patch that implements a feature or fixes a bug whenever possible. Please fold patches 3 and 4 in patch 1. On Tue, Jul 24, 2018 at 08:11:51PM +0300, Olga Arkhangelskaia wrote: > @@ -472,5 +472,40 @@ code = [[pcall(box.cfg, {log = 'syslog:identity=tarantool'}) > ]] > test:is(run_script(code), 0, "syslog log configuration") > > +-- > +-- Check syslog socket configuration > +-- > +code = [[ > +local tap = require('tap') Not needed. > +local socket = require('socket') > +local os = require('os') > +local test = tap.test("syslog_unix") Not needed. > +local log = require('log') > +local fio = require('fio') > + > +path = fio.pathjoin(fio.cwd(), 'log_unix_socket_test.sock') > +unix_socket = socket('AF_UNIX', 'SOCK_DGRAM',0) Nit: a space missing between , and 0. > +unix_socket:bind('unix/', path) > +socket.tcp_connect('unix', path) Why? > + > +opt = string.format("syslog:server=unix:%s,identity=tarantool", path) > +res = 1 > +if pcall (box.cfg, {log = opt, log_level = 5}) then You don't need pcall here - box.cfg() must succeed here. Also, log_level = 5 by default so you don't need to specify it explicitly. > + log.info("Test socket syslog destination") > + buf = unix_socket:read("Test socket syslog destination", 30) > + if buf ~= nil then > + if buf:match('Test socket syslog destination') then res = 0 > + else res = 1 end > + else > + res = 1 > + end This looks over-complicated. Can we rewrite it like shown below? buf = ... if buf and buf:match(...) then res = 0 else res = 1 end > +end > + > +unix_socket:close() > +os.remove(path) > +os.exit(res) > +]] > +test:is(run_script(code), 0, "unix socket syslog log configuration") > + > test:check() > os.exit(0) ^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] [PATCH v3 4/4] box-tap:syslog remote destination test 2018-07-24 17:11 [tarantool-patches] [PATCH v3 0/4] Syslog destination Olga Arkhangelskaia ` (2 preceding siblings ...) 2018-07-24 17:11 ` [tarantool-patches] [PATCH v3 3/4] box-tap: syslog destination test unix socket Olga Arkhangelskaia @ 2018-07-24 17:11 ` Olga Arkhangelskaia 2018-07-25 16:43 ` Vladimir Davydov 3 siblings, 1 reply; 8+ messages in thread From: Olga Arkhangelskaia @ 2018-07-24 17:11 UTC (permalink / raw) To: tarantool-patches; +Cc: Olga Arkhangelskaia Adds syslog remote destination test in box-tap. Test creates server, sets appropriate log level and redirects logs to it. If log level received by the server is the same - test passes. If syslogd is not running we use pcall. Closes #3487 --- test/box-tap/cfg.test.lua | 42 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/test/box-tap/cfg.test.lua b/test/box-tap/cfg.test.lua index d1e595b1c..629faefaa 100755 --- a/test/box-tap/cfg.test.lua +++ b/test/box-tap/cfg.test.lua @@ -6,7 +6,7 @@ local socket = require('socket') local fio = require('fio') local uuid = require('uuid') local msgpack = require('msgpack') -test:plan(97) +test:plan(98) -------------------------------------------------------------------------------- -- Invalid values @@ -507,5 +507,45 @@ os.exit(res) ]] test:is(run_script(code), 0, "unix socket syslog log configuration") +-- +-- Check syslog remote configuration +-- +code = [[ +local tap = require('tap') +local socket = require('socket') +local os = require('os') +local test = tap.test("syslog_remote") +local log = require('log') +local errno = require('errno') + +addr = '127.0.0.1' +port = 1000 + math.random(32768) + +sc = socket('AF_INET', 'SOCK_DGRAM', 'udp') +local attempt = 0 +while attempt < 10 do +if not sc:bind (addr, port) then + port = 1000 + math.random(32768) + attempt = attempt + 1 + else + break + end +end +sc:bind(addr, port) + +local opt = string.format("syslog:server=%s:%u,identity=tarantool", addr, port) +local res = 1 +if pcall(box.cfg,{log = opt, log_level = 5}) then + log.info('Test syslog destination') + while (sc:readable(1)) do + buf = sc:recv(1000) + if buf:match('Test syslog destination') then res = 0 end + end +end +sc:close() +os.exit(res) +]] +test:is(run_script(code), 0, "remote syslog log configuration") + test:check() os.exit(0) -- 2.14.3 (Apple Git-98) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [tarantool-patches] [PATCH v3 4/4] box-tap:syslog remote destination test 2018-07-24 17:11 ` [tarantool-patches] [PATCH v3 4/4] box-tap:syslog remote destination test Olga Arkhangelskaia @ 2018-07-25 16:43 ` Vladimir Davydov 0 siblings, 0 replies; 8+ messages in thread From: Vladimir Davydov @ 2018-07-25 16:43 UTC (permalink / raw) To: Olga Arkhangelskaia; +Cc: tarantool-patches Please fold this patch in patch 1. On Tue, Jul 24, 2018 at 08:11:52PM +0300, Olga Arkhangelskaia wrote: > @@ -507,5 +507,45 @@ os.exit(res) > ]] > test:is(run_script(code), 0, "unix socket syslog log configuration") > > +-- > +-- Check syslog remote configuration > +-- > +code = [[ > +local tap = require('tap') > +local socket = require('socket') > +local os = require('os') > +local test = tap.test("syslog_remote") > +local log = require('log') > +local errno = require('errno') > + > +addr = '127.0.0.1' > +port = 1000 + math.random(32768) > + > +sc = socket('AF_INET', 'SOCK_DGRAM', 'udp') > +local attempt = 0 > +while attempt < 10 do > +if not sc:bind (addr, port) then > + port = 1000 + math.random(32768) > + attempt = attempt + 1 > + else > + break Tab instead of spaces. Please fix. > + end > +end > +sc:bind(addr, port) > + > +local opt = string.format("syslog:server=%s:%u,identity=tarantool", addr, port) > +local res = 1 > +if pcall(box.cfg,{log = opt, log_level = 5}) then pcall is not needed. > + log.info('Test syslog destination') > + while (sc:readable(1)) do > + buf = sc:recv(1000) > + if buf:match('Test syslog destination') then res = 0 end > + end Hmm, here you use a different way of detecting the test string in the log rather than in patch 3. Please be consistent if possible and use the same method in both tests. > +end > +sc:close() > +os.exit(res) > +]] > +test:is(run_script(code), 0, "remote syslog log configuration") > + > test:check() > os.exit(0) ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-07-25 16:43 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-07-24 17:11 [tarantool-patches] [PATCH v3 0/4] Syslog destination Olga Arkhangelskaia 2018-07-24 17:11 ` [tarantool-patches] [PATCH v3 1/4] Configurable syslog destination Olga Arkhangelskaia 2018-07-25 16:27 ` Vladimir Davydov 2018-07-24 17:11 ` [tarantool-patches] [PATCH v3 2/4] box-tap: test valid log configuration Olga Arkhangelskaia 2018-07-24 17:11 ` [tarantool-patches] [PATCH v3 3/4] box-tap: syslog destination test unix socket Olga Arkhangelskaia 2018-07-25 16:40 ` Vladimir Davydov 2018-07-24 17:11 ` [tarantool-patches] [PATCH v3 4/4] box-tap:syslog remote destination test Olga Arkhangelskaia 2018-07-25 16:43 ` Vladimir Davydov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox