Skip to content

fix: sync CacheRuntime Client component spec on every reconcile - #6155

Open
adityaupasani2 wants to merge 1 commit into
fluid-cloudnative:masterfrom
adityaupasani2:fix/cacheruntime-client-spec-sync
Open

fix: sync CacheRuntime Client component spec on every reconcile#6155
adityaupasani2 wants to merge 1 commit into
fluid-cloudnative:masterfrom
adityaupasani2:fix/cacheruntime-client-spec-sync

Conversation

@adityaupasani2

Copy link
Copy Markdown
Contributor

Ⅰ. Describe what this PR does

CacheRuntime's Client (FUSE) DaemonSet was never reconciled after initial creation. Any later change to runtime.spec.client.* (image/version, resources, tieredStore, volumes, env vars) was silently and permanently ignored, with no error or log indication.

Two compounding gaps caused this:

  1. ShouldSetupClient() gates all client reconciliation on Status.Client.Phase == RuntimePhaseNone, which only holds true once, on the very first successful setup. SetupClientInternal (and therefore Reconciler()) never runs again for the runtime's lifetime.
  2. DaemonSetManager.SyncComponentSpec was a stub that unconditionally returned an error, and syncRuntimeSpec (which DOES sync ongoing spec drift for Master/Worker) explicitly skipped Client, with a comment claiming it "will be recreated when spec changes" — no code anywhere actually implemented that.

This PR:

  • Implements a real DaemonSetManager.SyncComponentSpec that diffs and patches image/resources, mirroring AdvancedStatefulSetManager's existing update pattern. Replicas is intentionally left unused since DaemonSet replica count is determined by node count, not user input.
  • Wires Client into CacheEngine.syncRuntimeSpec so it's synced on every reconcile pass, same as Master/Worker already are.
  • Fixes 7 test fixtures in sync_test.go where the client DaemonSet had no containers — harmless before since nothing ever touched it, but a legitimate (and correct) error once SyncComponentSpec actually runs against it, since a real DaemonSet always has at least one container.

Ⅱ. Does this pull request fix one issue?

Fixes #6154

Ⅲ. List the added test cases

New file pkg/ddc/cache/component/daemonset_sync_component_spec_test.go, mirroring the existing AdvancedStatefulSetManager test suite:

  • SyncComponentSpec: image update (full/partial/unchanged), resources update (changed/unchanged), replicas ignored without error, multiple fields together, no-op when nothing changed, error when DaemonSet not found, error when DaemonSet has no containers
  • updateImage / updateResources unit tests mirroring the AdvancedStatefulSetManager equivalents

Ⅳ. Describe how to verify it

  1. Create a CacheRuntime with a Client component and wait for it to become ready.
  2. Edit runtime.spec.client.resources (or bump the runtime version).
  3. Before this fix: the Client DaemonSet's pod template is unchanged, no error anywhere. After this fix: the DaemonSet is patched and Kubernetes rolls the change out to the FUSE pods automatically.

Client (FUSE) DaemonSet spec was never updated after initial creation.
Two compounding gaps caused this:

1. ShouldSetupClient() gates all client reconciliation on
   Status.Client.Phase == RuntimePhaseNone, which only holds true once,
   on the very first successful setup. SetupClientInternal (and
   therefore Reconciler()) never runs again afterwards.

2. DaemonSetManager.SyncComponentSpec was a stub that unconditionally
   returned an error, and syncRuntimeSpec (the function that DOES sync
   ongoing spec drift for Master/Worker) explicitly skipped Client with
   a comment claiming it "will be recreated when spec changes" -- no
   code anywhere actually implemented that.

Net effect: any change to runtime.spec.client.* (image, resources,
tieredStore, volumes, env) after the client pod already existed was
silently and permanently ignored, with no error or log indication.

This commit:
- Implements a real DaemonSetManager.SyncComponentSpec that diffs and
  patches image/resources, mirroring AdvancedStatefulSetManager's
  existing pattern. Replicas is intentionally left unused since
  DaemonSet replica count is determined by node count, not user input.
- Wires Client into CacheEngine.syncRuntimeSpec so it gets synced on
  every reconcile pass, same as Master/Worker.
- Fixes 7 test fixtures in sync_test.go where the client DaemonSet
  fixture had no containers -- harmless before since nothing touched
  it, but a real (and correct) error once SyncComponentSpec actually
  runs against it, since a real DaemonSet always has at least one
  container.

Fixes fluid-cloudnative#6154

Signed-off-by: Aditya Upasani <adityaupasani29@gmail.com>
@sonarqubecloud

sonarqubecloud Bot commented Aug 8, 2026

Copy link
Copy Markdown

@codecov

codecov Bot commented Aug 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.74699% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.19%. Comparing base (05f0665) to head (5f16e28).

Files with missing lines Patch % Lines
pkg/ddc/cache/engine/sync.go 63.15% 5 Missing and 2 partials ⚠️
pkg/ddc/cache/component/daemonset_manager.go 93.75% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6155      +/-   ##
==========================================
+ Coverage   65.13%   65.19%   +0.05%     
==========================================
  Files         485      485              
  Lines       34039    34120      +81     
==========================================
+ Hits        22171    22243      +72     
- Misses      10127    10133       +6     
- Partials     1741     1744       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

Title: [BUG] CacheRuntime Client (FUSE) component is never reconciled after initial creation — spec changes are silently ignored

1 participant