feat: add registry override support for init containers #41080
+19
−7
Conversation
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
shirobachi
requested review from
sharat87,
pratapaprasanna and
nidhi-nair
as code owners
WalkthroughThe Helm deployment template was updated to dynamically construct init container image references for Redis, MongoDB, and PostgreSQL using the Changes
Poem
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
deploy/helm/templates/deployment.yaml (2)
68-70: Fallback tag could be sourced from the main Redis image declarationNice use of
digfor graceful fall-through.
For perfect DRY-ness, consider pulling the default tag from.Values.redis.image.taginstead of hard-coding7.0.15, so a single change propagates everywhere.
78-80: Same DRY comment for MongoDBSame observation here—wire the fallback tag to
.Values.mongodb.image.tagto avoid future drift.deploy/helm/values.yaml (1)
124-137: Example section is out-of-sync with actual defaults
- The Postgres example still shows
14.5.0-debian-11-r21, but the live default is14.12.0.- Minor nit: comment alignment on
tag:is off by one space compared to the others.Keeps docs truthful and tidy.
- # tag: 14.5.0-debian-11-r21 # Tag override + # tag: 14.12.0 # Tag override
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
deploy/helm/templates/deployment.yaml(4 hunks)deploy/helm/values.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: appsmithorg/appsmith#0
File: .cursor/rules/index.mdc:0-0
Timestamp: 2025-06-23T12:22:10.042Z
Learning: Pull request titles in the Appsmith codebase should follow the Conventional Commits specification, using the format 'type(scope): description' with valid types such as feat, fix, docs, style, refactor, perf, test, build, ci, chore, and revert.
Learnt from: brayn003
PR: appsmithorg/appsmith#40462
File: app/client/src/instrumentation/index.ts:0-0
Timestamp: 2025-04-29T09:12:14.552Z
Learning: Only comment on files that are directly related to the PR's objectives, even if other files appear in the diff. For PR #40462, the focus is on the import override feature for artifacts, not on instrumentation or telemetry files.
Learnt from: Aishwarya-U-R
PR: appsmithorg/appsmith#29405
File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41
Timestamp: 2024-10-08T15:32:39.374Z
Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
Learnt from: Aishwarya-U-R
PR: appsmithorg/appsmith#29405
File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
Learnt from: sharat87
PR: appsmithorg/appsmith#30252
File: deploy/docker/fs/usr/lib/python3/dist-packages/supervisor/appsmith_supervisor_stdout.py:21-29
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The user has confirmed that the suggested changes to handle potential exceptions and improve the robustness of the `main` function in `appsmith_supervisor_stdout.py` are acceptable.
deploy/helm/templates/deployment.yaml (5)
Learnt from: abhvsn
PR: appsmithorg/appsmith#37068
File: deploy/docker/fs/opt/appsmith/pg-utils.sh:125-132
Timestamp: 2024-10-25T08:31:07.651Z
Learning: In `deploy/docker/fs/opt/appsmith/pg-utils.sh`, the `POSTGRES_ADMIN_USER` (`postgres`) uses trust-based authentication in the local environment, so password handling changes for local setup may not be needed.
Learnt from: abhvsn
PR: appsmithorg/appsmith#37068
File: deploy/docker/fs/opt/appsmith/entrypoint.sh:475-475
Timestamp: 2024-10-25T08:05:20.269Z
Learning: In `deploy/docker/fs/opt/appsmith/entrypoint.sh`, `pg_isready` does not check authentication, so even if there is no `pg_hba.conf` entry for host connections for the `postgres` user, we don't need to worry.
Learnt from: abhvsn
PR: appsmithorg/appsmith#36670
File: deploy/docker/fs/opt/appsmith/entrypoint.sh:465-483
Timestamp: 2024-10-08T15:32:34.115Z
Learning: In the `deploy/docker/fs/opt/appsmith/entrypoint.sh` script, `APPSMITH_PG_DATABASE` is a constant with the value `appsmith`, so we can assume it remains constant and doesn't require additional quoting in SQL commands.
Learnt from: abhvsn
PR: appsmithorg/appsmith#36670
File: deploy/docker/fs/opt/appsmith/entrypoint.sh:465-483
Timestamp: 2024-10-07T09:13:14.613Z
Learning: In the `deploy/docker/fs/opt/appsmith/entrypoint.sh` script, `APPSMITH_PG_DATABASE` is a constant with the value `appsmith`, so we can assume it remains constant and doesn't require additional quoting in SQL commands.
Learnt from: abhvsn
PR: appsmithorg/appsmith#38069
File: deploy/docker/tests/pg-test-utils.sh:3-6
Timestamp: 2024-12-13T05:11:54.976Z
Learning: In the `deploy/docker/tests/pg-test-utils.sh` script, prefer using the syntax `variable=${variable:-default_value}` to define default values for variables like `container_name`.
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
|
@sharat87 @pratapaprasanna @nidhi-nair any feedback / update / conclution? Can I know status of it? |
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
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.
Adds registry override support for init containers in the Helm chart. Previously, init containers (redis, mongodb, postgresql) only supported full image path overrides. This change allows granular control over registry, repository, and tag components separately.
Automation
/ok-to-test tags=""
🔍 Cypress test results
Caution
If you modify the content in this section, you are likely to disrupt the CI result for your PR.
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Documentation
Style