Issue #210: Waiverdb 0.11 database migration fails on unrecognised subject types - waiverdb

waiverdb

#210 Waiverdb 0.11 database migration fails on unrecognised subject types

Closed: Fixed 6 years ago Opened 6 years ago by dcallagh.

I tried to roll out Waiverdb 0.8 to Fedora production today, and it failed in the database migration:

2018-07-04 05:14:38 [pid     1] alembic.env INFO Connecting...
2018-07-04 05:14:38 [pid     1] alembic.runtime.migration INFO Context impl PostgresqlImpl.
2018-07-04 05:14:38 [pid     1] alembic.runtime.migration INFO Will assume transactional DDL.
2018-07-04 05:14:38 [pid     1] alembic.env INFO Beginning outermost transaction.
2018-07-04 05:14:38 [pid     1] alembic.env INFO Starting migrations.
2018-07-04 05:14:38 [pid     1] alembic.runtime.migration INFO Running upgrade ce8a1351ecdc -> f6bc296ba966, Replace waiver.subject dict with subject_type and subject_identifier
2018-07-04 05:14:38 [pid     1] alembic.env INFO Closing connection.
Traceback (most recent call last):
[...]
  File "/usr/lib/python3.6/site-packages/waiverdb/migrations/versions/f6bc296ba966_subject_dict_to_type_identifier.py", line 38, in upgrade
    subject_type, subject_identifier = subject_dict_to_type_identifier(subject)
  File "/usr/lib/python3.6/site-packages/waiverdb/models/waivers.py", line 23, in subject_dict_to_type_identifier
    raise ValueError('Unrecognised subject type: %r' % subject)
ValueError: Unrecognised subject type: {'item': 'kernel-4.16.7-200.fc27', 'type': 'test-result-missing'}

We saw this in Fedora stage and internal stage too. In those cases the data was bad from people just inserting random garbage to test. So in those cases, we just hacked up the database to fix the bad values.

However in a production environment we should not just alter rows that are supposed to be immutable (even if the data itself is invalid). The migration should tolerate this situation in a way that does not destroy data.

dcallagh commented 6 years ago

Oops... I said 0.8 but I meant 0.11. The latest release.

lholecek commented 6 years ago

I can reproduce this locally with following commands.

dropdb waiverdb
createdb waiverdb
curl -L https://infrastructure.fedoraproject.org/infra/db-dumps/waiverdb.dump.xz | xz -dc | psql waiverdb
DEV=true PYTHONPATH=. python3 waiverdb/manage.py db upgrade

dcallagh commented 6 years ago

So I see in the original migration I wrote, it already keeps the old subject column with the existing data, in case it is needed for downgrade. So that is good.

I think all we need to do is have a sensible way to map invalid subject values to something which fits into the new schema. Here are the problematic rows (seems jcline was having trouble understanding the waiverdb docs until threebean sorted it out for him):

 142 |           | jcline         | fedora-27       | t      | Test gating is broken                                                | 2018-05-09 18:32:06.742322 |            | {"item":"kernel-4.16.7-200.fc27","type":"test-result-missing"}                       | org.centos.prod.ci.pipeline.complete
 143 |           | jcline         | fedora-27       | t      | I don't even know what's wrong anymore                               | 2018-05-09 18:36:04.980667 |            | {"item":{"original_spec_nvr":"kernel-4.16.7-200.fc27"},"type":"test-result-missing"} | org.centos.prod.ci.pipeline.complete
 144 |           | jcline         | fedora-27       | t      | I don't even know what's wrong anymore                               | 2018-05-09 18:36:50.575931 |            | {"item":{"original_spec_nvr":"kernel-4.16.7-200.fc27"},"type":"koji_build"}          | org.centos.prod.ci.pipeline.complete
 145 |           | jcline         | fedora-27       | t      | I don't even know what's wrong anymore                               | 2018-05-09 19:00:15.188881 |            | {"item":{"original_spec_nvr":"kernel-4.16.7-200.fc27"}}                              | org.centos.prod.ci.pipeline.complete

Since the invalid values will never actually match anything that Greenwave is looking for, perhaps it would be okay to just map them to subject_type of koji_build and subject_identifier of empty string.