Tools » teuthology

This problem also occurs in kernel.py

(role_remote,) = ctx.cluster.only(role).remotes.keys()

The above two are the ones that hit on the test just run. I think that this is a problem all over teuthology/task. I have grepped for remote.keys() and found possible problems in:

autotest.py
ceph-deploy.py
ceph.py
common_fs_utils.py
kernel.py
qemu.py
rados_agent.py
rbd.py
s3readwrite.py
s3roundtrip.py
s3tests.py
swift.py

All of the above have statements of the form:

(x, ) = ctx.cluster.only(client).remotes.keys()

Okay, it looks like this code is trying to extract keys but only expects to find one entry. It breaks if the key list ends up being of a length other than one.

>>> (z,) = []
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: need more than 0 values to unpack
>>> (z,) = ['x', 'y']
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: too many values to unpack
>>> (z,) = ['a']
>>> print z
a

Note: My first iteritems comment is probably incorrect.

We should test for the keys list to be non-empty and just set using the first entry, if all we want is one key.

Does anyone know why teuthology uses the following pattern?

(remote,) = ctx.cluster.only(first_mon).remotes.keys()

This throws a value error when remotes.keys() ends up having more than one item in the list. Code that would not throw a value error would be some thing like:

remote = ctx.cluster.only(first_mon).remotes.keys()[0]

If it is an error for remotes to have more than one entry, then we should probably catch that and print something more helpful than throwing a value error. Or we can check the size of remotes first (this would also catch problems if remotes is empty, too).

But getting back to my original question, what's the reason for doing (x,) = [list] rather than x = [list]⁰ ?

A role is only assigned to one host, so there's a bug if it has more than one remote associated with it.

I think that the change here should be to fail but print a more meaningful error message.

Reverted the commit because of #7877.

I think the feature needs more review.

Edit: e.g. see https://github.com/ceph/teuthology/pull/214/files#r11031812

I think that the problem here is that we want to preserve the ValueError behavior of throwing an exception and terminating when this condition occurs.

So I think a better fix may be to throw the exception and only catch it in run.main where a less confusing error message could be written.