#207 mash directive crashes if koji directive downloaded no RPMs
Closed: Fixed Opened by kparal.

Here's the error:
http://taskotron-dev.fedoraproject.org/taskmaster/builders/x86_64/builds/35104/steps/runtask/logs/stdio

runtask -i f20-updates-testing-pending -t koji_tag -a x86_64 -j x86_64/35104 depcheck.yml
 in dir /home/buildslave/slave/x86_64/build (timeout 1200 secs)
 watching logfiles {'taskotron.log': {'follow': True, 'filename': '/var/log/taskotron/taskotron.log'}}
 argv: ['runtask', '-i', 'f20-updates-testing-pending', '-t', 'koji_tag', '-a', 'x86_64', '-j', 'x86_64/35104', 'depcheck.yml']
 environment:
  HOME=/home/buildslave
  LANG=en_US.UTF-8
  LOGNAME=buildslave
  PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin
  PWD=/home/buildslave/slave/x86_64/build
  SHELL=/bin/bash
  USER=buildslave
 using PTY: False
[libtaskotron:runner.py:228] 2014-10-09 18:48:39 DEBUG   Parsed arguments: Namespace(arch=['x86_64'], debug=False, item='f20-updates-testing-pending', jobid='x86_64/35104', override=[], task=['depcheck.yml'], type='koji_tag')
[libtaskotron:config.py:93] 2014-10-09 18:48:39 DEBUG   Using config file: /etc/taskotron/taskotron.yaml
[libtaskotron:config.py:64] 2014-10-09 18:48:39 DEBUG   Using config profile: production
[libtaskotron:koji_utils.py:212] 2014-10-09 18:48:40 INFO    Fetching 1 builds for tag: f20-updates-testing-pending
[libtaskotron:koji_utils.py:129] 2014-10-09 18:48:40 INFO    Querying Koji for a list of RPMS for: docker-io-1.2.0-5.fc20
[libtaskotron:koji_utils.py:188] 2014-10-09 18:48:40 INFO    Fetching 0 RPMs for: docker-io-1.2.0-5.fc20 (into /var/tmp/taskotron/task-J6PdhG/downloaded_tag/)
[libtaskotron:mash_directive.py:134] 2014-10-09 18:48:41 INFO    running createrepo on /var/tmp/taskotron/task-J6PdhG/downloaded_tag/
[libtaskotron:logger.py:70] 2014-10-09 18:48:41 CRITICAL Traceback (most recent call last):
  File "/usr/bin/runtask", line 9, in <module>
    load_entry_point('libtaskotron==0.3.7', 'console_scripts', 'runtask')()
  File "/usr/lib/python2.7/site-packages/libtaskotron/runner.py", line 244, in main
    task_runner.run()
  File "/usr/lib/python2.7/site-packages/libtaskotron/runner.py", line 50, in run
    self.do_actions()
  File "/usr/lib/python2.7/site-packages/libtaskotron/runner.py", line 134, in do_actions
    self.do_single_action(action)
  File "/usr/lib/python2.7/site-packages/libtaskotron/runner.py", line 112, in do_single_action
    self.envdata)
  File "/usr/lib/python2.7/site-packages/libtaskotron/directives/mash_directive.py", line 179, in process
    return self.do_mash(rpmdir, dodelta, arch, outdir)
  File "/usr/lib/python2.7/site-packages/libtaskotron/directives/mash_directive.py", line 147, in do_mash
    raise TaskotronDirectiveError(errors)
TaskotronDirectiveError: Directory /var/tmp/taskotron/task-J6PdhG/downloaded_tag/ must exist

It might sometimes happen that the only waiting build is a x86_64-only build, like this one:
http://koji.fedoraproject.org/koji/buildinfo?buildID=583840
Depcheck is "noarch", it checks all arches in a single run. But it does it in sequence, first i386, then x86_64. In this case, there were no RPMs to download for i386, therefore the directory was not created, and mash crashed on missing dir.

I see several approaches how to fix this:
a) let koji directive always create the target dir, and make sure mash behaves correctly (performs nothing, or maybe creates valid but empty repodata?) on empty dirs
b) make mash ignore non-existing input dirs and print "nothing to do" instead. however, this might hide legitimate errors.
c) something else? All other solutions I thought of were either clunky or doing radical changes to directive/task recipe design.

After you fix this, verify whether depcheck works OK or not with an empty input dir / empty repodata. If it doesn't, file a new bug against it.


This ticket had assigned some Differential requests:
D303
D267

I can see the solution going both as either A or B, and I'd be more inclined to the A option - our tools should IMHO be able to deal with "empty inputs", but it still needs to be a correct input - i.e. an empty dir is a correct input, that should be handled, but non-existent input dir is an error.

I'll check on what does mash do with empty directory, and how will the subsequent depcheck deal with it.

@jskladan, D267 doesn't seem to have fixed it:
http://taskotron-dev.fedoraproject.org/taskmaster/builders/x86_64/builds/73376/steps/runtask/logs/stdio

libtaskotron 0.3.11 should contain the fix. Can you look at it? Thanks.

Could not really reproduce, but changed the code so the file_utils.createdirs() is called every time.
Note that the method should not raise an exception, if the path provided is an already existing directory.

This can be reproduced if you adjust get_tagged_rpms() and add:

nvrs = ['docker-io-1.5.0-1.fc20']

into the middle.

I have a patch which fixes the issue here:
https://phab.qadevel.cloud.fedoraproject.org/differential/diff/827/

The only thing that was needed was to change rpms.append() to rpms.extend(), because otherwise the return value was [[]] instead of [].

I have decided to change a few more things, mainly how we create the destination dirs. We should create them not only in get_tagged_rpms(), but also in get_nvr_rpms(). Furthermore, I decided it's silly to wrap them into TaskotronRemoteError in an error occurs and that it's just easier to raise OSError for those cases which have nothing to do with our library, but with OS-related issue (wrong permissions, insufficient disk space, etc). I adjusted docstrings to reflect that.

Lastly, I placed some log messages directly into file_utils.makedirs() and that allowed me to simplify file_utils.download() considerably in the same spirit as the aforementioned koji_utils changes.

It's probably worth it to split this into 2 separate commits. I attached the patch just to hear your opinions.

One thing that is missing from the patch is a unit test for checking the return value of get_tagged_rpms() in case no NVRs are downloaded. @jskladan, do you want to write it or should I?

@kparal, he who wrote the patch shall unit test it :)

Hopefully resolved in D303.

Metadata Update from @kparal:
- Issue tagged with: easyfix

Metadata