Some fixes + enhancements

Ticket: CM-1438
Reviewed By:
Testing Done: Tested installing new ifupdown on the box

- fixed a few things in ifquery
- added new perfmode to skip some of the checks (useful during boot when there is
  no previous state)
- updated doc dir with example
- Added README, TODO and KNOWN_ISSUES file

KNOWN_ISSUES

@@ -0,0 +1,8 @@
+- Some state issues if multiple interfaces for the same interface config blocks are present for the same interface
+- `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
+- setup.py has ifupdown listed in data section instead of scripts: This is because default location for scripts is /usr/bin/. And ifupdown default location is /sbin. With newer versions we can specify --install-scripts directory. This needs to be fixed then.
+
+
+- 

README

@@ -37,3 +37,20 @@ It expects a few things from the pluggable modules:
 	- get_dependent_ifacenames() : must return a list of interfaces the
 	  interface is dependent on. This is used to build the dependency list
 	  for sorting and executing interfaces in parallel.
+
+install instructions
+====================
+- remove existing ifupdown package
+    dpkg -r ifupdown
+
+- download python-ifupdown2*.deb's and install
+
+- or apt-get from testing repo
+    apt-get install python-ifupdown2
+    apt-get install python-ifupdown2-addons
+
+- or install from deb
+    dpkg -i python-ifupdown2-<ver>.deb
+    dpkg -i python-ifupdown2-addons-<ver>.deb
+
+

TODO

@@ -1,7 +1,13 @@
 TODO:
 ====
 - support for /etc/networking.defaults
-- Implement the ifupdown interface mapping feature
+- implement legacy ifupdown mapping feature
+- support for the following ifupdown options:
+    -o OPTION=VALUE     set OPTION to VALUE as though it were in
+                        /etc/network/interfaces
+    --no-mappings       don't run any mappings
+    --no-scripts        don't run any hook scripts
+
 - Fix parallel implementation
 - Test all original ifupdown options for compatibility
 - Test scale
@@ -17,3 +23,8 @@ TODO:
         VERBOSITY indicates  whether --verbose was used; set to 1 if so, 0 if not.
         PATH   the  command   search   path:   /usr/local/sbin:/usr/local/bin:‐
                /usr/sbin:/usr/bin:/sbin:/bin
+
+- option to pretty print dependency graph. Also an option to output in dot format
+
+- make inet, inet6 address handling smarter: query for existing addresses and
+  decide

docs/examples/interfaces

@@ -0,0 +1,54 @@
+
+# The loopback network interface
+auto lo
+iface lo
+   address 10.0.18.3/24
+   address6 2002::2/128
+
+# The primary network interface
+auto eth0
+iface eth0 inet dhcp
+
+auto bond3
+iface bond3 inet static
+    address 100.0.0.4/24
+    bond-slaves swp2 swp3
+    bond-mode 802.3ad
+    bond-miimon 100
+    bond-use-carrier 1
+    bond-lacp-rate 1
+    bond-min-links 1
+    bond-xmit_hash_policy layer3+4
+
+auto br1000
+iface br1000 inet static
+     address 14.0.0.1/24
+     mstpctl_ports glob swp1-4.1000
+     mstpctl_stp on
+     mstpctl_treeprio 32768
+     mstpctl_ageing 200
+
+auto swp5
+iface swp5 inet static
+     address 15.0.0.2/24
+
+auto swp6
+iface swp6
+    address 11.0.3.2/24
+    post-up ip route add 11.0.0.0/8 via 11.0.3.1
+    post-up ip route add 12.0.0.0/8 via 11.0.3.1
+    post-up ip route add 13.0.0.0/8 via 11.0.3.1
+    post-up ip route add 60.0.0.0/8 via 11.0.3.1
+    pre-down ip route del 11.0.0.0/8 via 11.0.3.1
+    pre-down ip route del 12.0.0.0/8 via 11.0.3.1
+    pre-down ip route del 13.0.0.0/8 via 11.0.3.1
+    pre-down ip route del 60.0.0.0/8 via 11.0.3.1
+
+auto br1001
+iface br1001 inet static
+     address 14.0.0.2/24
+     bridge_ports glob swp7-8.1000
+     bridge_stp on
+     bridge_ageing 200
+     bridge_pathcosts swp5.1000=100
+     bridge_portprios swp5.1000=10

pkg/iface.py

@@ -297,18 +297,29 @@ class iface():
         if len(env) > 0:
             self.set_env(env)
 
-    def update_config(self, attr_name, attr_value, attr_status=None):
+    def update_config(self, attr_name, attr_value, attr_status=0):
         if attr_value is None:
             attr_value = ''
 
-        if attr_status == None:
+        if attr_status != 0:
+            self.set_status(ifaceStatus.ERROR)
+        else:
+            if self.get_status() != ifaceStatus.ERROR:
+                self.set_status(ifaceStatus.SUCCESS)
+        if self.config.get(attr_name) is not None:
+            self.config[attr_name].append(attr_value)
+        else:
+            self.config[attr_name] = [attr_value]
+
+        """ XXX: If status needs to be encoded in the query string
+        if attr_status == 0:
+            self.set_status(attr
             attr_status_str = ''
         elif attr_status == 0:
             attr_status_str = ' (success)'
         elif attr_status != 0:
             attr_status_str = ' (error)'
-
+        self.config[attr_name] = attr_value + attr_status_str """
-        self.config[attr_name] = attr_value + attr_status_str
 
     def dump_raw(self, logger):
         indent = '  '
@@ -316,12 +327,6 @@ class iface():
         for i in range(1, len(self.raw_lines)):
             print (indent + self.raw_lines[i])
         
-    def dump_state_pretty(self, logger):
-        indent = '  '
-        logger.info('iface %s' %self.get_name())
-        for attr_name, attr_value in self.get_config_current().items():
-            print (indent + '%s' %attr_name + ' %s' %attr_value)
-
     def dump(self, logger):
         indent = '\t'
         logger.info(self.get_name() + ' : {')
@@ -357,8 +362,9 @@ class iface():
 
         config = self.get_config()
         if config is not None:
-            for cname, cvalue in config.items():
+            for cname, cvaluelist in config.items():
-                outbuf += indent + '%s' %cname + ' %s\n' %cvalue
+                for cv in cvaluelist:
+                    outbuf += indent + '%s' %cname + ' %s\n' %cv
 
         #outbuf += ('%s' %indent + '%s' %self.get_state_str() +
         #                ' %s' %self.get_status_str())

pkg/ifupdown_main.py

@@ -17,12 +17,9 @@ import sys, traceback
 class ifupdown_main():
 
     # Flags
-    FORCE = False
-    NOWAIT = False
-    DRYRUN = False
     NODEPENDS = False
     ALL = False
-    STATE_CHECK = True
+    STATE_CHECK = False
 
     modules_dir='/etc/network'
     builtin_modules_dir='/usr/share/ifupdownaddons'
@@ -54,11 +51,18 @@ class ifupdown_main():
                                  ('post-down' , OrderedDict({}))])}
 
 
-    def __init__(self):
+    def __init__(self, force=False, dryrun=False, nowait=False,
+                 perfmode=False, nodepends=False, njobs=1):
         self.logger = logging.getLogger('ifupdown')
 
+        self.FORCE = force
+        self.DRYRUN = dryrun
+        self.NOWAIT = nowait
+        self.PERFMODE = perfmode
+        self.NODEPENDS = nodepends
+
         self.ifaces = OrderedDict()
-        self.njobs = 1
+        self.njobs = njobs
         self.pp = pprint.PrettyPrinter(indent=4)
         self.load_modules_builtin(self.builtin_modules_dir)
         self.load_modules(self.modules_dir)
@@ -102,6 +106,14 @@ class ifupdown_main():
     def get_dryrun(self):
         return self.DRYRUN
 
+    def set_perfmode(self, perfmode):
+        if perfmode == True:
+            self.logger.debug('setting perfmode to true')
+        self.PERFMODE = perfmode
+
+    def get_perfmode(self):
+        return self.PERFMODE
+
     def set_nowait(self, nowait):
         if nowait == True:
             self.logger.debug('setting dryrun to true')
@@ -326,7 +338,7 @@ class ifupdown_main():
             self.logger.debug('saved module %s' %mkind +
                              ' %s' %mname + ' %s' %mftype)
         else:
-            self.logger.warn('ignoring module %s' %mkind + ' %s' %msubkind +
+            self.logger.info('ignoring module %s' %mkind + ' %s' %msubkind +
                         ' %s' %mname + ' of type %s' %mftype)
 
 
@@ -358,7 +370,8 @@ class ifupdown_main():
 
                     minstance = mclass(force=self.get_force(),
                                        dryrun=self.get_dryrun(),
-                                       nowait=self.get_nowait())
+                                       nowait=self.get_nowait(),
+                                       perfmode=self.get_perfmode())
                     ops = minstance.get_ops()
                     for op in ops:
                         if re.search('up', op) is not None:
@@ -480,7 +493,7 @@ class ifupdown_main():
                 err_iface += ' ' + i
 
         if len(err_iface) != 0:
-            self.logger.error('cound not find ifaces: %s' %err_iface)
+            self.logger.error('did not find interfaces: %s' %err_iface)
             return -1
 
         return 0
@@ -579,7 +592,7 @@ class ifupdown_main():
         if ifacenames is not None:
             # If iface list is given, always check if iface is present
            if self.validate_ifaces(ifacenames) != 0:
-               raise Exception('all or some ifaces not found')
+               raise Exception('all or some interfaces not found')
 
         # if iface list not given by user, assume all from config file
         if ifacenames is None: ifacenames = self.ifaceobjdict.keys()
@@ -609,7 +622,11 @@ class ifupdown_main():
 
         if op == 'query':
             if query_state == 'curr':
-                return self.print_ifaceobjscurr_pretty(filtered_ifacenames)
+                # print curr state of all interfaces
+                ret = self.print_ifaceobjscurr_pretty(filtered_ifacenames)
+                if ret != 0:
+                    # if any of the object has an error, signal that silently
+                    raise Exception('')
             return
 
         # Update persistant iface states
@@ -652,11 +669,24 @@ class ifupdown_main():
             ifaceobjs = self.get_iface_objs(i)
             for io in ifaceobjs:
                 io.dump_raw(self.logger)
+                print '\n'
 
     def print_ifaceobjscurr_pretty(self, ifacenames):
+        """ Dumps current running state of interfaces.
+
+        returns 1 if any of the interface has an error,
+        else returns 0
+
+        """
+
+        ret = 0
         for i in ifacenames:
             ifaceobj = self.get_ifaceobjcurr(i)
             ifaceobj.dump_pretty(self.logger)
+            if ifaceobj.get_status() == ifaceStatus.ERROR:
+                ret = 1
+
+        return ret
 
     def print_ifaceobjs_saved_state_pretty(self, ifacenames):
         self.statemanager.print_state_pretty(ifacenames, self.logger)

pkg/networkinterfaces.py

@@ -58,10 +58,13 @@ class networkInterfaces():
 
     def process_source(self, lines, cur_idx, lineno):
         # Support regex
+        self.logger.debug('process_source ..%s' %lines[cur_idx])
         sourced_file = lines[cur_idx].split(' ', 2)[1]
         if sourced_file is not None:
+            self.logger.debug('process_source ..%s' %sourced_file)
             for f in glob.glob(sourced_file):
-                self.read_file(self, sourced_file)
+                self.logger.info('Reading sourced file %s' %f)
+                self.read_file(f)
         else:
             self.logger.warn('unable to read source line at %d', lineno)
 
@@ -95,18 +98,21 @@ class networkInterfaces():
             if self.ignore_line(l) == 1:
                 continue
 
-            ifaceobj.raw_lines.append(l)
-
             if self.is_keyword(l.split()[0]) == True:
                 line_idx -= 1
                 break
 
-            (attr_name, attrs) = l.split(' ', 1)
+            ifaceobj.raw_lines.append(l)
 
-            if iface_config.get(attr_name) == None:
+            attrs = l.split(' ', 1)
-                iface_config[attr_name] = [attrs.strip(' ')]
+            if len(attrs) < 2:
+                self.logger.warn('invalid syntax at line %d' %(line_idx + 1))
+                continue
+
+            if iface_config.get(attrs[0]) == None:
+                iface_config[attrs[0]] = [attrs[1].strip(' ')]
             else:
-                iface_config[attr_name].append(attrs.strip(' '))
+                iface_config[attrs[0]].append(attrs[1].strip(' '))
 
         lines_consumed = line_idx - cur_idx
 
@@ -175,7 +181,8 @@ class networkInterfaces():
         self.logger.debug('reading ifaces_file %s' %ifaces_file)
 
         with open(ifaces_file) as f:
-            lines = f.readlines()
+            raw_lines = f.readlines()
+            lines = [l.strip(' \n') for l in raw_lines]
 
             while (line_idx < len(lines)):
                 lineno = lineno + 1
@@ -184,8 +191,7 @@ class networkInterfaces():
                     line_idx += 1
                     continue
         
-                l = lines[line_idx].strip('\n ')
+                words = lines[line_idx].split()
-                words = l.split()
 
                 # Check if first element is a supported keyword
                 if self.is_keyword(words[0]):

pkg/statemanager.py

@@ -223,26 +223,27 @@ class stateManager():
             return True
 
     def print_state(self, ifaceobj, prefix, indent):
-        print ('%s' %indent + '%s' %prefix +
+        print (indent + '%s' %prefix +
-               '%s' %firstifaceobj.get_state_str(),
+               '%s' %ifaceobj.get_state_str() +
-               ', %s' %firstifaceobj.get_status_str())
+               ', %s' %ifaceobj.get_status_str())
-
 
     def print_state_pretty(self, ifacenames, logger):
         for ifacename in ifacenames:
             old_ifaceobjs = self.ifaceobjdict.get(ifacename)
             if old_ifaceobjs is not None:
                 firstifaceobj = old_ifaceobjs[0]
-                self.print_state(self, firstifaceobj,
+                self.print_state(firstifaceobj,
-                        '%s: ' %firstifaceobj.get_name(), indent)
+                        '%s: ' %firstifaceobj.get_name(), '')
 
     def print_state_detailed_pretty(self, ifacenames, logger):
+        indent = '\t'
         for ifacename in ifacenames:
             old_ifaceobjs = self.ifaceobjdict.get(ifacename)
             if old_ifaceobjs is not None:
                 for i in old_ifaceobjs:
                     i.dump_pretty(logger)
-                    self.print_state(self, firstifaceobj, '', indent)
+                    self.print_state(i, '', indent)
+            print '\n'
 
     def dump(self, ifacenames=None):
         print 'iface state:'

sbin/ifupdown

@@ -12,25 +12,20 @@ lockfile="/run/network/.lock"
 logger = None
 
 def run(args, op):
-
     logger.debug('args = %s' %str(args))
 
     try:
         logger.debug('creating ifupdown object ..')
-        ifupdown_handle = ifupdown_main()
-
         if op == 'up' or op == 'down':
-            if args.force == True:
+            ifupdown_handle = ifupdown_main(force=args.force,
-                ifupdown_handle.set_force(args.force)
+                                            dryrun=args.noact,
-
+                                            nodepends=args.nodepends,
-            if args.jobs > 0:
+                                            perfmode=args.perfmode,
-                ifupdown_handle.set_njobs(args.jobs)
+                                            njobs=args.jobs)
-
+        elif op == 'query':
-        if args.dryrun == True:
+            ifupdown_handle = ifupdown_main(dryrun=args.noact,
-            ifupdown_handle.set_dryrun(args.dryrun)
+                                            nodepends=args.nodepends,
-
+                                            perfmode=args.perfmode)
-        if args.nodepends == True:
-            ifupdown_handle.set_nodepends(args.nodepends)
 
         iflist = args.iflist
         if len(args.iflist) == 0:
@@ -38,9 +33,9 @@ def run(args, op):
 
         logger.debug('calling %s' %op + ' for all interfaces ..')
         if op == 'up':
-            ifupdown_handle.up(args.all, args.allow, iflist)
+            ifupdown_handle.up(args.all, args.CLASS, iflist)
         elif op == 'down':
-            ifupdown_handle.down(args.all, args.allow, iflist)
+            ifupdown_handle.down(args.all, args.CLASS, iflist)
         elif op == 'query':
             if args.curstate == True:
                 qstate='curr'
@@ -50,12 +45,11 @@ def run(args, op):
                 qstate='presumeddetailed'
             else:
                 qstate=None
-            ifupdown_handle.query(args.all, args.allow, iflist,
+            ifupdown_handle.query(args.all, args.CLASS, iflist,
                                   query_state=qstate)
     except:
         raise
 
-
 def init(args):
     global logger
 
@@ -69,23 +63,24 @@ def init(args):
 
     try:
         logging.basicConfig(level=log_level,
-            format='%(message)s')
+            format='%(levelname)s: %(message)s')
         logger = logging.getLogger('ifupdown')
     except:
         raise
 
 
 def deinit():
-    print 'deinit called'
+    {}
 
 def update_argparser(argparser):
 
     argparser.add_argument('iflist', metavar='IFACE',
                 nargs='*', help='interfaces list')
     argparser.add_argument('-a', '--all', action='store_true',
-                help='operate on all interfaces')
+                help='process all interfaces marked \"auto\"')
-    argparser.add_argument('-n', '--dry-run', dest='dryrun',
+    argparser.add_argument('-n', '--no-act', dest='noact',
-                action='store_true', help='dry run')
+                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',
@@ -94,10 +89,12 @@ def update_argparser(argparser):
     argparser.add_argument('-q', '--quiet', dest='quiet',
                 action='store_true',
                 help=argparse.SUPPRESS)
-    argparser.add_argument('--allow', dest='allow', 
+    argparser.add_argument('--allow', dest='CLASS', 
-                help='allow class')
+                help='ignore non-\"allow-CLASS\" interfaces')
     argparser.add_argument('--nodepends', dest='nodepends',
-                action='store_true', help='dont follow dependents')
+                action='store_true', help=argparse.SUPPRESS)
+    argparser.add_argument('--performance-mode', dest='perfmode',
+                action='store_true', help=argparse.SUPPRESS)
 
 
 def update_ifupdown_argparser(argparser):
@@ -115,10 +112,16 @@ def update_ifdown_argparser(argparser):
 
 def update_ifquery_argparser(argparser):
     group = argparser.add_mutually_exclusive_group(required=False)
-    group.add_argument('-s', '--query-state', dest='curstate',
+    group.add_argument('-r', '--running-state', dest='curstate',
-                       action='store_true', help=argparse.SUPPRESS)
+                       action='store_true',
+                       help='query running state of an interface')
+
+    # presumed-state is state maintained by ifupdown
     group.add_argument('--presumed-state', dest='presumedstate',
                        action='store_true', help=argparse.SUPPRESS)
+
+    # presumed-state-detailed prints all details about the object stored
+    # by ifupdown
     group.add_argument('--presumed-state-detailed',
                        dest='presumedstatedetailed',
                        action='store_true', help=argparse.SUPPRESS)
@@ -161,14 +164,17 @@ def main(argv):
         init(args)
         run(args, op)
     except Exception, e:
+        if str(e) == '':
+            exit(1)
+
         if args.debug == True:
             raise
         else:
             logger.error(str(e))
+        exit(1)
     finally:
         deinit()
 
-
 if __name__ == "__main__":
 
     if not os.geteuid() == 0:
@@ -176,6 +182,7 @@ if __name__ == "__main__":
         exit(1)
 
     """
+    XXX: Cannot use this. A spawned dhclient process can hold the lock
     if not utilities.lockFile(lockfile):
         print 'Another instance of this program is already running.'
         exit(0)

setup.py

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