[PATCH 1/6] monitoring: restructure tasks into modular architecture

From: Daniel Gomez <[email protected]>

Split monolithic monitor_run.yml and monitor_collect.yml into modular
structure with individual monitor subdirectories:

- Extract folio migration tasks to monitors/folio_migration/
- Create common tasks directory for shared functionality
- Move conditions to import level to reduce code duplication
- Convert orchestrator files to use import_tasks pattern
- Fix variable scope issues and remove cross-module dependencies
- Each monitor now handles its own matplotlib availability checking

This provides better separation of concerns and easier maintenance
for adding new monitoring capabilities.

Generated-by: Claude AI
Signed-off-by: Daniel Gomez <[email protected]>
---
 playbooks/roles/monitoring/tasks/common/setup.yml  |  29 +++
 .../roles/monitoring/tasks/monitor_collect.yml     | 207 +--------------------
 playbooks/roles/monitoring/tasks/monitor_run.yml   |  83 +--------
 .../tasks/monitors/folio_migration/collect.yml     | 147 +++++++++++++++
 .../folio_migration}/files/plot_migration_stats.py |   0
 .../tasks/monitors/folio_migration/run.yml         |  68 +++++++
 6 files changed, 254 insertions(+), 280 deletions(-)

diff --git a/playbooks/roles/monitoring/tasks/common/setup.yml b/playbooks/roles/monitoring/tasks/common/setup.yml
new file mode 100644
index 00000000..e3e8501d
--- /dev/null
+++ b/playbooks/roles/monitoring/tasks/common/setup.yml
@@ -0,0 +1,29 @@
+---
+# Common monitoring setup tasks
+
+- name: Debug monitoring collection start
+  ansible.builtin.debug:
+    msg: |
+      Starting monitoring collection
+      monitor_developmental_stats: {{ monitor_developmental_stats | default(false) }}
+      monitor_folio_migration: {{ monitor_folio_migration | default(false) }}
+      enable_monitoring: {{ enable_monitoring | default(false) }}
+      kdevops_run_fstests: {{ kdevops_run_fstests | default(false) }}
+
+- name: Set workflow-appropriate monitoring results path
+  ansible.builtin.set_fact:
+    monitoring_results_path: "{{ topdir_path }}/workflows/{{ kdevops_workflow_name | default('unknown') }}/results/monitoring"
+  when:
+    - enable_monitoring|default(false)|bool
+    - monitoring_results_path is not defined
+
+- name: Create local monitoring results directory
+  ansible.builtin.file:
+    path: "{{ monitoring_results_path }}"
+    state: directory
+    mode: '0755'
+  delegate_to: localhost
+  run_once: true
+  become: false
+  when:
+    - monitoring_results_path is defined
diff --git a/playbooks/roles/monitoring/tasks/monitor_collect.yml b/playbooks/roles/monitoring/tasks/monitor_collect.yml
index 5432fc87..79f2d7ae 100644
--- a/playbooks/roles/monitoring/tasks/monitor_collect.yml
+++ b/playbooks/roles/monitoring/tasks/monitor_collect.yml
@@ -1,208 +1,13 @@
 ---
-# Tasks to stop monitoring services and collect data after test execution
+# Monitoring orchestrator - includes individual collection modules
 
-- name: Check if folio migration monitoring was started
-  become: true
-  become_method: sudo
-  ansible.builtin.stat:
-    path: /root/monitoring/folio_migration.pid
-  register: folio_migration_pid_file
+# Import common monitoring setup tasks
+- ansible.builtin.import_tasks: common/setup.yml
   when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-
