fix: make imagePullSecrets optional when installing dynamo cloud #1324
Conversation
julienmancuso
requested review from
biswapanda,
hhzhang16,
hutm,
ishandhanani,
mohammedabdulwahhab and
nnshah1
as code owners
WalkthroughThis update removes default and redundant Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm Chart
participant Kubernetes
User->>Helm Chart: Deploy chart with values.yaml
alt imagePullSecrets specified
Helm Chart->>Kubernetes: Render deployment with imagePullSecrets
else imagePullSecrets not specified
Helm Chart->>Kubernetes: Render deployment without imagePullSecrets
end
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 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: 0
🧹 Nitpick comments (1)
deploy/cloud/helm/platform/components/operator/templates/deployment.yaml (1)
123-125: Conditionally renderimagePullSecretsonly when defined.The new
with .Values.imagePullSecretsblock prevents rendering an emptyimagePullSecretsarray, making pull secrets truly optional.
For more idiomatic YAML, consider usingtoYamlinstead oftoJson:- imagePullSecrets: {{ . | toJson }} + imagePullSecrets: + {{ . | toYaml | nindent 8 }}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
deploy/cloud/helm/platform/components/api-store/templates/deployment.yaml(0 hunks)deploy/cloud/helm/platform/components/api-store/values.yaml(1 hunks)deploy/cloud/helm/platform/components/operator/templates/deployment.yaml(1 hunks)deploy/cloud/helm/platform/components/operator/values.yaml(1 hunks)deploy/cloud/helm/platform/values.yaml(0 hunks)
💤 Files with no reviewable changes (2)
- deploy/cloud/helm/platform/components/api-store/templates/deployment.yaml
- deploy/cloud/helm/platform/values.yaml
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and Test - vllm
🔇 Additional comments (2)
deploy/cloud/helm/platform/components/operator/values.yaml (1)
110-110: MakeimagePullSecretsoptional by commenting out the default.Commenting out the default
imagePullSecrets: []ensures no empty list is rendered unless explicitly provided, aligning with the goal to make pull secrets opt-in.deploy/cloud/helm/platform/components/api-store/values.yaml (1)
26-27: Remove the defaultimagePullSecretsto enforce opt-in configuration.By commenting out the
imagePullSecrets: []entry, users must now explicitly define pull secrets when needed, keeping the chart lean and flexible.
|
What if the user wants to use ImagePullSecrets like $DOCKER_SECRET_NAME (which is currently created in deploy.sh)? Would everything still work properly? |
Yes, to clarify this change, it allows for users to provide any imagePullSecrets they want. |
Overview:
make imagePullSecrets optional when installing dynamo cloud
Details:
make imagePullSecrets optional when installing dynamo cloud
Where should the reviewer start?
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit