fix(ops): support operations for flat-ordinal instances - #10709
fix(ops): support operations for flat-ordinal instances#10709leon-ape wants to merge 26 commits into
Conversation
|
Auto Cherry-pick Instructions CLA Recheck Instructions |
| lastCompSpec.Replicas = *lastCompConfiguration.Replicas | ||
| lastCompSpec.Instances = lastCompConfiguration.Instances | ||
| lastCompSpec.OfflineInstances = lastCompConfiguration.OfflineInstances | ||
| lastPlan, err := runtime.GenerateInstanceNamePlan(opsRes.Cluster.Namespace, clusterName, fullCompName, lastCompSpec) |
There was a problem hiding this comment.
[P1] Do not reconstruct both the historical and desired participant sets from the current assignedOrdinals. getCreateAndDeletePodSet runs on every reconcile, while GenerateInstanceNamePlan seeds both plans from the live InstanceSet status. For example, with flat ordinals {0,2}, scaling 2 -> 3 initially identifies -1 as the new instance; after status converges to {0,1,2}, reconstructing the old 2-replica plan yields {0,1}, so the computed new instance changes to the already-existing -2. Because this path does not wait for the Component final state, Ops can report success while the actual new -1 is not ready. Participant identities must remain stable rather than being recomputed from an eventually changing workload status.
| } | ||
| for _, ins := range rebuildInstance.Instances { | ||
| insTplName := appsv1.GetInstanceTemplateName(opsRes.Cluster.Name, rebuildInstance.ComponentName, ins.Name) | ||
| insTplName, ok := plan.TemplateByName[ins.Name] |
There was a problem hiding this comment.
[P1] This rejects a retained-PVC-only/offline target that the Rebuild contract explicitly accepts. Action resolves such a target through GetInstance, and the later progress path has dedicated handling for an already-offline original instance. However, the name plan excludes every entry in OfflineInstances, so this lookup cannot find the target and returns a fatal error before the non-in-place rebuild starts. This affects both flat and non-flat naming and is a regression for the existing offline rebuild path.
| func (r *opsRuntime) GenerateTemplateInstanceNames(clusterName, compName, templateName string, replicas int32, offlineInstances []string, ordinals appsv1.Ordinals) ([]string, error) { | ||
| workloadName := constant.GenerateWorkloadNamePattern(clusterName, compName) | ||
| ordinalList, err := instanceset.ConvertOrdinalsToSortedList(ordinals) | ||
| itsExt, err := instancetemplate.BuildInstanceSetExt(protoITS, nil) |
There was a problem hiding this comment.
[P1] This moves the workloads controller authoritative instance-plan construction into Ops. The new Ops runtime synthesizes a partial InstanceSet, copies live status.assignedOrdinals, and directly invokes the internal instancetemplate planner. The base code already had deprecated name-generation debt, but this PR expands that dependency to workloads runtime state and allocation behavior. Ops should consume an explicit authoritative instance identity/template contract exposed by the owner API/status; it should not rerun the workloads plan builder.
| offlineTemplateByName := make(map[string]string, len(compSpec.OfflineInstances)) | ||
| if compSpec.FlatInstanceOrdinal && len(compSpec.OfflineInstances) > 0 { | ||
| offlineNames := sets.New(compSpec.OfflineInstances...) | ||
| volumes, err := r.loadVolumes(namespace, clusterName, compName) |
There was a problem hiding this comment.
[P2] Retained PVC labels are not a complete contract for an offline instance template identity. A flat-ordinal offline instance may have no PVC, an older PVC without this label, or retained resources that have been removed; the public OfflineInstances API does not require this lookup to succeed. In those cases OfflineTemplateByName remains incomplete and offlineInstancesToOnline later fails fatally. The template association needs to be represented explicitly by the authoritative workloads/apps API instead of inferred from storage artifacts.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #10709 +/- ##
==========================================
- Coverage 65.13% 64.89% -0.25%
==========================================
Files 510 511 +1
Lines 64007 64562 +555
==========================================
+ Hits 41694 41898 +204
- Misses 18574 18882 +308
- Partials 3739 3782 +43
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
github-actions
Bot
added
size/XXL
…inal-ops # Conflicts: # apis/workloads/v1/instanceset_types.go # config/crd/bases/workloads.kubeblocks.io_instancesets.yaml # deploy/helm/crds/workloads.kubeblocks.io_instancesets.yaml # docs/developer_docs/api-reference/cluster.md # pkg/controller/instance/reconciler_status.go # pkg/controller/instance/reconciler_status_test.go # pkg/controller/instanceset/reconciler_status.go # pkg/controller/instanceset2/reconciler_status.go # pkg/controller/instancetemplate/status_allocation.go # pkg/controller/instancetemplate/status_allocation_test.go # pkg/controller/workloads/instancestatus/builder.go # pkg/controller/workloads/instancestatus/builder_test.go
leon-ape
changed the title
leon-ape
changed the title
leon-ape
changed the title
leon-ape
changed the title
leon-ape
changed the title
3 participants
What changed
status.instanceStatusinstead of reproducing InstanceSet name allocationstatus.instanceStatusthe single identity source for Start, Stop, VerticalScaling, and VolumeExpansion in both ordinal modes; an incomplete status view remains waiting instead of falling back to name generationsourceInstanceAssignmentsrecord for ordinary HorizontalScaling and flat-ordinal non-in-place RebuildInstance; no generic source/target snapshot, UID, generation, or frozen participant protocolScaleOut.FromBackup; rejectflatInstanceOrdinal + FromBackupbefore Action because future identities do not exist yettargetNodeName, because placement would require a future identity before InstanceSet allocates itDependency
#10758 is merged into main and provides the richer InstanceStatus API consumed here.
Validation
scripts/codex-go-test.sh ./pkg/operations -count=1make lintFixes #10704