Delete deployment with proper delete propagation policy (#630)

buildermgr/envwatcher.go

@@ -45,6 +45,11 @@ const (
 	LABEL_ENV_RESOURCEVERSION = "envResourceVersion"
 )
 
+var (
+	deletePropagation = metav1.DeletePropagationBackground
+	delOpt            = metav1.DeleteOptions{PropagationPolicy: &deletePropagation}
+)
+
 type (
 	builderInfo struct {
 		envMetadata *metav1.ObjectMeta
@@ -341,17 +346,9 @@ func (envw *environmentWatcher) deleteBuilderService(sel map[string]string) erro
 	}
 	for _, svc := range svcList {
 		log.Printf("Removing builder service: %v", svc.ObjectMeta.Name)
-
-		// cascading deletion
-		// https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/
-		falseVal := false
-		delOpt := &metav1.DeleteOptions{
-			OrphanDependents: &falseVal,
-		}
-
 		err = envw.kubernetesClient.
 			Services(envw.builderNamespace).
-			Delete(svc.ObjectMeta.Name, delOpt)
+			Delete(svc.ObjectMeta.Name, nil)
 		if err != nil {
 			return fmt.Errorf("Error deleting builder service: %v", err)
 		}
@@ -366,15 +363,9 @@ func (envw *environmentWatcher) deleteBuilderDeployment(sel map[string]string) e
 	}
 	for _, deploy := range deployList {
 		log.Printf("Removing builder deployment: %v", deploy.ObjectMeta.Name)
-
-		falseVal := false
-		delOpt := &metav1.DeleteOptions{
-			OrphanDependents: &falseVal,
-		}
-
 		err = envw.kubernetesClient.ExtensionsV1beta1().
 			Deployments(envw.builderNamespace).
-			Delete(deploy.ObjectMeta.Name, delOpt)
+			Delete(deploy.ObjectMeta.Name, &delOpt)
 		if err != nil {
 			return fmt.Errorf("Error deleteing builder deployment: %v", err)
 		}

executor/cleanup.go

@@ -23,7 +23,6 @@ import (
 	"time"
 
 	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/pkg/api"
@@ -33,6 +32,11 @@ import (
 	"github.com/fission/fission/executor/fscache"
 )
 
+var (
+	deletePropagation = meta_v1.DeletePropagationBackground
+	delOpt            = meta_v1.DeleteOptions{PropagationPolicy: &deletePropagation}
+)
+
 // cleanupObjects cleans up resources created by old executortype instances
 func cleanupObjects(kubernetesClient *kubernetes.Clientset,
 	namespace string,
@@ -65,14 +69,6 @@ func cleanup(client *kubernetes.Clientset, namespace string, instanceId string)
 	if err != nil {
 		return err
 	}
-	// See K8s #33845 and related bugs: deleting a deployment
-	// through the API doesn't cause the associated ReplicaSet to
-	// be deleted.  (Fixed recently, but we may be running a
-	// version before the fix.)
-	err = cleanupReplicaSets(client, namespace, instanceId)
-	if err != nil {
-		return err
-	}
 
 	// Pods might still be running user functions, so we give them
 	// a few minutes before terminating them.  This time is the
@@ -161,10 +157,8 @@ func deleteKubeobject(kubeClient *kubernetes.Clientset, kubeobj *api.ObjectRefer
 		logErr(fmt.Sprintf("cleaning up service %v ", kubeobj.Name), err)
 
 	case "deployment":
-		depl, err := kubeClient.ExtensionsV1beta1().Deployments(kubeobj.Namespace).Get(kubeobj.Name, meta_v1.GetOptions{})
+		err := kubeClient.ExtensionsV1beta1().Deployments(kubeobj.Namespace).Delete(kubeobj.Name, &delOpt)
-		err = kubeClient.ExtensionsV1beta1().Deployments(kubeobj.Namespace).Delete(kubeobj.Name, nil)
 		logErr(fmt.Sprintf("cleaning up deployment %v ", kubeobj.Name), err)
-		cleanupDeploymentObjects(kubeClient, kubeobj.Namespace, depl.Labels)
 
 	case "horizontalpodautoscaler":
 		err := kubeClient.AutoscalingV1().HorizontalPodAutoscalers(kubeobj.Namespace).Delete(kubeobj.Name, nil)
@@ -176,22 +170,6 @@ func deleteKubeobject(kubeClient *kubernetes.Clientset, kubeobj *api.ObjectRefer
 	}
 }
 
-func cleanupDeploymentObjects(kubeClient *kubernetes.Clientset, namespace string, sel map[string]string) {
-	rsList, err := kubeClient.ExtensionsV1beta1().ReplicaSets(namespace).List(meta_v1.ListOptions{LabelSelector: labels.Set(sel).AsSelector().String()})
-	logErr("Getting replicaset for deployment ", err)
-	for _, rs := range rsList.Items {
-		err = kubeClient.ExtensionsV1beta1().ReplicaSets(namespace).Delete(rs.Name, nil)
-		logErr(fmt.Sprintf("Cleaning replicaset %v for deployment", rs.Name), err)
-	}
-
-	podList, err := kubeClient.CoreV1().Pods(namespace).List(meta_v1.ListOptions{LabelSelector: labels.Set(sel).AsSelector().String()})
-	logErr("Getting pods for deployment ", err)
-	for _, pod := range podList.Items {
-		err = kubeClient.CoreV1().Pods(namespace).Delete(pod.Name, nil)
-		logErr(fmt.Sprintf("Cleaning pod %v for deployment", pod.Name), err)
-	}
-}
-
 func cleanupDeployments(client *kubernetes.Clientset, namespace string, instanceId string) error {
 	deploymentList, err := client.ExtensionsV1beta1().Deployments(namespace).List(meta_v1.ListOptions{})
 	if err != nil {
@@ -201,7 +179,7 @@ func cleanupDeployments(client *kubernetes.Clientset, namespace string, instance
 		id, ok := dep.ObjectMeta.Labels[fission.EXECUTOR_INSTANCEID_LABEL]
 		if ok && id != instanceId {
 			log.Printf("Cleaning up deployment %v", dep.ObjectMeta.Name)
-			err := client.ExtensionsV1beta1().Deployments(namespace).Delete(dep.ObjectMeta.Name, nil)
+			err := client.ExtensionsV1beta1().Deployments(namespace).Delete(dep.ObjectMeta.Name, &delOpt)
 			logErr("cleaning up deployment", err)
 			// ignore err
 		}
@@ -209,7 +187,7 @@ func cleanupDeployments(client *kubernetes.Clientset, namespace string, instance
 		pid, pok := dep.ObjectMeta.Labels[fission.POOLMGR_INSTANCEID_LABEL]
 		if pok && pid != instanceId {
 			log.Printf("Cleaning up deployment %v", dep.ObjectMeta.Name)
-			err := client.ExtensionsV1beta1().Deployments(namespace).Delete(dep.ObjectMeta.Name, nil)
+			err := client.ExtensionsV1beta1().Deployments(namespace).Delete(dep.ObjectMeta.Name, &delOpt)
 			logErr("cleaning up deployment", err)
 			// ignore err
 		}
@@ -217,35 +195,13 @@ func cleanupDeployments(client *kubernetes.Clientset, namespace string, instance
 	return nil
 }
 
-func cleanupReplicaSets(client *kubernetes.Clientset, namespace string, instanceId string) error {
-	rsList, err := client.ExtensionsV1beta1().ReplicaSets(namespace).List(meta_v1.ListOptions{})
-	if err != nil {
-		return err
-	}
-	for _, rs := range rsList.Items {
-		id, ok := rs.ObjectMeta.Labels[fission.EXECUTOR_INSTANCEID_LABEL]
-		if ok && id != instanceId {
-			log.Printf("Cleaning up replicaset %v", rs.ObjectMeta.Name)
-			err := client.ExtensionsV1beta1().ReplicaSets(namespace).Delete(rs.ObjectMeta.Name, nil)
-			logErr("cleaning up replicaset", err)
-		}
-		// Backward compatibility with older label name
-		pid, pok := rs.ObjectMeta.Labels[fission.POOLMGR_INSTANCEID_LABEL]
-		if pok && pid != instanceId {
-			log.Printf("Cleaning up replicaset %v", rs.ObjectMeta.Name)
-			err := client.ExtensionsV1beta1().ReplicaSets(namespace).Delete(rs.ObjectMeta.Name, nil)
-			logErr("cleaning up replicaset", err)
-		}
-	}
-	return nil
-}
-
 func cleanupPods(client *kubernetes.Clientset, namespace string, instanceId string) error {
 	podList, err := client.CoreV1().Pods(namespace).List(meta_v1.ListOptions{})
 	if err != nil {
 		return err
 	}
 	for _, pod := range podList.Items {
+		log.Printf("Clean pod: %v", pod.ObjectMeta.Name)
 		id, ok := pod.ObjectMeta.Labels[fission.EXECUTOR_INSTANCEID_LABEL]
 		if ok && id != instanceId {
 			log.Printf("Cleaning up pod %v", pod.ObjectMeta.Name)

executor/poolmgr/gp.go

@@ -802,40 +802,15 @@ func (gp *GenericPool) GetFuncSvc(m *metav1.ObjectMeta) (*fscache.FuncSvc, error
 
 // destroys the pool -- the deployment, replicaset and pods
 func (gp *GenericPool) destroy() error {
-	// Destroy deployment
+	deletePropagation := metav1.DeletePropagationBackground
-	err := gp.kubernetesClient.ExtensionsV1beta1().Deployments(gp.namespace).Delete(gp.deployment.ObjectMeta.Name, nil)
+	delOpt := metav1.DeleteOptions{
+		PropagationPolicy: &deletePropagation,
+	}
+	err := gp.kubernetesClient.ExtensionsV1beta1().
+		Deployments(gp.namespace).Delete(gp.deployment.ObjectMeta.Name, &delOpt)
 	if err != nil {
 		log.Printf("Error destroying deployment: %v", err)
 		return err
 	}
-
-	// Destroy ReplicaSet.  Pre-1.6 K8s versions don't do this
-	// automatically but post-1.6 K8s will, and may beat us to it,
-	// so don't error out if we fail.
-	rsList, err := gp.kubernetesClient.ExtensionsV1beta1().ReplicaSets(gp.namespace).List(metav1.ListOptions{
-		LabelSelector: labels.Set(gp.labelsForPool).AsSelector().String(),
-	})
-	if len(rsList.Items) >= 0 {
-		for _, rs := range rsList.Items {
-			err = gp.kubernetesClient.ExtensionsV1beta1().ReplicaSets(gp.namespace).Delete(rs.ObjectMeta.Name, nil)
-			if err != nil {
-				log.Printf("Error deleting replicaset, ignoring: %v", err)
-			}
-		}
-	}
-
-	// Destroy Pods.  See note above.
-	podList, err := gp.kubernetesClient.CoreV1().Pods(gp.namespace).List(metav1.ListOptions{
-		LabelSelector: labels.Set(gp.labelsForPool).AsSelector().String(),
-	})
-	if len(podList.Items) >= 0 {
-		for _, pod := range podList.Items {
-			err = gp.kubernetesClient.CoreV1().Pods(gp.namespace).Delete(pod.ObjectMeta.Name, nil)
-			if err != nil {
-				log.Printf("Error deleting pod, ignoring: %v", err)
-			}
-		}
-	}
-
 	return nil
 }