Make provider changes to support image cache #1700
Conversation
|
WalkthroughThe changes enhance Kubernetes task operations by introducing configurable image settings and sophisticated label handling. In the main index file, new constants, functions, and a type are added to build image references with specific prefixes and to include additional image pull secrets dynamically. A new Changes
Sequence Diagram(s)sequenceDiagram
participant KP as Kubernetes Provider (index.ts)
participant Img as ImageRef Functions (getImagePrefix/getImageRef)
participant LH as CustomLabelHelper
participant K8s as Kubernetes API
KP->>Img: Request image reference for deployment/restore/utility
Img-->>KP: Return image reference with appropriate prefix
KP->>LH: Generate additional custom labels
LH-->>KP: Return label set for create/restore operations
KP->>K8s: Deploy pod spec with image and labels
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
apps/kubernetes-provider/src/index.tsOops! Something went wrong! :( ESLint: 8.45.0 ESLint couldn't find the config "custom" to extend from. Please check that the name of the config is correct. The config "custom" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. apps/kubernetes-provider/src/labelHelper.tsOops! Something went wrong! :( ESLint: 8.45.0 ESLint couldn't find the config "custom" to extend from. Please check that the name of the config is correct. The config "custom" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 0
🧹 Nitpick comments (2)
apps/kubernetes-provider/src/labelHelper.ts (1)
138-149: Unnecessarycontinuestatements in label loop.
Since there is no remaining logic after settingadditionalLabelsin each condition, thecontinuestatements serve no functional purpose. Removing them would simplify the loop.Proposed diff:
for (const { key, value, sampleRate } of labels) { if (sampleRate === 1) { additionalLabels[key] = value; - continue; } if (Math.random() <= sampleRate) { additionalLabels[key] = value; - continue; } }🧰 Tools
🪛 Biome (1.9.4)
[error] 147-147: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
apps/kubernetes-provider/src/index.ts (1)
42-49: Environment-based image configuration is flexible.
IntroducingADDITIONAL_PULL_SECRETS,PAUSE_IMAGE,BUSYBOX_IMAGE, and prefix variables for images provides a good level of configurability. Make sure to document these environment variables clearly so that users know how to override defaults.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/kubernetes-provider/src/index.ts(13 hunks)apps/kubernetes-provider/src/labelHelper.ts(1 hunks)apps/kubernetes-provider/tsconfig.json(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/kubernetes-provider/src/labelHelper.ts
[error] 147-147: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
🔇 Additional comments (10)
apps/kubernetes-provider/src/labelHelper.ts (3)
15-19: Well-structured class introduction.
TheCustomLabelHelperclass provides an organized approach to label handling. Storing label arrays in internal fields and computing them only once is efficient, and the overall design improves clarity and maintainability.
30-32: Good clamping logic infractionFromPercent.
Ensuring the returned fraction stays between 0 and 1 is correct and prevents invalid sample rates.
65-85: Thorough defaulting ingetSampleRateFromEnvVarKey.
This method properly defaults to 100% if no environment value is found or if the value is invalid. This ensures predictable label sampling behavior.apps/kubernetes-provider/src/index.ts (6)
20-21: Use ofassertExhaustiveandCustomLabelHelperimport is clean.
The import statements show a well-structured codebase that centralizes core type checks and label logic.
77-77: Single helper instance is efficient.
Instantiating oneCustomLabelHelperand reusing it across operations ensures consistent labeling logic and avoids redundant computations.
117-117: Consistent usage ofgetImageRef().
Leveraging thegetImageRef()function across deployment, restore, and utility images makes the image references more maintainable and uniform.Also applies to: 185-185, 247-247, 252-252, 268-268, 374-374, 388-388
171-171: Dynamic label application for "create" and "restore".
Retrieving additional labels through the helper consolidates all label-based logic in one place, enhancing clarity and maintainability.Also applies to: 233-233
692-710: Robust handling of image prefixes.
DefiningImageTypeunions, then cleanly switching on them ingetImagePrefix()lets TypeScript ensure coverage of all cases.assertExhaustiveis a good final check to avoid missing a future image type.
419-433: Inclusion of additional pull secrets.
Appending custom secrets from theADDITIONAL_PULL_SECRETSenvironment variable is helpful, but confirm that these secrets exist in the cluster before relying on them, as missing secrets can cause deployments to fail.Below is a basic script to list secrets in the cluster. Confirm that any secrets added via
ADDITIONAL_PULL_SECRETSappear here:❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Assuming the codebase is deployed to a Kubernetes cluster. # Use kubectl to list all secrets in the namespace. kubectl get secrets -n "$KUBERNETES_NAMESPACE"Length of output: 125
Manual Verification Required: Validate Kubernetes Secrets
The current test environment did not have a functioningkubectl(as seen from the "command not found" error), so we weren’t able to automatically verify if the secrets added viaADDITIONAL_PULL_SECRETSappear in your cluster. Please run the secret listing command in an environment withkubectlinstalled and configured (with$KUBERNETES_NAMESPACEset to a valid namespace) to ensure that all the pull secrets exist before deployment.apps/kubernetes-provider/tsconfig.json (1)
11-12: Expanded path mappings confirmed.
These additions improve local development ergonomics by matching the code structure in../../packages/core/src.
* add env var for additional pull secrets * make static images configurable * optional image prefixes * optional labels with sample rates * add missing core paths * remove excessive logs
* Uses image cards for the frameworks * Removes old snippets * New AI agents side menu section * WIP adding new ai agent pages * Better overview page * More copy added to the agent example pages * Copy improvements * Removes “Creating a project” page and side menu section * Fixes broken links * Updates to the latest Mintlify version, fixes issues, changes theme * Adds descriptions to the main dropdown menu items * Reformatted Introduction docs ‘landing page’ * Retry heartbeat timeouts by putting back in the queue (#1689) * If there’s a heartbeat error and no attempts we put it back in the queue to try again * When nacking, return whether it was put back in the queue or not * Try and nack, if it fails then fail the run * Consolidated switch statement * Fail executing/retrying runs * OOM retrying on larger machines (#1691) * OOM retrying on larger machines * Create forty-windows-shop.md * Update forty-windows-shop.md * Only retry again if the machine is different from the original * Kubernetes OOMs appear as non-zero sigkills, adding support for treating these as OOMs * Complete the original attempt span if retrying due to an OOM * Revert "Complete the original attempt span if retrying due to an OOM" This reverts commit 5f652c6. * chore: Update version for release (#1666) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Release 3.3.14 * Set machine when triggering docs * Batch queue runs that are waiting for deploy (#1693) * Detect ffmpeg OOM errors, added manual OutOfMemoryError (#1694) * Detect ffmpeg OOM errors, added manual OutOfMemoryError * Create eighty-spies-knock.md * Improved the machines docs, including the new OutOfMemoryError * chore: Update version for release (#1695) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Release 3.3.15 * Create new partitioned TaskEvent table, and switch to it gradually as new runs are created (#1696) * Create new partitioned TaskEvent table, and switch to it gradually as new runs are created * Add env var for partition window in seconds * Make startCreatedAt required in task event store * Don't create an attempt if the run is final, batchTriggerAndWait bad continue fix (#1698) * WIP fix for ResumeAttemptService selecting the wrong attempt (which has no error or output) * Don’t create an attempt if the run is already in a final status * Don’t get all the columns for the query. Improved the logging. * Added a log to the batch example * Filter out the undefined values * Fix missing logs on child runs by using the root task run createdAt if it exists (#1697) * Provider changes to support image cache (#1700) * add env var for additional pull secrets * make static images configurable * optional image prefixes * optional labels with sample rates * add missing core paths * remove excessive logs * Fix run container exits after OOM retries (#1701) * remove unused imports * tell run to exit before force requeue * handle exit for case where we already retried after oom * improve retry span and add machine props * don't try to exit run in dev * Upgrade local dev to use electric beta.15 (#1699) * Text fixes * Removed pnpm files --------- Co-authored-by: Matt Aitken <matt@mattaitken.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Eric Allam <eric@trigger.dev> Co-authored-by: nicktrn <55853254+nicktrn@users.noreply.github.com>
This makes a lot of things more easily configurable via optional env vars:
Summary by CodeRabbit