From 62ef17dc979e3166f89966d24147b3cd32b3601d Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Tue, 24 Oct 2017 11:56:05 -0700 Subject: [PATCH] cmd/flux-kvs: add [--json|--raw] options to "get" Problem: flux-get put presumes values should be interpreted as JSON, but the KVS no longer requires this. Add type options mirroring those added to the "put" subcommand to allow the user to choose how values are interpreted. If no options, value is interpreted as a NULL terminated string and written to stdout with a newline. If --raw, value is interpreted as raw data and is written to stdout without any formatting. If --json, value is interpreted as encoded JSON. If the value is an object or array, or any scalar type but string, it is displayed in its encoded form. If the value is a JSON strong, quotes are removed, which mimics the old default behavior of flux-kvs get expected by many sharness tests. Add --json to flux kvs get where used in various sharness tests. Fixes #1159 --- src/cmd/flux-kvs.c | 40 ++++++++++++++++++---- t/issues/t0441-kvs-put-get.sh | 4 +-- t/issues/t0821-kvs-segfault.sh | 4 +-- t/t1000-kvs.t | 62 +++++++++++++++++----------------- t/t1002-kvs-extra.t | 10 +++--- t/t2000-wreck.t | 10 +++--- t/t2001-jsc.t | 12 +++---- t/t2002-pmi.t | 2 +- t/t2005-hwloc-basic.t | 8 ++--- 9 files changed, 90 insertions(+), 62 deletions(-) diff --git a/src/cmd/flux-kvs.c b/src/cmd/flux-kvs.c index 4c2dbe7369dd..040ff0157819 100644 --- a/src/cmd/flux-kvs.c +++ b/src/cmd/flux-kvs.c @@ -56,6 +56,16 @@ static void dump_kvs_dir (const flux_kvsdir_t *dir, bool Ropt, bool dopt); #define min(a,b) ((a)<(b)?(a):(b)) +static struct optparse_option get_opts[] = { + { .name = "json", .key = 'j', .has_arg = 0, + .usage = "Interpret value(s) as encoded JSON", + }, + { .name = "raw", .key = 'r', .has_arg = 0, + .usage = "Interpret value(s) as raw data", + }, + OPTPARSE_TABLE_END +}; + static struct optparse_option put_opts[] = { { .name = "json", .key = 'j', .has_arg = 0, .usage = "Store value(s) as encoded JSON", @@ -130,11 +140,11 @@ static struct optparse_option unlink_opts[] = { static struct optparse_subcommand subcommands[] = { { "get", - "key [key...]", + "[-j|-r] key [key...]", "Get value stored under key", cmd_get, 0, - NULL + get_opts }, { "put", "[-j|-r] key=value [key=value...]", @@ -349,7 +359,7 @@ static void output_key_json_str (const char *key, int cmd_get (optparse_t *p, int argc, char **argv) { flux_t *h = (flux_t *)optparse_get_data (p, "flux_handle"); - const char *key, *json_str; + const char *key; flux_future_t *f; int optindex, i; @@ -360,10 +370,28 @@ int cmd_get (optparse_t *p, int argc, char **argv) } for (i = optindex; i < argc; i++) { key = argv[i]; - if (!(f = flux_kvs_lookup (h, 0, key)) - || flux_kvs_lookup_get (f, &json_str) < 0) + if (!(f = flux_kvs_lookup (h, 0, key))) log_err_exit ("%s", key); - output_key_json_str (NULL, json_str, key); + if (optparse_hasopt (p, "json")) { + const char *json_str; + if (flux_kvs_lookup_get (f, &json_str) < 0) + log_err_exit ("%s", key); + output_key_json_str (NULL, json_str, key); + } + else if (optparse_hasopt (p, "raw")) { + const void *data; + int len; + if (flux_kvs_lookup_get_raw (f, &data, &len) < 0) + log_err_exit ("%s", key); + if (write_all (STDOUT_FILENO, data, len) < 0) + log_err_exit ("%s", key); + } + else { + const char *value; + if (flux_kvs_lookup_get (f, &value) < 0) + log_err_exit ("%s", key); + printf ("%s\n", value); + } flux_future_destroy (f); } return (0); diff --git a/t/issues/t0441-kvs-put-get.sh b/t/issues/t0441-kvs-put-get.sh index 9079a48676a4..f5b844eb7b72 100755 --- a/t/issues/t0441-kvs-put-get.sh +++ b/t/issues/t0441-kvs-put-get.sh @@ -5,6 +5,6 @@ TEST=issue441 flux kvs put --json ${TEST}.x=foo -flux kvs get ${TEST}.x.y && test $? -eq 1 +flux kvs get --json ${TEST}.x.y && test $? -eq 1 -flux kvs get ${TEST}.x # fails if broker died +flux kvs get --json ${TEST}.x # fails if broker died diff --git a/t/issues/t0821-kvs-segfault.sh b/t/issues/t0821-kvs-segfault.sh index de3b2b051899..94c617ef98b4 100755 --- a/t/issues/t0821-kvs-segfault.sh +++ b/t/issues/t0821-kvs-segfault.sh @@ -3,5 +3,5 @@ TEST=issue0821 flux kvs put --json ${TEST}="xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" -flux kvs get ${TEST}.x && test $? -eq 1 -flux kvs get ${TEST} # fails if broker died +flux kvs get --json ${TEST}.x && test $? -eq 1 +flux kvs get --json ${TEST} # fails if broker died diff --git a/t/t1000-kvs.t b/t/t1000-kvs.t index 64729e007154..33bbb7a809a7 100755 --- a/t/t1000-kvs.t +++ b/t/t1000-kvs.t @@ -25,7 +25,7 @@ SUBDIR1=test.a.b.d SUBDIR2=test.a.b.e test_kvs_key() { - flux kvs get "$1" >output + flux kvs get --json "$1" >output echo "$2" >expected test_cmp expected output } @@ -139,43 +139,43 @@ EOF ' test_expect_success 'kvs: unlink works' ' flux kvs unlink $KEY.integer && - test_must_fail flux kvs get $KEY.integer + test_must_fail flux kvs get --json $KEY.integer ' test_expect_success 'kvs: unlink works' ' flux kvs unlink $KEY.double && - test_must_fail flux kvs get $KEY.double + test_must_fail flux kvs get --json $KEY.double ' test_expect_success 'kvs: unlink works' ' flux kvs unlink $KEY.string && - test_must_fail flux kvs get $KEY.string + test_must_fail flux kvs get --json $KEY.string ' test_expect_success 'kvs: unlink works' ' flux kvs unlink $KEY.emptystring && - test_must_fail flux kvs get $KEY.emptystring + test_must_fail flux kvs get --json $KEY.emptystring ' test_expect_success 'kvs: unlink works' ' flux kvs unlink $KEY.jsonnull && - test_must_fail flux kvs get $KEY.jsonnull + test_must_fail flux kvs get --json $KEY.jsonnull ' test_expect_success 'kvs: unlink works' ' flux kvs unlink $KEY.strnull && - test_must_fail flux kvs get $KEY.strnull + test_must_fail flux kvs get --json $KEY.strnull ' test_expect_success 'kvs: unlink works' ' flux kvs unlink $KEY.booleantrue && - test_must_fail flux kvs get $KEY.booleantrue + test_must_fail flux kvs get --json $KEY.booleantrue ' test_expect_success 'kvs: unlink works' ' flux kvs unlink $KEY.booleanfalse && - test_must_fail flux kvs get $KEY.booleanfalse + test_must_fail flux kvs get --json $KEY.booleanfalse ' test_expect_success 'kvs: unlink works' ' flux kvs unlink $KEY.array && - test_must_fail flux kvs get $KEY.array + test_must_fail flux kvs get --json $KEY.array ' test_expect_success 'kvs: unlink works' ' flux kvs unlink $KEY.object && - test_must_fail flux kvs get $KEY.object + test_must_fail flux kvs get --json $KEY.object ' test_expect_success 'kvs: unlink dir works' ' flux kvs unlink $SUBDIR1 && @@ -194,7 +194,7 @@ test_expect_success 'kvs: put (multiple)' ' flux kvs put --json $KEY.a=42 $KEY.b=3.14 $KEY.c=foo $KEY.d=true $KEY.e="[1,3,5]" $KEY.f="{\"a\":42}" ' test_expect_success 'kvs: get (multiple)' ' - flux kvs get $KEY.a $KEY.b $KEY.c $KEY.d $KEY.e $KEY.f >output && + flux kvs get --json $KEY.a $KEY.b $KEY.c $KEY.d $KEY.e $KEY.f >output && cat >expected < /dev/null)" != "$2" ] && [ $i -lt 50 ] + while [ "$(flux kvs get --json $1 2> /dev/null)" != "$2" ] && [ $i -lt 50 ] do sleep 0.1 i=$((i + 1)) @@ -740,7 +740,7 @@ wait_watch_put() { wait_watch_empty() { i=0 - while flux kvs get $1 2> /dev/null && [ $i -lt 50 ] + while flux kvs get --json $1 2> /dev/null && [ $i -lt 50 ] do sleep 0.1 i=$((i + 1)) diff --git a/t/t1002-kvs-extra.t b/t/t1002-kvs-extra.t index 859c665825e8..7084893a3ea6 100755 --- a/t/t1002-kvs-extra.t +++ b/t/t1002-kvs-extra.t @@ -29,7 +29,7 @@ KEY=test.a.b.c # # test_kvs_key() { - flux kvs get "$1" >output + flux kvs get --json "$1" >output echo "$2" >expected test_cmp output expected #if ! test "$OUTPUT" = "$2"; then @@ -201,7 +201,7 @@ test_expect_success 'kvs: put-treeobj: clobbers destination' ' flux kvs put --json $TEST.a=42 && ${KVSBASIC} get-treeobj . >snapshot2 && ${KVSBASIC} put-treeobj $TEST.a="`cat snapshot2`" && - ! flux kvs get $TEST.a && + ! flux kvs get --json $TEST.a && flux kvs dir $TEST.a ' @@ -294,7 +294,7 @@ test_expect_success 'kvs: valref that points to content store data can be read' flux kvs unlink -Rf $TEST && echo "$largeval" | flux content store && ${KVSBASIC} put-treeobj $TEST.largeval2="{\"data\":[\"${largevalhash}\"],\"type\":\"valref\",\"ver\":1}" && - flux kvs get $TEST.largeval2 | grep $largeval + flux kvs get --json $TEST.largeval2 | grep $largeval ' test_expect_success 'kvs: valref that points to zero size content store data can be read' ' @@ -400,13 +400,13 @@ test_expect_success 'kvs: store 3x4 directory tree using kvsdir_put functions' ' test_expect_success 'kvs: put on rank 0, exists on all ranks' ' flux kvs put --json $TEST.xxx=99 && VERS=$(flux kvs version) && - flux exec sh -c "flux kvs wait ${VERS} && flux kvs get $TEST.xxx" + flux exec sh -c "flux kvs wait ${VERS} && flux kvs get --json $TEST.xxx" ' test_expect_success 'kvs: unlink on rank 0, does not exist all ranks' ' flux kvs unlink -Rf $TEST.xxx && VERS=$(flux kvs version) && - flux exec sh -c "flux kvs wait ${VERS} && ! flux kvs get $TEST.xxx" + flux exec sh -c "flux kvs wait ${VERS} && ! flux kvs get --json $TEST.xxx" ' # commit test diff --git a/t/t2000-wreck.t b/t/t2000-wreck.t index 83d6a07dbf12..6ac1c1cb740a 100755 --- a/t/t2000-wreck.t +++ b/t/t2000-wreck.t @@ -182,25 +182,25 @@ test_expect_success 'wreckrun: -N without -n works' ' test_expect_success 'wreckrun: -N without -n sets ntasks in kvs' ' flux wreckrun -l -N${SIZE} /bin/true && LWJ=$(last_job_path) && - n=$(flux kvs get ${LWJ}.ntasks) && + n=$(flux kvs get --json ${LWJ}.ntasks) && test "$n" = "${SIZE}" ' test_expect_success 'wreckrun: -n without -N sets nnnodes in kvs' ' flux wreckrun -l -n${SIZE} /bin/true && LWJ=$(last_job_path) && - n=$(flux kvs get ${LWJ}.nnodes) && + n=$(flux kvs get --json ${LWJ}.nnodes) && test "$n" = "${SIZE}" ' test_expect_success 'wreckrun: -t1 -N${SIZE} sets ntasks in kvs' ' flux wreckrun -l -t1 -N${SIZE} /bin/true && LWJ=$(last_job_path) && - n=$(flux kvs get ${LWJ}.ntasks) && + n=$(flux kvs get --json ${LWJ}.ntasks) && test "$n" = "${SIZE}" ' test_expect_success 'wreckrun: -t1 -n${SIZE} sets nnodes in kvs' ' flux wreckrun -l -t1 -n${SIZE} /bin/true && LWJ=$(last_job_path) && - n=$(flux kvs get ${LWJ}.nnodes) && + n=$(flux kvs get --json ${LWJ}.nnodes) && test "$n" = "${SIZE}" ' @@ -374,7 +374,7 @@ test_expect_success NO_SCHED 'flux-submit: returns ENOSYS when sched not loaded' check_complete_link() { for i in `seq 0 5`; do lastepoch=$(flux kvs dir lwj-complete | awk -F. '{print $2}' | sort -n | tail -1) - flux kvs get lwj-complete.${lastepoch}.${1}.state && return 0 + flux kvs get --json lwj-complete.${lastepoch}.${1}.state && return 0 sleep 0.2 done return 1 diff --git a/t/t2001-jsc.t b/t/t2001-jsc.t index e5585b1e5adb..5901e732337e 100755 --- a/t/t2001-jsc.t +++ b/t/t2001-jsc.t @@ -208,7 +208,7 @@ test_expect_success 'jstat 8: query detects bad inputs' ' test_expect_success 'jstat 9: update state-pair' " flux jstat update 1 state-pair '{\"state-pair\": {\"ostate\": 13, \"nstate\": 12}}' && - flux kvs get $(flux wreck kvs-path 1).state > output.9.1 && + flux kvs get --json $(flux wreck kvs-path 1).state > output.9.1 && cat >expected.9.1 <<-EOF && cancelled EOF @@ -216,15 +216,15 @@ EOF " test_expect_success 'jstat 10: update procdescs' " - flux kvs get $(flux wreck kvs-path 1).0.procdesc > output.10.1 && + flux kvs get --json $(flux wreck kvs-path 1).0.procdesc > output.10.1 && flux jstat update 1 pdesc '{\"pdesc\": {\"procsize\":1, \"hostnames\":[\"0\"], \"executables\":[\"fake\"], \"pdarray\":[{\"pid\":8482,\"eindx\":0,\"hindx\":0}]}}' && - flux kvs get $(flux wreck kvs-path 1).0.procdesc > output.10.2 && + flux kvs get --json $(flux wreck kvs-path 1).0.procdesc > output.10.2 && test_expect_code 1 diff output.10.1 output.10.2 " test_expect_success 'jstat 11: update rdesc' " flux jstat update 1 rdesc '{\"rdesc\": {\"nnodes\": 128, \"ntasks\": 128, \"walltime\":3600}}' && - flux kvs get $(flux wreck kvs-path 1).ntasks > output.11.1 && + flux kvs get --json $(flux wreck kvs-path 1).ntasks > output.11.1 && cat > expected.11.1 <<-EOF && 128 EOF @@ -233,7 +233,7 @@ EOF test_expect_success 'jstat 12: update rdl' " flux jstat update 1 rdl '{\"rdl\": \"fake_rdl_string\"}' && - flux kvs get $(flux wreck kvs-path 1).rdl > output.12.1 && + flux kvs get --json $(flux wreck kvs-path 1).rdl > output.12.1 && cat > expected.12.1 <<-EOF && fake_rdl_string EOF @@ -242,7 +242,7 @@ EOF test_expect_success 'jstat 13: update rdl_alloc' " flux jstat update 1 rdl_alloc '{\"rdl_alloc\": [{\"contained\": {\"cmbdrank\": 0, \"cmbdncores\": 102}}]}' && - flux kvs get $(flux wreck kvs-path 1).rank.0.cores > output.13.1 && + flux kvs get --json $(flux wreck kvs-path 1).rank.0.cores > output.13.1 && cat > expected.13.1 <<-EOF && 102 EOF diff --git a/t/t2002-pmi.t b/t/t2002-pmi.t index 6c25e58a8a35..c945e7ce423f 100755 --- a/t/t2002-pmi.t +++ b/t/t2002-pmi.t @@ -93,7 +93,7 @@ test_expect_success 'pmi: wreck preputs PMI_process_mapping into kvs' ' #!/bin/sh if test \${FLUX_TASK_RANK} -eq 0; then KVS_PATH=\$(flux wreck kvs-path \${FLUX_JOB_ID}) - flux kvs get \${KVS_PATH}.pmi.PMI_process_mapping + flux kvs get --json \${KVS_PATH}.pmi.PMI_process_mapping fi EOF chmod +x print-pmi-map.sh && diff --git a/t/t2005-hwloc-basic.t b/t/t2005-hwloc-basic.t index d8c4ea04cea4..cc79aa606fe5 100755 --- a/t/t2005-hwloc-basic.t +++ b/t/t2005-hwloc-basic.t @@ -115,16 +115,16 @@ test_expect_success HAVE_LSTOPO 'hwloc: test failure of lstopo command' ' ' test_expect_success 'hwloc: no broken down resource info by default' ' - test_must_fail flux kvs get resource.hwloc.by_rank.0.Machine_0.OSName + test_must_fail flux kvs get --json resource.hwloc.by_rank.0.Machine_0.OSName ' test_expect_success 'hwloc: reload --walk-topology=yes works' ' flux hwloc reload --walk-topology=yes && - flux kvs get resource.hwloc.by_rank.0.Machine_0.OSName + flux kvs get --json resource.hwloc.by_rank.0.Machine_0.OSName ' test_expect_success 'hwloc: reload --walk-topology=no removes broken down topo' ' flux hwloc reload --walk-topology=no && - test_must_fail flux kvs get resource.hwloc.by_rank.0.Machine_0.OSName + test_must_fail flux kvs get --json resource.hwloc.by_rank.0.Machine_0.OSName ' test_expect_success 'hwloc: reload fails on invalid rank' ' @@ -133,7 +133,7 @@ test_expect_success 'hwloc: reload fails on invalid rank' ' ' test_expect_success 'hwloc: HostName is populated in by_rank' ' - HW_HOST=$(flux kvs get resource.hwloc.by_rank.0.HostName) && + HW_HOST=$(flux kvs get --json resource.hwloc.by_rank.0.HostName) && REAL_HOST=$(hostname) && test x"$HW_HOST" = x"$REAL_HOST" '