Add Loop Runtime workload for EKS - #98
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3b7cd9e53
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| {{- $resourceLabels := mergeOverwrite (deepCopy .Values.global.labels) (deepCopy .Values.loopRuntime.labels) -}} | ||
| {{- $podLabels := mergeOverwrite (deepCopy .Values.global.labels) (deepCopy .Values.loopRuntime.labels) (deepCopy .Values.loopRuntime.podLabels) (dict "app" .Values.loopRuntime.name) -}} |
There was a problem hiding this comment.
The new Deployment, ConfigMap, Service, and ServiceAccount all merge global.labels with Loop Runtime labels, but none of the added tests renders them alongside components with conflicting labels to verify that labels cannot bleed through Helm's mutating merge functions. Add Loop Runtime cases to the existing labels-merge-* isolation suites, including coverage for the pod-label merge.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
| {{- include "braintrust.loopRuntime.validate" . -}} | ||
| {{- if .Values.loopRuntime.enabled -}} | ||
| {{- $labels := mergeOverwrite (deepCopy .Values.global.labels) (deepCopy .Values.loopRuntime.labels) -}} | ||
| {{- $annotations := deepCopy .Values.loopRuntime.annotations.serviceaccount -}} |
There was a problem hiding this comment.
When a caller sets the newly declared loopRuntime.serviceAccount.annotations value, the generated ServiceAccount silently omits it because this template copies only loopRuntime.annotations.serviceaccount. This makes the value at values.yaml:321 ineffective for integrations that need custom ServiceAccount annotations; render or merge the nested annotation map here.
Useful? React with 👍 / 👎.
|
Codex (@codex) review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e8a133ecc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| httpGet: | ||
| path: /health/liveness | ||
| # Use the named Service port so this follows service.port changes. | ||
| port: http |
There was a problem hiding this comment.
When a deployment overrides loopRuntime.service.portName, the container port is rendered with that custom name, but both default probes continue targeting the literal name http. Kubernetes cannot resolve that named probe port, so the Loop Runtime pod remains unready and is repeatedly restarted by the liveness probe. Derive these probe ports from loopRuntime.service.portName or use the numeric service port.
Useful? React with 👍 / 👎.
|
Codex (@codex) review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback". |
1 participant
Context
The Helm chart had no Loop Runtime workload, so external EKS deployments could not run the Kubernetes-hosted runtime.
Description
Add an opt-in AWS EKS Loop Runtime Deployment, Service, ServiceAccount, and ConfigMap. Wire the runtime to the existing data-plane Secret, API URL, AWS MicroVM sandbox settings, and optional Istio route, with hardened pod defaults and public configuration documentation.
Testing
./test.sh