add new ifupdown2.conf option ifreload_down_changed to control ifreload

ifdown behaviour.

Testing Done: tested ifreload evo test case

ifreload_down_changed is 0 by default which will make
sure ifreload will not execute down on changed interfaces
but only on deleted interfaces making it non-disruptive.

some notes from CCR:

ifreload was designed to be an optimization for 'service networking
restart' or 'ifdown -a + ifup -a'.
essentially it is a combination of 'ifdown + ifup' with some smarts in
which interfaces it will execute ifdown on.

By default it does the below:
ifdown all interfaces that were deleted from the interfaces file
ifdown all interfaces that were changed from the last time they were
ifup'ed
ifup -a (execute ifup on all interfaces marked auto)

Did not realize people will use ifreload as much as they do today. Also,
they may execute it on a production box when changes are made. ifdown on a production box can be
disruptive because if the ifdown which is part of the ifreload.

To have a non-disruptive option to ifreload, 2.5 added a new option -c
that only executed 'ifup' on all interfaces. Thus reloading all auto +
any other interfaces that were once brought up on the box (essentially
all interfaces present in the saved state file). This by default did not
do anything to the interfaces that got deleted from the file. But had an
ifupdown2.conf toggle to do so.

Looking at the evo use case, they do want to use a single command that
modifies, adds, deletes with
minimum disruption. we can achieve maybe what they want with multiple
commands (But there is also a case of a bug in the build evo is running
which makes it not so easy ).

This patch fixes the bug and also tries to change the default ifreload
behaviour controllable via a variable in ifupdown2.conf.
when ifreload_down_changed=0 in ifupdown2.conf, ifreload will only
ifdown interfaces that were deleted
from the file but not the ones that changed. subsequent ifup as part of
ifreload on the interfaces
that changed will apply the delta. And ifreload_down_changed default
value is '0'.

WIth the patch, ifreload by default will do the below (going back to the
previous default is just a toggle in the ifupdown.conf file):
ifdown all interfaces that were deleted from the interfaces file
ifup -a (execute ifup on all interfaces marked auto)

It sounds like a big change of behaviour for a hotfix release, but
essentially the patch just moves a few things around. And the change in
behaviour is so subtle that it is not very visible.
It just makes it non-disruptive.
(cherry picked from commit 2f7977834d4912a69159d27e54ba201f58a321d8)
(cherry picked from commit 09f283009e7de62ef1d258de81c94cc86fa13323)
(cherry picked from commit 898562e0e2284ccdc201dafc62b25b928037e0c6)

diff --git a/ifupdown2/config/ifupdown2.conf b/ifupdown2/config/ifupdown2.conf
index 8186e488afc936ba735bd6208b09eea09376b06f..82c618e53f36144a9f002e8b91f43096c874bf8a 100644 (file)
--- a/ifupdown2/config/ifupdown2.conf
+++ b/ifupdown2/config/ifupdown2.conf
@@ -38,3 +38,7 @@ link_master_slave=1
 # Delay admin state change till the end
 delay_admin_state_change=0
 
+# ifreload by default downs: 'all interfaces for which config changed' +
+# 'interfaces that were deleted'. With the below variable set to '0'
+# ifreload will only down 'interfaces that were deleted'
+ifreload_down_changed=0

diff --git a/ifupdown2/ifupdown/ifupdownmain.py b/ifupdown2/ifupdown/ifupdownmain.py
index e6e53c6cf86612c62938e8c1a826adabcf978905..c03536bb78e8672451a5abe44d223645c6a6b201 100644 (file)
--- a/ifupdown2/ifupdown/ifupdownmain.py
+++ b/ifupdown2/ifupdown/ifupdownmain.py
@@ -713,7 +713,7 @@ class ifupdownMain(ifupdownBase):
                 pass
 
     def _sched_ifaces(self, ifacenames, ops, skipupperifaces=False,
-                      followdependents=True):
+                      followdependents=True, sort=False):
         self.logger.debug('scheduling \'%s\' for %s'
                           %(str(ops), str(ifacenames)))
         self._pretty_print_ordered_dict('dependency graph',
@@ -724,7 +724,8 @@ class ifupdownMain(ifupdownBase):
                             if 'down' in ops[0]
                                 else ifaceSchedulerFlags.POSTORDER,
                         followdependents=followdependents,
-                        skipupperifaces=skipupperifaces)
+                        skipupperifaces=skipupperifaces,
+                        sort=True if (sort or self.IFACE_CLASS) else False)
 
     def _render_ifacename(self, ifacename):
         new_ifacenames = []
@@ -1061,9 +1062,6 @@ class ifupdownMain(ifupdownBase):
 
         # Override auto to true
         auto = True
-        if auto:
-            self.ALL = True
-            self.WITH_DEPENDS = True
         try:
             self.read_iface_config()
         except:
@@ -1109,12 +1107,15 @@ class ifupdownMain(ifupdownBase):
            self.populate_dependency_info(downops,
                                          already_up_ifacenames_not_present)
            self._sched_ifaces(already_up_ifacenames_not_present, downops,
-                   followdependents=True if self.WITH_DEPENDS else False)
+                              followdependents=False, sort=True)
         else:
            self.logger.debug('no interfaces to down ..')
 
         # Now, run 'up' with new config dict
         # reset statemanager update flag to default
+        if auto:
+            self.ALL = True
+            self.WITH_DEPENDS = True
         if new_ifaceobjdict:
             self.ifaceobjdict = new_ifaceobjdict
             self.dependency_graph = new_dependency_graph
@@ -1136,9 +1137,6 @@ class ifupdownMain(ifupdownBase):
         allow_classes = []
         new_ifaceobjdict = {}
 
-        if auto:
-            self.ALL = True
-            self.WITH_DEPENDS = True
         try:
             self.read_iface_config()
         except:
