Skip to content

Commit

Permalink
Related-Bug: #1463208 - Bug fixes for Cliff Client based on Testing [1]
Browse files Browse the repository at this point in the history
This check-in has bug fixes for following bugs filed by Ritam:

- From SM client cli expansion/options don’t show up after ‘delete/display server -<tab>’ or ‘delete/display server --<tab>’.
- Server-manager-client delete server --where <where clause> , this gives ‘Match key not present’ error.
- Where clause doesn’t work for cluster deletion, same issue as no 5 above.
- In case of empty list display I see SM client cli and shell throwing a error “list index out of range”. Don’t see this happening when I use the -d option.
- Server-manager-client status server –detail/-d ---- I feel this option should be removed as there is nothing to display in detail over here.
- Mac id or id mandatory for modify
- Add tag/ Update tag (server-manager-client add/update tag --tags tag1=abc.tag2=cdf) -> JSON NOT TESTED
- Add server <enter> asks to specify the id to edit the object. Please change the message to say specify id to add the object.
- Add server --id <server_id> <tab> does not show the parameters.
- I feel these parameters should not be made mandatory. Without these server addition fails. cluster_id, password

Additional features:
- (server-manager-client) add server -f /root/sm_file   >>>>>>>>>>>>> Path doesn’t auto complete. Absolute paths should auto complete from the cli shell as well.
- While in the cli shell on a tab it shows all the available command options like display add delete etc. There should be a mention of q/quit/exit as the shell exit option as well.
- tab-completion of image/package in reimage/provision command - name comes after option given: reimage/provision --cluster_id abc [TAB] : shows image_id/package_id respectively.

Change-Id: I5727ced3b4e6e9508318b3365ad14c6ab04ffb1b
  • Loading branch information
nitishkrishna committed Oct 9, 2015
1 parent 58e2888 commit ee8b619
Show file tree
Hide file tree
Showing 9 changed files with 328 additions and 173 deletions.
2 changes: 2 additions & 0 deletions src/smgr_cliff_client/smgrcliapp/commandmanager.py
Expand Up @@ -56,13 +56,15 @@ def get_command_groups(self):

def get_added_commands(self):
command_list = []
default_command_list = ['quit', 'exit', 'help']
group_list = self.get_command_groups()
for ns in group_list:
for ep in pkg_resources.iter_entry_points(ns):
cmd_name = (ep.name.replace('_', ' ')
if self.convert_underscores
else ep.name)
command_list.append(cmd_name)
command_list += default_command_list
return command_list


287 changes: 174 additions & 113 deletions src/smgr_cliff_client/smgrcliapp/interactive.py

Large diffs are not rendered by default.

69 changes: 54 additions & 15 deletions src/smgr_cliff_client/smgrcliapp/smgr_add.py
Expand Up @@ -42,8 +42,7 @@ def get_description(self):
def get_parser(self, prog_name):

self.smgr_objects = ["server", "cluster", "image", "tag"]
self.mandatory_params["server"] = ['id', 'cluster_id', 'mac_address', 'ip_address', 'ipmi_address', 'password',
'subnet_mask', 'gateway']
self.mandatory_params["server"] = ['id', 'mac_address', 'ip_address', 'ipmi_address', 'subnet_mask', 'gateway']
self.mandatory_params["cluster"] = ['id']
self.mandatory_params["image"] = ['id', 'category', 'version', 'type', 'path']
self.mandatory_params["tag"] = []
Expand All @@ -64,7 +63,7 @@ def get_parser(self, prog_name):
"server", help='Create server')
parser_server.add_argument(
"--file_name", "-f",
help="json file containing server param values", default=None)
help="json file containing server param values", dest="file_name", default=None)
for param in self.object_dict["server"]:
if param not in self.multilevel_param_classes["server"]:
parser_server.add_argument(
Expand All @@ -76,9 +75,11 @@ def get_parser(self, prog_name):
# Subparser for server tags edit
parser_tag = subparsers.add_parser(
"tag", help='Create tags')
parser_tag.add_argument(
"--tags", help="Comma separated list of tag_number=tag_name pairs.", default=None)
parser_tag.add_argument(
"--file_name", "-f",
help="json file containing tag values", default=None)
help="json file containing tag values", dest="file_name", default=None)

# Subparser for cluster edit
parser_cluster = subparsers.add_parser(
Expand All @@ -92,7 +93,7 @@ def get_parser(self, prog_name):
)
parser_cluster.add_argument(
"--file_name", "-f",
help="json file containing cluster param values", default=None)
help="json file containing cluster param values", dest="file_name", default=None)

# Subparser for image edit
parser_image = subparsers.add_parser(
Expand All @@ -106,10 +107,12 @@ def get_parser(self, prog_name):
)
parser_image.add_argument(
"--file_name", "-f",
help="json file containing image param values", default=None)
help="json file containing image param values", dest="file_name", default=None)

for obj in self.smgr_objects:
self.command_dictionary[str(obj)] = ['f', 'file_name']
if obj == "tag":
self.command_dictionary[str(obj)] += ['tags']
for key in self.command_dictionary:
new_dict = dict()
new_dict[key] = [str("--" + s) for s in self.command_dictionary[key] if len(s) > 1]
Expand Down Expand Up @@ -198,6 +201,27 @@ def merge_with_defaults(self, object_item, payload):

