[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Cluster-devel] [PATCH] STABLE3: enhance cman_tool to validate and distribute configuration files



On 23/09/09 17:48, Fabio M. Di Nitto wrote:
Hi Chrissie,

On Wed, 2009-09-23 at 15:41 +0100, Christine Caulfield wrote:
This patch adds significant functionality to 'cman_tool version'. If -r0
is specified, then the configuration file is validated (using
ccs_config_validate), distributed around the cluster (if necessary,
using ccs_sync) and activated. This provides a single command to update
a configuration ... something people have been asking for for ages.

wooooo


I'm not 100% happy about bundling it into cman_tool version, but neither
am I convinced that this warrants another cman_tool sub-command ... so
if anyone has any better ideas please speak up.

No more subcommands.. I think this is enough.

The patch looks good but I think we need to add a few more details here
and there...

We agreed to have validation to work with 3 config options: off (-D),
warning and fail hard. So I think -D could just take an option to
none,warn,fail or something like that.

I'd like to see an option to disable sync too. Even if we have ccs_sync,
not everybody might be using conga/luci and can at least allow them to
do it gently.

I don't recall exactly why we needed to propagate COROSYNC_CONFIG_IFACE
from cman_tool invokation. I am sure there was a very good reason for
that. If so we might have to propagate also other COROSYNC_ envvars
including LDAP bits.. because they will need to be available to
ccs_config_validator and I'll also need to make the validator a bit less
picky about overriding the environment if the environment is already
loaded (this is my task of course).


Here's a revised patch that adds fail,warn & none to the -D switch and also adds it to cman_tool join

Chrissie

diff --git a/cman/cman_tool/Makefile b/cman/cman_tool/Makefile
index 0dee578..149b758 100644
--- a/cman/cman_tool/Makefile
+++ b/cman/cman_tool/Makefile
@@ -13,12 +13,14 @@ include $(OBJDIR)/make/uninstall.mk
 OBJS=	main.o \
 	join.o
 
-CFLAGS += -DCOROSYNCBIN=\"${corosyncbin}\"
+CFLAGS += -DCOROSYNCBIN=\"${corosyncbin}\" -DSBINDIR=\"${sbindir}\"
 CFLAGS += -I${cmanincdir}
 CFLAGS += -I${incdir}
+CFLAGS += -I${ccsincdir}
 
 LDFLAGS += -L${cmanlibdir} -lcman
 LDFLAGS += -L${libdir}
+LDFLAGS += -L${ccslibdir} -lconfdb -lccs
 
 ${TARGET}: ${OBJS}
 	$(CC) -o $@ $^ $(LDFLAGS)
diff --git a/cman/cman_tool/cman_tool.h b/cman/cman_tool/cman_tool.h
index d6374d5..e84b491 100644
--- a/cman/cman_tool/cman_tool.h
+++ b/cman/cman_tool/cman_tool.h
@@ -57,6 +57,13 @@ enum format_opt
 	FMT_STATE,
 };
 
+enum validate_options
+{
+	VALIDATE_FAIL = 0,
+	VALIDATE_WARN,
+	VALIDATE_NONE,
+};
+
 struct commandline
 {
 	int operation;
@@ -82,6 +89,7 @@ struct commandline
 	unsigned int config_version;
 
 	int config_version_opt;
+	int config_validate_opt;
 	int votes_opt;
 	int expected_votes_opt;
 	int port_opt;
@@ -91,6 +99,7 @@ struct commandline
 	int wait_quorate_opt;
 	int addresses_opt;
 	int noconfig_opt;
+	int nosync_opt;
 	int nosetpri_opt;
 	int noopenais_opt;
 };
diff --git a/cman/cman_tool/join.c b/cman/cman_tool/join.c
index 02ef73a..697e944 100644
--- a/cman/cman_tool/join.c
+++ b/cman/cman_tool/join.c
@@ -2,6 +2,7 @@
 #include <stdint.h>
 #include <signal.h>
 #include <netinet/in.h>
+#include <corosync/confdb.h>
 #include "libcman.h"
 #include "cman_tool.h"
 
@@ -111,10 +112,19 @@ int join(commandline_t *comline, char *main_envp[])
 	int envptr = 0;
 	int argvptr = 0;
 	char scratch[1024];
+	char config_modules[1024];
 	cman_handle_t h = NULL;
 	int status;
+	hdb_handle_t object_handle;
+	confdb_handle_t confdb_handle;
+	int res;
 	pid_t corosync_pid;
 	int p[2];
