Merge lp:~vila/uci-engine/unique-queues into lp:uci-engine

[Vincent Ladeuil]

Always use unique queue names for progress report

When running at scale, time.time() to "uniquify" a queue name is not enough, multiple workers can call time.time() at the same... time.

I ran into this while debugging uci-britney and ending up with orphaned queues that still had unprocessed messages.

'prefetch' was adding to the confusion but this collision on queue names is also a nasty trap that is hard to encounter with deployments with a single unit of each type (or even two for the lander).

[Paul Larson (pwlars)]

You may want to consider uuid4 instead of uuid1. IIRC uuid1 generates the uuid based on mac address and time... so if these happen on the same node at the same time, I think you could still risk collision right?

Something to consider, but if you know a good reason why that's not a risk, feel free. uuid4 is just a straight pseudorandom uuid, which isn't wonderful, but possibly a better fit here.
Otherwise +1 from me

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:862
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1626/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1626/rebuild

review: Needs Fixing (continuous-integration)

[Vincent Ladeuil (vila)]

Thanks for the review !

> You may want to consider uuid4 instead of uuid1. IIRC uuid1 generates the
> uuid based on mac address and time... so if these happen on the same node at
> the same time, I think you could still risk collision right?

As Joe verified, this doesn't seem to be case.

As I mentioned at the sprint, I just reused uuid1 because that's what was used elsewhere and we may want to revisit later. In the meantime, I think it's safe and I prefer to stay consistent.

>
> Something to consider, but if you know a good reason why that's not a risk,
> feel free. uuid4 is just a straight pseudorandom uuid, which isn't wonderful,
> but possibly a better fit here.

Yeah, that may apply everywhere we used uuid1.

[Vincent Ladeuil (vila)]

Thanks for the review !

> You may want to consider uuid4 instead of uuid1. IIRC uuid1 generates the
> uuid based on mac address and time... so if these happen on the same node at
> the same time, I think you could still risk collision right?

As Joe verified, this doesn't seem to be case.

As I mentioned at the sprint, I just reused uuid1 because that's what was used elsewhere and we may want to revisit later. In the meantime, I think it's safe and I prefer to stay consistent.

>
> Something to consider, but if you know a good reason why that's not a risk,
> feel free. uuid4 is just a straight pseudorandom uuid, which isn't wonderful,
> but possibly a better fit here.

Yeah, that may apply everywhere we used uuid1.

[Vincent Ladeuil (vila)]

Yeah, launchpad, I won't say it twice, I won't say it twice...

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:863
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1633/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1633/rebuild

review: Approve (continuous-integration)

[Ubuntu CI Bot (uci-bot)]

The attempt to merge lp:~vila/uci-engine/unique-queues into lp:uci-engine failed. Below is the output from the failed tests.

INFO:root:Creating a virtualenv to run under...
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/virtualenv.py", line 2339, in <module>
    main()
  File "/usr/lib/python2.7/dist-packages/virtualenv.py", line 825, in main
    symlink=options.symlink)
  File "/usr/lib/python2.7/dist-packages/virtualenv.py", line 985, in create_environment
    site_packages=site_packages, clear=clear, symlink=symlink))
  File "/usr/lib/python2.7/dist-packages/virtualenv.py", line 1193, in install_python
    writefile(site_filename_dst, SITE_PY)
  File "/usr/lib/python2.7/dist-packages/virtualenv.py", line 487, in writefile
    f.write(content.encode('utf-8'))
IOError: [Errno 28] No space left on device
INFO:root:virtualenv created in 0.12s.
Traceback (most recent call last):
  File "bin/called-by-tarmac.py", line 140, in <module>
    sys.exit(main())
  File "bin/called-by-tarmac.py", line 110, in main
    venv.install()
  File "/tmp/tarmac/branch.eq_OUn/bin/../testing/venv.py", line 139, in install
    path = create()
  File "/tmp/tarmac/branch.eq_OUn/bin/../testing/venv.py", line 53, in create
    subprocess.check_call(cmd, stdout=devnull)
  File "/usr/lib/python2.7/subprocess.py", line 540, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['virtualenv', '/dev/shm/venv-h7vcEN', '-p', 'python2.7']' returned non-zero exit status 1

[Ubuntu CI Bot (uci-bot)]

The attempt to merge lp:~vila/uci-engine/unique-queues into lp:uci-engine failed. Below is the output from the failed tests.

