fix: address CodeRabbit review comments and fetch directory issue

- Fix vllm CLI flag format from --no_enable_prefix_caching to --no-enable-prefix-caching
- Improve SSH security by using StrictHostKeyChecking=accept-new instead of no
- Add API key source resolution (env_var, file_path, vault_var, prompt) with no_log protection
- Add test_type validation in benchmark_embedding role
- Mask HF_TOKEN in logs for benchmark_guidellm host execution
- Add collection_id partition to results path for run-level organization
- Replace ansible.builtin.fetch with ansible.posix.synchronize for directory transfers

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Maryam Tahhan <mtahhan@redhat.com>

Author: maryamtahhan

automation/test-execution/ansible/embedding-core-sweep.yml

@@ -51,10 +51,11 @@
 
   tasks:
     - name: Fetch all core sweep results
-      ansible.builtin.fetch:
+      ansible.posix.synchronize:
         src: "{{ bench_config.results_dir }}/core-sweep/"
         dest: "{{ playbook_dir }}/../../results/embedding-models/core-sweep-{{ test_run_id }}/"
-        flat: false
+        mode: pull
+        recursive: true
 
     - name: Display results location
       ansible.builtin.debug:

automation/test-execution/ansible/inventory/group_vars/all/infrastructure.yml

@@ -6,7 +6,7 @@
 # These affect all test runs regardless of model or workload type
 
 # SSH configuration
-ansible_ssh_common_args: "-o StrictHostKeyChecking=no"
+ansible_ssh_common_args: "-o StrictHostKeyChecking=accept-new"
 
 # Platform setup options
 platform_setup:

automation/test-execution/ansible/inventory/group_vars/all/test-workloads.yml

@@ -31,7 +31,7 @@ test_configs:
     backend: "openai-completions"
     vllm_args:
       - "--dtype=bfloat16"
-      - "--no_enable_prefix_caching"
+      - "--no-enable-prefix-caching"
     kv_cache_space: "40GiB"
 
   # Chat workload (short input, short output)
@@ -42,7 +42,7 @@ test_configs:
     backend: "openai-chat"
     vllm_args:
       - "--dtype=bfloat16"
-      - "--no_enable_prefix_caching"
+      - "--no-enable-prefix-caching"
     kv_cache_space: "40GiB"
 
   # Code generation workload (long input, long output)
@@ -53,7 +53,7 @@ test_configs:
     backend: "openai-completions"
     vllm_args:
       - "--dtype=bfloat16"
-      - "--no_enable_prefix_caching"
+      - "--no-enable-prefix-caching"
     kv_cache_space: "60GiB"
 
   # RAG workload (very long input, short output)
@@ -64,5 +64,5 @@ test_configs:
     backend: "openai-completions"
     vllm_args:
       - "--dtype=bfloat16"
-      - "--no_enable_prefix_caching"
+      - "--no-enable-prefix-caching"
     kv_cache_space: "80GiB"

automation/test-execution/ansible/llm-benchmark-auto.yml

@@ -235,10 +235,11 @@
         dest: "{{ bench_config.results_dir }}/{{ test_model | replace('/', '__') }}/{{ workload_type }}/{{ core_configuration.name }}/test-metadata.json"
 
     - name: Fetch results to local machine
-      ansible.builtin.fetch:
+      ansible.posix.synchronize:
         src: "{{ bench_config.results_dir }}/{{ test_model | replace('/', '__') }}/{{ workload_type }}/{{ core_configuration.name }}/"
         dest: "{{ playbook_dir }}/../../../results/llm/{{ test_model | replace('/', '__') }}/{{ workload_type }}-{{ test_run_id }}/"
-        flat: false
+        mode: pull
+        recursive: true
 
     - name: Display results location
       ansible.builtin.debug:

automation/test-execution/ansible/llm-core-sweep-auto.yml

@@ -178,10 +178,11 @@
   tasks:
     - block:
         - name: Fetch all core sweep results
-          ansible.builtin.fetch:
+          ansible.posix.synchronize:
             src: "{{ bench_config.results_dir }}/{{ test_model | replace('/', '__') }}/{{ workload_type }}/"
             dest: "{{ playbook_dir }}/../../../results/llm/{{ test_model | replace('/', '__') }}/{{ workload_type }}-{{ test_run_id }}/"
-            flat: false
+            mode: pull
+            recursive: true
 
         - name: Display results location
           ansible.builtin.debug:

automation/test-execution/ansible/llm-core-sweep.yml

@@ -80,10 +80,11 @@
 
   tasks:
     - name: Fetch all core sweep results
-      ansible.builtin.fetch:
+      ansible.posix.synchronize:
         src: "{{ bench_config.results_dir }}/{{ test_model | replace('/', '__') }}/{{ workload_type }}/"
         dest: "{{ playbook_dir }}/../../../results/llm/{{ test_model | replace('/', '__') }}/{{ workload_type }}-{{ test_run_id }}/"
-        flat: false
+        mode: pull
+        recursive: true
 
     - name: Display results location
       ansible.builtin.debug:

automation/test-execution/ansible/playbooks/common/tasks/setup-vllm-api-key.yml

@@ -6,18 +6,76 @@
 - name: Setup vLLM API key for external endpoints
   when: vllm_endpoint.external.api_key.enabled | default(false) | bool
   block:
