-
-
Notifications
You must be signed in to change notification settings - Fork 743
Self-hosting: improved worker and bucket bootstrap #2209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
nicktrn
commented
Jun 29, 2025
- k8s-native worker token bootstrap
- auto s3 bucket creation for docker and k8s
- small docs fixes
|
WalkthroughThe changes span documentation, Docker Compose, and Kubernetes Helm chart configurations. Documentation updates clarify the default creation of the "packets" object storage bucket and correct an example for concurrency release in wait operations. The Docker Compose file for the webapp replaces the Minio image with Bitnami's variant, updates volume paths, and sets environment variables to ensure the "packets" bucket is created by default. Helm chart changes increment version numbers, adjust supervisor deployment logic to conditionally include certain pod specs based on a bootstrap flag, and introduce new RBAC resources and a sidecar container for secret management in the webapp deployment. The Helm values file adds a key to document default bucket creation. No public API or exported entity changes were made. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
docs/self-hosting/docker.mdx (1)
247-255
: Note contradicts the numbered steps – needs tighteningThe new
<Note>
says thepackets
bucket is created automatically, yet the very next step still instructs users to create the bucket manually. That can confuse newcomers.-1. Login to the dashboard: `http://localhost:9001` -2. Create a bucket named `packets`. +1. Login to the dashboard: `http://localhost:9001` +2. Verify that the `packets` bucket exists. + • If it did not get created automatically, create it manually.hosting/k8s/helm/templates/supervisor.yaml (1)
139-151
: MakeTRIGGER_WORKER_TOKEN
secret optional to avoid bootstrap race-conditionIf
webapp.bootstrap.enabled
istrue
, this env-var references a secret that doesn’t exist until the webapp side-car creates it.
A pod fails to start when a requiredsecretKeyRef
is missing, so the supervisor deployment will enter a crash-loop until the secret appears.Add
optional: true
to thesecretKeyRef
so the pod can start immediately (the variable will be empty until the secret is created).- name: {{ include "trigger-v4.fullname" . }}-worker-token - key: token + name: {{ include "trigger-v4.fullname" . }}-worker-token + key: token + optional: true
🧹 Nitpick comments (3)
hosting/k8s/helm/values.yaml (1)
479-481
: Minor YAML nit – clarify multi-bucket syntaxBitnami’s MinIO chart treats
defaultBuckets
as a comma-separated string. Consider adding an inline comment so future users know how to specify multiple buckets:defaultBuckets: "packets" # comma-separated, e.g. "packets,images,backups"hosting/k8s/helm/templates/webapp.yaml (2)
79-83
: Pin the kubectl side-car image by digest
bitnami/kubectl:1.28
will float to new patch versions.
For reproducible builds and supply-chain security, pin the image to a digest:- image: bitnami/kubectl:1.28 + image: bitnami/kubectl@sha256:<digest>Replace
<digest>
with the current digest from Docker Hub.
1-36
: Indentation warnings from yamllintStatic analysis flags several 2-vs-4-space indentation issues.
Although Kubernetes tolerates them, fixing the indent keeps the chart
yamllint-clean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/self-hosting/docker.mdx
(1 hunks)docs/upgrade-to-v4.mdx
(1 hunks)hosting/docker/webapp/docker-compose.yml
(2 hunks)hosting/k8s/helm/Chart.yaml
(1 hunks)hosting/k8s/helm/templates/supervisor.yaml
(5 hunks)hosting/k8s/helm/templates/webapp.yaml
(3 hunks)hosting/k8s/helm/values.yaml
(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/values-production-example.yaml:95-102
Timestamp: 2025-06-25T14:14:11.965Z
Learning: In the Trigger.dev Helm chart production examples, the maintainer prefers to include initial/bootstrap credentials with clear warnings that they should be changed after first login, rather than requiring external secret references that could complicate initial setup. This follows their pattern of providing working examples with explicit security guidance.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/values.yaml:22-51
Timestamp: 2025-06-25T13:20:17.174Z
Learning: In the Trigger.dev Helm chart values.yaml, the maintainer prefers to use explicit comprehensive warnings for security-sensitive default values rather than implementing secure-by-default behavior that would fail installation. The project uses deterministic default secrets with clear "TESTING ONLY" warnings and instructions for production deployment.
hosting/k8s/helm/values.yaml (1)
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/values.yaml:22-51
Timestamp: 2025-06-25T13:20:17.174Z
Learning: In the Trigger.dev Helm chart values.yaml, the maintainer prefers to use explicit comprehensive warnings for security-sensitive default values rather than implementing secure-by-default behavior that would fail installation. The project uses deterministic default secrets with clear "TESTING ONLY" warnings and instructions for production deployment.
hosting/k8s/helm/Chart.yaml (5)
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/Chart.yaml:1-18
Timestamp: 2025-06-25T13:18:44.103Z
Learning: For the Trigger.dev Helm chart, the chart name should be "trigger" (not "trigger-v4") since this is the first official chart release. Helper templates should use the actual chart name from .Chart.Name rather than hardcoded prefixes.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/values.yaml:22-51
Timestamp: 2025-06-25T13:20:17.174Z
Learning: In the Trigger.dev Helm chart values.yaml, the maintainer prefers to use explicit comprehensive warnings for security-sensitive default values rather than implementing secure-by-default behavior that would fail installation. The project uses deterministic default secrets with clear "TESTING ONLY" warnings and instructions for production deployment.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/templates/extra-manifests.yaml:1-4
Timestamp: 2025-06-25T13:18:04.827Z
Learning: In the Trigger.dev v4 Helm chart, the user prefers to rely on documentation and examples in values files rather than implementing defensive coding in templates, particularly for features like extraManifests where proper usage is documented.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/values-production-example.yaml:95-102
Timestamp: 2025-06-25T14:14:11.965Z
Learning: In the Trigger.dev Helm chart production examples, the maintainer prefers to include initial/bootstrap credentials with clear warnings that they should be changed after first login, rather than requiring external secret references that could complicate initial setup. This follows their pattern of providing working examples with explicit security guidance.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2155
File: docs/docs.json:179-183
Timestamp: 2025-06-06T16:54:23.316Z
Learning: In the docs.json configuration for the Trigger.dev documentation (Mintlify system), both "tags": ["v4"] and "tag": "v4" properties can be used together and work correctly, even though this behavior is undocumented and may not work in local development environments.
hosting/docker/webapp/docker-compose.yml (1)
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/templates/minio.yaml:49-55
Timestamp: 2025-06-25T13:29:54.392Z
Learning: In MinIO deployments, the MINIO_ROOT_USER and MINIO_ROOT_PASSWORD environment variables are only used during the first start/initialization of MinIO. These are bootstrap credentials that should be changed after the initial setup is complete.
hosting/k8s/helm/templates/supervisor.yaml (5)
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/values.yaml:22-51
Timestamp: 2025-06-25T13:20:17.174Z
Learning: In the Trigger.dev Helm chart values.yaml, the maintainer prefers to use explicit comprehensive warnings for security-sensitive default values rather than implementing secure-by-default behavior that would fail installation. The project uses deterministic default secrets with clear "TESTING ONLY" warnings and instructions for production deployment.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/templates/extra-manifests.yaml:1-4
Timestamp: 2025-06-25T13:18:04.827Z
Learning: In the Trigger.dev v4 Helm chart, the user prefers to rely on documentation and examples in values files rather than implementing defensive coding in templates, particularly for features like extraManifests where proper usage is documented.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/values-production-example.yaml:95-102
Timestamp: 2025-06-25T14:14:11.965Z
Learning: In the Trigger.dev Helm chart production examples, the maintainer prefers to include initial/bootstrap credentials with clear warnings that they should be changed after first login, rather than requiring external secret references that could complicate initial setup. This follows their pattern of providing working examples with explicit security guidance.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2150
File: apps/supervisor/src/workloadManager/docker.ts:115-115
Timestamp: 2025-06-04T16:02:22.957Z
Learning: In the Trigger.dev codebase, the supervisor component uses DOCKER_ENFORCE_MACHINE_PRESETS while the docker provider component uses ENFORCE_MACHINE_PRESETS. These are separate components with separate environment variable configurations for the same logical concept of enforcing machine presets.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2155
File: hosting/docker/.env.example:4-7
Timestamp: 2025-06-06T23:55:01.933Z
Learning: In the trigger.dev project, .env.example files should contain actual example secret values rather than placeholders, as these help users understand the expected format. The files include clear warnings about not using these defaults in production and instructions for generating proper secrets.
hosting/k8s/helm/templates/webapp.yaml (4)
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/values-production-example.yaml:95-102
Timestamp: 2025-06-25T14:14:11.965Z
Learning: In the Trigger.dev Helm chart production examples, the maintainer prefers to include initial/bootstrap credentials with clear warnings that they should be changed after first login, rather than requiring external secret references that could complicate initial setup. This follows their pattern of providing working examples with explicit security guidance.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/values.yaml:22-51
Timestamp: 2025-06-25T13:20:17.174Z
Learning: In the Trigger.dev Helm chart values.yaml, the maintainer prefers to use explicit comprehensive warnings for security-sensitive default values rather than implementing secure-by-default behavior that would fail installation. The project uses deterministic default secrets with clear "TESTING ONLY" warnings and instructions for production deployment.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/Chart.yaml:1-18
Timestamp: 2025-06-25T13:18:44.103Z
Learning: For the Trigger.dev Helm chart, the chart name should be "trigger" (not "trigger-v4") since this is the first official chart release. Helper templates should use the actual chart name from .Chart.Name rather than hardcoded prefixes.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/templates/extra-manifests.yaml:1-4
Timestamp: 2025-06-25T13:18:04.827Z
Learning: In the Trigger.dev v4 Helm chart, the user prefers to rely on documentation and examples in values files rather than implementing defensive coding in templates, particularly for features like extraManifests where proper usage is documented.
🪛 YAMLlint (1.37.1)
hosting/k8s/helm/templates/webapp.yaml
[warning] 7-7: wrong indentation: expected 2 but found 4
(indentation)
[warning] 15-15: wrong indentation: expected 2 but found 4
(indentation)
[warning] 27-27: wrong indentation: expected 2 but found 4
(indentation)
[error] 4-4: syntax error: expected , but found ''
(syntax)
[error] 90-90: trailing spaces
(trailing-spaces)
[error] 94-94: trailing spaces
(trailing-spaces)
[error] 100-100: trailing spaces
(trailing-spaces)
[error] 106-106: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (25)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
docs/upgrade-to-v4.mdx (1)
760-766
: Code snippet now matches the updated API – LGTM
PlacingreleaseConcurrency
inside the first argument object is the correct usage for v4.hosting/k8s/helm/Chart.yaml (1)
5-6
: Remember to updateChart.lock
after bumping versionsHelm will warn if
Chart.lock
isn’t regenerated to reflect the newversion
/appVersion
. Please runhelm dependency update
and commit the refreshed lockfile.hosting/docker/webapp/docker-compose.yml (1)
184-199
: Double-check Bitnami-specific envs & healthcheck
MINIO_BROWSER=on
is respected by the upstream MinIO server, but Bitnami’s wrapper also accepts it. ✅MINIO_DEFAULT_BUCKETS
is correct for auto-creation.- Health-check still targets
/minio/health/live
; that endpoint remains valid, so no change needed. ✅Just verify that your
.env.example
now exposesMINIO_ROOT_USER/PASSWORD
(not the oldMINIO_ACCESS_KEY/SECRET
) to avoid mismatched variable names during local setup.hosting/k8s/helm/templates/supervisor.yaml (1)
77-87
: Validate removal of shared volume when bootstrap is enabled
initContainers
,volumeMounts
, andvolumes
are now wrapped in
{{- if not .Values.webapp.bootstrap.enabled }}
.That matches the intent to drop the shared PVC once the secret-based flow is used.
Please confirm there are no other code paths that still expect/home/node/shared
to exist on the supervisor side (e.g. log/metrics off-load). A quick grep for that path in the supervisor image may avoid a hidden regression.Also applies to: 242-260
- name: token-syncer | ||
image: bitnami/kubectl:1.28 | ||
securityContext: | ||
runAsUser: 1000 | ||
runAsNonRoot: true | ||
command: | ||
- /bin/bash | ||
- -c | ||
- | | ||
TOKEN_FILE="/home/node/shared/worker_token" | ||
SECRET_NAME="{{ include "trigger-v4.fullname" . }}-worker-token" | ||
NAMESPACE="{{ .Release.Namespace }}" | ||
echo "Token syncer starting..." | ||
echo "Monitoring: $TOKEN_FILE" | ||
echo "Target secret: $SECRET_NAME" | ||
while true; do | ||
if [ -f "$TOKEN_FILE" ]; then | ||
TOKEN=$(cat "$TOKEN_FILE") | ||
if [ ! -z "$TOKEN" ]; then | ||
echo "Token file found, creating/updating secret..." | ||
# Create or update the secret | ||
kubectl create secret generic "$SECRET_NAME" \ | ||
--from-literal=token="$TOKEN" \ | ||
--namespace="$NAMESPACE" \ | ||
--dry-run=client -o yaml | kubectl apply -f - | ||
if [ $? -eq 0 ]; then | ||
echo "Secret successfully created/updated" | ||
# Continue monitoring for updates | ||
sleep 30 | ||
else | ||
echo "Failed to create/update secret, retrying in 5s..." | ||
sleep 5 | ||
fi | ||
else | ||
echo "Token file exists but is empty, waiting..." | ||
sleep 2 | ||
fi | ||
else | ||
echo "Waiting for token file..." | ||
sleep 2 | ||
fi | ||
done | ||
volumeMounts: | ||
- name: shared | ||
mountPath: /home/node/shared |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Hard-code of worker-token path risks drift – parameterise it
The side-car script uses:
TOKEN_FILE="/home/node/shared/worker_token"
but the actual file location is configurable through
.Values.webapp.bootstrap.workerTokenPath
, exported to the main container
as TRIGGER_BOOTSTRAP_WORKER_TOKEN_PATH
.
Keep the two in sync to avoid silent failures when the value is overridden:
- TOKEN_FILE="/home/node/shared/worker_token"
+ TOKEN_FILE="{{ .Values.webapp.bootstrap.workerTokenPath }}"
Optionally pass the same value via an env-var so the shell stays readable.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 90-90: trailing spaces
(trailing-spaces)
[error] 94-94: trailing spaces
(trailing-spaces)
[error] 100-100: trailing spaces
(trailing-spaces)
[error] 106-106: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
In hosting/k8s/helm/templates/webapp.yaml between lines 78 and 126, the
token-syncer container script hardcodes the token file path as
"/home/node/shared/worker_token", which can cause mismatches if the actual path
is overridden by the .Values.webapp.bootstrap.workerTokenPath Helm value. To fix
this, parameterize the token file path by passing the configured path into the
sidecar container via an environment variable, ideally reusing the
TRIGGER_BOOTSTRAP_WORKER_TOKEN_PATH value, and update the script to reference
this environment variable instead of the hardcoded path to ensure consistency
and prevent silent failures.