Skip to content

Commit c6526ed

Browse files
committed
fix(cache): propagate runtime-level podMetadata and imagePullSecrets
`CacheRuntimeSpec.PodMetadata` and `CacheRuntimeSpec.ImagePullSecrets` are in the CRD and the API server takes them, but nothing on the transform path reads them. `transformComponentPodTemplate` composes a component's pod template from the CacheRuntimeClass template and the component-level spec only, so the runtime-level layer never enters and neither field reaches the pod. Pass `runtime.Spec` into `transformComponentPodTemplate` and layer `podMetadata` the way `spec.options` already is: template < runtime level < component level. `imagePullSecrets` has no component-level counterpart, so the runtime-level list is merged onto whatever the template declares, comparing by name -- the CRD marks the field with a merge patch strategy keyed on name, so a secret named at both layers should be carried once. Both fields are read on the transform path, which runs at setup, so this does not make them updatable on a live CacheRuntime; `syncRuntimeSpec` still handles only runtimeVersion, resources and replicas. Fixes #6184
1 parent f2785f8 commit c6526ed

6 files changed

Lines changed: 608 additions & 12 deletions

File tree

Lines changed: 380 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,380 @@
1+
/*
2+
Copyright 2026 The Fluid Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package engine
18+
19+
import (
20+
"context"
21+
22+
. "github.com/onsi/ginkgo/v2"
23+
. "github.com/onsi/gomega"
24+
25+
workloadv1alpha1 "github.com/fluid-cloudnative/advanced-statefulset/api/workload/v1alpha1"
26+
datav1alpha1 "github.com/fluid-cloudnative/fluid/api/v1alpha1"
27+
"github.com/fluid-cloudnative/fluid/pkg/common"
28+
cruntime "github.com/fluid-cloudnative/fluid/pkg/runtime"
29+
corev1 "k8s.io/api/core/v1"
30+
"k8s.io/apimachinery/pkg/api/resource"
31+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
32+
"k8s.io/apimachinery/pkg/types"
33+
ctrl "sigs.k8s.io/controller-runtime"
34+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
35+
)
36+
37+
// This file is the executable form of the pod-template composition table tracked in #6185.
38+
//
39+
// A component's pod template is composed from three layers:
40+
//
41+
// L1 CacheRuntimeClass.topology.<component>.template (a full PodTemplateSpec)
42+
// L2 CacheRuntime.spec.* (runtime level, all components)
43+
// L3 CacheRuntime.spec.<component>.* (component level)
44+
//
45+
// Every row below states what each layer declares and what the component must end up
46+
// with. Each row is then replayed through BOTH code paths that compose those layers:
47+
//
48+
// create the transform path, which builds the workload from all three layers at once
49+
// update the sync path, which patches a workload created from L1 alone
50+
//
51+
// The two paths are written independently (transform_common.go and
52+
// advanced_statefulset_manager.go) and are expected to agree. A row that passes on one
53+
// path and fails on the other is a divergence bug, which is the class #6185 is about.
54+
//
55+
// Rows carrying a knownBug tag pin CURRENT behaviour, not intended behaviour. Do not
56+
// read them as an endorsement: each one names the issue that will change it, and the
57+
// comment states what the row should say once that issue is fixed.
58+
59+
// --- the table ------------------------------------------------------------------
60+
61+
type layerCase struct {
62+
// field names the row in the table. Several rows may share a field.
63+
field string
64+
desc string
65+
66+
l1 func(*corev1.PodTemplateSpec)
67+
l2 func(*datav1alpha1.CacheRuntimeSpec)
68+
l3 func(*datav1alpha1.CacheRuntimeWorkerSpec)
69+
70+
// want asserts on the pod template the component ends up with, whichever path produced
71+
// it. The whole template is handed over, not just the PodSpec, because podMetadata and
72+
// imagePullSecrets live on different halves of it.
73+
want func(g Gomega, tmpl corev1.PodTemplateSpec)
74+
75+
// onlyPaths restricts the row to the named paths. Empty means both. Use it only for
76+
// fields a path genuinely cannot carry, and say why -- not to silence a divergence.
77+
onlyPaths []string
78+
79+
// knownBug names the issue this row's expectation is wrong under. Non-empty means the
80+
// row pins current behaviour so a change to it is visible in review.
81+
knownBug string
82+
}
83+
84+
func compositionTable() []layerCase {
85+
return []layerCase{
86+
// -- podMetadata: union, later layer wins a key ------------------------------
87+
{
88+
field: "podMetadata.labels",
89+
desc: "L2 alone reaches the pod",
90+
l2: func(s *datav1alpha1.CacheRuntimeSpec) {
91+
s.PodMetadata.Labels = map[string]string{"from": "runtime"}
92+
},
93+
want: func(g Gomega, t corev1.PodTemplateSpec) {
94+
g.Expect(t.Labels).To(HaveKeyWithValue("from", "runtime"))
95+
},
96+
onlyPaths: []string{pathCreate}, // sync path does not patch pod metadata
97+
},
98+
99+
// -- imagePullSecrets: merge by name -----------------------------------------
100+
{
101+
field: "imagePullSecrets",
102+
desc: "L1 and L2 merge, a name declared twice appears once",
103+
l1: func(t *corev1.PodTemplateSpec) {
104+
t.Spec.ImagePullSecrets = []corev1.LocalObjectReference{{Name: "tmpl"}, {Name: "shared"}}
105+
},
106+
l2: func(s *datav1alpha1.CacheRuntimeSpec) {
107+
s.ImagePullSecrets = []corev1.LocalObjectReference{{Name: "shared"}, {Name: "rt"}}
108+
},
109+
want: func(g Gomega, t corev1.PodTemplateSpec) {
110+
g.Expect(t.Spec.ImagePullSecrets).To(ConsistOf(
111+
corev1.LocalObjectReference{Name: "tmpl"},
112+
corev1.LocalObjectReference{Name: "shared"},
113+
corev1.LocalObjectReference{Name: "rt"}))
114+
},
115+
onlyPaths: []string{pathCreate},
116+
},
117+
118+
// -- resources: the row #6173 and #6185 turn on ------------------------------
119+
{
120+
field: "resources",
121+
desc: "L3 restates only limits.memory over an L1 that declares four keys",
122+
l1: func(t *corev1.PodTemplateSpec) {
123+
t.Spec.Containers[0].Resources = corev1.ResourceRequirements{
124+
Requests: corev1.ResourceList{
125+
corev1.ResourceCPU: resource.MustParse("1"),
126+
corev1.ResourceMemory: resource.MustParse("2Gi"),
127+
},
128+
Limits: corev1.ResourceList{
129+
corev1.ResourceCPU: resource.MustParse("2"),
130+
corev1.ResourceMemory: resource.MustParse("4Gi"),
131+
},
132+
}
133+
},
134+
l3: func(w *datav1alpha1.CacheRuntimeWorkerSpec) {
135+
w.Resources = corev1.ResourceRequirements{
136+
Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("8Gi")},
137+
}
138+
},
139+
// CURRENT: the whole struct is replaced, so the three keys L3 did not restate
140+
// are lost and the container ends up with no CPU request or limit at all.
141+
// AFTER #6173: requests {cpu 1, memory 2Gi}, limits {cpu 2, memory 8Gi}.
142+
knownBug: "#6173",
143+
want: func(g Gomega, t corev1.PodTemplateSpec) {
144+
r := t.Spec.Containers[0].Resources
145+
g.Expect(r.Limits).To(HaveKeyWithValue(corev1.ResourceMemory, resource.MustParse("8Gi")))
146+
g.Expect(r.Limits).NotTo(HaveKey(corev1.ResourceCPU))
147+
g.Expect(r.Requests).To(BeEmpty())
148+
},
149+
},
150+
151+
// -- runtimeVersion: the guard #6178 is about --------------------------------
152+
{
153+
field: "runtimeVersion",
154+
desc: "L3 sets imageTag only, over an L1 that names an image",
155+
l1: func(t *corev1.PodTemplateSpec) {
156+
t.Spec.Containers[0].Image = "fluid/cache:v1"
157+
},
158+
l3: func(w *datav1alpha1.CacheRuntimeWorkerSpec) {
159+
w.RuntimeVersion = datav1alpha1.VersionSpec{ImageTag: "v2"}
160+
},
161+
// CURRENT: the guard wants both image and imageTag, so the tag is dropped.
162+
// AFTER #6178: fluid/cache:v2.
163+
knownBug: "#6178",
164+
want: func(g Gomega, t corev1.PodTemplateSpec) {
165+
g.Expect(t.Spec.Containers[0].Image).To(Equal("fluid/cache:v1"))
166+
},
167+
},
168+
169+
// -- args vs env: replace on one, append on the other ------------------------
170+
{
171+
field: "args",
172+
desc: "L3 replaces the template args wholesale",
173+
l1: func(t *corev1.PodTemplateSpec) {
174+
t.Spec.Containers[0].Args = []string{"--from-template"}
175+
},
176+
l3: func(w *datav1alpha1.CacheRuntimeWorkerSpec) {
177+
w.Args = []string{"--from-component"}
178+
},
179+
// Intended: the CRD marks args +listType=atomic, so replacing is correct.
180+
want: func(g Gomega, t corev1.PodTemplateSpec) {
181+
g.Expect(t.Spec.Containers[0].Args).To(Equal([]string{"--from-component"}))
182+
},
183+
onlyPaths: []string{pathCreate}, // args are not an in-place update field
184+
},
185+
{
186+
field: "env",
187+
desc: "L3 restates a name the template already sets",
188+
l1: func(t *corev1.PodTemplateSpec) {
189+
t.Spec.Containers[0].Env = []corev1.EnvVar{{Name: "SHARED", Value: "template"}}
190+
},
191+
l3: func(w *datav1alpha1.CacheRuntimeWorkerSpec) {
192+
w.Env = []corev1.EnvVar{{Name: "SHARED", Value: "component"}}
193+
},
194+
// CURRENT: appended without dedup, so SHARED appears twice. The CRD marks env
195+
// +patchStrategy=merge +patchMergeKey=name, which says it should appear once.
196+
// Kubernetes takes the last value, so the effective value is already correct.
197+
knownBug: "#6185 (env/annotation mismatch)",
198+
want: func(g Gomega, t corev1.PodTemplateSpec) {
199+
var shared []string
200+
for _, e := range t.Spec.Containers[0].Env {
201+
if e.Name == "SHARED" {
202+
shared = append(shared, e.Value)
203+
}
204+
}
205+
g.Expect(shared).To(Equal([]string{"template", "component"}))
206+
},
207+
onlyPaths: []string{pathCreate},
208+
},
209+
210+
// -- nodeSelector: implementation and annotation disagree --------------------
211+
{
212+
field: "nodeSelector",
213+
desc: "L3 sets a key the template does not",
214+
l1: func(t *corev1.PodTemplateSpec) {
215+
t.Spec.NodeSelector = map[string]string{"tier": "template"}
216+
},
217+
l3: func(w *datav1alpha1.CacheRuntimeWorkerSpec) {
218+
w.NodeSelector = map[string]string{"zone": "a"}
219+
},
220+
// CURRENT: union, so the template key survives. The CRD marks nodeSelector
221+
// +mapType=atomic, which says the whole map should be replaced.
222+
knownBug: "#6185 (nodeSelector/annotation mismatch)",
223+
want: func(g Gomega, t corev1.PodTemplateSpec) {
224+
g.Expect(t.Spec.NodeSelector).To(HaveKeyWithValue("tier", "template"))
225+
g.Expect(t.Spec.NodeSelector).To(HaveKeyWithValue("zone", "a"))
226+
},
227+
onlyPaths: []string{pathCreate},
228+
},
229+
}
230+
}
231+
232+
// --- the two paths --------------------------------------------------------------
233+
234+
const (
235+
pathCreate = "create (transform)"
236+
pathUpdate = "update (sync)"
237+
)
238+
239+
type compositionPath struct {
240+
name string
241+
// run applies the three layers and returns the pod template the component ends up with.
242+
run func(g Gomega, tc layerCase) corev1.PodTemplateSpec
243+
}
244+
245+
func baseTemplate() corev1.PodTemplateSpec {
246+
return corev1.PodTemplateSpec{
247+
Spec: corev1.PodSpec{
248+
Containers: []corev1.Container{{Name: "worker", Image: "fluid/cache:v1"}},
249+
},
250+
}
251+
}
252+
253+
func layersOf(tc layerCase) (corev1.PodTemplateSpec, datav1alpha1.CacheRuntimeSpec) {
254+
tmpl := baseTemplate()
255+
if tc.l1 != nil {
256+
tc.l1(&tmpl)
257+
}
258+
runtimeSpec := datav1alpha1.CacheRuntimeSpec{RuntimeClassName: "test-class"}
259+
if tc.l2 != nil {
260+
tc.l2(&runtimeSpec)
261+
}
262+
worker := datav1alpha1.CacheRuntimeWorkerSpec{Replicas: 1}
263+
if tc.l3 != nil {
264+
tc.l3(&worker)
265+
}
266+
runtimeSpec.Worker = worker
267+
return tmpl, runtimeSpec
268+
}
269+
270+
// createPath composes all three layers the way workload creation does.
271+
var createPath = compositionPath{
272+
name: pathCreate,
273+
run: func(g Gomega, tc layerCase) corev1.PodTemplateSpec {
274+
tmpl, runtimeSpec := layersOf(tc)
275+
e := &CacheEngine{name: "test", namespace: "default"}
276+
277+
value, err := e.initComponentValue(common.ComponentTypeWorker,
278+
&datav1alpha1.RuntimeComponentDefinition{Template: tmpl}, nil, runtimeSpec.Worker.Replicas)
279+
g.Expect(err).NotTo(HaveOccurred())
280+
281+
e.transformComponentPodTemplate(runtimeSpec, runtimeSpec.Worker.RuntimeComponentCommonSpec,
282+
&datav1alpha1.Dataset{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}}, value)
283+
284+
return value.PodTemplateSpec
285+
},
286+
}
287+
288+
// updatePath seeds a workload built from L1 alone -- the state a CacheRuntime that set
289+
// nothing would have produced -- then applies L2 and L3 through the sync path and reads
290+
// the workload back. The end state must match the create path's.
291+
var updatePath = compositionPath{
292+
name: pathUpdate,
293+
run: func(g Gomega, tc layerCase) corev1.PodTemplateSpec {
294+
tmpl, runtimeSpec := layersOf(tc)
295+
name := common.GetCacheComponentName("test", common.ComponentTypeWorker)
296+
297+
seeded := baseTemplate()
298+
if tc.l1 != nil {
299+
tc.l1(&seeded)
300+
}
301+
replicas := int32(1)
302+
asts := &workloadv1alpha1.AdvancedStatefulSet{
303+
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"},
304+
Spec: workloadv1alpha1.AdvancedStatefulSetSpec{
305+
Replicas: &replicas,
306+
Template: seeded,
307+
},
308+
}
309+
310+
runtimeObj := &datav1alpha1.CacheRuntime{
311+
ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"},
312+
Spec: runtimeSpec,
313+
}
314+
runtimeClass := &datav1alpha1.CacheRuntimeClass{
315+
ObjectMeta: metav1.ObjectMeta{Name: "test-class"},
316+
Topology: &datav1alpha1.RuntimeTopology{
317+
Worker: &datav1alpha1.RuntimeComponentDefinition{Template: tmpl},
318+
},
319+
}
320+
321+
client := fake.NewClientBuilder().
322+
WithScheme(CacheEngineTestScheme).
323+
WithObjects(asts, runtimeObj, runtimeClass).
324+
Build()
325+
326+
e := &CacheEngine{
327+
name: "test",
328+
namespace: "default",
329+
Client: client,
330+
Log: ctrl.Log.WithName("composition-matrix"),
331+
}
332+
333+
g.Expect(e.syncRuntimeSpec(cruntime.ReconcileRequestContext{}, runtimeObj, runtimeClass)).To(Succeed())
334+
335+
got := &workloadv1alpha1.AdvancedStatefulSet{}
336+
g.Expect(client.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: "default"}, got)).To(Succeed())
337+
return got.Spec.Template
338+
},
339+
}
340+
341+
// --- the runner -----------------------------------------------------------------
342+
343+
var _ = Describe("CacheRuntime pod template composition matrix",
344+
Label("pkg.ddc.cache.engine.composition_matrix_test.go"), func() {
345+
346+
for _, path := range []compositionPath{createPath, updatePath} {
347+
path := path
348+
349+
Describe(path.name, func() {
350+
for _, tc := range compositionTable() {
351+
tc := tc
352+
353+
if !runsOn(tc, path.name) {
354+
continue
355+
}
356+
357+
name := tc.field + ": " + tc.desc
358+
if tc.knownBug != "" {
359+
name += " [pins current behaviour, " + tc.knownBug + "]"
360+
}
361+
362+
It(name, func() {
363+
tc.want(Default, path.run(Default, tc))
364+
})
365+
}
366+
})
367+
}
368+
})
369+
370+
func runsOn(tc layerCase, path string) bool {
371+
if len(tc.onlyPaths) == 0 {
372+
return true
373+
}
374+
for _, p := range tc.onlyPaths {
375+
if p == path {
376+
return true
377+
}
378+
}
379+
return false
380+
}

pkg/ddc/cache/engine/transform_client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ func (e *CacheEngine) transformClient(dataset *datav1alpha1.Dataset, runtime *da
4545
// TODO: TieredStore handling
4646

4747
// transform container related config, currently only modify the first container
48-
e.transformComponentPodTemplate(runtimeClient.RuntimeComponentCommonSpec, dataset, value.Client)
48+
e.transformComponentPodTemplate(runtime.Spec, runtimeClient.RuntimeComponentCommonSpec, dataset, value.Client)
4949

5050
// transform tiered store configuration into pod resource request or volumes .
5151
err = e.TransformRuntimeTieredStore(&runtimeClient.TieredStore, &value.Client.PodTemplateSpec.Spec)

0 commit comments

Comments
 (0)