From 9eac1512fb4122d53f33bc3cfc245afcdc9772cb Mon Sep 17 00:00:00 2001 From: Ta-Ching Chen Date: Tue, 28 Jan 2020 00:58:37 +0800 Subject: [PATCH] Fix poolmanager wrongly delete env pool (#1511) The root cause of the problem is that eagerPoolCreator tries to create the deployment when the poolmanager is trying to delete it. To avoid this, start eager pool creator after executor starts serving requests. --- pkg/executor/executortype/newdeploy/newdeploy.go | 3 +++ pkg/executor/executortype/poolmgr/gp.go | 2 ++ pkg/executor/executortype/poolmgr/gpm.go | 4 +++- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/executor/executortype/newdeploy/newdeploy.go b/pkg/executor/executortype/newdeploy/newdeploy.go index 1bf2d409..4f669581 100644 --- a/pkg/executor/executortype/newdeploy/newdeploy.go +++ b/pkg/executor/executortype/newdeploy/newdeploy.go @@ -70,6 +70,9 @@ func (deploy *NewDeploy) createOrGetDeployment(fn *fv1.Function, env *fv1.Enviro existingDepl.Spec.Template.Spec.Containers = deployment.Spec.Template.Spec.Containers existingDepl.Spec.Template.Spec.ServiceAccountName = deployment.Spec.Template.Spec.ServiceAccountName existingDepl.Spec.Template.Spec.TerminationGracePeriodSeconds = deployment.Spec.Template.Spec.TerminationGracePeriodSeconds + + // Update with the latest deployment spec. Kubernetes will trigger + // rolling update if spec is different from the one in the cluster. existingDepl, err = deploy.kubernetesClient.AppsV1().Deployments(deployNamespace).Update(existingDepl) if err != nil { deploy.logger.Warn("error adopting deploy", zap.Error(err), diff --git a/pkg/executor/executortype/poolmgr/gp.go b/pkg/executor/executortype/poolmgr/gp.go index b8d51b11..23debafd 100644 --- a/pkg/executor/executortype/poolmgr/gp.go +++ b/pkg/executor/executortype/poolmgr/gp.go @@ -483,6 +483,8 @@ func (gp *GenericPool) createPool() error { if err == nil { if depl.Annotations[fv1.EXECUTOR_INSTANCEID_LABEL] != gp.instanceId { deployment.Annotations[fv1.EXECUTOR_INSTANCEID_LABEL] = gp.instanceId + // Update with the latest deployment spec. Kubernetes will trigger + // rolling update if spec is different from the one in the cluster. depl, err = gp.kubernetesClient.AppsV1().Deployments(gp.namespace).Update(deployment) } gp.deployment = depl diff --git a/pkg/executor/executortype/poolmgr/gpm.go b/pkg/executor/executortype/poolmgr/gpm.go index b8fa8fda..42cf9499 100644 --- a/pkg/executor/executortype/poolmgr/gpm.go +++ b/pkg/executor/executortype/poolmgr/gpm.go @@ -116,7 +116,6 @@ func MakeGenericPoolManager( } go gpm.service() - go gpm.eagerPoolCreator() if len(os.Getenv("ENABLE_ISTIO")) > 0 { istio, err := strconv.ParseBool(os.Getenv("ENABLE_ISTIO")) @@ -135,6 +134,9 @@ func MakeGenericPoolManager( } func (gpm *GenericPoolManager) Run(ctx context.Context) { + // eagerPoolCreator must run after CleanupOldExecutorObjects. + // Otherwise, the poolmanager may wrongly delete the deployment. + go gpm.eagerPoolCreator() go gpm.funcController.Run(ctx.Done()) go gpm.pkgController.Run(ctx.Done()) go gpm.idleObjectReaper()