From: Ondrej Lichtner <olichtne(a)redhat.com>
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>
---
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(-)
create mode 100644 regression-tests/tests/26/desc
create mode 100644 regression-tests/tests/26/recipe1.xml
create mode 100644 regression-tests/tests/26/recipe2.xml
create mode 100755 regression-tests/tests/26/run.sh
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
--
2.1.0
Show replies by date
From: Ondrej Lichtner <olichtne(a)redhat.com>
A log message was missing a space after the hardware address.
Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com>
---
lnst/Controller/VirtUtils.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lnst/Controller/VirtUtils.py b/lnst/Controller/VirtUtils.py
index 544de3b..25a6f39 100644
--- a/lnst/Controller/VirtUtils.py
+++ b/lnst/Controller/VirtUtils.py
@@ -94,7 +94,7 @@ class VirtDomainCtl:
net_name,
driver)
self._domain.attachDevice(device_xml)
- logging.debug("libvirt device with hwaddr '%s'"
+ logging.debug("libvirt device with hwaddr '%s' "
"driver '%s' attached" % (hw_addr, driver))
self._created_interfaces[hw_addr] = device_xml
return True
--
2.1.0