github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Alter graceful deletion to not use TTL

Browse Source 
Avoid TTL by deleting pods immediately when they aren't
scheduled, and letting the Kubelet delete them otherwise.

Ensure the Kubelet uses pod.Spec.TerminationGracePeriodSeconds
when no pod.DeletionGracePeriodSeconds is available.
This commit is contained in:
Clayton Coleman
2015-07-27 16:41:31 -04:00
parent b842a7dd15
commit 89f1f3b1b8
17 changed files with 223 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
pkg/api/rest/delete.go
View File

@@ -17,8 +17,11 @@ limitations under the License.
package rest
import (
"time"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/runtime"
"k8s.io/kubernetes/pkg/util"
)
// RESTDeleteStrategy defines deletion behavior on an object that follows Kubernetes
@@ -59,6 +62,8 @@ func BeforeDelete(strategy RESTDeleteStrategy, ctx api.Context, obj runtime.Obje
if period > *objectMeta.DeletionGracePeriodSeconds {
return false, true, nil
}
now := util.NewTime(util.Now().Add(time.Second * time.Duration(*options.GracePeriodSeconds)))
objectMeta.DeletionTimestamp = &now
objectMeta.DeletionGracePeriodSeconds = &period
options.GracePeriodSeconds = &period
return true, false, nil
@@ -71,6 +76,8 @@ func BeforeDelete(strategy RESTDeleteStrategy, ctx api.Context, obj runtime.Obje
if !strategy.CheckGracefulDelete(obj, options) {
return false, false, nil
}
now := util.NewTime(util.Now().Add(time.Second * time.Duration(*options.GracePeriodSeconds)))
objectMeta.DeletionTimestamp = &now
objectMeta.DeletionGracePeriodSeconds = options.GracePeriodSeconds
return true, false, nil
}

111
pkg/api/rest/resttest/resttest.go
View File

@@ -114,17 +114,20 @@ func (t *Tester) TestUpdate(valid runtime.Object, existing, older runtime.Object
// Test deleting an object.
func (t *Tester) TestDelete(createFn func() runtime.Object, wasGracefulFn func() bool, invalid ...runtime.Object) {
t.testDeleteNonExist(createFn)
t.testDeleteNoGraceful(createFn, wasGracefulFn)
t.testDeleteInvokesValidation(invalid...)
t.TestDeleteNonExist(createFn)
t.TestDeleteNoGraceful(createFn, wasGracefulFn)
t.TestDeleteInvokesValidation(invalid...)
// TODO: Test delete namespace mismatch rejection
// once #5684 is fixed.
}
// Test graceful deletion.
func (t *Tester) TestDeleteGraceful(createFn func() runtime.Object, expectedGrace int64, wasGracefulFn func() bool) {
t.testDeleteGracefulHasDefault(createFn(), expectedGrace, wasGracefulFn)
t.testDeleteGracefulUsesZeroOnNil(createFn(), 0)
t.TestDeleteGracefulHasDefault(createFn(), expectedGrace, wasGracefulFn)
t.TestDeleteGracefulWithValue(createFn(), expectedGrace, wasGracefulFn)
t.TestDeleteGracefulUsesZeroOnNil(createFn(), 0)
t.TestDeleteGracefulExtend(createFn(), expectedGrace, wasGracefulFn)
t.TestDeleteGracefulImmediate(createFn(), expectedGrace, wasGracefulFn)
}
// Test getting object.
@@ -298,6 +301,33 @@ func (t *Tester) testUpdateFailsOnVersion(older runtime.Object) {
// =============================================================================
// Deletion tests.
func (t *Tester) TestDeleteInvokesValidation(invalid ...runtime.Object) {
for i, obj := range invalid {
objectMeta := t.getObjectMetaOrFail(obj)
ctx := t.TestContext()
_, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, nil)
if !errors.IsInvalid(err) {
t.Errorf("%d: Expected to get an invalid resource error, got %v", i, err)
}
}
}
func (t *Tester) TestDeleteNonExist(createFn func() runtime.Object) {
existing := createFn()
objectMeta := t.getObjectMetaOrFail(existing)
context := t.TestContext()
t.withStorageError(&etcd.EtcdError{ErrorCode: tools.EtcdErrorCodeNotFound}, func() {
_, err := t.storage.(rest.GracefulDeleter).Delete(context, objectMeta.Name, nil)
if err == nil || !errors.IsNotFound(err) {
t.Fatalf("Unexpected error: %v", err)
}
})
}
// =============================================================================
// Graceful Deletion tests.
func (t *Tester) TestDeleteNoGraceful(createFn func() runtime.Object, wasGracefulFn func() bool) {
existing := createFn()
objectMeta := t.getObjectMetaOrFail(existing)
@@ -314,41 +344,7 @@ func (t *Tester) TestDeleteNoGraceful(createFn func() runtime.Object, wasGracefu
}
}
func (t *Tester) testDeleteInvokesValidation(invalid ...runtime.Object) {
for i, obj := range invalid {
objectMeta := t.getObjectMetaOrFail(obj)
ctx := t.TestContext()
_, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, nil)
if !errors.IsInvalid(err) {
t.Errorf("%d: Expected to get an invalid resource error, got %v", i, err)
}
}
}
func (t *Tester) testDeleteNonExist(createFn func() runtime.Object) {
existing := createFn()
objectMeta := t.getObjectMetaOrFail(existing)
context := t.TestContext()
t.withStorageError(&etcd.EtcdError{ErrorCode: tools.EtcdErrorCodeNotFound}, func() {
_, err := t.storage.(rest.GracefulDeleter).Delete(context, objectMeta.Name, nil)
if err == nil || !errors.IsNotFound(err) {
t.Fatalf("Unexpected error: %v", err)
}
})
}
// =============================================================================
// Graceful Deletion tests.
func (t *Tester) TestDeleteGraceful(createFn func() runtime.Object, expectedGrace int64, wasGracefulFn func() bool) {
t.TestDeleteGracefulHasDefault(createFn(), expectedGrace, wasGracefulFn)
t.TestDeleteGracefulWithValue(createFn(), expectedGrace, wasGracefulFn)
t.TestDeleteGracefulUsesZeroOnNil(createFn(), 0)
t.TestDeleteGracefulExtend(createFn(), expectedGrace, wasGracefulFn)
}
func (t *Tester) testDeleteGracefulHasDefault(existing runtime.Object, expectedGrace int64, wasGracefulFn func() bool) {
func (t *Tester) TestDeleteGracefulHasDefault(existing runtime.Object, expectedGrace int64, wasGracefulFn func() bool) {
objectMeta := t.getObjectMetaOrFail(existing)
ctx := api.WithNamespace(t.TestContext(), objectMeta.Namespace)
_, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, &api.DeleteOptions{})
@@ -450,7 +446,40 @@ func (t *Tester) TestDeleteGracefulExtend(existing runtime.Object, expectedGrace
}
}
func (t *Tester) testDeleteGracefulUsesZeroOnNil(existing runtime.Object, expectedGrace int64) {
func (t *Tester) TestDeleteGracefulImmediate(existing runtime.Object, expectedGrace int64, wasGracefulFn func() bool) {
objectMeta, err := api.ObjectMetaFor(existing)
if err != nil {
t.Fatalf("object does not have ObjectMeta: %v\n%#v", err, existing)
}
ctx := api.WithNamespace(t.TestContext(), objectMeta.Namespace)
_, err = t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, api.NewDeleteOptions(expectedGrace))
if err != nil {
t.Errorf("unexpected error: %v", err)
}
if !wasGracefulFn() {
t.Errorf("did not gracefully delete resource")
}
// second delete is immediate, resource is deleted
out, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, api.NewDeleteOptions(0))
if err != nil {
t.Errorf("unexpected error: %v", err)
}
_, err = t.storage.(rest.Getter).Get(ctx, objectMeta.Name)
if !errors.IsNotFound(err) {
t.Errorf("unexpected error, object should be deleted immediately: %v", err)
}
objectMeta, err = api.ObjectMetaFor(out)
if err != nil {
t.Errorf("unexpected error: %v", err)
return
}
if objectMeta.DeletionTimestamp == nil || objectMeta.DeletionGracePeriodSeconds == nil || *objectMeta.DeletionGracePeriodSeconds != 0 {
t.Errorf("unexpected deleted meta: %#v", objectMeta)
}
}
func (t *Tester) TestDeleteGracefulUsesZeroOnNil(existing runtime.Object, expectedGrace int64) {
objectMeta := t.getObjectMetaOrFail(existing)
ctx := api.WithNamespace(t.TestContext(), objectMeta.Namespace)
_, err := t.storage.(rest.GracefulDeleter).Delete(ctx, objectMeta.Name, nil)

2
pkg/api/validation/validation.go
View File

@@ -272,7 +272,7 @@ func ValidateObjectMetaUpdate(new, old *api.ObjectMeta) errs.ValidationErrorList
if old.DeletionGracePeriodSeconds != nil && new.DeletionGracePeriodSeconds == nil {
new.DeletionGracePeriodSeconds = old.DeletionGracePeriodSeconds
}
if new.DeletionGracePeriodSeconds != nil && *new.DeletionGracePeriodSeconds != *old.DeletionGracePeriodSeconds {
if new.DeletionGracePeriodSeconds != nil && old.DeletionGracePeriodSeconds != nil && *new.DeletionGracePeriodSeconds != *old.DeletionGracePeriodSeconds {
allErrs = append(allErrs, errs.NewFieldInvalid("deletionGracePeriodSeconds", new.DeletionGracePeriodSeconds, "field is immutable; may only be changed via deletion"))
}

27
pkg/api/validation/validation_test.go
View File

@@ -1317,6 +1317,9 @@ func TestValidatePod(t *testing.T) {
}
func TestValidatePodUpdate(t *testing.T) {
now := util.Now()
grace := int64(30)
grace2 := int64(31)
tests := []struct {
a api.Pod
b api.Pod
@@ -1403,6 +1406,30 @@ func TestValidatePodUpdate(t *testing.T) {
false,
"more containers",
},
{
api.Pod{
ObjectMeta: api.ObjectMeta{Name: "foo", DeletionTimestamp: &now},
Spec: api.PodSpec{Containers: []api.Container{{Image: "foo:V1"}}},
},
api.Pod{
ObjectMeta: api.ObjectMeta{Name: "foo"},
Spec: api.PodSpec{Containers: []api.Container{{Image: "foo:V1"}}},
},
true,
"deletion timestamp filled out",
},
{
api.Pod{
ObjectMeta: api.ObjectMeta{Name: "foo", DeletionTimestamp: &now, DeletionGracePeriodSeconds: &grace},
Spec: api.PodSpec{Containers: []api.Container{{Image: "foo:V1"}}},
},
api.Pod{
ObjectMeta: api.ObjectMeta{Name: "foo", DeletionTimestamp: &now, DeletionGracePeriodSeconds: &grace2},
Spec: api.PodSpec{Containers: []api.Container{{Image: "foo:V1"}}},
},
false,
"deletion grace period seconds cleared",
},
{
api.Pod{
ObjectMeta: api.ObjectMeta{Name: "foo"},

2
pkg/kubectl/describe.go
View File

@@ -419,7 +419,7 @@ func describePod(pod *api.Pod, rcs []api.ReplicationController, events *api.Even
fmt.Fprintf(out, "Labels:\t%s\n", formatLabels(pod.Labels))
if pod.DeletionTimestamp != nil {
fmt.Fprintf(out, "Status:\tTerminating (expires %s)\n", pod.DeletionTimestamp.Time.Format(time.RFC1123Z))
fmt.Fprintf(out, "Termination Grace Period:\t%ss\n", pod.DeletionGracePeriodSeconds)
fmt.Fprintf(out, "Termination Grace Period:\t%ds\n", pod.DeletionGracePeriodSeconds)
} else {
fmt.Fprintf(out, "Status:\t%s\n", string(pod.Status.Phase))
}

23
pkg/kubelet/dockertools/manager.go
View File

@@ -1234,14 +1234,19 @@ func (dm *DockerManager) killContainer(containerID types.UID, container *api.Con
}
gracePeriod := int64(minimumGracePeriodInSeconds)
if pod != nil && pod.DeletionGracePeriodSeconds != nil {
gracePeriod = *pod.DeletionGracePeriodSeconds
if pod != nil {
switch {
case pod.DeletionGracePeriodSeconds != nil:
gracePeriod = *pod.DeletionGracePeriodSeconds
case pod.Spec.TerminationGracePeriodSeconds != nil:
gracePeriod = *pod.Spec.TerminationGracePeriodSeconds
}
}
glog.V(2).Infof("Killing container %q with %d second grace period", name, gracePeriod)
start := util.Now()
if pod != nil && container != nil && container.Lifecycle != nil && container.Lifecycle.PreStop != nil {
glog.V(4).Infof("Running preStop hook for container %q", name)
start := util.Now()
// TODO: timebox PreStop execution to at most gracePeriod
if err := dm.runner.Run(ID, pod, container, container.Lifecycle.PreStop); err != nil {
glog.Errorf("preStop hook for container %q failed: %v", name, err)
@@ -1256,6 +1261,11 @@ func (dm *DockerManager) killContainer(containerID types.UID, container *api.Con
gracePeriod = minimumGracePeriodInSeconds
}
err := dm.client.StopContainer(ID, uint(gracePeriod))
if err == nil {
glog.V(2).Infof("Container %q exited after %s", name, util.Now().Sub(start.Time))
} else {
glog.V(2).Infof("Container %q termination failed after %s: %v", name, util.Now().Sub(start.Time), err)
}
ref, ok := dm.containerRefManager.GetRef(ID)
if !ok {
glog.Warningf("No ref for pod '%q'", name)
@@ -1498,11 +1508,6 @@ func (dm *DockerManager) computePodContainerChanges(pod *api.Pod, runningPod kub
containersToKeep := make(map[kubeletTypes.DockerID]int)
createPodInfraContainer := false
if pod.DeletionTimestamp != nil {
glog.V(4).Infof("Pod is terminating %q", podFullName)
return PodContainerChangesSpec{}, nil
}
var err error
var podInfraContainerID kubeletTypes.DockerID
var changed bool
@@ -1624,10 +1629,10 @@ func (dm *DockerManager) SyncPod(pod *api.Pod, runningPod kubecontainer.Pod, pod
podFullName := kubecontainer.GetPodFullName(pod)
containerChanges, err := dm.computePodContainerChanges(pod, runningPod, podStatus)
glog.V(3).Infof("Got container changes for pod %q: %+v", podFullName, containerChanges)
if err != nil {
return err
}
glog.V(3).Infof("Got container changes for pod %q: %+v", podFullName, containerChanges)
if containerChanges.StartInfraContainer || (len(containerChanges.ContainersToKeep) == 0 && len(containerChanges.ContainersToStart) == 0) {
if len(containerChanges.ContainersToKeep) == 0 && len(containerChanges.ContainersToStart) == 0 {

4
pkg/kubelet/dockertools/manager_test.go
View File

@@ -568,7 +568,7 @@ func replaceProber(dm *DockerManager, result probe.Result, err error) {
// Unknown or error.
//
// PLEASE READ THE PROBE DOCS BEFORE CHANGING THIS TEST IF YOU ARE UNSURE HOW PROBES ARE SUPPOSED TO WORK:
// (See https://github.com/GoogleCloudPlatform/kubernetes/blob/master/docs/pod-states.md#pod-conditions)
// (See https://k8s.io/kubernetes/blob/master/docs/pod-states.md#pod-conditions)
func TestProbeContainer(t *testing.T) {
manager, _ := newTestDockerManager()
dc := &docker.APIContainers{
@@ -1150,7 +1150,7 @@ func TestSyncPodBadHash(t *testing.T) {
// Check the pod infra container.
"inspect_container",
// Kill and restart the bad hash container.
"stop", "create", "start",
"stop", "create", "start", "inspect_container",
})
if err := fakeDocker.AssertStopped([]string{"1234"}); err != nil {

7
pkg/kubelet/kubelet.go
View File

@@ -1177,9 +1177,10 @@ func (kl *Kubelet) syncPod(pod *api.Pod, mirrorPod *api.Pod, runningPod kubecont
}()
// Kill pods we can't run.
err := canRunPod(pod)
if err != nil {
kl.killPod(pod, runningPod)
if err := canRunPod(pod); err != nil || pod.DeletionTimestamp != nil {
if err := kl.killPod(pod, runningPod); err != nil {
util.HandleError(err)
}
return err
}

15
pkg/kubelet/status_manager.go
View File

@@ -123,7 +123,7 @@ func (s *statusManager) SetPodStatus(pod *api.Pod, status api.PodStatus) {
// Currently this routine is not called for the same pod from multiple
// workers and/or the kubelet but dropping the lock before sending the
// status down the channel feels like an easy way to get a bullet in foot.
if !found || !isStatusEqual(&oldStatus, &status) {
if !found || !isStatusEqual(&oldStatus, &status) || pod.DeletionTimestamp != nil {
s.podStatuses[podFullName] = status
s.podStatusChannel <- podStatusSyncRequest{pod, status}
} else {
@@ -173,11 +173,16 @@ func (s *statusManager) syncBatch() error {
if err == nil {
glog.V(3).Infof("Status for pod %q updated successfully", kubeletUtil.FormatPodName(pod))
if statusPod.DeletionTimestamp == nil || !allTerminated(statusPod.Status.ContainerStatuses) {
if pod.DeletionTimestamp == nil {
return nil
}
if !notRunning(pod.Status.ContainerStatuses) {
glog.V(3).Infof("Pod %q is terminated, but some pods are still running", pod.Name)
return nil
}
if err := s.kubeClient.Pods(statusPod.Namespace).Delete(statusPod.Name, api.NewDeleteOptions(0)); err == nil {
glog.V(3).Infof("Pod %q fully terminated and removed from etcd", statusPod.Name)
s.DeletePodStatus(podFullName)
return nil
}
}
@@ -194,11 +199,11 @@ func (s *statusManager) syncBatch() error {
return fmt.Errorf("error updating status for pod %q: %v", kubeletUtil.FormatPodName(pod), err)
}
// allTerminated returns true if every status is terminated, or the status list
// notRunning returns true if every status is terminated or waiting, or the status list
// is empty.
func allTerminated(statuses []api.ContainerStatus) bool {
func notRunning(statuses []api.ContainerStatus) bool {
for _, status := range statuses {
if status.State.Terminated == nil {
if status.State.Terminated == nil && status.State.Waiting == nil {
return false
}
}

39
pkg/registry/generic/etcd/etcd.go
View File

@@ -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

13
pkg/registry/pod/etcd/etcd_test.go
View File

@@ -121,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{
@@ -137,9 +139,18 @@ func TestDelete(t *testing.T) {
if fakeEtcdClient.Data[key].R.Node == nil {
return false
}
return fakeEtcdClient.Data[key].R.Node.TTL != 0
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)
}
func expectPod(t *testing.T, out runtime.Object) (*api.Pod, bool) {

18
pkg/registry/pod/rest.go
View File

@@ -92,22 +92,38 @@ func (podStrategy) CheckGracefulDelete(obj runtime.Object, options *api.DeleteOp
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)
pod := obj.(*api.Pod)
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
}