Skip to content

Commit 9635a25

Browse files
authored
Merge pull request kubernetes#129749 from pohly/dra-cel-cost-estimate-during-validation
DRA: CEL cost estimate during validation
2 parents 2deb8af + d889bd1 commit 9635a25

File tree

3 files changed

+26
-10
lines changed

3 files changed

+26
-10
lines changed

staging/src/k8s.io/dynamic-resource-allocation/cel/cache.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ func NewCache(maxCacheEntries int) *Cache {
4343
// GetOrCompile checks whether the cache already has a compilation result
4444
// and returns that if available. Otherwise it compiles, stores successful
4545
// results and returns the new result.
46+
//
47+
// Cost estimation is disabled.
4648
func (c *Cache) GetOrCompile(expression string) CompilationResult {
4749
// Compiling a CEL expression is expensive enough that it is cheaper
4850
// to lock a mutex than doing it several times in parallel.
@@ -55,7 +57,7 @@ func (c *Cache) GetOrCompile(expression string) CompilationResult {
5557
return *cached
5658
}
5759

58-
expr := GetCompiler().CompileCELExpression(expression, Options{})
60+
expr := GetCompiler().CompileCELExpression(expression, Options{DisableCostEstimation: true})
5961
if expr.Error == nil {
6062
c.add(expression, &expr)
6163
}

staging/src/k8s.io/dynamic-resource-allocation/cel/cache_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package cel
1818

1919
import (
2020
"fmt"
21+
"math"
2122
"sync"
2223
"testing"
2324

@@ -73,6 +74,11 @@ func TestCacheSemantic(t *testing.T) {
7374
if resultFalse == resultFalseAgain {
7475
t.Fatal("result of compiling `false` should have been evicted from the cache")
7576
}
77+
78+
// Cost estimation must be off (not needed by scheduler).
79+
if resultFalseAgain.MaxCost != math.MaxUint64 {
80+
t.Error("cost estimation should have been disabled, was enabled")
81+
}
7682
}
7783

7884
func TestCacheConcurrency(t *testing.T) {

staging/src/k8s.io/dynamic-resource-allocation/cel/compile.go

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"errors"
2222
"fmt"
23+
"math"
2324
"reflect"
2425
"strings"
2526
"sync"
@@ -120,6 +121,10 @@ type Options struct {
120121

121122
// CostLimit allows overriding the default runtime cost limit [resourceapi.CELSelectorExpressionMaxCost].
122123
CostLimit *uint64
124+
125+
// DisableCostEstimation can be set to skip estimating the worst-case CEL cost.
126+
// If disabled or after an error, [CompilationResult.MaxCost] will be set to [math.Uint64].
127+
DisableCostEstimation bool
123128
}
124129

125130
// CompileCELExpression returns a compiled CEL expression. It evaluates to bool.
@@ -133,6 +138,7 @@ func (c compiler) CompileCELExpression(expression string, options Options) Compi
133138
Detail: errorString,
134139
},
135140
Expression: expression,
141+
MaxCost: math.MaxUint64,
136142
}
137143
}
138144

@@ -141,10 +147,6 @@ func (c compiler) CompileCELExpression(expression string, options Options) Compi
141147
return resultError(fmt.Sprintf("unexpected error loading CEL environment: %v", err), apiservercel.ErrorTypeInternal)
142148
}
143149

144-
// We don't have a SizeEstimator. The potential size of the input (= a
145-
// device) is already declared in the definition of the environment.
146-
estimator := c.newCostEstimator()
147-
148150
ast, issues := env.Compile(expression)
149151
if issues != nil {
150152
return resultError("compilation failed: "+issues.String(), apiservercel.ErrorTypeInvalid)
@@ -176,15 +178,21 @@ func (c compiler) CompileCELExpression(expression string, options Options) Compi
176178
OutputType: ast.OutputType(),
177179
Environment: env,
178180
emptyMapVal: env.CELTypeAdapter().NativeToValue(map[string]any{}),
181+
MaxCost: math.MaxUint64,
179182
}
180183

181-
costEst, err := env.EstimateCost(ast, estimator)
182-
if err != nil {
183-
compilationResult.Error = &apiservercel.Error{Type: apiservercel.ErrorTypeInternal, Detail: "cost estimation failed: " + err.Error()}
184-
return compilationResult
184+
if !options.DisableCostEstimation {
185+
// We don't have a SizeEstimator. The potential size of the input (= a
186+
// device) is already declared in the definition of the environment.
187+
estimator := c.newCostEstimator()
188+
costEst, err := env.EstimateCost(ast, estimator)
189+
if err != nil {
190+
compilationResult.Error = &apiservercel.Error{Type: apiservercel.ErrorTypeInternal, Detail: "cost estimation failed: " + err.Error()}
191+
return compilationResult
192+
}
193+
compilationResult.MaxCost = costEst.Max
185194
}
186195

187-
compilationResult.MaxCost = costEst.Max
188196
return compilationResult
189197
}
190198

0 commit comments

Comments
 (0)