diff --git a/api/external/nova/messages.go b/api/external/nova/messages.go index fba3a4cee..d3021ffd3 100644 --- a/api/external/nova/messages.go +++ b/api/external/nova/messages.go @@ -88,6 +88,75 @@ func (r ExternalSchedulerRequest) Filter(includedHosts map[string]float64) lib.F return r } +// IsForcedDestination reports whether Nova forced this request onto specific +// hosts/nodes and the scheduler filters should be skipped entirely. +// +// This replicates Nova's native behavior: when force_hosts or force_nodes is +// set and the scheduler hint "_nova_check_type" is not set, Nova skips the +// filters and returns only the forced destinations. +// See: https://github.com/sapcc/nova/blob/05f384a938e3d6a8740a8f404d79d767d6ebdbd7/nova/scheduler/host_manager.py#L610-L620 +func (r ExternalSchedulerRequest) IsForcedDestination() bool { + forceHosts := r.Spec.Data.ForceHosts != nil && len(*r.Spec.Data.ForceHosts) > 0 + forceNodes := r.Spec.Data.ForceNodes != nil && len(*r.Spec.Data.ForceNodes) > 0 + if !forceHosts && !forceNodes { + return false + } + // If _nova_check_type is set (e.g. rebuild/evacuate/resize), the host is + // forced but must still be validated by the filters, so don't skip. + checkType, err := r.Spec.Data.GetSchedulerHintStr("_nova_check_type") + if err == nil && checkType != "" { + return false + } + return true +} + +// ForcedHosts returns the subset of request hosts that match the forced +// destinations (force_hosts / force_nodes). Matching mirrors Nova: +// - force_hosts matches on the compute host name, case-insensitive. +// - force_nodes matches on the hypervisor hostname, case-sensitive. +// - When both are set, a host must match both (intersection). +// +// The returned slice preserves the order of the request hosts. If nothing +// matches, an empty slice is returned (Nova would then raise NoValidHost). +func (r ExternalSchedulerRequest) ForcedHosts() []string { + var forceHosts []string + if r.Spec.Data.ForceHosts != nil { + forceHosts = *r.Spec.Data.ForceHosts + } + var forceNodes []string + if r.Spec.Data.ForceNodes != nil { + forceNodes = *r.Spec.Data.ForceNodes + } + forceHostSet := make(map[string]bool, len(forceHosts)) + for _, h := range forceHosts { + forceHostSet[strings.ToLower(h)] = true + } + forceNodeSet := make(map[string]bool, len(forceNodes)) + for _, n := range forceNodes { + forceNodeSet[n] = true + } + matched := make([]string, 0, len(r.Hosts)) + // Candidates are (host, node) pairs, so a single compute host may appear + // multiple times (once per node). Deduplicate by compute host name, since + // the response only carries host names. + seen := make(map[string]bool, len(r.Hosts)) + for _, host := range r.Hosts { + if len(forceHostSet) > 0 && !forceHostSet[strings.ToLower(host.ComputeHost)] { + continue + } + if len(forceNodeSet) > 0 && !forceNodeSet[host.HypervisorHostname] { + continue + } + key := strings.ToLower(host.ComputeHost) + if seen[key] { + continue + } + seen[key] = true + matched = append(matched, host.ComputeHost) + } + return matched +} + type FlavorType string const ( diff --git a/api/external/nova/messages_test.go b/api/external/nova/messages_test.go index a9074bd3a..b76cddf03 100644 --- a/api/external/nova/messages_test.go +++ b/api/external/nova/messages_test.go @@ -569,3 +569,178 @@ func TestNovaImageMeta_GetHypervisorType(t *testing.T) { }) } } + +func strPtrSlice(s ...string) *[]string { + out := s + return &out +} + +func TestIsForcedDestination(t *testing.T) { + tests := []struct { + name string + forceHosts *[]string + forceNodes *[]string + hints map[string]any + expected bool + }{ + { + name: "no force", + expected: false, + }, + { + name: "empty force_hosts slice", + forceHosts: strPtrSlice(), + expected: false, + }, + { + name: "force_hosts set, no check_type", + forceHosts: strPtrSlice("node017-bb545"), + expected: true, + }, + { + name: "force_nodes set, no check_type", + forceNodes: strPtrSlice("domain-c123"), + expected: true, + }, + { + name: "force_hosts set with check_type rebuild", + forceHosts: strPtrSlice("node017-bb545"), + hints: map[string]any{"_nova_check_type": "rebuild"}, + expected: false, + }, + { + name: "force_hosts set with check_type as list", + forceHosts: strPtrSlice("node017-bb545"), + hints: map[string]any{"_nova_check_type": []any{"resize"}}, + expected: false, + }, + { + name: "force_hosts set with empty check_type", + forceHosts: strPtrSlice("node017-bb545"), + hints: map[string]any{"_nova_check_type": ""}, + expected: true, + }, + { + name: "force_hosts set, hints without check_type", + forceHosts: strPtrSlice("node017-bb545"), + hints: map[string]any{"some_other_hint": "value"}, + expected: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := ExternalSchedulerRequest{ + Spec: NovaObject[NovaSpec]{ + Data: NovaSpec{ + ForceHosts: tt.forceHosts, + ForceNodes: tt.forceNodes, + SchedulerHints: tt.hints, + }, + }, + } + if got := req.IsForcedDestination(); got != tt.expected { + t.Errorf("expected %v, got %v", tt.expected, got) + } + }) + } +} + +func TestForcedHosts(t *testing.T) { + hosts := []ExternalSchedulerHost{ + {ComputeHost: "node017-bb545", HypervisorHostname: "domain-c17"}, + {ComputeHost: "node018-bb545", HypervisorHostname: "domain-c18"}, + {ComputeHost: "NODE019-bb545", HypervisorHostname: "domain-c19"}, + } + tests := []struct { + name string + hosts []ExternalSchedulerHost + forceHosts *[]string + forceNodes *[]string + expected []string + }{ + { + name: "match single host", + forceHosts: strPtrSlice("node017-bb545"), + expected: []string{"node017-bb545"}, + }, + { + name: "match host case-insensitive", + forceHosts: strPtrSlice("node019-bb545"), + expected: []string{"NODE019-bb545"}, + }, + { + name: "match node case-sensitive", + forceNodes: strPtrSlice("domain-c18"), + expected: []string{"node018-bb545"}, + }, + { + name: "node case-sensitive no match", + forceNodes: strPtrSlice("DOMAIN-C18"), + expected: []string{}, + }, + { + name: "intersection of host and node", + forceHosts: strPtrSlice("node017-bb545", "node018-bb545"), + forceNodes: strPtrSlice("domain-c18"), + expected: []string{"node018-bb545"}, + }, + { + name: "intersection empty", + forceHosts: strPtrSlice("node017-bb545"), + forceNodes: strPtrSlice("domain-c18"), + expected: []string{}, + }, + { + name: "no match returns empty", + forceHosts: strPtrSlice("unknown-host"), + expected: []string{}, + }, + { + name: "multi-node host deduplicated by force_hosts", + hosts: []ExternalSchedulerHost{ + {ComputeHost: "hostA", HypervisorHostname: "node1"}, + {ComputeHost: "hostA", HypervisorHostname: "node2"}, + {ComputeHost: "hostB", HypervisorHostname: "node1"}, + }, + forceHosts: strPtrSlice("hostA"), + expected: []string{"hostA"}, + }, + { + name: "multi-node host intersection picks single node", + hosts: []ExternalSchedulerHost{ + {ComputeHost: "hostA", HypervisorHostname: "node1"}, + {ComputeHost: "hostA", HypervisorHostname: "node2"}, + {ComputeHost: "hostB", HypervisorHostname: "node1"}, + }, + forceHosts: strPtrSlice("hostA"), + forceNodes: strPtrSlice("node1"), + expected: []string{"hostA"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + candidates := tt.hosts + if candidates == nil { + candidates = hosts + } + req := ExternalSchedulerRequest{ + Hosts: candidates, + Spec: NovaObject[NovaSpec]{ + Data: NovaSpec{ + ForceHosts: tt.forceHosts, + ForceNodes: tt.forceNodes, + }, + }, + } + got := req.ForcedHosts() + if len(got) != len(tt.expected) { + t.Fatalf("expected %v, got %v", tt.expected, got) + } + for i, h := range tt.expected { + if got[i] != h { + t.Errorf("expected host[%d]=%s, got %s", i, h, got[i]) + } + } + }) + } +} diff --git a/helm/bundles/cortex-nova/values.yaml b/helm/bundles/cortex-nova/values.yaml index cec441eaa..40a82dc99 100644 --- a/helm/bundles/cortex-nova/values.yaml +++ b/helm/bundles/cortex-nova/values.yaml @@ -169,6 +169,11 @@ cortex-scheduling-controllers: # Number of top hosts to shuffle for evacuation requests. # Set to 0 or negative to disable shuffling. evacuationShuffleK: 3 + # If true (default), the external scheduler API replicates Nova's + # forced-destination behavior: requests with force_hosts/force_nodes skip + # the scheduling pipeline and filters. Set to false to disable this as a + # kill-switch and route forced requests through the normal pipeline. + forcedDestinationEnabled: true committedResourceReservationController: # Pipeline selection for CR reservation scheduling. The catch-all default covers # general-purpose flavors. For HANA flavor groups, add an explicit entry, e.g.: diff --git a/internal/scheduling/nova/external_scheduler_api.go b/internal/scheduling/nova/external_scheduler_api.go index 4464c46a0..8eed218b3 100644 --- a/internal/scheduling/nova/external_scheduler_api.go +++ b/internal/scheduling/nova/external_scheduler_api.go @@ -33,6 +33,17 @@ type HTTPAPIConfig struct { // NovaLimitHostsToRequest, if true, will filter the Nova scheduler response // to only include hosts that were in the original request. NovaLimitHostsToRequest bool `json:"novaLimitHostsToRequest,omitempty"` + // ForcedDestinationEnabled toggles replicating Nova's forced-destination + // behavior (force_hosts/force_nodes skip the scheduling pipeline and + // filters). Defaults to true when unset; set to false to disable and let + // forced requests flow through the normal pipeline instead. + ForcedDestinationEnabled *bool `json:"forcedDestinationEnabled,omitempty"` +} + +// forcedDestinationEnabled reports whether the forced-destination behavior is +// enabled. It defaults to true when the config value is unset. +func (c HTTPAPIConfig) forcedDestinationEnabled() bool { + return c.ForcedDestinationEnabled == nil || *c.ForcedDestinationEnabled } type HTTPAPIDelegate interface { @@ -214,6 +225,23 @@ func (httpAPI *httpAPI) NovaExternalScheduler(w http.ResponseWriter, r *http.Req return } + // Replicate Nova's forced-destination behavior: when the request is forced + // onto specific hosts/nodes (force_hosts/force_nodes) and no _nova_check_type + // is set, Nova skips its filters entirely. We do the same here and return + // only the forced hosts, bypassing pipeline inference and execution. + if httpAPI.config.forcedDestinationEnabled() && requestData.IsForcedDestination() { + hosts := requestData.ForcedHosts() + logger.Info("forced destination request, skipping filters", "hosts", hosts) + response := api.ExternalSchedulerResponse{Hosts: hosts} + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(response); err != nil { + c.Respond(logger, http.StatusInternalServerError, err, "failed to encode response") + return + } + c.Respond(logger, http.StatusOK, nil, "Success") + return + } + // If the pipeline name is not set, infer it from the request data. if requestData.Pipeline == "" { var err error diff --git a/internal/scheduling/nova/external_scheduler_api_test.go b/internal/scheduling/nova/external_scheduler_api_test.go index 70ce3c334..e5f3ca000 100644 --- a/internal/scheduling/nova/external_scheduler_api_test.go +++ b/internal/scheduling/nova/external_scheduler_api_test.go @@ -383,6 +383,102 @@ func TestHTTPAPI_NovaExternalScheduler_DecisionCreation(t *testing.T) { } } +func TestHTTPAPI_NovaExternalScheduler_ForcedDestination(t *testing.T) { + forceHosts := []string{"host2"} + requestData := novaapi.ExternalSchedulerRequest{ + Spec: novaapi.NovaObject[novaapi.NovaSpec]{ + Data: novaapi.NovaSpec{ + InstanceUUID: "test-uuid", + ForceHosts: &forceHosts, + }, + }, + Hosts: []novaapi.ExternalSchedulerHost{ + {ComputeHost: "host1", HypervisorHostname: "domain-c1"}, + {ComputeHost: "host2", HypervisorHostname: "domain-c2"}, + }, + Weights: map[string]float64{ + "host1": 1.0, + "host2": 2.0, + }, + } + + delegateCalled := false + delegate := &mockHTTPAPIDelegate{ + processDecisionFunc: func(ctx context.Context, decision *v1alpha1.Decision) error { + delegateCalled = true + return nil + }, + } + api := NewAPI(HTTPAPIConfig{}, delegate).(*httpAPI) + + body, err := json.Marshal(requestData) + if err != nil { + t.Fatalf("Failed to marshal request data: %v", err) + } + req := httptest.NewRequest(http.MethodPost, "/scheduler/nova/external", bytes.NewReader(body)) + w := httptest.NewRecorder() + + api.NovaExternalScheduler(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("Expected status %d, got %d", http.StatusOK, w.Code) + } + if delegateCalled { + t.Error("delegate should not be called for forced destination requests") + } + var response novaapi.ExternalSchedulerResponse + if err := json.NewDecoder(w.Body).Decode(&response); err != nil { + t.Fatalf("Failed to decode response: %v", err) + } + if len(response.Hosts) != 1 || response.Hosts[0] != "host2" { + t.Errorf("Expected hosts [host2], got %v", response.Hosts) + } +} + +func TestHTTPAPI_NovaExternalScheduler_ForcedDestinationDisabled(t *testing.T) { + forceHosts := []string{"host2"} + requestData := novaapi.ExternalSchedulerRequest{ + Spec: novaapi.NovaObject[novaapi.NovaSpec]{ + Data: novaapi.NovaSpec{ + InstanceUUID: "test-uuid", + ForceHosts: &forceHosts, + }, + }, + Hosts: []novaapi.ExternalSchedulerHost{ + {ComputeHost: "host1", HypervisorHostname: "domain-c1"}, + {ComputeHost: "host2", HypervisorHostname: "domain-c2"}, + }, + Weights: map[string]float64{ + "host1": 1.0, + "host2": 2.0, + }, + Pipeline: "test-pipeline", + } + + delegateCalled := false + delegate := &mockHTTPAPIDelegate{ + processDecisionFunc: func(ctx context.Context, decision *v1alpha1.Decision) error { + delegateCalled = true + return nil + }, + } + disabled := false + api := NewAPI(HTTPAPIConfig{ForcedDestinationEnabled: &disabled}, delegate).(*httpAPI) + + body, err := json.Marshal(requestData) + if err != nil { + t.Fatalf("Failed to marshal request data: %v", err) + } + req := httptest.NewRequest(http.MethodPost, "/scheduler/nova/external", bytes.NewReader(body)) + w := httptest.NewRecorder() + + api.NovaExternalScheduler(w, req) + + if !delegateCalled { + t.Error("delegate should be called when forced destination is disabled") + } +} + func TestLimitHostsToRequest(t *testing.T) { tests := []struct { name string