commit 3b7dd9fd66ee43b4ec51db5199d5e010a74927cd Author: Ondrej Lichtner olichtne@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@redhat.com Signed-off-by: Jiri Pirko jiri@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