feat(cg): add raw block device support for cache directories - #124
Conversation
Signed-off-by: Xuhui zhang <xuhui@juicedata.io>
Signed-off-by: Xuhui zhang <xuhui@juicedata.io>
Signed-off-by: Xuhui zhang <xuhui@juicedata.io>
| if r.actualShouldbeUpdate(updateStrategyType, expectWorker, actualState) { | ||
| // only update respecting maxUnavailable strategy | ||
| if actualState != nil { | ||
| if actualState != nil && utils.IsPodReady(*actualState) { |
There was a problem hiding this comment.
原先 起不来的 worker 被计入 numUnavailable,达到 maxUnavailable 后又禁止更新该 worker
worker 必须更新配置才能恢复,因此一直卡住
改成 已经 NotReady 的 worker:允许按新配置重建,因为更新不会增加不可用数量
There was a problem hiding this comment.
Pull request overview
This PR adds raw block-device support for CacheGroup worker cache directories by allowing PVC-backed cache dirs to be provisioned/used in Block mode, mounting them inside the container, and optionally formatting unknown devices; it also introduces CRD-level validation for cacheDir fields.
Changes:
- Add
volumeMode+formatto cacheDirs, and mount block-mode cache PVCs viavolumeDeviceswith ablkid/mkfs.ext4/mountflow. - Update clean-cache Job generation to handle cache dirs that are exposed as block devices.
- Add CRD/CEL validation rules and unit tests to validate cacheDir schema constraints.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/builder/pod_test.go | Adds unit test coverage for cacheDirs mounted as block volumeDevices and the resulting init/mount commands. |
| pkg/builder/job.go | Updates clean-cache job to mount block devices (when present) before removing cache contents. |
| pkg/builder/cache_group_pod.go | Implements block-device cache dir handling: volumeDevices + pre-mount script injected into container command. |
| internal/controller/cachegroup_controller.go | Tweaks update gating to respect maxUnavailable only when updating a ready pod; improves finalizer behavior when secret is missing. |
| api/v1/cachegroup_types.go | Adds API fields and kubebuilder XValidation rules for cacheDir constraints. |
| api/v1/cachegroup_validation_test.go | Adds CRD/CEL validation tests for cacheDir rules. |
| config/crd/bases/juicefs.io_cachegroups.yaml | Regenerates CRD schema to include new fields and validations. |
| dist/crd.yaml | Updates distributable CRD output with new schema and validations. |
| config/samples/v1_cachegroup.yaml | Updates sample manifest to demonstrate block-mode PVC cacheDir settings. |
| go.mod | Adds direct dependencies needed to run CRD/CEL validation tests. |
| go.sum | Updates module sums for newly introduced dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 11 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
pkg/builder/cache_group_pod.go:279
genCacheDirscan panic when a CacheDir usesVolumeClaimTemplatesbutvolumeClaimTemplateis nil (it dereferencesdir.VolumeClaimTemplatebefore any other validation). SinceNewCacheGroupWorkercallsgenCacheDirsunconditionally, this can crash the controller on invalid/older objects; add an early guard before reading template fields.
for i, dir := range p.spec.CacheDirs {
cachePathInContainer := fmt.Sprintf("%s%d", common.CacheDirVolumeMountPathPrefix, i)
volumeName := fmt.Sprintf("%s%d", common.CacheDirVolumeNamePrefix, i)
isVolumeDevice := dir.Type == juicefsiov1.CacheDirTypePVC &&
dir.VolumeMode == corev1.PersistentVolumeBlock
go.mod:17
k8s.io/apiextensions-apiserverandk8s.io/apiserverare pinned to v0.32.1 while the rest of the Kubernetes libraries are v0.32.2. Mixing patch versions in the Kubernetes module set commonly causes dependency skew and hard-to-debug build/test issues; please align these to the same patch version as the otherk8s.io/*deps (likely v0.32.2) and regenerate go.sum.
k8s.io/api v0.32.2
k8s.io/apiextensions-apiserver v0.32.1
k8s.io/apimachinery v0.32.2
k8s.io/apiserver v0.32.1
k8s.io/client-go v0.32.2
Signed-off-by: Xuhui zhang <xuhui@juicedata.io>
Signed-off-by: Xuhui zhang <xuhui@juicedata.io>
Signed-off-by: Xuhui zhang <xuhui@juicedata.io>
| echo "Cache device $CACHE_DEVICE does not contain a recognized filesystem; set cacheDirs[].format to true to format it" >&2 | ||
| exit 1 | ||
| fi | ||
| mkfs.ext4 -F "$CACHE_DEVICE" || exit 1 |
There was a problem hiding this comment.
blkid 无法识别文件系统,并不一定代表设备是空盘,而 mkfs.ext4 -F 会强制覆盖。
建议在格式化前检查:
| mkfs.ext4 -F "$CACHE_DEVICE" || exit 1 | |
| WIPEFS_OUTPUT=$(wipefs -n "$CACHE_DEVICE" 2>/dev/null) | |
| if [ -n "$WIPEFS_OUTPUT" ]; then | |
| echo "Cache device $CACHE_DEVICE is not empty; refusing to format it automatically" >&2 | |
| exit 1 | |
| fi | |
| mkfs.ext4 -F "$CACHE_DEVICE" || exit 1 |
There was a problem hiding this comment.
这里加了 format 字段,。format 默认为 false,显式设为 true,才执行 format
There was a problem hiding this comment.
即使是加了 format ,依然不能避免格式化非空盘,建议这里加一层检查,降低风险
There was a problem hiding this comment.
可是wipefs 只是检查是否是某些特定格式的签名(只检测几个字节),并不能证明他是非空呀, 比如直接通过 dd 写数据,这种是检测不出来的。真正证明完全是空的需要扫完完整盘,成本非常高
所以我们这里的 format 的含义就是检测出来非文件系统的设备,是否格式化,用户自己把握。
There was a problem hiding this comment.
跟 @SandyXSD 讨论后结论如下:
- wipefs 不能完全保证是空盘,但多加了一层保护,降低风险
mkfs.ext4 -F不应该加 -F,可以在mkfs.ext4 "$CACHE_DEVICE"出错后,再检查是否已经有文件系统,避免多 pod 同时格式化
There was a problem hiding this comment.
加上了 wipefs 二次检查
-F 就不去掉了,他只是确认是否格式化,加上他不会绕过已有文件系统检查,需要两次 -F -F 才可以。
Signed-off-by: Xuhui zhang <xuhui@juicedata.io>
What this PR does
volumeModeto PVC cache directories, supportingFilesystemandBlock.volumeClaimTemplate.spec.volumeModefor dynamically provisioned PVCs.volumeDevices.blkidto detect whether a block device contains a recognized filesystem.format: true.Add Validation rules
HostPathrequirespath.PVCrequiresname.volumeModeis only valid forPVC.formatis only valid for block-mode PVCs or block-mode volume claim templates.HostPathcache directories do not support block volume mode.