From 8b0a201f6965e68c986598aad74aec191b67ea19 Mon Sep 17 00:00:00 2001 From: Ta-Ching Chen Date: Wed, 30 Jan 2019 22:14:14 +0800 Subject: [PATCH] Fix executor tries to create same name deployment (#1082) The root cause of the issue was introduced by PR https://github.com/fission/fission/pull/1009/files . To be short, even the CRD of environment was delete, it still takes time for executor (poolmgr) to destroy env pool. In our cases, the previous test creates an env and delete it when test finished, then the next one creates the same name env, but failed to create pool due to the deploy name conflict. So the executor selects the pod from the first created env pool. Then, executor starts to delete the env pool, and makes the pod state became Termination state. To fix this problem, a unique name of deployment will be returned after this PR to prevent the name conflict. --- executor/api.go | 1 - executor/executor.go | 64 +---- executor/fscache/functionServiceCache.go | 4 +- executor/newdeploy/newdeploy.go | 11 +- executor/newdeploy/newdeploymgr.go | 287 +++++++++-------------- executor/poolmgr/gp.go | 7 +- executor/poolmgr/gpm.go | 55 ++++- test/tests/test_backend_newdeploy.sh | 2 +- 8 files changed, 187 insertions(+), 244 deletions(-) diff --git a/executor/api.go b/executor/api.go index 8e10a6f5..80121e3b 100644 --- a/executor/api.go +++ b/executor/api.go @@ -89,7 +89,6 @@ func (executor *Executor) getServiceForFunction(m *metav1.ObjectMeta) (string, e if resp.err != nil { return "", resp.err } - executor.fsCache.IncreaseColdStarts(m.Name, string(m.UID)) return resp.funcSvc.Address, resp.err } diff --git a/executor/executor.go b/executor/executor.go index d4345705..7520900b 100644 --- a/executor/executor.go +++ b/executor/executor.go @@ -31,7 +31,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/fission/fission" - "github.com/fission/fission/cache" "github.com/fission/fission/crd" "github.com/fission/fission/executor/fscache" "github.com/fission/fission/executor/newdeploy" @@ -41,9 +40,9 @@ import ( type ( Executor struct { - gpm *poolmgr.GenericPoolManager - ndm *newdeploy.NewDeploy - functionEnv *cache.Cache + gpm *poolmgr.GenericPoolManager + ndm *newdeploy.NewDeploy + fissionClient *crd.FissionClient fsCache *fscache.FunctionServiceCache @@ -65,7 +64,6 @@ func MakeExecutor(gpm *poolmgr.GenericPoolManager, ndm *newdeploy.NewDeploy, fis executor := &Executor{ gpm: gpm, ndm: ndm, - functionEnv: cache.MakeCache(10*time.Second, 0), fissionClient: fissionClient, fsCache: fsCache, @@ -154,60 +152,24 @@ func (executor *Executor) createServiceForFunction(meta *metav1.ObjectMeta) (*fs case fission.ExecutorTypeNewdeploy: fsvc, fsvcErr = executor.ndm.GetFuncSvc(meta) default: - // from Func -> get Env - log.Printf("[%v] getting environment for function", meta.Name) - env, err := executor.getFunctionEnv(meta) - if err != nil { - return nil, err - } - - pool, err := executor.gpm.GetPool(env) - if err != nil { - return nil, err - } - // from GenericPool -> get one function container - // (this also adds to the cache) - log.Printf("[%v] getting function service from pool", meta.Name) - fsvc, fsvcErr = pool.GetFuncSvc(meta) + fsvc, fsvcErr = executor.gpm.GetFuncSvc(meta) } if fsvcErr != nil { fsvcErr = errors.Wrap(fsvcErr, fmt.Sprintf("[%v] Error creating service for function", meta.Name)) log.Print(fsvcErr) + } else if fsvc != nil { + _, err = executor.fsCache.Add(*fsvc) + if err != nil { + return nil, err + } } + executor.fsCache.IncreaseColdStarts(meta.Name, string(meta.UID)) + return fsvc, fsvcErr } -func (executor *Executor) getFunctionEnv(m *metav1.ObjectMeta) (*crd.Environment, error) { - var env *crd.Environment - - // Cached ? - result, err := executor.functionEnv.Get(crd.CacheKey(m)) - if err == nil { - env = result.(*crd.Environment) - return env, nil - } - - // Cache miss -- get func from controller - f, err := executor.fissionClient.Functions(m.Namespace).Get(m.Name) - if err != nil { - return nil, err - } - - // Get env from metadata - log.Printf("[%v] getting env", m) - env, err = executor.fissionClient.Environments(f.Spec.Environment.Namespace).Get(f.Spec.Environment.Name) - if err != nil { - return nil, err - } - - // cache for future lookups - executor.functionEnv.Set(crd.CacheKey(m), env) - - return env, nil -} - // isValidAddress invokes isValidService or isValidPod depending on the type of executor func (executor *Executor) isValidAddress(fsvc *fscache.FuncSvc) bool { if fsvc.Executor == fscache.NEWDEPLOY { @@ -255,11 +217,11 @@ func StartExecutor(fissionNamespace string, functionNamespace string, envBuilder gpm := poolmgr.MakeGenericPoolManager( fissionClient, kubernetesClient, - functionNamespace, fsCache, poolID) + functionNamespace, poolID) ndm := newdeploy.MakeNewDeploy( fissionClient, kubernetesClient, restClient, - functionNamespace, fsCache, poolID) + functionNamespace, poolID) api := MakeExecutor(gpm, ndm, fissionClient, fsCache) diff --git a/executor/fscache/functionServiceCache.go b/executor/fscache/functionServiceCache.go index 6d263fc3..25acc3cb 100644 --- a/executor/fscache/functionServiceCache.go +++ b/executor/fscache/functionServiceCache.go @@ -175,14 +175,14 @@ func (fsc *FunctionServiceCache) GetByFunctionUID(uid types.UID) (*FuncSvc, erro func (fsc *FunctionServiceCache) Add(fsvc FuncSvc) (*FuncSvc, error) { err, existing := fsc.byFunction.Set(crd.CacheKey(fsvc.Function), &fsvc) if err != nil { - if existing != nil { + if IsNameExistError(err) { f := existing.(*FuncSvc) err2 := fsc.TouchByAddress(f.Address) if err2 != nil { return nil, err2 } fCopy := *f - return &fCopy, err + return &fCopy, nil } return nil, err } diff --git a/executor/newdeploy/newdeploy.go b/executor/newdeploy/newdeploy.go index c5988520..34c174d3 100644 --- a/executor/newdeploy/newdeploy.go +++ b/executor/newdeploy/newdeploy.go @@ -127,9 +127,8 @@ func (deploy *NewDeploy) setupRBACObjs(deployNamespace string, fn *crd.Function) return nil } -func (deploy *NewDeploy) getDeployment(fn *crd.Function) (*v1beta1.Deployment, error) { - deployName := deploy.getObjName(fn) - return deploy.kubernetesClient.ExtensionsV1beta1().Deployments(fn.Metadata.Namespace).Get(deployName, metav1.GetOptions{}) +func (deploy *NewDeploy) getDeployment(ns, name string) (*v1beta1.Deployment, error) { + return deploy.kubernetesClient.ExtensionsV1beta1().Deployments(ns).Get(name, metav1.GetOptions{}) } func (deploy *NewDeploy) updateDeployment(deployment *v1beta1.Deployment, ns string) error { @@ -428,9 +427,8 @@ func (deploy *NewDeploy) createOrGetHpa(hpaName string, execStrategy *fission.Ex } -func (deploy *NewDeploy) getHpa(ns string, fn *crd.Function) (*asv1.HorizontalPodAutoscaler, error) { - hpaName := deploy.getObjName(fn) - return deploy.kubernetesClient.AutoscalingV1().HorizontalPodAutoscalers(ns).Get(hpaName, metav1.GetOptions{}) +func (deploy *NewDeploy) getHpa(ns, name string) (*asv1.HorizontalPodAutoscaler, error) { + return deploy.kubernetesClient.AutoscalingV1().HorizontalPodAutoscalers(ns).Get(name, metav1.GetOptions{}) } func (deploy *NewDeploy) updateHpa(hpa *asv1.HorizontalPodAutoscaler) error { @@ -510,6 +508,7 @@ func (deploy *NewDeploy) waitForDeploy(depl *v1beta1.Deployment, replicas int32) return nil, errors.New("failed to create deployment within timeout window") } +// cleanupNewdeploy cleans all kubernetes objects related to function func (deploy *NewDeploy) cleanupNewdeploy(ns string, name string) error { var multierr *multierror.Error diff --git a/executor/newdeploy/newdeploymgr.go b/executor/newdeploy/newdeploymgr.go index 59863abb..bace001e 100644 --- a/executor/newdeploy/newdeploymgr.go +++ b/executor/newdeploy/newdeploymgr.go @@ -25,6 +25,8 @@ import ( "strings" "time" + "github.com/dchest/uniuri" + "github.com/fission/fission/throttler" multierror "github.com/hashicorp/go-multierror" "github.com/pkg/errors" apiv1 "k8s.io/api/core/v1" @@ -43,8 +45,6 @@ import ( ) type ( - requestType int - NewDeploy struct { kubernetesClient *kubernetes.Clientset fissionClient *crd.FissionClient @@ -60,33 +60,14 @@ type ( sharedCfgMapPath string useIstio bool - fsCache *fscache.FunctionServiceCache // cache funcSvc's by function, address and podname - requestChannel chan *fnRequest + fsCache *fscache.FunctionServiceCache // cache funcSvc's by function, address and pod name - functions []crd.Function + throttler *throttler.Throttler funcStore k8sCache.Store funcController k8sCache.Controller idlePodReapTime time.Duration } - - fnRequest struct { - reqType requestType - fn *crd.Function - responseChannel chan *fnResponse - firstcreate bool - } - - fnResponse struct { - error - fSvc *fscache.FuncSvc - } -) - -const ( - FnCreate requestType = iota - FnUpdate - FnDelete ) func MakeNewDeploy( @@ -94,7 +75,6 @@ func MakeNewDeploy( kubernetesClient *kubernetes.Clientset, crdClient *rest.RESTClient, namespace string, - fsCache *fscache.FunctionServiceCache, instanceID string, ) *NewDeploy { @@ -104,10 +84,6 @@ func MakeNewDeploy( if len(fetcherImg) == 0 { fetcherImg = "fission/fetcher" } - fetcherImagePullPolicy := os.Getenv("FETCHER_IMAGE_PULL_POLICY") - if len(fetcherImagePullPolicy) == 0 { - fetcherImagePullPolicy = "IfNotPresent" - } enableIstio := false if len(os.Getenv("ENABLE_ISTIO")) > 0 { @@ -125,21 +101,20 @@ func MakeNewDeploy( instanceID: instanceID, namespace: namespace, - fsCache: fsCache, + fsCache: fscache.MakeFunctionServiceCache(), + throttler: throttler.MakeThrottler(1 * time.Minute), - fetcherImg: fetcherImg, - sharedMountPath: "/userfunc", - sharedSecretPath: "/secrets", - sharedCfgMapPath: "/configs", - useIstio: enableIstio, + fetcherImg: fetcherImg, + fetcherImagePullPolicy: fission.GetImagePullPolicy(os.Getenv("FETCHER_IMAGE_PULL_POLICY")), + runtimeImagePullPolicy: fission.GetImagePullPolicy(os.Getenv("RUNTIME_IMAGE_PULL_POLICY")), + sharedMountPath: "/userfunc", + sharedSecretPath: "/secrets", + sharedCfgMapPath: "/configs", + useIstio: enableIstio, - requestChannel: make(chan *fnRequest), idlePodReapTime: 2 * time.Minute, } - nd.runtimeImagePullPolicy = fission.GetImagePullPolicy(os.Getenv("RUNTIME_IMAGE_PULL_POLICY")) - nd.fetcherImagePullPolicy = fission.GetImagePullPolicy(os.Getenv("FETCHER_IMAGE_PULL_POLICY")) - if nd.crdClient != nil { fnStore, fnController := nd.initFuncController() nd.funcStore = fnStore @@ -150,7 +125,7 @@ func MakeNewDeploy( } func (deploy *NewDeploy) Run(ctx context.Context) { - go deploy.service() + //go deploy.service() go deploy.funcController.Run(ctx.Done()) go deploy.idleObjectReaper() } @@ -161,128 +136,85 @@ func (deploy *NewDeploy) initFuncController() (k8sCache.Store, k8sCache.Controll store, controller := k8sCache.NewInformer(listWatch, &crd.Function{}, resyncPeriod, k8sCache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { fn := obj.(*crd.Function) - deploy.createFunction(fn, true) + _, err := deploy.createFunction(fn, true) + if err != nil { + log.Printf("Error eager creating function: %v", err) + } }, DeleteFunc: func(obj interface{}) { fn := obj.(*crd.Function) - deploy.deleteFunction(fn) + err := deploy.deleteFunction(fn) + if err != nil { + log.Printf("Error deleing the function: %v", err) + } }, UpdateFunc: func(oldObj interface{}, newObj interface{}) { oldFn := oldObj.(*crd.Function) newFn := newObj.(*crd.Function) - deploy.fnUpdate(oldFn, newFn) + err := deploy.updateFunction(oldFn, newFn) + if err != nil { + log.Printf("Error deleing the function: %v", err) + } }, }) return store, controller } -func (deploy *NewDeploy) service() { - for { - req := <-deploy.requestChannel - switch req.reqType { - case FnCreate: - fsvc, err := deploy.fnCreate(req.fn, req.firstcreate) - req.responseChannel <- &fnResponse{ - error: err, - fSvc: fsvc, - } - continue - case FnDelete: - _, err := deploy.fnDelete(req.fn) - req.responseChannel <- &fnResponse{ - error: err, - fSvc: nil, - } - continue - // Update needs two inputs and will be called directly by controller - } - } -} - func (deploy *NewDeploy) GetFuncSvc(metadata *metav1.ObjectMeta) (*fscache.FuncSvc, error) { - c := make(chan *fnResponse) fn, err := deploy.fissionClient.Functions(metadata.Namespace).Get(metadata.Name) if err != nil { return nil, err } - - deploy.requestChannel <- &fnRequest{ - fn: fn, - reqType: FnCreate, - responseChannel: c, - firstcreate: false, - } - - resp := <-c - if resp.error != nil { - return nil, resp.error - } - return resp.fSvc, nil + return deploy.createFunction(fn, false) } -func (deploy *NewDeploy) createFunction(fn *crd.Function, firstcreate bool) { +func (deploy *NewDeploy) createFunction(fn *crd.Function, firstcreate bool) (*fscache.FuncSvc, error) { if fn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType != fission.ExecutorTypeNewdeploy { - return + return nil, nil } - // Eager creation of function if minScale is greater than 0 - log.Printf("Eagerly creating newDeploy objects for function") - c := make(chan *fnResponse) - deploy.requestChannel <- &fnRequest{ - fn: fn, - reqType: FnCreate, - responseChannel: c, - firstcreate: firstcreate, - } - resp := <-c - if resp.error != nil { - log.Printf("Error eager creating function: %v", resp.error) + fsvcObj, err := deploy.throttler.RunOnce(string(fn.Metadata.UID), func(ableToCreate bool) (interface{}, error) { + if ableToCreate { + return deploy.fnCreate(fn, firstcreate) + } + return deploy.fsCache.GetByFunctionUID(fn.Metadata.UID) + }) + + fsvc, ok := fsvcObj.(*fscache.FuncSvc) + if !ok { + log.Panic("Receive unknown object, expect pointer of function service object") } + + return fsvc, err } -func (deploy *NewDeploy) updateFunction(fn *crd.Function) { - c := make(chan *fnResponse) - deploy.requestChannel <- &fnRequest{ - fn: fn, - reqType: FnUpdate, - responseChannel: c, +func (deploy *NewDeploy) deleteFunction(fn *crd.Function) error { + if fn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType != fission.ExecutorTypeNewdeploy { + return nil } - resp := <-c - if resp.error != nil { - log.Printf("Error eager updating function: %v", resp.error) - } -} - -func (deploy *NewDeploy) deleteFunction(fn *crd.Function) { - if fn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType == fission.ExecutorTypeNewdeploy { - c := make(chan *fnResponse) - deploy.requestChannel <- &fnRequest{ - fn: fn, - reqType: FnDelete, - responseChannel: c, - } - resp := <-c - if resp.error != nil { - log.Printf("Error deleing the function: %v", resp.error) - } + err := deploy.fnDelete(fn) + if err != nil { + err = errors.Wrap(err, fmt.Sprintf("error deleting kubernetes objects of function %v", fn.Metadata)) } + return err } func (deploy *NewDeploy) fnCreate(fn *crd.Function, firstcreate bool) (*fscache.FuncSvc, error) { - fsvc, err := deploy.fsCache.GetByFunction(&fn.Metadata) - if err == nil { - return fsvc, err - } - env, err := deploy.fissionClient. Environments(fn.Spec.Environment.Namespace). Get(fn.Spec.Environment.Name) if err != nil { - return fsvc, err + return nil, err } objName := deploy.getObjName(fn) - + if !firstcreate { + // retrieve back the previous obj name for later use. + fsvc, err := deploy.fsCache.GetByFunctionUID(fn.Metadata.UID) + if err == nil { + objName = fsvc.Name + } + } deployLabels := deploy.getDeployLabels(fn, env) // to support backward compatibility, if the function was created in default ns, we fall back to creating the @@ -301,25 +233,25 @@ func (deploy *NewDeploy) fnCreate(fn *crd.Function, firstcreate bool) (*fscache. if err != nil { log.Printf("Error creating the service %v: %v", objName, err) go deploy.cleanupNewdeploy(ns, objName) - return fsvc, errors.Wrap(err, fmt.Sprintf("error creating service %v", objName)) + return nil, errors.Wrap(err, fmt.Sprintf("error creating service %v", objName)) } svcAddress := fmt.Sprintf("%v.%v", svc.Name, svc.Namespace) depl, err := deploy.createOrGetDeployment(fn, env, objName, deployLabels, ns, firstcreate) if err != nil { log.Printf("Error creating the deployment %v: %v", objName, err) go deploy.cleanupNewdeploy(ns, objName) - return fsvc, errors.Wrap(err, fmt.Sprintf("error creating deployment %v", objName)) + return nil, errors.Wrap(err, fmt.Sprintf("error creating deployment %v", objName)) } hpa, err := deploy.createOrGetHpa(objName, &fn.Spec.InvokeStrategy.ExecutionStrategy, depl) if err != nil { go deploy.cleanupNewdeploy(ns, objName) - return fsvc, errors.Wrap(err, fmt.Sprintf("error creating the HPA %v:", objName)) + return nil, errors.Wrap(err, fmt.Sprintf("error creating the HPA %v:", objName)) } kubeObjRefs := []apiv1.ObjectReference{ { - //obj.TypeMeta.Kind does not work hence this, needs investigationa and a fix + //obj.TypeMeta.Kind does not work hence this, needs investigation and a fix Kind: "deployment", Name: depl.ObjectMeta.Name, APIVersion: depl.TypeMeta.APIVersion, @@ -345,7 +277,7 @@ func (deploy *NewDeploy) fnCreate(fn *crd.Function, firstcreate bool) (*fscache. }, } - fsvc = &fscache.FuncSvc{ + fsvc := &fscache.FuncSvc{ Name: objName, Function: &fn.Metadata, Environment: env, @@ -362,42 +294,48 @@ func (deploy *NewDeploy) fnCreate(fn *crd.Function, firstcreate bool) (*fscache. return fsvc, nil } -func (deploy *NewDeploy) fnUpdate(oldFn *crd.Function, newFn *crd.Function) { +func (deploy *NewDeploy) updateFunction(oldFn *crd.Function, newFn *crd.Function) error { if oldFn.Metadata.ResourceVersion == newFn.Metadata.ResourceVersion { - return + return nil } // Ignoring updates to functions which are not of NewDeployment type if newFn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType != fission.ExecutorTypeNewdeploy && oldFn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType != fission.ExecutorTypeNewdeploy { - return + return nil } + // Executor type is no longer New Deployment + if newFn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType != fission.ExecutorTypeNewdeploy && + oldFn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType == fission.ExecutorTypeNewdeploy { + log.Printf("function does not use new deployment executor anymore, deleting resources: %v", newFn) + // IMP - pass the oldFn, as the new/modified function is not in cache + return deploy.deleteFunction(oldFn) + } + + // Executor type changed to New Deployment from something else + if oldFn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType != fission.ExecutorTypeNewdeploy && + newFn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType == fission.ExecutorTypeNewdeploy { + log.Printf("function type changed to new deployment, creating resources: %v %v", oldFn.Metadata, newFn.Metadata) + _, err := deploy.createFunction(newFn, true) + if err != nil { + updateStatus(oldFn, err, "error changing the function's type to newdeploy") + } + return err + } + + fsvc, err := deploy.fsCache.GetByFunctionUID(newFn.Metadata.UID) + if err != nil { + err = errors.Wrap(err, fmt.Sprintf("error updating function due to unable to find function service cache: %v", oldFn)) + return err + } + + fnObjName := fsvc.Name deployChanged := false if oldFn.Spec.InvokeStrategy != newFn.Spec.InvokeStrategy { - // Executor type is no longer New Deployment - if newFn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType != fission.ExecutorTypeNewdeploy && - oldFn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType == fission.ExecutorTypeNewdeploy { - log.Printf("function does not use new deployment executor anymore, deleting resources: %v", newFn) - // IMP - pass the oldFn, as the new/modified function is not in cache - deploy.fnDelete(oldFn) - return - } - - // Executor type changed to New Deployment from something else - if oldFn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType != fission.ExecutorTypeNewdeploy && - newFn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType == fission.ExecutorTypeNewdeploy { - log.Printf("function type changed to new deployment, creating resources: %v", newFn) - _, err := deploy.fnCreate(newFn, true) - if err != nil { - updateStatus(oldFn, err, "error changing the function's type to newdeploy") - } - return - } - // to support backward compatibility, if the function was created in default ns, we fall back to creating the // deployment of the function in fission-function ns, so cleaning up resources there ns := deploy.namespace @@ -405,10 +343,10 @@ func (deploy *NewDeploy) fnUpdate(oldFn *crd.Function, newFn *crd.Function) { ns = newFn.Metadata.Namespace } - hpa, err := deploy.getHpa(ns, newFn) + hpa, err := deploy.getHpa(ns, fnObjName) if err != nil { updateStatus(oldFn, err, "error getting HPA while updating function") - return + return err } hpaChanged := false @@ -434,7 +372,7 @@ func (deploy *NewDeploy) fnUpdate(oldFn *crd.Function, newFn *crd.Function) { err := deploy.updateHpa(hpa) if err != nil { updateStatus(oldFn, err, "error updating HPA while updating function") - return + return err } } } @@ -472,15 +410,15 @@ func (deploy *NewDeploy) fnUpdate(oldFn *crd.Function, newFn *crd.Function) { Get(newFn.Spec.Environment.Name) if err != nil { updateStatus(oldFn, err, "failed to get environment while updating function") - return + return err } - deployName := deploy.getObjName(oldFn) + deployLabels := deploy.getDeployLabels(oldFn, env) - log.Printf("updating %v deployment due to function %v update", deployName, newFn.Metadata.Name) - newDeployment, err := deploy.getDeploymentSpec(newFn, env, deployName, deployLabels) + log.Printf("updating %v deployment due to function %v update", fnObjName, newFn.Metadata.Name) + newDeployment, err := deploy.getDeploymentSpec(newFn, env, fnObjName, deployLabels) if err != nil { updateStatus(oldFn, err, "failed to get new deployment spec while updating function") - return + return err } // to support backward compatibility, if the function was created in default ns, we fall back to creating the @@ -493,13 +431,14 @@ func (deploy *NewDeploy) fnUpdate(oldFn *crd.Function, newFn *crd.Function) { err = deploy.updateDeployment(newDeployment, ns) if err != nil { updateStatus(oldFn, err, "failed to update deployment while updating function") - return + return err } } + + return nil } -func (deploy *NewDeploy) fnDelete(fn *crd.Function) (*fscache.FuncSvc, error) { - +func (deploy *NewDeploy) fnDelete(fn *crd.Function) error { var multierr *multierror.Error // GetByFunction uses resource version as part of cache key, however, @@ -509,16 +448,17 @@ func (deploy *NewDeploy) fnDelete(fn *crd.Function) (*fscache.FuncSvc, error) { // fsvc entry. fsvc, err := deploy.fsCache.GetByFunctionUID(fn.Metadata.UID) if err != nil { - log.Printf("fsvc not found in cache: %v", fn.Metadata) - return nil, err + err = errors.Wrap(err, fmt.Sprintf("fsvc not found in cache: %v", fn.Metadata)) + return err } + objName := fsvc.Name + _, err = deploy.fsCache.DeleteOld(fsvc, time.Second*0) if err != nil { - log.Printf("Error deleting the function from cache: %v", fsvc) - multierror.Append(multierr, err) + multierr = multierror.Append(multierr, + errors.Wrap(err, fmt.Sprintf("error deleting the function from cache"))) } - objName := fsvc.Name // to support backward compatibility, if the function was created in default ns, we fall back to creating the // deployment of the function in fission-function ns, so cleaning up resources there @@ -528,17 +468,14 @@ func (deploy *NewDeploy) fnDelete(fn *crd.Function) (*fscache.FuncSvc, error) { } err = deploy.cleanupNewdeploy(ns, objName) - multierror.Append(multierr, err) + multierr = multierror.Append(multierr, err) - return nil, multierr.ErrorOrNil() + return multierr.ErrorOrNil() } +// getObjName returns a unique name for kubernetes objects of function func (deploy *NewDeploy) getObjName(fn *crd.Function) string { - // Use executor type as delimiter between function name and namespace to prevent deployment name conflict. - // For example: - // 1. fn-name: a-b fn-namespace: c => a-b-newdeploy-c - // 2. fn-name: a fn-namespace: b-c => a-newdeploy-b-c - return strings.ToLower(fmt.Sprintf("%v-newdeploy-%v", fn.Metadata.Name, fn.Metadata.Namespace)) + return strings.ToLower(fmt.Sprintf("newdeploy-%v-%v-%v", fn.Metadata.Name, fn.Metadata.Namespace, uniuri.NewLen(8))) } func (deploy *NewDeploy) getDeployLabels(fn *crd.Function, env *crd.Environment) map[string]string { diff --git a/executor/poolmgr/gp.go b/executor/poolmgr/gp.go index 34d42239..6563adec 100644 --- a/executor/poolmgr/gp.go +++ b/executor/poolmgr/gp.go @@ -362,12 +362,9 @@ func (gp *GenericPool) specializePod(pod *apiv1.Pod, metadata *metav1.ObjectMeta return nil } +// getPoolName returns a unique name of an environment func (gp *GenericPool) getPoolName() string { - // Use executor type as delimiter between function name and namespace to prevent deployment name conflict. - // For example: - // 1. fn-name: a-b fn-namespace: c => a-b-poolmgr-c - // 2. fn-name: a fn-namespace: b-c => a-poolmgr-b-c - return strings.ToLower(fmt.Sprintf("%v-poolmgr-%v", gp.env.Metadata.Name, gp.env.Metadata.Namespace)) + return strings.ToLower(fmt.Sprintf("poolmgr-%v-%v-%v", gp.env.Metadata.Name, gp.env.Metadata.Namespace, uniuri.NewLen(8))) } // A pool is a deployment of generic containers for an env. This diff --git a/executor/poolmgr/gpm.go b/executor/poolmgr/gpm.go index 0e1ccca8..e197398c 100644 --- a/executor/poolmgr/gpm.go +++ b/executor/poolmgr/gpm.go @@ -30,6 +30,7 @@ import ( k8sCache "k8s.io/client-go/tools/cache" "github.com/fission/fission" + "github.com/fission/fission/cache" "github.com/fission/fission/crd" "github.com/fission/fission/executor/fscache" "github.com/fission/fission/executor/reaper" @@ -49,6 +50,7 @@ type ( namespace string fissionClient *crd.FissionClient + functionEnv *cache.Cache fsCache *fscache.FunctionServiceCache instanceId string requestChannel chan *request @@ -77,7 +79,6 @@ func MakeGenericPoolManager( fissionClient *crd.FissionClient, kubernetesClient *kubernetes.Clientset, functionNamespace string, - fsCache *fscache.FunctionServiceCache, instanceId string) *GenericPoolManager { gpm := &GenericPoolManager{ @@ -85,7 +86,8 @@ func MakeGenericPoolManager( kubernetesClient: kubernetesClient, namespace: functionNamespace, fissionClient: fissionClient, - fsCache: fsCache, + functionEnv: cache.MakeCache(10*time.Second, 0), + fsCache: fscache.MakeFunctionServiceCache(), instanceId: instanceId, requestChannel: make(chan *request), idlePodReapTime: 2 * time.Minute, @@ -157,7 +159,7 @@ func (gpm *GenericPoolManager) service() { if !ok || poolsize == 0 { // Env no longer exists or pool size changed to zero - log.Printf("Destroying generic pool for environment [%v]", key) + log.Printf("Destroying generic pool for environment %v", pool.env.Metadata) delete(gpm.pools, key) // and delete the pool asynchronously. @@ -187,6 +189,53 @@ func (gpm *GenericPoolManager) CleanupPools(envs []crd.Environment) { } } +func (gpm *GenericPoolManager) GetFuncSvc(metadata *metav1.ObjectMeta) (*fscache.FuncSvc, error) { + // from Func -> get Env + log.Printf("[%v] getting environment for function", metadata.Name) + env, err := gpm.getFunctionEnv(metadata) + if err != nil { + return nil, err + } + + pool, err := gpm.GetPool(env) + if err != nil { + return nil, err + } + // from GenericPool -> get one function container + // (this also adds to the cache) + log.Printf("[%v] getting function service from pool", metadata.Name) + return pool.GetFuncSvc(metadata) +} + +func (gpm *GenericPoolManager) getFunctionEnv(m *metav1.ObjectMeta) (*crd.Environment, error) { + var env *crd.Environment + + // Cached ? + result, err := gpm.functionEnv.Get(crd.CacheKey(m)) + if err == nil { + env = result.(*crd.Environment) + return env, nil + } + + // Cache miss -- get func from controller + f, err := gpm.fissionClient.Functions(m.Namespace).Get(m.Name) + if err != nil { + return nil, err + } + + // Get env from metadata + log.Printf("[%v] getting env", m) + env, err = gpm.fissionClient.Environments(f.Spec.Environment.Namespace).Get(f.Spec.Environment.Name) + if err != nil { + return nil, err + } + + // cache for future lookups + gpm.functionEnv.Set(crd.CacheKey(m), env) + + return env, nil +} + func (gpm *GenericPoolManager) eagerPoolCreator() { pollSleep := time.Duration(2 * time.Second) for { diff --git a/test/tests/test_backend_newdeploy.sh b/test/tests/test_backend_newdeploy.sh index 97bae6eb..981692d3 100755 --- a/test/tests/test_backend_newdeploy.sh +++ b/test/tests/test_backend_newdeploy.sh @@ -56,7 +56,7 @@ log "Waiting for router & newdeploy deployment creation" sleep 5 log "Doing an HTTP GET on the function's route" -response1=$(curl http://$FISSION_ROUTER/$fn0) +response1=$(curl http://$FISSION_ROUTER/$fn1) log "Checking for valid response" echo $response1 | grep -i hello