-- name: Stop folio migration monitoring
-  become: true
-  become_method: sudo
-  ansible.builtin.shell: |
-    if [ -f /root/monitoring/folio_migration.pid ]; then
-      pid=$(cat /root/monitoring/folio_migration.pid)
-      if ps -p $pid > /dev/null 2>&1; then
-        kill $pid
-        echo "Stopped monitoring process $pid"
-      else
-        echo "Monitoring process $pid was not running"
-      fi
-      rm -f /root/monitoring/folio_migration.pid
-    fi
-  register: stop_monitor
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - folio_migration_pid_file.stat.exists|default(false)
-
-- name: Display stop monitoring status
-  ansible.builtin.debug:
-    msg: "{{ stop_monitor.stdout }}"
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - stop_monitor is defined
-    - stop_monitor.changed|default(false)
-
-- name: Check if monitoring data was collected
-  become: true
-  become_method: sudo
-  ansible.builtin.stat:
-    path: /root/monitoring/folio_migration_stats.txt
-  register: folio_migration_data_file
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-
-- name: Copy plot_migration_stats.py to target
-  become: true
-  become_method: sudo
-  ansible.builtin.copy:
-    src: "{{ playbook_dir }}/roles/monitoring/files/plot_migration_stats.py"
-    dest: /root/monitoring/plot_migration_stats.py
-    mode: "0755"
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - folio_migration_data_file.stat.exists|default(false)
-
-- name: Check if matplotlib is available for plotting
-  become: true
-  become_method: sudo
-  ansible.builtin.command: python3 -c "import matplotlib.pyplot"
-  register: matplotlib_check
-  ignore_errors: true
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - folio_migration_data_file.stat.exists|default(false)
-
-- name: Generate folio migration plots
-  become: true
-  become_method: sudo
-  ansible.builtin.command: |
-    python3 /root/monitoring/plot_migration_stats.py
-      /root/monitoring/folio_migration_stats.txt
-      /root/monitoring/folio_migration_plot.png
-  args:
-    chdir: /root/monitoring
-  register: plot_generation
-  ignore_errors: true
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - folio_migration_data_file.stat.exists|default(false)
-    - matplotlib_check.rc == 0
-
-- name: Log plot generation skip if matplotlib not available
-  ansible.builtin.debug:
-    msg: "Skipping plot generation - matplotlib not available on target system"
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - folio_migration_data_file.stat.exists|default(false)
-    - matplotlib_check.rc != 0
-
-- name: Debug monitoring collection start
-  ansible.builtin.debug:
-    msg: |
-      Starting monitoring collection
-      monitor_developmental_stats: {{ monitor_developmental_stats | default(false) }}
-      monitor_folio_migration: {{ monitor_folio_migration | default(false) }}
-      enable_monitoring: {{ enable_monitoring | default(false) }}
-      kdevops_run_fstests: {{ kdevops_run_fstests | default(false) }}
-
-- name: Set monitoring results path
-  ansible.builtin.set_fact:
-    monitoring_results_path: "{{ monitoring_results_base_path | default(topdir_path + '/workflows/fstests/results/monitoring') }}"
-
-- name: Create local monitoring results directory
-  ansible.builtin.file:
-    path: "{{ monitoring_results_path }}"
-    state: directory
-  delegate_to: localhost
-  run_once: true
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-
-- name: Copy folio migration stats data to localhost
-  become: true
-  become_method: sudo
-  ansible.builtin.fetch:
-    src: /root/monitoring/folio_migration_stats.txt
-    dest: "{{ monitoring_results_path }}/{{ ansible_hostname }}_folio_migration_stats.txt"
-    flat: true
-    validate_checksum: false
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - folio_migration_data_file.stat.exists|default(false)
-
-- name: Check if plot was generated
-  become: true
-  become_method: sudo
-  ansible.builtin.stat:
-    path: /root/monitoring/folio_migration_plot.png
-  register: folio_migration_plot_file
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-
-- name: Copy folio migration plot to localhost
-  become: true
-  become_method: sudo
-  ansible.builtin.fetch:
-    src: /root/monitoring/folio_migration_plot.png
-    dest: "{{ monitoring_results_path }}/{{ ansible_hostname }}_folio_migration_plot.png"
-    flat: true
-    validate_checksum: false
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - folio_migration_plot_file.stat.exists|default(false)
-
-- name: Display monitoring data collection summary
-  ansible.builtin.debug:
-    msg: |
-      Folio migration monitoring collection complete.
-      Data saved to: {{ monitoring_results_path }}/{{ ansible_hostname }}_folio_migration_stats.txt
-      {% if folio_migration_plot_file.stat.exists | default(false) %}
-      Plot saved to: {{ monitoring_results_path }}/{{ ansible_hostname }}_folio_migration_plot.png
-      {% endif %}
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - folio_migration_data_file.stat.exists|default(false)
-
-# Generate plots on localhost after collecting data
-- name: Check if matplotlib is available on localhost
-  ansible.builtin.command: python3 -c "import matplotlib.pyplot"
-  delegate_to: localhost
-  register: localhost_matplotlib_check
-  ignore_errors: true
-  run_once: true
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-
-- name: Generate folio migration plots on localhost
-  ansible.builtin.command: "python3 {{ playbook_dir }}/roles/monitoring/files/plot_migration_stats.py\n  -o {{ monitoring_results_path }}/{{ ansible_hostname }}_folio_migration_plot.png\n\
-    \  {{ monitoring_results_path }}/{{ ansible_hostname }}_folio_migration_stats.txt\n"
-  delegate_to: localhost
-  register: localhost_plot_generation
-  ignore_errors: true
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - folio_migration_data_file.stat.exists|default(false)
-    - localhost_matplotlib_check.rc == 0
+    - enable_monitoring|default(false)|bool
 
