[test_operator] Automatic sync of README.md #3589

sauragar · 2026-01-07T10:09:55Z

This commit will ensure to have the proper auto-sync of README.md with the defaults/main.yml file and also added the post-commit file so whenever someone commits in the defaults/main.yml file the script can be auto-run

Ticket: OSPRH-19423

This PR #3327 is closed since I am not able to reopen the PR

Signing off: Saurabh Agarwal sauragar@redhat.com

openshift-ci · 2026-01-07T10:10:03Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign rebtoor for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kstrenkova

Thank you for the PR! I have tested it locally and found a small issue with updating the defaults. It is explained in the comments, the fix should be rather simple. Other than that I saw the README sync was working nicely, so I think you're close to the finish 🎉 I like that the parameters will be in alphabetical order now :D Maybe there could be a mention of the script somewhere in the README so that people know they can use the sync when creating new variables or updating the old ones.

roles/test_operator/README.md

scripts/sync_test_operator_var_readme.py

softwarefactory-project-zuul · 2026-01-08T11:07:26Z

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/4de28a813ea54e73883c28ab7ff35c81

✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 18m 29s
✔️ podified-multinode-edpm-deployment-crc SUCCESS in 1h 20m 49s
✔️ cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 41m 18s
✔️ cifmw-crc-podified-edpm-baremetal-minor-update SUCCESS in 2h 04m 04s
✔️ cifmw-multinode-tempest SUCCESS in 1h 45m 54s
✔️ cifmw-pod-zuul-files SUCCESS in 4m 23s
✔️ noop SUCCESS in 0s
✔️ cifmw-pod-ansible-test SUCCESS in 8m 48s
❌ cifmw-pod-pre-commit FAILURE in 7m 42s
✔️ cifmw-molecule-test_operator SUCCESS in 3m 29s

kstrenkova

What is left are just a few transformations from defaults to to Default value: where it is possible. Other than that lgtm.

roles/test_operator/README.md

kstrenkova

I am adding 2 comments about the same issue atop of my previous comments.

roles/test_operator/README.md

softwarefactory-project-zuul · 2026-01-20T14:52:02Z

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/fe0fdd8386324989bd8ea335a95f52d6

❌ openstack-k8s-operators-content-provider FAILURE in 4m 53s
⚠️ podified-multinode-edpm-deployment-crc SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ cifmw-crc-podified-edpm-baremetal SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ cifmw-crc-podified-edpm-baremetal-minor-update SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ cifmw-multinode-tempest SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
✔️ cifmw-pod-zuul-files SUCCESS in 4m 32s
✔️ noop SUCCESS in 0s
✔️ cifmw-pod-ansible-test SUCCESS in 12m 50s
❌ cifmw-pod-pre-commit FAILURE in 8m 07s
✔️ cifmw-molecule-test_operator SUCCESS in 3m 03s

softwarefactory-project-zuul · 2026-01-20T17:59:30Z

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/5eac463796534f4b89b1f990b236fe22

✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 29m 11s
✔️ podified-multinode-edpm-deployment-crc SUCCESS in 1h 28m 37s
✔️ cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 39m 55s
✔️ cifmw-crc-podified-edpm-baremetal-minor-update SUCCESS in 2h 10m 53s
✔️ cifmw-multinode-tempest SUCCESS in 1h 53m 13s
✔️ cifmw-pod-zuul-files SUCCESS in 4m 24s
✔️ noop SUCCESS in 0s
✔️ cifmw-pod-ansible-test SUCCESS in 8m 36s
❌ cifmw-pod-pre-commit FAILURE in 8m 30s
✔️ cifmw-molecule-test_operator SUCCESS in 3m 00s

- Add automatic sync script (sync_test_operator_var_readme.py) to keep README.md in sync with defaults/main.yml - Reorganize parameter documentation in alphabetical order - Replace inline default value definitions with references to defaults/main.yml to reduce duplication and improve maintainability - Add auto-sync note at the top of README.md - Fix pre-commit ansible-lint compatibility Signed-off-by: Saurabh Agarwal <sauragar@redhat.com> Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

kstrenkova

I am leaving a few very small comments. I think the PR is almost done, so it would be a good idea to ask someone else for a review to get more perspective. I am also not sure about editing the pre-commit and setup_molecule files, we need someone from ci-framework for that :D

