Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support graceful deletion of resources #5085

Merged
merged 1 commit into from
Mar 20, 2015
+580 −93
Merged

Support graceful deletion of resources #5085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions pkg/api/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,11 @@ var standardResources = util.NewStringSet(
func IsStandardResourceName(str string) bool {
return standardResources.Has(str)
}

// NewDeleteOptions returns a DeleteOptions indicating the resource should
// be deleted within the specified grace period. Use zero to indicate
// immediate deletion. If you would prefer to use the default grace period,
// use &api.DeleteOptions{} directly.
func NewDeleteOptions(grace int64) *DeleteOptions {
return &DeleteOptions{GracePeriodSeconds: &grace}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why &grace?

}
2 changes: 2 additions & 0 deletions pkg/api/register.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ func init() {
&NamespaceList{},
&Secret{},
&SecretList{},
&DeleteOptions{},
)
// Legacy names are supported
Scheme.AddKnownTypeWithName("", "Minion", &Node{})
Expand Down Expand Up @@ -85,3 +86,4 @@ func (*Namespace) IsAnAPIObject() {}
func (*NamespaceList) IsAnAPIObject() {}
func (*Secret) IsAnAPIObject() {}
func (*SecretList) IsAnAPIObject() {}
func (*DeleteOptions) IsAnAPIObject() {}
51 changes: 51 additions & 0 deletions pkg/api/rest/delete.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
Copyright 2014 Google Inc. All rights reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package rest

import (
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
)

// RESTDeleteStrategy defines deletion behavior on an object that follows Kubernetes
// API conventions.
type RESTDeleteStrategy interface {
runtime.ObjectTyper

// CheckGracefulDelete should return true if the object can be gracefully deleted and set
// any default values on the DeleteOptions.
CheckGracefulDelete(obj runtime.Object, options *api.DeleteOptions) bool
}

// BeforeDelete tests whether the object can be gracefully deleted. If graceful is set the object
// should be gracefully deleted, if gracefulPending is set the object has already been gracefully deleted
// (and the provided grace period is longer than the time to deletion), and an error is returned if the
// condition cannot be checked or the gracePeriodSeconds is invalid. The options argument may be updated with
// default values if graceful is true.
func BeforeDelete(strategy RESTDeleteStrategy, ctx api.Context, obj runtime.Object, options *api.DeleteOptions) (graceful, gracefulPending bool, err error) {
if strategy == nil {
return false, false, nil
}
_, _, kerr := objectMetaAndKind(strategy, obj)
if kerr != nil {
return false, false, kerr
}
if !strategy.CheckGracefulDelete(obj, options) {
return false, false, nil
}
return true, false, nil
}
60 changes: 60 additions & 0 deletions pkg/api/rest/resttest/resttest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,3 +196,63 @@ func (t *Tester) TestCreateRejectsNamespace(valid runtime.Object) {
t.Errorf("Expected 'Controller.Namespace does not match the provided context' error, got '%v'", err.Error())
}
}

func (t *Tester) TestDeleteGraceful(createFn func() runtime.Object, expectedGrace int64, wasGracefulFn func() bool) {
t.TestDeleteGracefulHasDefault(createFn(), expectedGrace, wasGracefulFn)
t.TestDeleteGracefulUsesZeroOnNil(createFn(), 0)
}

func (t *Tester) TestDeleteNoGraceful(createFn func() runtime.Object, wasGracefulFn func() bool) {
existing := createFn()
objectMeta, err := api.ObjectMetaFor(existing)
if err != nil {
t.Fatalf("object does not have ObjectMeta: %v\n%#v", err, existing)
}

ctx := api.WithNamespace(api.NewContext(), objectMeta.Namespace)
_, err = t.storage.(apiserver.RESTGracefulDeleter).Delete(ctx, objectMeta.Name, api.NewDeleteOptions(10))
if err != nil {
t.Errorf("unexpected error: %v", err)
}
if _, err := t.storage.(apiserver.RESTGetter).Get(ctx, objectMeta.Name); !errors.IsNotFound(err) {
t.Errorf("unexpected error, object should not exist: %v", err)
}
if wasGracefulFn() {
t.Errorf("resource should not support graceful delete")
}
}

