Re: keepalived.data and keepalived.stats json output

Damien Clabaut <[email protected]> Wed, 19 Jul 2017 09:54:28 -0400
Newsgroups gmane.linux.keepalived.devel
Message-ID <[email protected]>
Hello,

Please find attached a first version of the patch.

It applies to master branch at latest commit.

What it does: Create a SIGJSON (kill -s 36) signal which calls a 
vrrp_print_json function which write json data to /tmp/keepalived.json.

I chose to have only one function instead of 2 (data / stats) as it is 
needed to dump instance name from 'data' into 'stats' anyway.

So far I do not dump much data; this can be changed easily. I dump all 
stats though.

I tried to find a way to descript de structure of _vrrp_t and 
_vrrp_stats once to generate dump_stats. dump_data and dump_json from it.

However, the dump_stats function uses heritage that is not in the C 
struct (advertisements->sent in the text, advert_sent in the struct), so 
I do not see a way to generate it from a structure description.

To compile, I pushed the debian/ folder from latest deployed on debian 
(1.3.2-1) and use these commands:

DEB_CFLAGS_SET="-I/usr/include/json-c/" DEB_LDFLAGS_SET="-ljson-c" 
MACVLAN_SUPPORT=yes ./configure --enable-sha1 --enable-debug

DEB_CFLAGS_SET="-I/usr/include/json-c/" DEB_LDFLAGS_SET="-ljson-c" 
MACVLAN_SUPPORT=yes dpkg-buildpackage -us -uc -b -j4

Todo:

- Finish dumping in json all the data that is dumped with -USR1

- Make the entire patch deployed only if specific compilation option is 
set, as is adds dependency to json-c

This is a work in progress and my first attempt at contributing to 
Keepalived, so I am assuming it does not match your code standards. 
Please tell me what you think of it.

Regards,


