Skip to content
This repository has been archived by the owner on Feb 9, 2024. It is now read-only.

Commit

Permalink
Delete completed status hook pods. (#373)
Browse files Browse the repository at this point in the history
  • Loading branch information
r0mant authored Apr 17, 2019
1 parent 3379dab commit ed8d165
Show file tree
Hide file tree
Showing 12 changed files with 70 additions and 29 deletions.
6 changes: 3 additions & 3 deletions lib/app/acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 10 additions & 2 deletions lib/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
7 changes: 5 additions & 2 deletions lib/app/apps.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions lib/app/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
10 changes: 9 additions & 1 deletion lib/app/handler/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down
23 changes: 19 additions & 4 deletions lib/app/hooks/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
8 changes: 4 additions & 4 deletions lib/app/hooks/hooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}
4 changes: 2 additions & 2 deletions lib/app/service/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions lib/app/suite/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
}

Expand Down
6 changes: 4 additions & 2 deletions lib/ops/opsservice/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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))
}
Expand Down
5 changes: 4 additions & 1 deletion lib/ops/opsservice/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
6 changes: 4 additions & 2 deletions tool/gravity/cli/backup_restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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))
Expand Down

0 comments on commit ed8d165

Please sign in to comment.