Skip to content

test: add test for cifs remediation#309

Merged
richm merged 1 commit intoredhat-cop:mainfrom
richm:remediate-cifs
Dec 15, 2025
Merged

test: add test for cifs remediation#309
richm merged 1 commit intoredhat-cop:mainfrom
richm:remediate-cifs

Conversation

@richm
Copy link
Collaborator

@richm richm commented Dec 5, 2025

Add a test to trigger and verify the cifs remediation, and test
playbooks for 7to8, 8to9, and 9to10 in order to run remediations
for those cases.

This adds a sort of test framework for doing test setup and test
verify, by adding tests/tasks/setup/ and tests/tasks/verify/
sub-directories with task files for those purposes. We currently
use it only for doing remediation setup and verify, but I could
imagine using it for other purposes, or adding a cleanup/
sub-directory for cleanup tasks.

Signed-off-by: Rich Megginson rmeggins@redhat.com

@richm richm requested a review from spetrosi December 5, 2025 23:47
@richm
Copy link
Collaborator Author

richm commented Dec 5, 2025

[citest]

@spetrosi
Copy link
Collaborator

spetrosi commented Dec 8, 2025

Please rebase, tests should be all green now

Copy link
Collaborator

@spetrosi spetrosi left a comment

Choose a reason for hiding this comment

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

e2e tests are pretty heavy already. Did you consider placing this test to a remediation role tests? So that it won't do any upgrade, just trigger an inhibitor and remediate it and check if remediation worked.

@spetrosi
Copy link
Collaborator

spetrosi commented Dec 9, 2025

Well not all green, there is a selinux failing on 8to9 update due to a known bug.

@richm
Copy link
Collaborator Author

richm commented Dec 11, 2025

e2e tests are pretty heavy already. Did you consider placing this test to a remediation role tests? So that it won't do any upgrade, just trigger an inhibitor and remediate it and check if remediation worked.

I think in order to test remediations, I need to do most of what common_upgrade_tasks.yml does, except for doing the actual upgrade - run analysis, show inhibitors, map inhibitors to remediations, run remediations, check remediations, run analysis after remediation. So, I could add another test in https://github.com/redhat-cop/infra.leapp/tree/main/tests e.g. tests_remediations.yml which would run common_upgrade_tasks.yml with __leapp_test_run_upgrade: false, and add a when: __leapp_test_run_upgrade | d(true) to the name: common_upgrade_tasks | Run upgrade role task.

wdyt?

@richm
Copy link
Collaborator Author

richm commented Dec 11, 2025

[citest]

@spetrosi
Copy link
Collaborator

e2e tests are pretty heavy already. Did you consider placing this test to a remediation role tests? So that it won't do any upgrade, just trigger an inhibitor and remediate it and check if remediation worked.

I think in order to test remediations, I need to do most of what common_upgrade_tasks.yml does, except for doing the actual upgrade - run analysis, show inhibitors, map inhibitors to remediations, run remediations, check remediations, run analysis after remediation. So, I could add another test in https://github.com/redhat-cop/infra.leapp/tree/main/tests e.g. tests_remediations.yml which would run common_upgrade_tasks.yml with __leapp_test_run_upgrade: false, and add a when: __leapp_test_run_upgrade | d(true) to the name: common_upgrade_tasks | Run upgrade role task.

wdyt?

Yep, good idea. Ff this works, it's better than make the current upgrade tests larger. In this case, we can do a similar test for #312 too. It's a lot of work to cover all existing remediations with tests. But I am for adding tests for what we develop going forward.

@richm
Copy link
Collaborator Author

richm commented Dec 12, 2025

e2e tests are pretty heavy already. Did you consider placing this test to a remediation role tests? So that it won't do any upgrade, just trigger an inhibitor and remediate it and check if remediation worked.

I think in order to test remediations, I need to do most of what common_upgrade_tasks.yml does, except for doing the actual upgrade - run analysis, show inhibitors, map inhibitors to remediations, run remediations, check remediations, run analysis after remediation. So, I could add another test in https://github.com/redhat-cop/infra.leapp/tree/main/tests e.g. tests_remediations.yml which would run common_upgrade_tasks.yml with __leapp_test_run_upgrade: false, and add a when: __leapp_test_run_upgrade | d(true) to the name: common_upgrade_tasks | Run upgrade role task.
wdyt?

