commit 07e6884884f7a06827173786708c7e3fc924ee62
Author: Thomas Thrainer <thomasth@google.com>
Date:   Wed Nov 6 10:36:37 2013 +0100

    Don't allow optional node parameters
    
    Ganeti does not support optional fields in parameters
    (hypervisor-params, disk-params, etc.). OpenVSwitch related node
    parameters were the exception to this rule, which caused numerous
    problems related to import/export and (de-)serialization.
    
    The reason for making those parameters optional in the first place was to
    disallow them when OpenVSwitch is not used. This was not consistent with
    other parts of Ganeti, where we allow parameters to be set even though they
    are not actively used.
    
    This patch makes all node parameters mandatory and provides sensible
    defaults for them. Checks which make sure that certain parameters are
    not set in some cases were removed, and the tests adapted. Also, the
    inheritance logic from cluster -> node group -> node was implemented, as
    it was missing previously.
    
    Signed-off-by: Thomas Thrainer <thomasth@google.com>
    Reviewed-by: Hrvoje Ribicic <riba@google.com>

diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py
index a723cfc..ef15532 100644
--- a/lib/cmdlib/cluster.py
+++ b/lib/cmdlib/cluster.py
@@ -196,14 +196,6 @@ class LUClusterPostInit(LogicalUnit):
                    " OpenvSwitch will not have an outside connection. This"
                    " might not be what you want.")
 
-    # OpenvSwitch: Warn if parameters are given, but OVS is not enabled.
-    if (not self.master_ndparams[constants.ND_OVS] and
-        (self.master_ndparams[constants.ND_OVS_NAME] or
-         self.master_ndparams.get(constants.ND_OVS_LINK, None))):
-      self.LogInfo("OpenvSwitch name or link were given, but"
-                   " OpenvSwitch is not enabled. Please enable"
-                   " OpenvSwitch with 'ovs=true' or create it manually")
-
   def BuildHooksEnv(self):
     """Build hooks env.
 
diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
index 3dee0e3..6d0ccd0 100644
--- a/lib/cmdlib/instance.py
+++ b/lib/cmdlib/instance.py
@@ -183,10 +183,6 @@ def _ComputeNics(op, cluster, default_ip, cfg, ec_id):
                                    " is allowed to be passed",
                                    errors.ECODE_INVAL)
 
-    if vlan is not None and nic_mode != constants.NIC_MODE_OVS:
-      raise errors.OpPrereqError("VLAN is given, but network mode is not"
-                                 " openvswitch", errors.ECODE_INVAL)
-
     # ip validity checks
     if ip is None or ip.lower() == constants.VALUE_NONE:
       nic_ip = None
diff --git a/lib/cmdlib/node.py b/lib/cmdlib/node.py
index 89155ac..72c28f6 100644
--- a/lib/cmdlib/node.py
+++ b/lib/cmdlib/node.py
@@ -112,23 +112,6 @@ class LUNodeAdd(LogicalUnit):
       raise errors.OpPrereqError("Cannot pass a node group when a node is"
                                  " being readded", errors.ECODE_INVAL)
 
-    if self.op.ndparams:
-      ovs = self.op.ndparams.get(constants.ND_OVS, None)
-      ovs_name = self.op.ndparams.get(constants.ND_OVS_NAME, None)
-      ovs_link = self.op.ndparams.get(constants.ND_OVS_LINK, None)
-
-      # OpenvSwitch: Warn user if link is missing
-      if ovs and not ovs_link:
-        self.LogInfo("No physical interface for OpenvSwitch was given."
-                     " OpenvSwitch will not have an outside connection. This"
-                     " might not be what you want.")
-      # OpenvSwitch: Fail if parameters are given, but OVS is not enabled.
-      if not ovs and (ovs_name or ovs_link):
-        raise errors.OpPrereqError("OpenvSwitch name or link were given, but"
-                                   " OpenvSwitch is not enabled. Please enable"
-                                   " OpenvSwitch with --ovs",
-                                   errors.ECODE_INVAL)
-
   def BuildHooksEnv(self):
     """Build hooks env.
 
@@ -319,6 +302,24 @@ class LUNodeAdd(LogicalUnit):
         raise errors.OpPrereqError("Checks on node PVs failed: %s" %
                                    "; ".join(errmsgs), errors.ECODE_ENVIRON)
 