+	confdb_callbacks_t callbacks = {
+		.confdb_key_change_notify_fn = NULL,
+		.confdb_object_create_change_notify_fn = NULL,
+		.confdb_object_delete_change_notify_fn = NULL
+	};
 
         /*
 	 * If we can talk to cman then we're already joined (or joining);
@@ -166,15 +176,15 @@ int join(commandline_t *comline, char *main_envp[])
 	}
 	if (comline->noconfig_opt) {
 		envp[envptr++] = strdup("CMAN_NOCONFIG=true");
-		snprintf(scratch, sizeof(scratch), "COROSYNC_DEFAULT_CONFIG_IFACE=cmanpreconfig%s",
+		snprintf(config_modules, sizeof(config_modules), "cmanpreconfig%s",
 			 comline->noopenais_opt?"":":openaisserviceenablestable");
-		envp[envptr++] = strdup(scratch);
 	}
 	else {
-		snprintf(scratch, sizeof(scratch), "COROSYNC_DEFAULT_CONFIG_IFACE=%s:cmanpreconfig%s", comline->config_lcrso,
+		snprintf(config_modules, sizeof(config_modules), "%s:cmanpreconfig%s", comline->config_lcrso,
 			 comline->noopenais_opt?"":":openaisserviceenablestable");
-		envp[envptr++] = strdup(scratch);
 	}
+	snprintf(scratch, sizeof(scratch), "COROSYNC_DEFAULT_CONFIG_IFACE=%s", config_modules);
+	envp[envptr++] = strdup(scratch);
 
 	/* Copy any COROSYNC_* env variables to the new daemon */
 	i=0;
@@ -324,5 +334,19 @@ int join(commandline_t *comline, char *main_envp[])
 		fprintf(stderr, "corosync started, but not joined the cluster yet.\n");
 
 	cman_finish(h);
+
+	/* Save the configuration information in corosync's objdb so we know where we came from */
+	res = confdb_initialize (&confdb_handle, &callbacks);
+	if (res != CS_OK)
+		goto join_exit;
+
+	res = confdb_object_create(confdb_handle, OBJECT_PARENT_HANDLE, "cman_private", strlen("cman_private"), &object_handle);
+	if (res == CS_OK) {
+		res = confdb_key_create(confdb_handle, object_handle, "config_modules", strlen("config_modules"), config_modules, strlen(config_modules));
+
+	}
+	confdb_finalize (confdb_handle);
+
+join_exit:
 	return 0;
 }
diff --git a/cman/cman_tool/main.c b/cman/cman_tool/main.c
index 2d28bc2..d5cfe17 100644
--- a/cman/cman_tool/main.c
+++ b/cman/cman_tool/main.c
@@ -2,6 +2,7 @@
 #include <unistd.h>
 #include <signal.h>
 #include <time.h>
+#include <ccs.h>
 #include <netinet/in.h>
 #include "copyright.cf"
 #include "libcman.h"
@@ -9,7 +10,7 @@
 
 #define DEFAULT_CONFIG_MODULE "xmlconfig"
 
-#define OPTION_STRING		("m:n:v:e:2p:c:r:i:N:t:o:k:F:C:VAPwfqah?Xd::")
+#define OPTION_STRING		("m:n:v:e:2p:c:r:i:N:t:o:k:F:C:VAPwfqah?XD::Sd::")
 #define OP_JOIN			1
 #define OP_LEAVE		2
 #define OP_EXPECTED		3
@@ -22,7 +23,6 @@
 #define OP_SERVICES		10
 #define OP_DEBUG		11
 
-
 static void print_usage(int subcmd)
 {
 	printf("Usage:\n");
@@ -57,6 +57,9 @@ static void print_usage(int subcmd)
 		printf("  -P               Don't set corosync to realtime priority\n");
 		printf("  -X               Use internal cman defaults for configuration\n");
 		printf("  -A               Don't load openais services\n");
+		printf("  -D <fail,warn,none> What to do about the config. Default (without -D) is to validate\n");
+		printf("                   the config. with -D no validation will be done. -Dwarn will print errors\n");
+		printf("                   but allow the operation to continue\n");
 		printf("\n");
 	}
 
