Skip to content
Permalink
Browse files

fix(helm): Delete hooks should wait for resource to be removed from e…

…tcd before continuing

Signed-off-by: Morten Torkildsen <mortent@google.com>
  • Loading branch information...
mortent committed Mar 31, 2019
1 parent 96258e3 commit cb2207c2fbddba7f0cabb626ecb7b02370ca9faa
@@ -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;
}
@@ -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
@@ -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
@@ -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)
@@ -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

Some generated files are not rendered by default. Learn more.

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

0 comments on commit cb2207c

Please sign in to comment.
You can’t perform that action at this time.