What is your environment(Kubernetes version, Fluid version, etc.)
Describe the bug
AdvancedStatefulSetManager.updateImage refuses to do anything unless both halves of the
image reference are restated on the CacheRuntime:
// pkg/ddc/cache/component/advanced_statefulset_manager.go:268
// Both image and imageTag must be specified
if version.Image == "" || version.ImageTag == "" {
return false
}VersionSpec (api/v1alpha1/common.go:205) declares image, imageTag and imagePullPolicy
as three independent optional strings, with no defaults and no validation. Setting only
imageTag is therefore a legal edit, and it is the obvious way to say "move this component to
a newer build of the same image". It silently does nothing: the AdvancedStatefulSet is not
patched, the pods keep the old image, and there is no error, no event and no condition. The
CacheRuntime's generation still increments, so from the outside the edit looks accepted.
What makes this worth reporting rather than dismissing as "just specify both" is that the
creation path already knows how to fall back. A CacheRuntime created with only imageTag set
lands on the CacheRuntimeClass template's image, because transformComponentPodTemplate
(pkg/ddc/cache/engine/transform_common.go:123) leaves the template value in place when the
version is incomplete. The same incomplete value expresses the same intent after creation, and
there the identical guard produces nothing at all.
What you expect to happen
Setting imageTag alone should roll the component onto the template's image with the new tag,
completing the missing half from the CacheRuntimeClass the same way creation already falls
back to it. Symmetrically for image alone.
If a partially specified version is instead meant to be rejected, it should be rejected
loudly - a webhook error, or at minimum a Warning event - rather than accepted and dropped.
How to reproduce it
A CacheRuntimeClass whose worker template pins a full image reference:
topology:
worker:
template:
spec:
containers:
- name: worker
image: btxu/mooncake:v3
imagePullPolicy: IfNotPresentCreate a CacheRuntime that leaves runtimeVersion unset. The worker inherits the template
image:
$ kubectl get cacheruntime bugtest -n bugtest -o jsonpath='{.metadata.generation} {.spec.worker.runtimeVersion}'
2 {}
$ kubectl get asts bugtest-worker -n bugtest -o jsonpath='{.metadata.generation} {.spec.template.spec.containers[0].image}'
1 btxu/mooncake:v3First a control, to establish that in-place sync is working in this session at all - bump
replicas:
$ kubectl patch cacheruntime bugtest -n bugtest --type=merge -p '{"spec":{"worker":{"replicas":2}}}'
cacheruntime.data.fluid.io/bugtest patched
$ kubectl get cacheruntime bugtest -n bugtest -o jsonpath='{.metadata.generation} {.spec.worker.runtimeVersion}'
3 {}
$ kubectl get asts bugtest-worker -n bugtest -o jsonpath='{.metadata.generation} {.spec.template.spec.containers[0].image}'
2 btxu/mooncake:v3
$ kubectl get asts bugtest-worker -n bugtest -o jsonpath='{.spec.replicas}'
2AdvancedStatefulSet generation 1 -> 2, replicas applied. Now bump only the tag:
$ kubectl patch cacheruntime bugtest -n bugtest --type=merge -p '{"spec":{"worker":{"runtimeVersion":{"imageTag":"v9"}}}}'
cacheruntime.data.fluid.io/bugtest patched
$ kubectl get cacheruntime bugtest -n bugtest -o jsonpath='{.metadata.generation} {.spec.worker.runtimeVersion}'
4 {"imageTag":"v9"}
$ kubectl get asts bugtest-worker -n bugtest -o jsonpath='{.metadata.generation} {.spec.template.spec.containers[0].image}'
2 btxu/mooncake:v3The CacheRuntime moved to generation 4 and holds the new tag. The AdvancedStatefulSet is
still on generation 2 with the old image. Setting both halves is the second control, and it
works:
$ kubectl patch cacheruntime bugtest -n bugtest --type=merge -p '{"spec":{"worker":{"runtimeVersion":{"image":"btxu/mooncake","imageTag":"v9"}}}}'
cacheruntime.data.fluid.io/bugtest patched
$ kubectl get cacheruntime bugtest -n bugtest -o jsonpath='{.metadata.generation} {.spec.worker.runtimeVersion}'
5 {"image":"btxu/mooncake","imageTag":"v9"}
$ kubectl get asts bugtest-worker -n bugtest -o jsonpath='{.metadata.generation} {.spec.template.spec.containers[0].image}'
3 btxu/mooncake:v9So the sync path was live throughout: replicas propagated, the complete version propagated,
only the partial one was dropped.
The complete edit also produced events, which makes the absence of any for the partial edit
easier to see:
$ kubectl get events -n bugtest --sort-by=.lastTimestamp
...
4s Normal Killing pod/bugtest-worker-1 Container worker definition changed, will be restarted
4s Normal SuccessfulUpdatePodInPlace advancedstatefulset/bugtest-worker successfully update pod bugtest-worker-1 in-place(revision bugtest-worker-6974f6cfd9)
Nothing was emitted between the imageTag-only patch and this one. The runtime also reports
itself healthy the whole time:
$ kubectl get cacheruntime bugtest -n bugtest -o jsonpath='{range .status.conditions[*]}{.type}={.status}{"\n"}{end}'
MasterInitialized=True
WorkersReady=True
FusesInitialized=TrueFor the contrast with the creation path, a second CacheRuntime created with imageTag: v9
set from the start, against the same CacheRuntimeClass:
$ kubectl get cacheruntime bugtest2 -n bugtest -o jsonpath='{.spec.worker.runtimeVersion}'
{"imageTag":"v9"}
$ kubectl get asts bugtest2-worker -n bugtest -o jsonpath='{.metadata.generation} {.spec.template.spec.containers[0].image}'
1 btxu/mooncake:v3
$ kubectl get pods -n bugtest -o jsonpath='{range .items[*]}{.metadata.name} {.spec.containers[0].image}{"\n"}{end}' | grep bugtest2
bugtest2-master-0 btxu/mooncake:v3
bugtest2-worker-0 btxu/mooncake:v3The incomplete version falls back to the template at creation, which is the sensible
behaviour. The same value after creation produces nothing.
Additional Information
In #6173 I wrote that image is "not affected" because updateImage returns early when either
field is empty. That is accurate as far as it goes - the early return is exactly why image
never loses data the way resources do. But the early return has a failure mode of its own,
and this issue is about that one: not a value being overwritten, but an edit being accepted
and discarded.
Both are the same root cause seen from opposite sides. The CacheRuntime and the
CacheRuntimeClass template are treated as alternatives rather than a delta on a baseline, so a
partially specified value has nowhere to get its missing half from. For resources the code
picks the CacheRuntime and drops the rest (#6173); for runtimeVersion it declines to pick at
all and drops the edit. Whatever is decided about merge semantics in #6173 should settle this
one too, which is why I am filing it separately rather than folding it into that thread - the
two want the same decision but need different code and can be closed independently.
imagePullPolicy is a third field on the same struct with a different problem again: it is
applied at creation (transform_common.go:126) but updateImage never touches it, while the
doc comment on SyncComponentSpec (advanced_statefulset_manager.go:180) lists it as an
in-place updatable field. That one is out of scope here and I will report it separately.
What is your environment(Kubernetes version, Fluid version, etc.)
fluidcloudnative/cacheruntime-controller:v1.1.0-quotafix. The fixfor [BUG]container
resourcesin CacheRuntimeClass are silently dropped #6161 (PR fix(cache): preserve CacheRuntimeClass template resources when unset #6165) is not relevant here - it only touched the resources path, andupdateImageis untouched by it on master.Describe the bug
AdvancedStatefulSetManager.updateImagerefuses to do anything unless both halves of theimage reference are restated on the CacheRuntime:
VersionSpec(api/v1alpha1/common.go:205) declaresimage,imageTagandimagePullPolicyas three independent optional strings, with no defaults and no validation. Setting only
imageTagis therefore a legal edit, and it is the obvious way to say "move this component toa newer build of the same image". It silently does nothing: the AdvancedStatefulSet is not
patched, the pods keep the old image, and there is no error, no event and no condition. The
CacheRuntime's
generationstill increments, so from the outside the edit looks accepted.What makes this worth reporting rather than dismissing as "just specify both" is that the
creation path already knows how to fall back. A CacheRuntime created with only
imageTagsetlands on the CacheRuntimeClass template's image, because
transformComponentPodTemplate(pkg/ddc/cache/engine/transform_common.go:123) leaves the template value in place when the
version is incomplete. The same incomplete value expresses the same intent after creation, and
there the identical guard produces nothing at all.
What you expect to happen
Setting
imageTagalone should roll the component onto the template's image with the new tag,completing the missing half from the CacheRuntimeClass the same way creation already falls
back to it. Symmetrically for
imagealone.If a partially specified version is instead meant to be rejected, it should be rejected
loudly - a webhook error, or at minimum a Warning event - rather than accepted and dropped.
How to reproduce it
A CacheRuntimeClass whose worker template pins a full image reference:
Create a CacheRuntime that leaves
runtimeVersionunset. The worker inherits the templateimage:
First a control, to establish that in-place sync is working in this session at all - bump
replicas:AdvancedStatefulSet generation 1 -> 2, replicas applied. Now bump only the tag:
The CacheRuntime moved to generation 4 and holds the new tag. The AdvancedStatefulSet is
still on generation 2 with the old image. Setting both halves is the second control, and it
works:
So the sync path was live throughout:
replicaspropagated, the complete version propagated,only the partial one was dropped.
The complete edit also produced events, which makes the absence of any for the partial edit
easier to see:
Nothing was emitted between the
imageTag-only patch and this one. The runtime also reportsitself healthy the whole time:
For the contrast with the creation path, a second CacheRuntime created with
imageTag: v9set from the start, against the same CacheRuntimeClass:
The incomplete version falls back to the template at creation, which is the sensible
behaviour. The same value after creation produces nothing.
Additional Information
In #6173 I wrote that image is "not affected" because
updateImagereturns early when eitherfield is empty. That is accurate as far as it goes - the early return is exactly why image
never loses data the way resources do. But the early return has a failure mode of its own,
and this issue is about that one: not a value being overwritten, but an edit being accepted
and discarded.
Both are the same root cause seen from opposite sides. The CacheRuntime and the
CacheRuntimeClass template are treated as alternatives rather than a delta on a baseline, so a
partially specified value has nowhere to get its missing half from. For
resourcesthe codepicks the CacheRuntime and drops the rest (#6173); for
runtimeVersionit declines to pick atall and drops the edit. Whatever is decided about merge semantics in #6173 should settle this
one too, which is why I am filing it separately rather than folding it into that thread - the
two want the same decision but need different code and can be closed independently.
imagePullPolicyis a third field on the same struct with a different problem again: it isapplied at creation (
transform_common.go:126) butupdateImagenever touches it, while thedoc comment on
SyncComponentSpec(advanced_statefulset_manager.go:180) lists it as anin-place updatable field. That one is out of scope here and I will report it separately.