From cb2207c2fbddba7f0cabb626ecb7b02370ca9faa Mon Sep 17 00:00:00 2001 From: Morten Torkildsen Date: Sat, 30 Mar 2019 18:11:03 -0700 Subject: [PATCH] fix(helm): Delete hooks should wait for resource to be removed from etcd before continuing Signed-off-by: Morten Torkildsen --- _proto/hapi/release/hook.proto | 2 + docs/charts_hooks.md | 4 + pkg/hooks/hooks.go | 2 + pkg/kube/client.go | 42 +++++++- pkg/kube/client_test.go | 48 ++++++++++ pkg/proto/hapi/release/hook.pb.go | 90 +++++++++-------- pkg/tiller/environment/environment.go | 18 ++++ pkg/tiller/environment/environment_test.go | 3 + pkg/tiller/hooks.go | 15 +++ pkg/tiller/hooks_test.go | 106 +++++++++++++++++++++ pkg/tiller/release_server.go | 3 +- pkg/tiller/release_server_test.go | 7 ++ 12 files changed, 298 insertions(+), 42 deletions(-) diff --git a/_proto/hapi/release/hook.proto b/_proto/hapi/release/hook.proto index cf7e25bf6..77fbbe5d5 100644 --- a/_proto/hapi/release/hook.proto +++ b/_proto/hapi/release/hook.proto @@ -56,4 +56,6 @@ message Hook { int32 weight = 7; // DeletePolicies are the policies that indicate when to delete the hook repeated DeletePolicy delete_policies = 8; + // DeleteTimeout indicates how long to wait for a resource to be deleted before timing out + int64 delete_timeout = 9; } diff --git a/docs/charts_hooks.md b/docs/charts_hooks.md index 3044414c3..1a9341d5c 100644 --- a/docs/charts_hooks.md +++ b/docs/charts_hooks.md @@ -199,6 +199,10 @@ You can choose one or more defined annotation values: * `"hook-failed"` specifies Tiller should delete the hook if the hook failed during execution. * `"before-hook-creation"` specifies Tiller should delete the previous hook before the new hook is launched. +By default Tiller will wait for 60 seconds for a deleted hook to no longer exist in the API server before timing out. This +behavior can be changed using the `helm.sh/hook-delete-timeout` annotation. The value is the number of seconds Tiller +should wait for the hook to be fully deleted. A value of 0 means Tiller does not wait at all. + ### Defining a CRD with the `crd-install` Hook Custom Resource Definitions (CRDs) are a special kind in Kubernetes. They provide diff --git a/pkg/hooks/hooks.go b/pkg/hooks/hooks.go index 13a09b08b..6d60fad51 100644 --- a/pkg/hooks/hooks.go +++ b/pkg/hooks/hooks.go @@ -27,6 +27,8 @@ const ( HookWeightAnno = "helm.sh/hook-weight" // HookDeleteAnno is the label name for the delete policy for a hook HookDeleteAnno = "helm.sh/hook-delete-policy" + // HookDeleteTimeoutAnno is the label name for the timeout value for delete policies + HookDeleteTimeoutAnno = "helm.sh/hook-delete-timeout" ) // Types of hooks diff --git a/pkg/kube/client.go b/pkg/kube/client.go index aa025eb0a..b9c7f8a96 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -449,15 +449,34 @@ func (c *Client) cleanup(newlyCreatedResources []*resource.Info) (cleanupErrors // // Namespace will set the namespace. func (c *Client) Delete(namespace string, reader io.Reader) error { + return c.DeleteWithTimeout(namespace, reader, 0, false) +} + +// DeleteWithTimeout deletes Kubernetes resources from an io.reader. If shouldWait is true, the function +// will wait for all resources to be deleted from etcd before returning, or when the timeout +// has expired. +// +// Namespace will set the namespace. +func (c *Client) DeleteWithTimeout(namespace string, reader io.Reader, timeout int64, shouldWait bool) error { infos, err := c.BuildUnstructured(namespace, reader) if err != nil { return err } - return perform(infos, func(info *resource.Info) error { + err = perform(infos, func(info *resource.Info) error { c.Log("Starting delete for %q %s", info.Name, info.Mapping.GroupVersionKind.Kind) err := deleteResource(info) return c.skipIfNotFound(err) }) + if err != nil { + return err + } + + if shouldWait { + c.Log("Waiting for %d seconds for delete to be completed", timeout) + return waitUntilAllResourceDeleted(infos, time.Duration(timeout)*time.Second) + } + + return nil } func (c *Client) skipIfNotFound(err error) error { @@ -468,6 +487,27 @@ func (c *Client) skipIfNotFound(err error) error { return err } +func waitUntilAllResourceDeleted(infos Result, timeout time.Duration) error { + return wait.Poll(2*time.Second, timeout, func() (bool, error) { + allDeleted := true + err := perform(infos, func(info *resource.Info) error { + innerErr := info.Get() + if errors.IsNotFound(innerErr) { + return nil + } + if innerErr != nil { + return innerErr + } + allDeleted = false + return nil + }) + if err != nil { + return false, err + } + return allDeleted, nil + }) +} + func (c *Client) watchTimeout(t time.Duration) ResourceActorFunc { return func(info *resource.Info) error { return c.watchUntilReady(t, info) diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go index 810abdf17..0c280f3e5 100644 --- a/pkg/kube/client_test.go +++ b/pkg/kube/client_test.go @@ -283,6 +283,54 @@ func TestUpdateNonManagedResourceError(t *testing.T) { } } +func TestDeleteWithTimeout(t *testing.T) { + testCases := map[string]struct { + deleteTimeout int64 + deleteAfter time.Duration + success bool + }{ + "resource is deleted within timeout period": { + int64((2 * time.Minute).Seconds()), + 10 * time.Second, + true, + }, + "resource is not deleted within the timeout period": { + int64((10 * time.Second).Seconds()), + 20 * time.Second, + false, + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + c := newTestClient() + defer c.Cleanup() + + service := newService("my-service") + startTime := time.Now() + c.TestFactory.UnstructuredClient = &fake.RESTClient{ + GroupVersion: schema.GroupVersion{Version: "v1"}, + NegotiatedSerializer: unstructuredSerializer, + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + currentTime := time.Now() + if startTime.Add(tc.deleteAfter).Before(currentTime) { + return newResponse(404, notFoundBody()) + } + return newResponse(200, &service) + }), + } + + err := c.DeleteWithTimeout(metav1.NamespaceDefault, strings.NewReader(testServiceManifest), tc.deleteTimeout, true) + if err != nil && tc.success { + t.Errorf("expected no error, but got %v", err) + } + if err == nil && !tc.success { + t.Errorf("expected error, but didn't get one") + } + }) + } +} + func TestBuild(t *testing.T) { tests := []struct { name string diff --git a/pkg/proto/hapi/release/hook.pb.go b/pkg/proto/hapi/release/hook.pb.go index bec2049b6..2faf756d7 100644 --- a/pkg/proto/hapi/release/hook.pb.go +++ b/pkg/proto/hapi/release/hook.pb.go @@ -69,7 +69,7 @@ func (x Hook_Event) String() string { return proto.EnumName(Hook_Event_name, int32(x)) } func (Hook_Event) EnumDescriptor() ([]byte, []int) { - return fileDescriptor_hook_8076b1a80af16030, []int{0, 0} + return fileDescriptor_hook_e64400ca8195038e, []int{0, 0} } type Hook_DeletePolicy int32 @@ -95,7 +95,7 @@ func (x Hook_DeletePolicy) String() string { return proto.EnumName(Hook_DeletePolicy_name, int32(x)) } func (Hook_DeletePolicy) EnumDescriptor() ([]byte, []int) { - return fileDescriptor_hook_8076b1a80af16030, []int{0, 1} + return fileDescriptor_hook_e64400ca8195038e, []int{0, 1} } // Hook defines a hook object. @@ -114,17 +114,19 @@ type Hook struct { // Weight indicates the sort order for execution among similar Hook type Weight int32 `protobuf:"varint,7,opt,name=weight,proto3" json:"weight,omitempty"` // DeletePolicies are the policies that indicate when to delete the hook - DeletePolicies []Hook_DeletePolicy `protobuf:"varint,8,rep,packed,name=delete_policies,json=deletePolicies,proto3,enum=hapi.release.Hook_DeletePolicy" json:"delete_policies,omitempty"` - XXX_NoUnkeyedLiteral struct{} `json:"-"` - XXX_unrecognized []byte `json:"-"` - XXX_sizecache int32 `json:"-"` + DeletePolicies []Hook_DeletePolicy `protobuf:"varint,8,rep,packed,name=delete_policies,json=deletePolicies,proto3,enum=hapi.release.Hook_DeletePolicy" json:"delete_policies,omitempty"` + // DeleteTimeout indicates how long to wait for a resource to be deleted before timing out + DeleteTimeout int64 `protobuf:"varint,9,opt,name=delete_timeout,json=deleteTimeout,proto3" json:"delete_timeout,omitempty"` + XXX_NoUnkeyedLiteral struct{} `json:"-"` + XXX_unrecognized []byte `json:"-"` + XXX_sizecache int32 `json:"-"` } func (m *Hook) Reset() { *m = Hook{} } func (m *Hook) String() string { return proto.CompactTextString(m) } func (*Hook) ProtoMessage() {} func (*Hook) Descriptor() ([]byte, []int) { - return fileDescriptor_hook_8076b1a80af16030, []int{0} + return fileDescriptor_hook_e64400ca8195038e, []int{0} } func (m *Hook) XXX_Unmarshal(b []byte) error { return xxx_messageInfo_Hook.Unmarshal(m, b) @@ -200,43 +202,51 @@ func (m *Hook) GetDeletePolicies() []Hook_DeletePolicy { return nil } +func (m *Hook) GetDeleteTimeout() int64 { + if m != nil { + return m.DeleteTimeout + } + return 0 +} + func init() { proto.RegisterType((*Hook)(nil), "hapi.release.Hook") proto.RegisterEnum("hapi.release.Hook_Event", Hook_Event_name, Hook_Event_value) proto.RegisterEnum("hapi.release.Hook_DeletePolicy", Hook_DeletePolicy_name, Hook_DeletePolicy_value) } -func init() { proto.RegisterFile("hapi/release/hook.proto", fileDescriptor_hook_8076b1a80af16030) } - -var fileDescriptor_hook_8076b1a80af16030 = []byte{ - // 453 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x6c, 0x91, 0x51, 0x8f, 0x9a, 0x40, - 0x10, 0x80, 0x8f, 0x53, 0x41, 0x47, 0xcf, 0xdb, 0x6e, 0x9a, 0x76, 0xe3, 0xcb, 0x19, 0x9f, 0x7c, - 0xc2, 0xe6, 0x9a, 0xfe, 0x00, 0x84, 0xb9, 0x6a, 0x24, 0x60, 0x16, 0x4c, 0x93, 0xbe, 0x10, 0xae, - 0xee, 0x29, 0x11, 0x81, 0x08, 0xb6, 0xe9, 0x1f, 0xed, 0x3f, 0xe8, 0xff, 0x68, 0x76, 0x45, 0x7a, - 0x49, 0xfb, 0x36, 0xf3, 0xcd, 0xb7, 0xb3, 0x33, 0xbb, 0xf0, 0x7e, 0x1f, 0x17, 0xc9, 0xec, 0x24, - 0x52, 0x11, 0x97, 0x62, 0xb6, 0xcf, 0xf3, 0x83, 0x59, 0x9c, 0xf2, 0x2a, 0xa7, 0x03, 0x59, 0x30, - 0xeb, 0xc2, 0xe8, 0x61, 0x97, 0xe7, 0xbb, 0x54, 0xcc, 0x54, 0xed, 0xf9, 0xfc, 0x32, 0xab, 0x92, - 0xa3, 0x28, 0xab, 0xf8, 0x58, 0x5c, 0xf4, 0xc9, 0xaf, 0x36, 0xb4, 0x17, 0x79, 0x7e, 0xa0, 0x14, - 0xda, 0x59, 0x7c, 0x14, 0x4c, 0x1b, 0x6b, 0xd3, 0x1e, 0x57, 0xb1, 0x64, 0x87, 0x24, 0xdb, 0xb2, - 0xdb, 0x0b, 0x93, 0xb1, 0x64, 0x45, 0x5c, 0xed, 0x59, 0xeb, 0xc2, 0x64, 0x4c, 0x47, 0xd0, 0x3d, - 0xc6, 0x59, 0xf2, 0x22, 0xca, 0x8a, 0xb5, 0x15, 0x6f, 0x72, 0xfa, 0x01, 0x74, 0xf1, 0x5d, 0x64, - 0x55, 0xc9, 0x3a, 0xe3, 0xd6, 0x74, 0xf8, 0xc8, 0xcc, 0xd7, 0x03, 0x9a, 0xf2, 0x6e, 0x13, 0xa5, - 0xc0, 0x6b, 0x8f, 0x7e, 0x82, 0x6e, 0x1a, 0x97, 0x55, 0x74, 0x3a, 0x67, 0x4c, 0x1f, 0x6b, 0xd3, - 0xfe, 0xe3, 0xc8, 0xbc, 0xac, 0x61, 0x5e, 0xd7, 0x30, 0xc3, 0xeb, 0x1a, 0xdc, 0x90, 0x2e, 0x3f, - 0x67, 0xf4, 0x1d, 0xe8, 0x3f, 0x44, 0xb2, 0xdb, 0x57, 0xcc, 0x18, 0x6b, 0xd3, 0x0e, 0xaf, 0x33, - 0xba, 0x80, 0xfb, 0xad, 0x48, 0x45, 0x25, 0xa2, 0x22, 0x4f, 0x93, 0x6f, 0x89, 0x28, 0x59, 0x57, - 0x4d, 0xf2, 0xf0, 0x9f, 0x49, 0x1c, 0x65, 0xae, 0xa5, 0xf8, 0x93, 0x0f, 0xb7, 0x7f, 0xb3, 0x44, - 0x94, 0x93, 0xdf, 0x1a, 0x74, 0xd4, 0xa8, 0xb4, 0x0f, 0xc6, 0xc6, 0x5b, 0x79, 0xfe, 0x17, 0x8f, - 0xdc, 0xd0, 0x7b, 0xe8, 0xaf, 0x39, 0x46, 0x4b, 0x2f, 0x08, 0x2d, 0xd7, 0x25, 0x1a, 0x25, 0x30, - 0x58, 0xfb, 0x41, 0xd8, 0x90, 0x5b, 0x3a, 0x04, 0x90, 0x8a, 0x83, 0x2e, 0x86, 0x48, 0x5a, 0xea, - 0x88, 0x34, 0x6a, 0xd0, 0xbe, 0xf6, 0xd8, 0xac, 0x3f, 0x73, 0xcb, 0x41, 0xd2, 0x69, 0x7a, 0x5c, - 0x89, 0xae, 0x08, 0xc7, 0x88, 0xfb, 0xae, 0x3b, 0xb7, 0xec, 0x15, 0x31, 0xe8, 0x1b, 0xb8, 0x53, - 0x4e, 0x83, 0xba, 0x94, 0xc1, 0x5b, 0x8e, 0x2e, 0x5a, 0x01, 0x46, 0x21, 0x06, 0x61, 0x14, 0x6c, - 0x6c, 0x1b, 0x83, 0x80, 0xf4, 0xfe, 0xa9, 0x3c, 0x59, 0x4b, 0x77, 0xc3, 0x91, 0x80, 0xbc, 0xdb, - 0xe6, 0x4e, 0x33, 0x6d, 0x7f, 0x62, 0xc3, 0xe0, 0xf5, 0x3b, 0xd0, 0x3b, 0xe8, 0xa9, 0x3e, 0xe8, - 0xa0, 0x43, 0x6e, 0x28, 0x80, 0x2e, 0x0f, 0xa3, 0x43, 0x34, 0xd9, 0x75, 0x8e, 0x4f, 0x3e, 0xc7, - 0x68, 0xe1, 0xfb, 0xab, 0xc8, 0xe6, 0x68, 0x85, 0x4b, 0xdf, 0x23, 0xb7, 0xf3, 0xde, 0x57, 0xa3, - 0x7e, 0xd9, 0x67, 0x5d, 0x7d, 0xdb, 0xc7, 0x3f, 0x01, 0x00, 0x00, 0xff, 0xff, 0x3b, 0xcf, 0xed, - 0xd9, 0xb4, 0x02, 0x00, 0x00, +func init() { proto.RegisterFile("hapi/release/hook.proto", fileDescriptor_hook_e64400ca8195038e) } + +var fileDescriptor_hook_e64400ca8195038e = []byte{ + // 473 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x6c, 0x91, 0xdb, 0x8e, 0xda, 0x3c, + 0x10, 0x80, 0x37, 0x1c, 0x02, 0x0c, 0x87, 0xf5, 0x6f, 0xfd, 0x6a, 0x2d, 0x6e, 0x16, 0x21, 0x55, + 0xe2, 0x2a, 0x54, 0x5b, 0xf5, 0x01, 0x42, 0xe2, 0x2d, 0x88, 0x88, 0x20, 0x27, 0xa8, 0x52, 0x6f, + 0xa2, 0x6c, 0xf1, 0x42, 0x44, 0x88, 0x23, 0x62, 0x5a, 0xf5, 0x81, 0xfb, 0x18, 0x95, 0x2a, 0x3b, + 0x21, 0x5d, 0xa9, 0xbd, 0x9b, 0xf9, 0xe6, 0xf3, 0x78, 0xc6, 0x86, 0xb7, 0xc7, 0x38, 0x4f, 0xe6, + 0x17, 0x9e, 0xf2, 0xb8, 0xe0, 0xf3, 0xa3, 0x10, 0x27, 0x2b, 0xbf, 0x08, 0x29, 0xf0, 0x40, 0x15, + 0xac, 0xaa, 0x30, 0x7e, 0x38, 0x08, 0x71, 0x48, 0xf9, 0x5c, 0xd7, 0x9e, 0xaf, 0x2f, 0x73, 0x99, + 0x9c, 0x79, 0x21, 0xe3, 0x73, 0x5e, 0xea, 0xd3, 0x5f, 0x2d, 0x68, 0x2d, 0x85, 0x38, 0x61, 0x0c, + 0xad, 0x2c, 0x3e, 0x73, 0x62, 0x4c, 0x8c, 0x59, 0x8f, 0xe9, 0x58, 0xb1, 0x53, 0x92, 0xed, 0x49, + 0xa3, 0x64, 0x2a, 0x56, 0x2c, 0x8f, 0xe5, 0x91, 0x34, 0x4b, 0xa6, 0x62, 0x3c, 0x86, 0xee, 0x39, + 0xce, 0x92, 0x17, 0x5e, 0x48, 0xd2, 0xd2, 0xbc, 0xce, 0xf1, 0x7b, 0x30, 0xf9, 0x37, 0x9e, 0xc9, + 0x82, 0xb4, 0x27, 0xcd, 0xd9, 0xe8, 0x91, 0x58, 0xaf, 0x07, 0xb4, 0xd4, 0xdd, 0x16, 0x55, 0x02, + 0xab, 0x3c, 0xfc, 0x11, 0xba, 0x69, 0x5c, 0xc8, 0xe8, 0x72, 0xcd, 0x88, 0x39, 0x31, 0x66, 0xfd, + 0xc7, 0xb1, 0x55, 0xae, 0x61, 0xdd, 0xd6, 0xb0, 0xc2, 0xdb, 0x1a, 0xac, 0xa3, 0x5c, 0x76, 0xcd, + 0xf0, 0x1b, 0x30, 0xbf, 0xf3, 0xe4, 0x70, 0x94, 0xa4, 0x33, 0x31, 0x66, 0x6d, 0x56, 0x65, 0x78, + 0x09, 0xf7, 0x7b, 0x9e, 0x72, 0xc9, 0xa3, 0x5c, 0xa4, 0xc9, 0xd7, 0x84, 0x17, 0xa4, 0xab, 0x27, + 0x79, 0xf8, 0xc7, 0x24, 0xae, 0x36, 0xb7, 0x4a, 0xfc, 0xc1, 0x46, 0xfb, 0x3f, 0x59, 0xc2, 0x0b, + 0xfc, 0x0e, 0x2a, 0x12, 0xa9, 0x57, 0x14, 0x57, 0x49, 0x7a, 0x13, 0x63, 0xd6, 0x64, 0xc3, 0x92, + 0x86, 0x25, 0x9c, 0xfe, 0x34, 0xa0, 0xad, 0x37, 0xc2, 0x7d, 0xe8, 0xec, 0x36, 0xeb, 0x8d, 0xff, + 0x79, 0x83, 0xee, 0xf0, 0x3d, 0xf4, 0xb7, 0x8c, 0x46, 0xab, 0x4d, 0x10, 0xda, 0x9e, 0x87, 0x0c, + 0x8c, 0x60, 0xb0, 0xf5, 0x83, 0xb0, 0x26, 0x0d, 0x3c, 0x02, 0x50, 0x8a, 0x4b, 0x3d, 0x1a, 0x52, + 0xd4, 0xd4, 0x47, 0x94, 0x51, 0x81, 0xd6, 0xad, 0xc7, 0x6e, 0xfb, 0x89, 0xd9, 0x2e, 0x45, 0xed, + 0xba, 0xc7, 0x8d, 0x98, 0x9a, 0x30, 0x1a, 0x31, 0xdf, 0xf3, 0x16, 0xb6, 0xb3, 0x46, 0x1d, 0xfc, + 0x1f, 0x0c, 0xb5, 0x53, 0xa3, 0x2e, 0x26, 0xf0, 0x3f, 0xa3, 0x1e, 0xb5, 0x03, 0x1a, 0x85, 0x34, + 0x08, 0xa3, 0x60, 0xe7, 0x38, 0x34, 0x08, 0x50, 0xef, 0xaf, 0xca, 0x93, 0xbd, 0xf2, 0x76, 0x8c, + 0x22, 0x50, 0x77, 0x3b, 0xcc, 0xad, 0xa7, 0xed, 0x4f, 0x1d, 0x18, 0xbc, 0x7e, 0x2e, 0x3c, 0x84, + 0x9e, 0xee, 0x43, 0x5d, 0xea, 0xa2, 0x3b, 0x0c, 0x60, 0xaa, 0xc3, 0xd4, 0x45, 0x86, 0xea, 0xba, + 0xa0, 0x4f, 0x3e, 0xa3, 0xd1, 0xd2, 0xf7, 0xd7, 0x91, 0xc3, 0xa8, 0x1d, 0xae, 0xfc, 0x0d, 0x6a, + 0x2c, 0x7a, 0x5f, 0x3a, 0xd5, 0x07, 0x3c, 0x9b, 0xfa, 0x77, 0x3f, 0xfc, 0x0e, 0x00, 0x00, 0xff, + 0xff, 0xce, 0x84, 0xd9, 0x98, 0xdb, 0x02, 0x00, 0x00, } diff --git a/pkg/tiller/environment/environment.go b/pkg/tiller/environment/environment.go index 21c23d421..bfb6c638f 100644 --- a/pkg/tiller/environment/environment.go +++ b/pkg/tiller/environment/environment.go @@ -127,6 +127,16 @@ type KubeClient interface { // by "\n---\n"). Delete(namespace string, reader io.Reader) error + // DeleteWithTimeout destroys one or more resources. If shouldWait is true, the function + // will not return until all the resources have been fully deleted or the provided + // timeout has expired. + // + // namespace must contain a valid existing namespace. + // + // reader must contain a YAML stream (one or more YAML documents separated + // by "\n---\n"). + DeleteWithTimeout(namespace string, reader io.Reader, timeout int64, shouldWait bool) error + // WatchUntilReady watch the resource in reader until it is "ready". // // For Jobs, "ready" means the job ran to completion (excited without error). @@ -182,6 +192,14 @@ func (p *PrintingKubeClient) Delete(ns string, r io.Reader) error { return err } +// DeleteWithTimeout implements KubeClient DeleteWithTimeout. +// +// It only prints out the content to be deleted. +func (p *PrintingKubeClient) DeleteWithTimeout(ns string, r io.Reader, timeout int64, shouldWait bool) error { + _, err := io.Copy(p.Out, r) + return err +} + // WatchUntilReady implements KubeClient WatchUntilReady. func (p *PrintingKubeClient) WatchUntilReady(ns string, r io.Reader, timeout int64, shouldWait bool) error { _, err := io.Copy(p.Out, r) diff --git a/pkg/tiller/environment/environment_test.go b/pkg/tiller/environment/environment_test.go index 24ff8b88d..4c6452b00 100644 --- a/pkg/tiller/environment/environment_test.go +++ b/pkg/tiller/environment/environment_test.go @@ -49,6 +49,9 @@ func (k *mockKubeClient) Get(ns string, r io.Reader) (string, error) { func (k *mockKubeClient) Delete(ns string, r io.Reader) error { return nil } +func (k *mockKubeClient) DeleteWithTimeout(ns string, r io.Reader, timeout int64, shouldWait bool) error { + return nil +} func (k *mockKubeClient) Update(ns string, currentReader, modifiedReader io.Reader, force bool, recreate bool, timeout int64, shouldWait bool) error { return nil } diff --git a/pkg/tiller/hooks.go b/pkg/tiller/hooks.go index 0fb7c92f8..5aa961080 100644 --- a/pkg/tiller/hooks.go +++ b/pkg/tiller/hooks.go @@ -53,6 +53,9 @@ var deletePolices = map[string]release.Hook_DeletePolicy{ hooks.BeforeHookCreation: release.Hook_BEFORE_HOOK_CREATION, } +// Timout used when deleting resources with a hook-delete-policy. +const defaultHookDeleteTimeoutInSeconds = int64(60) + // Manifest represents a manifest file, which has a name and some content. type Manifest = manifest.Manifest @@ -192,6 +195,18 @@ func (file *manifestFile) sort(result *result) error { log.Printf("info: skipping unknown hook delete policy: %q", value) } }) + + // Only check for delete timeout annotation if there is a deletion policy. + if len(h.DeletePolicies) > 0 { + h.DeleteTimeout = defaultHookDeleteTimeoutInSeconds + operateAnnotationValues(entry, hooks.HookDeleteTimeoutAnno, func(value string) { + timeout, err := strconv.ParseInt(value, 10, 64) + if err != nil || timeout < 0 { + log.Printf("info: ignoring invalid hook delete timeout value: %q", value) + } + h.DeleteTimeout = timeout + }) + } } return nil } diff --git a/pkg/tiller/hooks_test.go b/pkg/tiller/hooks_test.go index 8bd928500..daf07252e 100644 --- a/pkg/tiller/hooks_test.go +++ b/pkg/tiller/hooks_test.go @@ -17,8 +17,10 @@ limitations under the License. package tiller import ( + "bytes" "reflect" "testing" + "text/template" "github.com/ghodss/yaml" @@ -229,6 +231,110 @@ metadata: } } +var manifestTemplate = ` +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: example.com + labels: + app: example-crd + annotations: + helm.sh/hook: crd-install +{{- if .HookDeletePolicy}} + {{ .HookDeletePolicy }} +{{- end }} +{{- if .HookDeleteTimeout}} + {{ .HookDeleteTimeout }} +{{- end }} +spec: + group: example.com + version: v1alpha1 + names: + kind: example + plural: examples + scope: Cluster +` + +type manifestTemplateData struct { + HookDeletePolicy, HookDeleteTimeout string +} + +func TestSortManifestsHookDeletion(t *testing.T) { + testCases := map[string]struct { + templateData manifestTemplateData + hasDeletePolicy bool + deletePolicy release.Hook_DeletePolicy + deleteTimeout int64 + }{ + "No delete policy": { + templateData: manifestTemplateData{}, + hasDeletePolicy: false, + deletePolicy: release.Hook_BEFORE_HOOK_CREATION, + deleteTimeout: 0, + }, + "Delete policy, no delete timeout": { + templateData: manifestTemplateData{ + HookDeletePolicy: "helm.sh/hook-delete-policy: before-hook-creation", + }, + hasDeletePolicy: true, + deletePolicy: release.Hook_BEFORE_HOOK_CREATION, + deleteTimeout: defaultHookDeleteTimeoutInSeconds, + }, + "Delete policy and delete timeout": { + templateData: manifestTemplateData{ + HookDeletePolicy: "helm.sh/hook-delete-policy: hook-succeeded", + HookDeleteTimeout: `helm.sh/hook-delete-timeout: "420"`, + }, + hasDeletePolicy: true, + deletePolicy: release.Hook_SUCCEEDED, + deleteTimeout: 420, + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + tmpl := template.Must(template.New("manifest").Parse(manifestTemplate)) + var buf bytes.Buffer + err := tmpl.Execute(&buf, tc.templateData) + if err != nil { + t.Error(err) + } + + manifests := map[string]string{ + "exampleManifest": buf.String(), + } + + hs, _, err := sortManifests(manifests, chartutil.NewVersionSet("v1", "v1beta1"), InstallOrder) + if err != nil { + t.Error(err) + } + + if got, want := len(hs), 1; got != want { + t.Errorf("expected %d hooks, but got %d", want, got) + } + hook := hs[0] + + if len(hook.DeletePolicies) == 0 { + if tc.hasDeletePolicy { + t.Errorf("expected a policy, but got zero") + } + } else { + if !tc.hasDeletePolicy { + t.Errorf("expected no delete policies, but got one") + } + policy := hook.DeletePolicies[0] + if got, want := policy, tc.deletePolicy; got != want { + t.Errorf("expected delete policy %q, but got %q", want, got) + } + } + + if got, want := hook.DeleteTimeout, tc.deleteTimeout; got != want { + t.Errorf("expected timeout %d, but got %d", want, got) + } + }) + } +} + func TestVersionSet(t *testing.T) { vs := chartutil.NewVersionSet("v1", "v1beta1", "extensions/alpha5", "batch/v1") diff --git a/pkg/tiller/release_server.go b/pkg/tiller/release_server.go index 6733035f7..652234c5f 100644 --- a/pkg/tiller/release_server.go +++ b/pkg/tiller/release_server.go @@ -456,7 +456,8 @@ func (s *ReleaseServer) deleteHookByPolicy(h *release.Hook, policy string, name, b := bytes.NewBufferString(h.Manifest) if hookHasDeletePolicy(h, policy) { s.Log("deleting %s hook %s for release %s due to %q policy", hook, h.Name, name, policy) - if errHookDelete := kubeCli.Delete(namespace, b); errHookDelete != nil { + waitForDelete := h.DeleteTimeout > 0 + if errHookDelete := kubeCli.DeleteWithTimeout(namespace, b, h.DeleteTimeout, waitForDelete); errHookDelete != nil { s.Log("warning: Release %s %s %S could not be deleted: %s", name, hook, h.Path, errHookDelete) return errHookDelete } diff --git a/pkg/tiller/release_server_test.go b/pkg/tiller/release_server_test.go index 05b41be20..015fe0b54 100644 --- a/pkg/tiller/release_server_test.go +++ b/pkg/tiller/release_server_test.go @@ -540,6 +540,10 @@ func (d *deleteFailingKubeClient) Delete(ns string, r io.Reader) error { return kube.ErrNoObjectsVisited } +func (d *deleteFailingKubeClient) DeleteWithTimeout(ns string, r io.Reader, timeout int64, shouldWait bool) error { + return kube.ErrNoObjectsVisited +} + type mockListServer struct { val *services.ListReleasesResponse } @@ -612,6 +616,9 @@ func (kc *mockHooksKubeClient) Get(ns string, r io.Reader) (string, error) { return "", nil } func (kc *mockHooksKubeClient) Delete(ns string, r io.Reader) error { + return kc.DeleteWithTimeout(ns, r, 0, false) +} +func (kc *mockHooksKubeClient) DeleteWithTimeout(ns string, r io.Reader, timeout int64, shouldWait bool) error { manifest, err := kc.makeManifest(r) if err != nil { return err