From ed8d16534e79b736ea456e42a03ea42b3f41c2b7 Mon Sep 17 00:00:00 2001 From: Roman Tkachenko Date: Wed, 17 Apr 2019 11:49:58 -0700 Subject: [PATCH] Delete completed status hook pods. (#373) --- lib/app/acl.go | 6 +++--- lib/app/app.go | 12 ++++++++++-- lib/app/apps.go | 7 +++++-- lib/app/client/client.go | 6 +++--- lib/app/handler/handler.go | 10 +++++++++- lib/app/hooks/hooks.go | 23 +++++++++++++++++++---- lib/app/hooks/hooks_test.go | 8 ++++---- lib/app/service/app.go | 4 ++-- lib/app/suite/suite.go | 6 +++--- lib/ops/opsservice/hooks.go | 6 ++++-- lib/ops/opsservice/status.go | 5 ++++- tool/gravity/cli/backup_restore.go | 6 ++++-- 12 files changed, 70 insertions(+), 29 deletions(-) diff --git a/lib/app/acl.go b/lib/app/acl.go index 72b7cf006d..d2d86d2b1a 100644 --- a/lib/app/acl.go +++ b/lib/app/acl.go @@ -212,11 +212,11 @@ func (r *ApplicationsACL) WaitAppHook(ctx context.Context, ref HookRef) error { } // DeleteAppHookJob deletes app hook job to complete or fail -func (r *ApplicationsACL) DeleteAppHookJob(ctx context.Context, ref HookRef) error { - if err := r.check(ref.Application.Repository, teleservices.VerbRead); err != nil { +func (r *ApplicationsACL) DeleteAppHookJob(ctx context.Context, req DeleteAppHookJobRequest) error { + if err := r.check(req.Application.Repository, teleservices.VerbRead); err != nil { return trace.Wrap(err) } - return r.applications.DeleteAppHookJob(ctx, ref) + return r.applications.DeleteAppHookJob(ctx, req) } // StreamAppHookLogs streams app hook logs to output writer, this is a blocking call diff --git a/lib/app/app.go b/lib/app/app.go index be2c5cfb6e..76b80f2061 100644 --- a/lib/app/app.go +++ b/lib/app/app.go @@ -31,7 +31,7 @@ import ( "github.com/gravitational/gravity/lib/storage" "github.com/gravitational/trace" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" ) const ( @@ -105,7 +105,7 @@ type Applications interface { WaitAppHook(ctx context.Context, ref HookRef) error // DeleteAppHookJob deletes app hook job - DeleteAppHookJob(ctx context.Context, ref HookRef) error + DeleteAppHookJob(ctx context.Context, req DeleteAppHookJobRequest) error // StreamAppHookLogs streams app hook logs to output writer, this is a blocking call StreamAppHookLogs(ctx context.Context, ref HookRef, out io.Writer) error @@ -117,6 +117,14 @@ type Applications interface { FetchIndexFile() (io.Reader, error) } +// DeleteAppHookJobRequest is a request to delete a hook job. +type DeleteAppHookJobRequest struct { + // HookRef is the hook job reference. + HookRef + // Cascade specifies whether dependent pods should be deleted as well. + Cascade bool `json:"cascade"` +} + // ListAppsRequest is a request to show applications in a repository type ListAppsRequest struct { // Repository is repository to show apps for diff --git a/lib/app/apps.go b/lib/app/apps.go index 1ed78cf8f8..6ff4c20356 100644 --- a/lib/app/apps.go +++ b/lib/app/apps.go @@ -123,12 +123,15 @@ func StreamAppHookLogs(ctx context.Context, client *kubernetes.Clientset, ref Ho } // DeleteAppHookJob deletes app hook job -func DeleteAppHookJob(ctx context.Context, client *kubernetes.Clientset, ref HookRef) error { +func DeleteAppHookJob(ctx context.Context, client *kubernetes.Clientset, req DeleteAppHookJobRequest) error { runner, err := hooks.NewRunner(client) if err != nil { return trace.Wrap(err) } - return runner.DeleteJob(ctx, hooks.JobRef{Name: ref.Name, Namespace: ref.Namespace}) + return runner.DeleteJob(ctx, hooks.DeleteJobRequest{ + JobRef: hooks.JobRef{Name: req.Name, Namespace: req.Namespace}, + Cascade: req.Cascade, + }) } // GetUpdatedDependencies compares dependencies of the "installed" and "update" apps and diff --git a/lib/app/client/client.go b/lib/app/client/client.go index ca4a10bab6..0503e0fa64 100644 --- a/lib/app/client/client.go +++ b/lib/app/client/client.go @@ -372,10 +372,10 @@ func (c *Client) StreamAppHookLogs(ctx context.Context, ref app.HookRef, out io. } // DELETE app/v1/applications/:repository_id/:package_id/:version/hook/:namespace/:name -func (c *Client) DeleteAppHookJob(ctx context.Context, ref app.HookRef) error { +func (c *Client) DeleteAppHookJob(ctx context.Context, req app.DeleteAppHookJobRequest) error { _, err := c.Delete(c.Endpoint( - "applications", ref.Application.Repository, ref.Application.Name, ref.Application.Version, "hook", ref.Namespace, ref.Name), - url.Values{}) + "applications", req.Application.Repository, req.Application.Name, req.Application.Version, "hook", req.Namespace, req.Name), + url.Values{"cascade": []string{strconv.FormatBool(req.Cascade)}}) if err != nil { return trace.Wrap(err) } diff --git a/lib/app/handler/handler.go b/lib/app/handler/handler.go index 28d20a03aa..ecda3e6ee6 100644 --- a/lib/app/handler/handler.go +++ b/lib/app/handler/handler.go @@ -42,6 +42,7 @@ import ( "github.com/coreos/go-semver/semver" "github.com/gravitational/form" "github.com/gravitational/roundtrip" + telehttplib "github.com/gravitational/teleport/lib/httplib" teleservices "github.com/gravitational/teleport/lib/services" "github.com/gravitational/trace" "github.com/julienschmidt/httprouter" @@ -744,7 +745,14 @@ func (h *WebHandler) deleteAppHookJob(w http.ResponseWriter, Name: params.ByName("name"), Namespace: params.ByName("namespace"), } - err = context.applications.DeleteAppHookJob(req.Context(), hookRef) + cascade, _, err := telehttplib.ParseBool(req.URL.Query(), "cascade") + if err != nil { + return trace.Wrap(err) + } + err = context.applications.DeleteAppHookJob(req.Context(), app.DeleteAppHookJobRequest{ + HookRef: hookRef, + Cascade: cascade, + }) if err != nil { return trace.Wrap(err) } diff --git a/lib/app/hooks/hooks.go b/lib/app/hooks/hooks.go index 8aad26456a..c997e5d8b3 100644 --- a/lib/app/hooks/hooks.go +++ b/lib/app/hooks/hooks.go @@ -35,7 +35,7 @@ import ( "github.com/gravitational/trace" log "github.com/sirupsen/logrus" batchv1 "k8s.io/api/batch/v1" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" @@ -126,13 +126,28 @@ func NewRunner(client *kubernetes.Clientset) (*Runner, error) { return runner, nil } +// DeleteJobRequest combines parameters for job deletion. +type DeleteJobRequest struct { + // JobRef identifies the job to delete. + JobRef + // Cascade specifies whether dependent objects should be deleted. + Cascade bool +} + // DeleteJob deletes job by ref -func (r *Runner) DeleteJob(ctx context.Context, ref JobRef) error { - err := r.client.Batch().Jobs(ref.Namespace).Delete(ref.Name, nil) +func (r *Runner) DeleteJob(ctx context.Context, req DeleteJobRequest) error { + var opts *metav1.DeleteOptions + if req.Cascade { + propagationPolicy := metav1.DeletePropagationForeground + opts = &metav1.DeleteOptions{ + PropagationPolicy: &propagationPolicy, + } + } + err := r.client.Batch().Jobs(req.Namespace).Delete(req.Name, opts) if err = rigging.ConvertError(err); err != nil { return err } else { - r.Debugf("Deleted job %q in namespace %q.", ref.Name, ref.Namespace) + r.Debugf("Deleted job %q in namespace %q.", req.Name, req.Namespace) } return nil } diff --git a/lib/app/hooks/hooks_test.go b/lib/app/hooks/hooks_test.go index 672827d427..024090e70b 100644 --- a/lib/app/hooks/hooks_test.go +++ b/lib/app/hooks/hooks_test.go @@ -36,7 +36,7 @@ import ( log "github.com/sirupsen/logrus" "gopkg.in/check.v1" batchv1 "k8s.io/api/batch/v1" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -144,7 +144,7 @@ func (s *HooksSuite) TestHookSuccess(c *check.C) { c.Assert(utils.RemoveNewlines(out.String()), check.Matches, ".*hello, world 1.*") c.Assert(utils.RemoveNewlines(out.String()), check.Matches, ".*hello, world 2.*") - err = runner.DeleteJob(context.TODO(), *ref) + err = runner.DeleteJob(context.TODO(), DeleteJobRequest{JobRef: *ref}) c.Assert(err, check.IsNil) } @@ -217,7 +217,7 @@ func (s *HooksSuite) TestHookFailNewPods(c *check.C) { comment := check.Commentf("expected more matches in %v", output) c.Assert(strings.Count(output, "hello, world") >= 2, check.Equals, true, comment) - err = runner.DeleteJob(context.TODO(), *ref) + err = runner.DeleteJob(context.TODO(), DeleteJobRequest{JobRef: *ref}) c.Assert(err, check.IsNil) } @@ -289,6 +289,6 @@ func (s *HooksSuite) TestHookFailPodRestart(c *check.C) { comment := check.Commentf("expected more matches in %v", output) c.Assert(strings.Count(output, "hello, world") >= 2, check.Equals, true, comment) - err = runner.DeleteJob(context.TODO(), *ref) + err = runner.DeleteJob(context.TODO(), DeleteJobRequest{JobRef: *ref}) c.Assert(err, check.IsNil) } diff --git a/lib/app/service/app.go b/lib/app/service/app.go index 6c5873dfec..a6d86cd936 100644 --- a/lib/app/service/app.go +++ b/lib/app/service/app.go @@ -349,12 +349,12 @@ func (r *applications) StreamAppHookLogs(ctx context.Context, ref appservice.Hoo } // DeleteAppHookJob deletes app hook job -func (r *applications) DeleteAppHookJob(ctx context.Context, ref appservice.HookRef) error { +func (r *applications) DeleteAppHookJob(ctx context.Context, req appservice.DeleteAppHookJobRequest) error { client, err := r.getKubeClient() if err != nil { return trace.Wrap(err) } - return appservice.DeleteAppHookJob(ctx, client, ref) + return appservice.DeleteAppHookJob(ctx, client, req) } // StatusApp retrieves the status of a running application diff --git a/lib/app/suite/suite.go b/lib/app/suite/suite.go index aaf06cd3a6..0c79298564 100644 --- a/lib/app/suite/suite.go +++ b/lib/app/suite/suite.go @@ -51,7 +51,7 @@ import ( log "github.com/sirupsen/logrus" . "gopkg.in/check.v1" batchv1 "k8s.io/api/batch/v1" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/helm/pkg/chartutil" "k8s.io/helm/pkg/repo" @@ -596,10 +596,10 @@ metadata: c.Assert(err, IsNil) c.Assert(utils.RemoveNewlines(out.String()), Matches, ".*hello, world app hook.*") - err = apps.DeleteAppHookJob(ctx, *ref) + err = apps.DeleteAppHookJob(ctx, app.DeleteAppHookJobRequest{HookRef: *ref}) c.Assert(err, IsNil) - err = apps.DeleteAppHookJob(ctx, *ref) + err = apps.DeleteAppHookJob(ctx, app.DeleteAppHookJobRequest{HookRef: *ref}) c.Assert(trace.IsNotFound(err), Equals, true, Commentf("expected not found, got %T", err)) } diff --git a/lib/ops/opsservice/hooks.go b/lib/ops/opsservice/hooks.go index 97a941bbe4..2f7c25db22 100644 --- a/lib/ops/opsservice/hooks.go +++ b/lib/ops/opsservice/hooks.go @@ -33,7 +33,7 @@ import ( "github.com/gravitational/rigging" "github.com/gravitational/trace" batchv1 "k8s.io/api/batch/v1" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -311,7 +311,9 @@ func (s *site) runIntegrationHook(ctx *operationContext, job *batchv1.Job, reque return trace.Wrap(err) } defer func() { - err := s.appService.DeleteAppHookJob(context.TODO(), *ref) + err := s.appService.DeleteAppHookJob(context.TODO(), app.DeleteAppHookJobRequest{ + HookRef: *ref, + }) if err != nil { ctx.Warningf("Failed to delete hook job: %v.", trace.DebugReport(err)) } diff --git a/lib/ops/opsservice/status.go b/lib/ops/opsservice/status.go index bfecb72e4b..889793962b 100644 --- a/lib/ops/opsservice/status.go +++ b/lib/ops/opsservice/status.go @@ -109,7 +109,10 @@ func (s *site) checkStatusHook(ctx context.Context) error { ServiceUser: s.serviceUser(), }) if ref != nil { - err := s.service.cfg.Apps.DeleteAppHookJob(ctx, *ref) + err := s.service.cfg.Apps.DeleteAppHookJob(ctx, app.DeleteAppHookJobRequest{ + HookRef: *ref, + Cascade: true, + }) if err != nil { s.Warnf("Failed to delete status hook %v: %v.", ref, trace.DebugReport(err)) diff --git a/tool/gravity/cli/backup_restore.go b/tool/gravity/cli/backup_restore.go index da78c3d6f3..5debe4cc32 100644 --- a/tool/gravity/cli/backup_restore.go +++ b/tool/gravity/cli/backup_restore.go @@ -35,7 +35,7 @@ import ( dockerarchive "github.com/docker/docker/pkg/archive" teleutils "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/trace" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" ) func backup(env *localenv.LocalEnvironment, tarball string, timeout time.Duration, follow, silent bool) (err error) { @@ -61,7 +61,9 @@ func backup(env *localenv.LocalEnvironment, tarball string, timeout time.Duratio return trace.Wrap(err) } defer func() { - err := apps.DeleteAppHookJob(ctx, *ref) + err := apps.DeleteAppHookJob(ctx, app.DeleteAppHookJobRequest{ + HookRef: *ref, + }) if err != nil { log.Warningf("failed to delete hook %v: %v", ref, trace.DebugReport(err))