From 1e70f828da46fdbdcfba77fe5accdff980661c4e 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 1bf2d4097..4f669581d 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 b8d51b113..23debafd8 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 b8fa8fdad..42cf9499e 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()