Skip to content

fix(operations): use resolved component for switchover runtime#10568

Open
weicao wants to merge 4 commits into
mainfrom
bugfix/sharded-switchover-runtime-target
Open

fix(operations): use resolved component for switchover runtime#10568
weicao wants to merge 4 commits into
mainfrom
bugfix/sharded-switchover-runtime-target

Conversation

@weicao

@weicao weicao commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Problem

Fixes #10567.

A sharded switchover can validate successfully with the correct API form:

  • componentName: shard (the sharding spec name)
  • instanceName and candidateName inside one generated shard component

On a controller image containing #10360, Valkey CT09 still failed during execution with:

DoActionFailed ... Component.apps.kubeblocks.io "vlk-cluster-57598-shard" not found

The frozen evidence shows the #10360 label preconditions are satisfied: the concrete Component and both pods carry apps.kubeblocks.io/sharding-name=shard and apps.kubeblocks.io/component-name=shard-qwd. That rules out the old entry-resolution issue and leaves the runtime execution path.

Root cause

handleSwitchover already resolves the concrete target through buildSynthesizedComp, including the sharding instance to generated Component mapping.

opsRuntime.Switchover then rebuilt a synthesized component from compName by calling GenerateClusterComponentName(clusterName, compName). For sharding input compName=shard, that second resolution composes vlk-cluster-57598-shard, which is not a real generated Component.

Two review follow-ups exposed adjacent contract gaps:

  • after merging current main, stale identifiers left the combined branch unable to compile;
  • TemplateVars could override the five runtime-defined switchover parameters. In the no-candidate case, omitting the candidate keys also let conflicting template values survive, changing an automatic-candidate request into an explicit candidate request.

Solution

Reuse the already resolved SynthesizedComponent when entering the runtime switchover path.

This keeps the boundary narrower than adding another sharding-aware resolver in runtime:

  • handleSwitchover owns target resolution;
  • runtime owns the lifecycle action using that resolved target;
  • ordinary component switchover still passes the same concrete component name as before;
  • runtime map lookup still uses the existing logical key;
  • OpsRequest status/progress bookkeeping remains keyed by the user-facing component/sharding name; only the runtime action target uses the resolved concrete component.

The lifecycle merge now treats the five KB_SWITCHOVER_* runtime parameters as authoritative. It overwrites conflicting TemplateVars while preserving unrelated variables. When no candidate is specified, both candidate parameters are still emitted with explicit empty values, matching the API contract and preventing template values from selecting a candidate.

Tests

Validated at exact head d46081b83dced6a0b3a677eb3bbe7010b112f1b9:

  • go test -short ./pkg/controller/lifecycle ./pkg/operations -count=1 with envtest assets: PASS
  • focused go test -race for candidate and no-candidate collision cases: PASS
  • go vet ./pkg/controller/lifecycle ./pkg/operations: PASS
  • git diff --check: PASS

Tests cover all five conflicting runtime fields, unrelated TemplateVars remaining intact, and the no-candidate path overwriting both candidate fields with authoritative empty values. Runtime validation on a patched controller image remains pending (N=0).

@weicao
weicao requested review from a team and wangyelei as code owners July 6, 2026 18:29
@github-actions github-actions Bot added the size/S Denotes a PR that changes 10-29 lines. label Jul 6, 2026
@apecloud-bot

Copy link
Copy Markdown
Collaborator

Auto Cherry-pick Instructions

Usage:
  - /nopick: Not auto cherry-pick when PR merged.
  - /pick: release-x.x [release-x.x]: Auto cherry-pick to the specified branch when PR merged.

Example:
  - /nopick
  - /pick release-1.1

CLA Recheck Instructions

Usage:
  - /recheck-cla: Trigger a re-check of CLA status for this pull request.
Example:
  - /recheck-cla

