Tempest run --config-file option ineffective

[Hugh Saunders]

Tempest Version: tempest 17.0.0

When executing "tempest run --config-file ..." the --config-file option is ignored and the default config file is used.

Example:

----------------------------------------------
(venv) root@box:~/phobos_verify_workspace# tempest --verbose run --serial --config-file /root/phobos_verify_workspace/rpc-eng-ops/phobos-re-automation/tempest.conf -r 'tempest.scenario.test_minimum_basic.TestMinimumBasicScenario'
tempest initialize_app
2017-10-23 05:45:17.272 43286 INFO tempest [-] Using tempest config file /etc/tempest/tempest.conf
----------------------------------------------

The environment variable TEMPEST_CONFIG_DIR does work:

----------------------------------------------
(venv) root@infra01-utility-container-7c00a4d4:/tmp# TEMPEST_CONFIG_DIR="/root/phobos_verify_workspace/rpc-eng-ops/phobos-re-automation" tempest --verbose run --serial -r 'tempest.scenario.test_minimum_basic.TestMinimumBasicScenario'
tempest initialize_app
2017-10-23 06:11:38.733 43461 INFO tempest [-] Using tempest config file /root/phobos_verify_workspace/rpc-eng-ops/phobos-re-automation/tempest.conf
----------------------------------------------

My assessment of the probable cause:
When --config-file is supplied run.py calls CONF.set_config_path, passing in the config_file path. However config.TempestConfigProxy.set_config_path only sets self._path, which is only read once on the first __getattr__ call on the TempestConfigProxy object. So if there have been any attr lookups on that object prior to set_config_path being called, that path setting call will have no effect.

I added a couple of prints, and showed that getattr is indeed called before set_config_path:

----------------------------------------------
(venv) root@infra01-utility-container-7c00a4d4:/tmp# tempest --verbose run --serial --config-file /root/phobos_verify_workspace/rpc-eng-ops/phobos-re-automation/tempest.conf -r 'tempest.scenario.test_minimum_basic.TestMinimumBasicScenario'
tempest initialize_app
Setting TempestConfigProxy config 139649446298192
config_path: None
2017-10-23 06:23:00.491 43527 INFO tempest [-] Using tempest config file /etc/tempest/tempest.conf
Using tempest config file /etc/tempest/tempest.conf
Setting TempestConfigProxy path 139649446298192
----------------------------------------------

[Matthew Treinish (treinish)]

Have you tested this on master? There was a bug in the clients class causing a gettattr on import, which I think we should have worked around in:

https://review.openstack.org/#/c/494196/

Although we were tracking that on a different command, but it should be the same cause. So I'm thinking we've already fixed this. Can you confirm?

[Hugh Saunders (hughsaunders)]

Hi mtreinish, thanks for your quick response.

Installed from master, new version: tempest 17.0.1.dev182, however that does not seem to fix the problem:

----------------------------------------------
(venv) root@infra01-utility-container-7c00a4d4:~/phobos_verify_workspace/tempest_master# tempest --verbose run --serial --config-file /root/phobos_verify_workspace/rpc-eng-ops/phobos-re-automation/tempest.conf -r 'tempest.scenario.test_minimum_basic.TestMinimumBasicScenario'
tempest initialize_app
2017-10-23 07:47:52.935 43702 INFO tempest [-] Using tempest config file /etc/tempest/tempest.conf
----------------------------------------------

For the avoidance of doubt, the config file specified does exist:

----------------------------------------------
(venv) root@infra01-utility-container-7c00a4d4:~/phobos_verify_workspace/tempest_master# file /root/phobos_verify_workspace/rpc-eng-ops/phobos-re-automation/tempest.conf
/root/phobos_verify_workspace/rpc-eng-ops/phobos-re-automation/tempest.conf: ASCII text
----------------------------------------------

[Matthew Treinish (treinish)]

Hmm, yeah still looks like there are CONF getattrs occurring before the path set even after 494196:

http://paste.openstack.org/show/624375/

I think we need to track these issues down and some unit testing to prevent this in the future

[Hugh Saunders (hughsaunders)]

Maybe config.TempestConfigProxy.set_config_path should throw an exception if getattr has already been called, to show that it will be ignored?

[chandan kumar (chkumar246)]

https://review.openstack.org/#/c/494197/ will help this bug.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/515533
Committed: https://git.openstack.org/cgit/openstack/tempest/commit/?id=9cf0703f20f47143385260a6b63189f1c780f73e
Submitter: Zuul
Branch: master

commit 9cf0703f20f47143385260a6b63189f1c780f73e
Author: Matthew Treinish <email address hidden>
Date: Thu Oct 26 17:46:55 2017 -0400

    Add unit tests to check for CONF getattr during import

    Since the early days in tempest we've been fighting getattrs on CONF
    during imports. We're able to get around this during test runs by lazy
    loading the conf file. However, in things like the tempest commands this
    doesn't work because we rely on the config file not being parsed to set
    the config file path. This commit adds unit tests to check the import
    of the command files for getattrs on CONF. This should prevent future
    regressions.

    While not strictly necessary because of the lazy loading this also gives
    a framework to potentially address the CONF getatrr on discovery. The
    first revision of this patch includes the discovery test, for reference.
    But we have 212 cases of getattr during import (which includes lots of
    skip decorators) so it's unlikely to change any time soon.

    Change-Id: Ib2c15dbd06ca810cc899258758cc8a297055fdf8
    Closes-Bug: #1726357

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/tempest 17.2.0 release.

[Yuki Nishiwaki (uckey-1067)]

I could miss the point so if i'm wrong, please point out.

But I don't think it's possible to pass config-file parameter to each tempest test case in current logic, because basically tempest test case will be executed by new process tempest would spawn by testrepository.commands.run_argv so If we really want to specify that config parameter we have to implement IPC or shared memory or something mechanism.

This problem? or specification is still remaining even if I'm using master branch as of 25 Jan 2018.
So I would reopen this.

[Matthew Treinish (treinish)]

The config file is not used for each individual test case it's loaded once for the entire test suite at the beginning of the run. (it's techincally one load per worker, which occurs at the first get attr on tempest.conf.CONF) The config file is not expected to change at all during the run and the values are only loaded once. From the perspective of a running tempest test the config file is static. An ipc or shared memory mechanism between tests wouldn't provide any benefits because the tests cases are supposed to be isolated from each other during run time. (to provide proper test repeatability) What exactly are you trying to solve?

Either way I'm not sure how this relates to this bug? (which was specifically about the passed in config file being ignored) It sounds like you want to write a new feature for tempest. If so I'd suggest writing a spec to discuss what you think we should add in detail. I've never heard of anyone looking at that before.