Skip to content

fix(cache): propagate runtime-level podMetadata and imagePullSecrets to component pods - #6188

Open
btxu-db wants to merge 1 commit into
fluid-cloudnative:masterfrom
btxu-db:fix/cacheruntime-runtime-level-pod-fields
Open

fix(cache): propagate runtime-level podMetadata and imagePullSecrets to component pods#6188
btxu-db wants to merge 1 commit into
fluid-cloudnative:masterfrom
btxu-db:fix/cacheruntime-runtime-level-pod-fields

Conversation

@btxu-db

@btxu-db btxu-db commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Ⅰ. Describe what this PR does

CacheRuntimeSpec has two runtime-level fields whose doc comments say they apply to every
component:

// api/v1alpha1/cacheruntime_types.go:160
// PodMetadata contains labels and annotations that will be propagated to all component pods.
PodMetadata PodMetadata `json:"podMetadata,omitempty"`

// api/v1alpha1/cacheruntime_types.go:164
ImagePullSecrets []corev1.LocalObjectReference `json:"imagePullSecrets,omitempty" patchStrategy:"merge" patchMergeKey:"name"`

They are in the CRD and the API server takes them, but the transform path does not read
either. transformComponentPodTemplate composes a component's pod template from the
CacheRuntimeClass template and the component-level RuntimeComponentCommonSpec; the
runtime-level layer is not one of its arguments, so it never enters. Setting both fields on a
CacheRuntime is accepted, and then nothing lands on the pod:

imagePullSecrets            = []
labels.from-runtime-level   = []     <- spec.podMetadata
labels.from-component-level = [yes]  <- spec.worker.podMetadata  (control)
annotations.owner           = []     <- spec.podMetadata

The component-level control landing is what narrows it: template rendering and the manifest
are fine, only the middle layer goes missing. imagePullSecrets is the one likely to hurt —
the kubelet gets no credentials for a component image behind a private registry, and the pull
failure names the image and the registry rather than the field that was dropped, so the runtime
spec is not an obvious place to look.

Approach. podMetadata has the same three-layer shape as spec.options, which already
composes all three layers (cm.go:146), so it is given the same order here: CacheRuntimeClass
template < runtime level < component level. UnionMapsWithOverride copies both operands and
tolerates nil, so the old != nil guards around the component layer are no longer needed.

imagePullSecrets has no component-level counterpart, so there is no precedence to settle.
The runtime-level list is appended onto whatever the template declares, skipping names that
are already there. The CRD marks the field +patchStrategy=merge +patchMergeKey=name, which
I read as a name declared at both layers being one entry rather than two. Replacing the
template's list outright would be defensible too, but that seemed more likely to surprise an
owner whose class template already pins a registry secret — happy to switch if maintainers
prefer the simpler rule.

Master, worker and client all go through transformComponentPodTemplate, so all three call
sites now pass runtime.Spec.

One thing this does not do: both fields are read on the transform path, which runs at setup,
so editing either on a live CacheRuntime still has no effect — syncRuntimeSpec handles
runtimeVersion, resources and replicas only. That looks like a separate change, and it
is part of what #6185 is about.

Ⅱ. Does this pull request fix one issue?

fixes #6184

Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.

transform_common_test.go covers transformComponentPodTemplate directly:

  • pod metadata — the template's labels and annotations survive when neither spec sets any;
    the runtime level alone reaches the pod; the component level alone reaches the pod; all
    three layers union, and a key declared at more than one layer takes the value from the
    latest one.
  • image pull secrets — the template's secrets survive when the runtime names none; the
    runtime-level list alone reaches the pod; both layers merge, and a name declared at both
    appears once.

composition_matrix_test.go is the executable form of the composition table in #6185. Each
row states what the three layers declare and what the component should end up with, and every
row is replayed through both paths that compose them — the transform path on create and the
sync path on update — since the two are written independently and are expected to agree.

Rows tagged knownBug pin current behaviour rather than intended behaviour, so that a
change to any of them shows up in review; each names the issue that will change it and what
it should say afterwards (#6173 for resources, #6178 for runtimeVersion, and the
env/nodeSelector mismatches under #6185). If #6177 merges first, the #6173 row is the
one that will need updating.

Rows for fields the sync path does not patch are restricted to the create path, which is
stated per row rather than left implicit.

Existing specs in the package are unchanged. The 12 failures in ufs_test.go and
sync_test.go on this package are present on master as well and are unrelated to these
changes.

@fluid-e2e-bot

fluid-e2e-bot Bot commented Sep 6, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign yangyuliufeng for approval by writing /assign @yangyuliufeng in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fluid-e2e-bot

fluid-e2e-bot Bot commented Sep 6, 2026

Copy link
Copy Markdown

Hi @btxu-db. Thanks for your PR.

I'm waiting for a fluid-cloudnative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

`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 fluid-cloudnative#6184

Signed-off-by: btxu-db <btxu-db@outlook.com>
@btxu-db
btxu-db force-pushed the fix/cacheruntime-runtime-level-pod-fields branch from c6526ed to ce7ef73 Compare September 6, 2026 05:54
@sonarqubecloud

sonarqubecloud Bot commented Sep 6, 2026

Copy link
Copy Markdown

@codecov

codecov Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.29%. Comparing base (f2785f8) to head (ce7ef73).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6188      +/-   ##
==========================================
+ Coverage   65.24%   65.29%   +0.04%     
==========================================
  Files         486      486              
  Lines       34194    34217      +23     
==========================================
+ Hits        22309    22341      +32     
+ Misses      10135    10129       -6     
+ Partials     1750     1747       -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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]CacheRuntime: runtime-level imagePullSecrets and podMetadata never reach the component pods

1 participant