@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 70.58824% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.01%. Comparing base (116b639) to head (d46081b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/controller/lifecycle/kbagent.go 37.50% 4 Missing and 1 partial ⚠️
pkg/operations/ops_runtime.go 76.92% 3 Missing ⚠️
pkg/controller/lifecycle/lfa_member.go 80.00% 1 Missing ⚠️
pkg/operations/switchover.go 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10568      +/-   ##
==========================================
+ Coverage   63.98%   64.01%   +0.03%     
==========================================
  Files         520      520              
  Lines       62995    62996       +1     
==========================================
+ Hits        40305    40329      +24     
+ Misses      19101    19072      -29     
- Partials     3589     3595       +6     
Flag Coverage Δ
unittests 64.01% <70.58%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@weicao

weicao commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

PR #10568 patch-version runtime validation completed on the Valkey CT09 scene.

Controller image gate:

  • Source commit: 34e474cf62a8cc3b8a497eadb4f2bb6562eb6865
  • Local tag: docker.io/apecloud/kubeblocks:pr10568-34e474cf6-stella
  • Built local image digest: sha256:b2d4d6a8c4bf5f4adc87b16ff3748891ffa37c7a79f4909b72c77e4a9669e696
  • Image tar sha256: a1443776aa0ff498ea37a13c4000ba06d3e4016f7d199a522f69f6df03551d04
  • Host import: node1/node2/node3 ctr -n k8s.io images ls all show the exact tag and digest above
  • Live deployment revision: 9
  • Live controller pod: kubeblocks-7877ffdfd8-qcqzn
  • Live controller identity: image docker.io/apecloud/kubeblocks:pr10568-34e474cf6-stella, imagePullPolicy=Never, container imageID/config sha256:e72f85ce419b88ed4bbec271cf1dfbc3680df2686e844b7426c3c4f226a026e4, startTime 2026-07-06T18:42:47Z, restartCount 0, node node2, Ready true

Same frozen OpsRequest repair result:

  • Preserved namespace: valkey-ct09-fixed-foreground
  • Original stuck OpsRequest: vlk-cluster-57598-sw-t6gdq
  • After rolling this PR controller image, the same OpsRequest advanced to phase: Succeed
  • Status includes message: do switchover succeed, objectKey: Switchover/shard, status: Succeed
  • The previous DoActionFailed: Component.apps.kubeblocks.io "vlk-cluster-57598-shard" not found remains only as historical event history in describe; it did not recur after the patch rollout

Minimum clean CT09 contrast:

  • Command: ./run-tests.sh -t cluster-topology -n valkey-pr10568-clean-ct09 --valkey-major 9 --chart-version 0.1.1 --timeout-ops 600 --timeout-pod 300 with VALKEY_CLUSTER_TOPOLOGY_REQUIRED=1, VALKEY_CLUSTER_TOPOLOGY_NO_CLEANUP=1, RUNNER_CLEANUP_GATE=0
  • Result: PASS: 38, FAIL: 0, SKIP: 1, exit code 0
  • CT09 clean OpsRequest: vlk-cluster-97724-sw-ghvx6 reached Succeed; runner observed requested candidate vlk-cluster-97724-shard-6lt-0 became primary
  • Namespace valkey-pr10568-clean-ct09 preserved. Events show the CT09 OpsRequest was processed successfully and then deleted by the controller
  • CT11 skip is unrelated: default BackupRepo is Ready, but no cluster BackupPolicy was found for the clean-run cluster

Evidence packet path on the test machine:
artifacts/valkey-live/pr10568-patch-version-20260707/

Packet SHA256SUMS hash:
1f5b3a5b6be5ab9b2761583cc849f639f80af94466c537205af31f4a1fa42e93

Boundary: this validates PR #10568 patch-version CT09 scope only. It is not a full Valkey r9 or release-ready result.

@leon-ape

Copy link
Copy Markdown
Collaborator

[P2] The functional fix removes the duplicate component resolution, but the replacement expands the OpsRuntime boundary to a controller-internal type.

The merge base kept component.SynthesizedComponent inside the runtime implementation. This PR changes the public OpsRuntime.Switchover contract to accept the whole pkg/controller/component.SynthesizedComponent (pkg/operations/type.go:133). That type is the apps controller's synthesis model and carries substantially more controller/workload/template state than a switchover action needs; it is not a stable Ops or lifecycle API contract. Exporting the helper/type does not make it an appropriate cross-module contract.

Passing the already resolved concrete shard is the correct direction, but please pass a minimal stable switchover/action context instead of exposing the complete synthesized controller model through the OpsRuntime interface.

@weicao
weicao marked this pull request as draft July 14, 2026 07:30
@github-actions github-actions Bot added size/M Denotes a PR that changes 30-99 lines. and removed size/S Denotes a PR that changes 10-29 lines. labels Jul 14, 2026
@weicao

weicao commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the runtime-boundary review on exact head 984982cbb7ecfa48e1fb974ad78852329a61b9a3.

The sharded target is still resolved once by the switchover handler, but OpsRuntime now receives only a stable SwitchoverActionContext: namespace, cluster name, concrete component name, lifecycle actions, and resolved template variables. It no longer imports or accepts the controller-internal component.SynthesizedComponent. The runtime continues to list the concrete shard Pods and dispatch the same lifecycle action with the resolved candidate/current identity.

TDD/gates:

  • compile RED: *opsRuntime does not implement OpsRuntime after narrowing the interface
  • focused concrete-shard switchover GREEN
  • focused race GREEN
  • full pkg/operations GREEN
  • go vet ./pkg/operations GREEN
  • git diff --check GREEN

The PR remains Draft pending exact-head CI and review verification. Runtime N=0; this is source/local contract evidence only.

@weicao

weicao commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

/nopick

@apecloud-bot apecloud-bot added the nopick Not auto cherry-pick when PR merged label Jul 14, 2026
@weicao

weicao commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Scope disclosure added: this change intentionally fixes TemplateVars propagation as well as sharded target resolution. The handler resolves variables before dispatch; the old runtime rebuild skipped that resolver, while the new action context passes the resolved map into lifecycle.New. Runtime evidence is still N=0.

@weicao
weicao marked this pull request as ready for review July 14, 2026 15:14
@weicao

weicao commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Landing-order note: #10651 fixes an observed non-idempotent switchover replay window and touches the same handleSwitchover Pending path. Please land #10651 first. After that, #10568 will rebase onto the updated main branch, resolve the shared hunk, and rerun its exact-head focused/full gates before merge. The current #10568 head remains its reviewed source state, but it is not the final merge identity.

Comment thread pkg/operations/switchover.go
Comment thread pkg/operations/ops_runtime.go
@weicao
weicao force-pushed the bugfix/sharded-switchover-runtime-target branch from 984982c to 95b4972 Compare July 20, 2026 09:54
@github-actions github-actions Bot added size/L Denotes a PR that changes 100-499 lines. and removed size/M Denotes a PR that changes 30-99 lines. labels Jul 20, 2026
Comment thread pkg/controller/lifecycle/kbagent.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

nopick Not auto cherry-pick when PR merged size/L Denotes a PR that changes 100-499 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Sharded switchover fails at execution on #10360-fixed controller: runtime chain re-composes Component name from sharding spec name

3 participants