[CLI] Secure OSS secrets in cluster, fix engine port args, and add Qwen3.6 model by diw-zw · Pull Request #279 · sgl-project/rbg

diw-zw · 2026-04-21T01:38:27Z

PR Description

Summary

This PR includes three CLI improvements:

1. OSS Storage Credential Security Enhancement

Migrates OSS AccessKey credentials from local storage to Kubernetes Secrets
Introduces the PreAdd plugin interface to automatically create Secret resources before adding a storage configuration
Storage configs no longer store plaintext akId/akSecret; instead they use secretRef references
Affected files: cmd/cli/plugin/storage/interface.go, cmd/cli/plugin/storage/oss.go, and related tests

2. Fix Engine Plugin Port Arguments Not Taking Effect

Adds --host 0.0.0.0 and --port startup arguments to both SGLang and vLLM engines
Ensures the port configured by the engine plugin is correctly bound and effective
Affected files: cmd/cli/plugin/engine/sglang.go, cmd/cli/plugin/engine/vllm.go, and tests

3. New Model Support & Configuration Corrections

New model: Qwen/Qwen3.6-35B-A3B (SGLang engine, 2 GPUs, 128Gi memory)
Fixed distributed mode names:
- distributed-tp2 → distributed-sglang
- distributed-vllm-tp → distributed-vllm
Fixed distributed-vllm GPU resources: 2 → 1
Affected file: cmd/cli/cmd/llm/run/models.yaml

Changed Files (15 files changed, +573 / -213)

Category	Files
Storage Plugin	`cmd/cli/plugin/storage/interface.go`, `oss.go`, `pvc.go`, `oss_test.go`
Storage Config	`cmd/cli/cmd/llm/config/config.go`, `config_test.go`, `storage.go`, `storage_test.go`
CLI Commands	`cmd/cli/cmd/llm/llm.go`, `pull.go`
Engine Plugin	`cmd/cli/plugin/engine/sglang.go`, `sglang_test.go`, `vllm.go`, `vllm_test.go`
Model Config	`cmd/cli/cmd/llm/run/models.yaml`

Addition

a70130f — Flatten plugin config & improve config CLI

Flatten OSS secretRef nested object into secretName + secretNamespace top-level fields
Add printConfigItems(): alphabetically sorted output with sensitive field masking
Hook PreAdd into config init so AK/SK are stored as K8s Secrets, never written to local config
Unify ConfigFields() across engine/source/storage plugin interfaces

55229bd — Restructure CLI subcommands into resource groups

Split flat llm subcommands into two groups:
- llm model — model asset management (list, pull)
- llm svc — inference service lifecycle (run, list, delete, chat)
Move packages: llm/ → llm/model/, llm/svc/, llm/shared/
Update all help text and examples to new command paths
Update README accordingly

gemini-code-assist · 2026-04-21T01:38:30Z

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

Copilot

Pull request overview

This PR improves the kubectl rbg llm CLI by hardening storage credential handling (moving OSS credentials into Kubernetes Secrets), ensuring engine plugins bind to the configured port, and expanding/reshaping the LLM command surface (svc/model subgroups + model config updates).

Changes:

Add PreAdd hook for storage plugins and migrate OSS credentials from local config into Kubernetes Secrets (secretName/secretNamespace references).
Fix SGLang/vLLM engine arg generation so --host 0.0.0.0 --port <configured> is always applied.
Restructure llm commands into llm svc and llm model, add a chat implementation, and update builtin model configurations (including Qwen3.6 + distributed mode name fixes).

Reviewed changes