-- name: Log localhost plot generation status
-  ansible.builtin.debug:
-    msg: |
-      {% if localhost_matplotlib_check.rc != 0 %}
-      Skipping plot generation - matplotlib not available on localhost
-      {% else %}
-      Plot generated: {{ monitoring_results_path }}/{{ ansible_hostname }}_folio_migration_plot.png
-      {% endif %}
+# Import folio migration collection tasks
+- ansible.builtin.import_tasks: monitors/folio_migration/collect.yml
   when:
     - monitor_developmental_stats|default(false)|bool
     - monitor_folio_migration|default(false)|bool
-    - folio_migration_data_file.stat.exists|default(false)
diff --git a/playbooks/roles/monitoring/tasks/monitor_run.yml b/playbooks/roles/monitoring/tasks/monitor_run.yml
index f56d06e4..a2055f89 100644
--- a/playbooks/roles/monitoring/tasks/monitor_run.yml
+++ b/playbooks/roles/monitoring/tasks/monitor_run.yml
@@ -1,83 +1,8 @@
 ---
-# Tasks to start monitoring services before test execution
+# Monitoring orchestrator - includes individual monitor modules
 
-- name: Check if folio migration stats are available
-  become: true
-  become_method: sudo
-  ansible.builtin.stat:
-    path: /sys/kernel/debug/mm/migrate/stats
-  register: folio_migration_stats_file
+# Import folio migration monitoring tasks
+- ansible.builtin.import_tasks: monitors/folio_migration/run.yml
   when:
     - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-