INFO:root:Creating a virtualenv to run under...
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/virtualenv.py", line 2339, in <module>
    main()
  File "/usr/lib/python2.7/dist-packages/virtualenv.py", line 825, in main
    symlink=options.symlink)
  File "/usr/lib/python2.7/dist-packages/virtualenv.py", line 985, in create_environment
    site_packages=site_packages, clear=clear, symlink=symlink))
  File "/usr/lib/python2.7/dist-packages/virtualenv.py", line 1193, in install_python
    writefile(site_filename_dst, SITE_PY)
  File "/usr/lib/python2.7/dist-packages/virtualenv.py", line 487, in writefile
    f.write(content.encode('utf-8'))
IOError: [Errno 28] No space left on device
INFO:root:virtualenv created in 0.13s.
Traceback (most recent call last):
  File "bin/called-by-tarmac.py", line 140, in <module>
    sys.exit(main())
  File "bin/called-by-tarmac.py", line 110, in main
    venv.install()
  File "/tmp/tarmac/branch.3mbnF7/bin/../testing/venv.py", line 139, in install
    path = create()
  File "/tmp/tarmac/branch.3mbnF7/bin/../testing/venv.py", line 53, in create
    subprocess.check_call(cmd, stdout=devnull)
  File "/usr/lib/python2.7/subprocess.py", line 540, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['virtualenv', '/dev/shm/venv-AdF9XT', '-p', 'python2.7']' returned non-zero exit status 1

=== modified file 'lander/lander/tests/test_service_wrapper.py'
--- lander/lander/tests/test_service_wrapper.py	2014-10-07 10:04:01 +0000
+++ lander/lander/tests/test_service_wrapper.py	2014-10-23 22:13:26 +0000
@@ -38,9 +38,9 @@
         patch = mock.patch('ci_utils.amqp_utils.get_config')
         patch.start()
         self.addCleanup(patch.stop)
-        patchtime = mock.patch('time.time', return_value=1234567890)
-        patchtime.start()
-        self.addCleanup(patchtime.stop)
+        patch_uuid1 = mock.patch('uuid.uuid1', return_value='1234567890')
+        patch_uuid1.start()
+        self.addCleanup(patch_uuid1.stop)
 
         self.results = tempfile.mkstemp(prefix='lander-test-')[1]
         self.addCleanup(os.unlink, self.results)

=== modified file 'lander/lander/workflow.py'
--- lander/lander/workflow.py	2014-10-07 10:04:01 +0000
+++ lander/lander/workflow.py	2014-10-23 22:13:26 +0000
@@ -14,7 +14,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import json
-import time
+import uuid
 
 import lander.ticket_api
 
@@ -62,7 +62,7 @@
 
 def create(ts_url, ticket_data):
     data = {
-        'progress_trigger': 'lander-%d' % time.time(),
+        'progress_trigger': 'lander-{}'.format(uuid.uuid1()),
         'ticket_id': ticket_data['id'],
         'ts_url': ts_url,
         'steps': ticket_data['steps'],

=== modified file 'test_runner/bin/check_worker.py'
--- test_runner/bin/check_worker.py	2014-10-10 13:14:33 +0000
+++ test_runner/bin/check_worker.py	2014-10-23 22:13:26 +0000
@@ -16,8 +16,8 @@
 
 import json
 import logging
-import time
 import signal
+import uuid
 
 
 from ci_utils import (
@@ -74,7 +74,7 @@
 
 
 def check_worker():
-    queue = 'testrun-test-%d' % time.time()
+    queue = 'testrun-test-{}'.format(uuid.uuid1())
     series = 'trusty'
     architecture = 'amd64'
     image_id = image_store.uci_image_name('cloudimg', series, architecture)

=== modified file 'test_runner/run-integration.py'
--- test_runner/run-integration.py	2014-10-10 13:14:33 +0000
+++ test_runner/run-integration.py	2014-10-23 22:13:26 +0000
@@ -16,7 +16,7 @@
 
 import json
 import logging
-import time
+import uuid
 
 
 from ci_utils import (
@@ -57,7 +57,7 @@
 
 
 def run_test():
-    queue = 'testrun-test-%d' % time.time()
+    queue = 'testrun-test-{}'.format(uuid.uuid1())
     series = 'trusty'
     architecture = 'amd64'
     image_id = image_store.uci_image_name('cloudimg', series, architecture)