Copilot reviewed 36 out of 43 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
cmd/cli/plugin/storage/interface.go	Adds `PreAdd` to storage plugin interface and sorts registered plugin names.
cmd/cli/plugin/storage/oss.go	Switches OSS config to secret references and adds Secret-creating `PreAdd`.
cmd/cli/plugin/storage/oss_test.go	Updates OSS tests for SecretRef-based config and adds PreAdd coverage.
cmd/cli/plugin/storage/pvc.go	Implements no-op `PreAdd` for PVC storage.
cmd/cli/plugin/engine/sglang.go	Adds explicit `--host/--port` args to SGLang server startup.
cmd/cli/plugin/engine/sglang_test.go	Asserts SGLang args include `--host/--port`.
cmd/cli/plugin/engine/vllm.go	Adds explicit `--host/--port` args to vLLM startup args (leader + worker).
cmd/cli/plugin/engine/vllm_test.go	Asserts vLLM args include `--host/--port`.
cmd/cli/plugin/engine/interface.go	Sorts registered engine plugin names.
cmd/cli/plugin/source/interface.go	Sorts registered source plugin names.
cmd/cli/cmd/llm/llm.go	Registers new `svc` and `model` command groups; wires config with ConfigFlags.
cmd/cli/cmd/llm/llm_test.go	Updates subcommand expectations to match new command grouping.
cmd/cli/cmd/llm/config/config.go	Updates config command constructor signature and wiring.
cmd/cli/cmd/llm/config/config_test.go	Updates tests for new config constructor signature.
cmd/cli/cmd/llm/config/storage.go	Calls storage `PreAdd` before persisting storage config.
cmd/cli/cmd/llm/config/storage_test.go	Updates tests for `newAddStorageCmd(cf)` signature.
cmd/cli/cmd/llm/config/misc.go	Adds sorted/masked config printing and calls `PreAdd` during interactive init.
cmd/cli/cmd/llm/config/misc_test.go	Updates init tests for new signature and new helper usage.
cmd/cli/cmd/llm/model/model.go	Introduces `llm model` command group.
cmd/cli/cmd/llm/model/list.go	Renames “models” command to `model list` subcommand.
cmd/cli/cmd/llm/model/list_test.go	Improves stdout assertions around model list printing.
cmd/cli/cmd/llm/model/pull.go	Updates command path text and uses shared sanitization; tweaks job polling cadence.
cmd/cli/cmd/llm/model/pull_test.go	Fixes shell escaping test helper behavior and package rename.
cmd/cli/cmd/llm/svc/svc.go	Introduces `llm svc` command group (run/list/delete/chat).
cmd/cli/cmd/llm/svc/run.go	Updates run command for new paths and shared helpers.
cmd/cli/cmd/llm/svc/run_test.go	Package rename + metadata import path update.
cmd/cli/cmd/llm/svc/list.go	Updates list command text/examples to `llm svc list`.
cmd/cli/cmd/llm/svc/list_test.go	Updates metadata import path and package rename.
cmd/cli/cmd/llm/svc/list_printer.go	Package rename to `svc`.
cmd/cli/cmd/llm/svc/delete.go	Updates delete command text/examples to `llm svc delete`.
cmd/cli/cmd/llm/svc/delete_test.go	Package rename and removes old llm-root subcommand coverage.
cmd/cli/cmd/llm/svc/metadata/metadata.go	Adds svc-scoped metadata constants/types.
cmd/cli/cmd/llm/svc/chat/chat.go	Implements `llm svc chat` behavior and updates metadata expectations.
cmd/cli/cmd/llm/svc/chat/client.go	Implements OpenAI-compatible chat HTTP client (streaming + non-streaming).
cmd/cli/cmd/llm/svc/chat/portforward.go	Adds kubectl port-forward session manager for chat.
cmd/cli/cmd/llm/svc/chat/chat_test.go	Adds comprehensive tests for chat client + basic REPL behaviors.
cmd/cli/cmd/llm/svc/run/models.yaml	Fixes distributed mode names/resources and adds Qwen/Qwen3.6-35B-A3B.
cmd/cli/cmd/llm/svc/run/model_embed.go	Embeds builtin models.yaml for runtime loading.
cmd/cli/cmd/llm/svc/run/model_config.go	Adds builtin+user model config loader with conflict warnings and lookup helpers.
cmd/cli/cmd/llm/svc/run/model_config_test.go	Adds tests for user model loading/override/duplicates.
cmd/cli/cmd/llm/shared/shared.go	Exports shared sanitization/printing helpers for new package structure.
cmd/cli/cmd/llm/shared/shared_test.go	Updates tests for exported shared helper names.
cmd/cli/README.md	Updates docs for new command grouping and usage examples.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

cheyang

Bug: StorageName uses storageType instead of the actual storage name in init

In newInitCmd() (misc.go line 263), PreAddOptions.StorageName is set to storageType (the plugin type like "oss"), but it should use the user-provided storage name name. This results in Secret names like oss-oss-secret instead of my-pvc-oss-secret.

Compare with add-storage where name is correctly used. The init flow already has the variable name available — it just needs to be passed instead of storageType.

