Pulp

This was worked around by changing "elif 'last_sync' in importer.keys():" to "elif importer.get('last_sync') is not None:" in 0025_importer_schema_change.py. This appears to be working fine in production, though testing was extra-medium at best

vitaminmoo wrote:

This was worked around by changing "elif 'last_sync' in importer.keys():" to "elif importer.get('last_sync') is not None:" in 0025_importer_schema_change.py. This appears to be working fine in production, though testing was extra-medium at best

This is a diff, of the change described above:

diff --git a/server/db/migrations/0025_importer_schema_change.py b/server/db/migrations/0025_importer_schema_change.py
index 3de6284..6157760 100644
--- a//server/db/migrations/0025_importer_schema_change.py
+++ b/server/db/migrations/0025_importer_schema_change.py
@@ -15,7 +15,7 @@ def migrate(*args, **kwargs):

         if updated_key in importer.keys():
             continue
-        elif 'last_sync' in importer.keys():
+        elif importer.get('last_sync') is not None:
             importer[updated_key] = dateutils.parse_iso8601_datetime(importer['last_sync'])
         else:
             importer[updated_key] = dateutils.now_utc_datetime_with_tzinfo()

Where this file lands varies from distribution to distribution. We also need more testing before calling this "the" fix for folks running 2.10.1.

Yes, the real fix should also put the parsing in a try...except block.

This patch should be more robust. I'm not able to test it at the moment, so I could use help with that.

diff --git a/server/pulp/server/db/migrations/0025_importer_schema_change.py b/server/pulp/server/db/migrations/0025_importer_schema_change.py
index 3de6284..836b25f 100644
--- a/server/pulp/server/db/migrations/0025_importer_schema_change.py
+++ b/server/pulp/server/db/migrations/0025_importer_schema_change.py
@@ -15,9 +15,11 @@ def migrate(*args, **kwargs):

         if updated_key in importer.keys():
             continue
-        elif 'last_sync' in importer.keys():
+
+        try:
             importer[updated_key] = dateutils.parse_iso8601_datetime(importer['last_sync'])
-        else:
+        # The attribute doesn't exist, or parsing failed. It's safe to set a newer timestamp.
+        except:
             importer[updated_key] = dateutils.now_utc_datetime_with_tzinfo()

         collection.save(importer)

i was looking into this a bit and there is no way the last_sync would be set to none and stored in DB unless there was a manual update with a query to the db. When a repo is created with an importer set the last_sync field is not stored in the db.
To confirm, try to create a repo and see its importer in the DB, you will the that the last_sync is not in the DB
So the conclusion is - if the last_sync is among the keys of the dict then it has some valid value and not null( this is after mongoengine conversion https://github.com/pulp/pulp/blob/2.8-dev/server/pulp/server/db/model/__init__.py#L237)
So i looked into code before the conversion and saw that we were setting None value explicitly( https://github.com/pulp/pulp/blob/2.7-dev/server/pulp/server/db/model/repository.py#L108) so in case user upgraded from 2.7 he would have issue with the migration running.

I can confirm that our install was upgraded numerous times.

The final version of the fix was very similar to the workaround you put in place, with a little bit more protection again failure.

I'm building a 2.10.2 hotfix release now, which should get anyone on 2.10.0 or 2.10.1 (broken or not) upgraded successfully.