Add static code checking for style and errors

This required a change to logging, which now uses a singleton pattern.

For all future checkins, 'make codecheck' should pass.
This commit is contained in:
Brandon Heller
2010-01-09 17:59:43 -08:00
parent ca58c896dd
commit 723d068c51
11 changed files with 200 additions and 132 deletions
+7
View File
@@ -1,5 +1,12 @@
all: codecheck test
clean:
rm -rf build dist build *.egg-info *.pyc
codecheck: mininet/*.py mininet/test/*.py
pyflakes mininet/*.py mininet/test/*.py bin/*.py
pylint --rcfile=.pylint mininet/*.py mininet/test/*.py bin/*.py
pep8 --ignore=E251 mininet/*.py mininet/test/*.py bin/*.py
test: mininet/*.py mininet/test/*.py
mininet/test/test_nets.py
+11 -11
View File
@@ -12,16 +12,14 @@ irreplaceable!
"""
from subprocess import Popen, PIPE
import re
from mininet.util import quietRun
from mininet.xterm import cleanUpScreens
def sh( cmd ):
def sh(cmd):
"Print a command and send it to the shell"
print cmd
return Popen( [ '/bin/sh', '-c', cmd ],
stdout=PIPE ).communicate()[ 0 ]
return Popen(['/bin/sh', '-c', cmd], stdout=PIPE).communicate()[0]
def cleanup():
@@ -34,23 +32,25 @@ def cleanup():
# Note: real zombie processes can't actually be killed, since they
# are already (un)dead. Then again,
# you can't connect to them either, so they're mostly harmless.
sh( 'killall -9 ' + zombies + ' 2> /dev/null' )
sh('killall -9 ' + zombies + ' 2> /dev/null')
print "*** Removing junk from /tmp"
sh( 'rm -f /tmp/vconn* /tmp/vlogs* /tmp/*.out /tmp/*.log' )
sh('rm -f /tmp/vconn* /tmp/vlogs* /tmp/*.out /tmp/*.log')
print "*** Removing old screen sessions"
cleanUpScreens()
print "*** Removing excess kernel datapaths"
dps = sh( "ps ax | egrep -o 'dp[0-9]+' | sed 's/dp/nl:/'" ).split( '\n')
dps = sh("ps ax | egrep -o 'dp[0-9]+' | sed 's/dp/nl:/'").split('\n')
for dp in dps:
if dp != '': sh( 'dpctl deldp ' + dp )
if dp != '':
sh('dpctl deldp ' + dp)
print "*** Removing all links of the pattern foo-ethX"
links = sh( "ip link show | egrep -o '(\w+-eth\w+)'" ).split( '\n' )
links = sh("ip link show | egrep -o '(\w+-eth\w+)'").split('\n')
for link in links:
if link != '': sh( "ip link del " + link )
if link != '':
sh("ip link del " + link)
print "*** Cleanup complete."
+23 -22
View File
@@ -13,7 +13,7 @@ try:
except ImportError:
USE_RIPCORD = False
from mininet.logging_mod import lg, set_loglevel, LEVELS
from mininet.logging_mod import lg, LEVELS
from mininet.net import Mininet, init
from mininet.node import KernelSwitch, Host, Controller, ControllerParams, NOX
from mininet.node import RemoteController, UserSwitch
@@ -21,40 +21,41 @@ from mininet.topo import SingleSwitchTopo, LinearTopo, SingleSwitchReversedTopo
# built in topologies, created only when run
TOPO_DEF = 'minimal'
TOPOS = {'minimal' : (lambda: SingleSwitchTopo(k = 2)),
'reversed' : (lambda: SingleSwitchReversedTopo(k = 2)),
'single4' : (lambda: SingleSwitchTopo(k = 4)),
'single100' : (lambda: SingleSwitchTopo(k = 100)),
'linear2' : (lambda: LinearTopo(k = 2)),
'linear100' : (lambda: LinearTopo(k = 100))}
TOPOS = {'minimal': (lambda: SingleSwitchTopo(k = 2)),
'reversed': (lambda: SingleSwitchReversedTopo(k = 2)),
'single4': (lambda: SingleSwitchTopo(k = 4)),
'single100': (lambda: SingleSwitchTopo(k = 100)),
'linear2': (lambda: LinearTopo(k = 2)),
'linear100': (lambda: LinearTopo(k = 100))}
if USE_RIPCORD:
TOPOS_RIPCORD = {
'tree16' : (lambda: TreeTopo(depth = 3, fanout = 4)),
'tree64' : (lambda: TreeTopo(depth = 4, fanout = 4)),
'tree1024' : (lambda: TreeTopo(depth = 3, fanout = 32)),
'fattree4' : (lambda: FatTreeTopo(k = 4)),
'fattree6' : (lambda: FatTreeTopo(k = 6)),
'vl2' : (lambda: VL2Topo(da = 4, di = 4))}
'tree16': (lambda: TreeTopo(depth = 3, fanout = 4)),
'tree64': (lambda: TreeTopo(depth = 4, fanout = 4)),
'tree1024': (lambda: TreeTopo(depth = 3, fanout = 32)),
'fattree4': (lambda: FatTreeTopo(k = 4)),
'fattree6': (lambda: FatTreeTopo(k = 6)),
'vl2': (lambda: VL2Topo(da = 4, di = 4))}
TOPOS.update(TOPOS_RIPCORD)
SWITCH_DEF = 'kernel'
SWITCHES = {'kernel' : KernelSwitch,
'user' : UserSwitch}
SWITCHES = {'kernel': KernelSwitch,
'user': UserSwitch}
HOST_DEF = 'process'
HOSTS = {'process' : Host}
HOSTS = {'process': Host}
CONTROLLER_DEF = 'ref'
# a and b are the name and inNamespace params.
CONTROLLERS = {'ref' : Controller,
'nox_dump' : lambda a, b: NOX(a, b, 'packetdump'),
'nox_pysw' : lambda a, b: NOX(a, b, 'pyswitch'),
'remote' : lambda a, b: None,
'none' : lambda a, b: None}
CONTROLLERS = {'ref': Controller,
'nox_dump': lambda a, b: NOX(a, b, 'packetdump'),
'nox_pysw': lambda a, b: NOX(a, b, 'pyswitch'),
'remote': lambda a, b: None,
'none': lambda a, b: None}
# optional tests to run
TESTS = ['cli', 'build', 'ping_all', 'ping_pair', 'iperf', 'all', 'iperf_udp']
def add_dict_option(opts, choices_dict, default, name, help_str = None):
'''Convenience function to add choices dicts to OptionParser.
@@ -124,7 +125,7 @@ class MininetRunner(object):
'''Setup and validate environment.'''
# set logging verbosity
set_loglevel(self.options.verbosity)
lg.set_loglevel(self.options.verbosity)
# validate environment setup
init()
+1
View File
@@ -0,0 +1 @@
'''Docstring to silence pylint; ignores --ignore option for __init__.py'''
+60 -28
View File
@@ -1,6 +1,7 @@
'''Logging functions for Mininet.'''
import logging
from logging import Logger
import types
LEVELS = {'debug': logging.DEBUG,
@@ -16,7 +17,6 @@ LOG_LEVEL_DEFAULT = logging.WARNING
LOG_MSG_FORMAT = '%(message)s'
# Modified from python2.5/__init__.py
class StreamHandlerNoNewline(logging.StreamHandler):
'''StreamHandler that doesn't print newlines by default.
@@ -53,31 +53,52 @@ class StreamHandlerNoNewline(logging.StreamHandler):
self.handleError(record)
def set_loglevel(level_name = None):
'''Setup loglevel.
class Singleton(type):
'''Singleton pattern from Wikipedia
@param level_name level name from LEVELS
See http://en.wikipedia.org/wiki/Singleton_pattern#Python
Intended to be used as a __metaclass_ param, as shown for the class below.
Changed cls first args to mcs to satsify pylint.
'''
level = LOG_LEVEL_DEFAULT
if level_name != None:
if level_name not in LEVELS:
raise Exception('unknown loglevel seen in set_loglevel')
else:
level = LEVELS.get(level_name, level)
lg.setLevel(level)
if len(lg.handlers) != 1:
raise Exception('lg.handlers length not zero in logging_mod')
lg.handlers[0].setLevel(level)
def __init__(mcs, name, bases, dict_):
super(Singleton, mcs).__init__(name, bases, dict_)
mcs.instance = None
def __call__(mcs, *args, **kw):
if mcs.instance is None:
mcs.instance = super(Singleton, mcs).__call__(*args, **kw)
return mcs.instance
def _setup_logging():
'''Setup logging for Mininet.'''
global lg
class MininetLogger(Logger, object):
'''Mininet-specific logger
Enable each mininet .py file to with one import:
from mininet.logging_mod import lg
...get a default logger that doesn't require one newline per logging call.
Inherit from object to ensure that we have at least one new-style base
class, and can then use the __metaclass__ directive, to prevent this error:
TypeError: Error when calling the metaclass bases
a new-style class can't have only classic bases
If Python2.5/logging/__init__.py defined Filterer as a new-style class,
via Filterer(object): rather than Filterer, we wouldn't need this.
Use singleton pattern to ensure only one logger is ever created.
'''
__metaclass__ = Singleton
def __init__(self):
Logger.__init__(self, "mininet")
# create logger if first time
if 'lg' not in globals():
lg = logging.getLogger('mininet')
# create console handler
ch = StreamHandlerNoNewline()
# create formatter
@@ -85,15 +106,26 @@ def _setup_logging():
# add formatter to ch
ch.setFormatter(formatter)
# add ch to lg
lg.addHandler(ch)
self.addHandler(ch)
self.set_loglevel()
def set_loglevel(self, levelname = None):
'''Setup loglevel.
Convenience function to support lowercase names.
@param level_name level name from LEVELS
'''
level = LOG_LEVEL_DEFAULT
if levelname != None:
if levelname not in LEVELS:
raise Exception('unknown loglevel seen in set_loglevel')
else:
raise Exception('setup_logging called twice')
level = LEVELS.get(levelname, level)
set_loglevel()
self.setLevel(level)
self.handlers[0].setLevel(level)
# There has to be some better way to ensure we only ever have one logging
# variable. If this check isn't in, the order in which imports occur can
# affect whether a program runs, because the variable lg may get rebound.
if 'lg' not in globals():
_setup_logging()
lg = MininetLogger()
+25 -17
View File
@@ -210,7 +210,7 @@ class Mininet(object):
For use with the user datapath only right now.
@todo(brandonh) Test this code and verify that user-space works!
@todo(brandonh) Test this code!
'''
# params were: controller, switches, ips
@@ -242,12 +242,12 @@ class Mininet(object):
while not switch.intfIsUp(switch.intfs[0]):
lg.info('*** Waiting for %s to come up\n' % switch.intfs[0])
sleep(1)
if self.ping_test(hosts=[switch, controller]) != 0:
if self.ping(hosts = [switch, controller]) != 0:
lg.error('*** Error: control network test failed\n')
exit(1)
lg.info('\n')
def _config_hosts( self ):
def _config_hosts(self):
'''Configure a set of hosts.'''
# params were: hosts, ips
for host_dpid in self.topo.hosts():
@@ -343,7 +343,7 @@ class Mininet(object):
def start(self):
'''Start controller and switches\n'''
lg.info('*** Starting controller\n')
for cname, cnode in self.controllers.iteritems():
for cnode in self.controllers.values():
cnode.start()
lg.info('*** Starting %s switches\n' % len(self.topo.switches()))
for switch_dpid in self.topo.switches():
@@ -371,7 +371,7 @@ class Mininet(object):
switch.stop()
lg.info('\n')
lg.info('*** Stopping controller\n')
for cname, cnode in self.controllers.iteritems():
for cnode in self.controllers.values():
cnode.stop()
lg.info('*** Test complete\n')
@@ -387,7 +387,7 @@ class Mininet(object):
def _parse_ping(pingOutput):
'''Parse ping output and return packets sent, received.'''
r = r'(\d+) packets transmitted, (\d+) received'
m = re.search( r, pingOutput )
m = re.search(r, pingOutput)
if m == None:
lg.error('*** Error: could not parse ping output: %s\n' %
pingOutput)
@@ -422,7 +422,7 @@ class Mininet(object):
lg.error('*** Error: received too many packets')
lg.error('%s' % result)
node.cmdPrint('route')
exit( 1 )
exit(1)
lost += sent - received
lg.info(('%s ' % dest.name) if received else 'X ')
lg.info('\n')
@@ -490,7 +490,8 @@ class Mininet(object):
server = host0.cmd(iperf_args + '-s &')
if verbose:
lg.info('%s\n' % server)
client = host1.cmd(iperf_args + '-t 5 -c ' + host0.IP() + ' ' + bw_args)
client = host1.cmd(iperf_args + '-t 5 -c ' + host0.IP() + ' ' +
bw_args)
if verbose:
lg.info('%s\n' % client)
server = host0.cmd('killall -9 iperf')
@@ -529,6 +530,10 @@ class MininetCLI(object):
self.nodelist = self.nodemap.values()
self.run()
# Disable pylint "Unused argument: 'arg's'" messages.
# Each CLI function needs the same interface.
# pylint: disable-msg=W0613
# Commands
def help(self, args):
'''Semi-useful help for CLI.'''
@@ -561,7 +566,7 @@ class MininetCLI(object):
switch = self.mn.nodes[switch_dpid]
lg.info('%s <->', switch.name)
for intf in switch.intfs:
node, remoteIntf = switch.connection[intf]
node = switch.connection[intf]
lg.info(' %s' % node.name)
lg.info('\n')
@@ -588,25 +593,28 @@ class MininetCLI(object):
def intfs(self, args):
'''List interfaces.'''
for dpid, node in self.mn.nodes.iteritems():
for node in self.mn.nodes.values():
lg.info('%s: %s\n' % (node.name, ' '.join(node.intfs)))
def dump(self, args):
'''Dump node info.'''
for dpid, node in self.mn.nodes.iteritems():
for node in self.mn.nodes.values():
lg.info('%s\n' % node)
# Re-enable pylint "Unused argument: 'arg's'" messages.
# pylint: enable-msg=W0613
def run(self):
'''Read and execute commands.'''
lg.warn('*** Starting CLI:\n')
while True:
lg.warn('mininet> ')
input = sys.stdin.readline()
if input == '':
input_line = sys.stdin.readline()
if input_line == '':
break
if input[-1] == '\n':
input = input[:-1]
cmd = input.split(' ')
if input_line[-1] == '\n':
input_line = input_line[:-1]
cmd = input_line.split(' ')
first = cmd[0]
rest = cmd[1:]
if first in self.cmds and hasattr(self, first):
@@ -634,7 +642,7 @@ class MininetCLI(object):
elif first in ['exit', 'quit']:
break
elif first == '?':
self.help( rest )
self.help(rest)
else:
lg.error('CLI: unknown node or command: < %s >\n' % first)
#lg.info('*** CLI: command complete\n')
+22 -7
View File
@@ -2,13 +2,17 @@
'''Node objects for Mininet.'''
from subprocess import Popen, PIPE, STDOUT
import os, signal, sys, select
import os
import signal
import sys
import select
flush = sys.stdout.flush
from mininet.logging_mod import lg
from mininet.util import quietRun, macColonHex, ipStr
class Node(object):
'''A virtual network node is simply a shell in a network namespace.
We communicate with it using pipes.'''
@@ -129,7 +133,8 @@ class Node(object):
if len(data) > 0 and data[-1] == chr(0177):
output += data[:-1]
break
else: output += data
else:
output += data
self.waiting = False
return output
@@ -215,7 +220,7 @@ class Node(object):
def IP(self):
'''Return IP address of first interface'''
if len(self.intfs) > 0:
return self.ips.get(self.intfs[ 0 ], None)
return self.ips.get(self.intfs[0], None)
def intfIsUp(self):
'''Check if one of our interfaces is up.'''
@@ -258,6 +263,7 @@ class Switch(Node):
else:
return True, ''
class UserSwitch(Switch):
'''User-space switch.
@@ -269,7 +275,7 @@ class UserSwitch(Switch):
@param name
'''
Node.__init__(self, name, inNamespace = False)
Switch.__init__(self, name, inNamespace = False)
def start(self, controllers):
'''Start OpenFlow reference user datapath.
@@ -298,6 +304,12 @@ class UserSwitch(Switch):
class KernelSwitch(Switch):
'''Kernel-space switch.
Much faster than user-space!
Currently only works in the root namespace.
'''
def __init__(self, name, dp = None, dpid = None):
'''Init.
@@ -306,7 +318,7 @@ class KernelSwitch(Switch):
@param dp netlink id (0, 1, 2, ...)
@param dpid datapath ID as unsigned int; random value if None
'''
Node.__init__(self, name, inNamespace = False)
Switch.__init__(self, name, inNamespace = False)
self.dp = dp
self.dpid = dpid
@@ -333,7 +345,7 @@ class KernelSwitch(Switch):
controllers['c0'].IP() + ':' +
str(controllers['c0'].port) +
' --fail=closed 1> ' + ofplog + ' 2>' + ofplog + ' &')
self.execed = False # XXX until I fix it
self.execed = False
def stop(self):
'''Terminate reference kernel datapath.'''
@@ -372,7 +384,7 @@ class Controller(Node):
self.cmdPrint('cd ' + self.cdir)
self.cmdPrint(self.controller + ' ' + self.cargs +
' 1> ' + cout + ' 2> ' + cout + ' &')
self.execed = False # XXX Until I fix it
self.execed = False
def stop(self):
'''Stop controller.'''
@@ -386,6 +398,7 @@ class Controller(Node):
class ControllerParams(object):
'''Container for controller IP parameters.'''
def __init__(self, ip, subnet_size):
'''Init.
@@ -398,6 +411,7 @@ class ControllerParams(object):
class NOX(Controller):
'''Controller to run a NOX application.'''
def __init__(self, name, inNamespace = False, nox_args = None, **kwargs):
'''Init.
@@ -420,6 +434,7 @@ class NOX(Controller):
class RemoteController(Controller):
'''Controller running outside of Mininet's control.'''
def __init__(self, name, inNamespace = False, ip_address = '127.0.0.1',
port = 6633):
'''Init.
+1 -2
View File
@@ -4,7 +4,6 @@
Test creation and all-pairs ping for each included mininet topo type.
'''
from time import sleep
import unittest
from mininet.net import init, Mininet
@@ -12,7 +11,7 @@ from mininet.node import KernelSwitch, Host, Controller, ControllerParams
from mininet.topo import SingleSwitchTopo, LinearTopo
# temporary, until user-space side is tested
SWITCHES = {'kernel' : KernelSwitch}
SWITCHES = {'kernel': KernelSwitch}
class testSingleSwitch(unittest.TestCase):
+4 -1
View File
@@ -11,7 +11,7 @@ A Topo object can be a topology database for NOX, can represent a physical
setup for testing, and can even be emulated with the Mininet package.
'''
from networkx import Graph
from networkx.classes.graph import Graph
class NodeID(object):
@@ -87,6 +87,7 @@ class Edge(object):
class Topo(object):
'''Data center network representation for structured multi-trees.'''
def __init__(self):
'''Create Topo object.
@@ -182,6 +183,7 @@ class Topo(object):
@return dpids list of dpids
'''
def is_switch(n):
'''Returns true if node is a switch.'''
return self.node_info[n].is_switch
@@ -196,6 +198,7 @@ class Topo(object):
@return dpids list of dpids
'''
def is_host(n):
'''Returns true if node is a host.'''
return not self.node_info[n].is_switch
+9 -7
View File
@@ -99,6 +99,7 @@ def move_intf(intf, node):
# live in the root namespace and thus do not have to be
# explicitly moved.
def makeIntfPair(intf1, intf2):
'''Make a veth pair.
@@ -111,7 +112,7 @@ def makeIntfPair(intf1, intf2):
quietRun('ip link del ' + intf2)
# Create new pair
cmd = 'ip link add name ' + intf1 + ' type veth peer name ' + intf2
return checkRun( cmd )
return checkRun(cmd)
def moveIntf(intf, node, print_error = False):
@@ -132,7 +133,7 @@ def moveIntf(intf, node, print_error = False):
return True
def retry(n, retry_delay, fn, *args):
def retry(n, retry_delay, fn, *args, **keywords):
'''Try something N times before giving up.
@param n number of times to retry
@@ -141,12 +142,12 @@ def retry(n, retry_delay, fn, *args):
@param args args to apply to function call
'''
tries = 0
while not apply(fn, args) and tries < n:
while not fn(*args, **keywords) and tries < n:
sleep(retry_delay)
tries += 1
if tries >= n:
lg.error("*** gave up after %i retries\n" % tries)
exit( 1 )
exit(1)
# delay between interface move checks in seconds
@@ -154,6 +155,7 @@ MOVEINTF_DELAY = 0.0001
CREATE_LINK_RETRIES = 10
def createLink(node1, node2):
'''Create a link between nodes, making an interface for each.
@@ -174,8 +176,8 @@ def createLink(node1, node2):
def fixLimits():
'''Fix ridiculously small resource limits.'''
setrlimit( RLIMIT_NPROC, (4096, 8192))
setrlimit( RLIMIT_NOFILE, (16384, 32768))
setrlimit(RLIMIT_NPROC, (4096, 8192))
setrlimit(RLIMIT_NOFILE, (16384, 32768))
def _colonHex(val, bytes):
@@ -186,7 +188,7 @@ def _colonHex(val, bytes):
@return ch_str colon-hex string
'''
pieces = []
for i in range (bytes - 1, -1, -1):
for i in range(bytes - 1, -1, -1):
pieces.append('%02x' % (((0xff << (i * 8)) & val) >> (i * 8)))
ch_str = ':'.join(pieces)
return ch_str
+1 -1
View File
@@ -6,12 +6,12 @@ Utility functions to run an xterm (connected via screen(1)) on each host.
Requires xterm(1) and GNU screen(1).
"""
import os
import re
from subprocess import Popen
from mininet.util import quietRun
def makeXterm(node, title):
'''Run screen on a node, and hook up an xterm.