madengine v2 refactor(k8s) secrets handling, launcher mixin, and template updates#88
Merged
coketaste merged 2 commits intocoketaste/refactor-disfrom Mar 23, 2026
Merged
Conversation
Collaborator
coketaste
commented
Mar 22, 2026
- Add k8s_secrets helpers for image pull / runtime secrets with configurable strategies; add unit tests.
- Introduce KubernetesLauncherMixin for torchrun and related launcher command generation; wire into kubernetes deployment.
- Update K8s Job/ConfigMap templates, k8s defaults preset, and kubernetes.py.
- Refresh configuration/deployment docs and k8s-configs README.
- Adjust tests (conftest, dummy credential fixture) for new behavior.
- Fix CLI merge of --additional-context with --additional-context-file when the CLI value uses Python dict syntax (fallback after JSON parse failure).
- Add k8s_secrets helpers for image pull / runtime secrets with configurable strategies; add unit tests. - Introduce KubernetesLauncherMixin for torchrun and related launcher command generation; wire into kubernetes deployment. - Update K8s Job/ConfigMap templates, k8s defaults preset, and kubernetes.py. - Refresh configuration/deployment docs and k8s-configs README. - Adjust tests (conftest, dummy credential fixture) for new behavior. - Fix CLI merge of --additional-context with --additional-context-file when the CLI value uses Python dict syntax (fallback after JSON parse failure).
There was a problem hiding this comment.
Pull request overview
Refactors madengine’s Kubernetes (v2) deployment to improve secret handling, launcher command generation, and manifest templating—aiming to keep credentials out of ConfigMaps while supporting multiple secret strategies and more launchers.
Changes:
- Add
k8s_secretshelpers + defaults to create/resolve image-pull/runtime Secrets (with ConfigMap size preflight + unit tests). - Introduce
KubernetesLauncherMixinand wire it intoKubernetesDeployment; update K8s Job/ConfigMap templates to mount credentials via Secret when applicable. - Improve CLI
--additional-contextmerge behavior (JSON first, fallback to Python dict syntax) and refresh K8s-related docs.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_k8s_secrets.py | Adds unit tests for secret strategy merging, secret ref resolution, and ConfigMap payload size estimation. |
| tests/conftest.py | Ensures src/ is on sys.path for consistent test imports. |
| src/madengine/deployment/templates/kubernetes/job.yaml.j2 | Adds TTL support, conditional imagePullSecrets, and mounts runtime credentials from Secret when configured; gates privileged profiling securityContext. |
| src/madengine/deployment/templates/kubernetes/configmap.yaml.j2 | Stops embedding credential.json unless explicitly requested. |
| src/madengine/deployment/presets/k8s/defaults.json | Adds defaults for TTL, privileged profiling toggle, and k8s.secrets configuration. |
| src/madengine/deployment/kubernetes_launcher_mixin.py | New mixin providing launcher command string generators (torchrun/deepspeed/torchtitan/vLLM/SGLang/etc.). |
| src/madengine/deployment/kubernetes.py | Integrates secrets strategy + ConfigMap size preflight, uses launcher mixin, updates GPU plugin validation messaging, and propagates imagePullSecrets to collector pod. |
| src/madengine/deployment/k8s_secrets.py | New module for creating/deleting secrets from credential.json, resolving secret refs, and estimating ConfigMap payload sizes. |
| src/madengine/cli/commands/run.py | Fixes merging of --additional-context with file-based context by attempting JSON parse first, then ast.literal_eval. |
| examples/k8s-configs/README.md | Documents new Secrets behavior and multi-node DNS differences across launcher types. |
| docs/deployment.md | Documents Secrets behavior, manifest validation workflow, and clarifies TTL cleanup behavior. |
| docs/configuration.md | Updates K8s preset docs and documents new k8s.secrets/TTL/profiling fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…t helper - Import Path at module level in kubernetes_launcher_mixin so _generate_vllm_command and _generate_sglang_command do not raise NameError; drop redundant inner import in _generate_torchrun_command. - Rename _build_registry_secret_data to build_registry_secret_data and document it as the supported API for dockerconfigjson preview logic. - Update kubernetes.py and unit tests accordingly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.