diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/allocator.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/allocator.go index aa629cde8b4..bd7f1065970 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/structured/allocator.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/structured/allocator.go @@ -19,11 +19,13 @@ package structured import ( "context" "fmt" + "strings" v1 "k8s.io/api/core/v1" resourceapi "k8s.io/api/resource/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/dynamic-resource-allocation/cel" "k8s.io/dynamic-resource-allocation/structured/internal" "k8s.io/dynamic-resource-allocation/structured/internal/experimental" @@ -135,17 +137,40 @@ func NewAllocator(ctx context.Context, // file name!) into "stable", or individual chunks can be copied over. // // Unit tests are shared between all implementations. + var enabledAllocators []string for _, allocator := range availableAllocators { + // Disabled? + if !allocatorEnabled(allocator.name) { + continue + } + enabledAllocators = append(enabledAllocators, allocator.name) + // All required features supported? if allocator.supportedFeatures.Set().IsSuperset(features.Set()) { // Use it! return allocator.newAllocator(ctx, features, allocatedState, classLister, slices, celCache) } } - return nil, fmt.Errorf("internal error: no allocator available for feature set %v", features) + return nil, fmt.Errorf("internal error: no allocator available for feature set %+v, enabled allocators: %s", features, strings.Join(enabledAllocators, ", ")) +} + +// EnableAllocators, if passed a non-empty list, controls which allocators may get picked by NewAllocator. +// The entries are the names of the implementing package ("stable", "incubating", "experimental"). +// Not thread-safe, meant for use during testing. +func EnableAllocators(names ...string) { + explicitlyEnabledAllocators = sets.New(names...) +} + +// explicitlyEnabledAllocators stores the result of EnableAllocators. +// If empty (the default), all available allocators are enabled. +var explicitlyEnabledAllocators sets.Set[string] + +func allocatorEnabled(name string) bool { + return len(explicitlyEnabledAllocators) == 0 || explicitlyEnabledAllocators.Has(name) } var availableAllocators = []struct { + name string supportedFeatures Features newAllocator func(ctx context.Context, features Features, @@ -157,6 +182,7 @@ var availableAllocators = []struct { }{ // Most stable first. { + name: "stable", supportedFeatures: stable.SupportedFeatures, newAllocator: func(ctx context.Context, features Features, @@ -169,6 +195,7 @@ var availableAllocators = []struct { }, }, { + name: "incubating", supportedFeatures: incubating.SupportedFeatures, newAllocator: func(ctx context.Context, features Features, @@ -181,6 +208,7 @@ var availableAllocators = []struct { }, }, { + name: "experimental", supportedFeatures: experimental.SupportedFeatures, newAllocator: func(ctx context.Context, features Features, diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/README.md b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/README.md new file mode 100644 index 00000000000..81c68ee5ce7 --- /dev/null +++ b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/README.md @@ -0,0 +1,28 @@ +The allocator code exists in three variants which get picked depending on which +features are enabled: +- stable: for a "GA only" feature configuration, minimal changes going forward +- incubating: the default implementation, adds support for beta features +- experimental: under active development, including alpha features + +This structure serves as a safety net because experimental changes cannot break +more stable Kubernetes configurations, something that happened already once +despite careful reviews. + +The goal is to rotate the implementations wholesale instead of copying +individual code chunks, i.e. at some point "incubating" replaces "stable", +"experimental" replaces "incubating", and "experimental" becomes a copy of +"incubating" until new changes get added to it again. + +Ideally changes should be limited to "experimental", but sometimes changes have +to be applied the same way across different variants, for example bug fixes +or changes to the package API. + +Tests are shared between all implementations, with test cases applied depending +on what features they require. Further testing is covered by +test/integration/scheduler_perf. When promoting implementations, the selection +of implementations which support certain features there needs to be +updated. For example `[]string{"experimental"}` in `TestSchedulerPerf` of +`test/integration/scheduler_perf/dra/consumablecapacity/consumablecapacity_test.go` +will eventually become `[]string{"incubating", "experimental"}`. The explicit +selection of the implementation for benchmarking in +`EnableAllocators("experimental")` then becomes `EnableAllocators("incubating")`. diff --git a/test/integration/scheduler_perf/dra/consumablecapacity/consumablecapacity_test.go b/test/integration/scheduler_perf/dra/consumablecapacity/consumablecapacity_test.go index 16ee41eb337..95d77534c2e 100644 --- a/test/integration/scheduler_perf/dra/consumablecapacity/consumablecapacity_test.go +++ b/test/integration/scheduler_perf/dra/consumablecapacity/consumablecapacity_test.go @@ -22,6 +22,7 @@ import ( "testing" _ "k8s.io/component-base/logs/json/register" + "k8s.io/dynamic-resource-allocation/structured" perf "k8s.io/kubernetes/test/integration/scheduler_perf" ) @@ -35,9 +36,22 @@ func TestMain(m *testing.M) { } func TestSchedulerPerf(t *testing.T) { - perf.RunIntegrationPerfScheduling(t, "performance-config.yaml") + // Verify correct behavior with all available allocators for this feature. + for _, allocatorName := range []string{"experimental"} { + t.Run(allocatorName, func(t *testing.T) { + structured.EnableAllocators(allocatorName) + defer structured.EnableAllocators() + + perf.RunIntegrationPerfScheduling(t, "performance-config.yaml") + }) + } } func BenchmarkPerfScheduling(b *testing.B) { + // Restrict benchmarking to the allocator for this feature + // to ensure that we do not accidentally pick something else. + structured.EnableAllocators("experimental") + defer structured.EnableAllocators() + perf.RunBenchmarkPerfScheduling(b, "performance-config.yaml", "dra_consumablecapacity", nil) } diff --git a/test/integration/scheduler_perf/dra/devicetaints/devicetaints_test.go b/test/integration/scheduler_perf/dra/devicetaints/devicetaints_test.go index 46b4133d55d..d17b0143785 100644 --- a/test/integration/scheduler_perf/dra/devicetaints/devicetaints_test.go +++ b/test/integration/scheduler_perf/dra/devicetaints/devicetaints_test.go @@ -22,6 +22,7 @@ import ( "testing" _ "k8s.io/component-base/logs/json/register" + "k8s.io/dynamic-resource-allocation/structured" perf "k8s.io/kubernetes/test/integration/scheduler_perf" ) @@ -35,9 +36,22 @@ func TestMain(m *testing.M) { } func TestSchedulerPerf(t *testing.T) { - perf.RunIntegrationPerfScheduling(t, "performance-config.yaml") + // Verify correct behavior with all available allocators for this feature + for _, allocatorName := range []string{"experimental"} { + t.Run(allocatorName, func(t *testing.T) { + structured.EnableAllocators(allocatorName) + defer structured.EnableAllocators() + + perf.RunIntegrationPerfScheduling(t, "performance-config.yaml") + }) + } } func BenchmarkPerfScheduling(b *testing.B) { + // Restrict benchmarking to the allocator for this feature + // to ensure that we do not accidentally pick something else. + structured.EnableAllocators("experimental") + defer structured.EnableAllocators() + perf.RunBenchmarkPerfScheduling(b, "performance-config.yaml", "dra_devicetaints", nil) } diff --git a/test/integration/scheduler_perf/dra/dra_test.go b/test/integration/scheduler_perf/dra/dra_test.go index 419de6b0181..06a420480bd 100644 --- a/test/integration/scheduler_perf/dra/dra_test.go +++ b/test/integration/scheduler_perf/dra/dra_test.go @@ -21,7 +21,11 @@ import ( "os" "testing" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" _ "k8s.io/component-base/logs/json/register" + "k8s.io/dynamic-resource-allocation/structured" + "k8s.io/kubernetes/pkg/features" perf "k8s.io/kubernetes/test/integration/scheduler_perf" ) @@ -35,9 +39,38 @@ func TestMain(m *testing.M) { } func TestSchedulerPerf(t *testing.T) { - perf.RunIntegrationPerfScheduling(t, "performance-config.yaml") + // Verify correct behavior with all available allocators. + for _, allocatorName := range []string{"stable", "incubating", "experimental"} { + t.Run(allocatorName, func(t *testing.T) { + structured.EnableAllocators(allocatorName) + defer structured.EnableAllocators() + + // In order to run with the "stable" implementation, we have to disable + // some features, something that isn't specified in the YAML + // configuration because for other implementations we want the default + // features. Using "AllAlpha" and "AllBeta" would be better here, + // but interacts poorly with setting the emulated version to 1.33 later + // on ("scheduler_perf.go:1117: failed to set emulation version to 1.33 during test: + // cannot set feature gate NominatedNodeNameForExpectation to false, feature is PreAlpha at emulated version 1.33, ...") + // + // Once the current "incubating" becomes "stable", this can be replaced + // with two sub tests: + // - "ga-only": keep disabling optional features + // - "default": don't change features + if allocatorName == "stable" { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRAAdminAccess, false) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRAPrioritizedList, false) + } + + perf.RunIntegrationPerfScheduling(t, "performance-config.yaml") + }) + } } func BenchmarkPerfScheduling(b *testing.B) { + // Restrict benchmarking to the default allocator. + structured.EnableAllocators("incubating") + defer structured.EnableAllocators() + perf.RunBenchmarkPerfScheduling(b, "performance-config.yaml", "dra", nil) } diff --git a/test/integration/scheduler_perf/dra/extendedresource/extendedresource_test.go b/test/integration/scheduler_perf/dra/extendedresource/extendedresource_test.go index b2a8adfcaaf..d26b5528b7d 100644 --- a/test/integration/scheduler_perf/dra/extendedresource/extendedresource_test.go +++ b/test/integration/scheduler_perf/dra/extendedresource/extendedresource_test.go @@ -22,6 +22,7 @@ import ( "testing" _ "k8s.io/component-base/logs/json/register" + "k8s.io/dynamic-resource-allocation/structured" perf "k8s.io/kubernetes/test/integration/scheduler_perf" ) @@ -35,9 +36,22 @@ func TestMain(m *testing.M) { } func TestSchedulerPerf(t *testing.T) { - perf.RunIntegrationPerfScheduling(t, "performance-config.yaml") + // Verify correct behavior with all available allocators for this feature. + for _, allocatorName := range []string{"experimental"} { + t.Run(allocatorName, func(t *testing.T) { + structured.EnableAllocators(allocatorName) + defer structured.EnableAllocators() + + perf.RunIntegrationPerfScheduling(t, "performance-config.yaml") + }) + } } func BenchmarkPerfScheduling(b *testing.B) { + // Restrict benchmarking to the allocator for this feature + // to ensure that we do not accidentally pick something else. + structured.EnableAllocators("experimental") + defer structured.EnableAllocators() + perf.RunBenchmarkPerfScheduling(b, "performance-config.yaml", "dra_extendedresource", nil) } diff --git a/test/integration/scheduler_perf/dra/partitionabledevices/partitionabledevices_test.go b/test/integration/scheduler_perf/dra/partitionabledevices/partitionabledevices_test.go index d2789f6ae7d..76bd292958a 100644 --- a/test/integration/scheduler_perf/dra/partitionabledevices/partitionabledevices_test.go +++ b/test/integration/scheduler_perf/dra/partitionabledevices/partitionabledevices_test.go @@ -22,6 +22,7 @@ import ( "testing" _ "k8s.io/component-base/logs/json/register" + "k8s.io/dynamic-resource-allocation/structured" perf "k8s.io/kubernetes/test/integration/scheduler_perf" ) @@ -35,9 +36,22 @@ func TestMain(m *testing.M) { } func TestSchedulerPerf(t *testing.T) { - perf.RunIntegrationPerfScheduling(t, "performance-config.yaml") + // Verify correct behavior with all available allocators for this feature. + for _, allocatorName := range []string{"experimental"} { + t.Run(allocatorName, func(t *testing.T) { + structured.EnableAllocators(allocatorName) + defer structured.EnableAllocators() + + perf.RunIntegrationPerfScheduling(t, "performance-config.yaml") + }) + } } func BenchmarkPerfScheduling(b *testing.B) { + // Restrict benchmarking to the allocator for this feature + // to ensure that we do not accidentally pick something else. + structured.EnableAllocators("experimental") + defer structured.EnableAllocators() + perf.RunBenchmarkPerfScheduling(b, "performance-config.yaml", "dra_partitionabledevices", nil) } diff --git a/test/integration/scheduler_perf/dra/performance-config.yaml b/test/integration/scheduler_perf/dra/performance-config.yaml index e521c24e061..663f4cd183b 100644 --- a/test/integration/scheduler_perf/dra/performance-config.yaml +++ b/test/integration/scheduler_perf/dra/performance-config.yaml @@ -27,8 +27,6 @@ - name: PublishResourceSlices featureGates: DynamicResourceAllocation: true - DRAPrioritizedList: false - DRAAdminAccess: false workloadTemplate: - opcode: createNodes countParam: $nodesWithoutDRA @@ -315,7 +313,6 @@ - name: SchedulingWithResourceClaim featureGates: DynamicResourceAllocation: true - # SchedulerQueueingHints: true workloadTemplate: - opcode: createNodes countParam: $nodesWithoutDRA diff --git a/test/integration/scheduler_perf/dra/prioritizedlist/prioritizedlist_test.go b/test/integration/scheduler_perf/dra/prioritizedlist/prioritizedlist_test.go index 76cba27ae0d..605dcae1835 100644 --- a/test/integration/scheduler_perf/dra/prioritizedlist/prioritizedlist_test.go +++ b/test/integration/scheduler_perf/dra/prioritizedlist/prioritizedlist_test.go @@ -22,6 +22,7 @@ import ( "testing" _ "k8s.io/component-base/logs/json/register" + "k8s.io/dynamic-resource-allocation/structured" perf "k8s.io/kubernetes/test/integration/scheduler_perf" ) @@ -35,9 +36,22 @@ func TestMain(m *testing.M) { } func TestSchedulerPerf(t *testing.T) { - perf.RunIntegrationPerfScheduling(t, "performance-config.yaml") + // Verify correct behavior with all available allocators for this feature. + for _, allocatorName := range []string{"incubating", "experimental"} { + t.Run(allocatorName, func(t *testing.T) { + structured.EnableAllocators(allocatorName) + defer structured.EnableAllocators() + + perf.RunIntegrationPerfScheduling(t, "performance-config.yaml") + }) + } } func BenchmarkPerfScheduling(b *testing.B) { + // Restrict benchmarking to the allocator for this feature + // to ensure that we do not accidentally pick something else. + structured.EnableAllocators("incubating") + defer structured.EnableAllocators() + perf.RunBenchmarkPerfScheduling(b, "performance-config.yaml", "dra_prioritizedlist", nil) }