@@ -116,6 +119,10 @@ static void print_usage(int subcmd)
 	if (!subcmd || subcmd == OP_VERSION) {
 		printf("version\n");
 		printf("  -r <config>      A new config version to set on all members\n");
+		printf("  -D <fail,warn,none> What to do about the config. Default (without -D) is to validate\n");
+		printf("                   the config. with -D no validation will be done. -Dwarn will print errors\n");
+		printf("                   but allow the operation to continue\n");
+		printf("  -S               Don't run ccs_sync to distribute cluster.conf (if appropriate)\n");
 		printf("\n");
 	}
 }
@@ -642,11 +649,62 @@ static void set_votes(commandline_t *comline)
 	cman_finish(h);
 }
 
+static int validate_config(commandline_t *comline, char *config_value)
+{
+	struct stat st;
+	char command[PATH_MAX];
+	char validator[PATH_MAX];
+	int cmd_res;
+
+	/* Look for ccs_config_validate */
+	snprintf(validator, sizeof(validator), "%s/ccs_config_validate", SBINDIR);
+	if (stat(validator, &st) != 0 || !(st.st_mode & S_IXUSR)) {
+		fprintf(stderr, "Cannot find ccs_config_validate, configuration was not checked but assumed to be OK.\n");
+		return 0;
+	}
+
+	snprintf(command, sizeof(command), "%s -C %s", validator, config_value);
+
+	if (comline->verbose > 1)
+		printf("calling '%s'\n", command);
+
+	cmd_res = system(command);
+
+	return cmd_res;
+}
+
+
+static int get_config_variable(commandline_t *comline, char **config_modules)
+{
+	int ccs_handle;
+	int ccs_res;
+	char *config_value;
+
+	/* Get the config modules that corosync was loaded with */
+	ccs_handle = ccs_connect();
+	if (!ccs_handle) {
+		fprintf(stderr, "Cannot contact ccs to get configuration information\n");
+		return 1;
+	}
+
+	ccs_res = ccs_get(ccs_handle, "/cman_private/@config_modules", &config_value);
+	ccs_disconnect(ccs_handle);
+	if (ccs_res) {
+		fprintf(stderr, "Cannot work out where corosync got it's configuration information from so I\n");
+		fprintf(stderr, "can't validate it. Use the -D switch to bypass this and load the\n");
+		fprintf(stderr, "configuration unchecked.\n");
+		return 1;
+	}
+	*config_modules = config_value;
+	return 0;
+}
+
 static void version(commandline_t *comline)
 {
 	struct cman_version ver;
 	cman_handle_t h;
 	int result;
+	char *config_modules = NULL;
 
 	h = open_cman_handle(1);
 
@@ -659,10 +717,35 @@ static void version(commandline_t *comline)
 		goto out;
 	}
 
-	ver.cv_config = comline->config_version;
+	if (comline->verbose)
+	        printf("Getting config variables\n");
+	if (get_config_variable(comline, &config_modules))
+	        die("");
 
-	if ((result = cman_set_version(h, &ver)))
-		die("can't set version: %s", cman_error(errno));
+	/* By default we validate the configuration first */
+	if (comline->config_validate_opt != VALIDATE_NONE) {
+
+		if (comline->verbose)
+			printf("Validating configuration\n");
+		if (validate_config(comline, config_modules) &&
+		    comline->config_validate_opt == VALIDATE_FAIL)
+			die("Not reloading, configuration is not valid\n");
+	}
+
+	if (strstr(config_modules, "xmlconfig") && !comline->nosync_opt) {
+		if (comline->verbose > 1)
+			printf("calling ccs_sync\n");
+		result = system("/usr/bin/ccs_sync");
+	}
+	else {
+		if (comline->verbose)
+			printf("Telling cman the new version number\n");
+
+		ver.cv_config = comline->config_version;
+
+		if ((result = cman_set_version(h, &ver)))
+			die("can't set version: %s", cman_error(errno));
+	}
  out:
 	cman_finish(h);
 }
