From d5348ce7e8af0216a5e51f772eb44aa95d6cf782 Mon Sep 17 00:00:00 2001 From: Jakub Dyszkiewicz Date: Mon, 21 Oct 2019 15:26:07 +0200 Subject: [PATCH] feat(kuma-cp) ignore not found resource on delete all --- pkg/core/managers/apis/mesh/mesh_manager.go | 11 +++++++---- pkg/core/resources/manager/manager.go | 2 +- pkg/core/resources/manager/manager_test.go | 13 +++++++++++++ pkg/core/secrets/manager/manager.go | 2 +- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/pkg/core/managers/apis/mesh/mesh_manager.go b/pkg/core/managers/apis/mesh/mesh_manager.go index 46f6cd1c37e1..489f3965b99b 100644 --- a/pkg/core/managers/apis/mesh/mesh_manager.go +++ b/pkg/core/managers/apis/mesh/mesh_manager.go @@ -95,15 +95,18 @@ func (m *meshManager) Delete(ctx context.Context, resource core_model.Resource, } // delete Mesh first to avoid a state where a Mesh could exist without a Built-in CA. // even if removal of Built-in CA fails later on, delete opration can be safely tried again. + var notFoundErr error if err := m.store.Delete(ctx, mesh, fs...); err != nil { - if !core_store.IsResourceNotFound(err) { // ignore other errors so we can retry removing other resources + if core_store.IsResourceNotFound(err) { + notFoundErr = err + } else { // ignore other errors so we can retry removing other resources return err } } opts := core_store.NewDeleteOptions(fs...) // delete CA name := core_store.NewDeleteOptions(fs...).Mesh - if err := m.builtinCaManager.Delete(ctx, name); err != nil { + if err := m.builtinCaManager.Delete(ctx, name); err != nil && !core_store.IsResourceNotFound(err) { return errors.Wrapf(err, "failed to delete Builtin CA for a given mesh") } // delete all other secrets @@ -120,7 +123,7 @@ func (m *meshManager) Delete(ctx context.Context, resource core_model.Resource, return errors.Wrap(err, "could not delete associated resources") } } - return nil + return notFoundErr } func (m *meshManager) DeleteAll(ctx context.Context, list core_model.ResourceList, fs ...core_store.DeleteAllOptionsFunc) error { @@ -133,7 +136,7 @@ func (m *meshManager) DeleteAll(ctx context.Context, list core_model.ResourceLis return err } for _, item := range meshes.Items { - if err := m.Delete(ctx, item, core_store.DeleteBy(core_model.MetaToResourceKey(item.Meta))); err != nil { + if err := m.Delete(ctx, item, core_store.DeleteBy(core_model.MetaToResourceKey(item.Meta))); err != nil && !core_store.IsResourceNotFound(err) { return err } } diff --git a/pkg/core/resources/manager/manager.go b/pkg/core/resources/manager/manager.go index 5e968200d03a..ca4ea749f1ee 100644 --- a/pkg/core/resources/manager/manager.go +++ b/pkg/core/resources/manager/manager.go @@ -70,7 +70,7 @@ func (r *resourcesManager) DeleteAll(ctx context.Context, list model.ResourceLis return err } for _, obj := range list.GetItems() { - if err := r.Delete(ctx, obj, store.DeleteBy(model.MetaToResourceKey(obj.GetMeta()))); err != nil { + if err := r.Delete(ctx, obj, store.DeleteBy(model.MetaToResourceKey(obj.GetMeta()))); err != nil && !store.IsResourceNotFound(err) { return err } } diff --git a/pkg/core/resources/manager/manager_test.go b/pkg/core/resources/manager/manager_test.go index d9213dfe88c6..edfceb0943d4 100644 --- a/pkg/core/resources/manager/manager_test.go +++ b/pkg/core/resources/manager/manager_test.go @@ -5,6 +5,7 @@ import ( mesh_proto "github.com/Kong/kuma/api/mesh/v1alpha1" "github.com/Kong/kuma/pkg/core/resources/apis/mesh" "github.com/Kong/kuma/pkg/core/resources/manager" + "github.com/Kong/kuma/pkg/core/resources/model" "github.com/Kong/kuma/pkg/core/resources/store" "github.com/Kong/kuma/pkg/plugins/resources/memory" "github.com/Kong/kuma/pkg/test/apis/sample/v1alpha1" @@ -74,6 +75,14 @@ var _ = Describe("Resource Manager", func() { _, err = createSampleResource("mesh-2") Expect(err).ToNot(HaveOccurred()) + tlKey := model.ResourceKey{ + Mesh: "mesh-1", + Namespace: "default", + Name: "tl-1", + } + err = resManager.Create(context.Background(), &mesh.TrafficLogResource{}, store.CreateBy(tlKey)) + Expect(err).ToNot(HaveOccurred()) + // when err = resManager.DeleteAll(context.Background(), &sample.TrafficRouteResourceList{}, store.DeleteAllByMesh("mesh-1")) @@ -85,10 +94,14 @@ var _ = Describe("Resource Manager", func() { err = resManager.Get(context.Background(), &res1, store.GetByKey("default", "tr-1", "mesh-1")) Expect(store.IsResourceNotFound(err)).To(BeTrue()) + // and only TrafficRoutes are deleted + Expect(resManager.Get(context.Background(), &mesh.TrafficLogResource{}, store.GetBy(tlKey))).To(Succeed()) + // and resource from mesh-2 is retained res2 := sample.TrafficRouteResource{} err = resManager.Get(context.Background(), &res2, store.GetByKey("default", "tr-1", "mesh-2")) Expect(err).ToNot(HaveOccurred()) + }) }) }) diff --git a/pkg/core/secrets/manager/manager.go b/pkg/core/secrets/manager/manager.go index ca79f35bfc79..654f28d83eb2 100644 --- a/pkg/core/secrets/manager/manager.go +++ b/pkg/core/secrets/manager/manager.go @@ -77,7 +77,7 @@ func (s *secretManager) DeleteAll(ctx context.Context, fs ...core_store.DeleteAl return err } for _, item := range list.Items { - if err := s.Delete(ctx, item, core_store.DeleteBy(model.MetaToResourceKey(item.Meta))); err != nil { + if err := s.Delete(ctx, item, core_store.DeleteBy(model.MetaToResourceKey(item.Meta))); err != nil && !core_store.IsResourceNotFound(err) { return err } }