From 9f6bb41fb484e88ebe4cc5dc2b30e159b1d1926e Mon Sep 17 00:00:00 2001 From: Markus Zoeller Date: Tue, 10 May 2016 15:47:06 +0200 Subject: [PATCH] config options: move image_file_url download options In August 2013 the blueprint "image-multiple-location" got merged with commit 6f9ed56. This commit introduced another way of downloading images from glance, despite the normal way via HTTP. This feature introduced new config options which dynamically generate new "nova.conf" groups and therefore got never listed in the sample "nova.conf" file nor in the configuration reference manual. This change moves these options to "nova/conf/". A follow up change will deprecate those options as we believe that this feature is never used in any production environment. bp centralize-config-options-newton Change-Id: I140ce486d70e59eaca127d4bc8274c205a2355ee --- nova/conf/image_file_url.py | 23 +++++++++++++++++++ nova/image/download/file.py | 20 ---------------- .../tests/unit/image/test_transfer_modules.py | 9 ++++++++ 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/nova/conf/image_file_url.py b/nova/conf/image_file_url.py index 1eb7727dd924..eea37c177843 100644 --- a/nova/conf/image_file_url.py +++ b/nova/conf/image_file_url.py @@ -29,13 +29,36 @@ filesystems = cfg.ListOpt( 'image_file_url: ' 'sections')) +# NOTE(jbresnah) because the group under which these options are added is +# dyncamically determined these options need to stay out of global space +# or they will confuse generate_sample.sh +filesystem_opts = [ + cfg.StrOpt('id', + help=_('A unique ID given to each file system. This is ' + 'value is set in Glance and agreed upon here so ' + 'that the operator knowns they are dealing with ' + 'the same file system.')), + cfg.StrOpt('mountpoint', + help=_('The path at which the file system is mounted.')), +] + ALL_OPTS = [filesystems] def register_opts(conf): conf.register_group(image_file_url_group) conf.register_opts(ALL_OPTS, group=image_file_url_group) + for fs in conf.image_file_url.filesystems: + group_name = 'image_file_url:' + fs + conf.register_opts(filesystem_opts, group=group_name) def list_opts(): + # NOTE(markus_z): As the "filesystem" opt has an empty list as a default + # value and this value is necessary for a correct group name, we cannot + # list the "filesystem_opts" for the "nova.conf.sample" file here. A + # follow up patch will deprecate those. Due to their dynamic creation + # they never got shown in "nova.conf.sample" nor the config reference + # manual. I see no need to change this here with a dummy group or somehing + # like that. return {image_file_url_group: ALL_OPTS} diff --git a/nova/image/download/file.py b/nova/image/download/file.py index 308465044d66..7f6dba43e44d 100644 --- a/nova/image/download/file.py +++ b/nova/image/download/file.py @@ -13,7 +13,6 @@ # License for the specific language governing permissions and limitations # under the License. -from oslo_config import cfg from oslo_log import log as logging import nova.conf @@ -63,19 +62,6 @@ class FileTransfer(xfer_base.TransferBase): desc_required_keys = ['id', 'mountpoint'] - # NOTE(jbresnah) because the group under which these options are added is - # dyncamically determined these options need to stay out of global space - # or they will confuse generate_sample.sh - filesystem_opts = [ - cfg.StrOpt('id', - help=_('A unique ID given to each file system. This is ' - 'value is set in Glance and agreed upon here so ' - 'that the operator knowns they are dealing with ' - 'the same file system.')), - cfg.StrOpt('mountpoint', - help=_('The path at which the file system is mounted.')), - ] - def _get_options(self): fs_dict = {} for fs in CONF.image_file_url.filesystems: @@ -89,12 +75,6 @@ class FileTransfer(xfer_base.TransferBase): fs_dict[CONF[group_name].id] = CONF[group_name] return fs_dict - def __init__(self): - # create the needed options - for fs in CONF.image_file_url.filesystems: - group_name = 'image_file_url:' + fs - CONF.register_opts(self.filesystem_opts, group=group_name) - def _verify_config(self): for fs_key in self.filesystems: for r in self.desc_required_keys: diff --git a/nova/tests/unit/image/test_transfer_modules.py b/nova/tests/unit/image/test_transfer_modules.py index af7bc96e90ed..03b184f996c3 100644 --- a/nova/tests/unit/image/test_transfer_modules.py +++ b/nova/tests/unit/image/test_transfer_modules.py @@ -16,10 +16,13 @@ import mock import six.moves.urllib.parse as urlparse +import nova.conf from nova import exception from nova.image.download import file as tm_file from nova import test +CONF = nova.conf.CONF + class TestFileTransferModule(test.NoDBTestCase): @@ -27,6 +30,8 @@ class TestFileTransferModule(test.NoDBTestCase): def test_filesystem_success(self, copy_mock): self.flags(allowed_direct_url_schemes=['file'], group='glance') self.flags(group='image_file_url', filesystems=['gluster']) + # register opts for dynamically created group 'image_file_url:gluster' + nova.conf.image_file_url.register_opts(CONF) mountpoint = '/gluster' url = 'file:///gluster/my/image/path' @@ -52,6 +57,8 @@ class TestFileTransferModule(test.NoDBTestCase): def test_filesystem_mismatched_mountpoint(self, copy_mock): self.flags(allowed_direct_url_schemes=['file'], group='glance') self.flags(group='image_file_url', filesystems=['gluster']) + # register opts for dynamically created group 'image_file_url:gluster' + nova.conf.image_file_url.register_opts(CONF) mountpoint = '/gluster' # Should include the mountpoint before my/image/path @@ -78,6 +85,8 @@ class TestFileTransferModule(test.NoDBTestCase): def test_filesystem_mismatched_filesystem(self, copy_mock): self.flags(allowed_direct_url_schemes=['file'], group='glance') self.flags(group='image_file_url', filesystems=['gluster']) + # register opts for dynamically created group 'image_file_url:gluster' + nova.conf.image_file_url.register_opts(CONF) mountpoint = '/gluster' # Should include the mountpoint before my/image/path