Yep, good idea. Ff this works, it's better than make the current upgrade tests larger. In this case, we can do a similar test for #312 too. It's a lot of work to cover all existing remediations with tests. But I am for adding tests for what we develop going forward.

Ok - check out the latest commit

@richm
Copy link
Collaborator Author

richm commented Dec 12, 2025

[citest]

vars:
setup_task_files: "{{ setup_tasks.files | map(attribute='path') | reject('search', reject_pattern) | list }}"
reject_pattern: "{{ '/remediate_' if not leapp_test_remediations | d(false) else 'NOMATCH' }}"
when: setup_task_files | length > 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't have this test below for verify_remediation_tasks.files. Is it really necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not sure what you mean

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see now, you do setup only when leapp_test_remediations=true. Will this simplification work? I was looking at how you did the common_upgrade_tasks | Verify remediations task below.

- name: common_upgrade_tasks | Do setup tasks
  ansible.builtin.include_tasks: "{{ setup_task_file }}"
  loop: "{{ setup_tasks.files | map(attribute='path' | list }}"
  loop_control:
    loop_var: setup_task_file
  when: leapp_test_remediations

@richm
Copy link
Collaborator Author

richm commented Dec 12, 2025

[citest]

# environment-file:
# - https://<GitLab CEE RedHat instance>/oamg/tests-ansible-collection-leapp/-/raw/main/leapp_coll_env_file.yml
environment-file:
- leapp_coll_env_file.yml
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please keep this commented out, adding this will break the CI

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fixed in #314

vars:
setup_task_files: "{{ setup_tasks.files | map(attribute='path') | reject('search', reject_pattern) | list }}"
reject_pattern: "{{ '/remediate_' if not leapp_test_remediations | d(false) else 'NOMATCH' }}"
when: setup_task_files | length > 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see now, you do setup only when leapp_test_remediations=true. Will this simplification work? I was looking at how you did the common_upgrade_tasks | Verify remediations task below.

- name: common_upgrade_tasks | Do setup tasks
  ansible.builtin.include_tasks: "{{ setup_task_file }}"
  loop: "{{ setup_tasks.files | map(attribute='path' | list }}"
  loop_control:
    loop_var: setup_task_file
  when: leapp_test_remediations

Add a test to trigger and verify the cifs remediation, and test
playbooks for 7to8, 8to9, and 9to10 in order to run remediations
for those cases.

This adds a sort of test framework for doing test setup and test
verify, by adding `tests/tasks/setup/` and `tests/tasks/verify/`
sub-directories with task files for those purposes.  We currently
use it only for doing remediation setup and verify, but I could
imagine using it for other purposes, or adding a `cleanup/`
sub-directory for cleanup tasks.

Signed-off-by: Rich Megginson <rmeggins@redhat.com>
@richm
Copy link
Collaborator Author

richm commented Dec 15, 2025

@spetrosi

I see now, you do setup only when leapp_test_remediations=true.

For now, but I think this could be used for more general setup tasks.

Will this simplification work? I was looking at how you did the common_upgrade_tasks | Verify remediations task below.

It will work for now, but I would like to support setup for other than remediation in the future.

What I'm trying to do with task name: common_upgrade_tasks | Do setup tasks is - if leapp_test_remediations is false (or unset), then do not run the remediation setup tasks, but run the other setup tasks. Right now, there are only remediation tasks, but in the future, I would like to have more.

@spetrosi
Copy link
Collaborator

I see, thanks for the explanation. Good to have it as you wrote it then.
RHEL 7 tests probably fail because tests_remediations_7to8 runs prior to the upgrade tests, and there is something missing in the cleanup on RHEL 7 specifically. Not related to this PR.

Copy link
Collaborator

@spetrosi spetrosi left a comment

Choose a reason for hiding this comment

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

Please rebase and merge

@richm
Copy link
Collaborator Author

richm commented Dec 15, 2025

[citest]

@richm richm merged commit 8a3dcd9 into redhat-cop:main Dec 15, 2025
24 of 25 checks passed
@richm richm deleted the remediate-cifs branch December 15, 2025 20:52
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