-
-
Notifications
You must be signed in to change notification settings - Fork 147
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
Extract db name as an argument to value.yaml #1107
Conversation
WalkthroughThis pull request modifies the Kubernetes deployment configuration for the Yorkie cluster by introducing a new database configuration mechanism. The changes include adding a specific database name argument to the Yorkie container and implementing an initialization container that waits for MongoDB provisioning to complete before starting the main application. The modifications enhance the deployment's flexibility and ensure proper database connection setup. Changes
Sequence DiagramsequenceDiagram
participant InitContainer as Wait-for-DB Init Container
participant MongoDB as MongoDB Provisioning Job
participant YorkieApp as Yorkie Application
InitContainer->>MongoDB: Check Job Status
alt Job Not Complete
InitContainer-->>InitContainer: Wait and Retry
else Job Complete
InitContainer->>YorkieApp: Allow Startup
YorkieApp->>YorkieApp: Start with Specified Database
end
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
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 (1)
build/charts/yorkie-cluster/templates/yorkie/deployment.yaml (1)
Line range hint
38-54
: Enhance init container reliabilityThe init container's wait logic has several potential issues:
- No timeout mechanism - could wait indefinitely
- No error handling for job failures
- Continuous kubectl calls might stress the API server
Consider implementing:
- A timeout mechanism
- Error handling for failed jobs
- Exponential backoff for status checks
Here's a suggested improvement:
command: - /bin/sh - -c - | echo "Wait db start" + timeout=300 # 5 minutes timeout + start_time=$(date +%s) + backoff=5 while true; do + current_time=$(date +%s) + elapsed=$((current_time - start_time)) + if [ $elapsed -gt $timeout ]; then + echo "Timeout waiting for DB provisioning" + exit 1 + fi job_status=$(kubectl get job -n $NAMESPACE $JOB_NAME -o jsonpath='{.status.conditions[?(@.type=="Complete")].status}') if [ "$job_status" = "True" ]; then echo "DB provisioning Job completed successfully!" break + elif [ "$job_status" = "False" ]; then + echo "DB provisioning Job failed!" + exit 1 fi echo "Waiting for DB provisioning job to complete..." - sleep 5 + sleep $backoff + backoff=$((backoff * 2)) + [ $backoff -gt 60 ] && backoff=60 done
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
build/charts/yorkie-cluster/templates/yorkie/deployment.yaml
(1 hunks)build/charts/yorkie-cluster/values.yaml
(1 hunks)
🔇 Additional comments (2)
build/charts/yorkie-cluster/templates/yorkie/deployment.yaml (2)
75-76
: LGTM! Database argument correctly configured
The new --mongo-yorkie-database
argument properly references the database name from values.yaml.
Line range hint 67-67
: Verify service account permissions
The init container requires a service account with permissions to read job status. Ensure the yorkie-db-provisioning-job-account
follows the principle of least privilege.
Let's check the service account configuration:
✅ Verification successful
The previous results show that the service account yorkie-db-provisioning-job-account
is bound to a role named db-provisioning-job-status-reader
, but we need to see the actual permissions granted by this role to verify if they follow the principle of least privilege.
Service account permissions follow least privilege principle
The yorkie-db-provisioning-job-account
service account has appropriate minimal permissions through the db-provisioning-job-status-reader
role, which only grants:
- Read-only access (
get
,list
verbs) tojobs
resources in thebatch
API group - No write permissions or access to other resources
These permissions align perfectly with the stated requirement of reading job status for the init container, following the principle of least privilege.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for service account and role definitions
rg -A 10 "yorkie-db-provisioning-job-account" --type yaml
# Search for associated role bindings
rg -A 10 "kind: (Role|ClusterRole).*yorkie-db" --type yaml
Length of output: 2354
Script:
#!/bin/bash
# Let's search for the role definition that grants permissions
rg -A 10 "db-provisioning-job-status-reader" --type yaml
# Also search for any other roles or permissions related to db provisioning
rg -A 10 "kind: Role.*db.*provisioning" --type yaml
Length of output: 1753
What this PR does / why we need it:
Extract db name as an argument to value.yaml
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation:
Checklist:
Summary by CodeRabbit
New Features
initContainer
to ensure the MongoDB provisioning job completes before starting the main application.Bug Fixes