Hi, It looks as if QA is broken with this patch. Did you try to run it on your test cluster? I assume the problem has something do to with the configuration being parsed in Python and Haskell as well. I assumed (based on git blame) that you introduced the VALUE_HS_NOTHING constant, but if Michele knows more about this, feel free to assign the bug to him and then lets see next week why this was done.
Cheers, Thomas On Wed, Aug 14, 2013 at 9:34 PM, Jose A. Lopes <[email protected]> wrote: > From: "Jose A. Lopes" <[email protected]> > > Fix 'NICC_DEFAULTS' to use 'None' instead of 'VALUE_HS_NOTHING' since > that is the value expected by the opcodes, and add 2 tests to ensure > 'NICC_DEFAULTS' is a valid default value for the 'nicparams' parameter > of 'ClusterSetParams' opcode. > > Signed-off-by: Jose A. Lopes <[email protected]> > --- > > This patch SHOULD fix issue 552, but I couldn't run QA because master > is broken (see my previous email). Maybe I need to install something > and if so please let me know and I will run QA and resubmit the patch. > > In any case, this patch replace the use of 'VALUE_HS_NOTHING' with > 'None'. It seems that Michele created 'VALUE_HS_NOTHING' to represent > the value 'Nothing' from Haskell. However, opcodes use 'None' to > represent that value. Therefore, we should ask Michele if > 'VALUE_HS_NOTHING' is strictly necessary for some other reason, or > otherwise it can be completely replaced by 'None'. > > lib/constants.py | 2 +- > test/py/cmdlib/cluster_unittest.py | 5 +++++ > test/py/ganeti.opcodes_unittest.py | 5 +++++ > 3 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/lib/constants.py b/lib/constants.py > index 84b5fac..6e81c7e 100644 > --- a/lib/constants.py > +++ b/lib/constants.py > @@ -2286,7 +2286,7 @@ del _LV_DEFAULTS, _DRBD_DEFAULTS > NICC_DEFAULTS = { > NIC_MODE: NIC_MODE_BRIDGED, > NIC_LINK: DEFAULT_BRIDGE, > - NIC_VLAN: VALUE_HS_NOTHING, > + NIC_VLAN: None, > } > > # All of the following values are quite arbitrarily - there are no > diff --git a/test/py/cmdlib/cluster_unittest.py > b/test/py/cmdlib/cluster_unittest.py > index 55676c6..6809384 100644 > --- a/test/py/cmdlib/cluster_unittest.py > +++ b/test/py/cmdlib/cluster_unittest.py > @@ -611,6 +611,11 @@ class TestLUClusterSetParams(CmdlibTestCase): > op = opcodes.OpClusterSetParams(nicparams=nicparams) > self.ExecOpCode(op) > > + def testNicparamsNiccDefaults(self): > + nicparams = objects.FillDict(constants.NICC_DEFAULTS, {}) > + op = opcodes.OpClusterSetParams(nicparams=nicparams) > + self.ExecOpCode(op) > + > def testDefaultHvparams(self): > hvparams = constants.HVC_DEFAULTS > op = opcodes.OpClusterSetParams(hvparams=hvparams) > diff --git a/test/py/ganeti.opcodes_unittest.py b/test/py/ > ganeti.opcodes_unittest.py > index 63658c3..fb65466 100755 > --- a/test/py/ganeti.opcodes_unittest.py > +++ b/test/py/ganeti.opcodes_unittest.py > @@ -404,5 +404,10 @@ class TestOpInstanceSetParams(unittest.TestCase): > self.assertTrue(fn([[constants.DDM_ADD, {param: param}]])) > > > +class TestOpClusterSetParams(unittest.TestCase): > + def testNicparamsDefaults(self): > + self.assertTrue(ht.TINicParams(constants.NICC_DEFAULTS)) > + > + > if __name__ == "__main__": > testutils.GanetiTestProgram() > -- > 1.8.3 > > -- Thomas Thrainer | Software Engineer | [email protected] | Google Germany GmbH Dienerstr. 12 80331 München Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Graham Law, Christine Elizabeth Flores
