Skip to content

Commit

Permalink
Fix "unnamed-task" violations.
Browse files Browse the repository at this point in the history
1. Adding the missing tasks' names.
Fixing 31 violations:
  unnamed-task: All tasks should be named.
2. Fixing some names (i.e. typos, plural vs. single and vice versa).
3. Adding additional info to the names (like disk ID, VM name or Storage Domain name).
4. Some comments improvements.

Signed-off-by: Pavel Bar <pbar@redhat.com>
Bug-Url: https://bugzilla.redhat.com/2097332
  • Loading branch information
barpavel committed Jun 30, 2022
1 parent 3493a54 commit cb5ef90
Show file tree
Hide file tree
Showing 29 changed files with 119 additions and 59 deletions.
2 changes: 1 addition & 1 deletion automation/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ cd "$COLLECTION_DIR"
ansible-test sanity
antsibull-changelog lint -v
ansible-lint roles/* --exclude roles/hosted_engine_setup --exclude roles/disaster_recovery --exclude roles/remove_stale_lun -x experimental
ansible-lint -x unnamed-task,fqcn-builtins,no-changed-when,ignore-errors roles/disaster_recovery roles/remove_stale_lun
ansible-lint -x fqcn-builtins,no-changed-when,ignore-errors roles/disaster_recovery roles/remove_stale_lun

cd "$ROOT_PATH"

Expand Down
6 changes: 4 additions & 2 deletions roles/disaster_recovery/tasks/clean/remove_disks.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
- block:
- name: Remove disk
---
- name: Remove disk main block
block:
- name: "Remove disk '{{ disk.id }}'"
ovirt_disk:
state: absent
id: "{{ disk.id }}"
Expand Down
6 changes: 4 additions & 2 deletions roles/disaster_recovery/tasks/clean/remove_domain.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
- block:
---
- name: Remove storage domain main block
block:
# If we get the exception "Cannot deactivate Master Data Domain while there are running tasks on its Data Center."
# We should wait for some time and try again
- name: Remove storage domain
- name: "Remove storage domain '{{ sd.name }}'"
ovirt_storage_domain:
state: absent
id: "{{ sd.id }}"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
- block:
---
- name: Remove storage domain process main block
block:
# TODO: Check what happens when we force remove unattached storage domain (probably should add a default empty GUID as a data center
# Answer: When we force remove an unattached storage domain, ansible tries to move it to maintenance and detach it first,
# although it might be that this storage domain is already detached and has no related data center, therefor the move to maintenance will fail
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
- block:
---
- name: Remove invalid storage domain main block
block:
- name: Fetch invalid storage domain for remove
ovirt_storage_domain_info:
pattern: name={{ storage['dr_' + dr_source_map + '_name'] }} and {{ dr_inactive_domain_search }}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
- block:
---
- name: Remove valid storage domain main block
block:
- name: Fetch active/maintenance/detached storage domain for remove
ovirt_storage_domain_info:
pattern: >
Expand Down
6 changes: 4 additions & 2 deletions roles/disaster_recovery/tasks/clean/remove_vms.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
- block:
- name: Remove diskless VMs
---
- name: Remove diskless VM main block
block:
- name: "Remove diskless VM '{{ vm.name }}'"
ovirt_vm:
state: absent
name: "{{ vm.name }}"
Expand Down
6 changes: 4 additions & 2 deletions roles/disaster_recovery/tasks/clean/shutdown_vm.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
- block:
- name: Shutdown VM
---
- name: Shutdown VM main block
block:
- name: "Shutdown VM '{{ vms.name }}'"
ovirt_vm:
state: stopped
name: "{{ vms.name }}"
Expand Down
4 changes: 3 additions & 1 deletion roles/disaster_recovery/tasks/clean/shutdown_vms.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
- block:
---
- name: Shutdown VMs main block
block:
# Get all the running VMs related to a storage domain and shut them down
- name: Fetch VMs in the storage domain
ovirt_vm_info:
Expand Down
4 changes: 3 additions & 1 deletion roles/disaster_recovery/tasks/clean/update_ovf_store.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
- block:
---
- name: Update OVF store for active storage domain main block
block:
- name: Fetch storage domain only if active
ovirt_storage_domain_info:
pattern: status = active and storage.name={{ storage['dr_' + dr_source_map + '_name'] }}
Expand Down
8 changes: 5 additions & 3 deletions roles/disaster_recovery/tasks/clean_engine.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
- block:
---
- name: Clean engine main block
block:
- name: Obtain SSO token
ovirt_auth:
url: "{{ vars['dr_sites_' + dr_source_map + '_url'] }}"
Expand Down Expand Up @@ -44,7 +46,7 @@

# We use inactive filter only at the end, since we are not sure if there were any storage domains
# which became inactive on the process or if there were any at the beginning.
- name: Set force remove flag to true for non master domains
- name: Set force remove flag to true for non master storage domains
set_fact: dr_force=True

- name: Remove non master storage domains with invalid statuses using force remove
Expand Down Expand Up @@ -92,7 +94,7 @@
register: vm_info
when: dr_clean_orphaned_vms and storage_domain_info.ovirt_storage_domains | length == 0

- name: Remove vms if no storage domains left in setup
- name: Remove VMs if no storage domains left in setup
include_tasks: clean/remove_vms.yml
vars:
vm: "{{ item }}"
Expand Down
4 changes: 3 additions & 1 deletion roles/disaster_recovery/tasks/generate_mapping.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
- block:
---
- name: Generate mapping var file main block
block:
- name: Generate mapping var file
command: python3 {{ role_path }}/files/generate_mapping.py -a "{{ site }}" -u "{{ username }}" -p "{{ password }}" -c "{{ ca }}" -f "{{ var_file }}"
run_once: true
Expand Down
6 changes: 4 additions & 2 deletions roles/disaster_recovery/tasks/main.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
- block:
---
- name: Main block
block:
- name: Start to unregister entities
include_tasks: unregister_entities.yml
tags:
Expand Down Expand Up @@ -27,7 +29,7 @@
tags:
- fail_back

- name: Genereate mapping var file
- name: Generate mapping var file
include_tasks: generate_mapping.yml
tags:
- generate_mapping
18 changes: 12 additions & 6 deletions roles/disaster_recovery/tasks/recover/add_domain.yml
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
- block:
---
- name: Add storage domain main block
block:
- name: Fetch available hosts in data center
ovirt_host_info:
pattern: "status=up and datacenter={{ storage['dr_' + dr_target_host + '_dc_name'] }}"
auth: "{{ ovirt_auth }}"
register: host_info
- block:

- name: Check for available hosts block
block:
- name: "Check for available hosts"
fail: msg="No hosts available"
when: host_info.ovirt_hosts.0 is undefined
- block:

- name: Add storage domain block
block:
- name: Add storage domain if NFS
include_tasks: add_nfs_domain.yml
with_items:
Expand All @@ -25,23 +31,23 @@
loop_control:
loop_var: gluster_storage

- name: Add storage domain if Posix
- name: Add storage domain if POSIX
include_tasks: add_posixfs_domain.yml
with_items:
- "{{ storage }}"
when: "storage.dr_domain_type == 'posixfs'"
loop_control:
loop_var: posix_storage

- name: Add storage domain is scsi
- name: Add storage domain if iSCSI
include_tasks: add_iscsi_domain.yml
with_items:
- "{{ storage }}"
when: "storage.dr_domain_type == 'iscsi'"
loop_control:
loop_var: iscsi_storage

- name: Add storage domain if fcp
- name: Add storage domain if FCP
include_tasks: add_fcp_domain.yml
with_items:
- "{{ storage }}"
Expand Down
4 changes: 3 additions & 1 deletion roles/disaster_recovery/tasks/recover/add_fcp_domain.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
- block:
---
- name: Import FCP storage domain main block
block:
- name: Import FCP storage domain
ovirt_storage_domain:
state: imported
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
- block:
---
- name: Add Gluster storage domain main block
block:
- name: Add Gluster storage domain
ovirt_storage_domain:
name: "{{ gluster_storage['dr_' + dr_target_host + '_name'] }}"
Expand Down
9 changes: 6 additions & 3 deletions roles/disaster_recovery/tasks/recover/add_iscsi_domain.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
- block:
---
- name: Import iSCSI storage domain main block
block:
# TODO: Add support for connect to multiple targets with the same LUN.
# Every connect should be done using a different ip
- block:
# Every connect should be done using a different IP.
- name: Import iSCSI storage domain block
block:
- name: Login to iSCSI targets
ovirt_host:
state: iscsilogin
Expand Down
4 changes: 3 additions & 1 deletion roles/disaster_recovery/tasks/recover/add_nfs_domain.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
- block:
---
- name: Add NFS storage domain main block
block:
- name: Add NFS storage domain
ovirt_storage_domain:
name: "{{ nfs_storage['dr_' + dr_target_host + '_name'] }}"
Expand Down
6 changes: 4 additions & 2 deletions roles/disaster_recovery/tasks/recover/add_posixfs_domain.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
- block:
- name: Add posix storage domain
---
- name: Add POSIX storage domain main block
block:
- name: Add POSIX storage domain
ovirt_storage_domain:
name: "{{ posix_storage['dr_' + dr_target_host + '_name'] }}"
critical_space_action_blocker: "{{ posix_storage['dr_critical_space_action_blocker'] }}"
Expand Down
4 changes: 3 additions & 1 deletion roles/disaster_recovery/tasks/recover/print_info.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
- block:
---
- name: Print report file main block
block:
- name: Generate log file through template
template:
src: report_log_template.j2
Expand Down
10 changes: 6 additions & 4 deletions roles/disaster_recovery/tasks/recover/register_template.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
- block:
- name: Register unregistered Template
---
- name: Register unregistered template main block
block:
- name: "Register unregistered template '{{ unreg_template.id }}'"
ovirt_template:
state: registered
storage_domain: "{{ storage.name }}"
Expand All @@ -12,12 +14,12 @@
role_mappings: "{{ dr_role_map }}"
register: template_register_result

- name: Log append failed Template to issues failed_template_names
- name: Log append failed template to failed_template_names
set_fact:
failed_template_names: "{{ failed_template_names }} + [ '{{ unreg_template.name }}' ]"
when: template_register_result is failed

- name: Log append succeed_template_names
- name: Log append succeeded template succeed_template_names
set_fact:
succeed_template_names: "{{ succeed_template_names }} + [ '{{ unreg_template.name }}' ]"
when: template_register_result is succeeded
Expand Down
8 changes: 5 additions & 3 deletions roles/disaster_recovery/tasks/recover/register_templates.yml
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
- block:
- name: Fetch unregistered Templates from storage domain
---
- name: Register unregistered templates main block
block:
- name: Fetch unregistered templates from storage domain
ovirt_storage_template_info:
nested_attributes: "id"
unregistered: true
storage_domain: "{{ storage.name }}"
auth: "{{ ovirt_auth }}"
register: storage_template_info

- name: Register template
- name: Register unregistered templates
include: register_template.yml
# The main task is already declared to ignore errors so that might be
# redundant to put it here ignore_errors: "{{ ignore | default(yes) }}"
Expand Down
8 changes: 5 additions & 3 deletions roles/disaster_recovery/tasks/recover/register_vm.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
- block:
- name: Register VMs
---
- name: Register VM main block
block:
- name: Register VM
ovirt_vm:
state: registered
storage_domain: "{{ storage.name }}"
Expand All @@ -21,7 +23,7 @@
failed_vm_names: "{{ failed_vm_names }} + [ '{{ unreg_vm.name }}' ]"
when: vm_register_result is failed

- name: Log append succeed_vm_names
- name: Log append succeeded VM to succeed_vm_names
set_fact:
succeed_vm_names: "{{ succeed_vm_names }} + [ '{{ unreg_vm.name }}' ]"
when: vm_register_result is succeeded
Expand Down
6 changes: 4 additions & 2 deletions roles/disaster_recovery/tasks/recover/register_vms.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
- block:
---
- name: Register VMs main block
block:
- name: Fetch unregistered VMs from storage domain
ovirt_storage_vm_info:
nested_attributes: "id"
Expand All @@ -12,7 +14,7 @@
unreg_vms: "{{ unreg_vms | default([]) + storage_vm_info.ovirt_storage_vms }}"

# TODO: We should filter out VMs which already exist in the setup (diskless VMs)
- name: Register VM
- name: Register VMs
include: register_vm.yml
with_items: "{{ storage_vm_info.ovirt_storage_vms }}"
# We use loop_control so storage.name will not be overridden by the nested loop.
Expand Down
4 changes: 3 additions & 1 deletion roles/disaster_recovery/tasks/recover/run_vms.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
- block:
---
- name: Run VMs main block
block:
- name: Run VMs
ovirt_vm:
state: running
Expand Down
4 changes: 3 additions & 1 deletion roles/disaster_recovery/tasks/recover_engine.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
- block:
---
- name: Recover engine main block
block:
- name: Obtain SSO token
ovirt_auth:
url: "{{ vars['dr_sites_' + dr_target_host + '_url'] }}"
Expand Down
8 changes: 5 additions & 3 deletions roles/disaster_recovery/tasks/run_unregistered_entities.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
- block:
---
- name: Run unregistered entities main block
block:
- name: Obtain SSO token
ovirt_auth:
url: "{{ vars['dr_sites_' + dr_target_host + '_url'] }}"
Expand All @@ -14,14 +16,14 @@
path: "{{ dr_running_vms }}"
state: absent

- name: Run all the high availability VMs
- name: Run all the (previously running) high availability VMs
include_tasks: recover/run_vms.yml
vars:
vms: "{{ item }}"
with_items: "{{ running_vms_fail_back }}"
when: item.high_availability.enabled | bool

- name: Run all the entire running VMs
- name: Run all the (previously running) non high availability VMs
include_tasks: recover/run_vms.yml
vars:
vms: "{{ item }}"
Expand Down
Loading

0 comments on commit cb5ef90

Please sign in to comment.