4:56 a.m.
commit 3b7dd9fd66ee43b4ec51db5199d5e010a74927cd
Author: Ondrej Lichtner <olichtne(a)redhat.com>
Date: Fri Mar 6 12:28:44 2015 +0100
Machine: ensure interface deconfiguration if configure was called
This commit moves the "self._configured = True" assignment to the top of
the Interface configure() methods, right after the check is it's already
set. This ensures that if the configuration raised an exception the
deconfigure method will still be called. This ensures that there are no
internal data structures leftover after the failed configuration.
The regression test #26 was added to test for this bug.
This fixes #106.
Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com>
Signed-off-by: Jiri Pirko <jiri(a)resnulli.us>
lnst/Controller/Machine.py | 6 +++-
regression-tests/tests/26/desc | 7 +++++
regression-tests/tests/26/recipe1.xml | 46 +++++++++++++++++++++++++++++++++
regression-tests/tests/26/recipe2.xml | 46 +++++++++++++++++++++++++++++++++
regression-tests/tests/26/run.sh | 24 +++++++++++++++++
5 files changed, 127 insertions(+), 2 deletions(-)
---
diff --git a/lnst/Controller/Machine.py b/lnst/Controller/Machine.py
index 409e61b..783d357 100644
--- a/lnst/Controller/Machine.py
+++ b/lnst/Controller/Machine.py
@@ -669,6 +669,8 @@ class Interface(object):
"It has been configured already." % (self.get_id(),
self._machine.get_id())
raise MachineError(msg)
+ else:
+ self._configured = True
logging.info("Configuring interface %s on machine %s", self.get_id(),
self._machine.get_id())
@@ -680,7 +682,6 @@ class Interface(object):
else:
self._machine._rpc_call("configure_interface", self.get_id(),
self._get_config())
- self._configured = True
def deconfigure(self):
if not self._configured:
@@ -881,6 +882,8 @@ class SoftInterface(Interface):
def configure(self):
if self._configured:
return
+ else:
+ self._configured = True
logging.info("Configuring interface %s on machine %s", self.get_id(),
self._machine.get_id())
@@ -904,7 +907,6 @@ class SoftInterface(Interface):
dev_name = self._machine._rpc_call("create_soft_interface",
self._id, self._get_config())
self.set_devname(dev_name)
- self._configured = True
def deconfigure(self):
if not self._configured:
diff --git a/regression-tests/tests/26/desc b/regression-tests/tests/26/desc
new file mode 100644
index 0000000..b4d1721
--- /dev/null
+++ b/regression-tests/tests/26/desc
@@ -0,0 +1,7 @@
+always call interface deconfiguration if configuration was called
+
+Interface deconfiguration should always be called if configuration was called,
+even if the configuration itself failed. This ensures that all internal data
+structures are cleaned up.
+
+https://github.com/jpirko/lnst/issues/106
diff --git a/regression-tests/tests/26/recipe1.xml
b/regression-tests/tests/26/recipe1.xml
new file mode 100644
index 0000000..6f71463
--- /dev/null
+++ b/regression-tests/tests/26/recipe1.xml
@@ -0,0 +1,46 @@
+<lnstrecipe>
+ <network>
+ <host id="testmachine1">
+ <params/>
+ <interfaces>
+ <eth id="phy1" label="testnet"/>
+ <eth id="phy2" label="testnet"/>
+ <bond id="nic1">
+ <options>
+ <option name="mode"
value="alance-rr"/>
+ </options>
+ <slaves>
+ <slave id="phy1"/>
+ <slave id="phy2"/>
+ </slaves>
+ <addresses>
+ <address>192.168.100.2/24</address>
+ <address>fe01::1/64</address>
+ </addresses>
+ </bond>
+ </interfaces>
+ </host>
+ <host id="testmachine2">
+ <params/>
+ <interfaces>
+ <eth id="phy1" label="testnet"/>
+ <eth id="phy2" label="testnet"/>
+ <bond id="nic1">
+ <options>
+ <option name="mode"
value="balance-rr"/>
+ </options>
+ <slaves>
+ <slave id="phy1"/>
+ <slave id="phy2"/>
+ </slaves>
+ <addresses>
+ <address>192.168.100.3/24</address>
+ <address>fe01::2/64</address>
+ </addresses>
+ </bond>
+ </interfaces>
+ </host>
+ </network>
+
+ <task python="gre.py"/>
+</lnstrecipe>
diff --git a/regression-tests/tests/26/recipe2.xml
b/regression-tests/tests/26/recipe2.xml
new file mode 100644
index 0000000..7b4cbce
--- /dev/null
+++ b/regression-tests/tests/26/recipe2.xml
@@ -0,0 +1,46 @@
+<lnstrecipe>
+ <network>
+ <host id="testmachine1">
+ <params/>
+ <interfaces>
+ <eth id="phy1" label="testnet"/>
+ <eth id="phy2" label="testnet"/>
+ <bond id="nic1">
+ <options>
+ <option name="mode"
value="balance-rr"/>
+ </options>
+ <slaves>
+ <slave id="phy1"/>
+ <slave id="phy2"/>
+ </slaves>
+ <addresses>
+ <address>192.168.100.2/24</address>
+ <address>fe01::1/64</address>
+ </addresses>
+ </bond>
+ </interfaces>
+ </host>
+ <host id="testmachine2">
+ <params/>
+ <interfaces>
+ <eth id="phy1" label="testnet"/>
+ <eth id="phy2" label="testnet"/>
+ <bond id="nic1">
+ <options>
+ <option name="mode"
value="balance-rr"/>
+ </options>
+ <slaves>
+ <slave id="phy1"/>
+ <slave id="phy2"/>
+ </slaves>
+ <addresses>
+ <address>192.168.100.3/24</address>
+ <address>fe01::2/64</address>
+ </addresses>
+ </bond>
+ </interfaces>
+ </host>
+ </network>
+
+ <task python="gre.py"/>
+</lnstrecipe>
diff --git a/regression-tests/tests/26/run.sh b/regression-tests/tests/26/run.sh
new file mode 100755
index 0000000..a961ba5
--- /dev/null
+++ b/regression-tests/tests/26/run.sh
@@ -0,0 +1,24 @@
+#!/bin/bash
+
+. ../lib.sh
+
+init_test
+
+lnst-ctl -d config_only recipe1.xml | tee test.log
+rv1=${PIPESTATUS[0]}
+log1=`cat test.log`
+
+lnst-ctl -d config_only recipe2.xml | tee test.log
+rv2=${PIPESTATUS[0]}
+log2=`cat test.log`
+
+lnst-ctl -d deconfigure
+
+print_separator
+assert_status "error" "$rv1"
+assert_log "ERROR" "CommandException: Slave testmachine1"
"$log1"
+assert_status "pass" "$rv2"
+
+rm -f test.log
+
+end_test