Repository navigation
refactor: clean up pipeline - #1464
Merged
Merged
Conversation
…handling Signed-off-by: Markus Wieland <markus.wieland@sap.com>
Phase 2 of the pipeline cleanup. Flip the Nova filter-weigher pipeline's RequestType from the Nova wire type api.ExternalSchedulerRequest to the typed, Nova-agnostic compute.SchedulerRequest, and rewrite all 28 filter/weigher plugins (plus both zz_index.go registries) to read typed fields: - intent via request.Intent instead of GetIntent()+error - scheduler hints -> typed fields (SourceHost, DomainName, ResourceGroup) - trait:/capabilities: extra-spec iteration -> RequiredTraits/ForbiddenTraits/ RequestedCapabilities; accel:device_profile -> AcceleratorsRequested - flavor/hypervisor/image enums read from typed fields Spec refinements: FlavorSpec.MemoryMB (uint64) -> Memory (resource.Quantity, bytes), all consumers updated; add AcceleratorsRequested and fix the accelerators filter's request-side gate. WIP: the tree does not build at this commit. The plugins now require compute.SchedulerRequest but the Nova controller is still typed on api.ExternalSchedulerRequest; it builds once the wire->spec adapter lands (Phase 3). The compute and plugin packages build in isolation. Signed-off-by: Markus Wieland <markus.wieland@sap.com>
…wire adapter Phase 3 of the pipeline cleanup — closes the Phase 2 seam so the tree builds and all tests pass. - Add internal/scheduling/nova/adapter.go: toSchedulerRequest translates the Nova wire ExternalSchedulerRequest into the typed compute.SchedulerRequest, resolving scheduler hints, flavor/image extra specs, traits, capabilities and accelerators into typed fields. Intent is taken from the already-resolved decision.Spec.Intent. - Flip the Nova FilterWeigherPipelineController and candidate gatherer to compute.SchedulerRequest; process() builds the typed request via the adapter. vm_handler and the CR recorder keep consuming the Nova wire request (they are Nova-owned side effects). - GatherAllCandidates is derived from the hypervisor family (KVM gathers, VMware uses the caller's preselection) in the adapter — the Pipeline CRD's IgnorePreselection field is removed entirely (Go type, CRD manifest, helm values). - FlavorSpec.Memory is a resource.Quantity (bytes); all consumers updated. - image-properties filter emits its undetermined-hv diagnostic via the new ImageHypervisorTypeUnparseable enum value. - Export the controller's Gatherer field and guard a nil gatherer with a clear error instead of panicking. - Migrate all Nova filter/weigher plus gatherer/controller/integration tests to typed fixtures. Fix test-handler drift in the failover/failoverhost integration tests (resolve intent onto the Decision, as the production handler does). go build ./..., go test ./..., go vet ./... and make lint are all clean. Signed-off-by: Markus Wieland <markus.wieland@sap.com>
…heduler API Phase 4a. The Nova external-scheduler HTTP handler now calls the shared compute.Scheduler directly instead of ferrying a Decision CRD through the controller's ProcessNewDecisionFromAPI. - external_scheduler_api.go: NewAPI takes a *compute.Scheduler, VirtualMachineHandler, crs.Recorder and FeatureGates. The handler maps the wire request to a compute.SchedulerRequest (exported ToSchedulerRequest) and runs the scheduler with a single post-hook that performs VM-resource upsert and committed-resource tracking under the scheduler lock. A `runner` interface seam keeps the handler unit-testable. - filter_weigher_pipeline_controller.go: drop the Decision Reconcile/watch, ProcessNewDecisionFromAPI, process(), peekReadOnly and the processMu lock. The controller now only (re)configures pipelines via its Pipeline/Knowledge watches and exposes GetPipeline + the exported Gatherer for the scheduler. Delete the HTTPAPIDelegate interface. - vm_handler.Upsert and crs.Recorder.Record* now take the typed result + intent instead of a *v1alpha1.Decision. - main.go: construct one shared compute.Scheduler over the Nova controller and inject it, the vm handler, CR recorder and feature gates into nova.NewAPI. The reservation/commitment/capacity/failover controllers still reach the pipeline via the HTTP SchedulerClient (now backed by compute.Scheduler through the same endpoint); they move in-process in the next step, after which SchedulerClient and SchedulerURL are removed. Decision CRD remains the cinder/manila/machines/pods carrier until Phases 5-6. go build ./..., go test ./..., go vet ./... and make lint are all clean. Signed-off-by: Markus Wieland <markus.wieland@sap.com>
After the entrypoint swap the filter-weigher pipeline controller only (re)configures pipelines and exposes GetPipeline; the candidate gatherer is a scheduling-run concern owned by compute.Scheduler, not the controller. - Add nova.NewCandidateGatherer(client) and drop the controller's exported Gatherer field (and its assignment in SetupWithManager). - main.go builds the gatherer independently and passes it to compute.NewScheduler. - Tests construct the scheduler with an explicit gatherer instead of reading it off the controller. go build ./..., go test ./..., go vet ./... and make lint are all clean. Signed-off-by: Markus Wieland <markus.wieland@sap.com>
…TTP client Phase 4b. The committed-resource, capacity and failover reservation controllers now build a typed compute.SchedulerRequest and call the shared in-process compute.Scheduler directly (Run for write placements, RunWithoutLock for read-only capacity probes and failover reuse/evacuate checks) instead of POSTing to the Nova external-scheduler endpoint over HTTP. - Delete reservations.SchedulerClient and its ScheduleReservationRequest/Response DTOs (they only existed to build the HTTP wire body). - Remove the SchedulerURL config field from the commitments, capacity and failover configs and the related main.go log lines. - main.go hoists the shared novaScheduler and injects it into all three reservation controllers, guarding that nova-pipeline-controllers (the in-process scheduler) is enabled. - Reservation requests set GatherAllCandidates + HypervisorFamilyKVM, matching the previous behavior where the KVM pipelines gathered all candidates (ignorePreselection=true), so the eligible-host list is a starting set. - Migrate the reservation controller tests off the httptest mock-scheduler servers to an in-process mock compute.Scheduler returning canned hosts. Note: failover reservation_scheduling.go now also resolves required/forbidden traits from the flavor extra specs onto the typed request (traitsFromExtraSpecs), preserving trait filtering for HANA/trait reservations. Out of scope: the failoverhost controller keeps its own separate HTTP path-finder (ExternalSchedulerURL); it is migrated in a later follow-up. go build ./..., go test ./..., go vet ./... and make lint are all clean. Signed-off-by: Markus Wieland <markus.wieland@sap.com>
Signed-off-by: Markus Wieland <markus.wieland@sap.com>
SoWieMarkus
requested review from
PhilippMatthes,
auhlig,
juliusclausnitzer,
mblos and
umswmayj
as code owners
November 2, 2026 16:46
SoWieMarkus
marked this pull request as draft
November 2, 2026 16:47
…interface and related methods Signed-off-by: Markus Wieland <markus.wieland@sap.com>
…servations for capacity management Signed-off-by: Markus Wieland <markus.wieland@sap.com>
…y in FlavorSpec Signed-off-by: Markus Wieland <markus.wieland@sap.com>
…duling components Signed-off-by: Markus Wieland <markus.wieland@sap.com>
…l scheduler dependencies Signed-off-by: Markus Wieland <markus.wieland@sap.com>
… scheduler requests Signed-off-by: Markus Wieland <markus.wieland@sap.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical regressions can break startup, mutate shared informer state, overstate accelerator capacity, place reservations on incompatible hosts, and reject valid failover requests.
5 open issues
1 file not reviewed
api/v1alpha1/zz_generated.deepcopy.go: Generated file
What changed in this PR
Replaces transient Decision resources with direct pipeline execution across scheduling domains, introducing a shared in-process compute scheduler, typed requests, and domain-neutral results.
Changes:
- Adds shared scheduling locks, execution hooks, and pipeline result types.
- Migrates APIs, reservation controllers, filters, weighers, and tests to direct scheduling.
- Removes Decision API types, permissions, KPIs, alerts, and obsolete configuration.
This pull request requires changes. Six critical issues remain:
api/v1alpha1/decision_types.go:1— Shipped bundle defaults still registerDecisionandDecisionListGVKs, causing initialization to fail after these types are removed. Remove both entries from the Nova, Cinder, Manila, Pods, and Ironcore bundle configurations.internal/scheduling/machines/filter_weigher_pipeline_controller.go:64— Scheduling mutates informer-owned machine objects. Failed patches can leave false assignments in the cache and prevent retries; concurrent readers can also race. Deep-copy event objects before mutation.internal/scheduling/reservations/capacity/controller.go:843— Capacity probes omitAcceleratorsRequested, allowing ordinary hosts to inflate capacity estimates for accelerator flavors. Populate the flag from the presence ofaccel:device_profile.internal/scheduling/reservations/commitments/reservation_controller.go:377— Commitment requests also omitAcceleratorsRequested, permitting accelerator reservations on incompatible hosts. Populate the flag from the flavor’s device-profile key.internal/scheduling/reservations/extra_specs.go:46— The lowercaseqemufallback cannot match advertisedQEMUcapabilities, causing failover requests without an explicit hypervisor type to reject every host. Use canonical values, normalize supported lowercase inputs, and test against the real filter.internal/scheduling/reservations/extra_specs.go:48— Conversion preserves only the hypervisor capability, dropping constraints such as CPU architecture and allowing incompatible placements and capacity estimates. Preserve all supported capability entries and the adapter’s unsupported-operator handling.
| File | Description |
|---|---|
pkg/multicluster/client_test.go |
Replaces Decision scheme fixtures with Pipeline fixtures. |
internal/scheduling/reservations/failoverhost/generation_test.go |
Uses in-process scheduler stubs. |
internal/scheduling/reservations/failoverhost/controller.go |
Injects the shared compute scheduler. |
internal/scheduling/reservations/failover/reservation_scheduling_test.go |
Captures typed requests and checks flavor constraints. |
internal/scheduling/reservations/failover/integration_test.go |
Runs integration flows through the compute scheduler. |
internal/scheduling/reservations/failover/controller.go |
Replaces the HTTP scheduler client with an injected scheduler. |
internal/scheduling/reservations/failover/config.go |
Removes scheduler URL configuration. |
internal/scheduling/reservations/extra_specs.go |
Adds flavor-constraint extraction helpers. |
internal/scheduling/reservations/extra_specs_test.go |
Tests flavor-constraint extraction. |
internal/scheduling/reservations/commitments/reservation_controller.go |
Builds typed reservation requests and schedules directly. |
internal/scheduling/reservations/commitments/config.go |
Removes scheduler URL configuration. |
internal/scheduling/reservations/capacity/controller.go |
Executes typed capacity probes in-process. |
internal/scheduling/reservations/capacity/config.go |
Removes scheduler URL configuration. |
internal/scheduling/pods/filter_weigher_pipeline_controller.go |
Schedules pods directly without Decision objects. |
internal/scheduling/pods/filter_weigher_pipeline_controller_test.go |
Removes Decision reconciliation tests and updates result types. |
internal/scheduling/nova/vm_handler.go |
Writes scheduling results and explanations directly to VMs. |
internal/scheduling/nova/vm_handler_test.go |
Uses scheduler results instead of Decisions. |
internal/scheduling/nova/plugins/weighers/zz_index.go |
Registers weighers using typed compute requests. |
internal/scheduling/nova/plugins/weighers/vmware_binpack.go |
Reads sizing from typed compute requests. |
internal/scheduling/nova/plugins/weighers/vmware_binpack_test.go |
Updates bin-packing request fixtures. |
internal/scheduling/nova/plugins/weighers/vmware_avoid_short_term_contended_hosts.go |
Migrates the weigher to compute requests. |
internal/scheduling/nova/plugins/weighers/vmware_avoid_short_term_contended_hosts_test.go |
Updates contention request fixtures. |
internal/scheduling/nova/plugins/weighers/vmware_avoid_long_term_contended_hosts.go |
Migrates the weigher to compute requests. |
internal/scheduling/nova/plugins/weighers/vmware_avoid_long_term_contended_hosts_test.go |
Updates contention request fixtures. |
internal/scheduling/nova/plugins/weighers/vmware_anti_affinity_noisy_projects.go |
Reads project IDs from typed requests. |
internal/scheduling/nova/plugins/weighers/vmware_anti_affinity_noisy_projects_test.go |
Updates project-affinity request fixtures. |
internal/scheduling/nova/plugins/weighers/kvm_prefer_smaller_hosts.go |
Migrates the weigher to compute requests. |
internal/scheduling/nova/plugins/weighers/kvm_prefer_smaller_hosts_test.go |
Updates host-size request fixtures. |
internal/scheduling/nova/plugins/weighers/kvm_instance_group_soft_affinity.go |
Reads typed server-group fields. |
internal/scheduling/nova/plugins/weighers/kvm_instance_group_soft_affinity_test.go |
Updates server-group request fixtures. |
internal/scheduling/nova/plugins/weighers/kvm_failover_reservation_consolidation.go |
Reads resolved intent and resource-group fields. |
internal/scheduling/nova/plugins/weighers/kvm_failover_reservation_consolidation_test.go |
Updates consolidation request fixtures. |
internal/scheduling/nova/plugins/weighers/kvm_failover_evacuation.go |
Reads typed evacuation intent and instance ID. |
internal/scheduling/nova/plugins/weighers/kvm_failover_evacuation_test.go |
Updates evacuation request fixtures. |
internal/scheduling/nova/plugins/weighers/kvm_cr_migration_slot.go |
Reads typed migration intent and ownership fields. |
internal/scheduling/nova/plugins/weighers/kvm_cr_migration_slot_test.go |
Updates migration-slot request fixtures. |
internal/scheduling/nova/plugins/weighers/kvm_committed_resource_reservation.go |
Reads typed reservation-matching fields. |
internal/scheduling/nova/plugins/weighers/kvm_committed_resource_reservation_test.go |
Updates committed-resource request fixtures. |
internal/scheduling/nova/plugins/weighers/kvm_binpack.go |
Reads sizing from typed compute requests. |
internal/scheduling/nova/plugins/weighers/kvm_binpack_test.go |
Updates bin-packing request fixtures. |
internal/scheduling/nova/plugins/filters/zz_index.go |
Registers filters using typed compute requests. |
internal/scheduling/nova/plugins/filters/filter_status_conditions.go |
Migrates the filter to compute requests. |
internal/scheduling/nova/plugins/filters/filter_status_conditions_test.go |
Updates status-filter request fixtures. |
internal/scheduling/nova/plugins/filters/filter_requested_destination.go |
Reads typed destination constraints and intent. |
internal/scheduling/nova/plugins/filters/filter_quota_enforcement.go |
Reads resolved quota inputs from typed requests. |
internal/scheduling/nova/plugins/filters/filter_quota_enforcement_test.go |
Updates quota fixtures and intent mapping. |
internal/scheduling/nova/plugins/filters/filter_live_migratable.go |
Reads typed migration intent and source host. |
internal/scheduling/nova/plugins/filters/filter_instance_group_anti_affinity.go |
Reads typed server-group rules and membership. |
internal/scheduling/nova/plugins/filters/filter_instance_group_affinity.go |
Reads typed server-group host constraints. |
internal/scheduling/nova/plugins/filters/filter_image_properties.go |
Uses resolved image-hypervisor classifications. |
internal/scheduling/nova/plugins/filters/filter_host_instructions.go |
Reads typed host inclusion and exclusion lists. |
internal/scheduling/nova/plugins/filters/filter_has_requested_traits.go |
Consumes pre-extracted trait constraints. |
internal/scheduling/nova/plugins/filters/filter_has_enough_capacity_test.go |
Updates capacity-filter request fixtures. |
internal/scheduling/nova/plugins/filters/filter_has_accelerators.go |
Consumes the resolved accelerator-request flag. |
internal/scheduling/nova/plugins/filters/filter_external_customer.go |
Reads typed domain and intent fields. |
internal/scheduling/nova/plugins/filters/filter_exclude_hosts.go |
Migrates the filter to compute requests. |
internal/scheduling/nova/plugins/filters/filter_correct_az.go |
Reads the typed availability-zone field. |
internal/scheduling/nova/plugins/filters/filter_correct_az_test.go |
Updates availability-zone request fixtures. |
internal/scheduling/nova/plugins/filters/filter_capabilities.go |
Consumes typed capability requirements. |
internal/scheduling/nova/plugins/filters/filter_allowed_projects.go |
Reads typed project and intent fields. |
internal/scheduling/nova/plugins/filters/filter_aggregate_metadata.go |
Reads typed project and intent fields. |
internal/scheduling/nova/integration_test.go |
Wires the compute scheduler and side-effect handlers. |
internal/scheduling/nova/external_scheduler_api.go |
Calls the compute scheduler with VM and reservation post-hooks. |
internal/scheduling/nova/crs/recorder.go |
Accepts scheduling results and intent directly. |
internal/scheduling/nova/crs/recorder_test.go |
Replaces Decision fixtures with scheduler results. |
internal/scheduling/nova/cr_metrics_test.go |
Exercises tracking through the HTTP scheduler path. |
internal/scheduling/nova/candidate_gatherer.go |
Gathers candidates into typed compute requests. |
internal/scheduling/manila/filter_weigher_pipeline_controller.go |
Exposes direct, serialized pipeline scheduling. |
internal/scheduling/manila/external_scheduler_api.go |
Calls the scheduler directly without Decisions. |
internal/scheduling/machines/filter_weigher_pipeline_controller.go |
Schedules machines directly without Decisions. |
internal/scheduling/lib/weighing_explainer.go |
Uses domain-neutral pipeline result types. |
internal/scheduling/lib/pipeline_result.go |
Defines shared pipeline and step results. |
internal/scheduling/lib/filter_weigher_pipeline.go |
Returns domain-neutral pipeline results. |
internal/scheduling/lib/explanation.go |
Generates explanations from pipeline results. |
internal/scheduling/lib/explanation_test.go |
Updates explanation result fixtures. |
internal/scheduling/compute/scheduler.go |
Adds shared scheduling, locking, candidate gathering, and hooks. |
internal/scheduling/compute/result.go |
Aliases the shared pipeline result for compute scheduling. |
internal/scheduling/compute/request_test.go |
Tests typed request filtering and forced destinations. |
internal/scheduling/cinder/filter_weigher_pipeline_controller.go |
Exposes direct, serialized pipeline scheduling. |
internal/scheduling/cinder/external_scheduler_api.go |
Calls the scheduler directly without Decisions. |
internal/knowledge/kpis/plugins/deployment/decision_state.go |
Removes the Decision-state KPI implementation. |
helm/library/cortex/templates/rbac/role.yaml |
Removes Decision permissions. |
helm/library/cortex/files/crds/cortex.cloud_pipelines.yaml |
Removes the pipeline preselection option from the schema. |
helm/library/cortex-shim/templates/clusterrole.yaml |
Removes Decision permissions. |
helm/bundles/cortex-nova/values.yaml |
Removes the failover-host scheduler endpoint setting. |
helm/bundles/cortex-nova/templates/pipelines_kvm.yaml |
Removes pipeline-level preselection settings. |
helm/bundles/cortex-nova/templates/kpis.yaml |
Removes the Decision-state KPI resource. |
helm/bundles/cortex-nova/templates/alerts.yaml |
Removes Decision-state alerts. |
helm/bundles/cortex-manila/templates/kpis.yaml |
Removes the Decision-state KPI resource. |
helm/bundles/cortex-cinder/templates/kpis.yaml |
Removes the Decision-state KPI resource. |
helm/bundles/cortex-cinder/templates/alerts.yaml |
Removes Decision-state alerts. |
api/v1alpha1/scheduling_intent.go |
Preserves scheduling intent independently of Decisions. |
api/v1alpha1/pipeline_types.go |
Removes the pipeline preselection option. |
api/v1alpha1/decision_types.go |
Removes Decision API types and scheme registration. |
Review effort: Balanced
SoWieMarkus
marked this pull request as ready for review
November 3, 2026 16:00
…herPipelineController Signed-off-by: Markus Wieland <markus.wieland@sap.com>
Signed-off-by: Markus Wieland <markus.wieland@sap.com>
…utilizing shared functions from the Nova API Signed-off-by: Markus Wieland <markus.wieland@sap.com>
… functions Signed-off-by: Markus Wieland <markus.wieland@sap.com>
Signed-off-by: Markus Wieland <markus.wieland@sap.com>
…om multiple values.yaml files Signed-off-by: Markus Wieland <markus.wieland@sap.com>
Signed-off-by: Markus Wieland <markus.wieland@sap.com>
Signed-off-by: Markus Wieland <markus.wieland@sap.com>
Signed-off-by: Markus Wieland <markus.wieland@sap.com>
Contributor
Test Coverage ReportTest Coverage 📊: 71.9% |
umswmayj
approved these changes
Nov 4, 2026
PhilippMatthes
approved these changes
Nov 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

No description provided.