-
Notifications
You must be signed in to change notification settings - Fork 4
Dashboard integration - address coderabitai comments #93
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
0639bdc
53411d5
9bbaf34
653aed9
1b1ac3d
5bb7508
5159620
1657329
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -60,43 +60,59 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Auto-detect compose tool | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| block: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Check for podman-compose | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ansible.builtin.command: podman-compose --version | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| register: podman_compose_check | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| failed_when: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| changed_when: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Check for docker-compose | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ansible.builtin.command: docker-compose --version | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| register: docker_compose_check | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| failed_when: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| changed_when: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Check for podman-compose | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ansible.builtin.command: podman-compose --version | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| register: podman_compose_check | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Check for docker compose (CLI v2) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ansible.builtin.command: docker compose version | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| register: docker_cli_compose_check | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| failed_when: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| changed_when: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Set compose command (podman-compose) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ansible.builtin.set_fact: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| compose_cmd: podman-compose | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| runtime_name: Podman | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| when: podman_compose_check.rc == 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Set compose command (docker-compose) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ansible.builtin.set_fact: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| compose_cmd: docker-compose | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| runtime_name: Docker | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| when: docker_compose_check.rc == 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| when: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - podman_compose_check.rc != 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - docker_compose_check.rc == 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Set compose command (podman-compose) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Set compose command (docker compose CLI v2) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ansible.builtin.set_fact: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| compose_cmd: podman-compose | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| runtime_name: Podman | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| compose_cmd: docker compose | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| runtime_name: Docker | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| when: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - podman_compose_check.rc != 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - docker_compose_check.rc != 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - podman_compose_check.rc == 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - docker_cli_compose_check.rc == 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Fail if no compose tool found | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ansible.builtin.fail: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| msg: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Neither docker-compose nor podman-compose found! | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Neither podman-compose, docker-compose, nor docker compose found! | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| Install one of the following: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - Docker Desktop (includes docker-compose) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - Docker Desktop (includes docker-compose or docker compose) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - podman-compose: pip3 install podman-compose | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| when: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - docker_compose_check.rc != 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - podman_compose_check.rc != 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - docker_compose_check.rc != 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - docker_cli_compose_check.rc != 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| when: compose_cmd_override == "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| # ======================================================================== | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -240,11 +256,18 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||
| when: is_macos | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| register: datasource_update_macos | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Read current datasource version | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ansible.builtin.shell: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| grep -oP 'version: \K\d+' "{{ grafana_dir }}/provisioning/datasources/prometheus.yaml" || echo "1" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| register: current_version | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| changed_when: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| when: datasource_update_linux.changed or datasource_update_macos.changed | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Bump datasource version to force reload | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ansible.builtin.replace: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| path: "{{ grafana_dir }}/provisioning/datasources/prometheus.yaml" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| regexp: "version: \\d+" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| replace: "version: {{ 99 | random(start=10) }}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| replace: "version: {{ current_version.stdout | int + 1 }}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+259
to
+270
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify if the playbook is expected to run on macOS and check grep capabilities
# Check if there are any Darwin-specific conditions in the playbook
rg -n "Darwin|macos|macOS" automation/test-execution/ansible/start-grafana.yml
# Check if GNU grep might be installed via Homebrew on typical macOS setups
echo "---"
echo "Note: On macOS, GNU grep can be installed via 'brew install grep' as 'ggrep'"Repository: redhat-et/vllm-cpu-perf-eval Length of output: 1019 Use portable grep syntax to avoid BSD grep The Proposed fix - name: Read current datasource version
ansible.builtin.shell: |
- grep -oP 'version: \K\d+' "{{ grafana_dir }}/provisioning/datasources/prometheus.yaml" || echo "1"
+ grep -E 'version: [0-9]+' "{{ grafana_dir }}/provisioning/datasources/prometheus.yaml" | sed 's/.*version: //' | tr -d ' ' || echo "1"
register: current_version
changed_when: false
when: datasource_update_linux.changed or datasource_update_macos.changed📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| when: datasource_update_linux.changed or datasource_update_macos.changed | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Display datasource configuration | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -287,7 +310,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||
| register: container_logs | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| changed_when: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| failed_when: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| when: "'Up' not in container_status.stdout and 'running' not in container_status.stdout" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| when: compose_up.rc != 0 or "Exited" in container_status.stdout or ("Up" not in container_status.stdout and "running" not in container_status.stdout) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Display compose output | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ansible.builtin.debug: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -404,7 +427,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||
| block: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Check if tunnel already exists | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ansible.builtin.shell: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ps aux | grep "ssh.*8000:localhost:8000" | grep -v grep || true | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ps aux | grep "ssh.*8000:localhost:8000.*{{ dut_hostname }}" | grep -v grep || true | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| register: existing_tunnel | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| changed_when: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -425,7 +448,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Verify tunnel is active | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ansible.builtin.shell: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ps aux | grep "ssh.*8000:localhost:8000" | grep -v grep | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ps aux | grep "ssh.*8000:localhost:8000.*{{ dut_hostname }}" | grep -v grep | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| register: tunnel_status | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| changed_when: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| failed_when: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1247,7 +1247,7 @@ | |
| { | ||
| "datasource": { | ||
| "type": "prometheus", | ||
| "uid": "edx8memhpd9tsa" | ||
| "uid": "prometheus-vllm" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid hardcoded datasource UIDs in a variable-driven dashboard. These hardcoded entries bypass 🔧 Proposed fix- "uid": "prometheus-vllm"
+ "uid": "${DS_PROMETHEUS}"(Apply at Lines 1250, 1350, and 1463.) As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity." Also applies to: 1350-1350, 1463-1463 🤖 Prompt for AI Agents |
||
| }, | ||
| "disableTextWrap": false, | ||
| "editorMode": "code", | ||
|
|
@@ -1347,7 +1347,7 @@ | |
| { | ||
| "datasource": { | ||
| "type": "prometheus", | ||
| "uid": "edx8memhpd9tsa" | ||
| "uid": "prometheus-vllm" | ||
| }, | ||
| "disableTextWrap": false, | ||
| "editorMode": "code", | ||
|
|
@@ -1460,7 +1460,7 @@ | |
| { | ||
| "datasource": { | ||
| "type": "prometheus", | ||
| "uid": "edx8memhpd9tsa" | ||
| "uid": "prometheus-vllm" | ||
| }, | ||
| "disableTextWrap": false, | ||
| "editorMode": "code", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SSH tunnel pattern inconsistent with start-grafana.yml—may match unintended tunnels.
The pattern
ssh.*-L.*8000:localhost:8000here lacks hostname anchoring, whilestart-grafana.yml(lines 430, 451) usesssh.*8000:localhost:8000.*{{ dut_hostname }}. On systems with multiple SSH tunnels to different hosts, this could kill the wrong tunnel.Consider adding hostname anchoring for consistency:
Proposed fix
- name: Get tunnel PID ansible.builtin.shell: | - # Only kill SSH tunnel processes, not vLLM server or other services - pgrep -f "ssh.*-L.*8000:localhost:8000" | head -1 + # Only kill SSH tunnel processes for the DUT, not other tunnels + pgrep -f "ssh.*-L.*8000:localhost:8000.*{{ groups['dut'][0] }}" | head -1 register: tunnel_pid_check📝 Committable suggestion
🤖 Prompt for AI Agents