[PATCH 3/6] monitoring: restructure monitor_collect_only.yml to use modular architecture

From: Daniel Gomez <[email protected]>

Restructure the interim monitoring collection system to use the same
modular architecture as the main monitoring framework. This fixes
several issues:

- Replace hardcoded folio migration logic with modular import_tasks
pattern
- Fix broken matplotlib plotting tasks with proper parameters
- Eliminate code duplication by leveraging common/setup.yml
- Ensure consistent architecture across all monitoring tasks

The new structure mirrors the main monitoring system:
- monitors/folio_migration/collect_only.yml - Interim folio collection
- monitor_collect_only.yml - Orchestrator using import_tasks

This allows peeking at monitoring data during long-running tests without
stopping the monitoring services, now supporting all monitor types with
proper path resolution and plotting capabilities.

Generated-by: Claude AI
Signed-off-by: Daniel Gomez <[email protected]>
---
 .../monitoring/tasks/monitor_collect_only.yml      | 169 +++------------------
 playbooks/roles/monitoring/tasks/monitor_run.yml   |   2 +-
 .../tasks/monitors/folio_migration/collect.yml     |   2 +-
 .../monitors/folio_migration/collect_only.yml      |  91 +++++++++++
 .../tasks/monitors/folio_migration/run.yml         |   2 +-
 5 files changed, 114 insertions(+), 152 deletions(-)

diff --git a/playbooks/roles/monitoring/tasks/monitor_collect_only.yml b/playbooks/roles/monitoring/tasks/monitor_collect_only.yml
index 20e40a31..940661b6 100644
--- a/playbooks/roles/monitoring/tasks/monitor_collect_only.yml
+++ b/playbooks/roles/monitoring/tasks/monitor_collect_only.yml
@@ -1,164 +1,35 @@
 ---
-# Tasks to collect monitoring data WITHOUT stopping the monitoring services
+# Collect monitoring data WITHOUT stopping monitoring services
 # This allows peeking at intermediate results during long-running tests
 
