[scenario]/img_dir is deprecated but required

[Colleen Murphy]

The img_dir option is marked as deprecated[1] as part of an in-progress refactor[2]. However, glance scenarios require it[3] so if the user is not using devstack and image files are located somewhere other than /opt/stack/new/devstack/... it must be set. This creates a frustrating user experience because there are warning logs that can't be addressed. It would be better to un-deprecate the option until the refactor is completed and no longer requires this option to be set.

[1] http://git.openstack.org/cgit/openstack/tempest/tree/tempest/config.py?h=17.2.0#n994
[2] https://bugs.launchpad.net/tempest/+bug/1393881
[3] http://git.openstack.org/cgit/openstack/tempest/tree/tempest/scenario/manager.py?h=17.2.0#n389

[Attila Fazekas (afazekas)]

The issue is, if it become undeprecated for a second, it cannot be removed for more than 6~12 month,
if we really want to remove those options we should not do that. I have no issue with keeping those options but in this case the https://bugs.launchpad.net/tempest/+bug/1393881 needs to be invalidated.

[Colleen Murphy (krinkle)]

> if it become undeprecated for a second, it cannot be removed for more than 6~12 month

Why is that a problem? Once the work is complete, giving downstream users a few months to update their configuration to the new recommended config seems fine to me. As it is, there's no recommendation you can make on how to properly configure tempest. Requiring it to be set until the last minute and then ripping it out unexpectedly completely misses the point of deprecation.

[Attila Fazekas (afazekas)]

sounds reasonable,
propose a patch on gerrit and continue the discussion there.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/531142

[Matthew Treinish (treinish)]

The problem here is it's going to be a switchover with no warning when it happens. The deprecation was set to warn users that there is going to be a backwards incompatible change in the future on that option. img_dir is used in one place:

https://github.com/openstack/tempest/blob/master/tempest/scenario/manager.py#L389-L392

the plan has been to move that chunk to just become:

img_path = os.path.abspath(CONF.img_file)

which we could do today. The problem is img_dir would still mean something for the AMI image case which we originally thought would be confusing. So what was decided (admittedly a long time ago) to take a 2 phase approach. The first was to remove all of the AMI options, then the second was to make the switchover to a single option.

There has been a long standing patch to do the AMI option removal: https://review.openstack.org/#/c/338377/ but getting that working in the gate has been beyond tricky. There are huge headaches with devstack's image handling code that had to be fixed and then upgrade implications on the order backports were being merged. (there is a forward upgrade problem if you don't land patches in backwards order)

At this point I'm wondering if we should just abandon the plans to make the img_file option the only one and just accept that img_dir is there forever.

[Colleen Murphy (krinkle)]

> The problem here is it's going to be a switchover with no warning when it happens.

I don't fully understand why it can't be a graceful switchover. I get that tempest is special and doesn't have the same stability guarantees as other projects but it seems like there could be a graceful way to say "Stop using this, do it this other way, and Pretty Soon you won't be allowed to do what you're doing."

> The deprecation was set to warn users that there is going to be a backwards incompatible change in the future on that option.

The warning is doing a terrible job of that. When I see a deprecation warning in the logs with no suggestion of what to replace the option with, I take it to mean the option isn't necessary any more and I can stop setting it. That's not the case here, it must be set. Maybe instead of using oslo.config's deprecation utility you could just emit a regular log saying "be prepared that this will change unexpectedly". Because if it's going to change in a backwards incompatible way, well that's not what deprecation is supposed to help with.

> At this point I'm wondering if we should just abandon the plans to make the img_file option the only one and just accept that img_dir is there forever.

I have no stake in this so I can't help with this :) I'm just hoping to clean up my logs, but if this really is a hard problem it's not killing me to see the warning.

[Matthew Treinish (treinish)]

Tempest has the same stability guarantees as other projects. It just doesn't have a rest api. But for config, library interfaces, CLIs, or any other declared stable interface or end user facing bit it follows the same policy as any other project.

It can't be graceful because right now because img_dir has a default. Right now it's::

  os.path.join(CONF.scenario.img_dir, CONF.scenario.img_file)

and the desired end state is::

  os.path.abspath(CONF.scenario.img_file)

there isn't really a way to handle both without causing some interruption to end users. The normal path for something like this would be::

    if CONF.scenario.img_dir:
        LOG.warn()
        img_file = os.path.join(CONF.scenario.img_dir, CONF.scenario.img_file)
    else:
        img_file = os.path.abspath(CONF.scenario.img_file)

That way you can use the old and the new at the same time and not break anyone. But, because CONF.scenario.img_dir is always set(with that really bad default) we'll never run the else path. If we changed img_dir to not have a default that'll break people who rely on it. TBH we probably should have the deprecation warning that default would be removed in the future instead of it going away, because then that would let us make this transition in 2 cycles (after the UEC image support is removed)

The reason we don't have any alternatives suggested for the img_dir deprecation is that the current plan doesn't have any current steps It's all for a future things. The current plan (which is ~1.5 yrs behind schedule at this point) was to do:

1. deprecate img_dir and uec image options
2. remove uec options
3. remove img_dir option and switch to just using img_file as a path

and the deprecation warning is for #3 because that was supposed to happen in the same cycle as #2.

I think maybe if we still want to clean this up the path forward should be:

1. Change img_dir warning to be about the default changing (not sure of the best mechanism to do that)
2. Finish uec removal patches
3. in rocky (or s if the uec removal isn't done by the release) remove default setting for img_dir and implement the if statement above
4. in s remove img_dir

[OpenStack Infra (hudson-openstack)]

Change abandoned by Colleen Murphy (<email address hidden>) on branch: master
Review: https://review.openstack.org/531142
Reason: I'll abandon for now, I can help with a better solution in a few weeks

[Martin Kopec (mkopec)]

Is CONF.scenario.img_dir still important to glance? I'm wondering how the situation here changed over the last 2 years.

[Martin Kopec (mkopec)]

The following patch ensures in tempest that there is at least one release which accepts both behaviors, for more details check the patch: https://review.opendev.org/#/c/710996/

I proposed bunch of reviews to other projects to help them with the change, see:
https://review.opendev.org/#/q/topic:remove_img_dir

If I forgot any project, please propose a review.

More details regarding the deprecation and removal of the img_dir option can be found here:
https://bugs.launchpad.net/tempest/+bug/1393881

Based on the above facts I believe we can close this LP.