Generally, this looks, good, but I think there is the following problem:

This works in the way that when the cluster is initialized and file-storage
is not enabled, that then the directory is set properly.

However, it is possible to init the cluster with file storage (and setting
the dir), but later disabling file storage but not clearing the file
storage dir. This is a valid use case, because it might be a temporary
disabling of file storage where you still want to keep the storage dir
configured. We accept this in other cases as well.

In this case, I think cluster verify needs to be changed so that it does
not emit this warning when the file storage type is disabled. This would
solve the problem more generally and not only when initializiing the
cluster.

On Fri, 15 Jan 2016 at 17:18 'Viktor Bachraty' via ganeti-devel <
[email protected]> wrote:

> This removes superfluous warning about storage types that are not
> enabled during cluster initialization.
>
> Signed-off-by: Viktor Bachraty <[email protected]>
> ---
>  lib/bootstrap.py | 9 +++++++--
>  lib/cli_opts.py  | 3 +--
>  2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/lib/bootstrap.py b/lib/bootstrap.py
> index 80f0da7..eaad4ec 100644
> --- a/lib/bootstrap.py
> +++ b/lib/bootstrap.py
> @@ -354,8 +354,14 @@ def _PrepareFileBasedStorage(
>              constants.ST_FILE, constants.ST_SHARED_FILE,
> constants.ST_GLUSTER
>           ))
>
> +  file_storage_enabled = file_disk_template in enabled_disk_templates
> +
>    if file_storage_dir is None:
> -    file_storage_dir = default_dir
> +    if file_storage_enabled:
> +      file_storage_dir = default_dir
> +    else:
> +      file_storage_dir = ""
> +
>    if not acceptance_fn:
>      acceptance_fn = \
>          lambda path: filestorage.CheckFileStoragePathAcceptance(
> @@ -364,7 +370,6 @@ def _PrepareFileBasedStorage(
>    _storage_path_acceptance_fn(logging.warning, file_storage_dir,
>                                enabled_disk_templates)
>
> -  file_storage_enabled = file_disk_template in enabled_disk_templates
>    if file_storage_enabled:
>      try:
>        acceptance_fn(file_storage_dir)
> diff --git a/lib/cli_opts.py b/lib/cli_opts.py
> index 73a2ca9..1728684 100644
> --- a/lib/cli_opts.py
> +++ b/lib/cli_opts.py
> @@ -1188,8 +1188,7 @@ GLOBAL_GLUSTER_FILEDIR_OPT = cli_option(
>    help="Specify the default directory (cluster-wide) for mounting Gluster"
>    " file systems [%s]" %
>    pathutils.DEFAULT_GLUSTER_STORAGE_DIR,
> -  metavar="GLUSTERDIR",
> -  default=pathutils.DEFAULT_GLUSTER_STORAGE_DIR)
> +  metavar="GLUSTERDIR", default=None)
>
>  NOMODIFY_ETCHOSTS_OPT = cli_option("--no-etc-hosts",
> dest="modify_etc_hosts",
>                                     help="Don't modify %s" %
> pathutils.ETC_HOSTS,
> --
> 2.6.0.rc2.230.g3dd15c0
>
> --

Helga Velroyen
Software Engineer
[email protected]

Google Germany GmbH
Erika-Mann-Strasse 33
80636 München

Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den Absender und
löschen Sie die E-Mail und alle Anhänge. Vielen Dank.

This e-mail is confidential. If you are not the right addressee please do
not forward it, please inform the sender, and please erase this e-mail
including any attachments. Thanks.

Reply via email to