more fixes + cleanup + support for --exclude argument

Ticket: CM-1438
Reviewed By:
Testing Done:

KNOWN_ISSUES

@@ -1,4 +1,4 @@
-- Some state issues if multiple interfaces for the same interface config blocks are present for the same interface
+- There is a state issue if multiple configuration blocks are present for the same interface in the interfaces file
 - `ifquery -r` can give wrong result if dhclient is running + static addresses are configured
 - `ifquery -r` status is success for success case and also for cases where there
 is no support for query yet

TODO

@@ -1,5 +1,9 @@
 TODO:
 ====
+- update man-pages with new options. Convert them to rst
+- ifup hotplug support (basically needs some testing and fixing broken things)
+- syslog support
+- -q quiet option
 - support for /etc/networking.defaults
 - implement legacy ifupdown mapping feature
 - support for the following ifupdown options:

debian/python-ifupdown2.postrm

@@ -2,6 +2,8 @@
 
 set -e
 
+rm -f /sbin/ifup /sbin/ifdown /sbin/ifquery
+
 case "$1" in
   purge)
     # Note: We don't remove /etc/network/interfaces

init.d/networking

@@ -96,7 +96,7 @@ check_network_swap() {
 ifup_hotplug () {
     if [ -d /sys/class/net ]
     then
-	    ifaces=$(for iface in $(ifquery --list --allow=hotplug)
+	    ifaces=$(for iface in $(ifquery --list --allow=hotplug 2>/dev/null)
 			    do
 				    link=${iface##:*}
 				    link=${link##.*}
@@ -170,7 +170,7 @@ force-reload|restart)
 	fi
 	process_options
 
-	log_warning_msg "Running $0 $1 is deprecated because it may not re-enable some interfaces"
+	#log_warning_msg "Running $0 $1 is deprecated because it may not re-enable some interfaces"
 	log_action_begin_msg "Reconfiguring network interfaces"
 	ifdown -a --exclude=lo $verbose || true
 	set -f

pkg/exceptions.py

@@ -1,5 +1,11 @@
 #!/usr/bin/python
-
+#
+# Copyright 2013.  Cumulus Networks, Inc.
+# Author: Roopa Prabhu, roopa@cumulusnetworks.com
+#
+# ifupdown --
+#    exceptions
+#
 
 class Error(Exception):
     """Base class for exceptions in ifupdown"""

pkg/graph.py

@@ -1,4 +1,11 @@
 #!/usr/bin/python
+#
+# Copyright 2013.  Cumulus Networks, Inc.
+# Author: Roopa Prabhu, roopa@cumulusnetworks.com
+#
+# graph --
+#    graph helper module for ifupdown
+#
 
 import logging
 from collections import deque
@@ -38,6 +45,7 @@ class graph():
         # If some indegrees are non zero, we have a cycle
         for ifname,indegree in indegrees.items():
             if indegree != 0:
-                raise Exception('cycle found involving iface %s' %ifname)
+                raise Exception('cycle found involving iface %s' %ifname +
+                                ' (indegree %d)' %indegree)
 
         return S

pkg/iface.py

@@ -1,4 +1,11 @@
 #!/usr/bin/python
+#
+# Copyright 2013.  Cumulus Networks, Inc.
+# Author: Roopa Prabhu, roopa@cumulusnetworks.com
+#
+# iface --
+#    interface object
+#
 
 import logging
 
@@ -158,7 +165,12 @@ class iface():
     def get_classes(self):
         return self.classes
 
-    def set_classes(self, classname):
+    def set_classes(self, classes):
+        """ sets interface class list to the one passed as arg """
+        self.classes = classes
+
+    def set_class(self, classname):
+        """ Appends a class to the list """
         self.classes.append(classname)
 
     def belongs_to_class(self, intfclass):
@@ -209,49 +221,6 @@ class iface():
     def get_linkstate(self):
         return self.linkstate
 
-    def run_children(self, stage, module_list, force=0):
-
-        if (self.is_flag_on(ifaceFlags.FOLLOW_DEPENDENTS)
-                == False):
-            return
-
-        for c in self.children:
-            self.set_flag(
-                ifaceFlags.FOLLOW_DEPENDENTS)
-            ret = c.run(stage, module_list, force)
-            self.clear_flag(
-                ifaceFlags.FOLLOW_DEPENDENTS)
-
-            if ret != 0:
-                self.set_state(stage, ret)
-                if force == 0:
-                    break
-
-        return ret
-
-    def run(self, stage, module_list, force=0):
-        ret = 0
-        err = 0
-
-        ret = self.run_children()
-
-        if ret != 0 and force == 0:
-            return ret
-            
-        for m in module_list:
-            ret = m.run(self, stage, force)
-            if ret != 0:
-                self.set_state(stage, ret)
-
-                # If no force, return!
-                if force == 0:
-                    return -1
-                err += 1
-            else:
-                self.set_state(stage, ret)
-
-        return ret
-
     def get_attr_value(self, attr_name):
         config = self.get_config()
 

pkg/ifupdownbase.py

@@ -1,4 +1,11 @@
 #!/usr/bin/python
+#
+# Copyright 2013.  Cumulus Networks, Inc.
+# Author: Roopa Prabhu, roopa@cumulusnetworks.com
+#
+# ifupdownBase --
+#    base object for various ifupdown objects
+#
 
 import logging
 import subprocess
@@ -33,3 +40,23 @@ class ifupdownBase(object):
                 '\n(' + cmdout.strip('\n ') + ')')
 
         return cmdout
+
+    def ignore_error(self, errmsg):
+        if (self.FORCE == True or re.search(r'exists', errmsg,
+            re.IGNORECASE | re.MULTILINE) is not None):
+            return True
+        return False
+
+    def log_warn(self, str):
+        if self.ignore_error(str) == False:
+            if self.logger.getEffectiveLevel() == logging.DEBUG:
+                traceback.print_stack()
+            self.logger.warn(str)
+        pass
+
+    def log_error(self, str):
+        if self.ignore_error(str) == False:
+            raise Exception(str)
+        else:
+            pass
+

pkg/ifupdownmain.py

@@ -1,20 +1,28 @@
 #!/usr/bin/python
+#
+# Copyright 2013.  Cumulus Networks, Inc.
+# Author: Roopa Prabhu, roopa@cumulusnetworks.com
+#
+# ifupdownMain --
+#    ifupdown main module
+#
 
 import os
 import re
+import imp
+import pprint
+import logging
+import sys, traceback
 from statemanager import *
 from networkinterfaces import *
 from iface import *
 from scheduler import *
 from collections import deque
 from collections import OrderedDict
-import imp
-import pprint
-import logging
 from graph import *
-import sys, traceback
+from sets import Set
 
-class ifupdown_main():
+class ifupdownMain():
 
     # Flags
     NODEPENDS = False
@@ -129,12 +137,6 @@ class ifupdown_main():
     def get_njobs(self):
         return self.njobs
 
-    def ignore_error(self, errmsg):
-        if (self.FORCE == True or re.search(r'exists', errmsg,
-            re.IGNORECASE | re.MULTILINE) is not None):
-            return True
-        return False
-
     def get_nodepends(self):
         return self.NODEPENDS
 
@@ -281,18 +283,17 @@ class ifupdown_main():
             dlist = ifaceobj.get_dependents()
             if dlist is None:
                 dlist = self.get_dependents(ifaceobj, op)
+            else:
+                # we already have dependency info for this interface
+                continue
 
-            if dlist is not None:
             self.preprocess_dependency_list(dlist, op)
             ifaceobj.set_dependents(dlist)
 
             if dependency_graph.get(i) is None:
                 dependency_graph[i] = dlist
 
-            if dlist is not None:
+            self.generate_dependency_info(dlist, dependency_graph, op)
-                self.generate_dependency_info(dlist,
-                         dependency_graph, op)
-
 
     def is_valid_state_transition(self, ifname, to_be_state):
         return self.statemanager.is_valid_state_transition(ifname,
@@ -440,7 +441,7 @@ class ifupdown_main():
 
 
     def run_without_dependents(self, op, ifacenames):
-        ifaceSched = ifaceScheduler()
+        ifaceSched = ifaceScheduler(force=self.FORCE)
 
         self.logger.debug('run_without_dependents for op %s' %op +
                 ' for %s' %str(ifacenames))
@@ -499,7 +500,7 @@ class ifupdown_main():
         return 0
 
 
-    def iface_whitelisted(self, auto, allow_classes, ifacename):
+    def iface_whitelisted(self, auto, allow_classes, excludepats, ifacename):
         """ Checks if interface is whitelisted depending on set of parameters.
 
 
@@ -507,8 +508,11 @@ class ifupdown_main():
 
         """
 
-        self.logger.debug('checking if iface %s' %ifacename +
+        # If the interface matches
-                ' is whitelisted')
+        if excludepats is not None and len(excludepats) > 0:
+            for e in excludepats:
+                if re.search(e, ifacename) is not None:
+                    return False
 
         ifaceobjs = self.get_iface_objs(ifacename)
         if ifaceobjs is None:
@@ -519,7 +523,7 @@ class ifupdown_main():
         if allow_classes is not None and len(allow_classes) > 0:
             for i in ifaceobjs:
                 if (len(i.get_classes()) > 0):
-                    common = Set(allow_classes).intersection(
+                    common = Set([allow_classes]).intersection(
                                 Set(i.get_classes()))
                     if len(common) > 0:
                         return True
@@ -555,7 +559,7 @@ class ifupdown_main():
 
 
     def run(self, op, auto=False, allow_classes=None,
-            ifacenames=None, query_state=None):
+            ifacenames=None, query_state=None, excludepats=None):
         """ main ifupdown run method """
 
         if auto == True:
@@ -599,7 +603,8 @@ class ifupdown_main():
 
         # filter interfaces based on auto and allow classes
         filtered_ifacenames = [i for i in ifacenames
-             if self.iface_whitelisted(auto, allow_classes, i) == True]
+             if self.iface_whitelisted(auto, allow_classes, excludepats,
+                                       i) == True]
 
         if len(filtered_ifacenames) == 0:
                 raise Exception('no ifaces found matching ' +
@@ -642,16 +647,17 @@ class ifupdown_main():
             self.logger.warning('error saving state (%s)' %str(e))
 
 
-    def up(self, auto=False, allow=None, ifacenames=None):
+    def up(self, auto=False, allow=None, ifacenames=None, excludepats=None):
-        return self.run('up', auto, allow, ifacenames)
+        return self.run('up', auto, allow, ifacenames, excludepats=excludepats)
 
-    def down(self, auto=False, allow=None, ifacenames=None):
+    def down(self, auto=False, allow=None, ifacenames=None, excludepats=None):
-        return self.run('down', auto, allow, ifacenames);
+        return self.run('down', auto, allow, ifacenames,
+                        excludepats=excludepats);
 
     def query(self, auto=False, allow=None, ifacenames=None,
-              query_state=False):
+              query_state=False, excludepats=None):
         return self.run('query', auto, allow, ifacenames,
-                        query_state=query_state);
+                        query_state=query_state, excludepats=excludepats);
 
     def dump(self):
         """ all state dump """

pkg/log.py

@@ -1,28 +0,0 @@
-#!/usr/bin/python
-
-import logging
-
-
-class log:
-
-    @staticmethod
-    def log_error(obj, prefix, *args, **kwargs):
-        obj.get_logger().logger.log_error(''.join(args))
-
-    @staticmethod
-    def log_warn(obj, *args, **kwargs):
-        msg = ''
-        logger = obj.get_logger()
-        errmsg = obj.get_errmsg()
-        msg += ''.join(args)
-        if errmsg is not None and len(errmsg) > 0:
-            msg += '(%s)' %errmsg
-
-
-    @staticmethod
-    def log(obj, log_prefix, *args, **kwargs):
-        msg = ''
-        logger = obj.get_logger()
-        msg += ''.join(args)
-
-

pkg/networkinterfaces.py

@@ -1,10 +1,16 @@
 #!/usr/bin/python
+#
+# Copyright 2013.  Cumulus Networks, Inc.
+# Author: Roopa Prabhu, roopa@cumulusnetworks.com
+#
+# networkInterfaces --
+#    ifupdown network interfaces file parser
+#
 
 import collections
-from iface import *
 import logging
 import glob
-
+from iface import *
 
 class networkInterfaces():
 
@@ -38,7 +44,7 @@ class networkInterfaces():
         return 0
 
     def process_allow(self, lines, cur_idx, lineno):
-        allow_line = self.lines[cur_idx]
+        allow_line = lines[cur_idx]
 
         words = allow_line.split()
         if len(words) <= 1:

pkg/scheduler.py

@@ -1,4 +1,11 @@
 #!/usr/bin/python
+#
+# Copyright 2013.  Cumulus Networks, Inc.
+# Author: Roopa Prabhu, roopa@cumulusnetworks.com
+#
+# ifaceScheduler --
+#    interface scheduler
+#
 
 import os
 import re
@@ -23,9 +30,10 @@ class ifaceScheduler(ifupdownBase):
     or dependency graph format.
     """
 
-    def __init__(self):
+    def __init__(self, force=False):
         self.logger = logging.getLogger('ifupdown.' +
                     self.__class__.__name__)
+        self.FORCE = force
 
     def run_iface_subop(self, ifupdownobj, ifaceobj, op, subop, mdict, cenv):
         """ Runs sub operation on an interface """
@@ -55,10 +63,7 @@ class ifaceScheduler(ifupdownBase):
                     self.exec_command(m, cmdenv=cenv)
             except Exception, e:
                 err = 1
-                if ifupdownobj.ignore_error(str(e)) == True:
+                self.log_error(str(e))
-                    pass
-                else:
-                    raise
             finally:
                 if op != 'query':
                     if err == 1:
@@ -123,11 +128,7 @@ class ifaceScheduler(ifupdownBase):
             try:
                 self.run_iface(ifupdownobj, ifacename, operation)
             except Exception, e:
-                if (ifupdownobj.ignore_error(str(e)) == True):
+                self.log_error(str(e))
-                    pass
-                else:
-                    raise
-
 
     def run_iface_list_subop(self, ifupdownobj, ifacenames, op, subop, mdict,
                              sorted_by_dependency=False):
@@ -164,11 +165,7 @@ class ifaceScheduler(ifupdownBase):
                     self.run_iface_subop(ifupdownobj, ifaceobj, op, subop,
                                          mdict, cenv)
             except Exception, e:
-                if (ifupdownobj.ignore_error(str(e)) == True):
+                self.log_error(str(e))
-                    pass
-                else:
-                    raise
-
 
     def run_iface_list_stages(self, ifupdownobj, ifacenames, op,
                               sorted_by_dependency=False):
@@ -261,7 +258,7 @@ class ifaceScheduler(ifupdownBase):
                                                  dlist, op)
                     self.accquire_token(ifacename)
                 except Exception, e:
-                    if (ifupdownobj.ignore_error(str(e)) == True):
+                    if (self.ignore_error(str(e)) == True):
                         pass
                     else:
                         # Dont bring the iface up if children did not come up

pkg/statemanager.py

@@ -1,5 +1,11 @@
 #!/usr/bin/python
-
+#
+# Copyright 2013.  Cumulus Networks, Inc.
+# Author: Roopa Prabhu, roopa@cumulusnetworks.com
+#
+# stateManager --
+#    interface state manager
+#
 import cPickle
 from collections import OrderedDict
 from exceptions import *
@@ -65,6 +71,7 @@ class stateManager():
         for ifaceobj in pickling.load(pickle_filename):
             self.save_ifaceobj(ifaceobj)
             ifaceobj.set_refcnt(0)
+            ifaceobj.set_dependents(None)
 
         return 0
 

sbin/ifupdown

@@ -4,7 +4,7 @@ import sys
 import os
 import re
 import argparse
-from ifupdown.ifupdown_main import *
+from ifupdown.ifupdownmain import *
 
 import logging
 
@@ -17,15 +17,15 @@ def run(args, op):
     try:
         logger.debug('creating ifupdown object ..')
         if op == 'up' or op == 'down':
-            ifupdown_handle = ifupdown_main(force=args.force,
+            ifupdown_handle = ifupdownMain(force=args.force,
-                                            dryrun=args.noact,
                                             nodepends=args.nodepends,
                                             perfmode=args.perfmode,
-                                            njobs=args.jobs)
+                                            njobs=args.jobs,
+                                            dryrun=args.noact)
         elif op == 'query':
-            ifupdown_handle = ifupdown_main(dryrun=args.noact,
+            ifupdown_handle = ifupdownMain(nodepends=args.nodepends,
-                                            nodepends=args.nodepends,
+                                            perfmode=args.perfmode,
-                                            perfmode=args.perfmode)
+                                            njobs=args.jobs)
 
         iflist = args.iflist
         if len(args.iflist) == 0:
@@ -33,9 +33,11 @@ def run(args, op):
 
         logger.debug('calling %s' %op + ' for all interfaces ..')
         if op == 'up':
-            ifupdown_handle.up(args.all, args.CLASS, iflist)
+            ifupdown_handle.up(args.all, args.CLASS, iflist,
+                               excludepats=args.excludepats)
         elif op == 'down':
-            ifupdown_handle.down(args.all, args.CLASS, iflist)
+            ifupdown_handle.down(args.all, args.CLASS, iflist,
+                                 excludepats=args.excludepats)
         elif op == 'query':
             if args.curstate == True:
                 qstate='curr'
@@ -46,7 +48,8 @@ def run(args, op):
             else:
                 qstate=None
             ifupdown_handle.query(args.all, args.CLASS, iflist,
-                                  query_state=qstate)
+                                  query_state=qstate,
+                                  excludepats=args.excludepats)
     except:
         raise
 
@@ -73,14 +76,8 @@ def deinit():
     {}
 
 def update_argparser(argparser):
-
     argparser.add_argument('iflist', metavar='IFACE',
                 nargs='*', help='interfaces list')
-    argparser.add_argument('-a', '--all', action='store_true',
-                help='process all interfaces marked \"auto\"')
-    argparser.add_argument('-n', '--no-act', dest='noact',
-                action='store_true', help='print out what would happen,' +
-                'but don\'t do it')
     argparser.add_argument('-v', '--verbose', dest='verbose',
                 action='store_true', help='verbose')
     argparser.add_argument('-d', '--debug', dest='debug',
@@ -93,16 +90,24 @@ def update_argparser(argparser):
                 help='ignore non-\"allow-CLASS\" interfaces')
     argparser.add_argument('--nodepends', dest='nodepends',
                 action='store_true', help=argparse.SUPPRESS)
-    argparser.add_argument('--performance-mode', dest='perfmode',
+    argparser.add_argument('--perfmode', dest='perfmode',
                 action='store_true', help=argparse.SUPPRESS)
+    argparser.add_argument('-j', '--jobs', dest='jobs', type=int,
+                default=-1, choices=range(1,12), help=argparse.SUPPRESS)
+    argparser.add_argument('-X', '--exclude', dest='excludepats',
+                action='append', help='print out what would happen,' +
+                ' but don\'t do it')
 
 
 def update_ifupdown_argparser(argparser):
+    argparser.add_argument('-a', '--all', action='store_true',
+                help='process all interfaces marked \"auto\"')
     argparser.add_argument('-f', '--force', dest='force',
                 action='store_true',
                 help='force run all operations')
-    argparser.add_argument('-j', '--jobs', dest='jobs', type=int,
+    argparser.add_argument('-n', '--no-act', dest='noact',
-                default=-1, choices=range(1,12), help=argparse.SUPPRESS)
+                action='store_true', help='print out what would happen,' +
+                'but don\'t do it')
 
 def update_ifup_argparser(argparser):
     update_ifupdown_argparser(argparser)
@@ -111,6 +116,8 @@ def update_ifdown_argparser(argparser):
     update_ifupdown_argparser(argparser)
 
 def update_ifquery_argparser(argparser):
+    argparser.add_argument('-l', '--list', action='store_true', dest='all',
+                help='process all interfaces marked \"auto\"')
     group = argparser.add_mutually_exclusive_group(required=False)
     group.add_argument('-r', '--running-state', dest='curstate',
                        action='store_true',
@@ -126,6 +133,7 @@ def update_ifquery_argparser(argparser):
                        dest='presumedstatedetailed',
                        action='store_true', help=argparse.SUPPRESS)
 
+
 def parse_args(argsv, op):
     descr = 'interface management'
 
@@ -158,7 +166,7 @@ def main(argv):
         # Command line arg parser
         args = parse_args(argv[1:], op)
         if len(args.iflist) > 0 and args.all is True:
-            print 'iflist and all are mutually exclusive'
+            print 'interface list and \'-a\' are mutually exclusive'
             exit(1)
 
         init(args)

setup.py

@@ -13,7 +13,7 @@ setup(name='ifupdown2',
                       ['man/ifup.8', 'man/ifdown.8', 'man/ifquery.8']),
                   ('/etc/init.d/',
                       ['init.d/networking']),
-                  ('/sbin/ifupdown', ['sbin/ifupdown']),
+                  ('/sbin/', ['sbin/ifupdown']),
                   ('/usr/share/doc/ifupdown/examples/',
                       ['docs/examples/interfaces'])]
       )