Merge pull request #8641 from smarterclayton/gracefully_delete_pods

Gracefully delete pods from the Kubelet
kubernetes · Jun 2, 2015 · b7ae48e · b7ae48e
2 parents 3cb30e1 + f12a68c
commit b7ae48e
Show file tree

Hide file tree

Showing 66 changed files with 859 additions and 232 deletions.
diff --git a/cluster/saltbase/salt/fluentd-es/fluentd-es.yaml b/cluster/saltbase/salt/fluentd-es/fluentd-es.yaml
@@ -17,6 +17,7 @@ spec:
       mountPath: /varlog
     - name: containers
       mountPath: /var/lib/docker/containers
+  terminationGracePeriodSeconds: 30
   volumes:
   - name: varlog
     hostPath:

diff --git a/cluster/saltbase/salt/fluentd-gcp/fluentd-gcp.yaml b/cluster/saltbase/salt/fluentd-gcp/fluentd-gcp.yaml
@@ -17,6 +17,7 @@ spec:
       mountPath: /varlog
     - name: containers
       mountPath: /var/lib/docker/containers
+  terminationGracePeriodSeconds: 30
   volumes:
   - name: varlog
     hostPath:

diff --git a/cmd/integration/integration.go b/cmd/integration/integration.go
@@ -878,18 +878,24 @@ func runSchedulerNoPhantomPodsTest(client *client.Client) {
 
 	// Delete a pod to free up room.
 	glog.Infof("Deleting pod %v", bar.Name)
-	err = client.Pods(api.NamespaceDefault).Delete(bar.Name, nil)
+	err = client.Pods(api.NamespaceDefault).Delete(bar.Name, api.NewDeleteOptions(1))
 	if err != nil {
 		glog.Fatalf("FAILED: couldn't delete pod %q: %v", bar.Name, err)
 	}
 
+	time.Sleep(2 * time.Second)
+
 	pod.ObjectMeta.Name = "phantom.baz"
 	baz, err := client.Pods(api.NamespaceDefault).Create(pod)
 	if err != nil {
 		glog.Fatalf("Failed to create pod: %v, %v", pod, err)
 	}
 	if err := wait.Poll(time.Second, time.Second*60, podRunning(client, baz.Namespace, baz.Name)); err != nil {
-		glog.Fatalf("FAILED: (Scheduler probably didn't process deletion of 'phantom.bar') Pod never started running: %v", err)
+		if pod, perr := client.Pods(api.NamespaceDefault).Get("phantom.bar"); perr == nil {
+			glog.Fatalf("FAILED: 'phantom.bar' was never deleted: %#v", pod)
+		} else {
+			glog.Fatalf("FAILED: (Scheduler probably didn't process deletion of 'phantom.bar') Pod never started running: %v", err)
+		}
 	}
 
 	glog.Info("Scheduler doesn't make phantom pods: test passed.")

diff --git a/contrib/for-tests/network-tester/slow-pod.json b/contrib/for-tests/network-tester/slow-pod.json
@@ -0,0 +1,28 @@
+{
+  "kind": "Pod",
+  "apiVersion": "v1beta3",
+  "metadata": {
+    "name": "slow-pod",
+    "labels": {
+      "name": "nettest"
+    }
+  },
+  "spec": {
+    "containers": [
+      {
+        "name": "webserver",
+        "image": "gcr.io/google_containers/nettest:1.5",
+        "args": [
+          "-service=nettest",
+          "-delay-shutdown=10"
+        ],
+        "ports": [
+          {
+            "containerPort": 8080,
+            "protocol": "TCP"
+          }
+        ]
+      }
+    ]
+  }
+}
diff --git a/contrib/for-tests/network-tester/slow-rc.json b/contrib/for-tests/network-tester/slow-rc.json
@@ -0,0 +1,42 @@
+{
+  "kind": "ReplicationController",
+  "apiVersion": "v1beta3",
+  "metadata": {
+    "name": "slow-rc",
+    "labels": {
+      "name": "nettest"
+    }
+  },
+  "spec": {
+    "replicas": 8,
+    "selector": {
+      "name": "nettest"
+    },
+    "template": {
+      "metadata": {
+        "labels": {
+          "name": "nettest"
+        }
+      },
+      "spec": {
+        "terminationGracePeriodSeconds": 5,
+        "containers": [
+          {
+            "name": "webserver",
+            "image": "gcr.io/google_containers/nettest:1.5",
+            "args": [
+              "-service=nettest",
+              "-delay-shutdown=10"
+            ],
+            "ports": [
+              {
+                "containerPort": 8080,
+                "protocol": "TCP"
+              }
+            ]
+          }
+        ]
+      }
+    }
+  }
+}
diff --git a/contrib/for-tests/network-tester/webserver.go b/contrib/for-tests/network-tester/webserver.go
@@ -40,18 +40,21 @@ import (
 	"net"
 	"net/http"
 	"os"
+	"os/signal"
 	"sync"
+	"syscall"
 	"time"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 )
 
 var (
-	port      = flag.Int("port", 8080, "Port number to serve at.")
-	peerCount = flag.Int("peers", 8, "Must find at least this many peers for the test to pass.")
-	service   = flag.String("service", "nettest", "Service to find other network test pods in.")
-	namespace = flag.String("namespace", "default", "Namespace of this pod. TODO: kubernetes should make this discoverable.")
+	port          = flag.Int("port", 8080, "Port number to serve at.")
+	peerCount     = flag.Int("peers", 8, "Must find at least this many peers for the test to pass.")
+	service       = flag.String("service", "nettest", "Service to find other network test pods in.")
+	namespace     = flag.String("namespace", "default", "Namespace of this pod. TODO: kubernetes should make this discoverable.")
+	delayShutdown = flag.Int("delay-shutdown", 0, "Number of seconds to delay shutdown when receiving SIGTERM.")
 )
 
 // State tracks the internal state of our little http server.
@@ -179,6 +182,17 @@ func main() {
 		log.Fatalf("Error getting hostname: %v", err)
 	}
 
+	if *delayShutdown > 0 {
+		termCh := make(chan os.Signal)
+		signal.Notify(termCh, syscall.SIGTERM)
+		go func() {
+			<-termCh
+			log.Printf("Sleeping %d seconds before exit ...", *delayShutdown)
+			time.Sleep(time.Duration(*delayShutdown) * time.Second)
+			os.Exit(0)
+		}()
+	}
+
 	state := State{
 		Hostname:             hostname,
 		StillContactingPeers: true,

diff --git a/hack/lib/test.sh b/hack/lib/test.sh
@@ -23,7 +23,7 @@ readonly   red=$(tput setaf 1)
 readonly green=$(tput setaf 2)
 
 kube::test::clear_all() {
-  kubectl delete "${kube_flags[@]}" rc,pods --all
+  kubectl delete "${kube_flags[@]}" rc,pods --all --grace-period=0
 }
 
 kube::test::get_object_assert() {

diff --git a/hack/test-cmd.sh b/hack/test-cmd.sh
@@ -179,6 +179,11 @@ for version in "${kube_api_versions[@]}"; do
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:'
   # Command
   kubectl delete pod valid-pod "${kube_flags[@]}"
+  # Post-condition: pod is still there, in terminating
+  kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:'
+  [[ "$(kubectl get pods "${kube_flags[@]}" | grep Terminating)" ]]
+  # Command
+  kubectl delete pod valid-pod "${kube_flags[@]}" --grace-period=0
   # Post-condition: no POD is running
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" ''
 
@@ -194,7 +199,7 @@ for version in "${kube_api_versions[@]}"; do
   # Pre-condition: valid-pod POD is running
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:'
   # Command
-  kubectl delete -f examples/limitrange/valid-pod.json "${kube_flags[@]}"
+  kubectl delete -f examples/limitrange/valid-pod.json "${kube_flags[@]}" --grace-period=0
   # Post-condition: no POD is running
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" ''
 
@@ -210,7 +215,7 @@ for version in "${kube_api_versions[@]}"; do
   # Pre-condition: valid-pod POD is running
   kube::test::get_object_assert "pods -l'name in (valid-pod)'" '{{range.items}}{{$id_field}}:{{end}}' 'valid-pod:'
   # Command
-  kubectl delete pods -l'name in (valid-pod)' "${kube_flags[@]}"
+  kubectl delete pods -l'name in (valid-pod)' "${kube_flags[@]}" --grace-period=0
   # Post-condition: no POD is running
   kube::test::get_object_assert "pods -l'name in (valid-pod)'" '{{range.items}}{{$id_field}}:{{end}}' ''
 
@@ -242,7 +247,7 @@ for version in "${kube_api_versions[@]}"; do
   # Pre-condition: valid-pod POD is running
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:'
   # Command
-  kubectl delete --all pods "${kube_flags[@]}" # --all remove all the pods
+  kubectl delete --all pods "${kube_flags[@]}" --grace-period=0 # --all remove all the pods
   # Post-condition: no POD is running
   kube::test::get_object_assert "pods -l'name in (valid-pod)'" '{{range.items}}{{$id_field}}:{{end}}' ''
 
@@ -259,7 +264,7 @@ for version in "${kube_api_versions[@]}"; do
   # Pre-condition: valid-pod and redis-proxy PODs are running
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'redis-proxy:valid-pod:'
   # Command
-  kubectl delete pods valid-pod redis-proxy "${kube_flags[@]}" # delete multiple pods at once
+  kubectl delete pods valid-pod redis-proxy "${kube_flags[@]}" --grace-period=0 # delete multiple pods at once
   # Post-condition: no POD is running
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" ''
 
@@ -276,7 +281,7 @@ for version in "${kube_api_versions[@]}"; do
   # Pre-condition: valid-pod and redis-proxy PODs are running
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'redis-proxy:valid-pod:'
   # Command
-  kubectl stop pods valid-pod redis-proxy "${kube_flags[@]}" # stop multiple pods at once
+  kubectl stop pods valid-pod redis-proxy "${kube_flags[@]}" --grace-period=0 # stop multiple pods at once
   # Post-condition: no POD is running
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" ''
 
@@ -300,7 +305,7 @@ for version in "${kube_api_versions[@]}"; do
   # Pre-condition: valid-pod POD is running
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:'
   # Command
-  kubectl delete pods -lnew-name=new-valid-pod "${kube_flags[@]}"
+  kubectl delete pods -lnew-name=new-valid-pod --grace-period=0 "${kube_flags[@]}"
   # Post-condition: no POD is running
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" ''
 
@@ -332,7 +337,7 @@ for version in "${kube_api_versions[@]}"; do
   # Pre-condition: valid-pod POD is running
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:'
   # Command
-  kubectl delete pods -l'name in (valid-pod-super-sayan)' "${kube_flags[@]}"
+  kubectl delete pods -l'name in (valid-pod-super-sayan)' --grace-period=0 "${kube_flags[@]}"
   # Post-condition: no POD is running
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" ''
 
@@ -353,7 +358,7 @@ for version in "${kube_api_versions[@]}"; do
   # Pre-condition: valid-pod POD is running
   kube::test::get_object_assert 'pods --namespace=other' "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:'
   # Command
-  kubectl delete "${kube_flags[@]}" pod --namespace=other valid-pod
+  kubectl delete "${kube_flags[@]}" pod --namespace=other valid-pod --grace-period=0
   # Post-condition: no POD is running
   kube::test::get_object_assert 'pods --namespace=other' "{{range.items}}{{$id_field}}:{{end}}" ''
 

diff --git a/pkg/api/deep_copy_generated.go b/pkg/api/deep_copy_generated.go
@@ -1002,6 +1002,12 @@ func deepCopy_api_ObjectMeta(in ObjectMeta, out *ObjectMeta, c *conversion.Clone
 	} else {
 		out.DeletionTimestamp = nil
 	}
+	if in.DeletionGracePeriodSeconds != nil {
+		out.DeletionGracePeriodSeconds = new(int64)
+		*out.DeletionGracePeriodSeconds = *in.DeletionGracePeriodSeconds
+	} else {
+		out.DeletionGracePeriodSeconds = nil
+	}
 	if in.Labels != nil {
 		out.Labels = make(map[string]string)
 		for key, val := range in.Labels {

diff --git a/pkg/api/rest/create.go b/pkg/api/rest/create.go
@@ -59,6 +59,8 @@ func BeforeCreate(strategy RESTCreateStrategy, ctx api.Context, obj runtime.Obje
 	} else {
 		objectMeta.Namespace = api.NamespaceNone
 	}
+	objectMeta.DeletionTimestamp = nil
+	objectMeta.DeletionGracePeriodSeconds = nil
 	strategy.PrepareForCreate(obj)
 	api.FillObjectMetaSystemFields(ctx, objectMeta)
 	api.GenerateName(strategy, objectMeta)

diff --git a/pkg/api/rest/delete.go b/pkg/api/rest/delete.go
@@ -40,12 +40,37 @@ func BeforeDelete(strategy RESTDeleteStrategy, ctx api.Context, obj runtime.Obje
 	if strategy == nil {
 		return false, false, nil
 	}
-	_, _, kerr := objectMetaAndKind(strategy, obj)
+	objectMeta, _, kerr := objectMetaAndKind(strategy, obj)
 	if kerr != nil {
 		return false, false, kerr
 	}
+
+	// if the object is already being deleted
+	if objectMeta.DeletionTimestamp != nil {
+		// if we are already being deleted, we may only shorten the deletion grace period
+		// this means the object was gracefully deleted previously but deletionGracePeriodSeconds was not set,
+		// so we force deletion immediately
+		if objectMeta.DeletionGracePeriodSeconds == nil {
+			return false, false, nil
+		}
+		// only a shorter grace period may be provided by a user
+		if options.GracePeriodSeconds != nil {
+			period := int64(*options.GracePeriodSeconds)
+			if period > *objectMeta.DeletionGracePeriodSeconds {
+				return false, true, nil
+			}
+			objectMeta.DeletionGracePeriodSeconds = &period
+			options.GracePeriodSeconds = &period
+			return true, false, nil
+		}
+		// graceful deletion is pending, do nothing
+		options.GracePeriodSeconds = objectMeta.DeletionGracePeriodSeconds
+		return false, true, nil
+	}
+
 	if !strategy.CheckGracefulDelete(obj, options) {
 		return false, false, nil
 	}
+	objectMeta.DeletionGracePeriodSeconds = options.GracePeriodSeconds
 	return true, false, nil
 }