From: Ondrej Lichtner olichtne@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@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