diff --git a/renku/core/migration/migrate.py b/renku/core/migration/migrate.py index 233cf71304..661a5f375d 100644 --- 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 index f94a2f4cbe..d4fa18333e 100644 --- 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 index 50e168301f..b8ec000162 100644 --- 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 = ( diff --git a/tests/cli/test_migrate.py b/tests/cli/test_migrate.py index 4defed20a5..7c65d1b1be 100644 --- a/tests/cli/test_migrate.py +++ b/tests/cli/test_migrate.py @@ -53,6 +53,27 @@ def test_migrate_project(isolated_runner, old_project, with_injection): assert project_context.project.name +@pytest.mark.migration +@pytest.mark.parametrize( + "old_project", + [ + "v9-migration-docker-version-change.git", + "v9-migration-docker-mult-changes.git", + ], + indirect=["old_project"], +) +def test_migrate_old_project_with_docker_change(isolated_runner, old_project, with_injection): + """Test migrating projects with changes to the Dockerfile.""" + result = isolated_runner.invoke(cli, ["migrate", "--strict"]) + assert 0 == result.exit_code, format_result_exception(result) + assert not old_project.repository.is_dirty() + assert "Updated dockerfile" in result.output + + with project_context.with_path(old_project.path), with_injection(): + assert project_context.project + assert project_context.project.name + + @pytest.mark.migration @pytest.mark.serial def test_migration_check(isolated_runner, project): diff --git a/tests/data/v9-migration-docker-mult-changes.git.tar.gz b/tests/data/v9-migration-docker-mult-changes.git.tar.gz new file mode 100644 index 0000000000..5a70489756 Binary files /dev/null and b/tests/data/v9-migration-docker-mult-changes.git.tar.gz differ diff --git a/tests/data/v9-migration-docker-version-change.git.tar.gz b/tests/data/v9-migration-docker-version-change.git.tar.gz new file mode 100644 index 0000000000..5ee43c4f97 Binary files /dev/null and b/tests/data/v9-migration-docker-version-change.git.tar.gz differ diff --git a/tests/service/fixtures/service_integration.py b/tests/service/fixtures/service_integration.py index 33bcab7edd..b2f3ad46f8 100644 --- a/tests/service/fixtures/service_integration.py +++ b/tests/service/fixtures/service_integration.py @@ -25,7 +25,7 @@ from renku.core import errors from renku.infrastructure.repository import Repository -from tests.utils import format_result_exception, modified_environ +from tests.utils import clone_compressed_repository, format_result_exception, modified_environ @contextlib.contextmanager @@ -272,6 +272,57 @@ def _parse(href): pass +@pytest.fixture +def old_local_remote_project(request, svc_client, tmp_path, mock_redis, identity_headers, real_sync): + """Fixture for testing service with project tarfiles containing old projects.""" + from renku.domain_model import git + from renku.ui.service.cache import cache as redis_cache + from renku.ui.service.gateways.repository_cache import LocalRepositoryCache + from renku.ui.service.serializers.headers import RequiredIdentityHeaders + + name = request.param + remote_repo_path = tmp_path / name + remote_repo = clone_compressed_repository(base_path=tmp_path, name=name) + remote_repo_path = remote_repo_path / "repository" + remote_repo_checkout_path = tmp_path / "remote_repo_checkout" + remote_repo_checkout_path.mkdir() + + remote_repo_checkout = Repository.clone_from(url=remote_repo_path, path=remote_repo_checkout_path) + + # NOTE: Mock GitURL parsing for local URL + def _parse(href): + return git.GitURL(href=href, regex="", owner="dummy", name="project", slug="project", path=remote_repo_path) + + original_giturl_parse = git.GitURL.parse + git.GitURL.parse = _parse + + home = tmp_path / "user_home" + home.mkdir() + + user_data = RequiredIdentityHeaders().load(identity_headers) + user = redis_cache.ensure_user(user_data) + remote_url = f"file://{remote_repo_path}" + + project = LocalRepositoryCache().get(redis_cache, remote_url, branch=None, user=user, shallow=False) + + project_id = project.project_id + + try: + yield svc_client, identity_headers, project_id, remote_repo, remote_repo_checkout, remote_url + finally: + git.GitURL.parse = original_giturl_parse + + try: + shutil.rmtree(remote_repo_path) + except OSError: + pass + + try: + shutil.rmtree(remote_repo_checkout_path) + except OSError: + pass + + @pytest.fixture def quick_cache_synchronization(mocker): """Forces cache to synchronize on every request.""" diff --git a/tests/service/views/test_cache_views.py b/tests/service/views/test_cache_views.py index 2eb6430ec2..4f3d84ab4e 100644 --- a/tests/service/views/test_cache_views.py +++ b/tests/service/views/test_cache_views.py @@ -704,6 +704,25 @@ def test_execute_migrations(svc_client_setup): assert not response.json["result"]["errors"] +@pytest.mark.service +@pytest.mark.parametrize( + "old_local_remote_project", + [ + "v9-migration-docker-version-change.git", + "v9-migration-docker-mult-changes.git", + ], + indirect=["old_local_remote_project"], +) +def test_migrate_old_project(old_local_remote_project): + """Test migrating old projects with docker file changes.""" + svc_client, identity_headers, project_id, remote_repo, remote_repo_checkout, remote_url = old_local_remote_project + + response = svc_client.post("/cache.migrate", data=json.dumps(dict(git_url=remote_url)), headers=identity_headers) + assert response + assert 200 == response.status_code + assert response.json.get("result", {}).get("docker_migrated", False) + + @pytest.mark.service @pytest.mark.integration def test_execute_migrations_job(svc_client_setup):