On 18/07/17 09:11 AM, Damien Clabaut wrote:
>
> Hello,
>
> To be clear, we are talking about opening a AF_UNIX socket at 
> keepalived startup, and be able to ask for json through it ?
>
> The Bird router has something similar, I will check how it is 
> implemented and how long it would take to implement it for Keepalived.
>
> However, since the work with POSIX signals is already quite advanced, 
> in case this is too long / complicated to implement, can we consider 
> keeping the signal handling solution for now ?
>
> I am doing this on my work time and will be expected to provide 
> results soon.
>
> Regards,
>
>
> On 17/07/17 04:39 PM, Alexandre Cassen wrote:
>> Hi Guys,
>>
>> Well, maybe designing a req/rep json protocol to request keepalived 
>> states & related would be more futur proof instead of signal handling ?
>>
>> Best regs,
>> Alexandre
>>
>> On Mon, Jul 17, 2017 at 9:53 PM, Damien Clabaut 
>> <[email protected] <mailto:[email protected]>> wrote:
>>
>>     Hello,
>>
>>     I have made some progress on the matter. I did not know much
>>     about POSIX signals a few days ago, so I spent a lot of time
>>     figuring out how to manipulate them.
>>
>>     To keep the code explicit, I defined two constants SIGDATAJS and
>>     SIGSTATJS. They are currently defined this way:
>>
>>     --- configure.ac <http://configure.ac>  2017-02-18
>>     12:52:10.000000000 +0100
>>     +++ configure.ac <http://configure.ac>  2017-07-17
>>     21:07:37.994953146 +0200
>>     @@ -46,6 +46,8 @@
>>        [
>>      #ifndef _CONFIG_H
>>      #define _CONFIG_H
>>     +#define SIGDATAJS ( __SIGRTMIN + 4 )
>>     +#define SIGSTATJS ( __SIGRTMIN + 5 )
>>        ])
>>      AH_BOTTOM(
>>        [
>>
>>
>>     __SIGRTMIN to ( __SIGRTMIN + 3 ) are reserved on Linux so I
>>     started at +4.
>>
>>     The file configure.ac <http://configure.ac> being really long, I
>>     am not yet sure this is the best place to define those constants.
>>     It looks like I need them in config.h for the code to compile.
>>
>>     Can you confirm this is a clean place to define those constants ?
>>
>>     Meanwhile, I am starting to work on the functions that will
>>     actually write the json output (so far they create files and
>>     write one static string in it).
>>
>>     Regards,
>>
>>
>>     On 14/07/17 01:28 PM, Damien Clabaut wrote:
>>>
>>>     Hello Quentin and thank you for your answer.
>>>
>>>     I believe we will add two extra signals (USR3 and USR4, or
>>>     something else) and use them to write output to
>>>     /tmp/keepalived_data.json and /tmp/keepalived_stats.json.
>>>
>>>     This way users can keep using existing scripts with existing
>>>     behaviour, and develop future scripts on new behaviour, at the
>>>     same time.
>>>
>>>     Regards,
>>>
>>>
>>>     On 14/07/17 09:47 AM, Quentin Armitage wrote:
>>>>     On Fri, 2017-07-14 at 08:35 -0400, Damien Clabaut wrote:
>>>>>     Hello Keepalived developers,
>>>>>
>>>>>     My team and I are considering modifying the file
>>>>>     https://github.com/acassen/keepalived/blob/master/keepalived/vrrp/vrrp_print.c
>>>>>     <https://github.com/acassen/keepalived/blob/master/keepalived/vrrp/vrrp_print.c>  
>>>>>     in order to make the /tmp/keepalived.data and /tmp/keepalived.stats a
>>>>>     valid json.
>>>>>
>>>>>     In your opinion, what would be the best way to do this ?
>>>>>
>>>>>     - Patching current functions so that kill -USR1 and kill -USR2 return
>>>>>     valid json ?
>>>>>
>>>>>     - Adding handlers for kill -USR3 and kill -USR4 to return valid json,
>>>>>     leaving -USR1 and -USR2 in their current states ?
>>>>>
>>>>>     The idea would be to port this patch to the master branch in the end.
>>>>>
>>>>>     Regards,
>>>>>
>>>>     I don't think we can alter the current functionality - there
>>>>     might be users who have scripts that parse the output files.
>>>>
>>>>     Of course, there are no signals USR3 and USR4, but there is no
>>>>     reason why you can't use other signals that aren't currently used.
>>>>
>>>>     I can see three ways of doing this, each of which I would be
>>>>     happy with:
>>>>     1. Use 2 new signals (this seems to offer the greatest flexibility)
>>>>     2. Have a global configuration option, e.g. dump_json, which if
>>>>     set will make USR1 and USR2 write json output
>>>>     3. Use 1 additional signal that each time it is invoked toggles
>>>>     between the current format and json (the default would need to
>>>>     be the current format); this probably isn't the best approach.
>>>>
>>>>     Depending on how much overhead in terms of code size there is
>>>>     with the json output, it might be useful to make the inclusion
>>>>     of json output a compile time option enabled by a configure
>>>>     option --enable-json.
>>>>
>>>>     It would be good not to duplicate the existing code, since at
>>>>     the moment when a new option is added we already have to update
>>>>     vrrp_print.c, vrrp_data.c and vrrp_snmp.c, and to have to
>>>>     update a fourth place in the code would be somewhat excessive.
>>>>     In fact I have in mind that at sometime the code in
>>>>     vrrp_print.c and the dump_vrrp() code in vrrp_data.c should be
>>>>     merged.
>>>>
>>>>     I hope that helps,
>>>>
>>>>     Quentin Armitage
>>>>
>>>>
>>>
>>>     -- 
>>>     Damien Clabaut
>>>     R&D vRouter
>>>     ovh.qc.ca <http://ovh.qc.ca>
>>>
>>>
>>>     ------------------------------------------------------------------------------
>>>     Check out the vibrant tech community on one of the world's most
>>>     engaging tech sites, Slashdot.org!http://sdm.link/slashdot
>>>
>>>
>>>     _______________________________________________
>>>     Keepalived-devel mailing list
>>>     [email protected]
>>>     <mailto:[email protected]>
>>>     https://lists.sourceforge.net/lists/listinfo/keepalived-devel
>>>     <https://lists.sourceforge.net/lists/listinfo/keepalived-devel>
>>
>>     -- 
>>     Damien Clabaut
>>     R&D vRouter
>>     ovh.qc.ca <http://ovh.qc.ca>
>>
>>
>>     ------------------------------------------------------------------------------
>>     Check out the vibrant tech community on one of the world's most
>>     engaging tech sites, Slashdot.org! http://sdm.link/slashdot
>>     _______________________________________________
>>     Keepalived-devel mailing list
>>     [email protected]
>>     <mailto:[email protected]>
>>     https://lists.sourceforge.net/lists/listinfo/keepalived-devel
>>     <https://lists.sourceforge.net/lists/listinfo/keepalived-devel>
>>
>>
>
> -- 
> Damien Clabaut
> R&D vRouter
> ovh.qc.ca
>
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
>
>
> _______________________________________________
> Keepalived-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/keepalived-devel

-- 
Damien Clabaut
R&D vRouter
ovh.qc.ca

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

_______________________________________________
Keepalived-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/keepalived-devel
20170719_keepalived_sigjson_2.patch (text/x-patch, 7.9 KB)
diff --git a/configure.ac b/configure.ac
index da62c10..1b5dba7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -98,6 +98,7 @@ AH_TOP(
   [
 #ifndef _CONFIG_H
 #define _CONFIG_H
+#define SIGJSON ( __SIGRTMIN + 4 )
   ])
 AH_BOTTOM(
   [
diff --git a/keepalived/core/main.c b/keepalived/core/main.c
index be8492f..860c0dc 100644
--- a/keepalived/core/main.c
+++ b/keepalived/core/main.c
@@ -445,6 +445,7 @@ signal_init(void)
 	signal_set(SIGHUP, propogate_signal, NULL);
 	signal_set(SIGUSR1, propogate_signal, NULL);
 	signal_set(SIGUSR2, propogate_signal, NULL);
+	signal_set(SIGJSON, propogate_signal, NULL);
 	signal_set(SIGINT, sigend, NULL);
 	signal_set(SIGTERM, sigend, NULL);
 	signal_ignore(SIGPIPE);
diff --git a/keepalived/include/vrrp_print.h b/keepalived/include/vrrp_print.h
index 9eaba5f..e546230 100644
--- a/keepalived/include/vrrp_print.h
+++ b/keepalived/include/vrrp_print.h
@@ -25,3 +25,4 @@
 
 extern void vrrp_print_data(void);
 extern void vrrp_print_stats(void);
+extern void vrrp_print_json(void);
diff --git a/keepalived/vrrp/vrrp_daemon.c b/keepalived/vrrp/vrrp_daemon.c
index 29866f7..b80a70b 100644
--- a/keepalived/vrrp/vrrp_daemon.c
+++ b/keepalived/vrrp/vrrp_daemon.c
@@ -73,6 +73,7 @@
 /* Forward declarations */
 static int print_vrrp_data(thread_t * thread);
 static int print_vrrp_stats(thread_t * thread);
+static int print_vrrp_json(thread_t * thread);
 static int reload_vrrp_thread(thread_t * thread);
 
 static char *vrrp_syslog_ident;
@@ -359,6 +360,14 @@ sigusr2_vrrp(__attribute__((unused)) void *v, __attribute__((unused)) int sig)
 	thread_add_event(master, print_vrrp_stats, NULL, 0);
 }
 
+static void
+sigjson_vrrp(__attribute__((unused)) void *v, __attribute__((unused)) int sig)
+{
+	log_message(LOG_INFO, "Printing VRRP as json for process(%d) on signal",
+		getpid());
+	thread_add_event(master, print_vrrp_json, NULL, 0);
+}
+
 /* Terminate handler */
 static void
 sigend_vrrp(__attribute__((unused)) void *v, __attribute__((unused)) int sig)
@@ -377,6 +386,7 @@ vrrp_signal_init(void)
 	signal_set(SIGTERM, sigend_vrrp, NULL);
 	signal_set(SIGUSR1, sigusr1_vrrp, NULL);
 	signal_set(SIGUSR2, sigusr2_vrrp, NULL);
+	signal_set(SIGJSON, sigjson_vrrp, NULL);
 	signal_ignore(SIGPIPE);
 }
 
@@ -456,6 +466,13 @@ print_vrrp_stats(__attribute__((unused)) thread_t * thread)
 	return 0;
 }
 
+static int
+print_vrrp_json(__attribute__((unused)) thread_t * thread)
+{
+	vrrp_print_json();
+	return 0;
+}
+
 /* VRRP Child respawning thread */
 #ifndef _DEBUG_
 static int
diff --git a/keepalived/vrrp/vrrp_print.c b/keepalived/vrrp/vrrp_print.c
index 864db40..deb305b 100644
--- a/keepalived/vrrp/vrrp_print.c
+++ b/keepalived/vrrp/vrrp_print.c
@@ -38,6 +38,7 @@
 #include <time.h>
 #include <errno.h>
 #include <inttypes.h>
+#include <json.h>
 
 static void
 vrrp_print_list(FILE *file, list l, void (*fptr)(FILE*, void*))
@@ -418,3 +419,94 @@ vrrp_print_stats(void)
 	}
 	fclose(file);
 }
