Commit 57dc299a authored by Iustin Pop's avatar Iustin Pop
Browse files

Further fixes to instance policy validation



As a followup from "Remove extraneous check in policy creation", there
are more places where we build an ipolicy, and then manually check for
its validity. This is very bad style, as it duplicates the
verification code across many places.

This patch removes all such explicit checks (except for one in
cmdlib.py which is correct), and instead does a bit more validation in
the builder functions or in the actual dedicated verification
functions. It also fixes cluster init which used the wrong,
non-completed ipolicy (this was not detected before as we did call
check on it, but otherwise we ignored it), and fixes a too-strong
assert (due to the call chain, we first create the ipolicy from
cmdline params, and only then we fill it).

Finally, it removes an extraneous logging.info which I forgot from
debugging.
Signed-off-by: default avatarIustin Pop <iustin@google.com>
Reviewed-by: default avatarRené Nussbaumer <rn@google.com>
parent 3074ccaf
# #
# #
# Copyright (C) 2006, 2007, 2008, 2010, 2011 Google Inc. # Copyright (C) 2006, 2007, 2008, 2010, 2011, 2012 Google Inc.
# #
# This program is free software; you can redistribute it and/or modify # This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by # it under the terms of the GNU General Public License as published by
...@@ -414,15 +414,10 @@ def InitCluster(cluster_name, mac_prefix, # pylint: disable=R0913, R0914 ...@@ -414,15 +414,10 @@ def InitCluster(cluster_name, mac_prefix, # pylint: disable=R0913, R0914
objects.UpgradeBeParams(beparams) objects.UpgradeBeParams(beparams)
utils.ForceDictType(beparams, constants.BES_PARAMETER_TYPES) utils.ForceDictType(beparams, constants.BES_PARAMETER_TYPES)
utils.ForceDictType(nicparams, constants.NICS_PARAMETER_TYPES) utils.ForceDictType(nicparams, constants.NICS_PARAMETER_TYPES)
for key, val in ipolicy.items():
if key not in constants.IPOLICY_PARAMETERS:
raise errors.OpPrereqError("'%s' is not a valid key for instance policy"
" description", key)
utils.ForceDictType(val, constants.ISPECS_PARAMETER_TYPES)
objects.NIC.CheckParameterSyntax(nicparams) objects.NIC.CheckParameterSyntax(nicparams)
full_ipolicy = objects.FillIPolicy(constants.IPOLICY_DEFAULTS, ipolicy) full_ipolicy = objects.FillIPolicy(constants.IPOLICY_DEFAULTS, ipolicy)
objects.InstancePolicy.CheckParameterSyntax(full_ipolicy)
if ndparams is not None: if ndparams is not None:
utils.ForceDictType(ndparams, constants.NDS_PARAMETER_TYPES) utils.ForceDictType(ndparams, constants.NDS_PARAMETER_TYPES)
...@@ -430,7 +425,8 @@ def InitCluster(cluster_name, mac_prefix, # pylint: disable=R0913, R0914 ...@@ -430,7 +425,8 @@ def InitCluster(cluster_name, mac_prefix, # pylint: disable=R0913, R0914
ndparams = dict(constants.NDC_DEFAULTS) ndparams = dict(constants.NDC_DEFAULTS)
# This is ugly, as we modify the dict itself # This is ugly, as we modify the dict itself
# FIXME: Make utils.ForceDictType pure functional or write a wrapper around it # FIXME: Make utils.ForceDictType pure functional or write a wrapper
# around it
if hv_state: if hv_state:
for hvname, hvs_data in hv_state.items(): for hvname, hvs_data in hv_state.items():
utils.ForceDictType(hvs_data, constants.HVSTS_PARAMETER_TYPES) utils.ForceDictType(hvs_data, constants.HVSTS_PARAMETER_TYPES)
...@@ -526,7 +522,7 @@ def InitCluster(cluster_name, mac_prefix, # pylint: disable=R0913, R0914 ...@@ -526,7 +522,7 @@ def InitCluster(cluster_name, mac_prefix, # pylint: disable=R0913, R0914
primary_ip_family=ipcls.family, primary_ip_family=ipcls.family,
prealloc_wipe_disks=prealloc_wipe_disks, prealloc_wipe_disks=prealloc_wipe_disks,
use_external_mip_script=use_external_mip_script, use_external_mip_script=use_external_mip_script,
ipolicy=ipolicy, ipolicy=full_ipolicy,
hv_state_static=hv_state, hv_state_static=hv_state,
disk_state_static=disk_state, disk_state_static=disk_state,
) )
......
...@@ -149,8 +149,6 @@ def InitCluster(opts, args): ...@@ -149,8 +149,6 @@ def InitCluster(opts, args):
ispecs_disk_templates=ispecs_dts, ispecs_disk_templates=ispecs_dts,
fill_all=True) fill_all=True)
ipolicy = objects.FillIPolicy(constants.IPOLICY_DEFAULTS, ipolicy_raw) ipolicy = objects.FillIPolicy(constants.IPOLICY_DEFAULTS, ipolicy_raw)
for value in ipolicy.values():
utils.ForceDictType(value, constants.ISPECS_PARAMETER_TYPES)
if opts.candidate_pool_size is None: if opts.candidate_pool_size is None:
opts.candidate_pool_size = constants.MASTER_POOL_SIZE_DEFAULT opts.candidate_pool_size = constants.MASTER_POOL_SIZE_DEFAULT
......
# #
# #
# Copyright (C) 2010, 2011 Google Inc. # Copyright (C) 2010, 2011, 2012 Google Inc.
# #
# This program is free software; you can redistribute it and/or modify # This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by # it under the terms of the GNU General Public License as published by
...@@ -55,8 +55,6 @@ def AddGroup(opts, args): ...@@ -55,8 +55,6 @@ def AddGroup(opts, args):
ispecs_disk_size=opts.ispecs_disk_size, ispecs_disk_size=opts.ispecs_disk_size,
ispecs_nic_count=opts.ispecs_nic_count, ispecs_nic_count=opts.ispecs_nic_count,
group_ipolicy=True) group_ipolicy=True)
for key in ipolicy.keys():
utils.ForceDictType(ipolicy[key], constants.ISPECS_PARAMETER_TYPES)
(group_name,) = args (group_name,) = args
diskparams = dict(opts.diskparams) diskparams = dict(opts.diskparams)
......
# #
# #
   
# Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011 Google Inc. # Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012 Google Inc.
# #
# This program is free software; you can redistribute it and/or modify # This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by # it under the terms of the GNU General Public License as published by
...@@ -731,6 +731,9 @@ def _GetUpdatedIPolicy(old_ipolicy, new_ipolicy, group_policy=False): ...@@ -731,6 +731,9 @@ def _GetUpdatedIPolicy(old_ipolicy, new_ipolicy, group_policy=False):
use_none = use_default = group_policy use_none = use_default = group_policy
ipolicy = copy.deepcopy(old_ipolicy) ipolicy = copy.deepcopy(old_ipolicy)
for key, value in new_ipolicy.items(): for key, value in new_ipolicy.items():
if key not in constants.IPOLICY_ALL_KEYS:
raise errors.OpPrereqError("Invalid key in new ipolicy: %s" % key,
errors.ECODE_INVAL)
if key in constants.IPOLICY_PARAMETERS: if key in constants.IPOLICY_PARAMETERS:
utils.ForceDictType(value, constants.ISPECS_PARAMETER_TYPES) utils.ForceDictType(value, constants.ISPECS_PARAMETER_TYPES)
ipolicy[key] = _GetUpdatedParams(old_ipolicy.get(key, {}), value, ipolicy[key] = _GetUpdatedParams(old_ipolicy.get(key, {}), value,
...@@ -747,7 +750,6 @@ def _GetUpdatedIPolicy(old_ipolicy, new_ipolicy, group_policy=False): ...@@ -747,7 +750,6 @@ def _GetUpdatedIPolicy(old_ipolicy, new_ipolicy, group_policy=False):
" on the cluster'" % key, " on the cluster'" % key,
errors.ECODE_INVAL) errors.ECODE_INVAL)
else: else:
logging.info("Setting %s to %s", key, value)
ipolicy[key] = list(value) ipolicy[key] = list(value)
try: try:
objects.InstancePolicy.CheckParameterSyntax(ipolicy) objects.InstancePolicy.CheckParameterSyntax(ipolicy)
......
...@@ -224,6 +224,8 @@ def CreateIPolicyFromOpts(ispecs_mem_size=None, ...@@ -224,6 +224,8 @@ def CreateIPolicyFromOpts(ispecs_mem_size=None,
if ispecs_disk_templates is not None: if ispecs_disk_templates is not None:
ipolicy_out[constants.ISPECS_DTS] = list(ispecs_disk_templates) ipolicy_out[constants.ISPECS_DTS] = list(ispecs_disk_templates)
assert not (frozenset(ipolicy_out.keys()) - constants.IPOLICY_ALL_KEYS)
return ipolicy_out return ipolicy_out
...@@ -884,6 +886,10 @@ class InstancePolicy(ConfigObject): ...@@ -884,6 +886,10 @@ class InstancePolicy(ConfigObject):
InstancePolicy.CheckISpecSyntax(ipolicy, param) InstancePolicy.CheckISpecSyntax(ipolicy, param)
if constants.ISPECS_DTS in ipolicy: if constants.ISPECS_DTS in ipolicy:
InstancePolicy.CheckDiskTemplates(ipolicy[constants.ISPECS_DTS]) InstancePolicy.CheckDiskTemplates(ipolicy[constants.ISPECS_DTS])
wrong_keys = frozenset(ipolicy.keys()) - constants.IPOLICY_ALL_KEYS
if wrong_keys:
raise errors.ConfigurationError("Invalid keys in ipolicy: %s" %
utils.CommaJoin(wrong_keys))
@classmethod @classmethod
def CheckISpecSyntax(cls, ipolicy, name): def CheckISpecSyntax(cls, ipolicy, name):
......
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