Extend Cache runtime framework Phase 1: add reconcile logic, left main functions to implement in next phase#5738
Conversation
|
Skipping CI for Draft Pull Request. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the CacheRuntime and its corresponding controller to the Fluid project. The changes encompass new CRD definitions, Helm chart templates for deployment and RBAC, a dedicated Dockerfile, and the initial implementation of the controller and engine logic. Review feedback highlights several areas for improvement: a copy-paste error in the status update logic, hardcoded namespaces in Helm templates that should be parameterized, and Dockerfile optimizations to reduce image size and remove hardcoded timezones. Additionally, there is a request for consistency in the Makefile's build arguments and the removal of misleading log messages.
| cp /usr/share/zoneinfo/Asia/Shanghai /etc/localtime && \ | ||
| echo "Asia/Shanghai" > /etc/timezone |
There was a problem hiding this comment.
Hardcoding the timezone to Asia/Shanghai is not ideal for a container image that might be used globally. It's better to either use UTC as a default or not set a specific timezone at all, allowing the environment to determine it if necessary. Please consider removing these lines or changing the timezone to UTC.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #5738 +/- ##
==========================================
- Coverage 61.22% 60.30% -0.93%
==========================================
Files 444 465 +21
Lines 30557 31049 +492
==========================================
+ Hits 18710 18723 +13
- Misses 10307 10786 +479
Partials 1540 1540 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
xliuqq
force-pushed
the
cache-runtime-framework
branch
from
6f7d3f3 to
1ef0b78
Compare
Signed-off-by: xliuqq <xlzq1992@gmail.com>
Signed-off-by: xliuqq <xlzq1992@gmail.com>
xliuqq
force-pushed
the
cache-runtime-framework
branch
from
1ef0b78 to
266cb37
Compare
There was a problem hiding this comment.
Pull request overview
Adds an initial “Phase 1” integration for a new CacheRuntime controller/engine, wiring it into the existing runtime reconciliation framework and Helm deployment, while leaving core engine behaviors as TODOs for a follow-up phase (curvine integration).
Changes:
- Introduces
CacheRuntimeengine scaffolding and a dedicatedcacheruntime-controllerbinary/controller. - Wires CacheRuntime into engine factory/runtime watch/precheck paths and extends runtime info lookup to include CacheRuntime.
- Updates CRDs/RBAC/Helm chart templates and values to support deploying the new controller and cluster-scoped
CacheRuntimeClass.
Reviewed changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/utils/runtimes.go | Add getters for CacheRuntime / CacheRuntimeClass |
| pkg/runtime/context.go | Clarify EngineImpl usage in reconcile context |
| pkg/ddc/factory.go | Register Cache engine build in factory |
| pkg/ddc/cache/engine/worker.go | Cache worker setup scaffolding |
| pkg/ddc/cache/engine/volume.go | Cache PV/PVC volume scaffolding (TODO) |
| pkg/ddc/cache/engine/validate.go | Cache Validate stub |
| pkg/ddc/cache/engine/ufs.go | Cache UFS preparation stub |
| pkg/ddc/cache/engine/transform.go | Cache transform stub for runtime values |
| pkg/ddc/cache/engine/sync.go | Cache Sync stub + retry duration helper |
| pkg/ddc/cache/engine/status.go | Cache status update scaffolding |
| pkg/ddc/cache/engine/shutdown.go | Cache Shutdown stub |
| pkg/ddc/cache/engine/setup.go | Cache Setup reconcile flow scaffolding |
| pkg/ddc/cache/engine/runtime.go | Helper to fetch CacheRuntime |
| pkg/ddc/cache/engine/master.go | Cache master setup scaffolding |
| pkg/ddc/cache/engine/engine.go | CacheEngine struct + Build + Precheck |
| pkg/ddc/cache/engine/dataset.go | Dataset bind/status update logic for cache |
| pkg/ddc/cache/engine/data_operation.go | Data operation “not supported” scaffolding |
| pkg/ddc/cache/engine/cm.go | Runtime-value ConfigMap creation stub |
| pkg/ddc/cache/engine/client.go | Cache client setup scaffolding |
| pkg/ddc/base/runtime.go | Add CacheRuntime handling in runtime info |
| pkg/ddc/base/interface.go | Add CacheRuntimeInterface for watch support |
| pkg/ctrl/watch/runtime.go | Allow watching CacheRuntime via new interface |
| pkg/controllers/v1alpha1/cacheruntime/implement.go | Engine map management for CacheRuntime reconciler |
| pkg/controllers/v1alpha1/cacheruntime/cacheruntime_controller.go | New CacheRuntime controller reconciler |
| pkg/controllers/deploy/runtime_controllers.go | Add cacheruntime-controller precheck |
| pkg/common/cacheruntime.go | Add CacheRuntime constants and value structs |
| docker/Dockerfile.cacheruntime | New Dockerfile for cacheruntime-controller |
| config/rbac/role.yaml | Add RBAC rules for cacheruntimes |
| config/crd/bases/data.fluid.io_cacheruntimeclasses.yaml | Make CacheRuntimeClass cluster-scoped (base CRD) |
| cmd/cache/main.go | New cacheruntime-controller main |
| cmd/cache/app/version.go | Add version subcommand |
| cmd/cache/app/init.go | Cobra command wiring |
| cmd/cache/app/cache.go | Manager/controller startup for cache controller |
| charts/fluid/fluid/values.yaml | Add Helm values for cache controller |
| charts/fluid/fluid/templates/role/dataset/rbac.yaml | Allow dataset role access to cacheruntimes |
| charts/fluid/fluid/templates/role/csi/rbac.yaml | Allow CSI role access to cacheruntimes |
| charts/fluid/fluid/templates/role/cache/rbac.yaml | Add RBAC for cacheruntime-controller |
| charts/fluid/fluid/templates/controller/cacheruntime_controller.yaml | Add cacheruntime-controller Deployment |
| charts/fluid/fluid/crds/data.fluid.io_cacheruntimeclasses.yaml | Make CacheRuntimeClass cluster-scoped (chart CRD) |
| api/v1alpha1/cacheruntimeclass_types.go | Consolidate kubebuilder resource tags |
| api/v1alpha1/cacheruntime_types.go | Add GetStatus() for CacheRuntimeInterface |
| Makefile | Add build/docker targets for cacheruntime-controller |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} | ||
| serviceAccountName: cacheruntime-controller | ||
| {{ include "fluid.controlplane.affinity" . | nindent 6}} |
There was a problem hiding this comment.
Add a whitespace before "}}" in the template directive.
| "time" | ||
| ) | ||
|
|
||
| func (e *CacheEngine) setMasterComponentStatus(status *fluidapi.RuntimeComponentStatus) (ready bool, err error) { |
There was a problem hiding this comment.
For a framework PR, leaving the real status checks for a later phase makes sense. The risky part is that this placeholder currently returns ready=true. Once the component is enabled, that will let an unfinished runtime be reported as ready. It would be safer to return false (or an explicit not-implemented error) until the actual readiness checks are in place.
Signed-off-by: xliuqq <xlzq1992@gmail.com>
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cheyang The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Ⅰ. Describe what this PR does
Add CacheRuntime Reconciler logic, leave main functions empty.
Ⅱ. Does this pull request fix one issue?
part of #5412
Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews
The CacheRuntime image can only be built manually — pushing is disabled as it is not yet ready.
Next phase is to implement related functions for integrating curvine.