kstrenkova · 2026-01-21T15:59:57Z

scripts/sync_test_operator_var_readme.py

+
+        # 1️⃣ Redirect if value is complex
+        if should_redirect(value):
+            print("should_redirect =", should_redirect(value))


When you run the script there are a lot of these prints, which say nothing to the user. I think it could be removed.

kstrenkova · 2026-01-21T16:10:48Z

scripts/sync_test_operator_var_readme.py

+    # Convert Python bool to lowercase for consistency
+    if isinstance(value, bool):
+        value_str = str(value).lower()
+    elif value is None:


I don't think the if or elif is needed. It should reflect the real values that are in the defaults/main.yaml file. I am not sure why the value can be None when in the defaults/main.yaml there are no None, so you can correct me.

kstrenkova · 2026-01-21T16:13:55Z

scripts/sync_test_operator_var_readme.py

+
+def normalize_default_text(block, value):
+    # Normalize "Defaults to `x`" → "Default value: `x`"
+    block = re.sub(


I don't think this substitution is needed. If someone wanted to, they could also do Default: and this code wouldn't help to keep consistency. So I don't think it's necessary to normalize it, as you can never catch all cases.

michburk

A handful of comments and suggestions.

Overall, I understand the motivation to keep the readme up to date by using a script. We've done a pretty poor job of enforcing that the docs be updated with every change to the defaults, but I don't love the idea of designing and maintaining a script specifically for just one role's readme. I'd be curious to see what other cifmw folks have to say.

michburk · 2026-01-22T20:41:02Z

scripts/sync_test_operator_var_readme.py

+README_PATH = Path("roles/test_operator/README.md")
+DEFAULTS_PATH = Path("roles/test_operator/defaults/main.yml")
+DEFAULTS_REL = DEFAULTS_PATH.relative_to("roles/test_operator")
+DEFAULTS_LINK = f"[{DEFAULTS_REL.name}]({DEFAULTS_REL})"


[blocking] When rendered to html, the links to defaults/main.yml don't lead anywhere. main.yml is presented as a link and you can click on it, but it doesn't do anything. In general, since these docs are also meant to be accessed from the web browser, a suggestion to check main.yml isn't very helpful.

michburk · 2026-01-22T20:41:08Z

scripts/sync_test_operator_var_readme.py

+
+    # CASE 1: Variable already exists in README
+    if existing_block:
+        # 🔹 Normalize Defaults wording FIRST


[blocking] No emojis please
(same comment applies to lines 156, 171, and 181)

michburk · 2026-01-22T20:41:13Z

scripts/sync_test_operator_var_readme.py

+    # Rebuild README
+    for group in grouped_new:
+        new_content = "\n".join(
+            grouped_new[group][v] for v in sorted(grouped_new[group])


[non-blocking] I noticed that the vars are now listed in alphabetical order. I assume there was not much thought put into how the vars were ordered before so this is probably fine, but if there was some meaningful ordering to the vars before, or if you want to order the vars in some meaningful way in the future, it might be worth preserving the order instead of alphabetizing.
Willing to hear others' thoughts on this, feel free to disregard if nobody has any strong feelings :)

michburk · 2026-01-22T20:54:07Z

.pre-commit-config.yaml

    hooks:
      - id: ansible-lint
        additional_dependencies:
+          - ansible>=8.0.0,<9.0.0


[blocking] +1 to @kstrenkova 's suggestion to not edit .pre-commit-config.yaml and scripts/setup_molecule in this pr. If you'd like to suggest those changes, feel free to open a separate pr. I'd prefer to keep this pr focused strictly on your script and the test_operator readme.

michburk · 2026-01-22T20:54:15Z

roles/test_operator/README.md

-* `cifmw_test_operator_logs_image`: (String) Image that should be used to collect logs from the pods spawned by the test-operator. Default value: `quay.io/quay/busybox`
+## Generic parameters
+<!-- START GENERIC_PARAMETERS -->
+* `cifmw_test_operator_artifacts_basedir`: (String) Directory where we will have all test-operator related files. Default value: `{{ cifmw_basedir }}/tests/test_operator` which Default value: `~/ci-framework-data/tests/test_operator`


Wording here is weird: "Default value: ... which Default value: ..."

michburk · 2026-01-22T21:05:35Z

roles/test_operator/README.md

-* `cifmw_test_operator_horizontest_registry`: (String) The registry where to pull horizontest container. Default value: `{{ cifmw_test_operator_default_registry }}`
-* `cifmw_test_operator_horizontest_namespace`: (String) Registry's namespace where to pull horizontest container. Default value: `{{ cifmw_test_operator_default_namespace }}`
-* `cifmw_test_operator_horizontest_container`: (String) Name of the horizontest container. Default value: `openstack-horizontest`
-* `cifmw_test_operator_horizontest_image`: (String) Horizontest image to be used. Default value: `{{ cifmw_test_operator_horizontest_registry }}/{{ cifmw_test_operator_horizontest_namespace }}/{{ cifmw_test_operator_horizontest_container }}`


[Maybe blocking?] I see that several vars in the updated version of the readme get their default values changed from some specific value (like {{ cifmw_test_operator_horizontest_registry }}/{{ cifmw_test_operator_horizontest_namespace }}/{{ cifmw_test_operator_horizontest_container }} in this example) to see defaults in main.yml. Would it be possible to preserve more of these values rather than replacing with see defaults?

sauragar force-pushed the OSPRH-19423 branch from d6a032c to c271ce2 Compare January 7, 2026 10:11

sauragar requested a review from kstrenkova January 7, 2026 10:14

sauragar self-assigned this Jan 7, 2026

sauragar added the Ready For Review label Jan 7, 2026

github-actions bot removed the Ready For Review label Jan 7, 2026

kstrenkova requested changes Jan 7, 2026

View reviewed changes

roles/test_operator/README.md Outdated Show resolved Hide resolved

scripts/sync_test_operator_var_readme.py Show resolved Hide resolved

sauragar force-pushed the OSPRH-19423 branch from c271ce2 to 5a3fdc3 Compare January 8, 2026 08:22

kstrenkova requested changes Jan 13, 2026

View reviewed changes

sauragar force-pushed the OSPRH-19423 branch from 5a3fdc3 to 6440ded Compare January 20, 2026 14:38

kstrenkova requested changes Jan 20, 2026

View reviewed changes

roles/test_operator/README.md Outdated Show resolved Hide resolved

roles/test_operator/README.md Outdated Show resolved Hide resolved

sauragar force-pushed the OSPRH-19423 branch from 6440ded to 6465ef2 Compare January 20, 2026 15:16

sauragar force-pushed the OSPRH-19423 branch 5 times, most recently from f256f7e to c6e4dd2 Compare January 21, 2026 09:10

openshift-ci bot added the do-not-merge/contains-merge-commits label Jan 21, 2026

sauragar force-pushed the OSPRH-19423 branch 2 times, most recently from 082fce7 to f2cf98b Compare January 21, 2026 09:48

github-actions bot added Ready For Review and removed Ready For Review labels Jan 21, 2026

sauragar force-pushed the OSPRH-19423 branch from 829426a to b2d47b3 Compare January 21, 2026 15:16

sauragar force-pushed the OSPRH-19423 branch from 3f17c9c to 5053771 Compare January 21, 2026 15:34

openshift-ci bot removed the do-not-merge/contains-merge-commits label Jan 21, 2026

kstrenkova requested changes Jan 21, 2026

View reviewed changes

github-actions bot added the Ready For Review label Jan 21, 2026

michburk requested changes Jan 22, 2026

View reviewed changes

[test_operator] Automatic sync of README.md #3589

Are you sure you want to change the base?

[test_operator] Automatic sync of README.md #3589

Conversation

sauragar commented Jan 7, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Jan 7, 2026

Uh oh!

kstrenkova left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

softwarefactory-project-zuul bot commented Jan 8, 2026

Uh oh!

kstrenkova left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kstrenkova left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

softwarefactory-project-zuul bot commented Jan 20, 2026

Uh oh!

softwarefactory-project-zuul bot commented Jan 20, 2026

Uh oh!

kstrenkova left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michburk left a comment

Choose a reason for hiding this comment

Uh oh!

michburk Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sauragar commented Jan 7, 2026 •

edited by openshift-ci bot

Loading

kstrenkova left a comment •

edited

Loading

michburk Jan 22, 2026 •

edited

Loading