On Wed, Dec 7, 2011 at 9:02 PM, Iustin Pop <[email protected]> wrote:
> On Tue, Dec 06, 2011 at 06:52:10PM +0100, Agata Murawska wrote:
>> Introduction of instance policy - the required defaults and types are
>> provided, along with the syntax check and changes to config writer.
>
>> +# instance specs
>> +MEM_COUNT_SPEC = "memory-count"
>
> Hmm, this seems badly chosen (was it like this in the design?). I would
> rather expect MEM_SIZE.
Ack, changed
>
>> +CPU_COUNT_SPEC = "cpu-count"
>> +DISK_COUNT_SPEC = "disk-count"
>> +DISK_SIZE_SPEC = "disk-size"
>> +NIC_COUNT_SPEC = "nic-count"
>> +
>> +ISPECS_PARAMETER_TYPES = {
>> + MEM_COUNT_SPEC: VTYPE_INT,
>> + CPU_COUNT_SPEC: VTYPE_INT,
>> + DISK_COUNT_SPEC: VTYPE_INT,
>> + DISK_SIZE_SPEC: VTYPE_INT,
>> + NIC_COUNT_SPEC: VTYPE_INT,
>> + }
>
> also, the _SPEC suffix is somewhat opposite to a more common SPEC_
> prefix, but that's fine (at least it has a suffix).
Ack, I will submit a separate patch for that and for MIN_ISPECS ->
ISPECS_MIN, if that is fine (since these are used in many patches in
this series)
>
>> +IPOLICY_EMPTY = {
>> + MIN_ISPECS: {},
>> + MAX_ISPECS: {},
>> + STD_ISPECS: {},
>> + }
>
> This is somewhat dangerous. Especially when you do this:
>
>> + # instance policy added before 2.6
>> + if self.ipolicy is None:
>> + self.ipolicy = constants.IPOLICY_EMPTY
>
> This should be constants.IPOLICY.copy(), but that's not enough since you
> have embedded dicts.
>
> I would suggest a different way, replace IPOLICY_EMPTY with
> makeEmptyIPolicy: return dict(...). Similar to how we build defaults in
> the lib/ht.py code.
Ack
>
> thanks,
> iustin
Interdiff:
diff --git a/lib/constants.py b/lib/constants.py
index 3c8ad19..dd94a18 100644
--- a/lib/constants.py
+++ b/lib/constants.py
@@ -905,14 +905,14 @@ BES_PARAMETER_COMPAT.update(BES_PARAMETER_TYPES)
BES_PARAMETERS = frozenset(BES_PARAMETER_TYPES.keys())
# instance specs
-MEM_COUNT_SPEC = "memory-count"
+MEM_SIZE_SPEC = "memory-size"
CPU_COUNT_SPEC = "cpu-count"
DISK_COUNT_SPEC = "disk-count"
DISK_SIZE_SPEC = "disk-size"
NIC_COUNT_SPEC = "nic-count"
ISPECS_PARAMETER_TYPES = {
- MEM_COUNT_SPEC: VTYPE_INT,
+ MEM_SIZE_SPEC: VTYPE_INT,
CPU_COUNT_SPEC: VTYPE_INT,
DISK_COUNT_SPEC: VTYPE_INT,
DISK_SIZE_SPEC: VTYPE_INT,
@@ -1776,21 +1776,21 @@ NICC_DEFAULTS = {
IPOLICY_DEFAULTS = {
MIN_ISPECS: {
- MEM_COUNT_SPEC: 128,
+ MEM_SIZE_SPEC: 128,
CPU_COUNT_SPEC: 1,
DISK_COUNT_SPEC: 1,
DISK_SIZE_SPEC: 1024,
NIC_COUNT_SPEC: 1,
},
MAX_ISPECS: {
- MEM_COUNT_SPEC: 128,
+ MEM_SIZE_SPEC: 128,
CPU_COUNT_SPEC: 1,
DISK_COUNT_SPEC: 1,
DISK_SIZE_SPEC: 1024,
NIC_COUNT_SPEC: 1,
},
STD_ISPECS: {
- MEM_COUNT_SPEC: 128,
+ MEM_SIZE_SPEC: 128,
CPU_COUNT_SPEC: 1,
DISK_COUNT_SPEC: 1,
DISK_SIZE_SPEC: 1024,
@@ -1798,12 +1798,6 @@ IPOLICY_DEFAULTS = {
}
}
-IPOLICY_EMPTY = {
- MIN_ISPECS: {},
- MAX_ISPECS: {},
- STD_ISPECS: {},
- }
-
MASTER_POOL_SIZE_DEFAULT = 10
CONFD_PROTOCOL_VERSION = 1
diff --git a/lib/objects.py b/lib/objects.py
index 4616bb7..8761a36 100644
--- a/lib/objects.py
+++ b/lib/objects.py
@@ -148,6 +148,17 @@ def UpgradeDiskParams(diskparams):
return result
+def MakeEmptyIPolicy():
+ """Create empty IPolicy dictionary.
+
+ """
+ return dict([
+ (constants.MIN_ISPECS, dict()),
+ (constants.MAX_ISPECS, dict()),
+ (constants.STD_ISPECS, dict()),
+ ])
+
+
class ConfigObject(object):
"""A generic config object.
@@ -1403,7 +1414,7 @@ class Cluster(TaggableObject):
# instance policy added before 2.6
if self.ipolicy is None:
- self.ipolicy = constants.IPOLICY_EMPTY
+ self.ipolicy = MakeEmptyIPolicy()
@property
def primary_hypervisor(self):
