ironic and cinder fail to run unit tests with master oslo.config

Bug #1754026 reported by Javier Peña on 2018-03-07
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Cinder
Undecided
Unassigned
Ironic
Fix Released
High
Dmitry Tantsur
Ironic Inspector
In Progress
High
Dmitry Tantsur
oslo.config
Undecided
Unassigned

Bug Description

Trying to run unit test for Cinder or Ironic with the current oslo.config master branch result in a failure. Checking commits, the cause seems to be in https://review.openstack.org/537399 (a previous commit does not cause the issue), but I have not been able to find a cause.

The error message in Cinder unit tests is:

=========================
Failures during discovery
=========================
--- import errors ---
Failed to import test module: cinder.tests.unit.volume.drivers.test_zfssa
Traceback (most recent call last):
  File "/tmp/cinder/.tox/py27/lib/python2.7/site-packages/unittest2/loader.py", line 456, in _find_test_path
    module = self._get_module_from_name(name)
  File "/tmp/cinder/.tox/py27/lib/python2.7/site-packages/unittest2/loader.py", line 395, in _get_module_from_name
    __import__(name)
  File "cinder/tests/unit/volume/drivers/test_zfssa.py", line 38, in <module>
    from cinder.volume.drivers.zfssa import zfssanfs
  File "cinder/volume/drivers/zfssa/zfssanfs.py", line 76, in <module>
    CONF.register_opts(ZFSSA_OPTS, group=configuration.SHARED_CONF_GROUP)
  File "/tmp/cinder/.tox/py27/lib/python2.7/site-packages/oslo_config/cfg.py", line 2486, in __inner
    result = f(self, *args, **kwargs)
  File "/tmp/cinder/.tox/py27/lib/python2.7/site-packages/oslo_config/cfg.py", line 2684, in register_opts
    self.register_opt(opt, group, clear_cache=False)
  File "/tmp/cinder/.tox/py27/lib/python2.7/site-packages/oslo_config/cfg.py", line 2490, in __inner
    return f(self, *args, **kwargs)
  File "/tmp/cinder/.tox/py27/lib/python2.7/site-packages/oslo_config/cfg.py", line 2660, in register_opt
    return group._register_opt(opt, cli)
  File "/tmp/cinder/.tox/py27/lib/python2.7/site-packages/oslo_config/cfg.py", line 1887, in _register_opt
    if _is_opt_registered(self._opts, opt):
  File "/tmp/cinder/.tox/py27/lib/python2.7/site-packages/oslo_config/cfg.py", line 802, in _is_opt_registered
    raise DuplicateOptError(opt.name)
DuplicateOptError: duplicate option: zfssa_rest_timeout

In Ironic:

Captured traceback:
~~~~~~~~~~~~~~~~~~~
    Traceback (most recent call last):
      File "/usr/lib/python2.7/site-packages/mock/mock.py", line 1305, in patched
        return func(*args, **keywargs)
      File "/builddir/build/BUILD/ironic-10.2.0.dev84/ironic/tests/unit/common/test_utils.py", line 441, in test_check_dir_ok
        mock_exists.assert_called_once_with(CONF.tempdir)
      File "/usr/lib/python2.7/site-packages/mock/mock.py", line 318, in assert_called_once_with
        return mock.assert_called_once_with(*args, **kwargs)
      File "/usr/lib/python2.7/site-packages/mock/mock.py", line 947, in assert_called_once_with
        raise AssertionError(msg)
    AssertionError: Expected 'exists' to be called once. Called 61 times.

Doug Hellmann (doug-hellmann) wrote :

The DuplicateOptError is probably caused by reusing a ConfigOpts instance with a slightly different definition of the zfssa_rest_timeout option, and I do see at least 2 places that option is defined, in zfssaiscsi.py and zfssanfs.py:

 $ git grep zfssa_rest_timeout
 cinder/tests/unit/volume/drivers/test_zfssa.py: self.configuration.zfssa_rest_timeout = 60
 cinder/tests/unit/volume/drivers/test_zfssa.py: self.configuration.zfssa_rest_timeout = '30'
 cinder/volume/drivers/zfssa/zfssaiscsi.py: cfg.IntOpt('zfssa_rest_timeout',
 cinder/volume/drivers/zfssa/zfssaiscsi.py: self.zfssa.set_host(lcfg.san_ip, timeout=lcfg.zfssa_rest_timeout)
 cinder/volume/drivers/zfssa/zfssanfs.py: cfg.IntOpt('zfssa_rest_timeout',
 cinder/volume/drivers/zfssa/zfssanfs.py: self.zfssa.set_host(host, timeout=lcfg.zfssa_rest_timeout)
 doc/source/configuration/tables/cinder-zfssa-iscsi.inc: * - ``zfssa_rest_timeout`` = ``None``
 doc/source/configuration/tables/cinder-zfssa-nfs.inc: * - ``zfssa_rest_timeout`` = ``None``

Doug Hellmann (doug-hellmann) wrote :

The problem in Ironic appears to be that the mock is being created too early, so changes in the library behavior are invoking os.path.exists() more often than expected. I suggest moving the decorator that mocks out os.path.exists() to be a context manager inside the test function around the call where the mock is actually needed.

Changed in oslo.config:
status: New → Invalid
Dmitry Tantsur (divius) on 2018-03-12
Changed in ironic:
assignee: nobody → Dmitry Tantsur (divius)
Changed in ironic-inspector:
assignee: nobody → Dmitry Tantsur (divius)
Changed in ironic:
status: New → Triaged
importance: Undecided → High

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

Changed in ironic:
status: Triaged → In Progress
Dmitry Tantsur (divius) on 2018-03-12
Changed in ironic-inspector:
status: New → Confirmed
importance: Undecided → High

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

Changed in ironic-inspector:
status: Confirmed → In Progress

Reviewed: https://review.openstack.org/551973
Committed: https://git.openstack.org/cgit/openstack/ironic/commit/?id=ac65ec6a6eaf4a537e36ad1598e8ce5b818639b1
Submitter: Zuul
Branch: master

commit ac65ec6a6eaf4a537e36ad1598e8ce5b818639b1
Author: Dmitry Tantsur <email address hidden>
Date: Mon Mar 12 12:32:38 2018 +0100

    Use more granular mocking in test_utils

    oslo.config will start using the same functions that we mock in its
    next release, breaking our assumptions on how many times they are
    called.

    Change-Id: Ief2c3d3089916e021cfef38f45f2cc2c78bc4f9b
    Closes-Bug: #1754026

Changed in ironic:
status: In Progress → Fix Released

Reviewed: https://review.openstack.org/551975
Committed: https://git.openstack.org/cgit/openstack/ironic-inspector/commit/?id=5c63f1490c9429841a31af21de5d3461a718c8f5
Submitter: Zuul
Branch: master

commit 5c63f1490c9429841a31af21de5d3461a718c8f5
Author: Dmitry Tantsur <email address hidden>
Date: Mon Mar 12 12:39:58 2018 +0100

    Prevent mocks in test_dnsmasq_pxe_filter from conflicting with oslo.config

    oslo.config starts using os.path functions in the next release in set_override.
    This change modifies unit tests to prevent mocks from clashing with it.

    Change-Id: I6096cb61ef9733e761a5d3d7a24109575f685fe9
    Closes-Bug: #1754026

This issue was fixed in the openstack/ironic-inspector 7.3.0 release.

This issue was fixed in the openstack/ironic 11.0.0 release.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers