Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DR-Failback errors when gathering/detecting HA VM as well as storage domains #649

Closed
wants to merge 22 commits into from
Closed

DR-Failback errors when gathering/detecting HA VM as well as storage domains #649

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
88fab7b
DR-Failback errors when gathering/detecting HA VM as well as storage …
shubhaOracle Dec 8, 2022
8fc1bb6
DR-Failback errors when gathering/detecting HA VM as well as storage …
shubhaOracle Dec 8, 2022
0e9c9ec
remove trailing space
shubhaOracle Dec 9, 2022
4bc7748
rebase and remove trailing space
shubhaOracle Dec 9, 2022
f3af909
Merge branch 'master' into sd-detection-during-failback
shubhaOracle Dec 14, 2022
8fe7473
DR-Failback errors when gathering/detecting HA VM as well as storage …
shubhaOracle Dec 8, 2022
542310b
remove trailing space
shubhaOracle Dec 9, 2022
e145fc8
DR-Failback errors when gathering/detecting HA VM as well as storage …
shubhaOracle Dec 8, 2022
c722a8a
rebase and remove trailing space. Update task name
shubhaOracle Jan 10, 2023
4b9f968
Merge branch 'sd-detection-during-failback' of https://github.com/shu…
shubhaOracle Jan 10, 2023
07636db
DR-Failback errors when gathering/detecting HA VM as well as storage …
shubhaOracle Jan 11, 2023
06fbd1b
DR-Failback errors when gathering/detecting HA VM as well as storage …
shubhaOracle Jan 12, 2023
155e656
Update roles/disaster_recovery/tasks/clean/remove_valid_filtered_mast…
shubhaOracle Jan 12, 2023
60a9506
Update roles/disaster_recovery/tasks/clean/remove_valid_filtered_mast…
shubhaOracle Jan 12, 2023
9c65be5
DR-Failback errors when gathering/detecting HA VM as well as storage …
shubhaOracle Dec 8, 2022
d07f5bf
DR-Failback errors when gathering/detecting HA VM as well as storage …
shubhaOracle Jan 11, 2023
6749793
DR-Failback errors when gathering/detecting HA VM as well as storage …
shubhaOracle Jan 12, 2023
52fb262
Update roles/disaster_recovery/tasks/clean/remove_valid_filtered_mast…
shubhaOracle Jan 12, 2023
e1b63c1
Update roles/disaster_recovery/tasks/clean/remove_valid_filtered_mast…
shubhaOracle Jan 12, 2023
3736089
Storage domain detection was not finding any storage domains due to i…
shubhaOracle Jan 12, 2023
3b07711
Storage domain detection was not finding any storage domains due to i…
shubhaOracle Jan 12, 2023
86e358e
Created separate tasks for detecting domains in various states and fi…
shubhaOracle Jan 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 37 additions & 7 deletions roles/disaster_recovery/tasks/clean/remove_valid_filtered_master_domains.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,54 @@
---
- name: Remove valid storage domain main block
block:
- name: Fetch active/maintenance/detached storage domain for remove
- name: Fetch active storage domain for remove
ovirt_storage_domain_info:
pattern: >
name={{ storage['dr_' + dr_source_map + '_name'] }} and
Copy link
Member

Choose a reason for hiding this comment

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

1 small indentation fix: move 1 character to the left.

datacenter={{ storage['dr_' + dr_source_map + '_dc_name'] }} and {{ dr_active_domain_search }}
auth: "{{ ovirt_auth }}"
register: storage_domain_info_active

- name: Fetch maintenance storage domain for remove
ovirt_storage_domain_info:
pattern: >
name={{ storage['dr_' + dr_source_map + '_name'] }} and
datacenter={{ storage['dr_' + dr_source_map + '_dc_name'] }} and {{ dr_maintenance_domain_search }}
auth: "{{ ovirt_auth }}"
register: storage_domain_info_maintenance

- name: Fetch detached storage domain for remove
Copy link
Member

Choose a reason for hiding this comment

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

This section (lines 20-26) seems duplicate (same as lines 4-10) + wrong name?
AFAIU lines 20-26 should be removed.

ovirt_storage_domain_info:
pattern: >
Copy link
Member

@barpavel barpavel Jan 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I must admit I don't have a vast Ansible experience, but could some "playing" with () be a more simple option to fix the original code instead of splitting the code into 3 tasks with some amount of duplicate code?
Not sure whether the following syntax will work:

        pattern: >
          name={{ storage['dr_' + dr_source_map + '_name'] }} and
          (
            (
              datacenter={{ storage['dr_' + dr_source_map + '_dc_name'] }} and
              (
                {{ dr_active_domain_search }} or {{ dr_maintenance_domain_search }}
              )
            )
            or
            {{ dr_unattached_domain_search }}
          )

If not then let's leave your solution with 3 separate tasks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I must admit I don't have a vast Ansible experience, but could some "playing" with () be a more simple option to fix the original code instead of splitting the code into 3 tasks with some amount of duplicate code? Not sure whether the following syntax will work:

        pattern: >
          name={{ storage['dr_' + dr_source_map + '_name'] }} and
          (
            (
              datacenter={{ storage['dr_' + dr_source_map + '_dc_name'] }} and
              (
                {{ dr_active_domain_search }} or {{ dr_maintenance_domain_search }}
              )
            )
            or
            {{ dr_unattached_domain_search }}
          )

If not then let's leave your solution with 3 separate tasks.

Thanks for taking a look at the changes. I would have preferred to have one common condition as well. However, for readability and ease of maintenance sake, I feel my new approach is much simpler. I hope that works for you as well.

name={{ storage['dr_' + dr_source_map + '_name'] }} and
shubhaOracle marked this conversation as resolved.
Show resolved Hide resolved
datacenter={{ storage['dr_' + dr_source_map + '_dc_name'] }} and {{ dr_active_domain_search }}
Copy link
Member

@barpavel barpavel Jan 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Please fix 2 trailing spaces that cause ansible-lint 6 to fail, which breaks the CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

Copy link
Member

Choose a reason for hiding this comment

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

datacenter={{ storage['dr_' + dr_source_map + '_dc_name'] }} and {{ dr_active_domain_search }}
-->
{{ dr_unattached_domain_search }}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

datacenter={{ storage['dr_' + dr_source_map + '_dc_name'] }} and {{ dr_active_domain_search }} --> {{ dr_unattached_domain_search }}

Thanks for the feedback and review!!

auth: "{{ ovirt_auth }}"
register: storage_domain_info_active
Copy link
Member

Choose a reason for hiding this comment

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

storage_domain_info_active
-->
storage_domain_info_detached


- name: Fetch maintenance storage domain for remove
Copy link
Member

Choose a reason for hiding this comment

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

This section (lines 28-34) seems duplicate (same as lines 12-18)?
AFAIU lines 28-34 should be removed.

ovirt_storage_domain_info:
pattern: >
name={{ storage['dr_' + dr_source_map + '_name'] }} and
datacenter={{ storage['dr_' + dr_source_map + '_dc_name'] }} and {{ dr_maintenance_domain_search }}
auth: "{{ ovirt_auth }}"
register: storage_domain_info_maintenance

- name: Fetch detached storage domain for remove
ovirt_storage_domain_info:
pattern: >
name={{ storage['dr_' + dr_source_map + '_name'] }} and
(
datacenter={{ storage['dr_' + dr_source_map + '_dc_name'] }} and {{ dr_active_domain_search }} or
datacenter={{ storage['dr_' + dr_source_map + '_dc_name'] }} and {{ dr_maintenance_domain_search }} or
{{ dr_unattached_domain_search }}
)
auth: "{{ ovirt_auth }}"
register: storage_domain_info
register: storage_domain_info_detached

- name: Remove valid storage domain
include_tasks: remove_domain_process.yml
vars:
sd: "{{ sd }}"
with_items:
- "{{ storage_domain_info.ovirt_storage_domains }}"
- "{{ storage_domain_info_active.ovirt_storage_domains }}"
- "{{ storage_domain_info_maintenance.ovirt_storage_domains }}"
- "{{ storage_domain_info_detached.ovirt_storage_domains }}"
when: (not only_master and not sd.master) or (only_master and sd.master)
loop_control:
loop_var: sd
Expand Down
2 changes: 1 addition & 1 deletion roles/disaster_recovery/tasks/unregister_entities.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

- name: Init list property for running_vms
ansible.builtin.set_fact:
res_ovirt_vms="[]"
res_ovirt_vms=[]

- name: Map all running VMs in fact
ansible.builtin.set_fact:
Expand Down