Conversation
Signed-off-by: Jooho Lee <jlee@redhat.com>
WalkthroughThis update introduces a comprehensive set of Kubernetes manifests, scripts, and configuration files to automate the deployment and testing of the LLM-D (Large Language Model - Distributed) system on KIND clusters. It adds roles, playbooks, and supporting scripts for deploying LLM-D components, including Redis-backed KVCache, scheduler, decode, prefill, routing, and simulator-based deployments, along with smoke test scripts and deployment documentation. Changes
Sequence Diagram(s)LLM-D Deployment and Test FlowsequenceDiagram
participant User
participant main.sh
participant Kubernetes
participant test-request.sh
User->>main.sh: Execute deployment script
main.sh->>Kubernetes: Apply CRDs and manifests (KVCache, Scheduler, Prefill, Decode, Route)
main.sh->>Kubernetes: Wait for pods to be ready
main.sh->>test-request.sh: Run smoke test script
test-request.sh->>Kubernetes: Discover decode pod/gateway
test-request.sh->>Kubernetes: Send test requests (GET/POST)
test-request.sh->>main.sh: Return test result
main.sh->>User: Report deployment and test status
Playbook Flow for LLM-D on KINDsequenceDiagram
participant User
participant Playbook
participant kind-install role
participant llm-d-install-with-manifests role
User->>Playbook: Run llm-d-on-kind playbook
Playbook->>kind-install role: Setup KIND cluster
Playbook->>llm-d-install-with-manifests role: Deploy LLM-D components with manifests
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 31
♻️ Duplicate comments (4)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/route/httproute.yaml (1)
9-9: Invalid namespace placeholder.The
%test_namespace%token isn't valid YAML and will cause a parsing error. Please apply the same fix as indecode/svc.yaml.unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/kvcache/svc.yaml (1)
10-10: Invalid namespace placeholder.The
%test_namespace%token isn't valid YAML and will cause a parsing error. Please apply the same fix as indecode/svc.yaml.unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests_using_simulator/decode/svc.yaml (1)
5-5: Invalid namespace placeholder.The
%test_namespace%token isn't valid YAML and will cause a parsing error. Please apply the same fix as indecode/svc.yaml.unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/route/inferencePool.yaml (1)
8-8: Invalid namespace placeholder.The
%test_namespace%token isn't valid YAML and will cause a parsing error. Please apply the same fix as indecode/svc.yaml.
🧹 Nitpick comments (22)
hacks/download-cli.sh (1)
54-61: Add broader architecture support and consistent error handlingConsider extending the
casestatement to cover additional architectures (e.g.,armv7l,ppc64le) and consolidating error messaging by invoking adiehelper (instead ofecho; exit) for consistent logging and exit codes.unofficial_components/playbooks/llm-d-on-kind/config.yaml (1)
11-11: Add missing newline at end-of-file and remove trailing spaces.YAML lint reports no newline at EOF and trailing spaces on the final blank line.
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/kvcache/redis-script-configmap.yaml (1)
18-20: Include ACL file in Redis startup arguments.Although you copy
users.aclif present, it's not passed toredis-server. Consider adding:ARGS+=("--include" "/opt/bitnami/redis/etc/master.conf") +ARGS+=("--include" "/opt/bitnami/redis/etc/users.acl")This ensures ACL rules are loaded.
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests_using_simulator/prefill/deployment.yaml (1)
26-26: Remove server-assigned field.
creationTimestamp: nullis a server-populated field and should be omitted from the spec.unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/kvcache/redis-configuration-configmap.yaml (1)
30-30: Remove superfluous blank line.YAMLlint warns about extra blank lines; clean up to adhere to style guidelines.
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/scheduler/role.yaml (1)
46-46: Add trailing newline at end-of-file.YAMLlint flags the missing newline at EOF. Please ensure the file ends with a single newline character.
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests_using_simulator/decode/deployment.yaml (1)
76-77: Remove deprecatedserviceAccountfield.You’re specifying both
serviceAccountandserviceAccountName. The former is deprecated—keep onlyserviceAccountName:- serviceAccount: meta-llama-llama-3-2-3b-instruct-sa serviceAccountName: meta-llama-llama-3-2-3b-instruct-saunofficial_components/roles/poc-llm-d-install-with-manifests/files/test-request.sh (1)
226-227: Ensure cleanup on script exitThe final pod deletion may be skipped on early exit. Add a trap to remove the curl pod on any exit or interruption.
trap 'kubectl delete pod curl-$ID -n "$NAMESPACE" --force --grace-period=0' EXITunofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests_using_simulator/README.md (3)
4-8: Improve prerequisites list formattingItems under Download required binaries are not consistently formatted as a list. Use proper Markdown bullets:
- yq - kubectl or oc
9-13: Clarify the$MODEvariable usageThe snippets reference
$MODE(e.g.,kubectl $MODE -k ...) but it isn’t defined. Document acceptable values (e.g.,apply/delete) or remove it to avoid confusion.
11-14: Use proper Markdown code fencesReplace the literal
~~~with triple backticks for valid code blocks.- ~~~ - kubectl $MODE -k https://... - ~~~ +```bash +kubectl $MODE -k https://... +```unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/prefill/deployment.yaml (1)
41-43: Use block scalar for JSON configEmbedding complex JSON on one line is hard to maintain. Refactor to a block scalar:
- --kv-transfer-config - '{"kv_connector":"MultiConnector", ...}' + --kv-transfer-config + | + { + "kv_connector": "MultiConnector", + "kv_role": "kv_both", + "kv_connector_extra_config": { ... } + }unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/README.md (3)
29-34: Use proper Markdown code fencesThe snippet uses
~~~; switch to standard backticks for code blocks.- ~~~ - oc new-project llmd-test-manual - ~~~ +```bash +oc new-project llmd-test-manual +```
42-53: Inconsistent manifest path referencesThe Routing section refers to
../populated_manifests/${component}, whereas other sections use./${component}. Align these paths to the correct relative directory.
121-124: Incorrect namespace placeholder in test snippetThe test output includes
%test_namespace%; replace with a real namespace or use an environment variable placeholder (${TEST_NAMESPACE}).unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/scheduler/deployment.yaml (1)
126-127: Remove deprecatedserviceAccountfieldThe
serviceAccountfield is deprecated in favor ofserviceAccountName. Remove it to avoid redundancy:- serviceAccount: meta-llama-llama-3-2-3b-instruct-epp-saunofficial_components/roles/poc-llm-d-install-with-manifests/main.sh (2)
1-5: Enable strict mode and quote variable expansions
Addset -euo pipefailat the top and quote all$VARexpansions (e.g., incp,find,source, andoccommands) to harden the script and avoid word-splitting.
27-53: Remove or use unused variables
Variablesrole_name,prefill_manifests_dir_path,decode_manifests_dir_path, andPORTare assigned but never used. Either utilize them or delete these assignments to clear ShellCheck warnings.unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/llm-d-llmIsvcConfig.yaml (1)
148-150: Trim excessive blank lines at EOF
YAML files should end with a single newline. Remove the extra blank lines after line 148.unofficial_components/roles/poc-llm-d-install-with-manifests/config.yaml (3)
13-13: Remove trailing spaces
Line 13 has trailing whitespace. Delete the extra spaces to satisfy YAML linting.
16-17: Fix typo and spacing in description
Correct “ofLLM-D” → “of LLM-D” and “manfiests” → “manifests” for clarity.- This role is used to install Prefill/Decode type deployment ofLLM-D using llm-d-simulator. + This role deploys a Prefill/Decode LLM-D setup using llm-d-simulator.
80-80: Ensure newline at end of file
Add a single newline character at the end of the file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
hacks/download-cli.sh(2 hunks)unofficial_components/playbooks/llm-d-on-kind/config.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/config.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/files/test-request.sh(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/main.sh(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/README.md(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/decode/deployment.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/decode/sa.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/decode/svc.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/kvcache/deployment.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/kvcache/redis-configuration-configmap.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/kvcache/redis-health-configmap.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/kvcache/redis-script-configmap.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/kvcache/sa.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/kvcache/svc.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/llm-d-llmIsvc.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/llm-d-llmIsvcConfig.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/prefill/deployment.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/prefill/svc.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/route/gateway.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/route/httproute.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/route/inferenceModel.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/route/inferencePool.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/scheduler/deployment.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/scheduler/destinationRule.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/scheduler/role.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/scheduler/rolebinding.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/scheduler/sa.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/scheduler/svc.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests_using_simulator/README.md(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests_using_simulator/decode/deployment.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests_using_simulator/decode/sa.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests_using_simulator/decode/svc.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests_using_simulator/prefill/deployment.yaml(1 hunks)unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests_using_simulator/prefill/svc.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/scheduler/svc.yaml
[error] 8-8: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/kvcache/sa.yaml
[error] 9-9: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/scheduler/destinationRule.yaml
[error] 5-5: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests_using_simulator/decode/sa.yaml
[error] 5-5: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/playbooks/llm-d-on-kind/config.yaml
[error] 11-11: no new line character at the end of file
(new-line-at-end-of-file)
[error] 11-11: trailing spaces
(trailing-spaces)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/kvcache/redis-script-configmap.yaml
[error] 27-27: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/route/httproute.yaml
[error] 9-9: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/scheduler/sa.yaml
[error] 5-5: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/prefill/svc.yaml
[error] 5-5: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/route/inferencePool.yaml
[error] 8-8: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/kvcache/redis-configuration-configmap.yaml
[warning] 30-30: too many blank lines
(1 > 0) (empty-lines)
[error] 29-29: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests_using_simulator/decode/svc.yaml
[error] 5-5: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/decode/svc.yaml
[error] 5-5: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests_using_simulator/prefill/svc.yaml
[error] 5-5: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/decode/sa.yaml
[error] 5-5: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/kvcache/svc.yaml
[error] 10-10: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/llm-d-llmIsvc.yaml
[error] 5-5: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests_using_simulator/decode/deployment.yaml
[error] 9-9: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/scheduler/rolebinding.yaml
[error] 5-5: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/kvcache/redis-health-configmap.yaml
[error] 103-103: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/scheduler/role.yaml
[error] 46-46: no new line character at the end of file
(new-line-at-end-of-file)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests_using_simulator/prefill/deployment.yaml
[error] 9-9: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/route/gateway.yaml
[error] 13-13: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/route/inferenceModel.yaml
[error] 8-8: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/prefill/deployment.yaml
[error] 9-9: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/scheduler/deployment.yaml
[error] 10-10: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/llm-d-llmIsvcConfig.yaml
[error] 5-5: syntax error: found character '%' that cannot start any token
(syntax)
[warning] 149-149: too many blank lines
(3 > 2) (empty-lines)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/decode/deployment.yaml
[error] 9-9: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/kvcache/deployment.yaml
[error] 12-12: syntax error: found character '%' that cannot start any token
(syntax)
unofficial_components/roles/poc-llm-d-install-with-manifests/config.yaml
[error] 13-13: trailing spaces
(trailing-spaces)
[error] 63-63: trailing spaces
(trailing-spaces)
[error] 74-74: trailing spaces
(trailing-spaces)
[error] 75-75: trailing spaces
(trailing-spaces)
[error] 77-77: trailing spaces
(trailing-spaces)
[error] 80-80: no new line character at the end of file
(new-line-at-end-of-file)
[error] 80-80: trailing spaces
(trailing-spaces)
🪛 Shellcheck (0.10.0)
unofficial_components/roles/poc-llm-d-install-with-manifests/main.sh
[warning] 28-28: role_name appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 50-50: prefill_manifests_dir_path appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 52-52: decode_manifests_dir_path appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 53-53: PORT appears unused. Verify use (or export if used externally).
(SC2034)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: kind-cluster-lifecycle-test (3.12)
- GitHub Check: cluster-test (3.12)
- GitHub Check: non-cluster-test (3.12)
🔇 Additional comments (1)
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/decode/deployment.yaml (1)
38-40: Verify containerPort matches the--portargument
The container is launched with--port 8001but only ports 5557 and 80 are exposed. Confirm whether port 80 is intentional; otherwise, add or replace acontainerPort: 8001entry so Kubernetes networking aligns with the service port.Also applies to: 91-92
| # Install Helm | ||
| HELM_VERSION="v3.17.3" | ||
| HELM_TARBALL="helm-${HELM_VERSION}-${OS}-${ARCH}.tar.gz" | ||
|
|
||
| if ! check_binary_exists "${root_directory}/bin/helm"; then | ||
| echo "⬇️ Downloading HELM (v${HELM_VERSION})..." | ||
| wget --progress=bar:force:noscroll "https://get.helm.sh/${HELM_TARBALL}" | ||
| tar -zxvf "${HELM_TARBALL}" | ||
| delete_if_exists "${root_directory}/bin/helm" | ||
| mv "${OS}-${ARCH}/helm" "${root_directory}/bin/helm" | ||
| chmod +x "${root_directory}/bin/helm" | ||
| rm -rf "${OS}-${ARCH}" "${HELM_TARBALL}" | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Verify Helm download integrity
It’s best practice to validate the Helm tarball (e.g., by checking its SHA256 checksum or GPG signature) before extraction to prevent installation of corrupted or malicious binaries. Consider fetching and verifying the checksum or signature alongside the download.
🤖 Prompt for AI Agents
In hacks/download-cli.sh around lines 159 to 171, the Helm tarball is downloaded
and extracted without verifying its integrity. To fix this, add steps to
download the official SHA256 checksum or GPG signature for the Helm tarball,
then verify the downloaded file against this checksum or signature before
extracting it. If the verification fails, abort the installation to prevent
using corrupted or malicious binaries.
| kind: ServiceAccount | ||
| metadata: | ||
| name: meta-llama-llama-3-2-3b-instruct-sa | ||
| namespace: %test_namespace% |
There was a problem hiding this comment.
Invalid namespace placeholder syntax
namespace: %test_namespace% is not valid YAML. Use a templating syntax supported by your deployment tooling (e.g., {{ test_namespace }} for Ansible or ${TEST_NAMESPACE} for envsubst) or document how %test_namespace% is substituted.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 5-5: syntax error: found character '%' that cannot start any token
(syntax)
🤖 Prompt for AI Agents
In
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests_using_simulator/decode/sa.yaml
at line 5, the namespace placeholder uses invalid syntax `%test_namespace%`
which is not valid YAML. Replace this with a valid templating syntax supported
by your deployment tool, such as `{{ test_namespace }}` for Ansible or
`${TEST_NAMESPACE}` for envsubst, or add documentation explaining how
`%test_namespace%` is substituted during deployment.
| kind: ServiceAccount | ||
| metadata: | ||
| name: meta-llama-llama-3-2-3b-instruct-epp-sa | ||
| namespace: %test_namespace% |
There was a problem hiding this comment.
Invalid namespace placeholder syntax
namespace: %test_namespace% will break YAML parsing. Replace with a valid template or environment variable reference (e.g., {{ test_namespace }} or ${TEST_NAMESPACE}).
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 5-5: syntax error: found character '%' that cannot start any token
(syntax)
🤖 Prompt for AI Agents
In
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/scheduler/sa.yaml
at line 5, the namespace placeholder uses invalid syntax `%test_namespace%`
which breaks YAML parsing. Replace this with a valid template or environment
variable syntax such as `{{ test_namespace }}` or `${TEST_NAMESPACE}` to ensure
proper parsing and substitution.
| kind: ServiceAccount | ||
| metadata: | ||
| name: meta-llama-llama-3-2-3b-instruct-sa | ||
| namespace: %test_namespace% |
There was a problem hiding this comment.
Invalid namespace placeholder syntax
The %test_namespace% placeholder is not YAML-compliant. Switch to a supported variable syntax (e.g., Ansible’s {{ test_namespace }} or shell’s ${TEST_NAMESPACE}) or clarify the substitution mechanism.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 5-5: syntax error: found character '%' that cannot start any token
(syntax)
🤖 Prompt for AI Agents
In
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/decode/sa.yaml
at line 5, the namespace placeholder `%test_namespace%` uses invalid syntax for
YAML. Replace it with a valid variable substitution format such as Ansible's `{{
test_namespace }}` or shell's `${TEST_NAMESPACE}`, depending on the intended
templating system, to ensure proper parsing and substitution.
| kind: DestinationRule | ||
| metadata: | ||
| name: meta-llama-llama-3-2-3b-instruct-insecure-tls | ||
| namespace: %test_namespace% |
There was a problem hiding this comment.
Invalid namespace placeholder syntax
namespace: %test_namespace% isn’t valid YAML. Use a proper templating expression ({{ test_namespace }} or ${TEST_NAMESPACE}) or explain how %test_namespace% is replaced at runtime.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 5-5: syntax error: found character '%' that cannot start any token
(syntax)
🤖 Prompt for AI Agents
In
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/scheduler/destinationRule.yaml
at line 5, the namespace placeholder `%test_namespace%` is not valid YAML
syntax. Replace `%test_namespace%` with a proper templating expression such as
`{{ test_namespace }}` or `${TEST_NAMESPACE}`, depending on the templating
engine used, or add a comment explaining how `%test_namespace%` is replaced at
runtime to clarify its usage.
| app.kubernetes.io/name: redis | ||
| app.kubernetes.io/version: 7.4.3 | ||
| name: llm-d-redis-master | ||
| namespace: %test_namespace% |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Quote the namespace placeholder to satisfy YAML syntax
The raw %test_namespace% token on line 12 will trigger a YAML parser error. Wrap it in quotes so that the sed replacement still works and the manifest remains valid YAML.
Apply this diff:
- namespace: %test_namespace%
+ namespace: "%test_namespace%"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| namespace: %test_namespace% | |
| namespace: "%test_namespace%" |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 12-12: syntax error: found character '%' that cannot start any token
(syntax)
🤖 Prompt for AI Agents
In
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/kvcache/deployment.yaml
at line 12, the namespace placeholder %test_namespace% is not quoted, causing
YAML syntax errors. Fix this by enclosing %test_namespace% in double quotes to
ensure the YAML parser accepts it and the sed replacement works correctly.
| if [[ $DEBUG == "0" ]]; then | ||
| set -x | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Invert debug flag logic
Currently, debugging (set -x) is enabled when DEBUG == "0", which is counterintuitive. Swap the condition so set -x runs when debug is explicitly turned on (e.g. DEBUG=1).
Proposed change:
-if [[ $DEBUG == "0" ]]; then
- set -x
-fi
+if [[ "${DEBUG}" == "1" ]]; then
+ set -x
+fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [[ $DEBUG == "0" ]]; then | |
| set -x | |
| fi | |
| if [[ "${DEBUG}" == "1" ]]; then | |
| set -x | |
| fi |
🤖 Prompt for AI Agents
In unofficial_components/roles/poc-llm-d-install-with-manifests/main.sh around
lines 3 to 5, the debug flag logic is inverted causing set -x to run when DEBUG
is "0". Change the condition to enable set -x only when DEBUG is explicitly set
to "1" or a truthy value, so debugging activates when intended.
| llm-d.ai/model: meta-llama-llama-3-2-3b-instruct | ||
| llm-d.ai/role: decode | ||
| name: meta-llama-llama-3-2-3b-instruct-decode | ||
| namespace: %test_namespace% |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Quote the namespace placeholder to satisfy YAML syntax
The unquoted %test_namespace% token on line 9 breaks standard YAML parsing. Wrap it in quotes before performing the sed substitution.
- namespace: %test_namespace%
+ namespace: "%test_namespace%"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| namespace: %test_namespace% | |
| namespace: "%test_namespace%" |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 9-9: syntax error: found character '%' that cannot start any token
(syntax)
🤖 Prompt for AI Agents
In
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/decode/deployment.yaml
at line 9, the namespace placeholder %test_namespace% is unquoted, causing YAML
parsing errors. Fix this by enclosing %test_namespace% in double quotes to
ensure valid YAML syntax before any sed substitution is applied.
| - name: LMCACHE_LOOKUP_URL | ||
| value: llm-d-redis-master.llmd-test.svc.cluster.local:8100 | ||
| - name: HF_HUB_CACHE | ||
| value: /models |
There was a problem hiding this comment.
Hard-coded namespace in Redis addresses
Values like llm-d-redis-master.llmd-test.svc.cluster.local:8100 bypass the %test_namespace% parameter and will fail in other namespaces. Replace llmd-test with the placeholder (e.g., %test_namespace%) or make it configurable.
Example fix:
- value: llm-d-redis-master.llmd-test.svc.cluster.local:8100
+ value: llm-d-redis-master.%test_namespace%.svc.cluster.local:8100Also applies to: 213-216
🤖 Prompt for AI Agents
In
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/llm-d-llmIsvcConfig.yaml
around lines 87 to 90 and also lines 213 to 216, the Redis service address is
hard-coded with the namespace 'llmd-test', which prevents reuse in other
namespaces. Replace the hard-coded namespace segment 'llmd-test' in the Redis
URL with the placeholder '%test_namespace%' or another configurable variable to
allow dynamic namespace substitution during deployment.
| kind: LLMInferenceServiceConfig | ||
| metadata: | ||
| name: llm-d-config | ||
| namespace: %test_namespace% |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Quote the namespace placeholder to satisfy YAML syntax
The %test_namespace% on line 5 must be quoted to form valid YAML.
- namespace: %test_namespace%
+ namespace: "%test_namespace%"🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 5-5: syntax error: found character '%' that cannot start any token
(syntax)
🤖 Prompt for AI Agents
In
unofficial_components/roles/poc-llm-d-install-with-manifests/manifests/populated_manifests/llm-d-llmIsvcConfig.yaml
at line 5, the namespace placeholder %test_namespace% is not quoted, which
breaks YAML syntax. Fix this by enclosing %test_namespace% in quotes to ensure
it is treated as a string and the YAML remains valid.
|
/lgtm |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
1 participant
Summary by CodeRabbit
New Features
Documentation