Skip to content

fix(v2): standardise managed-by label across roles - #82

Open
tech2734 wants to merge 7 commits into
redhat-cop:v2from
tech2734:fix/managed-by-label
Open

fix(v2): standardise managed-by label across roles#82
tech2734 wants to merge 7 commits into
redhat-cop:v2from
tech2734:fix/managed-by-label

Conversation

@tech2734

@tech2734 tech2734 commented Sep 1, 2026

Copy link
Copy Markdown

Summary

  • Replace hardcoded app.kubernetes.io/managed-by values with configurable role variables defaulting to ansible-migration-factory
  • Align utility_openshift_target_credential default from "ansible" to "ansible-migration-factory" for consistency across all roles
  • Add docsible # title: / # required: / # description: headings to aap_seed and mtv_provider defaults, completing standardisation across all v2 roles
  • Regenerate READMEs via docsible for aap_seed, mtv_provider, and utility_openshift_target_credential
  • Add .docsible to .gitignore

Managed-by label changes

Role Change
mtv_provider Added mtv_provider_managed_by_label variable; templates now reference it instead of hardcoding
utility_openshift_target_credential Changed default of utility_openshift_target_credential_managed_by_label from "ansible" to "ansible-migration-factory"

Docsible heading standardisation

All v2 roles now have docsible headings on every variable in defaults/main.yml:

Role Variables
aap_seed 41 (added in this PR)
mtv_provider 14 (added in this PR)
mtv_query_inventory 15 (already done)
utility_aap_deploy 28 (already done)
utility_openshift_target_credential 10 (already done)

Note

The mtv_maps role (PR #80) also hardcodes the managed-by value and will need the same treatment once merged. The mtv_plans role (PR #81) is missing the label entirely.

Test plan

  • Verify mtv_provider templates render with the correct label
  • Verify utility_openshift_target_credential resources get the updated label
  • Confirm label is overridable via inventory/playbook vars
  • Verify docsible-generated READMEs render correctly

Replace hardcoded app.kubernetes.io/managed-by values with a
configurable role variable defaulting to ansible-migration-factory.

- mtv_provider: add mtv_provider_managed_by_label variable, use it
  in provider.yml.j2 and provider_secret.yml.j2 templates
- utility_openshift_target_credential: change default from "ansible"
  to "ansible-migration-factory" for consistency

Co-authored-by: Cursor <cursoragent@cursor.com>
@tech2734
tech2734 requested a review from sabre1041 as a code owner September 1, 2026 14:11
@tech2734
tech2734 deployed to external-ci September 1, 2026 14:12 — with GitHub Actions Active
tech2734 added a commit to tech2734/openshift_virtualization_migration that referenced this pull request Sep 1, 2026
Replace hardcoded app.kubernetes.io/managed-by value with
mtv_maps_managed_by_label variable, consistent with the approach
in mtv_provider (PR redhat-cop#82).

Co-authored-by: Cursor <cursoragent@cursor.com>

@sabre1041 sabre1041 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @tech2734 Can you update the README documentation. Might this be an opportunity to align with docsible based comments in the defaults ?

Update docsible-generated README to reflect the managed-by label
default change from "ansible" to "ansible-migration-factory".

Co-authored-by: Cursor <cursoragent@cursor.com>
@tech2734
tech2734 deployed to external-ci September 2, 2026 16:48 — with GitHub Actions Active
tech2734 and others added 4 commits September 2, 2026 11:52
Add title, required, and description comment blocks above each
variable in defaults/main.yml for both roles. Regenerate READMEs
via docsible. Add .docsible to .gitignore.

All roles now have standardised docsible headings:
- aap_seed (41 variables)
- mtv_provider (14 variables)
- mtv_query_inventory (already done)
- utility_aap_deploy (already done)
- utility_openshift_target_credential (already done)

Co-authored-by: Cursor <cursoragent@cursor.com>
Re-run update-documentation.sh to regenerate READMEs with the
same custom template used by CI, fixing pre-commit check failures
caused by docsible version/template mismatch.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
READMEs generated by docsible start with H2 (## role_name) which
violates MD041 (first-line-heading). Add a STATIC CONTENT block
with an H1 header to aap_seed, mtv_provider, and
utility_openshift_target_credential, matching the pattern used
by mtv_query_inventory.

Co-authored-by: Cursor <cursoragent@cursor.com>
@tech2734
tech2734 requested a review from sabre1041 September 2, 2026 17:08
@sabre1041

Copy link
Copy Markdown
Contributor

@tech2734 a few concurrent enhancements have been integrated. Would you be able to resolve the merge conflicts and we can look at getting this feature integrated?

@tech2734
tech2734 deployed to external-ci September 2, 2026 17:32 — with GitHub Actions Active
Resolve conflicts in aap_seed/defaults/main.yml by merging
upstream's new EE variables (PR redhat-cop#76) with our docsible headings.
Accept upstream's CHANGELOG.rst and aap_seed README, then
regenerate README via docsible to reflect the merged defaults.

Co-authored-by: Cursor <cursoragent@cursor.com>
@tech2734
tech2734 deployed to external-ci September 2, 2026 18:43 — with GitHub Actions Active
@tech2734

tech2734 commented Sep 2, 2026

Copy link
Copy Markdown
Author

@tech2734 a few concurrent enhancements have been integrated. Would you be able to resolve the merge conflicts and we can look at getting this feature integrated?

should be good now

tech2734 added a commit to tech2734/openshift_virtualization_migration that referenced this pull request Sep 2, 2026
Replace hardcoded app.kubernetes.io/managed-by value with
mtv_maps_managed_by_label variable, consistent with the approach
in mtv_provider (PR redhat-cop#82).

Co-authored-by: Cursor <cursoragent@cursor.com>
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