From 49074bec740d086ca338eeea98dfa9e0d3a4f466 Mon Sep 17 00:00:00 2001 From: jbjohnso Date: Mon, 21 Apr 2014 10:48:18 -0400 Subject: [PATCH] General code cleanup --- confluent/config/configmanager.py | 6 +-- confluent/consoleserver.py | 2 +- confluent/httpapi.py | 13 +++--- confluent/messages.py | 4 +- confluent/noderange.py | 2 +- plugins/configuration/attributes.py | 42 +++++++++--------- plugins/hardwaremanagement/ipmi.py | 68 ++++++++++++++++------------- 7 files changed, 75 insertions(+), 62 deletions(-) diff --git a/confluent/config/configmanager.py b/confluent/config/configmanager.py index 8414e2b2..34132a42 100644 --- a/confluent/config/configmanager.py +++ b/confluent/config/configmanager.py @@ -207,7 +207,7 @@ def _load_dict_from_dbm(dpath, tdb): def is_tenant(tenant): try: return tenant in _cfgstore['tenant'] - except: + except KeyError: return False @@ -219,7 +219,7 @@ def get_global(globalname): """ try: return _cfgstore['globals'][globalname] - except: + except KeyError: return None @@ -509,7 +509,7 @@ class ConfigManager(object): """ try: return copy.deepcopy(self._cfgstore['users'][name]) - except: + except KeyError: return None def set_user(self, name, attributemap): diff --git a/confluent/consoleserver.py b/confluent/consoleserver.py index f0b1131b..2ac18443 100644 --- a/confluent/consoleserver.py +++ b/confluent/consoleserver.py @@ -208,7 +208,7 @@ class _ConsoleHandler(object): for rcpt in self.rcpts.itervalues(): try: rcpt(data) - except: + except: # No matter the reason, advance to next recipient pass def get_recent(self): diff --git a/confluent/httpapi.py b/confluent/httpapi.py index b37d6b0a..46418788 100644 --- a/confluent/httpapi.py +++ b/confluent/httpapi.py @@ -98,7 +98,7 @@ create_resource_functions = { def _sessioncleaner(): - while (1): + while True: currtime = time.time() for session in httpsessions.keys(): if httpsessions[session]['expiry'] < currtime: @@ -148,7 +148,7 @@ def _authorize_request(env, operation): """Grant/Deny access based on data from wsgi env """ - authdata = False + authdata = None name = '' cookie = Cookie.SimpleCookie() if 'HTTP_COOKIE' in env: @@ -161,7 +161,7 @@ def _authorize_request(env, operation): httpsessions[sessionid]['expiry'] = time.time() + 90 name = httpsessions[sessionid]['name'] authdata = auth.authorize(name, element=None) - if authdata is False and 'HTTP_AUTHORIZATION' in env: + if (not authdata) and 'HTTP_AUTHORIZATION' in env: name, passphrase = base64.b64decode( env['HTTP_AUTHORIZATION'].replace('Basic ', '')).split(':', 1) authdata = auth.check_user_passphrase(name, passphrase, element=None) @@ -306,13 +306,13 @@ def resourcehandler_backend(env, start_response): yield '{"session":"%s","data":""}' % sessid return elif 'bytes' in querydict.keys(): # not keycodes... - input = querydict['bytes'] + myinput = querydict['bytes'] sessid = querydict['session'] if sessid not in consolesessions: start_response('400 Expired Session', headers) return consolesessions[sessid]['expiry'] = time.time() + 90 - consolesessions[sessid]['session'].write(input) + consolesessions[sessid]['session'].write(myinput) start_response('200 OK', headers) yield json.dumps({'session': querydict['session']}) return # client has requests to send or receive, not both... @@ -477,6 +477,9 @@ def serve(): class HttpApi(object): + def __init__(self): + self.server = None + def start(self): global auditlog global tracelog diff --git a/confluent/messages.py b/confluent/messages.py index 589ceb7b..fb6b0dc0 100644 --- a/confluent/messages.py +++ b/confluent/messages.py @@ -228,7 +228,7 @@ class InputAttributes(ConfluentMessage): # is unable to do so, meaning it is an expression tv = self.attribs[attrib].format() self.attribs[attrib] = tv - except: + except (KeyError, IndexError): # this means format() actually thought there was work # that suggested parameters, push it in as an # expression @@ -258,7 +258,7 @@ class InputAttributes(ConfluentMessage): # expression, store value back in case of escapes tv = nodeattr[attr].format() nodeattr[attr] = tv - except: + except (KeyError, IndexError): # an expression string will error if format() done # use that as cue to put it into config as an expr nodeattr[attr] = {'expression': nodeattr[attr]} diff --git a/confluent/noderange.py b/confluent/noderange.py index fa62ad2e..0937adfb 100644 --- a/confluent/noderange.py +++ b/confluent/noderange.py @@ -18,7 +18,7 @@ # considered ast, but a number of things violate python grammar like [] in # the middle of strings and use of @ for anything is not in their syntax -#construct custom grammer with pyparsing +#construct custom grammar with pyparsing #>>> grammar = pyparsing.Word(pyparsing.alphanums+'/', pyparsing.alphanums+'[]-.*') | ',-' | ',' | '@' #>>> parser = pyparsing.nestedExpr('(',')',content=grammar) diff --git a/plugins/configuration/attributes.py b/plugins/configuration/attributes.py index adc8ac38..2bf4580e 100644 --- a/plugins/configuration/attributes.py +++ b/plugins/configuration/attributes.py @@ -16,11 +16,13 @@ import confluent.exceptions as exc import confluent.messages as msg import confluent.config.attributes as allattributes + def retrieve(nodes, element, configmanager, inputdata): if nodes is not None: return retrieve_nodes(nodes, element, configmanager, inputdata) elif element[0] == 'groups': - return retrieve_nodegroup(element[1], element[3], configmanager, inputdata) + return retrieve_nodegroup( + element[1], element[3], configmanager, inputdata) def retrieve_nodegroup(nodegroup, element, configmanager, inputdata): @@ -51,7 +53,6 @@ def retrieve_nodegroup(nodegroup, element, configmanager, inputdata): if element == 'current': for attribute in sorted(grpcfg.iterkeys()): currattr = grpcfg[attribute] - desc="" if attribute == 'nodes': desc = 'The nodes belonging to this group' else: @@ -60,7 +61,7 @@ def retrieve_nodegroup(nodegroup, element, configmanager, inputdata): except KeyError: desc = 'Unknown' if 'value' in currattr or 'expression' in currattr: - yield msg.Attributes( kv={attribute: currattr}, desc=desc) + yield msg.Attributes(kv={attribute: currattr}, desc=desc) elif 'cryptvalue' in currattr: yield msg.CryptedAttributes( kv={attribute: currattr}, @@ -84,23 +85,23 @@ def retrieve_nodes(nodes, element, configmanager, inputdata): if element[-1] == 'all': for node in nodes: for attribute in sorted(allattributes.node.iterkeys()): - if attribute in attributes[node]: #have a setting for it + if attribute in attributes[node]: # have a setting for it val = attributes[node][attribute] - elif attribute == 'groups': # no setting, provide a blank + elif attribute == 'groups': # no setting, provide a blank val = [] - else: # no setting, provide a blank + else: # no setting, provide a blank val = {'value': None} if attribute.startswith('secret.'): - yield msg.CryptedAttributes(node, - {attribute: val}, + yield msg.CryptedAttributes( + node, {attribute: val}, allattributes.node[attribute]['description']) elif isinstance(val, list): - yield msg.ListAttributes(node, - {attribute: val}, + yield msg.ListAttributes( + node, {attribute: val}, allattributes.node[attribute]['description']) else: - yield msg.Attributes(node, - {attribute: val}, + yield msg.Attributes( + node, {attribute: val}, allattributes.node[attribute]['description']) elif element[-1] == 'current': for node in attributes.iterkeys(): @@ -111,15 +112,13 @@ def retrieve_nodes(nodes, element, configmanager, inputdata): except KeyError: desc = 'Unknown' if 'value' in currattr or 'expression' in currattr: - yield msg.Attributes(node, - {attribute: currattr}, - desc) + yield msg.Attributes(node, {attribute: currattr}, desc) elif 'cryptvalue' in currattr: - yield msg.CryptedAttributes(node, - {attribute: currattr}, desc) + yield msg.CryptedAttributes( + node, {attribute: currattr}, desc) elif isinstance(currattr, list): - yield msg.ListAttributes(node, - {attribute: currattr}, desc) + yield msg.ListAttributes( + node, {attribute: currattr}, desc) else: print attribute print repr(currattr) @@ -130,9 +129,11 @@ def update(nodes, element, configmanager, inputdata): if nodes is not None: return update_nodes(nodes, element, configmanager, inputdata) elif element[0] == 'groups': - return update_nodegroup(element[1], element[3], configmanager, inputdata) + return update_nodegroup( + element[1], element[3], configmanager, inputdata) raise Exception("This line should never be reached") + def update_nodegroup(group, element, configmanager, inputdata): try: configmanager.set_group_attributes({group: inputdata.attribs}) @@ -140,6 +141,7 @@ def update_nodegroup(group, element, configmanager, inputdata): raise exc.InvalidArgumentException() return retrieve_nodegroup(group, element, configmanager, inputdata) + def update_nodes(nodes, element, configmanager, inputdata): updatedict = {} for node in nodes: diff --git a/plugins/hardwaremanagement/ipmi.py b/plugins/hardwaremanagement/ipmi.py index 4789122e..25e59330 100644 --- a/plugins/hardwaremanagement/ipmi.py +++ b/plugins/hardwaremanagement/ipmi.py @@ -25,12 +25,14 @@ import pyghmi.exceptions as pygexc import pyghmi.ipmi.console as console import pyghmi.ipmi.command as ipmicommand import socket + console.session.select = eventlet.green.select console.session.threading = eventlet.green.threading _ipmithread = None _ipmiwaiters = [] + def _ipmi_evtloop(): while True: try: @@ -38,8 +40,9 @@ def _ipmi_evtloop(): while _ipmiwaiters: waiter = _ipmiwaiters.pop() waiter.send() - except: + except: # TODO(jbjohnso): log the trace into the log import traceback + traceback.print_exc() @@ -51,7 +54,7 @@ def get_conn_params(node, configdata): if 'secret.hardwaremanagementpassphrase' in configdata: passphrase = configdata['secret.hardwaremanagementpassphrase']['value'] else: - passphrase = 'PASSW0RD' # for lack of a better guess + passphrase = 'PASSW0RD' # for lack of a better guess if 'hardwaremanagement.manager' in configdata: bmc = configdata['hardwaremanagement.manager']['value'] else: @@ -70,9 +73,11 @@ def get_conn_params(node, configdata): 'port': 623, } + _configattributes = ('secret.hardwaremanagementuser', - 'secret.hardwaremanagementpassphrase', - 'secret.ipmikg', 'hardwaremanagement.manager') + 'secret.hardwaremanagementpassphrase', + 'secret.ipmikg', 'hardwaremanagement.manager') + def _donothing(data): # a dummy function to avoid some awkward exceptions from @@ -84,7 +89,10 @@ class IpmiConsole(conapi.Console): configattributes = frozenset(_configattributes) def __init__(self, node, config): + self.error = None + self.datacallback = None crypt = config.decrypt + self.solconnection = None config.decrypt = True self.broken = False configdata = config.get_node_attributes([node], _configattributes) @@ -103,7 +111,6 @@ class IpmiConsole(conapi.Console): def handle_data(self, data): if type(data) == dict: - disconnect = frozenset(('Session Disconnected', 'timeout')) if 'error' in data: self.solconnection = None self.broken = True @@ -113,17 +120,17 @@ class IpmiConsole(conapi.Console): else: self.datacallback(data) - def connect(self,callback): + def connect(self, callback): self.datacallback = callback # we provide a weak reference to pyghmi as otherwise we'd # have a circular reference and reference counting would never get # out... try: self.solconnection = console.Console(bmc=self.bmc, port=self.port, - userid=self.username, - password=self.password, - kg=self.kg, force=True, - iohandler=self.handle_data) + userid=self.username, + password=self.password, + kg=self.kg, force=True, + iohandler=self.handle_data) while not self.solconnection.connected and not self.broken: w = eventlet.event.Event() _ipmiwaiters.append(w) @@ -136,9 +143,8 @@ class IpmiConsole(conapi.Console): except socket.gaierror as err: raise exc.TargetEndpointUnreachable(str(err)) - def close(self): - if hasattr(self, 'solconnection') and self.solconnection is not None: + if self.solconnection is not None: # break the circular reference here self.solconnection.out_handler = _donothing self.solconnection = None @@ -178,15 +184,19 @@ class IpmiIterator(object): self.currdata = None return retdata + def perform_request(operator, node, element, configdata, inputdata, cfg): return IpmiHandler(operator, node, element, configdata, inputdata, cfg - ).handle_request() + ).handle_request() + persistent_ipmicmds = {} + class IpmiHandler(object): def __init__(self, operation, node, element, cfd, inputdata, cfg): self.broken = False + self.error = None eventlet.sleep(0) self.cfg = cfd[node] self.loggedin = False @@ -198,13 +208,13 @@ class IpmiHandler(object): self.inputdata = inputdata tenant = cfg.tenant self._logevt = None - if (node,tenant) not in persistent_ipmicmds: + if (node, tenant) not in persistent_ipmicmds: self._logevt = threading.Event() - persistent_ipmicmds[(node,tenant)] = ipmicommand.Command( + persistent_ipmicmds[(node, tenant)] = ipmicommand.Command( bmc=connparams['bmc'], userid=connparams['username'], password=connparams['passphrase'], kg=connparams['kg'], port=connparams['port'], onlogon=self.logged) - self.ipmicmd = persistent_ipmicmds[(node,tenant)] + self.ipmicmd = persistent_ipmicmds[(node, tenant)] bootdevices = { 'optical': 'cd' @@ -227,14 +237,15 @@ class IpmiHandler(object): raise exc.TargetEndpointUnreachable('Target timed out') else: raise Exception(self.error) - if self.element == [ 'power', 'state' ]: + if self.element == ['power', 'state']: return self.power() - elif self.element == [ 'boot', 'nextdevice' ]: + elif self.element == ['boot', 'nextdevice']: return self.bootdevice() - elif self.element == [ 'health', 'hardware' ]: + elif self.element == ['health', 'hardware']: return self.health() - def _str_health(self, health): + @staticmethod + def _str_health(health): if pygconstants.Health.Failed & health: health = 'failed' elif pygconstants.Health.Critical & health: @@ -246,11 +257,9 @@ class IpmiHandler(object): return health def _dict_sensor(self, pygreading): - retdict = {} - retdict['name'] = pygreading.name - retdict['value'] = pygreading.value - retdict['states'] = pygreading.states - retdict['health'] = self._str_health(pygreading.health) + retdict = {'name': pygreading.name, 'value': pygreading.value, + 'states': pygreading.states, + 'health': self._str_health(pygreading.health)} return retdict def health(self): @@ -273,14 +282,14 @@ class IpmiHandler(object): if bootdev['bootdev'] in self.bootdevices: bootdev['bootdev'] = self.bootdevices[bootdev['bootdev']] return msg.BootDevice(node=self.node, - device=bootdev['bootdev']) + device=bootdev['bootdev']) elif 'update' == self.op: bootdev = self.inputdata.bootdevice(self.node) bootdev = self.ipmicmd.set_bootdev(bootdev) if bootdev['bootdev'] in self.bootdevices: bootdev['bootdev'] = self.bootdevices[bootdev['bootdev']] return msg.BootDevice(node=self.node, - device=bootdev['bootdev']) + device=bootdev['bootdev']) def power(self): if 'read' == self.op: @@ -296,7 +305,6 @@ class IpmiHandler(object): state=power['powerstate']) - def initthread(): global _ipmithread if _ipmithread is None: @@ -305,19 +313,19 @@ def initthread(): def create(nodes, element, configmanager, inputdata): initthread() - if element == [ '_console', 'session' ]: + if element == ['_console', 'session']: if len(nodes) > 1: raise Exception("_console/session does not support multiple nodes") return IpmiConsole(nodes[0], configmanager) else: return IpmiIterator('update', nodes, element, configmanager, inputdata) + def update(nodes, element, configmanager, inputdata): initthread() return create(nodes, element, configmanager, inputdata) - def retrieve(nodes, element, configmanager, inputdata): initthread() return IpmiIterator('read', nodes, element, configmanager, inputdata)