# end merge_with_defaults

def verify_added_tags(self, obj, obj_payload):
existing_tags = smgrutils.send_REST_request(
self.smgr_ip, self.smgr_port,
obj="tag", detail=True, method="GET")
tag_dict = json.loads(existing_tags)
rev_tag_dict = dict((v, k) for k, v in tag_dict.iteritems())
allowed_tags = self.object_dict["tag"].keys()
if obj == "tag":
for tag_idx in obj_payload:
if tag_idx not in allowed_tags:
self.app.print_error_message_and_quit("\nThe tag " + str(tag_idx) +
" is not a valid tag index. Please use tags1-7\n\n")
elif obj == "server":
added_tag_dict = obj_payload["tag"]
added_tags = added_tag_dict.keys()
for tag in added_tags:
if tag not in rev_tag_dict:
self.app.print_error_message_and_quit("\nThe tag " + str(tag) +
" has been added to server config but hasn't been"
" added as a user defined tag. Add this tag first\n\n")

def pairwise(self, iterable):
a = iter(iterable)
return izip(a, a)
Expand Down Expand Up @@ -343,6 +367,14 @@ def add_object(self, obj, parsed_args, remaining_args=None):
self.parse_remaining_args(obj, obj_payload, multilevel_obj_params, remaining_args)
return obj_payload

def add_tag(self, parsed_args, remaining_args=None):
tag_payload = {}
allowed_tags = self.object_dict["tag"].keys()
if hasattr(parsed_args, "tags"):
add_tag_dict = self.process_val(getattr(parsed_args, "tags", None))
tag_payload = {k: v for k, v in add_tag_dict.iteritems() if k in allowed_tags}
return tag_payload

def take_action(self, parsed_args, remaining_args=None):

try:
Expand All @@ -367,18 +399,25 @@ def take_action(self, parsed_args, remaining_args=None):
try:
if getattr(parsed_args, "file_name", None) and smgr_obj in self.smgr_objects:
payload = json.load(open(parsed_args.file_name))
for obj_payload in payload[str(smgr_obj)]:
if "tag" in obj_payload and smgr_obj == "server":
self.verify_edited_tags(smgr_obj, obj_payload)
if "id" not in obj_payload:
self.app.print_error_message_and_quit("No id specified for object being added")
elif not getattr(parsed_args, "id", None):
if smgr_obj == "tag":
self.verify_added_tags(smgr_obj, payload)
else:
for obj_payload in payload[str(smgr_obj)]:
if "tag" in obj_payload and smgr_obj == "server":
self.verify_added_tags(smgr_obj, obj_payload)
if "id" not in obj_payload and smgr_obj != "tag":
self.app.print_error_message_and_quit("No id specified for object being added")
elif not (getattr(parsed_args, "id", None) or getattr(parsed_args, "mac_address", None)) \
and smgr_obj != "tag":
# 1. Check if parsed args has id for object
self.app.print_error_message_and_quit(
"\nYou need to specify the id to edit an object (Arguement --id).\n")
self.app.print_error_message_and_quit("\nYou need to specify the id or mac_address to add an object"
" (Arguement --id/--mac_address).\n")
elif smgr_obj not in self.smgr_objects:
self.app.print_error_message_and_quit(
"\nThe object: " + str(smgr_obj) + " is not a valid one.\n")
elif smgr_obj == "tag":
payload = self.add_tag(parsed_args, remaining_args)
self.verify_added_tags(smgr_obj, payload)
else:
payload = {}
# 2. Check that id duplicate doesn't exist for this added object
Expand All @@ -402,7 +441,7 @@ def take_action(self, parsed_args, remaining_args=None):
# 5. Verify tags and mandatory params added for given object
for obj_payload in payload[smgr_obj]:
if "tag" in obj_payload and smgr_obj == "server":
self.verify_edited_tags(smgr_obj, obj_payload)
self.verify_added_tags(smgr_obj, obj_payload)
mandatory_params_set = set(self.mandatory_params[smgr_obj])
added_params_set = set(obj_payload.keys())
if mandatory_params_set.difference(added_params_set):
Expand Down
4 changes: 2 additions & 2 deletions src/smgr_cliff_client/smgrcliapp/smgr_cli_main.py
Expand Up @@ -99,9 +99,9 @@ def build_option_parser(self, description, version,
)
parser.add_argument(
'-c', '--config_file',
default='/tmp/sm-client-config.ini',
default='/etc/contrail/sm-client-config.ini',
help='The ini file that specifies the default parameter values for Objects like Cluster, Server, etc.'
'Default is /tmp/sm-client-config.ini'
'Default is /etc/contrail/sm-client-config.ini'
)
return parser