-- name: Check if monitoring data exists (without stopping monitoring)
-  become: true
-  become_method: sudo
-  ansible.builtin.stat:
-    path: /root/monitoring/folio_migration_stats.txt
-  register: folio_migration_data_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
-
-# Note: We don't need matplotlib on target hosts - plots are generated on localhost
-
-- name: Create snapshot of monitoring data
-  become: true
-  become_method: sudo
-  ansible.builtin.shell: |
-    # Create a snapshot copy to avoid interfering with ongoing monitoring
-    cp /root/monitoring/folio_migration_stats.txt /root/monitoring/folio_migration_stats_snapshot.txt
-  args:
-    chdir: /root/monitoring
-  register: snapshot_creation
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - folio_migration_data_file.stat.exists|default(false)
-
-# Plot generation happens on localhost, not on target hosts
-
-- name: Debug monitoring collection status
-  ansible.builtin.debug:
-    msg: |
-      Collecting monitoring data (without stopping monitoring)
-      monitor_developmental_stats: {{ monitor_developmental_stats | default(false) }}
-      monitor_folio_migration: {{ monitor_folio_migration | default(false) }}
-      enable_monitoring: {{ enable_monitoring | default(false) }}
-      Data file exists: {{ folio_migration_data_file.stat.exists | 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:
-  delegate_to: localhost
-  become: false
-  run_once: true
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-
-- name: Check if snapshot was created
-  become: true
-  become_method: sudo
-  ansible.builtin.stat:
-    path: /root/monitoring/folio_migration_stats_snapshot.txt
-  register: folio_migration_snapshot_file
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-
-- name: Copy folio migration stats snapshot to localhost
-  become: true
-  become_method: sudo
-  ansible.builtin.fetch:
-    src: /root/monitoring/folio_migration_stats_snapshot.txt
-    dest: "{{ monitoring_results_path }}/{{ ansible_hostname }}_folio_migration_stats_interim.txt"
-    flat: true
-    validate_checksum: false
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - folio_migration_snapshot_file.stat.exists|default(false)
-
-# Plots are generated on localhost, not fetched from targets
-
-- name: Display interim monitoring data collection summary
-  ansible.builtin.debug:
-    msg: |
-      Interim folio migration monitoring data collected (monitoring still running).
-      Data saved to: {{ monitoring_results_path }}/{{ ansible_hostname }}_folio_migration_stats_interim.txt
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - folio_migration_snapshot_file.stat.exists|default(false)
-
-# Generate plots on localhost from the collected 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
-  changed_when: false
-  failed_when: false
-  run_once: true
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-
-- name: Collect all monitoring data files
-  ansible.builtin.find:
-  delegate_to: localhost
-  register: all_monitoring_files
-  run_once: true
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - localhost_matplotlib_check.rc == 0
+    - enable_monitoring|default(false)|bool
 
-- name: Generate A/B comparison plots for each configuration
-  ansible.builtin.command:
-  delegate_to: localhost
-  with_items: "{{ all_monitoring_files.files | map(attribute='path') | map('basename') | list }}"
-  register: ab_plot_generation
-  ignore_errors: true
-  run_once: true
+# Import folio migration interim collection tasks
+- ansible.builtin.import_tasks: monitors/folio_migration/collect_only.yml
   when:
     - monitor_developmental_stats|default(false)|bool
     - monitor_folio_migration|default(false)|bool
-    - localhost_matplotlib_check.rc == 0
-    - all_monitoring_files.files | length > 0
 
-- name: Generate comprehensive plot with all results
-  ansible.builtin.command:
-  delegate_to: localhost
-  register: comprehensive_plot_generation
-  ignore_errors: true
-  run_once: true
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-    - localhost_matplotlib_check.rc == 0
-
-- name: Log plot generation summary
+# Display comprehensive interim monitoring collection summary
+- name: Display interim monitoring collection summary
   ansible.builtin.debug:
     msg: |
-      {% if localhost_matplotlib_check.rc != 0 %}
-      Skipping plot generation - matplotlib not available on localhost
+      Interim monitoring data collection complete (monitoring still running).
+      Results available in: {{ monitoring_results_path }}
+      {% if monitor_developmental_stats|default(false)|bool and monitor_folio_migration|default(false)|bool %}
+      - Folio migration interim data collected
+      {% endif %}
+      {% if localhost_matplotlib_check is defined and localhost_matplotlib_check.rc is defined and localhost_matplotlib_check.rc == 0 %}
+      Plot generation status:
+      {% if folio_interim_plot_generation is defined and folio_interim_plot_generation.rc is defined and folio_interim_plot_generation.rc == 0 %}
+      - Folio migration interim plots generated
+      {% endif %}
       {% else %}
-      Plot generation complete:
-      - A/B comparison plots generated for each configuration pair
-      - Comprehensive plot generated: all_hosts_comprehensive.png
+      Plot generation skipped - matplotlib not available on localhost
       {% endif %}
   run_once: true
   when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-
-- name: Clean up snapshot files on target
-  become: true
-  become_method: sudo
-  ansible.builtin.file:
-    path: "{{ item }}"
-    state: absent
-  loop:
-    - /root/monitoring/folio_migration_stats_snapshot.txt
-    - /root/monitoring/folio_migration_plot_snapshot.png
-  when:
-    - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
-  ignore_errors: true
+    - enable_monitoring|default(false)|bool
diff --git a/playbooks/roles/monitoring/tasks/monitor_run.yml b/playbooks/roles/monitoring/tasks/monitor_run.yml
index a2055f89..9b1c056c 100644
--- a/playbooks/roles/monitoring/tasks/monitor_run.yml
+++ b/playbooks/roles/monitoring/tasks/monitor_run.yml
@@ -5,4 +5,4 @@
 - ansible.builtin.import_tasks: monitors/folio_migration/run.yml
   when:
     - monitor_developmental_stats|default(false)|bool
-    - monitor_folio_migration|default(false)|bool
\ No newline at end of file
+    - monitor_folio_migration|default(false)|bool
diff --git a/playbooks/roles/monitoring/tasks/monitors/folio_migration/collect.yml b/playbooks/roles/monitoring/tasks/monitors/folio_migration/collect.yml
index bd04ebe2..a0467bd1 100644
--- a/playbooks/roles/monitoring/tasks/monitors/folio_migration/collect.yml
+++ b/playbooks/roles/monitoring/tasks/monitors/folio_migration/collect.yml
@@ -144,4 +144,4 @@
       {% 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
+  when: folio_migration_data_file.stat.exists|default(false)
diff --git a/playbooks/roles/monitoring/tasks/monitors/folio_migration/collect_only.yml b/playbooks/roles/monitoring/tasks/monitors/folio_migration/collect_only.yml
new file mode 100644
index 00000000..7696cc99
--- /dev/null
+++ b/playbooks/roles/monitoring/tasks/monitors/folio_migration/collect_only.yml
@@ -0,0 +1,91 @@
+---
+# Folio migration interim data collection (without stopping monitoring)
+- name: Check if folio migration monitoring data exists
+  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: Create folio migration snapshot for interim collection
+  become: true
+  become_method: sudo
+  ansible.builtin.shell: |
+    cp /root/monitoring/folio_migration_stats.txt /root/monitoring/folio_migration_stats_snapshot.txt
+  when:
+    - monitor_developmental_stats|default(false)|bool
+    - monitor_folio_migration|default(false)|bool
+    - folio_migration_data_file.stat.exists|default(false)
+
+- name: Copy folio migration interim data to localhost
+  become: true
+  become_method: sudo
+  ansible.builtin.fetch:
+    src: /root/monitoring/folio_migration_stats_snapshot.txt
+    dest: "{{ monitoring_results_path }}/{{ ansible_hostname }}_folio_migration_stats_interim.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: Clean up folio migration snapshot
+  become: true
+  become_method: sudo
+  ansible.builtin.file:
+    path: /root/monitoring/folio_migration_stats_snapshot.txt
+    state: absent
+  when:
+    - monitor_developmental_stats|default(false)|bool
+    - monitor_folio_migration|default(false)|bool
+  ignore_errors: true
+
+- name: Display folio migration interim collection status
+  ansible.builtin.debug:
+    msg: |
+      Folio migration interim data {% if folio_migration_data_file.stat.exists|default(false) %}collected{% else %}not available{% endif %}.
+      {% if folio_migration_data_file.stat.exists|default(false) %}
+      Data saved to: {{ monitoring_results_path }}/{{ ansible_hostname }}_folio_migration_stats_interim.txt
+      {% endif %}
+  when:
+    - monitor_developmental_stats|default(false)|bool
+    - monitor_folio_migration|default(false)|bool
+
+# Generate plots from interim data on localhost
+- 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:
+    - enable_monitoring|default(false)|bool
+
+- name: Find interim monitoring data files for plotting
+  ansible.builtin.find:
+    paths: "{{ monitoring_results_path }}"
+    patterns: "*_interim.txt"
+  delegate_to: localhost
+  register: interim_monitoring_files
+  run_once: true
+  when:
+    - enable_monitoring|default(false)|bool
+    - localhost_matplotlib_check.rc == 0
+
+- name: Generate folio migration interim plots on localhost
+  ansible.builtin.command: |
+    python3 {{ playbook_dir }}/roles/monitoring/files/plot_folio_migration_stats.py
+      -o {{ monitoring_results_path }}/interim_folio_migration_plot.png
+      {{ interim_monitoring_files.files | selectattr('path', 'match', '.*folio_migration.*') | map(attribute='path') | join(' ') }}
+  delegate_to: localhost
+  register: folio_interim_plot_generation
+  ignore_errors: true
+  run_once: true
+  when:
+    - enable_monitoring|default(false)|bool
+    - localhost_matplotlib_check.rc == 0
+    - interim_monitoring_files.files | selectattr('path', 'match', '.*folio_migration.*') | list | length > 0
diff --git a/playbooks/roles/monitoring/tasks/monitors/folio_migration/run.yml b/playbooks/roles/monitoring/tasks/monitors/folio_migration/run.yml
index 40ffc918..f38f54f1 100644
--- a/playbooks/roles/monitoring/tasks/monitors/folio_migration/run.yml
+++ b/playbooks/roles/monitoring/tasks/monitors/folio_migration/run.yml
@@ -65,4 +65,4 @@
     msg: "{{ monitor_status.stdout }}"
   when:
     - folio_migration_stats_file.stat.exists|default(false)
-    - monitor_status is defined
\ No newline at end of file
+    - monitor_status is defined

-- 
2.50.1