-
Notifications
You must be signed in to change notification settings - Fork 91
[Feature] PVC Expansion Support for StarRocks Kubernetes Operator #655
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Deng Liu <[email protected]>
Signed-off-by: Deng Liu <[email protected]>
2543e60
to
888632e
Compare
Thank you very much for submitting such an important Feature again. If this feature can be used as a switch option, I think it would be better for existing users. |
|
||
1. **Storage Class Support**: Your storage class must support volume expansion | ||
2. **Kubernetes Version**: Kubernetes 1.11+ (for PVC expansion support) | ||
3. **Operator Version**: StarRocks operator v1.9.9+ (with PVC expansion feature) |
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.
Now the newest version is v1.10.2.
return nil | ||
} else { | ||
// PVC sizes + other changes - delete StatefulSet first, then expand PVCs, then recreate | ||
logger.Info("PVC expansion with other StatefulSet changes, will delete StatefulSet first for safe expansion", "count", len(expansionResult.PVCsToExpand)) |
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.
The are some errors reported by golangci-lint
, please see https://github.com/StarRocks/starrocks-kubernetes-operator/actions/runs/15699899736/job/44232198023?pr=655
|
||
// ApplyStatefulSetWithPVCExpansion applies a StatefulSet with support for PVC expansion. | ||
// It detects when PVC sizes need to be expanded and handles the expansion process. | ||
func ApplyStatefulSetWithPVCExpansion(ctx context.Context, k8sClient client.Client, expect *appv1.StatefulSet, |
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.
Can we provide a command - line argument for the operator as an optional feature? That is, by default, the operator still directly calls ApplyStatefulSet
. When the operator is launched with a certain parameter, for example, --with- pvc-expansion
, then the operator calls ApplyStatefulSetWithPVCExpansion
.
} | ||
|
||
// waitForStatefulSetDeletion waits for a StatefulSet to be fully deleted | ||
func waitForStatefulSetDeletion(ctx context.Context, k8sClient client.Client, namespace, name string) error { |
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.
According to my tests, deleting a StatefulSet is a very quick action and does not require loop waiting. If you need to wait for the POD to be deleted, the logic of waiting for five minutes is a bit imprecise. If it returns due to timeout, the operator will fail before Step 3: Wait for PVCs to be fully detached
} | ||
|
||
// Step 3: Wait for PVCs to be fully detached | ||
err = waitForPVCsDetached(ctx, k8sClient, expansionInfos) |
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.
If waitForPVCsDetached
fails, it will cause the expansion of PVC to fail and the creation of the Statefulset object to fail. Then, the Operator will rebuild the Statefulset object in the subsequent reconciliation. However, there will be the following error: The storage size set in StarRocksCluster is equal to the storage size set in the Statefulset. But the value set in the PVC has not changed at all.
err := k8sClient.Get(ctx, types.NamespacedName{ | ||
Namespace: namespace, | ||
Name: statefulSetName, | ||
}, &sts) |
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.
There will be an situation: The storage size set in StarRocksCluster is equal to the storage size set in the Statefulset. But the value set in the PVC has not changed at all.
So I think check the existing Statefulset is not enough.
} | ||
|
||
// Step 5: Wait for PVC expansion to complete | ||
err = waitForPVCExpansionComplete(ctx, k8sClient, expansionInfos) |
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.
If the PVC Expansion has not been completed and a timeout occurs, what problems will occur if the newly created POD is attached to the PVC?
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.
In addition, there is a situation where although the Operator has executed the ExpandPVCs
operation, Kubernetes has not started the Expansion, and the Operator may mistakenly think that the Expand has been successful.
{{- fail (printf "Log storage size cannot be zero for %s component: %s" $component $logStorageSize) -}} | ||
{{- end -}} | ||
{{- if and $spillStorageSize (regexMatch "^0" $spillStorageSize) -}} | ||
{{- fail (printf "Spill storage size cannot be zero for %s component: %s" $component $spillStorageSize) -}} |
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.
Setting the storage size to 0 is no longer allowed here. This conflicts with the behavior of the Operator. Please refer to
# If you set it to 0Gi, the related PVC will not be created, and the log will not be persisted. |
// Delete the StatefulSet immediately since VolumeClaimTemplates are immutable | ||
// This is more efficient than scaling to 0 first | ||
logger.Info("deleting StatefulSet to update VolumeClaimTemplates") | ||
err = k8sClient.Delete(ctx, ¤t) |
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.
Do we need to adjust the RBAC configuration of Operator?
Deng Liu seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Description
This PR implements comprehensive PVC (Persistent Volume Claim) expansion support for the StarRocks Kubernetes operator, enabling users to expand storage volumes for FE, BE, and CN components without data loss.
✨ Key Features
🔧 Core Functionality
allowVolumeExpansion: true
)🛡️ Safety Features
⚡ Performance Optimizations
🔄 Expansion Strategies
The operator implements three intelligent expansion strategies:
🧪 Testing
Comprehensive Test Coverage
Test Results
$ go test ./pkg/k8sutils/... -v === RUN TestDetectPVCExpansion === RUN TestExpandPVCs === RUN TestValidateStorageClassExpansion === RUN TestCheckStorageClassRequiresDetachment --- PASS: All tests (0.366s)
🔍 Technical Implementation
Key Components
PVC Expansion Detection
Storage Class Validation
allowVolumeExpansion: true
before attempting expansionSmart StatefulSet Management
🛡️ Safety Guarantees
Data Protection
Operational Safety
📖 Documentation
Please list any related issues and link them here.
#121
Checklist
For operator, please complete the following checklist:
make generate
to generate the code.golangci-lint run
to check the code style.make test
to run UT.make manifests
to update the yaml files of CRD.For helm chart, please complete the following checklist:
file of starrocks chart.
scripts
directory, runbash create-parent-chart-values.sh
to update the values.yaml file of the parentchart( kube-starrocks chart).