Looks good Adam, nice work.
I have a suggestion: let's reserve the first line of the output for a
status message of our own: either "OK" or "ERROR", say. If there's
an
error message we received from puppet/augeas we can add that on the next
line. That way, there's no way we can cause confusion by e.g. getting an
error message back that says something like "OK, everything is borked".
Or simply getting an empty error string.
cheers,
Zane.
On 19/08/11 16:20, Adam Stokes wrote:
This allows 3 ways to test for configuration
status
1) If file exists then in the least the execution
was issued
2) If file exists and contents of file report an
error, then process failed with at least 1 execution
3) If file exists and contents contain a success
message, then binary was executed and successfully
completed.
Signed-off-by: Adam Stokes<astokes(a)fedoraproject.org>
---
src/include/matahari/sysconfig.h | 12 ++++++------
src/lib/sysconfig.c | 32 ++++++++++++--------------------
src/lib/sysconfig_linux.c | 12 +++++++-----
src/sysconfig/sysconfig-qmf.cpp | 24 ++++++++++--------------
src/tests/mh_api_sysconfig.h | 4 ++--
5 files changed, 37 insertions(+), 47 deletions(-)
diff --git a/src/include/matahari/sysconfig.h b/src/include/matahari/sysconfig.h
index 0390528..ede9c7c 100644
--- a/src/include/matahari/sysconfig.h
+++ b/src/include/matahari/sysconfig.h
@@ -77,21 +77,21 @@ mh_sysconfig_query(const char *query, uint32_t flags, const char
*scheme);
* Set system as configured
*
* \param[in] key config item to define
+ * \param[in] contents specifies success/fail with error where applicable
*
- * \return TRUE if applying key to config succeeds
+ * \return STATUS of spawned process return code
*/
-extern gboolean
-mh_sysconfig_set_configured(const char *key);
+extern const char *
+mh_sysconfig_set_configured(const char *key, const char *contents);
/**
* Test if system has been configured
*
- * \param[in] flags allow a force reconfiguration
* \param[in] key config item to test if used
*
- * \return TRUE if system is configured
+ * \return STATUS of key
*/
-extern gboolean
+extern const char *
mh_sysconfig_is_configured(const char *key);
/**
diff --git a/src/lib/sysconfig.c b/src/lib/sysconfig.c
index 1dc5753..a8c78cf 100644
--- a/src/lib/sysconfig.c
+++ b/src/lib/sysconfig.c
@@ -64,10 +64,9 @@ mh_sysconfig_keys_dir_set(const char *path)
}
static gboolean
-set_key(const char *key)
+set_key(const char *key, const char *contents)
{
char key_file[PATH_MAX];
- const char contents[] = "ok";
g_snprintf(key_file, sizeof(key_file), "%s%s", keys_dir_get(), key);
if (!g_file_set_contents(key_file, contents, strlen(contents), NULL)) {
@@ -77,42 +76,35 @@ set_key(const char *key)
return TRUE;
}
-static gboolean
+static char *
get_key(const char *key)
{
char key_file[PATH_MAX];
- char *contents;
+ char *contents = NULL;
size_t length;
g_snprintf(key_file, sizeof(key_file), "%s%s", keys_dir_get(), key);
if (g_file_test(key_file, G_FILE_TEST_EXISTS)) {
if(g_file_get_contents(key_file,&contents,&length, NULL)) {
- if (strcmp(contents, "ok") == 0) {
- free(contents);
- return TRUE;
- }
+ return contents;
}
- free(contents);
}
- return FALSE;
+ return contents;
}
-gboolean
-mh_sysconfig_set_configured(const char *key)
+const char *
+mh_sysconfig_set_configured(const char *key, const char *contents)
{
- if (!set_key(key)) {
- return FALSE;
+ if (!set_key(key, contents)) {
+ return NULL;
}
- return TRUE;
+ return get_key(key);
}
-gboolean
+const char *
mh_sysconfig_is_configured(const char *key)
{
- if (!get_key(key)) {
- return FALSE;
- }
- return TRUE;
+ return get_key(key);
}
int
diff --git a/src/lib/sysconfig_linux.c b/src/lib/sysconfig_linux.c
index a809267..4fabdda 100644
--- a/src/lib/sysconfig_linux.c
+++ b/src/lib/sysconfig_linux.c
@@ -59,7 +59,7 @@ sysconfig_os_download(const char *uri, FILE *fp)
static int
-sysconfig_os_run_puppet(const char *uri, const char *data)
+sysconfig_os_run_puppet(const char *uri, const char *data, const char *key)
{
gboolean ret;
GError *error = NULL;
@@ -99,14 +99,16 @@ sysconfig_os_run_puppet(const char *uri, const char *data)
ret = g_spawn_async(NULL, cmd, NULL, G_SPAWN_SEARCH_PATH,
NULL, NULL, NULL,&error);
if (ret == FALSE) {
+ mh_sysconfig_set_configured(key, error->message);
g_error_free(error);
return -1;
}
+ mh_sysconfig_set_configured(key, "OK");
return 0;
}
static int
-sysconfig_os_run_augeas(const char *query, const char *data)
+sysconfig_os_run_augeas(const char *query, const char *data, const char *key)
{
mh_warn("not implemented\n");
return -1;
@@ -128,9 +130,9 @@ sysconfig_os_run_uri(const char *uri, uint32_t flags, const char
*scheme,
if (mh_sysconfig_is_configured(key) == FALSE || (flags&
MH_SYSCONFIG_FLAG_FORCE)) {
if (strcasecmp(scheme, "puppet") == 0) {
- rc = sysconfig_os_run_puppet(uri, NULL);
+ rc = sysconfig_os_run_puppet(uri, NULL, key);
} else if (strcasecmp(scheme, "augeas") == 0) {
- rc = sysconfig_os_run_augeas(uri, NULL);
+ rc = sysconfig_os_run_augeas(uri, NULL, key);
} else {
rc = -1;
}
@@ -146,7 +148,7 @@ sysconfig_os_run_string(const char *data, uint32_t flags, const char
*scheme,
if (mh_sysconfig_is_configured(key) == FALSE || (flags&
MH_SYSCONFIG_FLAG_FORCE)) {
if (strcasecmp(scheme, "puppet") == 0) {
- rc = sysconfig_os_run_puppet(NULL, data);
+ rc = sysconfig_os_run_puppet(NULL, data, key);
} else {
rc = -1;
}
diff --git a/src/sysconfig/sysconfig-qmf.cpp b/src/sysconfig/sysconfig-qmf.cpp
index 962ca3d..dfe1e6c 100644
--- a/src/sysconfig/sysconfig-qmf.cpp
+++ b/src/sysconfig/sysconfig-qmf.cpp
@@ -74,7 +74,7 @@ ConfigAgent::setup(qmf::AgentSession session)
gboolean
ConfigAgent::invoke(qmf::AgentSession session, qmf::AgentEvent event, gpointer
user_data)
{
- int rc = 0;
+ const char *status;
const std::string& methodName(event.getMethodName());
if (event.getType() != qmf::AGENT_METHOD) {
@@ -84,23 +84,19 @@ ConfigAgent::invoke(qmf::AgentSession session, qmf::AgentEvent event,
gpointer u
qpid::types::Variant::Map& args = event.getArguments();
if (methodName == "run_uri") {
- rc = mh_sysconfig_run_uri(args["uri"].asString().c_str(),
+ mh_sysconfig_run_uri(args["uri"].asString().c_str(),
args["flags"].asUint32(),
args["scheme"].asString().c_str(),
args["key"].asString().c_str());
- if (rc == 0) {
- mh_sysconfig_set_configured(args["key"].asString().c_str());
- }
- event.addReturnArgument("status", rc);
+ status = mh_sysconfig_is_configured(args["key"].asString().c_str());
+ event.addReturnArgument("status", status);
} else if (methodName == "run_string") {
- rc = mh_sysconfig_run_string(args["data"].asString().c_str(),
+ mh_sysconfig_run_string(args["data"].asString().c_str(),
args["flags"].asUint32(),
args["scheme"].asString().c_str(),
args["key"].asString().c_str());
- if (rc == 0) {
- mh_sysconfig_set_configured(args["key"].asString().c_str());
- }
- event.addReturnArgument("status", rc);
+ status = mh_sysconfig_is_configured(args["key"].asString().c_str());
+ event.addReturnArgument("status", status);
} else if (methodName == "query") {
const char *data = NULL;
data = mh_sysconfig_query(args["query"].asString().c_str(),
@@ -108,9 +104,9 @@ ConfigAgent::invoke(qmf::AgentSession session, qmf::AgentEvent event,
gpointer u
args["scheme"].asString().c_str());
event.addReturnArgument("query", data);
} else if (methodName == "is_configured") {
- rc = mh_sysconfig_is_configured(args["key"].asString().c_str());
- _instance.setProperty("is_postboot_configured", rc);
- event.addReturnArgument("status", rc ? "ok" :
"no");
+ status = mh_sysconfig_is_configured(args["key"].asString().c_str());
+ _instance.setProperty("is_postboot_configured", status);
+ event.addReturnArgument("status", status);
} else {
session.raiseException(event, MH_NOT_IMPLEMENTED);
goto bail;
diff --git a/src/tests/mh_api_sysconfig.h b/src/tests/mh_api_sysconfig.h
index 879bb11..1cc82bc 100644
--- a/src/tests/mh_api_sysconfig.h
+++ b/src/tests/mh_api_sysconfig.h
@@ -31,8 +31,8 @@ class MhApiSysconfigSuite : public CxxTest::TestSuite
mh_sysconfig_keys_dir_set("/tmp/");
- TS_ASSERT((mh_sysconfig_set_configured(key)) == TRUE);
- TS_ASSERT((mh_sysconfig_is_configured(key)) == TRUE);
+ TS_ASSERT((mh_sysconfig_set_configured(key, "OK")) != NULL);
+ TS_ASSERT((mh_sysconfig_is_configured(key)) != NULL);
TS_ASSERT((mh_sysconfig_run_uri(uri, flags, "puppet", key)) != -1);
}
};