Expand Down
4 changes: 3 additions & 1 deletion src/smgr_cliff_client/smgrcliapp/smgr_client_utils.py
Expand Up @@ -249,6 +249,8 @@ def convert_json_to_table(obj, json_resp, select_item=None):
if len(data_dict.keys()) == 1 and obj != "tag":
obj_type, obj_value = data_dict.popitem()
dict_list = eval(str(obj_value))
if len(dict_list) == 0:
return []
sample_dict = dict(dict_list[0])
sameple_dict_key_list = sample_dict.keys()
sameple_dict_key_list.remove("id")
Expand Down Expand Up @@ -293,7 +295,7 @@ def convert_json_to_table(obj, json_resp, select_item=None):
data_dict = {}
if isinstance(data_item, dict):
data_dict = data_item
elif isinstance(data_item, list):
elif isinstance(data_item, list) and len(data_item) >= 1:
data_dict = data_item[0]
elif data_item:
data_dict[str(select_item)] = data_item
Expand Down
42 changes: 33 additions & 9 deletions src/smgr_cliff_client/smgrcliapp/smgr_delete.py
Expand Up @@ -53,10 +53,13 @@ def get_parser(self, prog_name):
group.add_argument("--tag",
help=("tag values for the server to be deleted "
"in t1=v1,t2=v2,... format"))
group.add_argument("--discovered",
help=("flag to get list of "
"newly discovered server(s)"))
group.add_argument("--where",
help=("sql where statement in quotation marks"))
parser_server.set_defaults(which='server')
self.command_dictionary["server"] = ['server_id', 'mac', 'ip', 'cluster_id', 'tag', 'where']
self.command_dictionary["server"] = ['server_id', 'mac', 'ip', 'cluster_id', 'tag', 'where', 'discovered']

# Subparser for cluster delete
parser_cluster = subparsers.add_parser(
Expand All @@ -66,9 +69,6 @@ def get_parser(self, prog_name):
help=("cluster id for cluster to be deleted"))
parser_cluster_group.add_argument("--where",
help=("sql where statement in quotation marks"))
parser_cluster.add_argument("--force", "-f", action="store_true",
help=("optional parameter to indicate ,"
"if cluster association to be removed from server"))
parser_cluster.set_defaults(which='cluster')
self.command_dictionary["cluster"] = ['cluster_id', 'where', 'f', 'force']

Expand All @@ -82,6 +82,14 @@ def get_parser(self, prog_name):
parser_image.set_defaults(which='image')
self.command_dictionary["image"] = ['image_id', 'where']

# Subparser for tag delete
parser_tag = subparsers.add_parser(
"tag", help='Delete tag')
parser_tag.add_argument("--tags",
help="comma separated list of tag indexes to delete. Eg: tag1,tag5")
parser_tag.set_defaults(which='tag')
self.command_dictionary["tag"] = ['tags']

for key in self.command_dictionary:
new_dict = dict()
new_dict[key] = [str("--" + s) for s in self.command_dictionary[key] if len(s) > 1]
Expand Down Expand Up @@ -112,6 +120,9 @@ def delete_server(self, parsed_args):
elif getattr(parsed_args, "discovered", None):
rest_api_params['match_key'] = 'discovered'
rest_api_params['match_value'] = getattr(parsed_args, "discovered", None)
elif getattr(parsed_args, "where", None):
rest_api_params['match_key'] = 'where'
rest_api_params['match_value'] = getattr(parsed_args, "where", None)
else:
rest_api_params['match_key'] = None
rest_api_params['match_value'] = None
Expand All @@ -122,7 +133,7 @@ def delete_cluster(self, parsed_args):
if getattr(parsed_args, "cluster_id", None):
match_key = 'id'
match_value = getattr(parsed_args, "cluster_id", None)
elif getattr(parsed_args, "server_id", None):
elif getattr(parsed_args, "where", None):
match_key = 'where'
match_value = getattr(parsed_args, "where", None)
else:
Expand Down Expand Up @@ -154,6 +165,21 @@ def delete_image(self, parsed_args):
return rest_api_params
#end def delete_image

def delete_tag(self, parsed_args):
if getattr(parsed_args, "tags", None):
match_key = 'tag'
match_value = getattr(parsed_args, "tags", None)
else:
match_key = ''
match_value = ''
rest_api_params = {
'object': 'tag',
'match_key': match_key,
'match_value': match_value
}
return rest_api_params
# end def delete_tag

def take_action(self, parsed_args):
try:
self.smgr_ip = self.smgr_port = None
Expand All @@ -177,15 +203,13 @@ def take_action(self, parsed_args):
rest_api_params = self.delete_cluster(parsed_args)
elif obj == "image":
rest_api_params = self.delete_image(parsed_args)
elif obj == "tag":
rest_api_params = self.delete_tag(parsed_args)

force = False
if hasattr(parsed_args, "force"):
force = getattr(parsed_args, "force")
if rest_api_params:
resp = smgrutils.send_REST_request(self.smgr_ip, self.smgr_port, obj=rest_api_params["object"],
match_key=rest_api_params["match_key"],
match_value=rest_api_params["match_value"],
force=force,
method="DELETE")
smgrutils.print_rest_response(resp)
self.app.stdout.write("\n" + str(smgrutils.print_rest_response(resp)) + "\n")

0 comments on commit ee8b619

Please sign in to comment.