Skip to content

Commit

Permalink
Merge pull request #513 from tpatki/issue494
Browse files Browse the repository at this point in the history
resource: add set- and get-property support to match service
  • Loading branch information
dongahn committed Sep 20, 2019
2 parents 17553f7 + f6dfbf1 commit a317aad
Show file tree
Hide file tree
Showing 4 changed files with 293 additions and 0 deletions.
123 changes: 123 additions & 0 deletions resource/modules/resource_match.cpp
Expand Up @@ -108,12 +108,20 @@ static void stat_request_cb (flux_t *h, flux_msg_handler_t *w,
static void next_jobid_request_cb (flux_t *h, flux_msg_handler_t *w,
const flux_msg_t *msg, void *arg);

static void set_property_request_cb (flux_t *h, flux_msg_handler_t *w,
const flux_msg_t *msg, void *arg);

static void get_property_request_cb (flux_t *h, flux_msg_handler_t *w,
const flux_msg_t *msg, void *arg);

static const struct flux_msg_handler_spec htab[] = {
{ FLUX_MSGTYPE_REQUEST, "resource.match", match_request_cb, 0},
{ FLUX_MSGTYPE_REQUEST, "resource.cancel", cancel_request_cb, 0},
{ FLUX_MSGTYPE_REQUEST, "resource.info", info_request_cb, 0},
{ FLUX_MSGTYPE_REQUEST, "resource.stat", stat_request_cb, 0},
{ FLUX_MSGTYPE_REQUEST, "resource.next_jobid", next_jobid_request_cb, 0},
{ FLUX_MSGTYPE_REQUEST, "resource.set_property", set_property_request_cb, 0},
{ FLUX_MSGTYPE_REQUEST, "resource.get_property", get_property_request_cb, 0},
FLUX_MSGHANDLER_TABLE_END
};

Expand Down Expand Up @@ -816,6 +824,121 @@ static void next_jobid_request_cb (flux_t *h, flux_msg_handler_t *w,
flux_log_error (h, "%s: flux_respond_error", __FUNCTION__);
}

static void set_property_request_cb (flux_t *h, flux_msg_handler_t *w,
const flux_msg_t *msg, void *arg)
{
const char *rp = NULL, *kv = NULL;
string resource_path = "", keyval = "";
string property_key = "", property_value = "";
size_t pos;
resource_ctx_t *ctx = getctx ((flux_t *)arg);
std::map<std::string, std::vector <vtx_t>>::const_iterator it;
std::pair<std::map<std::string,std::string>::iterator, bool> ret;
vtx_t v;

if (flux_request_unpack (msg, NULL, "{s:s s:s}",
"sp_resource_path", &rp,
"sp_keyval", &kv) < 0)
goto error;

resource_path = rp;
keyval = kv;

pos = keyval.find ('=');

if (pos == 0 || (pos == keyval.size () - 1) || pos == string::npos) {
errno = EINVAL;
flux_log_error (h,
"Incorrect format.\nUse set-property <resource> PROPERTY=VALUE");
goto error;
}

property_key = keyval.substr (0, pos);
property_value = keyval.substr (pos + 1);

it = ctx->db.by_path.find (resource_path);

if (it == ctx->db.by_path.end ()) {
errno = ENOENT;
flux_log_error (h, "Couldn't find %s in resource graph.",
resource_path.c_str ());
goto error;
}

v = it->second[0];

ret = ctx->db.resource_graph[v].properties.insert (
std::pair<std::string, std::string> (property_key,property_value));

if (ret.second == false) {
ctx->db.resource_graph[v].properties.erase (property_key);
ctx->db.resource_graph[v].properties.insert (
std::pair<std::string, std::string> (property_key,property_value));
}

if (flux_respond_pack (h, msg, "{}") < 0)
flux_log_error (h, "%s", __FUNCTION__);

return;

error:
if (flux_respond_error (h, msg, errno, NULL) < 0)
flux_log_error (h, "%s: flux_respond_error", __FUNCTION__);
}

static void get_property_request_cb (flux_t *h, flux_msg_handler_t *w,
const flux_msg_t *msg, void *arg)
{
const char *rp = NULL, *gp_key = NULL;
string resource_path = "", property_key = "";
resource_ctx_t *ctx = getctx ((flux_t *)arg);
std::map<std::string, std::vector <vtx_t>>::const_iterator it;
std::map<std::string, std::string>::const_iterator p_it;
vtx_t v;
string resp_value = "";

if (flux_request_unpack (msg, NULL, "{s:s s:s}",
"gp_resource_path", &rp,
"gp_key", &gp_key) < 0)
goto error;

resource_path = rp;
property_key = gp_key;

it = ctx->db.by_path.find (resource_path);

if (it == ctx->db.by_path.end ()) {
errno = ENOENT;
flux_log_error (h, "Couldn't find %s in resource graph.",
resource_path.c_str ());
goto error;
}

v = it->second[0];

for (p_it = ctx->db.resource_graph[v].properties.begin ();
p_it != ctx->db.resource_graph[v].properties.end (); p_it++) {

if (property_key.compare (p_it->first) == 0)
resp_value = p_it->second;
}

if (resp_value.empty ()) {
errno = ENOENT;
flux_log_error (h,"Property %s was not found for resource %s.",
property_key.c_str (), resource_path.c_str ());
goto error;
}

if (flux_respond_pack (h, msg, "{s:s}", "value", resp_value.c_str ()) < 0)
flux_log_error (h, "%s", __FUNCTION__);

return;

error:
if (flux_respond_error (h, msg, errno, NULL) < 0)
flux_log_error (h, "%s: flux_respond_error", __FUNCTION__);
}

/******************************************************************************
* *
Expand Down
1 change: 1 addition & 0 deletions t/Makefile.am
Expand Up @@ -54,6 +54,7 @@ TESTS = \
t4003-cancel-info.t \
t4004-match-hwloc.t \
t4005-match-unsat.t \
t4006-properties.t \
t5000-valgrind.t \
t6000-graph-size.t \
t6001-match-formats.t
Expand Down
49 changes: 49 additions & 0 deletions t/scripts/flux-resource
Expand Up @@ -53,6 +53,16 @@ class ResourceModuleInterface:
def rpc_cancel (self, jobid):
payload = {'jobid' : jobid}
return self.f.rpc ("resource.cancel", payload).get ()

def rpc_set_property (self, sp_resource_path, sp_keyval):
payload = {'sp_resource_path' : sp_resource_path,
'sp_keyval' : sp_keyval}
return self.f.rpc ("resource.set_property", payload).get ()

def rpc_get_property (self, gp_resource_path, gp_key):
payload = {'gp_resource_path' : gp_resource_path, 'gp_key' : gp_key}
return self.f.rpc ("resource.get_property", payload).get ()


"""
Action for match allocate sub-command
Expand Down Expand Up @@ -129,6 +139,25 @@ def stat_action (args):
print "Max. Match Time: ", resp['max-match'], "Secs"
print "Avg. Match Time: ", resp['avg-match'], "Secs"

"""
Action for set-property sub-command
"""
def set_property_action (args):
r = ResourceModuleInterface ()
sp_resource_path = args.sp_resource_path
sp_keyval = args.sp_keyval
resp = r.rpc_set_property (sp_resource_path, sp_keyval)

"""
Action for get-property sub-command
"""
def get_property_action (args):
r = ResourceModuleInterface ()
gp_resource_path = args.gp_resource_path
gp_key = args.gp_key
resp = r.rpc_get_property (gp_resource_path, gp_key)
print args.gp_key, "=", resp['value']

"""
Main entry point
"""
Expand All @@ -154,10 +183,14 @@ def main ():
istr = "Print info on a single job"
sstr = "Print overall performance statistics"
cstr = "Cancel an allocated or reserved job"
pstr = "Set property-key=value for specified resource."
gstr = "Get value for specified resource and property-key."
parser_m = subpar.add_parser ('match', help=mstr, description=mstr)
parser_i = subpar.add_parser ('info', help=istr, description=istr)
parser_s = subpar.add_parser ('stat', help=sstr, description=sstr)
parser_c = subpar.add_parser ('cancel', help=cstr, description=cstr)
parser_sp = subpar.add_parser ('set-property', help=pstr, description=pstr)
parser_gp = subpar.add_parser ('get-property', help=gstr, description=gstr)

#
# Add subparser for the match sub-command
Expand Down Expand Up @@ -214,6 +247,22 @@ def main ():
help='Jobspec file name')
parser_mr.set_defaults (func=match_reserve_action)

# Positional arguments for set-property sub-command
#
parser_sp.add_argument ('sp_resource_path', metavar='ResourcePath',
type=str, help='set-property resource_path property-key=val')
parser_sp.add_argument ('sp_keyval', metavar='PropertyKeyVal', type=str,
help='set-property resource_path property-key=val')
parser_sp.set_defaults (func=set_property_action)

# Positional argument for get-property sub-command
#
parser_gp.add_argument ('gp_resource_path', metavar='ResourcePath',
type=str, help='get-property resource_path property-key')
parser_gp.add_argument ('gp_key', metavar='PropertyKey', type=str,
help='get-property resource_path property-key')
parser_gp.set_defaults (func=get_property_action)

#
# Parse the args and call an action routine as part of that
#
Expand Down
120 changes: 120 additions & 0 deletions t/t4006-properties.t
@@ -0,0 +1,120 @@
#!/bin/sh
#set -x

test_description='Test the basic functionality of properties (get/set) within resource
'

ORIG_HOME=${HOME}

. `dirname $0`/sharness.sh

#
# sharness modifies $HOME environment variable, but this interferes
# with python's package search path, in particular its user site package.
#
HOME=${ORIG_HOME}

grug="${SHARNESS_TEST_SRCDIR}/data/resource/grugs/tiny.graphml"
jobspec="${SHARNESS_TEST_SRCDIR}/data/resource/jobspecs/basics/test001.yaml"

#
# test_under_flux is under sharness.d/
#
test_under_flux 1

#
# print only with --debug
#

test_debug '
echo ${grug} &&
echo ${jobspec}
'

test_expect_success 'loading resource module with a tiny machine config works' '
flux module load resource grug-conf=${grug} prune-filters=ALL:core \
subsystems=containment policy=high
'

test_expect_success 'set/get property basic test works' '
flux resource set-property /tiny0/rack0/node0 class=one &&
flux resource get-property /tiny0/rack0/node0 class > sp.0 &&
echo "class = one" > expected &&
test_cmp expected sp.0
'

test_expect_success 'set/get property multiple resources works' '
flux resource set-property /tiny0/rack0/node0 nodeprop=1 &&
flux resource set-property /tiny0/rack0/node0/socket1 sockprop=abc &&
flux resource set-property /tiny0/rack0/node1/socket0/core17 coreprop=z &&
flux resource get-property /tiny0/rack0/node0 nodeprop > sp.1 &&
flux resource get-property /tiny0/rack0/node0/socket1 sockprop >> sp.1 &&
flux resource get-property /tiny0/rack0/node1/socket0/core17 coreprop >> sp.1 &&
cat <<-EOF >expected &&
nodeprop = 1
sockprop = abc
coreprop = z
EOF
test_cmp expected sp.1
'

test_expect_success 'set/get property multiple properties works' '
flux resource set-property /tiny0/rack0/node0 prop1=a &&
flux resource set-property /tiny0/rack0/node0 prop2=foo &&
flux resource set-property /tiny0/rack0/node0 prop3=123 &&
flux resource set-property /tiny0/rack0/node0 prop4=bar &&
flux resource set-property /tiny0/rack0/node0 prop5=baz &&
flux resource get-property /tiny0/rack0/node0 prop1 > sp.2 &&
flux resource get-property /tiny0/rack0/node0 prop2 >> sp.2 &&
flux resource get-property /tiny0/rack0/node0 prop3 >> sp.2 &&
flux resource get-property /tiny0/rack0/node0 prop4 >> sp.2 &&
flux resource get-property /tiny0/rack0/node0 prop5 >> sp.2 &&
cat <<-EOF >expected &&
prop1 = a
prop2 = foo
prop3 = 123
prop4 = bar
prop5 = baz
EOF
test_cmp expected sp.2
'

test_expect_success 'test with no path works' '
test_expect_code 1 flux resource set-property /dont/exist random=1
'

test_expect_success 'test with no property works' '
test_expect_code 1 flux resource get-property /tiny0/rack0/node0 dontexist
'

test_expect_success 'test with malformed inputs works' '
test_expect_code 1 flux resource set-property /tiny0/rack0/node0 badprop &&
test_expect_code 1 flux resource get-property /tiny0/rack0/node0 badprop &&
test_expect_code 1 flux resource set-property /tiny0/rack0/node0 badprop= &&
test_expect_code 1 flux resource get-property /tiny0/rack0/node0 badprop &&
test_expect_code 1 flux resource set-property /tiny0/rack0/node0 =badprop &&
test_expect_code 1 flux resource get-property /tiny0/rack0/node0 badprop &&
test_expect_code 1 flux resource set-property /tiny0/rack0/node0 = &&
test_expect_code 1 flux resource get-property /tiny0/rack0/node0 badprop
'

test_expect_success 'test with complex inputs works' '
flux resource set-property /tiny0/rack0/node0 badprop==1 &&
flux resource get-property /tiny0/rack0/node0 badprop > sp.5 &&
flux resource set-property /tiny0/rack0/node0 badprop=1=class=random &&
flux resource get-property /tiny0/rack0/node0 badprop >> sp.5 &&
flux resource set-property /tiny0/rack0/node0 badprop=1 &&
flux resource get-property /tiny0/rack0/node0 badprop >> sp.5 &&
cat <<-EOF >expected &&
badprop = =1
badprop = 1=class=random
badprop = 1
EOF
test_cmp expected sp.5
'

test_expect_success 'removing resource works' '
flux module remove resource
'

test_done

0 comments on commit a317aad

Please sign in to comment.