fix: sync CacheRuntime Client component spec on every reconcile - #6155
Open
adityaupasani2 wants to merge 1 commit into
Open
fix: sync CacheRuntime Client component spec on every reconcile#6155adityaupasani2 wants to merge 1 commit into
adityaupasani2 wants to merge 1 commit into
Conversation
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>
|
Codecov Report❌ Patch coverage is
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. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Ⅰ. 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:
This PR:
Ⅱ. 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:
Ⅳ. Describe how to verify it