Skip to content

feat: MTV migrate role - #88

Open
sabre1041 wants to merge 1 commit into
redhat-cop:v2from
sabre1041:mtv_migrate
Open

feat: MTV migrate role#88
sabre1041 wants to merge 1 commit into
redhat-cop:v2from
sabre1041:mtv_migrate

Conversation

@sabre1041

Copy link
Copy Markdown
Contributor

Description

Added mtv_migrate role that enables triggering a migration from an existing plan

Type of Change

  • feat: A new feature
  • fix: A bug fix
  • docs: Documentation changes
  • style: Formatting, missing semi colons, etc; no code change
  • refactor: Refactoring production code
  • test: Adding missing tests, refactoring tests; no production code change
  • chore: Updating configs, etc; no production code change

Signed-off-by: Andrew Block <andy.block@gmail.com>

@tech2734 tech2734 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff — the custom module with unit tests and the recursive polling pattern are solid. A few things I noticed:

  1. meta/main.yml missing role_name: Should add role_name: mtv_migrate to galaxy_info for consistency with the other roles.

  2. Playbook host filtering: vmf_migrate.yml targets migration_clusters group but doesn't have a guard to only execute on the relevant host. Might need a when condition or similar pattern to prevent running on all hosts in the group.

  3. Missing OCP connection params on Create Migration task: The redhat.openshift.k8s call in main.yml doesn't pass host, api_key, validate_certs, or ca_cert — but the polling tasks in _poll_migration_status.yml do. Probably works via K8S_AUTH_* env vars in AAP, but it's inconsistent.

  4. README boilerplate: The static section still has the generic Ansible Galaxy scaffolding ("pre-requisites that may not be covered", example string_arg1 argument spec, etc.). The docsible section below is fine — just the static part could use a cleanup pass.

  5. _mtv_migrate_mtv_cutover referenced but not set: In migration.yml.j2, the template checks _mtv_migrate_mtv_cutover but I don't see a set_fact that populates it. mtv_migrate_warm_cutover_time is the user-facing default — looks like either a missing fact or a variable name mismatch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants