Add PVC volume expansion by HoustonPutman · Pull Request #712 · apache/solr-operator

HoustonPutman · 2024-06-14T20:10:27Z

Resolves #709

Implement
Changelog
Tests
Documentation

…supported

HoustonPutman · 2024-09-06T17:41:08Z

Unfortunately, KinD does not support volume expansion by default, so we will need to find a new PVC provisioner for our integration tests.
kubernetes-sigs/kind#3734

controllers/solr_cluster_ops_util.go

janhoy · 2025-03-26T09:31:03Z

I just handled a volume scale for customer's customer today. Disks were full and we did the dance of updating sts, deleting it in orphan mode, adjusting each PVC manually, waiting for resize, and rolling restart of sts. A true pain :)

So is there perhaps any news on this, or is it an alternative to test this feature only on unit level or with mocks until we can use a true provisioner?

HoustonPutman · 2026-03-28T00:26:37Z

Ok, I removed part of the test that relied on the volumes actually changing. The tests pass and, as expected, the operator successfully updates the size requested.

This should be close to good-to-go. If you want to review, we can merge and hopefully get the next release out soon.

Co-authored-by: Steffen Moldenhauer <54577793+smoldenhauer-ish@users.noreply.github.com>

janhoy · 2026-03-30T15:10:21Z

Great. I trust Copilot’s review more than mine, so triggered one 😉

Copilot

Pull request overview

This PR adds first-class support in the Solr Operator for expanding (resizing) persistent data PVCs, working around the current Kubernetes StatefulSet limitation by patching PVCs directly and triggering a rolling restart.

Changes:

Introduces a new cluster operation (PVCExpansion) and related annotations to coordinate PVC resizing + restart.
Expands RBAC to allow patch/update on PersistentVolumeClaims.
Adds/updates E2E test coverage and supporting test harness/log collection, plus documentation/changelog updates.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`controllers/solrcloud_controller.go`	Adds PVC expansion reconciliation logic and hooks it into the reconcile loop.
`controllers/solr_cluster_ops_util.go`	Adds new `PVCExpansion` cluster-op type and handler.
`controllers/util/solr_util.go`	Adds `storageMinimumSize` annotation tracking on StatefulSet + pod template.
`config/rbac/role.yaml`	Grants patch/update permissions on PVCs for the manager role.
`helm/solr-operator/templates/role.yaml`	Grants patch/update permissions on PVCs for the Helm-installed operator role.
`tests/e2e/solrcloud_storage_test.go`	Adds an E2E test validating PVC request-size updates during expansion flow.
`tests/e2e/suite_test.go`	Enhances failure artifact collection (PVC info) and tweaks operator log filtering.
`tests/scripts/manage_e2e_tests.sh`	Patches KinD’s default StorageClass to allow expansion (workaround for kind issue).
`docs/solr-cloud/solr-cloud-crd.md`	Documents that PVC request-size can be updated for expansion, other fields remain immutable.
`docs/upgrade-notes.md`	Fixes a small typo.
`helm/solr/Chart.yaml`	Updates ArtifactHub changelog entry for the feature.

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

controllers/solrcloud_controller.go

docs/solr-cloud/solr-cloud-crd.md

Copilot · 2026-03-30T15:35:05Z

tests/e2e/suite_test.go

+	statusFile, err := os.Create(baseFilename + ".status.json")
+	defer statusFile.Close()
+	Expect(err).ToNot(HaveOccurred(), "Could not open file to save PVC status: %s", baseFilename+".status.json")


In writeAllPvcInfoToFiles, statusFile/eventsFile are deferred closed before checking the os.Create error. If Create fails, the deferred Close will panic on a nil file handle. Move the defer until after verifying err == nil (and consider handling Close errors consistently).

Copilot · 2026-03-30T15:35:05Z

tests/e2e/suite_test.go

 	if filterLinesWithString != "" {
-		filteredWriter := bytes.NewBufferString("")
 		scanner := bufio.NewScanner(podLogs)
 		for scanner.Scan() {
 			line := scanner.Text()
 			if strings.Contains(line, filterLinesWithString) {
-				io.WriteString(filteredWriter, line)
-				io.WriteString(filteredWriter, "\n")
+				io.WriteString(logFile, line)
+				io.WriteString(logFile, "\n")
 			}
 		}
-		logReader = filteredWriter
+	} else {
+		_, err = io.Copy(logFile, podLogs)
 	}

-	_, err = io.Copy(logFile, logReader)
 	Expect(err).ToNot(HaveOccurred(), "Could not write podLogs to file: %s", filename)
 }


When filterLinesWithString != "", the function never assigns to err and does not check scanner.Err() / io.WriteString errors. As a result, read/write failures (including bufio.Scanner token-too-long) can be silently ignored and the final Expect(err) will still pass. Capture and assert scanner.Err(), and propagate write errors so the test output reflects failures.

controllers/solrcloud_controller.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

HoustonPutman · 2026-03-30T19:26:42Z

good suggestions by Copilot. Implemented, and so this should probably be done?

HoustonPutman · 2026-03-31T00:16:00Z

So the integration tests are just noisier than they used to be. I'm not sure why that would be the case (after fixing the bug in the CoPilot code), but I'm going to hold-off merging until I can nail it down.

HoustonPutman · 2026-04-01T21:08:17Z

Ok, good and bad news here:

Good news: I got a local expandable-PVC setup working using the RawFile PV project from open-ebs.

Bad news: This PR doesn't work because of the way that PVC expansion works, at least for this storage provider.

The way we do it in the operator, we increase the request for all PVCs manually, then wait for them to be updated and have the new capacity. Then we issue a rolling restart so that the pods get the new storage that has been allocated to them. However this storage provider, and possibly all of them, require a pod to be restarted before the PVC's storage status gets updated to the new value. So we stall out because the Solr-operator is waiting on the PVC's statuses to be updated, and the Storage-class operator is waiting on the Pods to be updated.

We need to investigate if all storage classes operate this way, and if so change the logic to do a rolling restart when the PVCs are in FileSystemResizePending condition. Then at the end make sure all the PVCs are the right size. This is definitely more complex than the current logic.

Also in newer versions of Kubernetes, there are better ways of monitoring the PVC to see what is happening: https://kubernetes.io/blog/2025/09/19/kubernetes-v1-34-recover-expansion-failure/
These new PVC fields are beta in 1.32 and GA in 1.34, and off by default in all versions before 1.32. Since the Solr Operator supports many more versions than that, I'm hesitant to bump up the required version by so much just to make use of these. But it would make the code better IMO and more resilient when used across many different storage providers.

Lots to think about, and certainly not decisions I want to make myself.

@janhoy , would you be able to test in your setup to ensure that your storage provider behaves the same way?

janhoy · 2026-04-02T15:27:54Z

Insight from Claude on the deadlock, not sure if that helps:

The operator waits on status.capacity reaching the new size before issuing the rolling restart, but for drivers that don't implement ExpandInUsePersistentVolumes, the kubelet only updates status.capacity after expanding the filesystem on pod mount — which requires a pod restart first. Deadlock.

Fix

Pivot the wait condition from status.capacity to status.conditions. After patching PVCs, wait until each PVC is in one of:

status.capacity >= requested → online expansion complete, no restart needed
status.conditions contains FileSystemResizePending=True → block device resized, pod restart needed to expand filesystem. Then issue the rolling restart, and verify status.capacity as a post-condition rather than a gate.

Which providers require this

AWS EBS CSI, GCE PD CSI, Azure Disk CSI: support ExpandInUsePersistentVolumes, no pod restart needed, won't hit this path
OpenEBS LocalPV, local-path-provisioner, Longhorn (some modes), NFS-backed: require pod restart, set FileSystemResizePending
Rook/Ceph RBD: online. Ceph FS: offline.

Rough rule: cloud-managed block storage → online. Local/file/distributed → offline. The operator needs to handle both.

HoustonPutman added 5 commits June 14, 2024 15:09

Add PVC volume expansion

49a0c84

Fix error

7f81577

Add PVC permissions

83fc30e

Add changelog and update docs

069c684

Add integration test. Cannot test yet, since volume expansion is not …

441931f

…supported

smoldenhauer-ish reviewed Mar 19, 2025

View reviewed changes

controllers/solr_cluster_ops_util.go Outdated Show resolved Hide resolved

HoustonPutman added 2 commits March 27, 2026 15:26

Merge remote-tracking branch 'apache/main' into pvc-expanding

1fbd139

Limit testing until volume expansion is supported locally

75b9c73

Update controllers/solr_cluster_ops_util.go

0121247

Co-authored-by: Steffen Moldenhauer <54577793+smoldenhauer-ish@users.noreply.github.com>

janhoy requested a review from Copilot March 30, 2026 15:09

Copilot started reviewing on behalf of janhoy March 30, 2026 15:27 View session

Copilot AI reviewed Mar 30, 2026

View reviewed changes

HoustonPutman and others added 2 commits March 30, 2026 12:16

Apply suggestions from code review

deb8685

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Fix closing of test files on error

b100ae8

Fix cluster op logic

89a38cb

Conversation

HoustonPutman commented Jun 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HoustonPutman commented Sep 6, 2024

Uh oh!

Uh oh!

janhoy commented Mar 26, 2025

Uh oh!

HoustonPutman commented Mar 28, 2026

Uh oh!

janhoy commented Mar 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

HoustonPutman commented Mar 30, 2026

Uh oh!

HoustonPutman commented Mar 31, 2026

Uh oh!

HoustonPutman commented Apr 1, 2026

Uh oh!

janhoy commented Apr 2, 2026

Fix

Which providers require this

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

HoustonPutman commented Jun 14, 2024 •

edited

Loading