Comment 26 for bug 1960758

Revision history for this message
Mauricio Faria de Oliveira (mfo) wrote :

Hi Andreas o/

Thanks for the discussion! Addressing your points:

> When I see a new config file option being added already in a deprecated
> state, I have to wonder if it's the right fix: [...]

The usage of a deprecated config option is arbitrary (it may be not used),
but the semantics are actually correct [1] for the scenario of this issue
(as the config option is not available in future releases):

  class oslo_config.cfg.Opt(...)

    deprecated_for_removal – indicates whether this opt is planned for removal in a future release
    deprecated_reason – indicates why this opt is planned for removal in a future release. [...]
    deprecated_since – indicates which release this opt was deprecated in. [...]

> If we need a workaround, maybe it shouldn't be in the config file like
> that. It's opt-in, the default value is such that it doesn't change
> behavior, sounds like the config file is just a means for documentation.

I can appreciate your opinion (and have a similar take on this, in general),
however, the package/project goes with all options being in the config file
(with docs), including a dedicated 'workarounds' section, with many opt-ins:

  $ grep -e '^\[' -e '^#[^ ]' nova.conf
  ...
  [workarounds]
  #disable_rootwrap = false
  #disable_libvirt_livesnapshot = false
  #handle_virt_lifecycle_events = true
  #disable_group_policy_check_upcall = false
  #enable_numa_live_migration = false
  #ensure_libvirt_rbd_instance_dir_cleanup = false
  #disable_fallback_pcpu_query = false
  #never_download_image_if_on_rbd = false
  #disable_native_luksv1 = false
  #rbd_volume_local_attach = false
  #reserve_disk_resource_for_image_cache = false
  ...

(There are technical reasons this workaround is in the defaults section,
instead of workarounds: for integration with the juju charm, see MR [2].)

> If this is indeed the only way to address this, could it not be in the
> default config file, and perhaps just be a "hidden" option that will be
> honored in this version? Its documentation could be elsewhere.

Although there are technical means to achieve that suggestion (eg, d/rules
sed, or non-standard usage of the config option library in the source code):
- that does not seem to be what the users of this package expect; and
- hiding it could hinder someone affected to find out the solution needed.

So, I'd suggest that we continue with that the package/project already use.

...

Hopefully the points above address your questions and make the upload be
considered for SRU acceptance again, as-is.

Summary:
- there have been other config file changes in the past (~25% of uploads
after focal-release, as in #21);
- the package/project already describes all config options in nova.conf
(with documentation), including workarounds and deprecations, which are
correct/in-line with the release-scope of the introduced config option.

Thanks again,
Mauricio

[1] https://docs.openstack.org/oslo.config/ussuri/reference/api/oslo_config.html#oslo_config.cfg.ConfigOpts
[2] https://code.launchpad.net/~mfo/ubuntu/+source/nova/+git/nova/+merge/447799/comments/1198731