Hi! you are right, I think I mistook this with similar bug.
LGTM to your code then! Cheers, Helga On Mon, 18 Jan 2016 at 13:07 Viktor Bachraty <[email protected]> wrote: > Sorry, didn't hit reply to all. > > On Mon, Jan 18, 2016 at 11:00 AM, Viktor Bachraty <[email protected]> > wrote: > >> >> >> On Fri, Jan 15, 2016 at 5:25 PM, Helga Velroyen <[email protected]> >> wrote: >> >>> 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. >>> >> >> I have tried to simulate this scenario by enabling additional disk >> templates (file, sharedfile) with specified paths, then disabling them and >> running cluster verify. Ganeti didn't seem to emit this warning (there were >> similar warnings about the path not being valid, but that was a >> configuration issue - only had to create it). Also I don't see this code >> being called in other places than cluster initialization. Maybe I have >> missed something, we can talk about it in person when you are back. >> >> >>> 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. >>> >>> >> > -- 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.
