diff --git a/changelog.d/183-migrate-observations-missing.md b/changelog.d/183-migrate-observations-missing.md new file mode 100644 index 0000000..3ca03aa --- /dev/null +++ b/changelog.d/183-migrate-observations-missing.md @@ -0,0 +1,3 @@ +### Fixed + +- `cloudfall migrate --yes` no longer crashes with `HANDLER_CRASHED` when it reaches its `audit` or `verify-routes` step on a project with no server observations: the step fails with `migrate_observations_missing`, naming the missing directory and `cloudfall observe`, and a rerun after `observe` resumes at that step (#183) diff --git a/sdk/src/cloudfall/migrate.py b/sdk/src/cloudfall/migrate.py index dda44c6..fe8a750 100644 --- a/sdk/src/cloudfall/migrate.py +++ b/sdk/src/cloudfall/migrate.py @@ -80,6 +80,7 @@ _PAUSE_CODES = frozenset({_ERROR_DNS_UNVERIFIED, _ERROR_TTL_HIGH}) _ERROR_AUDIT_DRIFT = "migrate_audit_drift" _ERROR_ROUTES_UNHEALTHY = "migrate_routes_unhealthy" +_ERROR_OBSERVATIONS_MISSING = "migrate_observations_missing" _ERROR_DATABASE_UNKNOWN = "migrate_database_unknown" _ERROR_DATA_SCHEMA_WITHOUT_DATA = "migrate_data_schema_without_data" @@ -729,7 +730,23 @@ def _resolve_names(hostname: str) -> set[str]: return {str(record[4][0]) for record in records} +def _require_observations(config: AgentConfig) -> None: + """Refuse a verify step whose server snapshots were never recorded. + + Only the missing directory: a malformed snapshot still fails validation, + and a server without a snapshot is drift the audit reports. + """ + if not config.observed_directory.is_dir(): + detail = ( + "no server observations at " + f"{config.observed_directory}; record them with " + "`cloudfall observe`, then rerun the migration" + ) + raise MigrateError(_ERROR_OBSERVATIONS_MISSING, detail) + + def _verify_audit(config: AgentConfig) -> dict[str, object]: + _require_observations(config) state = validate_config(config.project_directory, config.schema_directory) observations = load_observations( config.observed_directory, config.schema_directory @@ -746,6 +763,8 @@ def _verify_audit(config: AgentConfig) -> dict[str, object]: def _verify_routes(config: AgentConfig) -> dict[str, object]: + # Before any domain is probed: the view needs the server snapshots too. + _require_observations(config) state = validate_config(config.project_directory, config.schema_directory) inventory = PlatformInventory.from_state(state) inspect_domains( diff --git a/sdk/tests/test_migrate.py b/sdk/tests/test_migrate.py index 354d08b..fb59c1a 100644 --- a/sdk/tests/test_migrate.py +++ b/sdk/tests/test_migrate.py @@ -325,6 +325,59 @@ def test_s007_12_cli_migrate_refuses_a_bad_data_schema_before_any_playbook( assert not (tmp_path / "inventory.json").exists() +@pytest.mark.parametrize("step_id", ["audit", "verify-routes"]) +def test_cli_migrate_names_missing_observations_at_the_verify_step( + step_id: str, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + """A verify step without server snapshots fails typed, not crashed (#183).""" + options = _options(tmp_path, execute=True) + # Every step before this one completes; the CLI run resumes here. + stop = MigrateError("migrate_step_failed", "stopped for the test") + execute_migration( + _config(tmp_path), + options, + runners=_runners([], {step_id: stop}), + ) + observed = tmp_path / "observed" + + exit_code = main( + [ + "migrate", + "--project", + str(EXAMPLES), + "--schemas", + str(SCHEMAS), + "--engine", + str(ENGINE), + "--plan-file", + str(options.plan_file), + "--observed", + str(observed), + "--service-observed", + str(tmp_path / "observed-services"), + "--build", + "crm-backend=main", + "--yes", + ] + ) + + payload = json.loads(capsys.readouterr().out) + assert payload["error"]["code"] != "HANDLER_CRASHED" + assert payload["error"]["code"] == "ENGINE_STEP_FAILED" + assert exit_code == 87 + context = payload["error"]["context"] + assert context["step"] == step_id + assert context["code"] == "migrate_observations_missing" + assert str(observed) in context["message"] + assert "cloudfall observe" in context["message"] + assert payload["data"]["next"] == step_id + plan = json.loads(options.plan_file.read_text(encoding="utf-8")) + pending = [step["id"] for step in plan["steps"] if step["status"] == "pending"] + assert pending[0] == step_id + + # dns-verify's proxy addresses, pinned as they were before domains apply. diff --git a/sdk/tests/test_owned_postgresql_converge.py b/sdk/tests/test_owned_postgresql_converge.py index 3c5e6b7..b4eb68f 100644 --- a/sdk/tests/test_owned_postgresql_converge.py +++ b/sdk/tests/test_owned_postgresql_converge.py @@ -48,7 +48,6 @@ SopsSecretProvider, ) from cloudfall.validation import ( - ConfigValidationError, SchemaCatalog, validate_config, ) @@ -436,12 +435,16 @@ def test_s006_6_migrate_records_the_hash_in_its_services_step(tmp_path: Path) -> secrets = _secrets(project, f"BILLING_PASSWORD={PASSWORD}\n") recorder = Recorder() - # Every playbook "passes"; the run stops at the audit, which reads - # snapshots no stand-in wrote, after the services step was saved. - with pytest.raises(ConfigValidationError, match="observation directory"): - execute_migration( - _agent_config(project, recorder, secrets), _migrate_options(project) - ) + # Every playbook "passes"; the run stops at the audit, which finds no + # snapshots (no stand-in wrote any), after the services step was saved. + result = execute_migration( + _agent_config(project, recorder, secrets), _migrate_options(project) + ) + + assert result["status"] == "error" + assert result["step"] == "audit" + error = cast("dict[str, object]", result["error"]) + assert error["code"] == "migrate_observations_missing" plan = (project / "tmp" / "plan.json").read_text(encoding="utf-8") services = next(