fix(ops): support flat ordinal instance names - #10709
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.
2 participants
What changed
InstanceNamePlanbacked byinstancetemplate.PodNameBuilderstatus.assignedOrdinalswhen planning desired instance namesWhy
OpsRequest progress tracking generated legacy template-prefixed pod names even when the InstanceSet controller used flattened global ordinals. The expected names therefore never matched the actual pods, leaving affected operations pending or causing them to time out. Flat pod names also do not encode their instance template, so parsing the template from the name was incorrect.
Impact
OpsRequest progress tracking now uses the same naming implementation and ordinal assignments as the InstanceSet controller. Existing non-flat naming behavior remains covered by the operations test suite.
Validation
go test ./pkg/operations -count=1Fixes #10704