Change in vdsm[master]: sourceroute: split gathering of routes and rules off configu...

Ondřej Svoboda has uploaded a new change for review.

Change subject: sourceroute: split gathering of routes and rules off configure() and
remove()
......................................................................

sourceroute: split gathering of routes and rules off configure() and remove()

The code is too complex (for me) to understand. I want to remove the use of
network configurators from the module because they actually call sourceroute
classes' methods only to be called back:

Ifcfg.configureNic, Configurator._addSourceRoute, StaticSourceRoute.configure,
Ifcfg.configureSourceRoute

Ifcfg.removeNic, Ifcfg._ifaceDownAndCleanup, Configurator._removeSourceRoute,
StaticSourceRoute.remove (or DynamicSourceRoute), Ifcfg.removeSourceRoute

Change-Id: I69d8949afbd5b8ba2cf14930533cda99a4580eb0
Signed-off-by: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
---
M vdsm/network/sourceroute.py
1 file changed, 30 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/93/41593/1

diff --git a/vdsm/network/sourceroute.py b/vdsm/network/sourceroute.py
index 5f06de0..d7916ed 100644
--- a/vdsm/network/sourceroute.py
+++ b/vdsm/network/sourceroute.py
@@ -25,11 +25,7 @@
 
 from vdsm import netinfo
 from vdsm.constants import P_VDSM_RUN
-from vdsm.ipwrapper import IPRoute2Error
-from vdsm.ipwrapper import Route
-from vdsm.ipwrapper import routeShowTable
-from vdsm.ipwrapper import Rule
-from vdsm.ipwrapper import ruleList
+from vdsm.ipwrapper import IPRoute2Error, Route, routeShowTable, Rule, ruleList
 from vdsm.utils import rmFile
 
 
@@ -67,23 +63,32 @@
                 Rule(destination=self._network, table=self._table,
                      srcDevice=self.device)]
 
-    def configure(self):
+    def _prepare_configuration(self):
         logging.info(("Configuring gateway - ip: %s, network: %s, " +
                       "subnet: %s, gateway: %s, table: %s, device: %s") %
                      (self._ipaddr, self._network, self._mask, self._gateway,
                       self._table, self.device))
 
-        routes = self._buildRoutes()
-        rules = self._buildRules()
+        self.routes = self._buildRoutes()
+        self.rules = self._buildRules()
 
+    def configure(self):
+        self._prepare_configuration()
         try:
-            self._configurator.configureSourceRoute(routes, rules, self.device)
+            self._configurator.configureSourceRoute(self.routes, self.rules,
+                                                    self.device)
         except IPRoute2Error as e:
             logging.error('ip binary failed during source route configuration'
                           ': %s', e.message)
 
+    def _prepare_removal(self):
+        self.routes = None
+        self.rules = None
+
     def remove(self):
-        self._configurator.removeSourceRoute(None, None, self.device)
+        self._prepare_removal()
+        self._configurator.removeSourceRoute(self.routes, self.rules,
+                                             self.device)
 
 
 class DynamicSourceRoute(StaticSourceRoute):
@@ -157,20 +162,23 @@
 
         return rules
 
+    def _prepare_removal(self):
+        self.rules = self._getRules(self.device)
+        if self.rules:
+            self.table = self._getTable(self.rules)
+            if self.table:
+                self.routes = self._getRoutes(self.table)
+
     def remove(self):
         logging.info("Removing gateway - device: %s", self.device)
-
-        rules = self._getRules(self.device)
-        if rules:
-            table = self._getTable(rules)
-            if table:
-                try:
-                    self._configurator.removeSourceRoute(
-                        self._getRoutes(table), rules,
-                        self.device)
-                except IPRoute2Error as e:
-                    logging.error('ip binary failed during source route '
-                                  'removal: %s' % e.message)
+        self._prepare_removal()
+        if self.rules and self.table:
+            try:
+                self._configurator.removeSourceRoute(
+                    self.routes, self.rules, self.device)
+            except IPRoute2Error as e:
+                logging.error('ip binary failed during source route '
+                              'removal: %s' % e.message)
 
     @staticmethod
     def _isLibvirtInterfaceFallback(device):


-- 
To view, visit https://gerrit.ovirt.org/41593
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I69d8949afbd5b8ba2cf14930533cda99a4580eb0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com&gt;

automation(a)ovirt.org has posted comments on this change.

Change subject: sourceroute: split gathering of routes and rules off configure() and
remove()
......................................................................


Patch Set 1:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/41593
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I69d8949afbd5b8ba2cf14930533cda99a4580eb0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: sourceroute: split gathering of routes and rules off configure() and
remove()
......................................................................


Patch Set 2:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/41593
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I69d8949afbd5b8ba2cf14930533cda99a4580eb0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: sourceroute: split gathering of routes and rules off configure() and
remove()
......................................................................


Patch Set 3:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/41593
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I69d8949afbd5b8ba2cf14930533cda99a4580eb0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: sourceroute: split gathering of routes and rules off configure() and
remove()
......................................................................


Patch Set 4:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/41593
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I69d8949afbd5b8ba2cf14930533cda99a4580eb0
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

[Jenkins CI RO]

Jenkins CI RO has abandoned this change.

Change subject: sourceroute: split gathering of routes and rules off configure() and
remove()
......................................................................


Abandoned

Abandoned due to no activity - please restore if still relevant

-- 
To view, visit https://gerrit.ovirt.org/41593
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: I69d8949afbd5b8ba2cf14930533cda99a4580eb0
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;

gerrit-hooks has posted comments on this change.

Change subject: sourceroute: split gathering of routes and rules off configure() and
remove()
......................................................................


Patch Set 4:

* Update tracker: IGNORE, no Bug-Url found

-- 
To view, visit https://gerrit.ovirt.org/41593
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I69d8949afbd5b8ba2cf14930533cda99a4580eb0
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No