Skip to content

fix: add replicas configuration for KV buckets#37

Closed
AlanSherman wants to merge 1 commit intomainfrom
asherman/nats-replicas
Closed

fix: add replicas configuration for KV buckets#37
AlanSherman wants to merge 1 commit intomainfrom
asherman/nats-replicas

Conversation

@AlanSherman
Copy link
Contributor

No description provided.

@AlanSherman AlanSherman requested a review from a team as a code owner March 18, 2026 22:28
Copilot AI review requested due to automatic review settings March 18, 2026 22:28
@coderabbitai
Copy link

coderabbitai bot commented Mar 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6418509f-567f-49de-97e0-f481332a8c47

📥 Commits

Reviewing files that changed from the base of the PR and between c460dfc and 4b7b447.

📒 Files selected for processing (2)
  • charts/lfx-v2-mailing-list-service/templates/nats-kv-buckets.yaml
  • charts/lfx-v2-mailing-list-service/values.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
  • charts/lfx-v2-mailing-list-service/templates/nats-kv-buckets.yaml
  • charts/lfx-v2-mailing-list-service/values.yaml

Walkthrough

Adds a replicas field to each NATS KeyValue bucket spec in the Helm template and corresponding replicas entries in values for five bucket types (services, service settings, mailing lists, mailing list settings, members).

Changes

Cohort / File(s) Summary
NATS KV Buckets Template
charts/lfx-v2-mailing-list-service/templates/nats-kv-buckets.yaml
Inserted replicas under each KeyValue bucket spec, sourcing value from .Values.nats.<bucket>.replicas.
NATS KV Buckets Values
charts/lfx-v2-mailing-list-service/values.yaml
Added replicas: 1 entries for five KV bucket configurations: groupsio_services_kv_bucket, groupsio_service_settings_kv_bucket, groupsio_mailing_lists_kv_bucket, groupsio_mailing_list_settings_kv_bucket, groupsio_members_kv_bucket.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided by the author, which is a significant omission for understanding the rationale and context. Add a meaningful description explaining why replicas configuration was added, any related issues, and the impact of this change on the deployment.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding replicas configuration for KV buckets in the Helm templates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch asherman/nats-replicas
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds configurable JetStream KeyValue (KV) bucket replica counts to the lfx-v2-mailing-list-service Helm chart, enabling higher durability/HA for the NATS-backed KV buckets used by the service.

Changes:

  • Add replicas settings to the chart’s default values for each GroupsIO KV bucket.
  • Render spec.replicas into each KeyValue resource manifest.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
charts/lfx-v2-mailing-list-service/values.yaml Introduces per-KV-bucket replicas defaults (currently set to 3).
charts/lfx-v2-mailing-list-service/templates/nats-kv-buckets.yaml Adds spec.replicas to the jetstream.nats.io/v1beta2 KeyValue resources.
Comments suppressed due to low confidence (1)

charts/lfx-v2-mailing-list-service/values.yaml:90

  • replicas: 3 is duplicated across multiple KV bucket configs. To reduce the chance of future drift, consider introducing a single default (e.g., nats.kvBucketDefaults.replicas) and using per-bucket overrides only where needed.
    # replicas is the number of replicas for the KV bucket
    replicas: 3
    # history is the number of history entries to keep for the KV bucket
    history: 20
    # storage is the storage type for the KV bucket
    storage: file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
charts/lfx-v2-mailing-list-service/templates/nats-kv-buckets.yaml (1)

16-16: Consider adding a default value for robustness.

The template references .Values.nats.groupsio_services_kv_bucket.replicas without a fallback default. If users upgrade this chart without updating their values files, the template could render an empty or invalid replicas field.

This applies to all five bucket definitions (lines 16, 36, 56, 76, 96).

♻️ Suggested improvement with default value
-  replicas: {{ .Values.nats.groupsio_services_kv_bucket.replicas }}
+  replicas: {{ .Values.nats.groupsio_services_kv_bucket.replicas | default 1 }}

Apply the same pattern to all five bucket definitions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@charts/lfx-v2-mailing-list-service/templates/nats-kv-buckets.yaml` at line
16, The replicas field references
.Values.nats.groupsio_services_kv_bucket.replicas without a fallback; update
each of the five bucket replicas entries to use the Helm default function (e.g.,
default 1) so the template always renders a valid integer. Locate the five
occurrences of .Values.nats.groupsio_services_kv_bucket.replicas in the
nats-kv-buckets.yaml template and replace them with a default-wrapped expression
(using default) to provide a safe fallback value for replicas across all bucket
definitions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@charts/lfx-v2-mailing-list-service/templates/nats-kv-buckets.yaml`:
- Line 16: The replicas field references
.Values.nats.groupsio_services_kv_bucket.replicas without a fallback; update
each of the five bucket replicas entries to use the Helm default function (e.g.,
default 1) so the template always renders a valid integer. Locate the five
occurrences of .Values.nats.groupsio_services_kv_bucket.replicas in the
nats-kv-buckets.yaml template and replace them with a default-wrapped expression
(using default) to provide a safe fallback value for replicas across all bucket
definitions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: de1814f7-f710-47a4-ba29-c15724bf9b5b

📥 Commits

Reviewing files that changed from the base of the PR and between ef392dd and c460dfc.

📒 Files selected for processing (2)
  • charts/lfx-v2-mailing-list-service/templates/nats-kv-buckets.yaml
  • charts/lfx-v2-mailing-list-service/values.yaml

Signed-off-by: Alan Sherman <asherman@linuxfoundation.org>
@AlanSherman AlanSherman force-pushed the asherman/nats-replicas branch from c460dfc to 4b7b447 Compare March 18, 2026 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants