From f074ef8181c2b93cbfcd870eb67c8c95cb0e442a Mon Sep 17 00:00:00 2001
From: Peter Amstutz <peter.amstutz@curoverse.com>
Date: Thu, 9 Mar 2017 17:41:55 -0500
Subject: [PATCH] 8567: Fix migrate links to use PDH instead of UUID.

Better error reporting.

Migrate script cleans up /var/lib/docker inside container.
---
 docker/migrate-docker19/migrate.sh        | 15 ++++---
 sdk/python/arvados/commands/keepdocker.py | 53 ++++++++++++++++-------
 2 files changed, 46 insertions(+), 22 deletions(-)

diff --git a/docker/migrate-docker19/migrate.sh b/docker/migrate-docker19/migrate.sh
index 63d7274360..58d6665f22 100755
--- a/docker/migrate-docker19/migrate.sh
+++ b/docker/migrate-docker19/migrate.sh
@@ -2,6 +2,14 @@
 
 set -e
 
+function cleanup {
+    kill $(cat /var/run/docker.pid)
+    sleep 1
+    rm -rf /var/lib/docker/*
+}
+
+trap cleanup EXIT
+
 /root/dnd.sh &
 sleep 2
 
@@ -28,11 +36,6 @@ sleep 2
 
 docker images -a
 
-UUID=$(arv-keepdocker --project-uuid=$project_uuid $image_repo $image_tag)
-
-kill $(cat /var/run/docker.pid)
-sleep 1
-
-chmod ugo+rwx -R /var/lib/docker
+UUID=$(arv-keepdocker --force-image-format --project-uuid=$project_uuid $image_repo $image_tag)
 
 echo "Migrated uuid is $UUID"
diff --git a/sdk/python/arvados/commands/keepdocker.py b/sdk/python/arvados/commands/keepdocker.py
index 569b3152e9..5603ade492 100644
--- a/sdk/python/arvados/commands/keepdocker.py
+++ b/sdk/python/arvados/commands/keepdocker.py
@@ -393,16 +393,26 @@ def migrate19():
     for m in migration_links:
         already_migrated.add(m["tail_uuid"])
 
-    need_migrate = [img for img in old_images if img["collection"] not in already_migrated]
+    items = arvados.util.list_all(api_client.collections().list,
+                                  filters=[["uuid", "in", [img["collection"] for img in old_images]]],
+                                  select=["uuid", "portable_data_hash"])
+    uuid_to_pdh = {i["uuid"]: i["portable_data_hash"] for i in items}
+    need_migrate = [img for img in old_images
+                    if uuid_to_pdh[img["collection"]] not in already_migrated]
 
     logger.info("Already migrated %i images", len(already_migrated))
     logger.info("Need to migrate %i images", len(need_migrate))
 
+    success = []
+    failures = []
     for old_image in need_migrate:
-        logger.info("Migrating %s", old_image["collection"])
+        if uuid_to_pdh[old_image["collection"]] in already_migrated:
+            continue
+
+        logger.info("Migrating %s:%s (%s)", old_image["repo"], old_image["tag"], old_image["collection"])
 
-        col = CollectionReader(old_image["collection"])
-        tarfile = col.keys()[0]
+        oldcol = CollectionReader(old_image["collection"])
+        tarfile = oldcol.keys()[0]
 
         try:
             varlibdocker = tempfile.mkdtemp()
@@ -424,26 +434,37 @@ def migrate19():
                              tarfile[0:40],
                              old_image["repo"],
                              old_image["tag"],
-                             col.api_response()["owner_uuid"]]
+                             oldcol.api_response()["owner_uuid"]]
 
                 out = subprocess.check_output(dockercmd)
 
-            new_collection = re.search(r"Migrated uuid is ([a-z0-9]{5}-[a-z0-9]{5}-[a-z0-9]{15})", out)
-            api_client.links().create(body={"link": {
-                'owner_uuid': col.api_response()["owner_uuid"],
-                'link_class': arvados.commands.keepdocker._migration_link_class,
-                'name': arvados.commands.keepdocker._migration_link_name,
-                'tail_uuid': old_image["collection"],
-                'head_uuid': new_collection.group(1)
-                }}).execute(num_retries=3)
-
-            logger.info("Migrated '%s' to '%s'", old_image["collection"], new_collection.group(1))
+            migrated = re.search(r"Migrated uuid is ([a-z0-9]{5}-[a-z0-9]{5}-[a-z0-9]{15})", out)
+            if migrated:
+                newcol = CollectionReader(migrated.group(1))
+
+                api_client.links().create(body={"link": {
+                    'owner_uuid': oldcol.api_response()["owner_uuid"],
+                    'link_class': arvados.commands.keepdocker._migration_link_class,
+                    'name': arvados.commands.keepdocker._migration_link_name,
+                    'tail_uuid': oldcol.portable_data_hash(),
+                    'head_uuid': newcol.portable_data_hash()
+                    }}).execute(num_retries=3)
+
+                logger.info("Migrated '%s' to '%s'", oldcol.portable_data_hash(), newcol.portable_data_hash())
+                already_migrated.add(oldcol.portable_data_hash())
+                success.append(old_image["collection"])
+            else:
+                logger.error("Error migrating '%s'", old_image["collection"])
+                failures.append(old_image["collection"])
         except Exception as e:
             logger.exception("Migration failed")
+            failures.append(old_image["collection"])
         finally:
             shutil.rmtree(varlibdocker)
 
-    logger.info("All done")
+    logger.info("Successfully migrated %i images", len(success))
+    if failures:
+        logger.error("Failure migrating images: %s", failures)
 
 
 def main(arguments=None, stdout=sys.stdout):
-- 
2.30.2