Merge pull request #9165 from smarterclayton/graceful

Enable graceful deletion using reconciliation loops in the Kubelet without TTL

pkg/registry/controller/etcd/etcd_test.go

@@ -695,7 +695,7 @@ func TestDelete(t *testing.T) {
 		// If the controller is still around after trying to delete either the delete
 		// failed, or we're deleting it gracefully.
 		if fakeClient.Data[key].R.Node != nil {
-			return true
+			return fakeClient.Data[key].R.Node.TTL != 0
 		}
 		return false
 	}

pkg/registry/event/etcd/etcd_test.go

@@ -37,6 +37,7 @@ var testTTL uint64 = 60
 
 func NewTestEventStorage(t *testing.T) (*tools.FakeEtcdClient, *REST) {
 	f := tools.NewFakeEtcdClient(t)
+	f.HideExpires = true
 	f.TestIndex = true
 
 	s := etcdstorage.NewEtcdStorage(f, testapi.Codec(), etcdtest.PathPrefix())

pkg/registry/generic/etcd/etcd.go

@@ -341,6 +341,11 @@ func (e *Etcd) Get(ctx api.Context, name string) (runtime.Object, error) {
 	return obj, nil
 }
 
+var (
+	errAlreadyDeleting = fmt.Errorf("abort delete")
+	errDeleteNow       = fmt.Errorf("delete now")
+)
+
 // Delete removes the item from etcd.
 func (e *Etcd) Delete(ctx api.Context, name string, options *api.DeleteOptions) (runtime.Object, error) {
 	key, err := e.KeyFunc(ctx, name)
@@ -367,13 +372,41 @@ func (e *Etcd) Delete(ctx api.Context, name string, options *api.DeleteOptions)
 	if pendingGraceful {
 		return e.finalizeDelete(obj, false)
 	}
-	if graceful && *options.GracePeriodSeconds != 0 {
+	if graceful {
 		trace.Step("Graceful deletion")
 		out := e.NewFunc()
-		if err := e.Storage.Set(key, obj, out, uint64(*options.GracePeriodSeconds)); err != nil {
+		lastGraceful := int64(0)
+		err := e.Storage.GuaranteedUpdate(
+			key, out, false,
+			storage.SimpleUpdate(func(existing runtime.Object) (runtime.Object, error) {
+				graceful, pendingGraceful, err := rest.BeforeDelete(e.DeleteStrategy, ctx, existing, options)
+				if err != nil {
+					return nil, err
+				}
+				if pendingGraceful {
+					return nil, errAlreadyDeleting
+				}
+				if !graceful {
+					return nil, errDeleteNow
+				}
+				lastGraceful = *options.GracePeriodSeconds
+				return existing, nil
+			}),
+		)
+		switch err {
+		case nil:
+			if lastGraceful > 0 {
+				return out, nil
+			}
+			// fall through and delete immediately
+		case errDeleteNow:
+			// we've updated the object to have a zero grace period, or it's already at 0, so
+			// we should fall through and truly delete the object.
+		case errAlreadyDeleting:
+			return e.finalizeDelete(obj, true)
+		default:
 			return nil, etcderr.InterpretUpdateError(err, e.EndpointName, name)
 		}
-		return e.finalizeDelete(out, true)
 	}
 
 	// delete immediately, or no graceful deletion supported

pkg/registry/persistentvolume/etcd/etcd_test.go

@@ -300,7 +300,7 @@ func TestEtcdUpdateStatus(t *testing.T) {
 	key, _ := storage.KeyFunc(ctx, "foo")
 	key = etcdtest.AddPrefix(key)
 	pvStart := validNewPersistentVolume("foo")
-	fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, pvStart), 1)
+	fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, pvStart), 0)
 
 	pvIn := &api.PersistentVolume{
 		ObjectMeta: api.ObjectMeta{

pkg/registry/persistentvolumeclaim/etcd/etcd_test.go

@@ -298,7 +298,7 @@ func TestEtcdUpdateStatus(t *testing.T) {
 	key, _ := storage.KeyFunc(ctx, "foo")
 	key = etcdtest.AddPrefix(key)
 	pvcStart := validNewPersistentVolumeClaim("foo", api.NamespaceDefault)
-	fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, pvcStart), 1)
+	fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, pvcStart), 0)
 
 	pvc := &api.PersistentVolumeClaim{
 		ObjectMeta: api.ObjectMeta{

pkg/registry/pod/etcd/etcd_test.go

@@ -56,6 +56,7 @@ func newStorage(t *testing.T) (*REST, *BindingREST, *StatusREST, *tools.FakeEtcd
 }
 
 func validNewPod() *api.Pod {
+	grace := int64(30)
 	return &api.Pod{
 		ObjectMeta: api.ObjectMeta{
 			Name:      "foo",
@@ -64,6 +65,8 @@ func validNewPod() *api.Pod {
 		Spec: api.PodSpec{
 			RestartPolicy: api.RestartPolicyAlways,
 			DNSPolicy:     api.DNSClusterFirst,
+
+			TerminationGracePeriodSeconds: &grace,
 			Containers: []api.Container{
 				{
 					Name:            "foo",
@@ -118,8 +121,10 @@ func TestDelete(t *testing.T) {
 	key = etcdtest.AddPrefix(key)
 	test := resttest.New(t, storage, fakeEtcdClient.SetError)
 
+	expectedNode := "some-node"
 	createFn := func() runtime.Object {
 		pod := validChangedPod()
+		pod.Spec.NodeName = expectedNode
 		fakeEtcdClient.Data[key] = tools.EtcdResponseWithError{
 			R: &etcd.Response{
 				Node: &etcd.Node{
@@ -134,8 +139,17 @@ func TestDelete(t *testing.T) {
 		if fakeEtcdClient.Data[key].R.Node == nil {
 			return false
 		}
-		return fakeEtcdClient.Data[key].R.Node.TTL == 30
+		obj, err := latest.Codec.Decode([]byte(fakeEtcdClient.Data[key].R.Node.Value))
+		if err != nil {
+			return false
+		}
+		pod := obj.(*api.Pod)
+		t.Logf("found object %#v", pod.ObjectMeta)
+		return pod.DeletionTimestamp != nil && pod.DeletionGracePeriodSeconds != nil && *pod.DeletionGracePeriodSeconds != 0
 	}
+	test.TestDeleteGraceful(createFn, 30, gracefulSetFn)
+
+	expectedNode = ""
 	test.TestDelete(createFn, gracefulSetFn)
 }
 
@@ -1027,6 +1041,7 @@ func TestEtcdUpdateScheduled(t *testing.T) {
 		},
 	}), 1)
 
+	grace := int64(30)
 	podIn := api.Pod{
 		ObjectMeta: api.ObjectMeta{
 			Name:            "foo",
@@ -1048,6 +1063,8 @@ func TestEtcdUpdateScheduled(t *testing.T) {
 			},
 			RestartPolicy: api.RestartPolicyAlways,
 			DNSPolicy:     api.DNSClusterFirst,
+
+			TerminationGracePeriodSeconds: &grace,
 		},
 	}
 	_, _, err := registry.Update(ctx, &podIn)
@@ -1088,7 +1105,7 @@ func TestEtcdUpdateStatus(t *testing.T) {
 			},
 		},
 	}
-	fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, &podStart), 1)
+	fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, &podStart), 0)
 
 	podIn := api.Pod{
 		ObjectMeta: api.ObjectMeta{
@@ -1117,6 +1134,8 @@ func TestEtcdUpdateStatus(t *testing.T) {
 
 	expected := podStart
 	expected.ResourceVersion = "2"
+	grace := int64(30)
+	expected.Spec.TerminationGracePeriodSeconds = &grace
 	expected.Spec.RestartPolicy = api.RestartPolicyAlways
 	expected.Spec.DNSPolicy = api.DNSClusterFirst
 	expected.Spec.Containers[0].ImagePullPolicy = api.PullIfNotPresent

pkg/registry/pod/rest.go

@@ -81,15 +81,49 @@ func (podStrategy) ValidateUpdate(ctx api.Context, obj, old runtime.Object) fiel
 	return append(errorList, validation.ValidatePodUpdate(obj.(*api.Pod), old.(*api.Pod))...)
 }
 
+// AllowUnconditionalUpdate allows pods to be overwritten
 func (podStrategy) AllowUnconditionalUpdate() bool {
 	return true
 }
 
-// CheckGracefulDelete allows a pod to be gracefully deleted.
+// CheckGracefulDelete allows a pod to be gracefully deleted. It updates the DeleteOptions to
+// reflect the desired grace value.
 func (podStrategy) CheckGracefulDelete(obj runtime.Object, options *api.DeleteOptions) bool {
+	if options == nil {
+		return false
+	}
+	pod := obj.(*api.Pod)
+	period := int64(0)
+	// user has specified a value
+	if options.GracePeriodSeconds != nil {
+		period = *options.GracePeriodSeconds
+	} else {
+		// use the default value if set, or deletes the pod immediately (0)
+		if pod.Spec.TerminationGracePeriodSeconds != nil {
+			period = *pod.Spec.TerminationGracePeriodSeconds
+		}
+	}
+	// if the pod is not scheduled, delete immediately
+	if len(pod.Spec.NodeName) == 0 {
+		period = 0
+	}
+	// ensure the options and the pod are in sync
+	options.GracePeriodSeconds = &period
+	return true
+}
+
+type podStrategyWithoutGraceful struct {
+	podStrategy
+}
+
+// CheckGracefulDelete prohibits graceful deletion.
+func (podStrategyWithoutGraceful) CheckGracefulDelete(obj runtime.Object, options *api.DeleteOptions) bool {
 	return false
 }
 
+// StrategyWithoutGraceful implements the legacy instant delele behavior.
+var StrategyWithoutGraceful = podStrategyWithoutGraceful{Strategy}
+
 type podStatusStrategy struct {
 	podStrategy
 }
@@ -100,6 +134,7 @@ func (podStatusStrategy) PrepareForUpdate(obj, old runtime.Object) {
 	newPod := obj.(*api.Pod)
 	oldPod := old.(*api.Pod)
 	newPod.Spec = oldPod.Spec
+	newPod.DeletionTimestamp = nil
 }
 
 func (podStatusStrategy) ValidateUpdate(ctx api.Context, obj, old runtime.Object) fielderrors.ValidationErrorList {

pkg/registry/resourcequota/etcd/etcd_test.go

@@ -389,7 +389,7 @@ func TestEtcdUpdateStatus(t *testing.T) {
 	key, _ := registry.KeyFunc(ctx, "foo")
 	key = etcdtest.AddPrefix(key)
 	resourcequotaStart := validNewResourceQuota()
-	fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, resourcequotaStart), 1)
+	fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, resourcequotaStart), 0)
 
 	resourcequotaIn := &api.ResourceQuota{
 		ObjectMeta: api.ObjectMeta{