On Tue, Apr 2, 2013 at 12:56 PM, Helga Velroyen <hel...@google.com> wrote:
> Hi!
>
>
> On Tue, Apr 2, 2013 at 12:50 PM, Michele Tartara <mtart...@google.com>wrote:
>
>> On Thu, Mar 28, 2013 at 3:50 PM, Helga Velroyen <hel...@google.com>wrote:
>>
>>> This patch adds constants for enabling disk templates to the constants,
>>> and the cluster configuration in haskell and python. It removes the
>>> obsolete preference list for enabled storage types.
>>>
>>> Signed-off-by: Helga Velroyen <hel...@google.com>
>>> ---
>>> lib/constants.py | 53
>>> +++++++++++++++++++++++++++++----------------------
>>> lib/objects.py | 50
>>> ++++++++++++++++++++++++++----------------------
>>> src/Ganeti/Objects.hs | 3 +++
>>> 3 files changed, 60 insertions(+), 46 deletions(-)
>>>
>>> diff --git a/lib/constants.py b/lib/constants.py
>>> index 3926f02..5a3b7a2 100644
>>> --- a/lib/constants.py
>>> +++ b/lib/constants.py
>>> @@ -398,18 +398,6 @@ DEFAULT_ENABLED_STORAGE_TYPES =
>>> compat.UniqueFrozenset([
>>> ST_LVM_VG,
>>> ])
>>>
>>> -# This is used to order determine the default storage type when the list
>>> -# of enabled storage types is inferred from the current state of the
>>> cluster.
>>> -# This only happens on an upgrade from a version of Ganeti that did not
>>> -# support the 'enabled_storage_methods' so far.
>>> -STORAGE_TYPES_PREFERENCE = [
>>> - ST_LVM_VG,
>>> - ST_FILE,
>>> - ST_SHARED_FILE,
>>> - ST_RADOS,
>>> - ST_BLOCK,
>>> - ]
>>> -
>>> # Storage fields
>>> # first two are valid in LU context only, not passed to backend
>>> SF_NODE = "node"
>>> @@ -458,6 +446,36 @@ DT_PLAIN = "plain"
>>> DT_RBD = "rbd"
>>> DT_SHARED_FILE = "sharedfile"
>>>
>>> +# This is used to order determine the default disk template when the
>>> list
>>> +# of enabled disk templates is inferred from the current state of the
>>> cluster.
>>> +# This only happens on an upgrade from a version of Ganeti that did not
>>> +# support the 'enabled_disk_templates' so far.
>>> +DISK_TEMPLATE_PREFERENCE = [
>>> + DT_DRBD8,
>>> + DT_PLAIN,
>>> + DT_FILE,
>>> + DT_SHARED_FILE,
>>> + DT_RBD,
>>> + DT_BLOCK,
>>> + ]
>>> +
>>> +DISK_TEMPLATES = compat.UniqueFrozenset([
>>> + DT_DISKLESS,
>>> + DT_PLAIN,
>>> + DT_DRBD8,
>>> + DT_FILE,
>>> + DT_SHARED_FILE,
>>> + DT_BLOCK,
>>> + DT_RBD,
>>> + DT_EXT
>>> + ])
>>> +
>>>
>>
>> Why are DISKLESS and EXT not in the template preference list as well?
>> And is there a particular reason why DISK_TEMPLATE_PREFERENCE is not a
>> UniqueFrozenset?
>>
>
> They are not in the list, because I did not find good reason for where to
> put it in the list. It does, however, not mean that they are not considered
> at all when applying the preference list. It is just that they get added to
> the configuration "randomly" at the end. If you have a strong opinion about
> which of those should be added at which position, I am happy to discuss
> that.
>
I don't really have a strong opinion about what their position should be,
but I guess they should have one. Even a random one might be ok, but it
should be explicitly written in the preference list.
> It is not a Frozenset, because it should not be a set. The whole point of
> the list is that it is an ordered list indicating in which order the disk
> templates should be added on an upgrade.
>
Oh, right!
>
>
>>
>> +# disk templates that are enabled by default
>>> +DEFAULT_ENABLED_DISK_TEMPLATES = compat.UniqueFrozenset([
>>> + DT_DRBD8,
>>> + DT_PLAIN,
>>> + ])
>>> +
>>> # mapping of disk templates to storage types
>>> DISK_TEMPLATES_STORAGE_TYPE = {
>>> DT_BLOCK: ST_BLOCK,
>>> @@ -645,17 +663,6 @@ RIE_CONNECT_RETRIES = 10
>>> #: Give child process up to 5 seconds to exit after sending a signal
>>> CHILD_LINGER_TIMEOUT = 5.0
>>>
>>> -DISK_TEMPLATES = compat.UniqueFrozenset([
>>> - DT_DISKLESS,
>>> - DT_PLAIN,
>>> - DT_DRBD8,
>>> - DT_FILE,
>>> - DT_SHARED_FILE,
>>> - DT_BLOCK,
>>> - DT_RBD,
>>> - DT_EXT
>>> - ])
>>> -
>>> FILE_DRIVER = compat.UniqueFrozenset([FD_LOOP, FD_BLKTAP])
>>>
>>> # import/export config options
>>> diff --git a/lib/objects.py b/lib/objects.py
>>> index d8dbca0..4cc4c0c 100644
>>> --- a/lib/objects.py
>>> +++ b/lib/objects.py
>>> @@ -477,37 +477,38 @@ class ConfigData(ConfigObject):
>>> self.networks = {}
>>> for network in self.networks.values():
>>> network.UpgradeConfig()
>>> - self._UpgradeStorageTypes()
>>> + self._UpgradeEnabledDiskTemplates()
>>>
>>> - def _UpgradeStorageTypes(self):
>>> - """Upgrade the cluster's enabled storage types by inspecting the
>>> currently
>>> - enabled and/or used storage types.
>>> + def _UpgradeEnabledDiskTemplates(self):
>>> + """Upgrade the cluster's enabled disk templates by inspecting the
>>> currently
>>> + enabled and/or used disk templates.
>>>
>>> """
>>> - # enabled_storage_types in the cluster config were introduced in
>>> 2.8. Remove
>>> - # this code once upgrading from earlier versions is deprecated.
>>> - if not self.cluster.enabled_storage_types:
>>> - storage_type_set = \
>>> - set([constants.DISK_TEMPLATES_STORAGE_TYPE[inst.disk_template]
>>> - for inst in self.instances.values()])
>>> - # Add lvm, file and shared file storage, if they are enabled,
>>> even though
>>> - # they might currently not be used.
>>> + # enabled_disk_templates in the cluster config were introduced in
>>> 2.8.
>>> + # Remove this code once upgrading from earlier versions is
>>> deprecated.
>>> + if not self.cluster.enabled_disk_templates:
>>> + template_set = \
>>> + set([inst.disk_template for inst in self.instances.values()])
>>> + # Add drbd and plain, if lvm is enabled (by specifying a volume
>>> group)
>>> if self.cluster.volume_group_name:
>>> - storage_type_set.add(constants.ST_LVM_VG)
>>> + template_set.add(constants.DT_DRBD8)
>>> + template_set.add(constants.DT_PLAIN)
>>> # FIXME: Adapt this when dis/enabling at configure time is
>>> removed.
>>> + # Enable 'file' and 'sharedfile', if they are enabled, even
>>> though they
>>> + # might currently not be used.
>>> if constants.ENABLE_FILE_STORAGE:
>>> - storage_type_set.add(constants.ST_FILE)
>>> + template_set.add(constants.DT_FILE)
>>> if constants.ENABLE_SHARED_FILE_STORAGE:
>>> - storage_type_set.add(constants.ST_SHARED_FILE)
>>> - # Set enabled_storage_types to the inferred storage types. Order
>>> them
>>> + template_set.add(constants.DT_SHARED_FILE)
>>> + # Set enabled_disk_templates to the inferred disk templates.
>>> Order them
>>> # according to a preference list that is based on Ganeti's
>>> history of
>>> - # supported storage types.
>>> - self.cluster.enabled_storage_types = []
>>> - for preferred_type in constants.STORAGE_TYPES_PREFERENCE:
>>> - if preferred_type in storage_type_set:
>>> - self.cluster.enabled_storage_types.append(preferred_type)
>>> - storage_type_set.remove(preferred_type)
>>> - self.cluster.enabled_storage_types.extend(list(storage_type_set))
>>> + # supported disk templates.
>>> + self.cluster.enabled_disk_templates = []
>>> + for preferred_template in constants.DISK_TEMPLATE_PREFERENCE:
>>> + if preferred_template in template_set:
>>> + self.cluster.enabled_disk_templates.append(preferred_template)
>>> + template_set.remove(preferred_template)
>>> + self.cluster.enabled_disk_templates.extend(list(template_set))
>>>
>>>
>>> class NIC(ConfigObject):
>>> @@ -1540,7 +1541,10 @@ class Cluster(TaggableObject):
>>> "prealloc_wipe_disks",
>>> "hv_state_static",
>>> "disk_state_static",
>>> + # Keeping this in temporarily to not break the build between
>>> patches of
>>> + # this series. Remove after 'enabled_disk_templates' is fully
>>> implemented.
>>> "enabled_storage_types",
>>> + "enabled_disk_templates",
>>> ] + _TIMESTAMPS + _UUID
>>>
>>> def UpgradeConfig(self):
>>> diff --git a/src/Ganeti/Objects.hs b/src/Ganeti/Objects.hs
>>> index bfbab6b..2cb9984 100644
>>> --- a/src/Ganeti/Objects.hs
>>> +++ b/src/Ganeti/Objects.hs
>>> @@ -706,7 +706,10 @@ $(buildObject "Cluster" "cluster" $
>>> , simpleField "primary_ip_family" [t| IpFamily |]
>>> , simpleField "prealloc_wipe_disks" [t| Bool |]
>>> , simpleField "ipolicy" [t| FilledIPolicy |]
>>> + -- FIXME: Remove enabled storage types once enabled disk templates
>>> + -- is fully implemented.
>>> , simpleField "enabled_storage_types" [t| [StorageType] |]
>>> + , simpleField "enabled_disk_templates" [t| [DiskTemplate] |]
>>> ]
>>> ++ timeStampFields
>>> ++ uuidFields
>>> --
>>> 1.8.1.3
>>>
>>>
>> Thanks,
>> Michele
>>
>
> Cheers,
> Helga
>
Thanks,
Michele