@@ -752,6 +835,7 @@ static void decode_arguments(int argc, char *argv[], commandline_t *comline)
 {
 	int cont = TRUE;
 	int optchar, i;
+	int suboptchar;
 	int show_help = 0;
 
 	while (cont) {
@@ -767,6 +851,34 @@ static void decode_arguments(int argc, char *argv[], commandline_t *comline)
 			comline->addresses_opt = 1;
 			break;
 
+		case 'D':
+			/* Just look at the upper-cased version of the first letter of the argument */
+			if (optarg) {
+				suboptchar = optarg[0] & 0x5F;
+				switch (suboptchar)
+				{
+				case 'F':
+					comline->config_validate_opt = VALIDATE_FAIL;
+					break;
+				case 'W':
+					comline->config_validate_opt = VALIDATE_WARN;
+					break;
+				case 'N':
+					comline->config_validate_opt = VALIDATE_NONE;
+					break;
+				default:
+					die("invalid option to -D, it should be 'force', 'warn' or 'none'\n");
+					break;
+				}
+			}
+			else {
+				comline->config_validate_opt = VALIDATE_NONE;
+			}
+			break;
+		case 'S':
+			comline->nosync_opt = 1;
+			break;
+
 		case 'n':
 		        i = comline->num_nodenames;
 			if (i >= MAX_INTERFACES)
@@ -981,10 +1093,43 @@ static void check_arguments(commandline_t *comline)
 		die("timeout is only appropriate with wait");
 }
 
+
+static void do_join(commandline_t *comline, char *envp[])
+{
+	int ret;
+
+	check_arguments(comline);
+
+	if (comline->timeout) {
+		signal(SIGALRM, sigalarm_handler);
+		alarm(comline->timeout);
+	}
+
+	/* By default we validate the configuration first */
+	if (comline->config_validate_opt != VALIDATE_NONE) {
+
+		if (comline->verbose)
+			printf("Validating configuration\n");
+
+		if (validate_config(comline, comline->config_lcrso) &&
+		    comline->config_validate_opt == VALIDATE_FAIL)
+			die("Not reloading, configuration is not valid\n");
+	}
+
+	join(comline, envp);
+	if (comline->wait_opt || comline->wait_quorate_opt) {
+		do {
+			ret = cluster_wait(comline);
+			if (ret == ENOTCONN)
+				join(comline, envp);
+
+		} while (ret == ENOTCONN);
+	}
+}
+
 int main(int argc, char *argv[], char *envp[])
 {
 	commandline_t comline;
-	int ret;
 
 	prog_name = argv[0];
 
@@ -994,22 +1139,7 @@ int main(int argc, char *argv[], char *envp[])
 
 	switch (comline.operation) {
 	case OP_JOIN:
-		check_arguments(&comline);
-
-		if (comline.timeout) {
-			signal(SIGALRM, sigalarm_handler);
-			alarm(comline.timeout);
-		}
-
-		join(&comline, envp);
-		if (comline.wait_opt || comline.wait_quorate_opt) {
-			do {
-				ret = cluster_wait(&comline);
-				if (ret == ENOTCONN)
-					join(&comline, envp);
-
-			} while (ret == ENOTCONN);
-		}
+		do_join(&comline, envp);
 		break;
 
 	case OP_LEAVE:
diff --git a/cman/man/cman_tool.8 b/cman/man/cman_tool.8
index 8d45048..4d400a6 100644
--- a/cman/man/cman_tool.8
+++ b/cman/man/cman_tool.8
@@ -69,12 +69,11 @@ with -r to tell cluster members to update.
 .br
 The argument to -r is the version number that cman should look for. If 
 that version is not currently available then cman will poll for it. If
-a version of 0 is specified then cman will simply re-read the configuration
-and use the version number it finds there. 
+a version of 0 is specified then cman will read the configuration file,
+validate it, distribute it around the cluster (if necessary) and
+activate it.
 .br
-NOTE: this happens cluster-wide and the highest version number in the
-cluster will be the one everyone looks for. version -r0 is not a way
-to have different config versions across nodes in the cluster!
+The -D flag can disable the validation stage. This is NOT recommended.
 
 .TP
 .I wait 
@@ -151,6 +150,9 @@ cman_tool version on its own will always show the current version
 and not the one being looked for. So be aware that the display
 will possible not update immediately after you have run
 cman_tool version -r.
+.TP
+.I -D<option>
+see "JOIN" options
 .SH "WAIT" OPTIONS
 .TP
 .I -q
@@ -290,6 +292,17 @@ Don't load openais services. Normally cman_tool join will load the configuration
 module 'openaisserviceenablestable' which will load the services installed by openais.
 If you don't want to use these services or have not installed openais then
 this switch will disable them.
+.TP
+.I -D
+Tells cman_tool whether to validate the configuration before loading or reloading it.
+By default the configuration
+.B is
+validated, which is equivalent to -Dfail.
+.br
+-Dwarn will validate the configuration and print any messages arising, but will attempt
+to use it regardless of its validity.
+.br
+-Dnone (or just -D) will skip the validation completely.
 .SH "NODES" OPTIONS
 .TP
 .I -a

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]