-- name: Create monitoring directory
-  become: true
-  become_method: sudo
-  ansible.builtin.file:
-    path: /root/monitoring
-    state: directory
-    mode: "0755"
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - folio_migration_stats_file.stat.exists|default(false)
-
-- name: Start folio migration monitoring in background
-  become: true
-  become_method: sudo
-  ansible.builtin.shell: |
-    nohup bash -c 'while true; do
-      echo "$(date +"%Y-%m-%d %H:%M:%S")" >> /root/monitoring/folio_migration_stats.txt
-      cat /sys/kernel/debug/mm/migrate/stats >> /root/monitoring/folio_migration_stats.txt
-      echo "" >> /root/monitoring/folio_migration_stats.txt
-      sleep {{ monitor_folio_migration_interval|default(60) }}
-    done' > /root/monitoring/folio_migration.log 2>&1 &
-    echo $! > /root/monitoring/folio_migration.pid
-  async: 86400 # Run for up to 24 hours
-  poll: 0
-  register: folio_migration_monitor
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - folio_migration_stats_file.stat.exists|default(false)
-
-- name: Save async job ID for later termination
-  ansible.builtin.set_fact:
-    folio_migration_monitor_job: "{{ folio_migration_monitor.ansible_job_id }}"
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - folio_migration_stats_file.stat.exists|default(false)
-    - folio_migration_monitor is defined
-
-- name: Verify monitoring started successfully
-  become: true
-  become_method: sudo
-  ansible.builtin.shell: |
-    if [ -f /root/monitoring/folio_migration.pid ]; then
-      pid=$(cat /root/monitoring/folio_migration.pid)
-      if ps -p $pid > /dev/null 2>&1; then
-        echo "Monitoring process $pid is running"
-      else
-        echo "ERROR: Monitoring process $pid is not running" >&2
-        exit 1
-      fi
-    else
-      echo "ERROR: PID file not found" >&2
-      exit 1
-    fi
-  register: monitor_status
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - folio_migration_stats_file.stat.exists|default(false)
-
-- name: Display monitoring status
-  ansible.builtin.debug:
-    msg: "{{ monitor_status.stdout }}"
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - folio_migration_stats_file.stat.exists|default(false)
-    - monitor_status is defined
+    - monitor_folio_migration|default(false)|bool
\ No newline at end of file
diff --git a/playbooks/roles/monitoring/tasks/monitors/folio_migration/collect.yml b/playbooks/roles/monitoring/tasks/monitors/folio_migration/collect.yml
new file mode 100644
index 00000000..bd04ebe2
--- /dev/null
+++ b/playbooks/roles/monitoring/tasks/monitors/folio_migration/collect.yml
@@ -0,0 +1,147 @@
+---
+# Folio migration monitoring data collection tasks
+# Conditions are checked at import level in monitor_collect.yml
+
+- name: Check if folio migration monitoring was started
+  become: true
+  become_method: sudo
+  ansible.builtin.stat:
+    path: /root/monitoring/folio_migration.pid
+  register: folio_migration_pid_file
+
+- name: Stop folio migration monitoring
+  become: true
+  become_method: sudo
+  ansible.builtin.shell: |
+    if [ -f /root/monitoring/folio_migration.pid ]; then
+      pid=$(cat /root/monitoring/folio_migration.pid)
+      if ps -p $pid > /dev/null 2>&1; then
+        kill $pid
+        echo "Stopped monitoring process $pid"
+      else
+        echo "Monitoring process $pid was not running"
+      fi
+      rm -f /root/monitoring/folio_migration.pid
+    fi
+  register: stop_monitor
+  when: folio_migration_pid_file.stat.exists|default(false)
+
+- name: Display stop monitoring status
+  ansible.builtin.debug:
+    msg: "{{ stop_monitor.stdout }}"
+  when:
+    - stop_monitor is defined
+    - stop_monitor.changed|default(false)
+
+- name: Check if monitoring data was collected
+  become: true
+  become_method: sudo
+  ansible.builtin.stat:
+    path: /root/monitoring/folio_migration_stats.txt
+  register: folio_migration_data_file
+
+- name: Copy plot_migration_stats.py to target
+  become: true
+  become_method: sudo
+  ansible.builtin.copy:
+    src: "{{ playbook_dir }}/roles/monitoring/files/plot_migration_stats.py"
+    dest: /root/monitoring/plot_migration_stats.py
+    mode: "0755"
+  when: folio_migration_data_file.stat.exists|default(false)
+
+- name: Check if matplotlib is available for plotting
+  become: true
+  become_method: sudo
+  ansible.builtin.command: python3 -c "import matplotlib.pyplot"
+  register: matplotlib_check
+  ignore_errors: true
+  when: folio_migration_data_file.stat.exists|default(false)
+
+- name: Generate folio migration plots
+  become: true
+  become_method: sudo
+  ansible.builtin.command: |
+    python3 /root/monitoring/plot_migration_stats.py
+      /root/monitoring/folio_migration_stats.txt
+      /root/monitoring/folio_migration_plot.png
+  args:
+    chdir: /root/monitoring
+  register: plot_generation
+  ignore_errors: true
+  when:
+    - folio_migration_data_file.stat.exists|default(false)
+    - matplotlib_check.rc == 0
+
+- name: Log plot generation skip if matplotlib not available
+  ansible.builtin.debug:
+    msg: "Skipping plot generation - matplotlib not available on target system"
+  when:
+    - folio_migration_data_file.stat.exists|default(false)
+    - matplotlib_check.rc != 0
+
+- name: Copy folio migration stats data to localhost
+  become: true
+  become_method: sudo
+  ansible.builtin.fetch:
+    src: /root/monitoring/folio_migration_stats.txt
+    dest: "{{ monitoring_results_path }}/{{ ansible_hostname }}_folio_migration_stats.txt"
+    flat: true
+    validate_checksum: false
+  when: folio_migration_data_file.stat.exists|default(false)
+
+- name: Check if plot was generated
+  become: true
+  become_method: sudo
+  ansible.builtin.stat:
+    path: /root/monitoring/folio_migration_plot.png
+  register: folio_migration_plot_file
+
+- name: Copy folio migration plot to localhost
+  become: true
+  become_method: sudo
+  ansible.builtin.fetch:
+    src: /root/monitoring/folio_migration_plot.png
+    dest: "{{ monitoring_results_path }}/{{ ansible_hostname }}_folio_migration_plot.png"
+    flat: true
+    validate_checksum: false
+  when: folio_migration_plot_file.stat.exists|default(false)
+
+- name: Display monitoring data collection summary
+  ansible.builtin.debug:
+    msg: |
+      Folio migration monitoring collection complete.
+      Data saved to: {{ monitoring_results_path }}/{{ ansible_hostname }}_folio_migration_stats.txt
+      {% if folio_migration_plot_file.stat.exists | default(false) %}
+      Plot saved to: {{ monitoring_results_path }}/{{ ansible_hostname }}_folio_migration_plot.png
+      {% endif %}
+  when: folio_migration_data_file.stat.exists|default(false)
+
+- name: Check if matplotlib is available on localhost for folio migration
+  ansible.builtin.command: python3 -c "import matplotlib.pyplot"
+  delegate_to: localhost
+  register: folio_localhost_matplotlib_check
+  ignore_errors: true
+  run_once: true
+  when: folio_migration_data_file.stat.exists|default(false)
+
+- name: Generate folio migration plots on localhost
+  ansible.builtin.command: |
+    python3 {{ playbook_dir }}/roles/monitoring/files/plot_migration_stats.py
+      -o {{ monitoring_results_path }}/{{ ansible_hostname }}_folio_migration_plot.png
+      {{ monitoring_results_path }}/{{ ansible_hostname }}_folio_migration_stats.txt
+  delegate_to: localhost
+  register: localhost_plot_generation
+  ignore_errors: true
+  when:
+    - folio_migration_data_file.stat.exists|default(false)
+    - folio_localhost_matplotlib_check.rc == 0
+
+- name: Log localhost plot generation status
+  ansible.builtin.debug:
+    msg: |
+      {% if folio_localhost_matplotlib_check.rc != 0 %}
+      Skipping plot generation - matplotlib not available on localhost
+      {% else %}
+      Plot generated: {{ monitoring_results_path }}/{{ ansible_hostname }}_folio_migration_plot.png
+      {% endif %}
+  when: folio_migration_data_file.stat.exists|default(false)
\ No newline at end of file
diff --git a/playbooks/roles/monitoring/files/plot_migration_stats.py b/playbooks/roles/monitoring/tasks/monitors/folio_migration/files/plot_migration_stats.py
similarity index 100%
rename from playbooks/roles/monitoring/files/plot_migration_stats.py
rename to playbooks/roles/monitoring/tasks/monitors/folio_migration/files/plot_migration_stats.py
diff --git a/playbooks/roles/monitoring/tasks/monitors/folio_migration/run.yml b/playbooks/roles/monitoring/tasks/monitors/folio_migration/run.yml
new file mode 100644
index 00000000..40ffc918
--- /dev/null
+++ b/playbooks/roles/monitoring/tasks/monitors/folio_migration/run.yml
@@ -0,0 +1,68 @@
+---
+# Folio migration monitoring startup tasks
+# Conditions are checked at import level in monitor_run.yml
+
+- name: Check if folio migration stats are available
+  become: true
+  become_method: sudo
+  ansible.builtin.stat:
+    path: /sys/kernel/debug/mm/migrate/stats
+  register: folio_migration_stats_file
+
+- name: Create monitoring directory
+  become: true
+  become_method: sudo
+  ansible.builtin.file:
+    path: /root/monitoring
+    state: directory
+    mode: "0755"
+  when: folio_migration_stats_file.stat.exists|default(false)
+
+- name: Start folio migration monitoring in background
+  become: true
+  become_method: sudo
+  ansible.builtin.shell: |
+    nohup bash -c 'while true; do
+      echo "$(date +"%Y-%m-%d %H:%M:%S")" >> /root/monitoring/folio_migration_stats.txt
+      cat /sys/kernel/debug/mm/migrate/stats >> /root/monitoring/folio_migration_stats.txt
+      echo "" >> /root/monitoring/folio_migration_stats.txt
+      sleep {{ monitor_folio_migration_interval|default(60) }}
+    done' > /root/monitoring/folio_migration.log 2>&1 &
+    echo $! > /root/monitoring/folio_migration.pid
+  async: 86400 # Run for up to 24 hours
+  poll: 0
+  register: folio_migration_monitor
+  when: folio_migration_stats_file.stat.exists|default(false)
+
+- name: Save async job ID for later termination
+  ansible.builtin.set_fact:
+    folio_migration_monitor_job: "{{ folio_migration_monitor.ansible_job_id }}"
+  when:
+    - folio_migration_stats_file.stat.exists|default(false)
+    - folio_migration_monitor is defined
+
+- name: Verify monitoring started successfully
+  become: true
+  become_method: sudo
+  ansible.builtin.shell: |
+    if [ -f /root/monitoring/folio_migration.pid ]; then
+      pid=$(cat /root/monitoring/folio_migration.pid)
+      if ps -p $pid > /dev/null 2>&1; then
+        echo "Monitoring process $pid is running"
+      else
+        echo "ERROR: Monitoring process $pid is not running" >&2
+        exit 1
+      fi
+    else
+      echo "ERROR: PID file not found" >&2
+      exit 1
+    fi
+  register: monitor_status
+  when: folio_migration_stats_file.stat.exists|default(false)
+
+- name: Display monitoring status
+  ansible.builtin.debug:
+    msg: "{{ monitor_status.stdout }}"
+  when:
+    - folio_migration_stats_file.stat.exists|default(false)
+    - monitor_status is defined
\ No newline at end of file

-- 
2.50.1