From c86e36b48316d1427bb943dee2770edf5140e908 Mon Sep 17 00:00:00 2001 From: Pires Date: Tue, 26 Nov 2019 16:34:27 +0000 Subject: [PATCH 1/3] Clean-up installed CRDs on Stop() But only if USE_EXISTING_CLUSTER is set to true as there's no point in doing it if the control-plane is going away. Signed-off-by: Pires --- pkg/envtest/crd.go | 31 ++++++++++ pkg/envtest/envtest_test.go | 114 +++++++++++++++++++++++++++++++++++- pkg/envtest/server.go | 9 ++- 3 files changed, 152 insertions(+), 2 deletions(-) diff --git a/pkg/envtest/crd.go b/pkg/envtest/crd.go index c14c8688ab..8efa29cb2f 100644 --- a/pkg/envtest/crd.go +++ b/pkg/envtest/crd.go @@ -27,6 +27,7 @@ import ( apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" @@ -51,6 +52,11 @@ type CRDInstallOptions struct { // PollInterval is the interval to check PollInterval time.Duration + + // CleanUpAfterUse will cause the CRDs listed for installation to be + // uninstalled when terminating the test environment. + // Defaults to false. + CleanUpAfterUse bool } const defaultPollInterval = 100 * time.Millisecond @@ -180,6 +186,31 @@ func (p *poller) poll() (done bool, err error) { return allFound, nil } +// UninstallCRDs uninstalls a collection of CRDs by reading the crd yaml files from a directory +func UninstallCRDs(config *rest.Config, options CRDInstallOptions) error { + + // Read the CRD yamls into options.CRDs + if err := readCRDFiles(&options); err != nil { + return err + } + + // Delete the CRDs from the apiserver + cs, err := clientset.NewForConfig(config) + if err != nil { + return err + } + + // Uninstall each CRD + for _, crd := range options.CRDs { + log.V(1).Info("uninstalling CRD", "crd", crd.Name) + if err := cs.ApiextensionsV1beta1().CustomResourceDefinitions().Delete(crd.Name, &metav1.DeleteOptions{}); err != nil { + return err + } + } + + return nil +} + // CreateCRDs creates the CRDs func CreateCRDs(config *rest.Config, crds []*apiextensionsv1beta1.CustomResourceDefinition) error { cs, err := clientset.NewForConfig(config) diff --git a/pkg/envtest/envtest_test.go b/pkg/envtest/envtest_test.go index 73a78973ac..2e6b9cf07f 100644 --- a/pkg/envtest/envtest_test.go +++ b/pkg/envtest/envtest_test.go @@ -19,12 +19,12 @@ package envtest import ( "context" "path/filepath" - "time" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -52,6 +52,17 @@ var _ = Describe("Test", func() { // Cleanup CRDs AfterEach(func(done Done) { for _, crd := range crds { + // Delete only if CRD exists. + crdObjectKey := client.ObjectKey{ + Name: crd.Name, + } + var placeholder v1beta1.CustomResourceDefinition + err := c.Get(context.TODO(), crdObjectKey, &placeholder) + if err != nil && apierrors.IsNotFound(err) { + // CRD doesn't need to be deleted. + continue + } + Expect(err).NotTo(HaveOccurred()) Expect(c.Delete(context.TODO(), crd)).To(Succeed()) } close(done) @@ -216,4 +227,105 @@ var _ = Describe("Test", func() { close(done) }, 5) }) + + Describe("UninstallCRDs", func() { + It("should uninstall the CRDs from the cluster", func(done Done) { + + crds, err = InstallCRDs(env.Config, CRDInstallOptions{ + Paths: []string{filepath.Join(".", "testdata")}, + }) + Expect(err).NotTo(HaveOccurred()) + + // Expect to find the CRDs + + crd := &v1beta1.CustomResourceDefinition{} + err = c.Get(context.TODO(), types.NamespacedName{Name: "foos.bar.example.com"}, crd) + Expect(err).NotTo(HaveOccurred()) + Expect(crd.Spec.Names.Kind).To(Equal("Foo")) + + crd = &v1beta1.CustomResourceDefinition{} + err = c.Get(context.TODO(), types.NamespacedName{Name: "bazs.qux.example.com"}, crd) + Expect(err).NotTo(HaveOccurred()) + Expect(crd.Spec.Names.Kind).To(Equal("Baz")) + + crd = &v1beta1.CustomResourceDefinition{} + err = c.Get(context.TODO(), types.NamespacedName{Name: "captains.crew.example.com"}, crd) + Expect(err).NotTo(HaveOccurred()) + Expect(crd.Spec.Names.Kind).To(Equal("Captain")) + + crd = &v1beta1.CustomResourceDefinition{} + err = c.Get(context.TODO(), types.NamespacedName{Name: "firstmates.crew.example.com"}, crd) + Expect(err).NotTo(HaveOccurred()) + Expect(crd.Spec.Names.Kind).To(Equal("FirstMate")) + + crd = &v1beta1.CustomResourceDefinition{} + err = c.Get(context.TODO(), types.NamespacedName{Name: "drivers.crew.example.com"}, crd) + Expect(err).NotTo(HaveOccurred()) + Expect(crd.Spec.Names.Kind).To(Equal("Driver")) + + err = WaitForCRDs(env.Config, []*v1beta1.CustomResourceDefinition{ + { + Spec: v1beta1.CustomResourceDefinitionSpec{ + Group: "qux.example.com", + Version: "v1beta1", + Names: v1beta1.CustomResourceDefinitionNames{ + Plural: "bazs", + }}, + }, + { + Spec: v1beta1.CustomResourceDefinitionSpec{ + Group: "bar.example.com", + Version: "v1beta1", + Names: v1beta1.CustomResourceDefinitionNames{ + Plural: "foos", + }}, + }, + { + Spec: v1beta1.CustomResourceDefinitionSpec{ + Group: "crew.example.com", + Version: "v1beta1", + Names: v1beta1.CustomResourceDefinitionNames{ + Plural: "captains", + }}, + }, + { + Spec: v1beta1.CustomResourceDefinitionSpec{ + Group: "crew.example.com", + Version: "v1beta1", + Names: v1beta1.CustomResourceDefinitionNames{ + Plural: "firstmates", + }}, + }, + { + Spec: v1beta1.CustomResourceDefinitionSpec{ + Group: "crew.example.com", + Names: v1beta1.CustomResourceDefinitionNames{ + Plural: "drivers", + }, + Versions: []v1beta1.CustomResourceDefinitionVersion{ + { + Name: "v1", + Storage: true, + Served: true, + }, + { + Name: "v2", + Storage: false, + Served: true, + }, + }}, + }, + }, + CRDInstallOptions{MaxTime: 50 * time.Millisecond, PollInterval: 15 * time.Millisecond}, + ) + Expect(err).NotTo(HaveOccurred()) + + err = UninstallCRDs(env.Config, CRDInstallOptions{ + Paths: []string{filepath.Join(".", "testdata")}, + }) + Expect(err).NotTo(HaveOccurred()) + + close(done) + }, 10) + }) }) diff --git a/pkg/envtest/server.go b/pkg/envtest/server.go index ead8972d98..a340b948b4 100644 --- a/pkg/envtest/server.go +++ b/pkg/envtest/server.go @@ -135,8 +135,15 @@ type Environment struct { } // Stop stops a running server. -// If USE_EXISTING_CLUSTER is set to true, this method is a no-op. +// Previously installed CRDs, as listed in CRDInstallOptions.CRDs, will be uninstalled +// if CRDInstallOptions.CleanUpAfterUse are set to true. func (te *Environment) Stop() error { + if te.CRDInstallOptions.CleanUpAfterUse { + if err := UninstallCRDs(te.Config, te.CRDInstallOptions); err != nil { + // This error should be harmless so just log it. + log.Error(err, "failed to uninstall CRDs") + } + } if te.useExistingCluster() { return nil } From bc5ae78df300967f5c1672552447643da048a1e2 Mon Sep 17 00:00:00 2001 From: Pires Date: Wed, 4 Dec 2019 16:52:27 +0000 Subject: [PATCH 2/3] Validate uninstall does clean-up resources Signed-off-by: Pires --- pkg/envtest/envtest_test.go | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/pkg/envtest/envtest_test.go b/pkg/envtest/envtest_test.go index 2e6b9cf07f..dd3f101b43 100644 --- a/pkg/envtest/envtest_test.go +++ b/pkg/envtest/envtest_test.go @@ -325,7 +325,28 @@ var _ = Describe("Test", func() { }) Expect(err).NotTo(HaveOccurred()) + // Expect to NOT find the CRDs + + crds := []string{ + "foos.bar.example.com", + "bazs.qux.example.com", + "captains.crew.example.com", + "firstmates.crew.example.com", + "drivers.crew.example.com", + } + placeholder := &v1beta1.CustomResourceDefinition{} + Eventually(func() bool { + for _, crd := range crds { + err = c.Get(context.TODO(), types.NamespacedName{Name: crd}, placeholder) + notFound := err != nil && apierrors.IsNotFound(err) + if !notFound { + return false + } + } + return true + }, 20).Should(BeTrue()) + close(done) - }, 10) + }, 30) }) }) From 2a965fc278527c6244b679e59bb6d4719266c7e2 Mon Sep 17 00:00:00 2001 From: Pires Date: Wed, 4 Dec 2019 17:19:00 +0000 Subject: [PATCH 3/3] Return CRD uninstall error Signed-off-by: Pires --- pkg/envtest/crd.go | 6 +++++- pkg/envtest/server.go | 3 +-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/envtest/crd.go b/pkg/envtest/crd.go index 8efa29cb2f..0beeb351d7 100644 --- a/pkg/envtest/crd.go +++ b/pkg/envtest/crd.go @@ -27,6 +27,7 @@ import ( apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" @@ -204,7 +205,10 @@ func UninstallCRDs(config *rest.Config, options CRDInstallOptions) error { for _, crd := range options.CRDs { log.V(1).Info("uninstalling CRD", "crd", crd.Name) if err := cs.ApiextensionsV1beta1().CustomResourceDefinitions().Delete(crd.Name, &metav1.DeleteOptions{}); err != nil { - return err + // If CRD is not found, we can consider success + if !apierrors.IsNotFound(err) { + return err + } } } diff --git a/pkg/envtest/server.go b/pkg/envtest/server.go index a340b948b4..bb4c34b6bc 100644 --- a/pkg/envtest/server.go +++ b/pkg/envtest/server.go @@ -140,8 +140,7 @@ type Environment struct { func (te *Environment) Stop() error { if te.CRDInstallOptions.CleanUpAfterUse { if err := UninstallCRDs(te.Config, te.CRDInstallOptions); err != nil { - // This error should be harmless so just log it. - log.Error(err, "failed to uninstall CRDs") + return err } } if te.useExistingCluster() {