+    - name: Get API key from environment variable
+      ansible.builtin.set_fact:
+        vllm_api_key_resolved: "{{ lookup('env', vllm_endpoint.external.api_key.env_var) }}"
+      when:
+        - vllm_endpoint.external.api_key.source | default('value') == 'env_var'
+        - vllm_endpoint.external.api_key.env_var is defined
+      no_log: true
+
+    - name: Get API key from file
+      ansible.builtin.slurp:
+        src: "{{ vllm_endpoint.external.api_key.file_path }}"
+      register: vllm_api_key_file_content
+      when:
+        - vllm_endpoint.external.api_key.source | default('value') == 'file_path'
+        - vllm_endpoint.external.api_key.file_path is defined
+      no_log: true
+
+    - name: Set API key from file content
+      ansible.builtin.set_fact:
+        vllm_api_key_resolved: "{{ vllm_api_key_file_content.content | b64decode | trim }}"
+      when:
+        - vllm_endpoint.external.api_key.source | default('value') == 'file_path'
+        - vllm_api_key_file_content is defined
+      no_log: true
+
+    - name: Get API key from Ansible vault
+      ansible.builtin.set_fact:
+        vllm_api_key_resolved: "{{ vllm_endpoint.external.api_key.vault_var }}"
+      when:
+        - vllm_endpoint.external.api_key.source | default('value') == 'vault_var'
+        - vllm_endpoint.external.api_key.vault_var is defined
+      no_log: true
+
+    - name: Prompt for API key
+      ansible.builtin.pause:
+        prompt: "Enter your vLLM API key"
+        echo: false
+      register: vllm_api_key_prompt
+      when:
+        - vllm_endpoint.external.api_key.source | default('value') == 'prompt'
+      no_log: true
+
+    - name: Set API key from prompt
+      ansible.builtin.set_fact:
+        vllm_api_key_resolved: "{{ vllm_api_key_prompt.user_input }}"
+      when:
+        - vllm_endpoint.external.api_key.source | default('value') == 'prompt'
+        - vllm_api_key_prompt is defined
+      no_log: true
+
+    - name: Use direct value if no source specified
+      ansible.builtin.set_fact:
+        vllm_api_key_resolved: "{{ vllm_endpoint.external.api_key.value }}"
+      when:
+        - vllm_endpoint.external.api_key.source | default('value') == 'value'
+        - vllm_endpoint.external.api_key.value is defined
+      no_log: true
+
     - name: Validate API key configuration
       ansible.builtin.assert:
         that:
-          - vllm_endpoint.external.api_key.value is defined
-          - vllm_endpoint.external.api_key.value is not none
-          - vllm_endpoint.external.api_key.value | length > 0
+          - vllm_api_key_resolved is defined
+          - vllm_api_key_resolved is not none
+          - vllm_api_key_resolved | length > 0
         fail_msg: "API key value is required when api_key.enabled is true"
         success_msg: "API key configuration validated"
 
     - name: Set vLLM API key fact
       ansible.builtin.set_fact:
-        vllm_api_key: "{{ vllm_endpoint.external.api_key.value }}"
+        vllm_api_key: "{{ vllm_api_key_resolved }}"
       no_log: true
 
     - name: Display API key status

automation/test-execution/ansible/roles/benchmark_embedding/tasks/main.yml

@@ -2,6 +2,14 @@
 # Benchmark Embedding Role - Main Entry Point
 # Runs embedding model benchmarks using vllm-bench
 
+- name: Validate test_type
+  ansible.builtin.assert:
+    that:
+      - test_type is defined
+      - test_type in ['baseline', 'latency', 'all']
+    fail_msg: "Invalid test_type '{{ test_type | default('undefined') }}'. Must be one of: baseline, latency, all"
+    success_msg: "test_type validated: {{ test_type }}"
+
 - name: Run baseline performance test (throughput sweep)
   ansible.builtin.include_tasks: baseline.yml
   when: test_type == 'baseline' or test_type == 'all'

automation/test-execution/ansible/roles/benchmark_guidellm/tasks/main.yml

@@ -233,6 +233,7 @@
   register: guidellm_host_result
   changed_when: true
   failed_when: false
+  no_log: true
   when: not (use_guidellm_container | bool)
 
 # ============================================================================

automation/test-execution/ansible/roles/results_collector/tasks/collect-test-results.yml

@@ -13,11 +13,11 @@
 - name: Fetch test results to controller
   ansible.posix.synchronize:
     src: "{{ bench_config.results_dir }}/"
-    dest: "{{ playbook_dir }}/../../../results/benchmark-results/{{ inventory_hostname }}/"
+    dest: "{{ playbook_dir }}/../../../results/benchmark-results/{{ inventory_hostname }}/{{ collection_id }}/"
     mode: pull
     recursive: true
   when: results_dir_stat.stat.exists
 
 - name: Display results collection status
   ansible.builtin.debug:
-    msg: "{{ 'Results fetched from ' + bench_config.results_dir + ' to results/benchmark-results/' + inventory_hostname + '/' if results_dir_stat.stat.exists else 'No results directory found' }}"
+    msg: "{{ 'Results fetched from ' + bench_config.results_dir + ' to results/benchmark-results/' + inventory_hostname + '/' + collection_id + '/' if results_dir_stat.stat.exists else 'No results directory found' }}"