diff --git a/pkg/ddc/cache/engine/sync.go b/pkg/ddc/cache/engine/sync.go index 29b8849cd06..dede362a0da 100644 --- a/pkg/ddc/cache/engine/sync.go +++ b/pkg/ddc/cache/engine/sync.go @@ -194,7 +194,7 @@ func (e *CacheEngine) syncRuntimeSpec(ctx cruntime.ReconcileRequestContext, runt } manager := component.NewComponentHelper(common.ComponentTypeMaster, e.Client) masterSpec := component.ComponentSpec{ - Version: runtime.Spec.Master.RuntimeVersion, + Version: desiredComponentVersion(runtime.Spec.Master.RuntimeVersion, componentTemplateImage(runtimeClass.Topology.Master)), Resources: desiredComponentResources(runtime.Spec.Master.Resources, runtimeClass.Topology.Master), Replicas: &runtime.Spec.Master.Replicas, } @@ -231,7 +231,7 @@ func (e *CacheEngine) syncRuntimeSpec(ctx cruntime.ReconcileRequestContext, runt } workerSpec := component.ComponentSpec{ - Version: runtime.Spec.Worker.RuntimeVersion, + Version: desiredComponentVersion(runtime.Spec.Worker.RuntimeVersion, componentTemplateImage(runtimeClass.Topology.Worker)), Resources: workerResources, Replicas: &runtime.Spec.Worker.Replicas, } diff --git a/pkg/ddc/cache/engine/transform_common.go b/pkg/ddc/cache/engine/transform_common.go index d95f92c1199..283faa8d8b8 100644 --- a/pkg/ddc/cache/engine/transform_common.go +++ b/pkg/ddc/cache/engine/transform_common.go @@ -18,6 +18,7 @@ package engine import ( "fmt" + "strings" datav1alpha1 "github.com/fluid-cloudnative/fluid/api/v1alpha1" "github.com/fluid-cloudnative/fluid/pkg/common" @@ -119,9 +120,11 @@ func (e *CacheEngine) transformComponentPodTemplate(runtimeCompSpec datav1alpha1 // transform container related config, currently only modify the first container if len(podTemplate.Spec.Containers) > 0 { - // transform Container Image name etc. - if len(runtimeCompSpec.RuntimeVersion.Image) > 0 && len(runtimeCompSpec.RuntimeVersion.ImageTag) > 0 { - podTemplate.Spec.Containers[0].Image = runtimeCompSpec.RuntimeVersion.Image + ":" + runtimeCompSpec.RuntimeVersion.ImageTag + // transform Container Image name etc. A version naming only one of image and imageTag + // takes the other half from the template rather than being dropped. + version := desiredComponentVersion(runtimeCompSpec.RuntimeVersion, podTemplate.Spec.Containers[0].Image) + if len(version.Image) > 0 && len(version.ImageTag) > 0 { + podTemplate.Spec.Containers[0].Image = version.Image + ":" + version.ImageTag } if len(runtimeCompSpec.RuntimeVersion.ImagePullPolicy) > 0 { podTemplate.Spec.Containers[0].ImagePullPolicy = (corev1.PullPolicy)(runtimeCompSpec.RuntimeVersion.ImagePullPolicy) @@ -148,3 +151,62 @@ func (e *CacheEngine) transformComponentPodTemplate(runtimeCompSpec datav1alpha1 componentValue.PodTemplateSpec.Spec.InitContainers[0].Env = append(addEnvs, componentValue.PodTemplateSpec.Spec.InitContainers[0].Env...) } } + +// desiredComponentVersion completes a partially specified runtime version against the image +// carried by the CacheRuntimeClass template. +// +// VersionSpec declares image and imageTag as independent optional strings, so naming only one +// of them is a legal way to say "the same image on a newer tag". Both the creation path and +// updateImage used to require both halves and otherwise leave the template image alone, which +// silently discards that edit. Completing the missing half here gives both paths the same +// desired image, so a component created with a partial version and one updated to it end up +// identical instead of rolling on the next reconcile. +// +// A version naming neither half is returned untouched, leaving the template image in place. So +// is one whose missing half cannot be recovered - a template with no image, or an image pinned +// by digest - because guessing there would move the workload onto something nobody asked for. +func desiredComponentVersion(runtimeVersion datav1alpha1.VersionSpec, templateImage string) datav1alpha1.VersionSpec { + if runtimeVersion.Image == "" && runtimeVersion.ImageTag == "" { + return runtimeVersion + } + if runtimeVersion.Image != "" && runtimeVersion.ImageTag != "" { + return runtimeVersion + } + + templateRepository, templateTag := splitImageReference(templateImage) + if runtimeVersion.Image == "" { + runtimeVersion.Image = templateRepository + } + if runtimeVersion.ImageTag == "" { + runtimeVersion.ImageTag = templateTag + } + + return runtimeVersion +} + +// splitImageReference splits a container image reference into its repository and tag. The tag +// is empty when the reference carries none, and both are empty for a reference pinned by +// digest, where there is no tag to complete and appending one would not be valid. +func splitImageReference(image string) (repository string, tag string) { + if image == "" || strings.Contains(image, "@") { + return "", "" + } + + lastColon := strings.LastIndex(image, ":") + // A colon before the last slash belongs to a registry port, not to a tag. + if lastColon == -1 || lastColon < strings.LastIndex(image, "/") { + return image, "" + } + + return image[:lastColon], image[lastColon+1:] +} + +// componentTemplateImage returns the image the CacheRuntimeClass declares for a component, +// which is the baseline a partially specified runtime version is completed against. +func componentTemplateImage(componentDefinition *datav1alpha1.RuntimeComponentDefinition) string { + if componentDefinition == nil || len(componentDefinition.Template.Spec.Containers) == 0 { + return "" + } + + return componentDefinition.Template.Spec.Containers[0].Image +} diff --git a/pkg/ddc/cache/engine/transform_common_test.go b/pkg/ddc/cache/engine/transform_common_test.go new file mode 100644 index 00000000000..fcda1fb945e --- /dev/null +++ b/pkg/ddc/cache/engine/transform_common_test.go @@ -0,0 +1,163 @@ +/* +Copyright 2026 The Fluid Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package engine + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + datav1alpha1 "github.com/fluid-cloudnative/fluid/api/v1alpha1" + corev1 "k8s.io/api/core/v1" +) + +var _ = Describe("CacheEngine desiredComponentVersion Tests", Label("pkg.ddc.cache.engine.transform_common_test.go"), func() { + const templateImage = "btxu/mooncake:v3" + + Describe("desiredComponentVersion", func() { + It("should leave a version naming neither half untouched", func() { + desired := desiredComponentVersion(datav1alpha1.VersionSpec{}, templateImage) + + Expect(desired.Image).To(BeEmpty()) + Expect(desired.ImageTag).To(BeEmpty()) + }) + + It("should leave a complete version untouched", func() { + desired := desiredComponentVersion(datav1alpha1.VersionSpec{ + Image: "other/mooncake", + ImageTag: "v9", + }, templateImage) + + Expect(desired.Image).To(Equal("other/mooncake")) + Expect(desired.ImageTag).To(Equal("v9")) + }) + + It("should take the repository from the template when only imageTag is named", func() { + desired := desiredComponentVersion(datav1alpha1.VersionSpec{ImageTag: "v9"}, templateImage) + + Expect(desired.Image).To(Equal("btxu/mooncake")) + Expect(desired.ImageTag).To(Equal("v9")) + }) + + It("should take the tag from the template when only image is named", func() { + desired := desiredComponentVersion(datav1alpha1.VersionSpec{Image: "other/mooncake"}, templateImage) + + Expect(desired.Image).To(Equal("other/mooncake")) + Expect(desired.ImageTag).To(Equal("v3")) + }) + + It("should not carry imagePullPolicy into the completion", func() { + desired := desiredComponentVersion(datav1alpha1.VersionSpec{ + ImageTag: "v9", + ImagePullPolicy: "Always", + }, templateImage) + + Expect(desired.ImagePullPolicy).To(Equal("Always")) + }) + + It("should stay incomplete when the template declares no image", func() { + desired := desiredComponentVersion(datav1alpha1.VersionSpec{ImageTag: "v9"}, "") + + Expect(desired.Image).To(BeEmpty()) + Expect(desired.ImageTag).To(Equal("v9")) + }) + + It("should stay incomplete when the template pins a digest", func() { + desired := desiredComponentVersion(datav1alpha1.VersionSpec{ImageTag: "v9"}, + "btxu/mooncake@sha256:067614b70d25b496e3edc3480747d558ee8a364ef47a67f669f5d96ca5098552") + + Expect(desired.Image).To(BeEmpty()) + Expect(desired.ImageTag).To(Equal("v9")) + }) + }) + + Describe("splitImageReference", func() { + It("should split a plain tagged reference", func() { + repository, tag := splitImageReference("btxu/mooncake:v3") + + Expect(repository).To(Equal("btxu/mooncake")) + Expect(tag).To(Equal("v3")) + }) + + It("should report no tag rather than inventing one", func() { + repository, tag := splitImageReference("nginx") + + Expect(repository).To(Equal("nginx")) + Expect(tag).To(BeEmpty()) + }) + + It("should not mistake a registry port for a tag", func() { + repository, tag := splitImageReference("registry.local:5000/mooncake") + + Expect(repository).To(Equal("registry.local:5000/mooncake")) + Expect(tag).To(BeEmpty()) + }) + + It("should split a tag off a reference that also carries a registry port", func() { + repository, tag := splitImageReference("registry.local:5000/mooncake:v3") + + Expect(repository).To(Equal("registry.local:5000/mooncake")) + Expect(tag).To(Equal("v3")) + }) + + It("should refuse to split a digest reference", func() { + repository, tag := splitImageReference("btxu/mooncake@sha256:0676") + + Expect(repository).To(BeEmpty()) + Expect(tag).To(BeEmpty()) + }) + }) + + Describe("componentTemplateImage", func() { + It("should return an empty image for a nil component definition", func() { + Expect(componentTemplateImage(nil)).To(BeEmpty()) + }) + + It("should return an empty image for a template with no containers", func() { + Expect(componentTemplateImage(&datav1alpha1.RuntimeComponentDefinition{})).To(BeEmpty()) + }) + + It("should return the first container's image", func() { + Expect(componentTemplateImage(&datav1alpha1.RuntimeComponentDefinition{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Name: "worker", Image: templateImage}}, + }, + }, + })).To(Equal(templateImage)) + }) + }) + + Describe("the creation and the sync path agreeing", func() { + It("should resolve an imageTag-only version to the same image on both paths", func() { + // creation resolves against the image already on the template copy + creationVersion := desiredComponentVersion(datav1alpha1.VersionSpec{ImageTag: "v9"}, templateImage) + + // sync resolves against the CacheRuntimeClass component definition + syncVersion := desiredComponentVersion(datav1alpha1.VersionSpec{ImageTag: "v9"}, + componentTemplateImage(&datav1alpha1.RuntimeComponentDefinition{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Name: "worker", Image: templateImage}}, + }, + }, + })) + + Expect(creationVersion).To(Equal(syncVersion)) + Expect(creationVersion.Image + ":" + creationVersion.ImageTag).To(Equal("btxu/mooncake:v9")) + }) + }) +})