[dev][stable][release][manila] Should we revert these stable branch backports?
Goutham Pacha Ravi
gouthampravi at gmail.com
Mon Apr 1 04:15:51 UTC 2019
On Sat, Mar 30, 2019 at 1:40 PM Tom Barron <tpb at dyncloud.net> wrote:
> On 27/03/19 11:26 -0700, Goutham Pacha Ravi wrote:
> >On Wed, Mar 27, 2019 at 5:58 AM Tom Barron <tpb at dyncloud.net> wrote:
> >> In manila we recently merged backports of a change  that aims
> >> to fix up faulty configuration option deprecations in the
> >> Dell-EMC VMAX driver. The question at hand is whether (a) we
> >> should revert these backports on the grounds that the
> >> deprecations were faulty and therefore are only effective from
> >> Stein forwards, or whether (b) the deprecations actually worked
> >> and took effect back when Ocata was the development branch but
> >> had faults that should be corrected all the way back to Pike
> >> before it goes EM.
> >> I think (b) is the correct answer but let's check.
> >> On Jan 10 2017 a review  merged that deprecated generic
> >> Dell-EMC driver options in favor of model-specific options .
> >> There were two models at the time, Unity and VNX. This change
> >> was in itself unproblematic.
> >> On Jan 24 2017 a review  merged that introduced a third
> >> Dell-EMC model, VMAX. This new code introduced VMAX specific
> >> options, consistent with the deprecation of the generic Dell-EMC
> >> options. However it had two problems which review  corrects.
> >> When it defined the new VMAX-specific options it failed to
> >> indicate the corresponding old generic options via
> >> 'deprecated_name' . Worse, the code that consumed the options
> >> actually looked for the old generic 'emc_interface_ports' option
> >> instead of the new 'vmax_ethernet_ports' option. The only way to
> >> set a value for this option was to use its deprecated
> >> form.
> >> The change that we backported fixes both of these problems.
> >> I think it is a valid stable backport.
> >I don't agree with some parts of the change. However, I didn't review
> >this patch in time, so my opinion is late, and possibly annoying now.
> >A couple of things with this change are weird, and we could decide to
> >fix these:
> >1) The EMC VMAX driver never had "emc_nas_server_container" or
> >"emc_nas_pool_names" as config options, but the bug fix , adds
> >these as deprecated names for valid options (vmax_server_container,
> >vmax_share_data_pools), in a retrospective manner.
> True, they are added as synonyms for the corresponing model-specific
> options, and are marked as deprecated. This aligns the VMAX config
> with the other EMC models.
> >2) The EMC VMAX driver always used "emc_interface_ports", however,
> >there was no such configuration option wrt that backend. How would
> >users know to set it? Things passed because there was a "safe_get"
> >operation, and perhaps the vendor had called this out in their docs?
> >It's good they fixed this part in Stein.
> There is no question that the VMAX driver code was broken.
> >> If people set this "ports" option using the deprecated option it
> >> will still work. The deprecated option has not been removed yet
> >> and will never be removed from any of the stable branches.
> >> If they tried to set it via the proper option then they would have
> >> had a functional problem, a bug, which this change now fixes.
> >Agree with this point.
> >> If on the other hand we say we cannot backport this then until
> >> stable/stein there will be no way for VMAX users of earlier stable
> >> branches to set this option except by magic -- by using an old
> >> deprecated form of the option that without the backported fix is not
> >> even visible in the customer-facing configuration for VMAX.
> >> All this said, I could well be missing something so I welcome
> >> analysis by stable/core folks. The backports in question have not
> >> yet been released so it would not be a problem to revert them.
> >I think it would be worth back porting the deprecation and association
> >between "emc_interface_ports" and "vmax_ethernet_ports" in the
> >interest of customers/users. However, I think  introduces another
> >bug by exposing net new options for this driver
> >("emc_nas_server_container" and "emc_nas_pool_names") as deprecated
> >forms of existing options ("vmax_server_container",
> >"vmax_share_data_pools"), and that is concerning.
> What is the new bug that this change introduces? Won't the net new
> options work and isn't the configuration backwards compatible in the
> sense that there are no options that used to work that will not work
The bug is introducing deprecated forms of the options, and back
porting that to older releases. Perhaps there's some business/customer
understanding reasoning I don't know of. It can be termed harmless,
because the original and legitimate options will continue to work.
> >> Thanks!
> >> -- Tom Barron
> >>  https://review.openstack.org/#/c/608725/
> >>  https://review.openstack.org/#/c/415079/
> >>  https://review.openstack.org/#/c/404859/
> >>  If this was the only issue then arguably it wasn't an error
> >> since the VMAX driver never operated with the old options.
More information about the openstack-discuss