Suggested fix:

StorageName: name,  // instead of storageType

Bug: injectMetadataSave Case 2 missing {{DOWNLOADED_AT}} timestamp replacement

In injectMetadataSave() (model/pull.go), Case 1 (existing /bin/sh -c) correctly builds a metadataCmd that uses sed to replace the {{DOWNLOADED_AT}} placeholder with an actual timestamp. But Case 2 (direct binary execution) uses wrappedCmd which only does printf ... > file without sed replacement — so the metadata JSON will contain literal {{DOWNLOADED_AT}} string instead of the actual download timestamp.

The two cases should produce equivalent metadata. Case 2 should also include the downloadedAt=$(date -u ...) + sed replacement logic.

Suggestion: PreAdd should allow updating existing Secret on credential mismatch

Currently, when PreAdd finds an existing Secret with different credentials, it returns an error: "already exists with different credentials". This blocks credential rotation (AK key updates). Consider either:

Updating the Secret when credentials differ (with a warning)
Adding a --force / --update-secret flag to add-storage and init

Nit: chat_test.go copyright year is 2025, rest of PR uses 2026

cheyang

/lgtm
/approve

diw-zw · 2026-04-22T07:16:11Z

Bug: StorageName uses storageType instead of the actual storage name in init

In newInitCmd() (misc.go line 263), PreAddOptions.StorageName is set to storageType (the plugin type like "oss"), but it should use the user-provided storage name name. This results in Secret names like oss-oss-secret instead of my-pvc-oss-secret.

Compare with add-storage where name is correctly used. The init flow already has the variable name available — it just needs to be passed instead of storageType.

Suggested fix:
StorageName: name,  // instead of storageType

To streamline the init command and reduce manual input, we removed the prompts for storageName and sourceName. The name field is now automatically set to storageType instead. This is the intended behavior.

diw-zw requested a review from Syspretor April 21, 2026 01:38

照微 added 4 commits April 21, 2026 19:55

[cli] init/add-storage save oss secret in cluster instead of local

0d4da1c

Fix: engine-plugin port arg not works

64a640d

[CLI] Add model support: Qwen/Qwen3.6-35B-A3B

8af773f

improve config cli: save flat plugin config

a70130f

diw-zw force-pushed the oss-sec branch from d4a1372 to d8e73e2 Compare April 21, 2026 11:57

[cli] refector

55229bd

diw-zw force-pushed the oss-sec branch from d8e73e2 to 55229bd Compare April 22, 2026 04:53

cheyang requested a review from Copilot April 22, 2026 05:53

Copilot started reviewing on behalf of cheyang April 22, 2026 05:53 View session

Copilot AI reviewed Apr 22, 2026

View reviewed changes

Comment thread cmd/cli/plugin/storage/oss.go

Comment thread cmd/cli/plugin/storage/oss.go Outdated

Comment thread cmd/cli/plugin/storage/oss.go

Comment thread cmd/cli/plugin/storage/interface.go

.

a046103

cheyang requested changes Apr 22, 2026

View reviewed changes

cheyang self-requested a review April 22, 2026 06:38

cheyang approved these changes Apr 22, 2026

View reviewed changes

Syspretor merged commit 626ce54 into sgl-project:main Apr 22, 2026
9 checks passed

diw-zw deleted the oss-sec branch April 30, 2026 08:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CLI] Secure OSS secrets in cluster, fix engine port args, and add Qwen3.6 model#279

[CLI] Secure OSS secrets in cluster, fix engine port args, and add Qwen3.6 model#279
Syspretor merged 6 commits intosgl-project:mainfrom
diw-zw:oss-sec

diw-zw commented Apr 21, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot commented Apr 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cheyang left a comment

Uh oh!

cheyang left a comment

Uh oh!

Uh oh!

diw-zw commented Apr 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

diw-zw commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Description

Summary

1. OSS Storage Credential Security Enhancement

2. Fix Engine Plugin Port Arguments Not Taking Effect

3. New Model Support & Configuration Corrections

Changed Files (15 files changed, +573 / -213)

Addition

Uh oh!

gemini-code-assist Bot commented Apr 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cheyang left a comment

Choose a reason for hiding this comment

Uh oh!

cheyang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

diw-zw commented Apr 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

diw-zw commented Apr 21, 2026 •

edited

Loading