fix(core): migrate Dockerfile after metadata

SwissDataScienceCenter · Jan 19, 2024 · 2277181 · 2277181
1 parent ae3618c
commit 2277181
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 33 deletions.
diff --git a/renku/core/migration/migrate.py b/renku/core/migration/migrate.py
@@ -123,6 +123,42 @@ def migrate_project(
     if not is_renku_project():
         return False, template_updated, docker_updated
 
+    n_migrations_executed = 0
+
+    if not skip_migrations:
+        project_version = project_version or get_project_version()
+
+        migration_context = MigrationContext(
+            strict=strict, type=migration_type, preserve_identifiers=preserve_identifiers
+        )
+
+        version = 1
+        for version, path in get_migrations():
+            if max_version and version > max_version:
+                break
+            if version > project_version:
+                module = importlib.import_module(path)
+                module_name = module.__name__.split(".")[-1]
+                communication.echo(f"Applying migration {module_name}...")
+                try:
+                    module.migrate(migration_context)
+                except (Exception, BaseException) as e:
+                    raise MigrationError("Couldn't execute migration") from e
+                n_migrations_executed += 1
+
+        if not is_using_temporary_datasets_path():
+            if n_migrations_executed > 0:
+                project_context.project.version = str(version)
+                project_gateway.update_project(project_context.project)
+
+                communication.echo(f"Successfully applied {n_migrations_executed} migrations.")
+
+            _remove_untracked_renku_files(metadata_path=project_context.metadata_path)
+
+        # we might not have been able to tell if a docker update is possible due to outstanding migrations.
+        # so we need to check again here.
+        skip_docker_update |= not is_docker_update_possible()
+
     try:
         project = project_context.project
     except ValueError:
@@ -155,36 +191,6 @@ def migrate_project(
         except Exception as e:
             raise DockerfileUpdateError("Couldn't update renku version in Dockerfile.") from e
 
-    if skip_migrations:
-        return False, template_updated, docker_updated
-
-    project_version = project_version or get_project_version()
-    n_migrations_executed = 0
-
-    migration_context = MigrationContext(strict=strict, type=migration_type, preserve_identifiers=preserve_identifiers)
-
-    version = 1
-    for version, path in get_migrations():
-        if max_version and version > max_version:
-            break
-        if version > project_version:
-            module = importlib.import_module(path)
-            module_name = module.__name__.split(".")[-1]
-            communication.echo(f"Applying migration {module_name}...")
-            try:
-                module.migrate(migration_context)
-            except (Exception, BaseException) as e:
-                raise MigrationError("Couldn't execute migration") from e
-            n_migrations_executed += 1
-    if not is_using_temporary_datasets_path():
-        if n_migrations_executed > 0:
-            project_context.project.version = str(version)
-            project_gateway.update_project(project_context.project)
-
-            communication.echo(f"Successfully applied {n_migrations_executed} migrations.")
-
-        _remove_untracked_renku_files(metadata_path=project_context.metadata_path)
-
     return n_migrations_executed != 0, template_updated, docker_updated
 
 

diff --git a/renku/ui/cli/migrate.py b/renku/ui/cli/migrate.py
@@ -98,6 +98,7 @@ def migrate(check, skip_template_update, skip_docker_update, strict, preserve_id
 
     template_update_possible = status & TEMPLATE_UPDATE_POSSIBLE and status & AUTOMATED_TEMPLATE_UPDATE_SUPPORTED
     docker_update_possible = status & DOCKERFILE_UPDATE_POSSIBLE
+    migration_required = status & MIGRATION_REQUIRED
 
     if check:
         if template_update_possible:
@@ -113,7 +114,7 @@ def migrate(check, skip_template_update, skip_docker_update, strict, preserve_id
                 + "using 'renku migrate'."
             )
 
-        if status & MIGRATION_REQUIRED:
+        if migration_required:
             raise MigrationRequired
 
     if status & UNSUPPORTED_PROJECT:
@@ -125,7 +126,9 @@ def migrate(check, skip_template_update, skip_docker_update, strict, preserve_id
     if check:
         return
 
-    skip_docker_update = skip_docker_update or not docker_update_possible
+    # In case where a migration is required, we can't tell if a dockerupdate is possible, as the metadata for deciding
+    # might not be there yet. We'll check this again after the migration
+    skip_docker_update = skip_docker_update or (not migration_required and not docker_update_possible)
     skip_template_update = skip_template_update or not template_update_possible
 
     communicator = ClickCallback()

diff --git a/renku/ui/service/controllers/cache_migrate_project.py b/renku/ui/service/controllers/cache_migrate_project.py
@@ -33,6 +33,7 @@ def execute_migration(
     from renku.command.migrate import (
         AUTOMATED_TEMPLATE_UPDATE_SUPPORTED,
         DOCKERFILE_UPDATE_POSSIBLE,
+        MIGRATION_REQUIRED,
         TEMPLATE_UPDATE_POSSIBLE,
         check_project,
         migrate_project_command,
@@ -47,8 +48,9 @@ def execute_migration(
 
         template_update_possible = status & TEMPLATE_UPDATE_POSSIBLE and status & AUTOMATED_TEMPLATE_UPDATE_SUPPORTED
         docker_update_possible = status & DOCKERFILE_UPDATE_POSSIBLE
+        migration_required = status & MIGRATION_REQUIRED
 
-        skip_docker_update = skip_docker_update or not docker_update_possible
+        skip_docker_update = skip_docker_update or (not migration_required and not docker_update_possible)
         skip_template_update = skip_template_update or not template_update_possible
 
         result = (