+  def _InitOpenVSwitch(self):
+    filled_ndparams = self.cfg.GetClusterInfo().FillND(
+      self.new_node, self.cfg.GetNodeGroup(self.new_node.group))
+
+    ovs = filled_ndparams.get(constants.ND_OVS, None)
+    ovs_name = filled_ndparams.get(constants.ND_OVS_NAME, None)
+    ovs_link = filled_ndparams.get(constants.ND_OVS_LINK, None)
+
+    if ovs:
+      if not ovs_link:
+        self.LogInfo("No physical interface for OpenvSwitch was given."
+                     " OpenvSwitch will not have an outside connection. This"
+                     " might not be what you want.")
+
+      result = self.rpc.call_node_configure_ovs(
+                 self.new_node.name, ovs_name, ovs_link)
+      result.Raise("Failed to initialize OpenVSwitch on new node")
+
   def Exec(self, feedback_fn):
     """Adds the new node to the cluster.
 
@@ -392,14 +393,7 @@ class LUNodeAdd(LogicalUnit):
                       (verifier, nl_payload[failed]))
         raise errors.OpExecError("ssh/hostname verification failed")
 
-    # OpenvSwitch initialization on the node
-    ovs = self.new_node.ndparams.get(constants.ND_OVS, None)
-    ovs_name = self.new_node.ndparams.get(constants.ND_OVS_NAME, None)
-    ovs_link = self.new_node.ndparams.get(constants.ND_OVS_LINK, None)
-
-    if ovs:
-      result = self.rpc.call_node_configure_ovs(
-                 self.new_node.name, ovs_name, ovs_link)
+    self._InitOpenVSwitch()
 
     if self.op.readd:
       self.context.ReaddNode(self.new_node)
diff --git a/lib/utils/__init__.py b/lib/utils/__init__.py
index 276c6fc..23650ca 100644
--- a/lib/utils/__init__.py
+++ b/lib/utils/__init__.py
@@ -99,11 +99,6 @@ def ForceDictType(target, key_types, allowed_values=None):
 
     if ktype in (constants.VTYPE_STRING, constants.VTYPE_MAYBE_STRING):
       if target[key] is None and ktype == constants.VTYPE_MAYBE_STRING:
-        msg = ("'None' is not a valid Maybe value for '%s'. "
-               "Use 'VALUE_HS_NOTHING'") % (key, )
-        logging.warning(msg)
-      elif (target[key] == constants.VALUE_HS_NOTHING
-            and ktype == constants.VTYPE_MAYBE_STRING):
         pass
       elif not isinstance(target[key], basestring):
         if isinstance(target[key], bool) and not target[key]:
diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
index e76388e..a66c30d 100644
--- a/src/Ganeti/Constants.hs
+++ b/src/Ganeti/Constants.hs
@@ -42,8 +42,8 @@ import Data.Map (Map)
 import qualified Data.Map as Map (empty, fromList, keys, insert)
 
 import qualified AutoConf
-import Ganeti.ConstantUtils (PythonChar(..), PythonNone(..), FrozenSet,
-                             Protocol(..), buildVersion)
+import Ganeti.ConstantUtils (PythonChar(..), FrozenSet, Protocol(..),
+                             buildVersion)
 import qualified Ganeti.ConstantUtils as ConstantUtils
 import Ganeti.HTools.Types (AutoRepairResult(..), AutoRepairType(..))
 import qualified Ganeti.HTools.Types as Types
@@ -600,9 +600,6 @@ valueTrue = "true"
 valueFalse :: String
 valueFalse = "false"
 
-valueHsNothing :: Map String PythonNone
-valueHsNothing = Map.fromList [("Nothing", PythonNone)]
-
 -- * Hooks
 
 hooksNameCfgupdate :: String
@@ -2249,7 +2246,7 @@ nicsParameterTypes :: Map String VType
 nicsParameterTypes =
   Map.fromList [(nicMode, vtypeString),
                 (nicLink, vtypeString),
-                (nicVlan, vtypeMaybeString)]
+                (nicVlan, vtypeString)]
 
 nicsParameters :: FrozenSet String
 nicsParameters = ConstantUtils.mkSet (Map.keys nicsParameterTypes)
@@ -3795,7 +3792,7 @@ niccDefaults =
   Map.fromList
   [ (nicMode, PyValueEx nicModeBridged)
   , (nicLink, PyValueEx defaultBridge)
-  , (nicVlan, PyValueEx valueHsNothing)
+  , (nicVlan, PyValueEx "")
   ]
 
 -- | All of the following values are quite arbitrary - there are no
diff --git a/src/Ganeti/Objects.hs b/src/Ganeti/Objects.hs
index ce80d51..5b49713 100644
--- a/src/Ganeti/Objects.hs
+++ b/src/Ganeti/Objects.hs
@@ -245,7 +245,7 @@ instance TimeStampObject Network where
 $(buildParam "Nic" "nicp"
   [ simpleField "mode" [t| NICMode |]
   , simpleField "link" [t| String  |]
-  , simpleField "vlan" [t| Maybe String |]
+  , simpleField "vlan" [t| String |]
   ])
 
 $(buildObject "PartialNic" "nic" $
diff --git a/test/py/cmdlib/instance_unittest.py b/test/py/cmdlib/instance_unittest.py
index 7997dd2..cb191fa 100644
--- a/test/py/cmdlib/instance_unittest.py
+++ b/test/py/cmdlib/instance_unittest.py
@@ -180,15 +180,6 @@ class TestLUInstanceCreate(CmdlibTestCase):
     self.ExecOpCodeExpectOpPrereqError(
       op, "If network is given, no mode or link is allowed to be passed")
 
-  def testVlanWithWrongMode(self):
-    op = self.CopyOpCode(self.diskless_op,
-                         nics=[{
-                           constants.INIC_VLAN: ":1",
-                           constants.INIC_MODE: constants.NIC_MODE_BRIDGED
-                         }])
-    self.ExecOpCodeExpectOpPrereqError(
-      op, "VLAN is given, but network mode is not openvswitch")
-
   def testAutoIpNoNameCheck(self):
     op = self.CopyOpCode(self.diskless_op,
                          nics=[{
diff --git a/test/py/cmdlib/node_unittest.py b/test/py/cmdlib/node_unittest.py
index a3a6384..dc8e124 100644
--- a/test/py/cmdlib/node_unittest.py
+++ b/test/py/cmdlib/node_unittest.py
@@ -35,6 +35,7 @@ from testsupport import *
 
 import testutils
 
+
 # pylint: disable=W0613
 def _TcpPingFailSecondary(cfg, mock_fct, target, port, timeout=None,
                           live_port_needed=None, source=None):
@@ -42,6 +43,7 @@ def _TcpPingFailSecondary(cfg, mock_fct, target, port, timeout=None,
   # and False if not.
   return "192.0.2." in target
 
+
 class TestLUNodeAdd(CmdlibTestCase):
   def setUp(self):
     super(TestLUNodeAdd, self).setUp()
@@ -80,17 +82,6 @@ class TestLUNodeAdd(CmdlibTestCase):
     self.rpc.call_node_verify.return_value = \
       defaultdict(lambda: node_verify_result, {})
 
-  def testOvsParamsButNotEnabled(self):
-    ndparams = {
-      constants.ND_OVS: False,
-      constants.ND_OVS_NAME: "testswitch",
-    }
-
-    op = self.CopyOpCode(self.op_add,
-                         ndparams=ndparams)
-
-    self.ExecOpCodeExpectOpPrereqError(op, "OpenvSwitch is not enabled")
-
   def testOvsNoLink(self):
     ndparams = {
       constants.ND_OVS: True,
diff --git a/test/py/ganeti.utils_unittest.py b/test/py/ganeti.utils_unittest.py
index 2fc0ee5..f57b713 100755
--- a/test/py/ganeti.utils_unittest.py
+++ b/test/py/ganeti.utils_unittest.py
@@ -192,8 +192,7 @@ class TestForceDictType(unittest.TestCase):
     self.assertEqual(self._fdt({"b": "True"}), {"b": True})
     self.assertEqual(self._fdt({"d": "4"}), {"d": 4})
     self.assertEqual(self._fdt({"d": "4M"}), {"d": 4})
-    self.assertEqual(self._fdt({"e": constants.VALUE_HS_NOTHING, }), {"e":
-                               constants.VALUE_HS_NOTHING, })
+    self.assertEqual(self._fdt({"e": None, }), {"e": None, })
     self.assertEqual(self._fdt({"e": "Hello World", }), {"e": "Hello World", })
     self.assertEqual(self._fdt({"e": False, }), {"e": "", })
     self.assertEqual(self._fdt({"b": "hello", }, ["hello"]), {"b": "hello"})
