From c94e02b302bc90f380cc7464b7e7b92636bc5dc6 Mon Sep 17 00:00:00 2001
From: Ta-Ching Chen <contact@tachingchen.com>
Date: Mon, 16 Sep 2019 15:43:36 +0800
Subject: [PATCH] Fix newdeploy doesn't handle error properly (#1316)

---
 pkg/executor/newdeploy/newdeploy.go    | 30 +++++++-------------------
 pkg/executor/newdeploy/newdeploymgr.go |  8 +++++--
 2 files changed, 14 insertions(+), 24 deletions(-)

diff --git a/pkg/executor/newdeploy/newdeploy.go b/pkg/executor/newdeploy/newdeploy.go
index 1e92b087..f077edbb 100644
--- a/pkg/executor/newdeploy/newdeploy.go
+++ b/pkg/executor/newdeploy/newdeploy.go
@@ -156,13 +156,9 @@ func (deploy *NewDeploy) deleteDeployment(ns string, name string) error {
 	// DeletePropagationBackground deletes the object immediately and dependent are deleted later
 	// DeletePropagationForeground not advisable; it markes for deleteion and API can still serve those objects
 	deletePropagation := metav1.DeletePropagationBackground
-	err := deploy.kubernetesClient.ExtensionsV1beta1().Deployments(ns).Delete(name, &metav1.DeleteOptions{
+	return deploy.kubernetesClient.ExtensionsV1beta1().Deployments(ns).Delete(name, &metav1.DeleteOptions{
 		PropagationPolicy: &deletePropagation,
 	})
-	if err != nil {
-		return err
-	}
-	return nil
 }
 
 func (deploy *NewDeploy) getDeploymentSpec(fn *fv1.Function, env *fv1.Environment,
@@ -364,18 +360,14 @@ func (deploy *NewDeploy) updateHpa(hpa *asv1.HorizontalPodAutoscaler) error {
 }
 
 func (deploy *NewDeploy) deleteHpa(ns string, name string) error {
-	err := deploy.kubernetesClient.AutoscalingV1().HorizontalPodAutoscalers(ns).Delete(name, &metav1.DeleteOptions{})
-	return err
+	return deploy.kubernetesClient.AutoscalingV1().HorizontalPodAutoscalers(ns).Delete(name, &metav1.DeleteOptions{})
 }
 
 func (deploy *NewDeploy) createOrGetSvc(deployLabels map[string]string, svcName string, svcNamespace string) (*apiv1.Service, error) {
-
 	existingSvc, err := deploy.kubernetesClient.CoreV1().Services(svcNamespace).Get(svcName, metav1.GetOptions{})
 	if err == nil {
 		return existingSvc, err
-	}
-
-	if err != nil && k8s_err.IsNotFound(err) {
+	} else if k8s_err.IsNotFound(err) {
 		service := &apiv1.Service{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:   svcName,
@@ -398,19 +390,13 @@ func (deploy *NewDeploy) createOrGetSvc(deployLabels map[string]string, svcName
 		if err != nil {
 			return nil, err
 		}
-
 		return svc, nil
 	}
-
 	return nil, err
 }
 
 func (deploy *NewDeploy) deleteSvc(ns string, name string) error {
-	err := deploy.kubernetesClient.CoreV1().Services(ns).Delete(name, &metav1.DeleteOptions{})
-	if err != nil {
-		return err
-	}
-	return nil
+	return deploy.kubernetesClient.CoreV1().Services(ns).Delete(name, &metav1.DeleteOptions{})
 }
 
 func (deploy *NewDeploy) waitForDeploy(depl *v1beta1.Deployment, replicas int32, specializationTimeout int) (*v1beta1.Deployment, error) {
@@ -443,7 +429,7 @@ func (deploy *NewDeploy) cleanupNewdeploy(ns string, name string) error {
 	result := &multierror.Error{}
 
 	err := deploy.deleteSvc(ns, name)
-	if err != nil {
+	if err != nil && !k8s_err.IsNotFound(err) {
 		deploy.logger.Error("error deleting service for newdeploy function",
 			zap.Error(err),
 			zap.String("function_name", name),
@@ -452,8 +438,8 @@ func (deploy *NewDeploy) cleanupNewdeploy(ns string, name string) error {
 	}
 
 	err = deploy.deleteHpa(ns, name)
-	if err != nil {
-		deploy.logger.Error("error deleting service for newdeploy function",
+	if err != nil && !k8s_err.IsNotFound(err) {
+		deploy.logger.Error("error deleting HPA for newdeploy function",
 			zap.Error(err),
 			zap.String("function_name", name),
 			zap.String("function_namespace", ns))
@@ -461,7 +447,7 @@ func (deploy *NewDeploy) cleanupNewdeploy(ns string, name string) error {
 	}
 
 	err = deploy.deleteDeployment(ns, name)
-	if err != nil {
+	if err != nil && !k8s_err.IsNotFound(err) {
 		deploy.logger.Error("error deleting deployment for newdeploy function",
 			zap.Error(err),
 			zap.String("function_name", name),
diff --git a/pkg/executor/newdeploy/newdeploymgr.go b/pkg/executor/newdeploy/newdeploymgr.go
index 91263a41..472649ba 100644
--- a/pkg/executor/newdeploy/newdeploymgr.go
+++ b/pkg/executor/newdeploy/newdeploymgr.go
@@ -187,10 +187,12 @@ func (deploy *NewDeploy) initEnvController() (k8sCache.Store, k8sCache.Controlle
 					function, err := deploy.fissionClient.Functions(f.Metadata.Namespace).Get(f.Metadata.Name)
 					if err != nil {
 						deploy.logger.Error("Error getting function", zap.Error(err), zap.Any("function", function))
+						continue
 					}
 					err = deploy.updateFuncDeployment(function, newEnv)
 					if err != nil {
 						deploy.logger.Error("Error updating function", zap.Error(err), zap.Any("function", function))
+						continue
 					}
 				}
 			}
@@ -312,9 +314,10 @@ func (deploy *NewDeploy) fnCreate(fn *fv1.Function, firstcreate bool) (*fscache.
 	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
+		if err != nil {
+			return nil, err
 		}
+		objName = fsvc.Name
 	}
 	deployLabels := deploy.getDeployLabels(fn.Metadata, env.Metadata)
 
@@ -337,6 +340,7 @@ func (deploy *NewDeploy) fnCreate(fn *fv1.Function, firstcreate bool) (*fscache.
 		return nil, errors.Wrapf(err, "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 {
 		deploy.logger.Error("error creating deployment", zap.Error(err), zap.String("deployment", objName))