From e63d7f9fe3756d6f438c1a4da7d5010f1caed103 Mon Sep 17 00:00:00 2001 From: Jarrod Johnson Date: Wed, 15 Nov 2017 09:41:01 -0500 Subject: [PATCH 1/5] Correct typo in the nodeshell command The November 6th change contained a typo. --- confluent_client/bin/nodeshell | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/confluent_client/bin/nodeshell b/confluent_client/bin/nodeshell index f80071d7..c1959d0d 100755 --- a/confluent_client/bin/nodeshell +++ b/confluent_client/bin/nodeshell @@ -107,7 +107,7 @@ def run(): run_cmdv(node, cmdv, all, pipedesc) for node in sortutil.natural_sort(pernodeout): for line in pernodeout[node]: - sys.stdout.ouwrite('{0}: {1}'.format(node, line)) + sys.stdout.write('{0}: {1}'.format(node, line)) sys.stdout.flush() if all: rdy, _, _ = select.select(all, [], [], 10) From 03293d88b085bfc8d5895b5feef943ad5ff35964 Mon Sep 17 00:00:00 2001 From: Jarrod Johnson Date: Wed, 15 Nov 2017 15:38:59 -0500 Subject: [PATCH 2/5] Have nodeeventlog print help on incorrect arguments --- confluent_client/bin/nodeeventlog | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/confluent_client/bin/nodeeventlog b/confluent_client/bin/nodeeventlog index 4614f81b..d5245e8d 100755 --- a/confluent_client/bin/nodeeventlog +++ b/confluent_client/bin/nodeeventlog @@ -45,9 +45,15 @@ except IndexError: sys.exit(1) client.check_globbing(noderange) deletemode = False +if len(sys.argv) > 3: + argparser.print_help() + sys.exit(1) if len(sys.argv) == 3: if sys.argv[2] == 'clear': deletemode = True + else: + argparser.print_help() + sys.exit(1) session = client.Command() exitcode = 0 From d69cca46d0e8a10410806baa7ccc60a7b7c416c5 Mon Sep 17 00:00:00 2001 From: Jarrod Johnson Date: Mon, 27 Nov 2017 10:04:23 -0500 Subject: [PATCH 3/5] Rework check_globbing to reduce false positives First, globbing can only be the cause of a mess up if the given noderange is a file that matches. With this we still have: for node in $(nodelist compute); do nodepower $node; done As a potential false positive if any node is a range. For this, offer suggestion of changing directories. Also, if it had been: for NODE in $(nodelist compute); do export NODE; nodepower $NODE; done Another clause can detect that, which has been added. --- confluent_client/confluent/client.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/confluent_client/confluent/client.py b/confluent_client/confluent/client.py index d3bfb66d..7f7c3670 100644 --- a/confluent_client/confluent/client.py +++ b/confluent_client/confluent/client.py @@ -469,15 +469,24 @@ def updateattrib(session, updateargs, nodetype, noderange, options): # if we glob to something, then bash will change noderange and this should # detect it and save the user from tragedy def check_globbing(noderange): + if not os.path.exists(noderange): + return True rawargs = os.environ.get('CURRENT_CMDLINE', None) if rawargs: rawargs = shlex.split(rawargs) for arg in rawargs: + if arg.startswith('$'): + arg = arg[1:] + if arg.endswith(';'): + arg = arg[:-1] + arg = os.environ.get(arg, '$' + arg) if arg.startswith(noderange): break else: sys.stderr.write( - 'Shell glob conflict detected, specified target {0} ' - 'not in command line (if bash, try set -f)' + 'Shell glob conflict detected, specified target "{0}" ' + 'not in command line, but is a file. You can use "set -f" in ' + 'bash or change directories such that there is no filename ' + 'that would conflict.' '\n'.format(noderange)) sys.exit(1) \ No newline at end of file From 72af8f1631478205319c74ae2a0f8cad1286ad68 Mon Sep 17 00:00:00 2001 From: Jarrod Johnson Date: Mon, 27 Nov 2017 10:36:29 -0500 Subject: [PATCH 4/5] Fix custom and net.* attributes for groups and alias clearing The _group function was not using fixup_attribute, add that. Additionally, on the clear_ functions, use the aliases to make clearing work with the shorthand as well. --- .../confluent/config/configmanager.py | 28 +++++++++++++++---- confluent_server/confluent/discovery/core.py | 2 +- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/confluent_server/confluent/config/configmanager.py b/confluent_server/confluent/config/configmanager.py index 449caa37..233ba9a6 100644 --- a/confluent_server/confluent/config/configmanager.py +++ b/confluent_server/confluent/config/configmanager.py @@ -633,7 +633,7 @@ class ConfigManager(object): def watch_attributes(self, nodes, attributes, callback): """ Watch a list of attributes for changes on a list of nodes. The - attributes may be literal, or a filename style wildcard like + attributes may be literal, or a filename style wildcard like 'net*.switch' :param nodes: An iterable of node names to be watching @@ -1022,9 +1022,12 @@ class ConfigManager(object): newattr = _attraliases[attr] attribmap[group][newattr] = attribmap[group][attr] del attribmap[group][attr] - if (attr not in ('nodes', 'noderange') and - attribute_is_invalid(attr, attribmap[group][attr])): - raise ValueError("{0} attribute is invalid".format(attr)) + if attr not in ('nodes', 'noderange'): + attrval = fixup_attribute(attr, attribmap[group][attr]) + if attribute_is_invalid(attr, attrval): + errstr = "{0} attribute is invalid".format(attrname) + raise ValueError(errstr) + attribmap[group][attr] = attrval if attr == 'nodes': if not isinstance(attribmap[group][attr], list): if type(attribmap[group][attr]) is unicode or type(attribmap[group][attr]) is str: @@ -1045,7 +1048,8 @@ class ConfigManager(object): if attr == 'nodes': newdict = set(attribmap[group][attr]) elif (isinstance(attribmap[group][attr], str) or - isinstance(attribmap[group][attr], unicode)): + isinstance(attribmap[group][attr], unicode) or + isinstance(attribmap[group][attr], bool)): newdict = {'value': attribmap[group][attr]} else: newdict = attribmap[group][attr] @@ -1069,6 +1073,13 @@ class ConfigManager(object): def clear_group_attributes(self, groups, attributes): changeset = {} + realattributes = [] + for attrname in list(attributes): + if attrname in _attraliases: + realattributes.append(_attraliases[attrname]) + else: + realattributes.append(attrname) + attributes = realattributes if type(groups) in (str, unicode): groups = (groups,) for group in groups: @@ -1200,6 +1211,13 @@ class ConfigManager(object): def clear_node_attributes(self, nodes, attributes): # accumulate all changes into a changeset and push in one go changeset = {} + realattributes = [] + for attrname in list(attributes): + if attrname in _attraliases: + realattributes.append(_attraliases[attrname]) + else: + realattributes.append(attrname) + attributes = realattributes for node in nodes: node = node.encode('utf-8') try: diff --git a/confluent_server/confluent/discovery/core.py b/confluent_server/confluent/discovery/core.py index 1f68f9a0..63844429 100644 --- a/confluent_server/confluent/discovery/core.py +++ b/confluent_server/confluent/discovery/core.py @@ -644,7 +644,7 @@ def get_nodename(cfg, handler, info): 'of SMM, nodename would have been ' \ '{0}'.format(nodename) log.log({'error': errorstr}) - return None + return None return nodename From c5dd024557ccde06df37357418450589ca52dbea Mon Sep 17 00:00:00 2001 From: Jarrod Johnson Date: Mon, 27 Nov 2017 16:59:13 -0500 Subject: [PATCH 5/5] Move the switch discoverable check for non-SMM into eval_node eval_node can establish that this is a direct discovery attempt. In that specific context, the check can be performed. Otherwise, we can't check in this way, but the enclosure manager should raise the error on behalf of the rest of the situation. --- confluent_server/confluent/discovery/core.py | 27 ++++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/confluent_server/confluent/discovery/core.py b/confluent_server/confluent/discovery/core.py index 63844429..7fbef1de 100644 --- a/confluent_server/confluent/discovery/core.py +++ b/confluent_server/confluent/discovery/core.py @@ -476,7 +476,7 @@ def _recheck_single_unknown(configmanager, mac): rechecker = eventlet.spawn_after(300, _periodic_recheck, configmanager) return - nodename = get_nodename(configmanager, handler, info) + nodename, info['maccount'] = get_nodename(configmanager, handler, info) if nodename: if handler.https_supported: dp = configmanager.get_node_attributes([nodename], @@ -591,7 +591,7 @@ def detected(info): #TODO, eventlet spawn after to recheck sooner, or somehow else # influence periodic recheck to shorten delay? return - nodename = get_nodename(cfg, handler, info) + nodename, info['maccount'] = get_nodename(cfg, handler, info) if nodename and handler.https_supported: dp = cfg.get_node_attributes([nodename], ('pubkeys.tls_hardwaremanager',)) @@ -619,11 +619,12 @@ def detected(info): def get_nodename(cfg, handler, info): nodename = None + maccount = None if handler.https_supported: currcert = handler.https_cert if not currcert: info['discofailure'] = 'nohttps' - return None + return None, None currprint = util.get_fingerprint(currcert) nodename = nodes_by_fprint.get(currprint, None) if not nodename: @@ -635,6 +636,7 @@ def get_nodename(cfg, handler, info): nodename = nodes_by_uuid.get(curruuid, None) if not nodename: # as a last resort, search switch for info nodename, macinfo = macmap.find_nodeinfo_by_mac(info['hwaddr'], cfg) + maccount = macinfo['maccount'] if (nodename and not handler.discoverable_by_switch(macinfo['maccount'])): if handler.devname == 'SMM': @@ -644,8 +646,8 @@ def get_nodename(cfg, handler, info): 'of SMM, nodename would have been ' \ '{0}'.format(nodename) log.log({'error': errorstr}) - return None - return nodename + return None, None + return nodename, maccount def eval_node(cfg, handler, info, nodename, manual=False): @@ -698,6 +700,9 @@ def eval_node(cfg, handler, info, nodename, manual=False): nl = cfg.filter_node_attributes( 'enclosure.bay={0}'.format(info['enclosure.bay']), nl) nl = list(nl) + # sadly, we cannot detect when user has a daisy chain smm config + # without ability to disambiguate, however the SMM will be caught so + # at least one actionable error will be encountered. if len(nl) != 1: info['discofailure'] = 'ambigconfig' if len(nl): @@ -724,6 +729,18 @@ def eval_node(cfg, handler, info, nodename, manual=False): pending_nodes[nodename] = info else: # we can and did accurately discover by switch or in enclosure + # but... is this really ok? could be on an upstream port or + # erroneously put in the enclosure with no nodes yet + if (info['maccount'] and + not handler.discoverable_by_switch(info['maccount'])): + errorstr = 'The detected node {0} was detected using switch, ' \ + 'however the relevant port has too many macs learned ' \ + 'for this type of device ({1}) to be discovered by ' \ + 'switch.'.format(nodename, handler.devname) + if manual: + raise exc.InvalidArgumentException(errorstr) + log.log({'error': errorstr}) + return if not discover_node(cfg, handler, info, nodename, manual): pending_nodes[nodename] = info