+
+void
+vrrp_print_json(void)
+{
+	FILE *file;
+	file = fopen ("/tmp/keepalived.json","w");
+
+	if (!file) {
+		log_message(LOG_INFO, "Can't open /tmp/keepalived.json (%d: %s)",
+			errno, strerror(errno));
+		return;
+	}
+
+	list l = vrrp_data->vrrp;
+	element e;
+	vrrp_t *vrrp;
+	struct json_object *array;
+	array = json_object_new_array();
+	int index = 0;
+
+	for (e = LIST_HEAD(l); e; ELEMENT_NEXT(e)) {
+		log_message(LOG_INFO, "Begin loop for instance %d\n", index);
+		vrrp = ELEMENT_DATA(e);
+		struct json_object *instance_json, *json_stats, *json_data;
+		instance_json = json_object_new_object();
+		json_stats = json_object_new_object();
+		json_data = json_object_new_object();
+
+		log_message(LOG_INFO, "Dumping data for instance %d\n", index);
+		// Dump data to json 
+		json_object_object_add(json_data, "iname",
+			json_object_new_string(vrrp->iname));
+		json_object_object_add(json_data, "version",
+		json_object_new_int(vrrp->version));
+//		json_object_object_add(json_data, "state",
+//			json_object_new_string(vrrp->state));
+		json_object_object_add(json_data, "last_transition",
+		json_object_new_int(vrrp->last_transition.tv_sec));
+//		json_object_object_add(json_data, "ifp",
+//			json_object_new_string(vrrp->ifp));
+		json_object_object_add(json_data, "dont_track_primary",
+			json_object_new_int(vrrp->dont_track_primary));
+		json_object_object_add(json_data, "skip_check_adv_addr",
+			json_object_new_int(vrrp->skip_check_adv_addr));
+
+		log_message(LOG_INFO, "Dumping stats for instance %d\n", index);
+		// Dump stats to json 
+		json_object_object_add(json_stats, "advert_rcvd",
+			json_object_new_int64(vrrp->stats->advert_rcvd));
+		json_object_object_add(json_stats, "advert_sent",
+			json_object_new_int(vrrp->stats->advert_sent));
+		json_object_object_add(json_stats, "become_master",
+			json_object_new_int(vrrp->stats->become_master));
+		json_object_object_add(json_stats, "release_master",
+			json_object_new_int(vrrp->stats->release_master));
+		json_object_object_add(json_stats, "packet_len_err",
+			json_object_new_int64(vrrp->stats->packet_len_err));
+		json_object_object_add(json_stats, "advert_interval_err",
+			json_object_new_int64(vrrp->stats->advert_interval_err));
+		json_object_object_add(json_stats, "ip_ttl_err",
+			json_object_new_int64(vrrp->stats->ip_ttl_err));
+		json_object_object_add(json_stats, "invalid_type_rcvd",
+			json_object_new_int64(vrrp->stats->invalid_type_rcvd));
+		json_object_object_add(json_stats, "addr_list_err",
+			json_object_new_int64(vrrp->stats->addr_list_err));
+		json_object_object_add(json_stats, "invalid_authtype",
+			json_object_new_int(vrrp->stats->invalid_authtype));
+		json_object_object_add(json_stats, "authtype_mismatch",
+			json_object_new_int(vrrp->stats->authtype_mismatch));
+		json_object_object_add(json_stats, "auth_failure",
+			json_object_new_int(vrrp->stats->auth_failure));
+		json_object_object_add(json_stats, "pri_zero_rcvd",
+			json_object_new_int64(vrrp->stats->pri_zero_rcvd));
+		json_object_object_add(json_stats, "pri_zero_sent",
+			json_object_new_int64(vrrp->stats->pri_zero_sent));
+
+		log_message(LOG_INFO, "Add both json_data and json_stats to main instance_json for instance %d\n", index);
+		// Add both json_data and json_stats to main instance_json 
+		json_object_object_add(instance_json, "data", json_data);
+		json_object_object_add(instance_json, "stats", json_stats);
+
+		log_message(LOG_INFO, "Add instance_json to main array for instance %d\n", index);
+		// Add instance_json to main array 
+		json_object_array_put_idx(array, index, instance_json);
+		index += 1;
+
+	}
+	fprintf(file, "%s", json_object_to_json_string(array));
+	fclose(file);
+}
+
diff --git a/lib/signals.c b/lib/signals.c
index 756c67e..0c3d866 100644
--- a/lib/signals.c
+++ b/lib/signals.c
@@ -58,6 +58,8 @@ static void (*signal_SIGUSR1_handler) (void *, int sig);
 static void *signal_SIGUSR1_v;
 static void (*signal_SIGUSR2_handler) (void *, int sig);
 static void *signal_SIGUSR2_v;
