Skip to content

feat: default Sourcebot data volume to 100Gi for new installs - #154

Open
brendan-kellam wants to merge 3 commits into
mainfrom
brendan/bump-default-persistence-size
Open

brendan-kellam wants to merge 3 commits into
mainfrom
brendan/bump-default-persistence-size

Conversation

@brendan-kellam

@brendan-kellam brendan-kellam commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

New installs now get a 100Gi Sourcebot data volume instead of 10Gi. Existing volumes keep their current size on upgrade.

Why

The 10Gi default is below the sizing guide's smallest tier, which is 50 GB for up to 100 repos.

Service ping data from k8s deployments in the last 7 days:

  • The median install indexes about 400 repos and uses about 36 MiB of disk per repo, or about 14 GiB in total. That is already more than the 10Gi default.
  • 16 deployments run on the 10Gi default. Two of them are at 99–100% disk used, with 372 and 579 repos. When the disk fills, indexing fails.

100Gi matches the sizing guide's tier for 100–500 repos. It costs about $8–10 a month on gp3 or pd-balanced.

How it works

sourcebot.persistence.size now defaults to "". templates/pvc.yaml resolves the size this way:

size Existing PVC? Result
set either the value you set (same as before)
unset yes the existing PVC's current request
unset no 100Gi

Why not just change the default to 100Gi?

  • The PVC is a standalone template. Changing the default would make every upgrade on the default patch the PVC from 10Gi to 100Gi.
  • On a storage class without allowVolumeExpansion, that upgrade fails. The test run below shows this.

Why not look up the StorageClass instead?

  • Helm's lookup errors on Forbidden, not only on NotFound.
  • A lookup of the cluster-scoped StorageClass would break upgrades for anyone deploying with namespace-only permissions.
  • The existing-PVC lookup is already in the template and is namespaced, so it has no such problem.

Caveat: Argo CD

Argo CD renders the chart without cluster access, so lookup returns nothing. Argo CD users on the old default will see a PVC change from 10Gi to 100Gi. If you use Argo CD:

  • To keep your current volume, set sourcebot.persistence.size: 10Gi.
  • If your storage class can resize volumes, you can instead let the volume grow.

This caveat is in the CHANGELOG.

Testing

Unit tests

  • New tests/pvc_test.yaml suite. It fakes the existing-PVC lookup with kubernetesProvider. It covers:
    • new install defaults to 100Gi
    • an explicit size on a new install
    • an existing PVC keeps its size, and its volumeName
    • an explicit size on an existing PVC
    • a PVC with the same name in another namespace is ignored
  • I checked that the suite catches a regression: when the template ignores the existing PVC, the "keeps its size" test fails.
  • helm unittest: 25/25 pass. helm lint passes. helm-docs is up to date.

Real cluster

I ran real helm install and helm upgrade commands on minikube. Its standard StorageClass has no allowVolumeExpansion.

Step Result
Install main (10Gi default) PVC 10Gi, bound
Upgrade main with size=100Gi (what a plain default bump would do) UPGRADE FAILED: … storageclass that provisions the pvc must support resize
Upgrade to this branch, size unset deployed. PVC still 10Gi, same bound volume
Fresh install of this branch PVC 100Gi
Upgrade with explicit size=20Gi fails with the same resize error as on main. This is unchanged: an explicit size is passed through as before.

🤖 Generated with Claude Code


Note

Medium Risk
Changes persistent volume sizing behavior on upgrade and GitOps renders; Argo CD users on the old implicit default may get an unintended 100Gi PVC spec unless they set size explicitly.

Overview
New installs now provision a 100Gi Sourcebot data volume instead of 10Gi. The chart value sourcebot.persistence.size defaults to "", and pvc.yaml resolves storage at render time: an explicit size still wins; when unset, an existing PVC keeps its current request (via Helm lookup), otherwise 100Gi is used.

This avoids upgrade failures on storage classes that cannot resize volumes, which a straight default bump would trigger. Argo CD users who render without cluster access may still see a manifest jump to 100Gi unless they pin sourcebot.persistence.size: 10Gi (documented in CHANGELOG/README).

Adds a helm unittest suite for PVC sizing and updates chart/docs copy to point at the sizing guide.

Reviewed by Cursor Bugbot for commit 8ae7e89. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • New Features
    • New Sourcebot installations now default to a 100Gi data volume. On upgrades, existing volumes retain their current size unless you explicitly configure a size in the persistence settings.
    • You can set a specific size for new or existing volumes; changing the size does not change an existing volume’s name.
  • Documentation
    • Updated storage sizing guidance, including instructions for Argo CD users who want to retain the previous 10Gi default or choose a larger size when supported by their storage class.

@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

New installs use a 100Gi PVC request by default. When size is unset, the chart retains an existing PVC’s requested size. An explicitly configured size takes precedence. Tests and documentation describe these sizing rules and upgrade behavior.

Changes

PVC sizing

Layer / File(s) Summary
PVC sizing resolution and documented defaults
charts/sourcebot/values.yaml, charts/sourcebot/templates/pvc.yaml, charts/sourcebot/tests/pvc_test.yaml, charts/sourcebot/README.md, README.md, CHANGELOG.md
The chart uses an explicit size when set. Otherwise, it uses the existing PVC request or defaults to 100Gi. Tests cover new and existing PVCs, including namespace isolation. Documentation describes the default and upgrade behavior, and the changelog notes the Argo CD guidance.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: Minimal · up to 8ae7e

The default example now preserves an existing volume’s requested size on upgrade. No actionable merge-blocking risk remains from this change.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "feat: default Sourcebot data volume to 100Gi for new installs" matches the main change in the changeset. The chart default for sourcebot.persistence.size moves from 10Gi to unset, and the P…
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "feat: default Sourcebot data volume to 100Gi for new installs" matches the main change in the changeset. The chart default for sourcebot.persistence.size moves from 10Gi to unset, and the PVC template resolves to 100Gi for new installs. Existing PVCs keep their requested size on upgrade. The title names the primary behavior change and the affected scope (new installs), and it follows a conventional commit style. It is short, specific, and free of file lists or vague terms.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)

Finishing Touches
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 54a5a1a5-7b52-424d-9b0a-1281e74dcfb1
Commits

Reviewing files that changed from the base of the PR and between 2842ec4 and 8ae7e89.

Files selected for processing: 1
  • README.md
Files skipped from review as they are similar to previous changes: 1
  • README.md

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  • Fix comments: Fix CodeRabbit comments on this PR

Comment thread README.md Resolved
brendan-kellam and others added 2 commits November 3, 2026 13:28
The 10Gi default is below the sizing guide's smallest tier (50 GB for up
to 100 repos). The typical k8s install indexes about 400 repos, and
several deployments on the default have filled their volume.

`sourcebot.persistence.size` now defaults to unset:
- an explicit size always wins (unchanged behaviour)
- when unset, an existing PVC keeps its current size, so the upgrade
  never tries to resize a live volume. Resizing fails on storage classes
  without allowVolumeExpansion.
- when unset on a new install, the PVC gets 100Gi

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@brendan-kellam
brendan-kellam force-pushed the brendan/bump-default-persistence-size branch from b12fe7b to 2842ec4 Compare November 3, 2026 21:28
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Helm Unit Test Results

25 tests  +5   25 ✅ +5   0s ⏱️ ±0s
 4 suites +1    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 8ae7e89. ± Comparison against base commit 60a6828.

♻️ This comment has been updated with latest results.

Copying an explicit size into an upgrade would resize an existing volume,
which fails on storage classes without allowVolumeExpansion.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

1 participant