On Thu, Dec 8, 2011 at 9:14 AM, Iustin Pop <[email protected]> wrote:
> On Tue, Dec 06, 2011 at 06:52:11PM +0100, Agata Murawska wrote:
>> Introduction of command line arguments used later to set instance policy
>> elements, creation of InstancePolicy dictionary from the command line
>> arguments.
>>
>> If there is a nicer way of setting these values, I'd gladly
>> learn about that...
>>
>> Signed-off-by: Agata Murawska <[email protected]>
>> ---
>> lib/cli.py | 27 ++++++++++++++++++
>> lib/client/gnt_cluster.py | 7 +++-
>> lib/objects.py | 66
>> +++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 98 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/cli.py b/lib/cli.py
>> index cde9991..060b2e2 100644
>> --- a/lib/cli.py
>> +++ b/lib/cli.py
>> @@ -65,11 +65,14 @@ __all__ = [
>> "CLUSTER_DOMAIN_SECRET_OPT",
>> "CONFIRM_OPT",
>> "CP_SIZE_OPT",
>> + "CPU_COUNT_SPECS_OPT",
>> "DEBUG_OPT",
>> "DEBUG_SIMERR_OPT",
>> "DISKIDX_OPT",
>> + "DISK_COUNT_SPECS_OPT",
>> "DISK_OPT",
>> "DISK_PARAMS_OPT",
>> + "DISK_SIZE_SPECS_OPT",
>> "DISK_TEMPLATE_OPT",
>> "DRAINED_OPT",
>> "DRY_RUN_OPT",
>> @@ -106,6 +109,7 @@ __all__ = [
>> "MASTER_NETDEV_OPT",
>> "MASTER_NETMASK_OPT",
>> "MC_OPT",
>> + "MEM_COUNT_SPECS_OPT",
>> "MIGRATION_MODE_OPT",
>> "NET_OPT",
>> "NEW_CLUSTER_CERT_OPT",
>> @@ -114,6 +118,7 @@ __all__ = [
>> "NEW_RAPI_CERT_OPT",
>> "NEW_SECONDARY_OPT",
>> "NEW_SPICE_CERT_OPT",
>> + "NIC_COUNT_SPECS_OPT",
>> "NIC_PARAMS_OPT",
>> "NODE_FORCE_JOIN_OPT",
>> "NODE_LIST_OPT",
>> @@ -758,6 +763,28 @@ DISK_PARAMS_OPT = cli_option("-D", "--disk-parameters",
>> dest="diskparams",
>> " template:option=value,option=value,...",
>> type="identkeyval", action="append",
>> default=[])
>>
>> +MEM_COUNT_SPECS_OPT = cli_option("--mem-count-specs",
>> dest="mem_count_ispecs",
>> + type="keyval", default={},
>> + help="Memory count specs: min, max, std"
>> + " (in MB)")
>> +
>> +CPU_COUNT_SPECS_OPT = cli_option("--cpu-count-specs",
>> dest="cpu_count_ispecs",
>> + type="keyval", default={},
>> + help="CPU count specs: min, max, std")
>> +
>> +DISK_COUNT_SPECS_OPT = cli_option("--disk-count-specs",
>> + dest="disk_count_ispecs",
>> + type="keyval", default={},
>> + help="Disk count specs: min, max, std")
>> +
>> +DISK_SIZE_SPECS_OPT = cli_option("--disk-size-specs",
>> dest="disk_size_ispecs",
>> + type="keyval", default={},
>> + help="Disk size specs: min, max, std (in
>> MB)")
>> +
>> +NIC_COUNT_SPECS_OPT = cli_option("--nic-count-specs",
>> dest="nic_count_ispecs",
>> + type="keyval", default={},
>> + help="NIC count specs: min, max, std")
>> +
>
> Once comment is that maybe these would be better groups with prefix,
> e.g. --specs-nic-count. Not sure though.
Makes sense - it's easier to see what all the specs are then, changed.
>
>> HYPERVISOR_OPT = cli_option("-H", "--hypervisor-parameters",
>> dest="hypervisor",
>> help="Hypervisor and hypervisor options, in the"
>> " format
>> hypervisor:option=value,option=value,...",
>> diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
>> index 15a99c8..9d77e59 100644
>> --- a/lib/client/gnt_cluster.py
>> +++ b/lib/client/gnt_cluster.py
>> @@ -1403,7 +1403,8 @@ commands = {
>> MAINTAIN_NODE_HEALTH_OPT, UIDPOOL_OPT, DRBD_HELPER_OPT,
>> NODRBD_STORAGE_OPT,
>> DEFAULT_IALLOCATOR_OPT, PRIMARY_IP_VERSION_OPT,
>> PREALLOC_WIPE_DISKS_OPT,
>> NODE_PARAMS_OPT, GLOBAL_SHARED_FILEDIR_OPT, USE_EXTERNAL_MIP_SCRIPT,
>> - DISK_PARAMS_OPT],
>> + DISK_PARAMS_OPT, MEM_COUNT_SPECS_OPT, CPU_COUNT_SPECS_OPT,
>> + DISK_COUNT_SPECS_OPT, DISK_SIZE_SPECS_OPT, NIC_COUNT_SPECS_OPT],
>> "[opts...] <cluster_name>", "Initialises a new cluster configuration"),
>> "destroy": (
>> DestroyCluster, ARGS_NONE, [YES_DOIT_OPT],
>> @@ -1480,7 +1481,9 @@ commands = {
>> MAINTAIN_NODE_HEALTH_OPT, UIDPOOL_OPT, ADD_UIDS_OPT, REMOVE_UIDS_OPT,
>> DRBD_HELPER_OPT, NODRBD_STORAGE_OPT, DEFAULT_IALLOCATOR_OPT,
>> RESERVED_LVS_OPT, DRY_RUN_OPT, PRIORITY_OPT, PREALLOC_WIPE_DISKS_OPT,
>> - NODE_PARAMS_OPT, USE_EXTERNAL_MIP_SCRIPT, DISK_PARAMS_OPT],
>> + NODE_PARAMS_OPT, USE_EXTERNAL_MIP_SCRIPT, DISK_PARAMS_OPT,
>> + MEM_COUNT_SPECS_OPT, CPU_COUNT_SPECS_OPT, DISK_COUNT_SPECS_OPT,
>> + DISK_SIZE_SPECS_OPT, NIC_COUNT_SPECS_OPT],
>> "[opts...]",
>> "Alters the parameters of the cluster"),
>> "renew-crypto": (
>> diff --git a/lib/objects.py b/lib/objects.py
>> index 4616bb7..fed3f93 100644
>> --- a/lib/objects.py
>> +++ b/lib/objects.py
>> @@ -44,6 +44,7 @@ from cStringIO import StringIO
>> from ganeti import errors
>> from ganeti import constants
>> from ganeti import netutils
>> +from ganeti import utils
>>
>> from socket import AF_INET
>>
>> @@ -54,6 +55,29 @@ __all__ = ["ConfigObject", "ConfigData", "NIC", "Disk",
>> "Instance",
>> _TIMESTAMPS = ["ctime", "mtime"]
>> _UUID = ["uuid"]
>>
>> +# constants used to create InstancePolicy dictionary
>> +TISPECS_GROUP_DEFAULTS = {
>> + constants.MIN_ISPECS: 0,
>> + constants.MAX_ISPECS: 0,
>> + }
>> +
>> +TISPECS_CLUSTER_DEFAULTS = {
>> + constants.MIN_ISPECS: 0,
>> + constants.MAX_ISPECS: 0,
>> + constants.STD_ISPECS: 0,
>> + }
>
> Hmm, why 0? Are you using 0 as "not modified" tag later?
Nah, I just took ForceDictType for something it is not (i.e. I thought
it required
all the fields to be there, and not 'if the field is there it should
have the type'.
No idea why, though. Anyway - removed.
>
>> +TISPECS_GROUP_TYPES = {
>> + constants.MIN_ISPECS: constants.VTYPE_INT,
>> + constants.MAX_ISPECS: constants.VTYPE_INT,
>> +}
>> +
>> +TISPECS_CLUSTER_TYPES = {
>> + constants.MIN_ISPECS: constants.VTYPE_INT,
>> + constants.MAX_ISPECS: constants.VTYPE_INT,
>> + constants.STD_ISPECS: constants.VTYPE_INT,
>> + }
>> +
>>
>> def FillDict(defaults_dict, custom_dict, skip_keys=None):
>> """Basic function to apply settings on top a default dict.
>> @@ -148,6 +172,48 @@ def UpgradeDiskParams(diskparams):
>> return result
>>
>>
>> +def CreateIPolicyFromOpts(mem_count_ispecs=None,
>> + cpu_count_ispecs=None,
>> + disk_count_ispecs=None,
>> + disk_size_ispecs=None,
>> + nic_count_ispecs=None,
>> + group_ipolicy=False,
>> + allowed_values=None):
>> + """Creation of instane policy based on command line options.
>> +
>> +
>> + """
>> + # prepare ipolicy dict
>> + ipolicy_transposed = {
>> + constants.MEM_COUNT_SPEC: mem_count_ispecs,
>> + constants.CPU_COUNT_SPEC: cpu_count_ispecs,
>> + constants.DISK_COUNT_SPEC: disk_count_ispecs,
>> + constants.DISK_SIZE_SPEC: disk_size_ispecs,
>> + constants.NIC_COUNT_SPEC: nic_count_ispecs,
>> + }
>> +
>> + # first, check that the values given are correct
>> + if group_ipolicy:
>> + defaults = TISPECS_GROUP_DEFAULTS
>> + forced_type = TISPECS_GROUP_TYPES
>> + else:
>> + defaults = TISPECS_CLUSTER_DEFAULTS
>> + forced_type = TISPECS_CLUSTER_TYPES
>> +
>> + for specs in ipolicy_transposed.values():
>> + specs = FillDict(defaults, specs)
>> + utils.ForceDictType(specs, forced_type, allowed_values=allowed_values)
>> +
>> + # then transpose
>> + ipolicy_out = constants.IPOLICY_EMPTY
>
> Again, the bug I referred to in the first patch. Mutable types, hate
> hate hate.
Ack, ack, ack
>
>> + for name, specs in ipolicy_transposed.iteritems():
>
> items() please (instead of iteritems()).
Ack
>
>> + assert name in constants.ISPECS_PARAMETERS
>> + for key, val in specs.iteritems(): # {min: .. ,max: .., std: ..}
>> + ipolicy_out[key][name] = val
>
> thanks,
> iustin
Also, since both cluster init and modify use options for instance
policy, I extracted them as a separate list.
Interdiff:
diff --git a/lib/cli.py b/lib/cli.py
index 060b2e2..41adb79 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -65,14 +65,11 @@ __all__ = [
"CLUSTER_DOMAIN_SECRET_OPT",
"CONFIRM_OPT",
"CP_SIZE_OPT",
- "CPU_COUNT_SPECS_OPT",
"DEBUG_OPT",
"DEBUG_SIMERR_OPT",
"DISKIDX_OPT",
- "DISK_COUNT_SPECS_OPT",
"DISK_OPT",
"DISK_PARAMS_OPT",
- "DISK_SIZE_SPECS_OPT",
"DISK_TEMPLATE_OPT",
"DRAINED_OPT",
"DRY_RUN_OPT",
@@ -109,7 +106,6 @@ __all__ = [
"MASTER_NETDEV_OPT",
"MASTER_NETMASK_OPT",
"MC_OPT",
- "MEM_COUNT_SPECS_OPT",
"MIGRATION_MODE_OPT",
"NET_OPT",
"NEW_CLUSTER_CERT_OPT",
@@ -118,7 +114,6 @@ __all__ = [
"NEW_RAPI_CERT_OPT",
"NEW_SECONDARY_OPT",
"NEW_SPICE_CERT_OPT",
- "NIC_COUNT_SPECS_OPT",
"NIC_PARAMS_OPT",
"NODE_FORCE_JOIN_OPT",
"NODE_LIST_OPT",
@@ -171,6 +166,11 @@ __all__ = [
"SHOWCMD_OPT",
"SHUTDOWN_TIMEOUT_OPT",
"SINGLE_NODE_OPT",
+ "SPECS_CPU_COUNT_OPT",
+ "SPECS_DISK_COUNT_OPT",
+ "SPECS_DISK_SIZE_OPT",
+ "SPECS_MEM_SIZE_OPT",
+ "SPECS_NIC_COUNT_OPT",
"SPICE_CACERT_OPT",
"SPICE_CERT_OPT",
"SRC_DIR_OPT",
@@ -763,25 +763,25 @@ DISK_PARAMS_OPT = cli_option("-D",
"--disk-parameters", dest="diskparams",
" template:option=value,option=value,...",
type="identkeyval", action="append", default=[])
-MEM_COUNT_SPECS_OPT = cli_option("--mem-count-specs", dest="mem_count_ispecs",
+SPECS_MEM_SIZE_OPT = cli_option("--specs-mem-size", dest="ispecs_mem_size",
type="keyval", default={},
help="Memory count specs: min, max, std"
" (in MB)")
-CPU_COUNT_SPECS_OPT = cli_option("--cpu-count-specs", dest="cpu_count_ispecs",
+SPECS_CPU_COUNT_OPT = cli_option("--specs-cpu-count", dest="ispecs_cpu_count",
type="keyval", default={},
help="CPU count specs: min, max, std")
-DISK_COUNT_SPECS_OPT = cli_option("--disk-count-specs",
- dest="disk_count_ispecs",
+SPECS_DISK_COUNT_OPT = cli_option("--specs-disk-count",
+ dest="ispecs_disk_count",
type="keyval", default={},
help="Disk count specs: min, max, std")
-DISK_SIZE_SPECS_OPT = cli_option("--disk-size-specs", dest="disk_size_ispecs",
+SPECS_DISK_SIZE_OPT = cli_option("--specs-disk-size", dest="ispecs_disk_size",
type="keyval", default={},
help="Disk size specs: min, max, std (in MB)")
-NIC_COUNT_SPECS_OPT = cli_option("--nic-count-specs", dest="nic_count_ispecs",
+SPECS_NIC_COUNT_OPT = cli_option("--specs-nic-count", dest="ispecs_nic_count",
type="keyval", default={},
help="NIC count specs: min, max, std")
diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
index 9d77e59..ccf6137 100644
--- a/lib/client/gnt_cluster.py
+++ b/lib/client/gnt_cluster.py
@@ -1392,6 +1392,13 @@ def Epo(opts, args):
else:
return _EpoOff(opts, node_list, inst_map)
+instance_policy_opts = [
+ SPECS_CPU_COUNT_OPT,
+ SPECS_DISK_COUNT_OPT,
+ SPECS_DISK_SIZE_OPT,
+ SPECS_MEM_SIZE_OPT,
+ SPECS_NIC_COUNT_OPT,
+ ]
commands = {
"init": (
@@ -1403,8 +1410,7 @@ commands = {
MAINTAIN_NODE_HEALTH_OPT, UIDPOOL_OPT, DRBD_HELPER_OPT,
NODRBD_STORAGE_OPT,
DEFAULT_IALLOCATOR_OPT, PRIMARY_IP_VERSION_OPT, PREALLOC_WIPE_DISKS_OPT,
NODE_PARAMS_OPT, GLOBAL_SHARED_FILEDIR_OPT, USE_EXTERNAL_MIP_SCRIPT,
- DISK_PARAMS_OPT, MEM_COUNT_SPECS_OPT, CPU_COUNT_SPECS_OPT,
- DISK_COUNT_SPECS_OPT, DISK_SIZE_SPECS_OPT, NIC_COUNT_SPECS_OPT],
+ DISK_PARAMS_OPT] + instance_policy_opts,
"[opts...] <cluster_name>", "Initialises a new cluster configuration"),
"destroy": (
DestroyCluster, ARGS_NONE, [YES_DOIT_OPT],
@@ -1481,9 +1487,8 @@ commands = {
MAINTAIN_NODE_HEALTH_OPT, UIDPOOL_OPT, ADD_UIDS_OPT, REMOVE_UIDS_OPT,
DRBD_HELPER_OPT, NODRBD_STORAGE_OPT, DEFAULT_IALLOCATOR_OPT,
RESERVED_LVS_OPT, DRY_RUN_OPT, PRIORITY_OPT, PREALLOC_WIPE_DISKS_OPT,
- NODE_PARAMS_OPT, USE_EXTERNAL_MIP_SCRIPT, DISK_PARAMS_OPT,
- MEM_COUNT_SPECS_OPT, CPU_COUNT_SPECS_OPT, DISK_COUNT_SPECS_OPT,
- DISK_SIZE_SPECS_OPT, NIC_COUNT_SPECS_OPT],
+ NODE_PARAMS_OPT, USE_EXTERNAL_MIP_SCRIPT, DISK_PARAMS_OPT] +
+ instance_policy_opts,
"[opts...]",
"Alters the parameters of the cluster"),
"renew-crypto": (
diff --git a/lib/objects.py b/lib/objects.py
index b8fc04d..47dd451 100644
--- a/lib/objects.py
+++ b/lib/objects.py
@@ -56,17 +56,6 @@ _TIMESTAMPS = ["ctime", "mtime"]
_UUID = ["uuid"]
# constants used to create InstancePolicy dictionary
-TISPECS_GROUP_DEFAULTS = {
- constants.MIN_ISPECS: 0,
- constants.MAX_ISPECS: 0,
- }
-
-TISPECS_CLUSTER_DEFAULTS = {
- constants.MIN_ISPECS: 0,
- constants.MAX_ISPECS: 0,
- constants.STD_ISPECS: 0,
- }
-
TISPECS_GROUP_TYPES = {
constants.MIN_ISPECS: constants.VTYPE_INT,
constants.MAX_ISPECS: constants.VTYPE_INT,
@@ -183,11 +172,11 @@ def MakeEmptyIPolicy():
])
-def CreateIPolicyFromOpts(mem_count_ispecs=None,
- cpu_count_ispecs=None,
- disk_count_ispecs=None,
- disk_size_ispecs=None,
- nic_count_ispecs=None,
+def CreateIPolicyFromOpts(ispecs_mem_size=None,
+ ispecs_cpu_count=None,
+ ispecs_disk_count=None,
+ ispecs_disk_size=None,
+ ispecs_nic_count=None,
group_ipolicy=False,
allowed_values=None):
"""Creation of instane policy based on command line options.
@@ -196,30 +185,27 @@ def CreateIPolicyFromOpts(mem_count_ispecs=None,
"""
# prepare ipolicy dict
ipolicy_transposed = {
- constants.MEM_COUNT_SPEC: mem_count_ispecs,
- constants.CPU_COUNT_SPEC: cpu_count_ispecs,
- constants.DISK_COUNT_SPEC: disk_count_ispecs,
- constants.DISK_SIZE_SPEC: disk_size_ispecs,
- constants.NIC_COUNT_SPEC: nic_count_ispecs,
+ constants.MEM_SIZE_SPEC: ispecs_mem_size,
+ constants.CPU_COUNT_SPEC: ispecs_cpu_count,
+ constants.DISK_COUNT_SPEC: ispecs_disk_count,
+ constants.DISK_SIZE_SPEC: ispecs_disk_size,
+ constants.NIC_COUNT_SPEC: ispecs_nic_count,
}
# first, check that the values given are correct
if group_ipolicy:
- defaults = TISPECS_GROUP_DEFAULTS
forced_type = TISPECS_GROUP_TYPES
else:
- defaults = TISPECS_CLUSTER_DEFAULTS
forced_type = TISPECS_CLUSTER_TYPES
for specs in ipolicy_transposed.values():
- specs = FillDict(defaults, specs)
utils.ForceDictType(specs, forced_type, allowed_values=allowed_values)
# then transpose
- ipolicy_out = constants.IPOLICY_EMPTY
+ ipolicy_out = MakeEmptyIPolicy()
for name, specs in ipolicy_transposed.iteritems():
assert name in constants.ISPECS_PARAMETERS
- for key, val in specs.iteritems(): # {min: .. ,max: .., std: ..}
+ for key, val in specs.items(): # {min: .. ,max: .., std: ..}
ipolicy_out[key][name] = val
return ipolicy_out