+static void (*signal_SIGJSON_handler) (void *, int sig);
+static void *signal_SIGJSON_v;
 
 static int signal_pipe[2] = { -1, -1 };
 
@@ -165,6 +167,11 @@ signal_set(int signo, void (*func) (void *, int), void *v)
 		signal_SIGUSR2_handler = func;
 		signal_SIGUSR2_v = v;
 		break;
+	case SIGJSON:
+		signal_SIGJSON_handler = func;
+		signal_SIGJSON_v = v;
+		break;
+
 	}
 
 	if (ret < 0)
@@ -193,6 +200,7 @@ clear_signal_handler_addresses(void)
 	signal_SIGCHLD_handler = NULL;
 	signal_SIGUSR1_handler = NULL;
 	signal_SIGUSR2_handler = NULL;
+	signal_SIGJSON_handler = NULL;
 }
 
 /* Handlers intialization */
@@ -295,6 +303,7 @@ signal_handlers_clear(void *state)
 	signal_set(SIGCHLD, state, NULL);
 	signal_set(SIGUSR1, state, NULL);
 	signal_set(SIGUSR2, state, NULL);
+  signal_set(SIGJSON, state, NULL);
 }
 
 void
@@ -368,6 +377,10 @@ signal_run_callback(void)
 			if (signal_SIGUSR2_handler)
 				signal_SIGUSR2_handler(signal_SIGUSR2_v, SIGUSR2);
 			break;
+		case SIGJSON:
+			if (signal_SIGJSON_handler)
+				signal_SIGJSON_handler(signal_SIGJSON_v, SIGJSON);
+			break;
 		default:
 			break;
 		}