@@ -1169,13 +1167,17 @@ class ifupdownMain(ifupdownBase):
             filtered_ifacenames = [i for i in ifacenames
                                if self._iface_whitelisted(auto, allow_classes,
                                excludepats, i)]
+
+            # if config file had 'ifreload_down_changed' variable
+            # set, also look for interfaces that changed to down them
+            down_changed = int(self.config.get('ifreload_down_changed', '1'))
+
             # Generate the interface down list
             # Interfaces that go into the down list:
             #   - interfaces that were present in last config and are not
             #     present in the new config
             #   - interfaces that were changed between the last and current
             #     config
-            #
             ifacedownlist = []
             for ifname in filtered_ifacenames:
                 lastifaceobjlist = self.ifaceobjdict.get(ifname)
@@ -1186,11 +1188,14 @@ class ifupdownMain(ifupdownBase):
                 if not newifaceobjlist:
                     ifacedownlist.append(ifname)
                     continue
-                # If interface has changed between the current file
-                # and the last installed append it to the down list
                 if len(newifaceobjlist) != len(lastifaceobjlist):
                     ifacedownlist.append(ifname)
                     continue
+                if not down_changed:
+                    continue
+
+                # If interface has changed between the current file
+                # and the last installed append it to the down list
                 # compare object list
                 for objidx in range(0, len(lastifaceobjlist)):
                     oldobj = lastifaceobjlist[objidx]
@@ -1208,7 +1213,8 @@ class ifupdownMain(ifupdownBase):
                 self.populate_dependency_info(downops, ifacedownlist)
                 try:
                     self._sched_ifaces(ifacedownlist, downops,
-                                       followdependents=False)
+                                       followdependents=False,
+                                       sort=True)
                 except Exception, e:
                     self.logger.error(str(e))
                     pass
@@ -1221,6 +1227,10 @@ class ifupdownMain(ifupdownBase):
         # reset statemanager update flag to default
         if not new_ifaceobjdict:
             return
+
+        if auto:
+            self.ALL = True
+            self.WITH_DEPENDS = True
         self.ifaceobjdict = new_ifaceobjdict
         self.dependency_graph = new_dependency_graph
         ifacenames = self.ifaceobjdict.keys()

diff --git a/ifupdown2/ifupdown/scheduler.py b/ifupdown2/ifupdown/scheduler.py
index 4b4bb57d41790c0fde47642fca685d4fbb4f214e..af9f28c7226c60b19ae5aa642064dbec0deef367 100644 (file)
--- a/ifupdown2/ifupdown/scheduler.py
+++ b/ifupdown2/ifupdown/scheduler.py
@@ -419,7 +419,7 @@ class ifaceScheduler():
     def sched_ifaces(cls, ifupdownobj, ifacenames, ops,
                 dependency_graph=None, indegrees=None,
                 order=ifaceSchedulerFlags.POSTORDER,
-                followdependents=True, skipupperifaces=False):
+                followdependents=True, skipupperifaces=False, sort=False):
         """ runs interface configuration modules on interfaces passed as
             argument. Runs topological sort on interface dependency graph.
 
@@ -439,6 +439,8 @@ class ifaceScheduler():
 
             **followdependents** (bool): follow dependent interfaces if true
 
+            **sort** (bool): sort ifacelist in the case where ALL is not set
+
         """
         #
         # Algo:
@@ -461,25 +463,26 @@ class ifaceScheduler():
                 indegrees[ifacename] = ifupdownobj.get_iface_refcnt(ifacename)
 
         if not ifupdownobj.ALL:
-            # If there is any interface that does exist, maybe it is a
-            # logical interface and we have to followupperifaces when it
-            # comes up, so get that list.
-            if any([True for i in ifacenames
-                    if ifupdownobj.must_follow_upperifaces(i)]):
-                followupperifaces = (True if
+            if 'up' in ops[0]:
+                # If there is any interface that does not exist, maybe it
+                # is a logical interface and we have to followupperifaces
+                # when it comes up, so lets get that list.
+                if any([True for i in ifacenames
+                        if ifupdownobj.must_follow_upperifaces(i)]):
+                    followupperifaces = (True if
                                     [i for i in ifacenames
                                         if not ifupdownobj.link_exists(i)]
                                         else False)
-            if not skip_ifacesort and ifupdownobj.IFACE_CLASS:
-                # sort interfaces only if allow class was specified and
-                # not skip_ifacesort
+            # sort interfaces only if the caller asked to sort
+            # and skip_ifacesort is not on.
+            if not skip_ifacesort and sort:
                 run_queue = cls.get_sorted_iface_list(ifupdownobj, ifacenames,
                                     ops, dependency_graph, indegrees)
                 if run_queue and 'up' in ops[0]:
                     run_queue.reverse()
         else:
-            # if -a is set, we dont really have to sort. We pick the interfaces
-            # that have no parents and 
+            # if -a is set, we pick the interfaces
+            # that have no parents and use a sorted list of those
             if not skip_ifacesort:
                 sorted_ifacenames = cls.get_sorted_iface_list(ifupdownobj,
                                             ifacenames, ops, dependency_graph,
@@ -496,9 +499,14 @@ class ifaceScheduler():
                 else:
                     ifupdownobj.logger.warn('interface sort returned None')
 
-        # If queue not present, just run interfaces that were asked by the user
+        # If queue not present, just run interfaces that were asked by the
+        # user
         if not run_queue:
             run_queue = list(ifacenames)
+            # if we are taking the order of interfaces as specified
+            # in the interfaces file, we should reverse the list if we
+            # want to down. This can happen if 'skip_ifacesort'
+            # is been specified.
             if 'down' in ops[0]:
                 run_queue.reverse()