From 19c7616a4b9e34e076df24b95027d341fdb6c318 Mon Sep 17 00:00:00 2001 From: Sanket Sudake Date: Mon, 19 Jul 2021 15:43:10 +0530 Subject: [PATCH] Newdeploy/container function service names should fit in 63 characters (#2117) * Add error message on deployment provision failure Signed-off-by: Sanket Sudake * Add 63 characters limit for objects created via newdeploy/container Signed-off-by: Sanket Sudake --- .../executortype/container/containermgr.go | 11 ++++++++- .../executortype/container/deployment.go | 8 ++++++- .../executortype/newdeploy/newdeploy.go | 10 ++++++-- .../executortype/newdeploy/newdeploymgr.go | 24 +++++++++++++++---- 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/pkg/executor/executortype/container/containermgr.go b/pkg/executor/executortype/container/containermgr.go index 1ac1c54c..1c896894 100644 --- a/pkg/executor/executortype/container/containermgr.go +++ b/pkg/executor/executortype/container/containermgr.go @@ -672,7 +672,16 @@ func (caaf *Container) fnDelete(fn *fv1.Function) error { func (caaf *Container) getObjName(fn *fv1.Function) string { // use meta uuid of function, this ensure we always get the same name for the same function. uid := fn.ObjectMeta.UID[len(fn.ObjectMeta.UID)-17:] - return strings.ToLower(fmt.Sprintf("Container-%v-%v-%v", fn.ObjectMeta.Name, fn.ObjectMeta.Namespace, uid)) + var functionMetadata string + if len(fn.ObjectMeta.Name)+len(fn.ObjectMeta.Namespace) < 35 { + functionMetadata = fn.ObjectMeta.Name + "-" + fn.ObjectMeta.Namespace + } else { + functionMetadata = fn.ObjectMeta.Name[:17] + "-" + fn.ObjectMeta.Namespace[:17] + } + // contructed name should be 63 characters long, as it is a valid k8s name + // functionMetadata should be 35 characters long, as we take 17 characters from functionUid + // with newdeploy 10 character prefix + return strings.ToLower(fmt.Sprintf("container-%s-%s", functionMetadata, uid)) } func (caaf *Container) getDeployLabels(fnMeta metav1.ObjectMeta) map[string]string { diff --git a/pkg/executor/executortype/container/deployment.go b/pkg/executor/executortype/container/deployment.go index 911170da..062d15f4 100644 --- a/pkg/executor/executortype/container/deployment.go +++ b/pkg/executor/executortype/container/deployment.go @@ -123,7 +123,9 @@ func (cn *Container) deleteDeployment(ns string, name string) error { }) } -func (cn *Container) waitForDeploy(depl *appsv1.Deployment, replicas int32, specializationTimeout int) (*appsv1.Deployment, error) { +func (cn *Container) waitForDeploy(depl *appsv1.Deployment, replicas int32, specializationTimeout int) (latestDepl *appsv1.Deployment, err error) { + oldStatus := depl.Status + // if no specializationTimeout is set, use default value if specializationTimeout < fv1.DefaultSpecializationTimeOut { specializationTimeout = fv1.DefaultSpecializationTimeOut @@ -143,6 +145,10 @@ func (cn *Container) waitForDeploy(depl *appsv1.Deployment, replicas int32, spec time.Sleep(time.Second) } + cn.logger.Error("Deployment provision failed within timeout window", + zap.String("name", latestDepl.ObjectMeta.Name), zap.Any("old_status", oldStatus), + zap.Any("current_status", latestDepl.Status), zap.Int("timeout", specializationTimeout)) + // this error appears in the executor pod logs timeoutError := fmt.Errorf("failed to create deployment within the timeout window of %d seconds", specializationTimeout) return nil, timeoutError diff --git a/pkg/executor/executortype/newdeploy/newdeploy.go b/pkg/executor/executortype/newdeploy/newdeploy.go index c4a222c6..e4574987 100644 --- a/pkg/executor/executortype/newdeploy/newdeploy.go +++ b/pkg/executor/executortype/newdeploy/newdeploy.go @@ -501,14 +501,16 @@ func (deploy *NewDeploy) deleteSvc(ns string, name string) error { return deploy.kubernetesClient.CoreV1().Services(ns).Delete(context.TODO(), name, metav1.DeleteOptions{}) } -func (deploy *NewDeploy) waitForDeploy(depl *appsv1.Deployment, replicas int32, specializationTimeout int) (*appsv1.Deployment, error) { +func (deploy *NewDeploy) waitForDeploy(depl *appsv1.Deployment, replicas int32, specializationTimeout int) (latestDepl *appsv1.Deployment, err error) { + oldStatus := depl.Status + // if no specializationTimeout is set, use default value if specializationTimeout < fv1.DefaultSpecializationTimeOut { specializationTimeout = fv1.DefaultSpecializationTimeOut } for i := 0; i < specializationTimeout; i++ { - latestDepl, err := deploy.kubernetesClient.AppsV1().Deployments(depl.ObjectMeta.Namespace).Get(context.TODO(), depl.Name, metav1.GetOptions{}) + latestDepl, err = deploy.kubernetesClient.AppsV1().Deployments(depl.ObjectMeta.Namespace).Get(context.TODO(), depl.Name, metav1.GetOptions{}) if err != nil { return nil, err } @@ -521,6 +523,10 @@ func (deploy *NewDeploy) waitForDeploy(depl *appsv1.Deployment, replicas int32, time.Sleep(time.Second) } + deploy.logger.Error("Deployment provision failed within timeout window", + zap.String("name", latestDepl.ObjectMeta.Name), zap.Any("old_status", oldStatus), + zap.Any("current_status", latestDepl.Status), zap.Int("timeout", specializationTimeout)) + // this error appears in the executor pod logs timeoutError := fmt.Errorf("failed to create deployment within the timeout window of %d seconds", specializationTimeout) return nil, timeoutError diff --git a/pkg/executor/executortype/newdeploy/newdeploymgr.go b/pkg/executor/executortype/newdeploy/newdeploymgr.go index be25492e..ca3d27a2 100644 --- a/pkg/executor/executortype/newdeploy/newdeploymgr.go +++ b/pkg/executor/executortype/newdeploy/newdeploymgr.go @@ -410,6 +410,13 @@ func (deploy *NewDeploy) deleteFunction(fn *fv1.Function) error { } func (deploy *NewDeploy) fnCreate(fn *fv1.Function) (*fscache.FuncSvc, error) { + cleanupFunc := func(ns string, name string) { + err := deploy.cleanupNewdeploy(ns, name) + if err != nil { + deploy.logger.Error("received error while cleaning function resources", + zap.String("namespace", ns), zap.String("name", name)) + } + } env, err := deploy.fissionClient.CoreV1(). Environments(fn.Spec.Environment.Namespace). Get(context.TODO(), fn.Spec.Environment.Name, metav1.GetOptions{}) @@ -436,7 +443,7 @@ func (deploy *NewDeploy) fnCreate(fn *fv1.Function) (*fscache.FuncSvc, error) { svc, err := deploy.createOrGetSvc(deployLabels, deployAnnotations, objName, ns) if err != nil { deploy.logger.Error("error creating service", zap.Error(err), zap.String("service", objName)) - go deploy.cleanupNewdeploy(ns, objName) //nolint: errcheck + go cleanupFunc(ns, objName) return nil, errors.Wrapf(err, "error creating service %v", objName) } svcAddress := fmt.Sprintf("%v.%v", svc.Name, svc.Namespace) @@ -444,14 +451,14 @@ func (deploy *NewDeploy) fnCreate(fn *fv1.Function) (*fscache.FuncSvc, error) { depl, err := deploy.createOrGetDeployment(fn, env, objName, deployLabels, deployAnnotations, ns) if err != nil { deploy.logger.Error("error creating deployment", zap.Error(err), zap.String("deployment", objName)) - go deploy.cleanupNewdeploy(ns, objName) //nolint: errcheck + go cleanupFunc(ns, objName) return nil, errors.Wrapf(err, "error creating deployment %v", objName) } hpa, err := deploy.createOrGetHpa(objName, &fn.Spec.InvokeStrategy.ExecutionStrategy, depl, deployLabels, deployAnnotations) if err != nil { deploy.logger.Error("error creating HPA", zap.Error(err), zap.String("hpa", objName)) - go deploy.cleanupNewdeploy(ns, objName) //nolint: errcheck + go cleanupFunc(ns, objName) return nil, errors.Wrapf(err, "error creating the HPA %v", objName) } @@ -713,7 +720,16 @@ func (deploy *NewDeploy) fnDelete(fn *fv1.Function) error { func (deploy *NewDeploy) getObjName(fn *fv1.Function) string { // use meta uuid of function, this ensure we always get the same name for the same function. uid := fn.ObjectMeta.UID[len(fn.ObjectMeta.UID)-17:] - return strings.ToLower(fmt.Sprintf("newdeploy-%v-%v-%v", fn.ObjectMeta.Name, fn.ObjectMeta.Namespace, uid)) + var functionMetadata string + if len(fn.ObjectMeta.Name)+len(fn.ObjectMeta.Namespace) < 35 { + functionMetadata = fn.ObjectMeta.Name + "-" + fn.ObjectMeta.Namespace + } else { + functionMetadata = fn.ObjectMeta.Name[:17] + "-" + fn.ObjectMeta.Namespace[:17] + } + // contructed name should be 63 characters long, as it is a valid k8s name + // functionMetadata should be 35 characters long, as we take 17 characters from functionUid + // with newdeploy 10 character prefix + return strings.ToLower(fmt.Sprintf("newdeploy-%s-%s", functionMetadata, uid)) } func (deploy *NewDeploy) getDeployLabels(fnMeta metav1.ObjectMeta, envMeta metav1.ObjectMeta) map[string]string {