func (t *Tester) TestDeleteGracefulHasDefault(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(api.NewContext(), objectMeta.Namespace)
_, err = t.storage.(apiserver.RESTGracefulDeleter).Delete(ctx, objectMeta.Name, &api.DeleteOptions{})
if err != nil {
t.Errorf("unexpected error: %v", err)
}
if _, err := t.storage.(apiserver.RESTGetter).Get(ctx, objectMeta.Name); err != nil {
t.Errorf("unexpected error, object should exist: %v", err)
}
if !wasGracefulFn() {
t.Errorf("did not gracefully delete resource")
}
}

func (t *Tester) TestDeleteGracefulUsesZeroOnNil(existing runtime.Object, expectedGrace int64) {
objectMeta, err := api.ObjectMetaFor(existing)
if err != nil {
t.Fatalf("object does not have ObjectMeta: %v\n%#v", err, existing)
}

ctx := api.WithNamespace(api.NewContext(), objectMeta.Namespace)
_, err = t.storage.(apiserver.RESTGracefulDeleter).Delete(ctx, objectMeta.Name, nil)
if err != nil {
t.Errorf("unexpected error: %v", err)
}
if _, err := t.storage.(apiserver.RESTGetter).Get(ctx, objectMeta.Name); !errors.IsNotFound(err) {
t.Errorf("unexpected error, object should exist: %v", err)
}
}
21 changes: 21 additions & 0 deletions pkg/api/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,17 @@ type ObjectMeta struct {
// Clients may not set this value. It is represented in RFC3339 form and is in UTC.
CreationTimestamp util.Time `json:"creationTimestamp,omitempty"`

// DeletionTimestamp is the time after which this resource will be deleted. This
// field is set by the server when a graceful deletion is requested by the user, and is not
// directly settable by a client. The resource will be deleted (no longer visible from
// resource lists, and not reachable by name) after the time in this field. Once set, this
// value may not be unset or be set further into the future, although it may be shortened
// or the resource may be deleted prior to this time. For example, a user may request that
// a pod is deleted in 30 seconds. The Kubelet will react by sending a graceful termination
// signal to the containers in the pod. Once the resource is deleted in the API, the Kubelet
// will send a hard termination signal to the container.
DeletionTimestamp *util.Time `json:"deletionTimestamp,omitempty"`

// Labels are key value pairs that may be used to scope and select individual resources.
// Label keys are of the form:
// label-key ::= prefixed-name | name
Expand Down Expand Up @@ -995,6 +1006,16 @@ type Binding struct {
Target ObjectReference `json:"target"`
}

// DeleteOptions may be provided when deleting an API object
type DeleteOptions struct {
TypeMeta `json:",inline"`

// Optional duration in seconds before the object should be deleted. Value must be non-negative integer.
// The value zero indicates delete immediately. If this value is nil, the default grace period for the
// specified type will be used.
GracePeriodSeconds *int64 `json:"gracePeriodSeconds"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a pointer so we can distinguish Go defaulting from the user specifying 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. All the other constructs were more awkward (struct wrapped type, etc).

On Mar 18, 2015, at 8:38 PM, Brian Grant notifications@github.com wrote:

In pkg/api/types.go:

@@ -983,6 +994,16 @@ type Binding struct {
Target ObjectReference json:"target"
}

+// DeleteOptions may be provided when deleting an API object
+type DeleteOptions struct {

  • TypeMeta json:",inline"
  • // Optional duration in seconds before the object should be deleted. Value must be non-negative integer.
  • // The value zero indicates delete immediately. If this value is nil, the default grace period for the
  • // specified type will be used.
  • GracePeriodSeconds *int64 json:"gracePeriodSeconds"
    This is a pointer so we can distinguish Go defaulting from the user specifying 0?

—
Reply to this email directly or view it on GitHub.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brainstorming other options:

  • -1 to imply either immediate deletion or default grace period
  • use a string
  • use time.Duration

I maybe could get on board with time.Duration (here and elsewhere). It would be similar in spirit to the resource quantity type.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current approach is acceptable, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's versioned so before we introduce the first consumer we can rev it.

}

// Status is a return value for calls that don't return other objects.
// TODO: this could go in apiserver, but I'm including it here so clients needn't
// import both.
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/v1beta1/conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ func init() {
out.GenerateName = in.GenerateName
out.UID = in.UID
out.CreationTimestamp = in.CreationTimestamp
out.DeletionTimestamp = in.DeletionTimestamp
out.SelfLink = in.SelfLink
if len(in.ResourceVersion) > 0 {
v, err := strconv.ParseUint(in.ResourceVersion, 10, 64)
Expand All @@ -100,6 +101,7 @@ func init() {
out.GenerateName = in.GenerateName
out.UID = in.UID
out.CreationTimestamp = in.CreationTimestamp
out.DeletionTimestamp = in.DeletionTimestamp
out.SelfLink = in.SelfLink
if in.ResourceVersion != 0 {
out.ResourceVersion = strconv.FormatUint(in.ResourceVersion, 10)
Expand Down
11 changes: 11 additions & 0 deletions pkg/api/v1beta1/conversion_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,17 @@ import (

var Convert = newer.Scheme.Convert

func TestEmptyObjectConversion(t *testing.T) {
s, err := current.Codec.Encode(&current.LimitRange{})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
// DeletionTimestamp is not included, while CreationTimestamp is (would always be set)
if string(s) != `{"kind":"LimitRange","creationTimestamp":null,"apiVersion":"v1beta1","spec":{"limits":null}}` {
t.Errorf("unexpected empty object: %s", string(s))
}
}

func TestNodeConversion(t *testing.T) {
version, kind, err := newer.Scheme.ObjectVersionAndKind(&current.Minion{})
if err != nil {
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/v1beta1/register.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ func init() {
&NamespaceList{},
&Secret{},
&SecretList{},
&DeleteOptions{},
)
// Future names are supported
api.Scheme.AddKnownTypeWithName("v1beta1", "Node", &Minion{})
Expand Down Expand Up @@ -92,3 +93,4 @@ func (*Namespace) IsAnAPIObject() {}
func (*NamespaceList) IsAnAPIObject() {}
func (*Secret) IsAnAPIObject() {}
func (*SecretList) IsAnAPIObject() {}
func (*DeleteOptions) IsAnAPIObject() {}
21 changes: 21 additions & 0 deletions pkg/api/v1beta1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,17 @@ type TypeMeta struct {
APIVersion string `json:"apiVersion,omitempty" description:"version of the schema the object should have"`
Namespace string `json:"namespace,omitempty" description:"namespace to which the object belongs; must be a DNS_SUBDOMAIN; 'default' by default; cannot be updated"`

// DeletionTimestamp is the time after which this resource will be deleted. This
// field is set by the server when a graceful deletion is requested by the user, and is not
// directly settable by a client. The resource will be deleted (no longer visible from
// resource lists, and not reachable by name) after the time in this field. Once set, this
// value may not be unset or be set further into the future, although it may be shortened
// or the resource may be deleted prior to this time. For example, a user may request that
// a pod is deleted in 30 seconds. The Kubelet will react by sending a graceful termination
// signal to the containers in the pod. Once the resource is deleted in the API, the Kubelet
// will send a hard termination signal to the container.
DeletionTimestamp *util.Time `json:"deletionTimestamp,omitempty" description:"RFC 3339 date and time at which the object will be deleted; populated by the system when a graceful deletion is requested, read-only; if not set, graceful deletion of the object has not been requested"`

// GenerateName indicates that the name should be made unique by the server prior to persisting
// it. A non-empty value for the field indicates the name will be made unique (and the name
// returned to the client will be different than the name passed). The value of this field will
Expand Down Expand Up @@ -813,6 +824,16 @@ type Binding struct {
Host string `json:"host" description:"host to which to bind the specified pod"`
}

// DeleteOptions may be provided when deleting an API object
type DeleteOptions struct {
TypeMeta `json:",inline"`

// Optional duration in seconds before the object should be deleted. Value must be non-negative integer.
// The value zero indicates delete immediately. If this value is nil, the default grace period for the
// specified type will be used.
GracePeriodSeconds *int64 `json:"gracePeriodSeconds" description:"the duration in seconds to wait before deleting this object; defaults to a per object value if not specified; zero means delete immediately"`
}

// Status is a return value for calls that don't return other objects.
// TODO: this could go in apiserver, but I'm including it here so clients needn't
// import both.
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/v1beta2/conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ func init() {
out.GenerateName = in.GenerateName
out.UID = in.UID
out.CreationTimestamp = in.CreationTimestamp
out.DeletionTimestamp = in.DeletionTimestamp
out.SelfLink = in.SelfLink
if len(in.ResourceVersion) > 0 {
v, err := strconv.ParseUint(in.ResourceVersion, 10, 64)
Expand All @@ -100,6 +101,7 @@ func init() {
out.GenerateName = in.GenerateName
out.UID = in.UID
out.CreationTimestamp = in.CreationTimestamp
out.DeletionTimestamp = in.DeletionTimestamp
out.SelfLink = in.SelfLink
if in.ResourceVersion != 0 {
out.ResourceVersion = strconv.FormatUint(in.ResourceVersion, 10)
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/v1beta2/register.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ func init() {
&NamespaceList{},
&Secret{},
&SecretList{},
&DeleteOptions{},
)
// Future names are supported
api.Scheme.AddKnownTypeWithName("v1beta2", "Node", &Minion{})
Expand Down Expand Up @@ -92,3 +93,4 @@ func (*Namespace) IsAnAPIObject() {}
func (*NamespaceList) IsAnAPIObject() {}
func (*Secret) IsAnAPIObject() {}
func (*SecretList) IsAnAPIObject() {}
func (*DeleteOptions) IsAnAPIObject() {}
21 changes: 21 additions & 0 deletions pkg/api/v1beta2/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,17 @@ type TypeMeta struct {
APIVersion string `json:"apiVersion,omitempty" description:"version of the schema the object should have"`
Namespace string `json:"namespace,omitempty" description:"namespace to which the object belongs; must be a DNS_SUBDOMAIN; 'default' by default; cannot be updated"`

// DeletionTimestamp is the time after which this resource will be deleted. This
// field is set by the server when a graceful deletion is requested by the user, and is not
// directly settable by a client. The resource will be deleted (no longer visible from
// resource lists, and not reachable by name) after the time in this field. Once set, this
// value may not be unset or be set further into the future, although it may be shortened
// or the resource may be deleted prior to this time. For example, a user may request that
// a pod is deleted in 30 seconds. The Kubelet will react by sending a graceful termination
// signal to the containers in the pod. Once the resource is deleted in the API, the Kubelet
// will send a hard termination signal to the container.
DeletionTimestamp *util.Time `json:"deletionTimestamp,omitempty" description:"RFC 3339 date and time at which the object will be deleted; populated by the system when a graceful deletion is requested, read-only; if not set, graceful deletion of the object has not been requested"`

// GenerateName indicates that the name should be made unique by the server prior to persisting
// it. A non-empty value for the field indicates the name will be made unique (and the name
// returned to the client will be different than the name passed). The value of this field will
Expand Down Expand Up @@ -831,6 +842,16 @@ type Binding struct {
Host string `json:"host" description:"host to which to bind the specified pod"`
}

// DeleteOptions may be provided when deleting an API object
type DeleteOptions struct {
TypeMeta `json:",inline"`

// Optional duration in seconds before the object should be deleted. Value must be non-negative integer.
// The value zero indicates delete immediately. If this value is nil, the default grace period for the
// specified type will be used.
GracePeriodSeconds *int64 `json:"gracePeriodSeconds" description:"the duration in seconds to wait before deleting this object; defaults to a per object value if not specified; zero means delete immediately"`
}

// Status is a return value for calls that don't return other objects.
// TODO: this could go in apiserver, but I'm including it here so clients needn't
// import both.
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/v1beta3/register.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ func init() {
&NamespaceList{},
&Secret{},
&SecretList{},
&DeleteOptions{},
)
// Legacy names are supported
api.Scheme.AddKnownTypeWithName("v1beta3", "Minion", &Node{})
Expand Down Expand Up @@ -86,3 +87,4 @@ func (*Namespace) IsAnAPIObject() {}
func (*NamespaceList) IsAnAPIObject() {}
func (*Secret) IsAnAPIObject() {}
func (*SecretList) IsAnAPIObject() {}
func (*DeleteOptions) IsAnAPIObject() {}
21 changes: 21 additions & 0 deletions pkg/api/v1beta3/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,17 @@ type ObjectMeta struct {
// Clients may not set this value. It is represented in RFC3339 form and is in UTC.
CreationTimestamp util.Time `json:"creationTimestamp,omitempty" description:"RFC 3339 date and time at which the object was created; populated by the system, read-only; null for lists"`

// DeletionTimestamp is the time after which this resource will be deleted. This
// field is set by the server when a graceful deletion is requested by the user, and is not
// directly settable by a client. The resource will be deleted (no longer visible from
// resource lists, and not reachable by name) after the time in this field. Once set, this
// value may not be unset or be set further into the future, although it may be shortened
// or the resource may be deleted prior to this time. For example, a user may request that
// a pod is deleted in 30 seconds. The Kubelet will react by sending a graceful termination
// signal to the containers in the pod. Once the resource is deleted in the API, the Kubelet
// will send a hard termination signal to the container.
DeletionTimestamp *util.Time `json:"deletionTimestamp,omitempty" description:"RFC 3339 date and time at which the object will be deleted; populated by the system when a graceful deletion is requested, read-only; if not set, graceful deletion of the object has not been requested"`

// Labels are key value pairs that may be used to scope and select individual resources.
// TODO: replace map[string]string with labels.LabelSet type
Labels map[string]string `json:"labels,omitempty" description:"map of string keys and values that can be used to organize and categorize objects; may match selectors of replication controllers and services"`
Expand Down Expand Up @@ -982,6 +993,16 @@ type Binding struct {
Target ObjectReference `json:"target" description:"an object to bind to"`
}

// DeleteOptions may be provided when deleting an API object
type DeleteOptions struct {
TypeMeta `json:",inline"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind and version of the object being deleted? But not namespace, name, etc.? Why are the kind and version needed? I suppose even Status has TypeMeta and ListMeta, but I don't think we need to be able to serialize DeleteOptions, such as in config files.

Actually, it would be nice to at least be able to specify uid and resourceVersion, to facilitate preconditions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind and version are needed so that we can properly version this struct when it comes in from the API. All objects have to be versioned.

I'll add ObjectMeta so this is consistent with Bindings.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it has to be versioned, but I imagine we'll typically just infer the version from the URL path.

I'm fine with ObjectMeta, however.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ObjectMeta not needed for this PR.


// Optional duration in seconds before the object should be deleted. Value must be non-negative integer.
// The value zero indicates delete immediately. If this value is nil, the default grace period for the
// specified type will be used.
GracePeriodSeconds *int64 `json:"gracePeriodSeconds" description:"the duration in seconds to wait before deleting this object; defaults to a per object value if not specified; zero means delete immediately"`
}

// Status is a return value for calls that don't return other objects.
type Status struct {
TypeMeta `json:",inline"`
Expand Down
Loading