Fix poolmanager sets 0 timeout for function specialization (#1439)

This commit is contained in:
Ta-Ching Chen
2019-11-26 19:22:49 +08:00
committed by GitHub
parent e38baeec36
commit 7e8e968013
7 changed files with 229 additions and 111 deletions
+10 -1
View File
@@ -111,8 +111,17 @@ func (executor *Executor) serveCreateFuncServices() {
// still can serve other subsequent requests.
buffer := 10 // add some buffer time for specialization
specializationTimeout := req.function.Spec.InvokeStrategy.ExecutionStrategy.SpecializationTimeout
// set minimum specialization timeout to avoid illegal input and
// compatibility problem when applying old spec file that doesn't
// have specialization timeout field.
if specializationTimeout < fv1.DefaultSpecializationTimeOut {
specializationTimeout = fv1.DefaultSpecializationTimeOut
}
fnSpecializationTimeoutContext, cancel := context.WithTimeout(context.Background(),
time.Duration(req.function.Spec.InvokeStrategy.ExecutionStrategy.SpecializationTimeout+buffer)*time.Second)
time.Duration(specializationTimeout+buffer)*time.Second)
defer cancel()
fsvc, err := executor.createServiceForFunction(fnSpecializationTimeoutContext, req.function)
@@ -138,6 +138,9 @@ func (deploy *NewDeploy) GetTypeName() fv1.ExecutorType {
}
func (deploy *NewDeploy) GetFuncSvc(ctx context.Context, fn *fv1.Function) (*fscache.FuncSvc, error) {
// TODO: client-go doesn't support to pass in context.
// Once it supports context, we should change the signature of method.
// https://github.com/kubernetes/kubernetes/issues/46503
return deploy.createFunction(fn, false)
}
+11 -3
View File
@@ -9,9 +9,9 @@ import (
"strings"
"time"
"go.uber.org/zap"
"github.com/pkg/errors"
"go.opencensus.io/plugin/ochttp"
"go.uber.org/zap"
"golang.org/x/net/context/ctxhttp"
ferror "github.com/fission/fission/pkg/error"
@@ -91,12 +91,20 @@ func sendRequest(logger *zap.Logger, ctx context.Context, httpClient *http.Clien
if err != nil {
logger.Error("error reading response body", zap.Error(err))
}
resp.Body.Close()
defer resp.Body.Close()
return body, err
}
err = ferror.MakeErrorFromHTTP(resp)
}
// skip retry and return directly due to context deadline exceeded
if err == context.DeadlineExceeded {
msg := "error specializing function pod, either increase the specialization timeout for function or check function pod log would help."
err = errors.Wrap(err, msg)
logger.Error(msg, zap.Error(err), zap.String("url", url))
return nil, err
}
if i < maxRetries-1 {
time.Sleep(50 * time.Duration(2*i) * time.Millisecond)
logger.Error("error specializing/fetching/uploading package, retrying", zap.Error(err), zap.String("url", url))
+132 -43
View File
@@ -362,32 +362,45 @@ func (opts *CreateSubCommand) run(input cli.Input) error {
}
func getInvokeStrategy(input cli.Input, existingInvokeStrategy *fv1.InvokeStrategy) (strategy *fv1.InvokeStrategy, err error) {
var fnExecutor, newFnExecutor fv1.ExecutorType
var es *fv1.ExecutionStrategy
if existingInvokeStrategy == nil {
es, err = getExecutionStrategy(input)
} else {
es, err = updateExecutionStrategy(input, &existingInvokeStrategy.ExecutionStrategy)
}
if err != nil {
return nil, err
}
return &fv1.InvokeStrategy{
ExecutionStrategy: *es,
StrategyType: fv1.StrategyTypeExecution,
}, nil
}
func getExecutionStrategy(input cli.Input) (strategy *fv1.ExecutionStrategy, err error) {
var fnExecutor fv1.ExecutorType
switch input.String(flagkey.FnExecutorType) {
case "":
fallthrough
case string(fv1.ExecutorTypePoolmgr):
newFnExecutor = fv1.ExecutorTypePoolmgr
fnExecutor = fv1.ExecutorTypePoolmgr
case string(fv1.ExecutorTypeNewdeploy):
newFnExecutor = fv1.ExecutorTypeNewdeploy
fnExecutor = fv1.ExecutorTypeNewdeploy
default:
return nil, errors.Errorf("executor type must be one of '%v' or '%v'", fv1.ExecutorTypePoolmgr, fv1.ExecutorTypeNewdeploy)
}
if existingInvokeStrategy != nil {
fnExecutor = existingInvokeStrategy.ExecutionStrategy.ExecutorType
specializationTimeout := fv1.DefaultSpecializationTimeOut
// override the executor type if user specified a new executor type
if input.IsSet(flagkey.FnExecutorType) {
fnExecutor = newFnExecutor
if input.IsSet(flagkey.FnSpecializationTimeout) {
specializationTimeout = input.Int(flagkey.FnSpecializationTimeout)
if specializationTimeout < fv1.DefaultSpecializationTimeOut {
return nil, errors.Errorf("%v must be greater than or equal to 120 seconds", flagkey.FnSpecializationTimeout)
}
} else {
fnExecutor = newFnExecutor
}
if input.IsSet(flagkey.FnSpecializationTimeout) && fnExecutor != fv1.ExecutorTypeNewdeploy {
return nil, errors.Errorf("%v flag is only applicable for newdeploy type of executor", flagkey.FnSpecializationTimeout)
}
if fnExecutor == fv1.ExecutorTypePoolmgr {
@@ -398,24 +411,102 @@ func getInvokeStrategy(input cli.Input, existingInvokeStrategy *fv1.InvokeStrate
if input.IsSet(flagkey.RuntimeMincpu) || input.IsSet(flagkey.RuntimeMaxcpu) || input.IsSet(flagkey.RuntimeMinmemory) || input.IsSet(flagkey.RuntimeMaxmemory) {
console.Warn("To limit CPU/Memory for function with executor type \"poolmgr\", please specify resources limits when creating environment")
}
strategy = &fv1.InvokeStrategy{
StrategyType: fv1.StrategyTypeExecution,
ExecutionStrategy: fv1.ExecutionStrategy{
ExecutorType: fv1.ExecutorTypePoolmgr,
},
strategy = &fv1.ExecutionStrategy{
ExecutorType: fv1.ExecutorTypePoolmgr,
SpecializationTimeout: specializationTimeout,
}
} else {
// set default value
targetCPU := DEFAULT_TARGET_CPU_PERCENTAGE
minScale := DEFAULT_MIN_SCALE
maxScale := minScale
specializationTimeout := fv1.DefaultSpecializationTimeOut
if input.IsSet(flagkey.RuntimeTargetcpu) {
targetCPU, err = getTargetCPU(input)
if err != nil {
return nil, err
}
}
if existingInvokeStrategy != nil && existingInvokeStrategy.ExecutionStrategy.ExecutorType == fv1.ExecutorTypeNewdeploy {
minScale = existingInvokeStrategy.ExecutionStrategy.MinScale
maxScale = existingInvokeStrategy.ExecutionStrategy.MaxScale
targetCPU = existingInvokeStrategy.ExecutionStrategy.TargetCPUPercent
specializationTimeout = existingInvokeStrategy.ExecutionStrategy.SpecializationTimeout
minScale := DEFAULT_MIN_SCALE
if input.IsSet(flagkey.ReplicasMinscale) {
minScale = input.Int(flagkey.ReplicasMinscale)
}
maxScale := minScale
if input.IsSet(flagkey.ReplicasMaxscale) {
maxScale = input.Int(flagkey.ReplicasMaxscale)
if maxScale <= 0 {
return nil, errors.Errorf("%v must be greater than 0", flagkey.ReplicasMaxscale)
}
}
if minScale > maxScale {
return nil, fmt.Errorf("minscale (%v) can not be greater than maxscale (%v)", minScale, maxScale)
}
// Right now a simple single case strategy implementation
// This will potentially get more sophisticated once we have more strategies in place
strategy = &fv1.ExecutionStrategy{
ExecutorType: fnExecutor,
MinScale: minScale,
MaxScale: maxScale,
TargetCPUPercent: targetCPU,
SpecializationTimeout: specializationTimeout,
}
}
return strategy, nil
}
func updateExecutionStrategy(input cli.Input, existingExecutionStrategy *fv1.ExecutionStrategy) (strategy *fv1.ExecutionStrategy, err error) {
fnExecutor := existingExecutionStrategy.ExecutorType
oldExecutor := existingExecutionStrategy.ExecutorType
if input.IsSet(flagkey.FnExecutorType) {
switch input.String(flagkey.FnExecutorType) {
case "":
fallthrough
case string(fv1.ExecutorTypePoolmgr):
fnExecutor = fv1.ExecutorTypePoolmgr
case string(fv1.ExecutorTypeNewdeploy):
fnExecutor = fv1.ExecutorTypeNewdeploy
default:
return nil, errors.Errorf("executor type must be one of '%v' or '%v'", fv1.ExecutorTypePoolmgr, fv1.ExecutorTypeNewdeploy)
}
}
specializationTimeout := existingExecutionStrategy.SpecializationTimeout
if input.IsSet(flagkey.FnSpecializationTimeout) {
specializationTimeout = input.Int(flagkey.FnSpecializationTimeout)
if specializationTimeout < fv1.DefaultSpecializationTimeOut {
return nil, errors.Errorf("%v must be greater than or equal to 120 seconds", flagkey.FnSpecializationTimeout)
}
} else {
if specializationTimeout < fv1.DefaultSpecializationTimeOut {
specializationTimeout = fv1.DefaultSpecializationTimeOut
}
}
if fnExecutor == fv1.ExecutorTypePoolmgr {
if input.IsSet(flagkey.RuntimeTargetcpu) || input.IsSet(flagkey.ReplicasMinscale) || input.IsSet(flagkey.ReplicasMaxscale) {
return nil, errors.New("to set target CPU or min/max scale for function, please specify \"--executortype newdeploy\"")
}
if input.IsSet(flagkey.RuntimeMincpu) || input.IsSet(flagkey.RuntimeMaxcpu) || input.IsSet(flagkey.RuntimeMinmemory) || input.IsSet(flagkey.RuntimeMaxmemory) {
console.Warn("To limit CPU/Memory for function with executor type \"poolmgr\", please specify resources limits when creating environment")
}
strategy = &fv1.ExecutionStrategy{
ExecutorType: fv1.ExecutorTypePoolmgr,
SpecializationTimeout: specializationTimeout,
}
} else {
targetCPU := existingExecutionStrategy.TargetCPUPercent
minScale := existingExecutionStrategy.MinScale
maxScale := existingExecutionStrategy.MaxScale
if fnExecutor != oldExecutor { // from poolmanager to newdeploy
targetCPU = DEFAULT_TARGET_CPU_PERCENTAGE
minScale = DEFAULT_MIN_SCALE
maxScale = minScale
}
if input.IsSet(flagkey.RuntimeTargetcpu) {
@@ -423,6 +514,10 @@ func getInvokeStrategy(input cli.Input, existingInvokeStrategy *fv1.InvokeStrate
if err != nil {
return nil, err
}
} else {
if targetCPU <= 0 || targetCPU > 100 {
targetCPU = DEFAULT_TARGET_CPU_PERCENTAGE
}
}
if input.IsSet(flagkey.ReplicasMinscale) {
@@ -434,12 +529,9 @@ func getInvokeStrategy(input cli.Input, existingInvokeStrategy *fv1.InvokeStrate
if maxScale <= 0 {
return nil, errors.Errorf("%v must be greater than 0", flagkey.ReplicasMaxscale)
}
}
if input.IsSet(flagkey.FnSpecializationTimeout) {
specializationTimeout = input.Int(flagkey.FnSpecializationTimeout)
if specializationTimeout < fv1.DefaultSpecializationTimeOut {
return nil, errors.Errorf("%v must be greater than or equal to 120 seconds", flagkey.FnSpecializationTimeout)
} else {
if maxScale <= 0 {
maxScale = 1
}
}
@@ -449,15 +541,12 @@ func getInvokeStrategy(input cli.Input, existingInvokeStrategy *fv1.InvokeStrate
// Right now a simple single case strategy implementation
// This will potentially get more sophisticated once we have more strategies in place
strategy = &fv1.InvokeStrategy{
StrategyType: fv1.StrategyTypeExecution,
ExecutionStrategy: fv1.ExecutionStrategy{
ExecutorType: fnExecutor,
MinScale: minScale,
MaxScale: maxScale,
TargetCPUPercent: targetCPU,
SpecializationTimeout: specializationTimeout,
},
strategy = &fv1.ExecutionStrategy{
ExecutorType: fnExecutor,
MinScale: minScale,
MaxScale: maxScale,
TargetCPUPercent: targetCPU,
SpecializationTimeout: specializationTimeout,
}
}
+73 -49
View File
@@ -29,37 +29,40 @@ import (
func TestGetInvokeStrategy(t *testing.T) {
cases := []struct {
name string
testArgs map[string]interface{}
existingInvokeStrategy *fv1.InvokeStrategy
expectedResult *fv1.InvokeStrategy
expectError bool
}{
{
// case: use default executor poolmgr
name: "use default executor poolmgr",
testArgs: map[string]interface{}{},
existingInvokeStrategy: nil,
expectedResult: &fv1.InvokeStrategy{
StrategyType: fv1.StrategyTypeExecution,
ExecutionStrategy: fv1.ExecutionStrategy{
ExecutorType: fv1.ExecutorTypePoolmgr,
ExecutorType: fv1.ExecutorTypePoolmgr,
SpecializationTimeout: 120,
},
},
expectError: false,
},
{
// case: executor type set to poolmgr
name: "executor type set to poolmgr",
testArgs: map[string]interface{}{flagkey.FnExecutorType: string(fv1.ExecutorTypePoolmgr)},
existingInvokeStrategy: nil,
expectedResult: &fv1.InvokeStrategy{
StrategyType: fv1.StrategyTypeExecution,
ExecutionStrategy: fv1.ExecutionStrategy{
ExecutorType: fv1.ExecutorTypePoolmgr,
ExecutorType: fv1.ExecutorTypePoolmgr,
SpecializationTimeout: 120,
},
},
expectError: false,
},
{
// case: executor type set to newdeploy
name: "executor type set to newdeploy",
testArgs: map[string]interface{}{flagkey.FnExecutorType: string(fv1.ExecutorTypeNewdeploy)},
existingInvokeStrategy: nil,
expectedResult: &fv1.InvokeStrategy{
@@ -75,7 +78,7 @@ func TestGetInvokeStrategy(t *testing.T) {
expectError: false,
},
{
// case: executor type change from poolmgr to newdeploy
name: "executor type change from poolmgr to newdeploy",
testArgs: map[string]interface{}{flagkey.FnExecutorType: string(fv1.ExecutorTypeNewdeploy)},
existingInvokeStrategy: &fv1.InvokeStrategy{
StrategyType: fv1.StrategyTypeExecution,
@@ -96,7 +99,7 @@ func TestGetInvokeStrategy(t *testing.T) {
expectError: false,
},
{
// case: executor type change from newdeploy to poolmgr
name: "executor type change from newdeploy to poolmgr",
testArgs: map[string]interface{}{flagkey.FnExecutorType: string(fv1.ExecutorTypePoolmgr)},
existingInvokeStrategy: &fv1.InvokeStrategy{
StrategyType: fv1.StrategyTypeExecution,
@@ -111,13 +114,14 @@ func TestGetInvokeStrategy(t *testing.T) {
expectedResult: &fv1.InvokeStrategy{
StrategyType: fv1.StrategyTypeExecution,
ExecutionStrategy: fv1.ExecutionStrategy{
ExecutorType: fv1.ExecutorTypePoolmgr,
ExecutorType: fv1.ExecutorTypePoolmgr,
SpecializationTimeout: fv1.DefaultSpecializationTimeOut,
},
},
expectError: false,
},
{
// case: minscale < maxscale
name: "minscale < maxscale",
testArgs: map[string]interface{}{
flagkey.FnExecutorType: string(fv1.ExecutorTypeNewdeploy),
flagkey.ReplicasMinscale: 2,
@@ -137,7 +141,7 @@ func TestGetInvokeStrategy(t *testing.T) {
expectError: false,
},
{
// case: minscale > maxscale
name: "minscale > maxscale",
testArgs: map[string]interface{}{
flagkey.FnExecutorType: string(fv1.ExecutorTypeNewdeploy),
flagkey.ReplicasMinscale: 5,
@@ -148,17 +152,26 @@ func TestGetInvokeStrategy(t *testing.T) {
expectError: true,
},
{
// case: maxscale not specified
name: "maxscale not specified",
testArgs: map[string]interface{}{
flagkey.FnExecutorType: string(fv1.ExecutorTypeNewdeploy),
flagkey.ReplicasMinscale: 5,
},
existingInvokeStrategy: nil,
expectedResult: nil,
expectError: true,
expectedResult: &fv1.InvokeStrategy{
StrategyType: fv1.StrategyTypeExecution,
ExecutionStrategy: fv1.ExecutionStrategy{
ExecutorType: fv1.ExecutorTypeNewdeploy,
MinScale: 5,
MaxScale: 5,
TargetCPUPercent: DEFAULT_TARGET_CPU_PERCENTAGE,
SpecializationTimeout: fv1.DefaultSpecializationTimeOut,
},
},
expectError: false,
},
{
// case: minscale not specified
name: "minscale not specified",
testArgs: map[string]interface{}{
flagkey.FnExecutorType: string(fv1.ExecutorTypeNewdeploy),
flagkey.ReplicasMaxscale: 3,
@@ -177,7 +190,7 @@ func TestGetInvokeStrategy(t *testing.T) {
expectError: false,
},
{
// case: maxscale set to 0
name: "maxscale set to 0",
testArgs: map[string]interface{}{
flagkey.FnExecutorType: string(fv1.ExecutorTypeNewdeploy),
flagkey.ReplicasMaxscale: 0,
@@ -187,7 +200,26 @@ func TestGetInvokeStrategy(t *testing.T) {
expectError: true,
},
{
// case: maxscale set to 9 when existing is 5
name: "update minscale with value larger than existing maxScale",
testArgs: map[string]interface{}{
flagkey.FnExecutorType: string(fv1.ExecutorTypeNewdeploy),
flagkey.ReplicasMinscale: 9,
},
existingInvokeStrategy: &fv1.InvokeStrategy{
StrategyType: fv1.StrategyTypeExecution,
ExecutionStrategy: fv1.ExecutionStrategy{
ExecutorType: fv1.ExecutorTypeNewdeploy,
MinScale: 2,
MaxScale: 5,
TargetCPUPercent: DEFAULT_TARGET_CPU_PERCENTAGE,
SpecializationTimeout: fv1.DefaultSpecializationTimeOut,
},
},
expectedResult: nil,
expectError: true,
},
{
name: "maxscale set to 9 when existing is 5",
testArgs: map[string]interface{}{
flagkey.FnExecutorType: string(fv1.ExecutorTypeNewdeploy),
flagkey.ReplicasMaxscale: 9,
@@ -215,7 +247,7 @@ func TestGetInvokeStrategy(t *testing.T) {
expectError: false,
},
{
// case: change nothing for existing strategy
name: "change nothing for existing strategy",
testArgs: map[string]interface{}{
flagkey.FnExecutorType: string(fv1.ExecutorTypeNewdeploy),
},
@@ -242,7 +274,7 @@ func TestGetInvokeStrategy(t *testing.T) {
expectError: false,
},
{
// case: set target cpu percentage
name: "set target cpu percentage",
testArgs: map[string]interface{}{
flagkey.FnExecutorType: string(fv1.ExecutorTypeNewdeploy),
flagkey.RuntimeTargetcpu: 50,
@@ -261,7 +293,7 @@ func TestGetInvokeStrategy(t *testing.T) {
expectError: false,
},
{
// case: change target cpu percentage
name: "change target cpu percentage",
testArgs: map[string]interface{}{
flagkey.FnExecutorType: string(fv1.ExecutorTypeNewdeploy),
flagkey.RuntimeTargetcpu: 20,
@@ -289,7 +321,7 @@ func TestGetInvokeStrategy(t *testing.T) {
expectError: false,
},
{
// case: change specializationtimeout
name: "change specializationtimeout",
testArgs: map[string]interface{}{
flagkey.FnExecutorType: string(fv1.ExecutorTypeNewdeploy),
flagkey.FnSpecializationTimeout: 200,
@@ -316,17 +348,7 @@ func TestGetInvokeStrategy(t *testing.T) {
expectError: false,
},
{
// case: specializationtimeout should not work for poolmgr
testArgs: map[string]interface{}{
flagkey.FnExecutorType: string(fv1.ExecutorTypePoolmgr),
flagkey.FnSpecializationTimeout: 10,
},
existingInvokeStrategy: nil,
expectedResult: nil,
expectError: true,
},
{
// case: specializationtimeout should not be less than 120
name: "specializationtimeout should not be less than 120",
testArgs: map[string]interface{}{
flagkey.FnExecutorType: string(fv1.ExecutorTypeNewdeploy),
flagkey.FnSpecializationTimeout: 90,
@@ -337,25 +359,27 @@ func TestGetInvokeStrategy(t *testing.T) {
},
}
for i, c := range cases {
fmt.Printf("=== Test Case %v ===\n", i)
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
flags := dummy.TestFlagSet()
flags := dummy.TestFlagSet()
for k, v := range c.testArgs {
flags.Set(k, v)
}
strategy, err := getInvokeStrategy(flags, c.existingInvokeStrategy)
if c.expectError {
assert.NotNil(t, err)
if err != nil {
fmt.Println(err)
for k, v := range c.testArgs {
flags.Set(k, v)
}
} else {
assert.Nil(t, err)
assert.NoError(t, strategy.Validate(), fmt.Sprintf("Failed at test case %v", i))
assert.Equal(t, *c.expectedResult, *strategy)
}
strategy, err := getInvokeStrategy(flags, c.existingInvokeStrategy)
if c.expectError {
assert.NotNil(t, err)
if err != nil {
fmt.Println(err)
}
} else {
assert.Nil(t, err)
if err == nil {
assert.NoError(t, strategy.Validate())
assert.Equal(t, *c.expectedResult, *strategy)
}
}
})
}
}
-14
View File
@@ -87,8 +87,6 @@ func (opts *UpdateSubCommand) complete(input cli.Input) error {
secretNames := input.StringSlice(flagkey.FnSecret)
cfgMapNames := input.StringSlice(flagkey.FnCfgMap)
specializationTimeout := input.Int(flagkey.FnSpecializationTimeout)
var secrets []fv1.SecretReference
var configMaps []fv1.ConfigMapReference
@@ -169,18 +167,6 @@ func (opts *UpdateSubCommand) complete(input cli.Input) error {
}
function.Spec.InvokeStrategy = *strategy
if input.IsSet(flagkey.FnSpecializationTimeout) {
if strategy.ExecutionStrategy.ExecutorType != fv1.ExecutorTypeNewdeploy {
return errors.Errorf("--%v flag is only applicable for newdeploy type of executor", flagkey.FnSpecializationTimeout)
}
if specializationTimeout < fv1.DefaultSpecializationTimeOut {
return errors.Errorf("--%v must be greater than or equal to 120 seconds", flagkey.FnSpecializationTimeout)
} else {
function.Spec.InvokeStrategy.ExecutionStrategy.SpecializationTimeout = specializationTimeout
}
}
resReqs, err := util.GetResourceReqs(input, &function.Spec.Resources)
if err != nil {
return err
@@ -20,7 +20,6 @@ pushd $(dirname $0)
fission spec apply
fission fn test --name $fn_p
fission fn test --name $fn_nd
hnd=$(kubectl -n $FUNCTION_NAMESPACE get deployment -l=functionName=$fn_nd -ojsonpath='{.items[0].spec.template.spec.hostname}')