feat: add helm charts #128
Conversation
Signed-off-by: georgi-l95 <[email protected]>
WalkthroughThis PR introduces a complete Helm chart for rock-node deployment in Kubernetes, updates the persistence plugin to announce block reader capability during initialization, modifies the observability plugin's readiness check to verify this capability, adds an enabled flag to the state management plugin, and extends nginx.conf with gRPC passthrough and HTTP/2 configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as HTTP Client
participant ObsPlugin as Observability<br/>Plugin
participant PersPlugin as Persistence<br/>Plugin
participant CapReg as Capability<br/>Registry
Note over PersPlugin,CapReg: Initialization Phase
PersPlugin->>CapReg: register ProvidesBlockReader<br/>(async task)
CapReg-->>PersPlugin: capability registered
Note over Client,ObsPlugin: Readiness Check
Client->>ObsPlugin: GET /readiness
ObsPlugin->>CapReg: check ProvidesBlockReader?
alt Capability Registered
CapReg-->>ObsPlugin: ✓ Found
ObsPlugin-->>Client: (StatusCode::OK, "READY")
else Capability Not Yet Registered
CapReg-->>ObsPlugin: ✗ Not Found
ObsPlugin-->>Client: (StatusCode::SERVICE_UNAVAILABLE,<br/>"NOT_READY")
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
charts/rock-node/Chart.yaml (1)
1-9: Consider using semantic versioning forappVersion.The
appVersionis currently set to"latest", which makes it difficult to track which application version is deployed and can lead to unexpected behavior during upgrades. Consider using a specific semantic version (e.g.,"0.1.0") that matches your application releases.Apply this diff:
-appVersion: "latest" +appVersion: "0.1.0"charts/rock-node/README.md (1)
15-16: Optional: Use hyphen in compound adjective.Consider hyphenating "environment specific" to "environment-specific" when used as a compound adjective before a noun, per standard English grammar conventions.
Apply this diff:
-Override image repository, tag, and any environment specific configuration as +Override image repository, tag, and any environment-specific configuration asnginx.conf (1)
49-83: Consider using nginx map or include for repeated gRPC configuration.All gRPC location blocks share identical configuration (proxy_request_buffering, timeouts, grpc_pass, headers). While this works correctly, consider refactoring to reduce duplication, for example:
- Use an nginx map directive to handle multiple service paths
- Extract common configuration to an included file
- Use a server-level grpc_pass configuration
This would improve maintainability when adding new gRPC services or updating timeouts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
charts/rock-node/Chart.yaml(1 hunks)charts/rock-node/README.md(1 hunks)charts/rock-node/templates/_helpers.tpl(1 hunks)charts/rock-node/templates/configmap.yaml.tpl(1 hunks)charts/rock-node/templates/deployment.yaml.tpl(1 hunks)charts/rock-node/templates/ingress.yaml.tpl(1 hunks)charts/rock-node/templates/pvc.yaml.tpl(1 hunks)charts/rock-node/templates/service.yaml.tpl(1 hunks)charts/rock-node/templates/serviceaccount.yaml.tpl(1 hunks)charts/rock-node/templates/servicemonitor.yaml.tpl(1 hunks)charts/rock-node/values.yaml(1 hunks)crates/rock-node-observability-plugin/src/lib.rs(3 hunks)crates/rock-node-persistence-plugin/src/lib.rs(1 hunks)crates/rock-node-state-management-plugin/src/plugin.rs(5 hunks)nginx.conf(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
crates/rock-node-state-management-plugin/src/plugin.rs (3)
crates/rock-node-core/src/test_utils.rs (1)
create_isolated_metrics(55-58)crates/rock-node-core/src/config.rs (10)
default(23-31)default(58-63)default(76-83)default(93-95)default(115-126)default(141-149)default(165-167)default(177-179)default(189-191)default(231-239)crates/rock-node-state-management-plugin/src/state_manager.rs (1)
new(36-46)
crates/rock-node-observability-plugin/src/lib.rs (3)
crates/rock-node-state-management-plugin/src/plugin.rs (1)
create_test_context(231-251)crates/rock-node-subscriber-plugin/src/session.rs (1)
create_test_context(551-628)tests/integration/src/fixtures/mock_plugins.rs (1)
create_test_context(103-120)
🪛 LanguageTool
charts/rock-node/README.md
[grammar] ~15-~15: Use a hyphen to join words.
Context: ...age repository, tag, and any environment specific configuration as needed: ```ba...
(QB_NEW_EN_HYPHEN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Security Audit
- GitHub Check: Build and Basic Quality Checks
🔇 Additional comments (22)
crates/rock-node-persistence-plugin/src/lib.rs (1)
210-218: LGTM! Appropriate async capability registration pattern.The asynchronous registration of
ProvidesBlockReadercapability allows the observability plugin's readiness check to properly gate traffic until persistence is fully initialized. The fire-and-forget spawn pattern is appropriate here since the readiness endpoint will naturally surface any registration issues.crates/rock-node-state-management-plugin/src/plugin.rs (4)
27-43: LGTM! Proper field initialization.The
enabledfield is correctly added to the struct and safely initialized tofalsein the constructor, with the actual value being set during initialization from configuration.
54-64: LGTM! Clean conditional initialization.The initialization correctly reads the
enabledflag from configuration, stores theAppContexteven when disabled (preserving lifecycle consistency), and returns early with appropriate logging. This follows best practices for optional plugin initialization.
126-135: LGTM! Consistent enabled check in start.The start method correctly checks the
enabledflag and returns early with appropriate logging when disabled, maintaining consistency with the initialization pattern.
213-292: LGTM! Comprehensive test coverage for disabled state.The tests thoroughly verify the plugin's behavior when disabled:
- Initialization skips internal setup and doesn't register providers
- Start method doesn't launch the plugin when disabled
- Both tests use a properly configured test context with the
enabledflagThe test coverage ensures the feature works as expected.
crates/rock-node-observability-plugin/src/lib.rs (3)
27-27: LGTM! Appropriate test import.The
Capabilityimport is correctly added to support the new test that validates readiness behavior based on capability registration.
345-362: LGTM! Excellent test coverage for capability-based readiness.The test thoroughly validates the readiness check behavior:
- Correctly verifies
NOT_READYstate before capability registration- Confirms
READYstate afterProvidesBlockReadercapability is registered- Validates both status codes and response bodies
This ensures the readiness endpoint properly gates traffic until the persistence layer is initialized.
403-418: LGTM! Robust capability-based readiness check.The readiness check implementation follows Kubernetes best practices:
- Returns appropriate status codes (
SERVICE_UNAVAILABLEwhen not ready,OKwhen ready)- Checks the critical
ProvidesBlockReadercapability before reporting ready- Includes diagnostic logging for troubleshooting
- Uses a concrete return type that's more testable than
impl IntoResponseThis ensures traffic is only routed to the service after the persistence layer is fully initialized and ready to serve block queries.
charts/rock-node/templates/configmap.yaml.tpl (1)
1-15: LGTM! Well-structured ConfigMap template.The template follows Helm best practices:
- Conditional creation based on
existingConfigMapprovides deployment flexibility- Standard Kubernetes metadata with appropriate labels
- Proper use of chart helper templates for naming and labeling
- Correct indentation for the configuration content
The pattern of allowing users to provide their own ConfigMap is a good practice for advanced use cases.
charts/rock-node/templates/servicemonitor.yaml.tpl (1)
1-29: LGTM! Standard ServiceMonitor configuration.The template correctly implements Prometheus Operator integration:
- Dual condition ensures ServiceMonitor is only created when both monitoring and metrics are enabled
- Uses the correct
monitoring.coreos.com/v1apiVersion- Endpoint configuration (port, path, interval, scrapeTimeout) follows Prometheus best practices
- Label selector properly matches the service using standard Helm labels
- Optional labels and annotations provide customization flexibility
This integrates well with the observability plugin's metrics endpoint.
charts/rock-node/templates/serviceaccount.yaml.tpl (1)
1-16: LGTM! Standard ServiceAccount template.The template follows Helm best practices:
- Conditional creation based on
serviceAccount.createprovides deployment flexibility- Standard Kubernetes metadata with appropriate labels using chart helpers
- Optional annotations block allows customization when needed
- Consistent use of the
rock-node.serviceAccountNamehelper for namingThis is a well-structured template that integrates properly with the chart's naming conventions.
charts/rock-node/templates/pvc.yaml.tpl (1)
1-29: LGTM! PVC template is well-structured.The PVC template follows Helm best practices with proper conditional rendering, standard Kubernetes resource structure, and correct use of toYaml for nested objects.
nginx.conf (1)
9-20: LGTM! TCP passthrough for gRPC is correctly configured.The stream block provides raw TCP passthrough for gRPC connections. The 24-hour timeouts are appropriate for long-lived streaming connections.
charts/rock-node/values.yaml (2)
52-116: LGTM! Configuration structure is well-organized.The embedded TOML configuration is comprehensive and well-structured with:
- Clear separation between core and plugin configurations
- Reasonable defaults for timeouts and resource limits
- Explicit enabled/disabled flags for optional plugins
118-174: LGTM! Service and infrastructure configurations are well-defined.The service, ingress, probes, and persistence configurations follow Helm best practices with:
- Sensible defaults for readiness and liveness probes
- Flexible service type configuration
- Optional ingress and ServiceMonitor support
- Reasonable persistence defaults
charts/rock-node/templates/service.yaml.tpl (1)
1-36: LGTM! Service template is well-structured.The Service template correctly:
- Renders metadata with standard Kubernetes labels
- Conditionally includes annotations and nodePort values
- Exposes both gRPC and optional metrics ports
- Uses proper selectors for pod targeting
charts/rock-node/templates/ingress.yaml.tpl (2)
1-29: LGTM! Ingress template structure is well-organized.The Ingress template correctly:
- Gates rendering with ingress.enabled flag
- Supports optional ingressClassName for different ingress controllers
- Allows multiple hosts and paths configuration
- Includes standard Kubernetes labels
47-51: LGTM! TLS configuration is properly handled.The TLS section correctly uses conditional rendering and toYaml for flexible TLS configuration.
charts/rock-node/templates/deployment.yaml.tpl (3)
1-29: LGTM! Deployment metadata and replica configuration are correct.The deployment metadata, selectors, and pod template labels follow Kubernetes and Helm best practices.
62-86: LGTM! Container ports and health probes are correctly configured.The container ports and health probes are properly configured with:
- Conditional metrics port exposure
- Configurable liveness and readiness probes
- Appropriate initial delays and periods from values
87-128: LGTM! Volume mounts and scheduling configurations are well-structured.The volume mounts and scheduling sections correctly:
- Mount configuration and data volumes appropriately
- Support both existing and chart-created ConfigMaps and PVCs
- Allow customization through extraVolumes and extraVolumeMounts
- Enable flexible pod scheduling with nodeSelector, tolerations, and affinity
charts/rock-node/templates/_helpers.tpl (1)
1-32: LGTM! Helper templates follow Helm best practices.All helper templates are correctly implemented with:
- Proper name truncation to 63 characters (Kubernetes DNS label limit)
- Trailing dash trimming to avoid invalid names
- Override support for name, fullname, and namespace
- Correct service account name resolution logic based on create flag
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #128 +/- ##
==========================================
+ Coverage 79.82% 80.19% +0.36%
==========================================
Files 56 56
Lines 10504 10584 +80
==========================================
+ Hits 8385 8488 +103
+ Misses 2119 2096 -23
🚀 New features to boost your workflow:
|
Description
Add helm charts
Related Issues
Changes
Please provide a summary of the changes in this pull request.
Reviewer Checklist
Summary by CodeRabbit
Release Notes
New Features
Documentation
Bug Fixes