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

fix: better deletion of resources #1790

Merged
merged 6 commits into from
Apr 19, 2024
Merged

fix: better deletion of resources #1790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
140ad94
fix: better deletion of resources
adityathebe Apr 15, 2024
c2cad36
fix: perform deletion in bulk so collective waiting time is much lower
adityathebe Apr 15, 2024
cda3da5
feat: use polling to ensure deleted and make waiting for deletion
adityathebe Apr 16, 2024
1c81362
improve deletion polling
adityathebe Apr 18, 2024
77c8f2a
fix: cache clients by gvk
adityathebe Apr 18, 2024
25c4266
chore: remove max retries
adityathebe Apr 18, 2024
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
17 changes: 10 additions & 7 deletions api/v1/checks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,10 +790,9 @@ type KubernetesResourceChecks struct {

type KubernetesResourceCheckRetries struct {
// Delay is the initial delay
Delay string `json:"delay,omitempty"`
Timeout string `json:"timeout,omitempty"`
Interval string `json:"interval,omitempty"`
MaxRetries int `json:"maxRetries,omitempty"`
Delay string `json:"delay,omitempty"`
Timeout string `json:"timeout,omitempty"`
Interval string `json:"interval,omitempty"`

parsedDelay *time.Duration `json:"-"`
parsedTimeout *time.Duration `json:"-"`
Expand Down Expand Up @@ -863,16 +862,17 @@ type KubernetesResourceCheckWaitFor struct {
// Disable waiting for resources to get to their desired state.
Disable bool `json:"disable,omitempty"`

// Whether to wait for deletion or not
Delete bool `json:"delete,omitempty"`

// Timeout to wait for all static & non-static resources to be ready.
// Default: 10m
Timeout string `json:"timeout,omitempty"`

// Interval to check if all static & non-static resources are ready.
// Default: 30s
// Default: 5s
Interval string `json:"interval,omitempty"`

MaxRetries int `json:"maxRetries,omitempty"`

parsedTimeout *time.Duration `json:"-"`
parsedInterval *time.Duration `json:"-"`
}
Expand Down Expand Up @@ -936,6 +936,9 @@ type KubernetesResourceCheck struct {
// Set initial delays and retry intervals for checks.
CheckRetries KubernetesResourceCheckRetries `json:"checkRetries,omitempty"`

// Ensure that the resources are deleted before creating them.
ClearResources bool `json:"clearResources,omitempty"`

// Kubeconfig is the kubeconfig or the path to the kubeconfig file.
Kubeconfig *types.EnvVar `yaml:"kubeconfig,omitempty" json:"kubeconfig,omitempty"`

Expand Down
133 changes: 115 additions & 18 deletions checks/kubernetes_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,27 @@ package checks

import (
gocontext "context"
"errors"
"fmt"
"strconv"
"strings"
"sync"
"time"

"github.com/flanksource/gomplate/v3"
"github.com/samber/lo"
"github.com/sethvargo/go-retry"

"github.com/flanksource/commons/logger"
"github.com/flanksource/commons/utils"
"github.com/flanksource/duty/types"
"golang.org/x/sync/errgroup"
apiErrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
cliresource "k8s.io/cli-runtime/pkg/resource"

"github.com/flanksource/canary-checker/api/context"
v1 "github.com/flanksource/canary-checker/api/v1"
"github.com/flanksource/canary-checker/pkg"
"github.com/flanksource/commons/utils"
"github.com/flanksource/duty/types"
)

const (
Expand Down Expand Up @@ -72,19 +78,30 @@ func (c *KubernetesResourceChecker) Check(ctx *context.Context, check v1.Kuberne
}
}

// Keep track of all the created resources
// so we can delete them together instead of deleting them one by one.
var createdResources []unstructured.Unstructured
defer func() {
if err := deleteResources(ctx, check.WaitFor.Delete, createdResources...); err != nil {
results.Failf(err.Error())
}
}()

if check.ClearResources {
if err := deleteResources(ctx, true, check.Resources...); err != nil {
results.Failf(err.Error())
}
}

for i := range check.Resources {
resource := check.Resources[i]

resource.SetAnnotations(map[string]string{annotationkey: ctx.Canary.ID()})
if err := ctx.Kommons().ApplyUnstructured(utils.Coalesce(resource.GetNamespace(), ctx.Namespace), &resource); err != nil {
return results.Failf("failed to apply resource %s: %v", resource.GetName(), err)
return results.Failf("failed to apply resource (%s/%s/%s): %v", resource.GetKind(), resource.GetNamespace(), resource.GetName(), err)
}

defer func() {
if err := ctx.Kommons().DeleteUnstructured(utils.Coalesce(resource.GetNamespace(), ctx.Namespace), &resource); err != nil {
logger.Errorf("failed to delete resource %s: %v", resource.GetName(), err)
results.ErrorMessage(fmt.Errorf("failed to delete resource %s: %v", resource.GetName(), err))
}
}()
createdResources = append(createdResources, resource)
}

if !check.WaitFor.Disable {
Expand Down Expand Up @@ -128,10 +145,6 @@ func (c *KubernetesResourceChecker) Check(ctx *context.Context, check v1.Kuberne
backoff = retry.NewConstant(retryInterval)
}

if check.CheckRetries.MaxRetries > 0 {
backoff = retry.WithMaxRetries(uint64(check.CheckRetries.MaxRetries), backoff)
}

if maxRetryTimeout, _ := check.CheckRetries.GetTimeout(); maxRetryTimeout > 0 {
backoff = retry.WithMaxDuration(maxRetryTimeout, backoff)
}
Expand Down Expand Up @@ -181,9 +194,6 @@ func (c *KubernetesResourceChecker) evalWaitFor(ctx *context.Context, check v1.K

var attempts int
backoff := retry.WithMaxDuration(waitTimeout, retry.NewConstant(waitInterval))
if check.WaitFor.MaxRetries > 0 {
backoff = retry.WithMaxRetries(uint64(check.WaitFor.MaxRetries), backoff)
}
retryErr := retry.Do(ctx, backoff, func(_ctx gocontext.Context) error {
ctx = _ctx.(*context.Context)
attempts++
Expand Down Expand Up @@ -277,3 +287,90 @@ func (c *KubernetesResourceChecker) validate(ctx *context.Context, check v1.Kube

return nil
}

func deleteResources(ctx *context.Context, waitForDelete bool, resources ...unstructured.Unstructured) error {
ctx.Logger.V(4).Infof("deleting %d resources", len(resources))

// cache dynamic clients
clients := sync.Map{}

eg, _ := errgroup.WithContext(ctx)
for i := range resources {
resource := resources[i]

eg.Go(func() error {
rc, err := ctx.Kommons().GetRestClient(resource)
adityathebe marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return fmt.Errorf("failed to get rest client for (%s/%s/%s): %w", resource.GetKind(), resource.GetNamespace(), resource.GetName(), err)
}
gvk := resource.GetObjectKind().GroupVersionKind()
clients.Store(gvk, rc)

namespace := utils.Coalesce(resource.GetNamespace(), ctx.Namespace)
deleteOpt := &metav1.DeleteOptions{
GracePeriodSeconds: lo.ToPtr(int64(0)),
PropagationPolicy: lo.ToPtr(metav1.DeletePropagationOrphan),
}
if _, err := rc.DeleteWithOptions(namespace, resource.GetName(), deleteOpt); err != nil {
var statusErr *apiErrors.StatusError
if errors.As(err, &statusErr) {
switch statusErr.ErrStatus.Code {
case 404:
return nil
}
}

return fmt.Errorf("failed to delete resource (%s/%s/%s): %w", resource.GetKind(), resource.GetNamespace(), resource.GetName(), err)
}

return nil
})
}
if err := eg.Wait(); err != nil {
return err
}

if !waitForDelete {
return nil
}

ticker := time.NewTicker(2 * time.Second)
adityathebe marked this conversation as resolved.
Show resolved Hide resolved
defer ticker.Stop()

for {
select {
case <-ticker.C:
if len(resources) == 0 {
ctx.Logger.V(5).Infof("all the resources have been deleted")
return nil
}

deleted := make(map[string]struct{})
for _, resource := range resources {
cachedClient, _ := clients.Load(resource.GetObjectKind().GroupVersionKind())
rc := cachedClient.(*cliresource.Helper)

if _, err := rc.Get(resource.GetNamespace(), resource.GetName()); err != nil {
if !apiErrors.IsNotFound(err) {
return fmt.Errorf("error getting resource (%s/%s/%s) while polling: %w", resource.GetKind(), resource.GetNamespace(), resource.GetName(), err)
}

deleted[string(resource.GetUID())] = struct{}{}
ctx.Logger.V(5).Infof("(%s/%s/%s) has been deleted", resource.GetKind(), resource.GetNamespace(), resource.GetName())
} else {
ctx.Logger.V(5).Infof("(%s/%s/%s) has not been deleted", resource.GetKind(), resource.GetNamespace(), resource.GetName())
}
}

resources = lo.Filter(resources, func(item unstructured.Unstructured, _ int) bool {
_, ok := deleted[string(item.GetUID())]
return !ok
})

break

case <-ctx.Done():
return ctx.Err()
}
}
}
12 changes: 7 additions & 5 deletions config/deploy/crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4997,8 +4997,6 @@ spec:
type: string
interval:
type: string
maxRetries:
type: integer
timeout:
type: string
type: object
Expand All @@ -5013,6 +5011,9 @@ spec:
type: object
type: array
x-kubernetes-preserve-unknown-fields: true
clearResources:
description: Ensure that the resources are deleted before creating them.
type: boolean
description:
description: Description for the check
type: string
Expand Down Expand Up @@ -5145,6 +5146,9 @@ spec:
type: string
waitFor:
properties:
delete:
description: Whether to wait for deletion or not
type: boolean
disable:
description: Disable waiting for resources to get to their desired state.
type: boolean
Expand All @@ -5157,10 +5161,8 @@ spec:
interval:
description: |-
Interval to check if all static & non-static resources are ready.
Default: 30s
Default: 5s
type: string
maxRetries:
type: integer
timeout:
description: |-
Timeout to wait for all static & non-static resources to be ready.
Expand Down
12 changes: 7 additions & 5 deletions config/deploy/manifests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4996,8 +4996,6 @@ spec:
type: string
interval:
type: string
maxRetries:
type: integer
timeout:
type: string
type: object
Expand All @@ -5012,6 +5010,9 @@ spec:
type: object
type: array
x-kubernetes-preserve-unknown-fields: true
clearResources:
description: Ensure that the resources are deleted before creating them.
type: boolean
description:
description: Description for the check
type: string
Expand Down Expand Up @@ -5144,6 +5145,9 @@ spec:
type: string
waitFor:
properties:
delete:
description: Whether to wait for deletion or not
type: boolean
disable:
description: Disable waiting for resources to get to their desired state.
type: boolean
Expand All @@ -5156,10 +5160,8 @@ spec:
interval:
description: |-
Interval to check if all static & non-static resources are ready.
Default: 30s
Default: 5s
type: string
maxRetries:
type: integer
timeout:
description: |-
Timeout to wait for all static & non-static resources to be ready.
Expand Down
12 changes: 6 additions & 6 deletions config/schemas/canary.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2326,6 +2326,9 @@
"checkRetries": {
"$ref": "#/$defs/KubernetesResourceCheckRetries"
},
"clearResources": {
"type": "boolean"
},
"kubeconfig": {
"$ref": "#/$defs/EnvVar"
},
Expand All @@ -2350,9 +2353,6 @@
},
"interval": {
"type": "string"
},
"maxRetries": {
"type": "integer"
}
},
"additionalProperties": false,
Expand All @@ -2366,14 +2366,14 @@
"disable": {
"type": "boolean"
},
"delete": {
"type": "boolean"
},
"timeout": {
"type": "string"
},
"interval": {
"type": "string"
},
"maxRetries": {
"type": "integer"
}
},
"additionalProperties": false,
Expand Down
12 changes: 6 additions & 6 deletions config/schemas/component.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2583,6 +2583,9 @@
"checkRetries": {
"$ref": "#/$defs/KubernetesResourceCheckRetries"
},
"clearResources": {
"type": "boolean"
},
"kubeconfig": {
"$ref": "#/$defs/EnvVar"
},
Expand All @@ -2607,9 +2610,6 @@
},
"interval": {
"type": "string"
},
"maxRetries": {
"type": "integer"
}
},
"additionalProperties": false,
Expand All @@ -2623,14 +2623,14 @@
"disable": {
"type": "boolean"
},
"delete": {
"type": "boolean"
},
"timeout": {
"type": "string"
},
"interval": {
"type": "string"
},
"maxRetries": {
"type": "integer"
}
},
"additionalProperties": false,
Expand Down
Loading
Loading