Commit 07e68848 authored by Thomas Thrainer's avatar Thomas Thrainer
Browse files

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: default avatarThomas Thrainer <thomasth@google.com>
Reviewed-by: default avatarHrvoje Ribicic <riba@google.com>
parent 818e28cf
......@@ -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.
......
......@@ -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
......
......@@ -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)
......
......@@ -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]:
......
......@@ -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
......
......@@ -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" $
......
......@@ -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=[{
......
......@@ -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,
......
......@@ -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"})
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment