From c726458a7935c6c03f7d810f4de4d45e034d4b57 Mon Sep 17 00:00:00 2001 From: Ginny Ji Date: Thu, 3 Dec 2020 19:46:43 +0000 Subject: [PATCH] Create client for checking CRD deletion The default client uses cache, and therefore, it might fail to notice if a CR is deleted. We instead use our own client who doesn't use the cache. Tested: make test-e2e --- incubator/hnc/cmd/manager/main.go | 2 +- incubator/hnc/internal/reconcilers/anchor.go | 2 +- .../internal/reconcilers/hierarchy_config.go | 2 +- .../hnc/internal/reconcilers/hnc_config.go | 2 +- incubator/hnc/internal/reconcilers/setup.go | 38 +++++++++++++++++-- .../hnc/internal/reconcilers/suite_test.go | 3 +- 6 files changed, 41 insertions(+), 8 deletions(-) diff --git a/incubator/hnc/cmd/manager/main.go b/incubator/hnc/cmd/manager/main.go index a7d0f6a2b..da7239ea8 100644 --- a/incubator/hnc/cmd/manager/main.go +++ b/incubator/hnc/cmd/manager/main.go @@ -192,7 +192,7 @@ func startControllers(mgr ctrl.Manager, certsCreated chan struct{}) { // Create all reconciling controllers setupLog.Info("Creating controllers", "maxReconciles", maxReconciles) - if err := reconcilers.Create(mgr, f, maxReconciles); err != nil { + if err := reconcilers.Create(mgr, f, maxReconciles, false); err != nil { setupLog.Error(err, "cannot create controllers") os.Exit(1) } diff --git a/incubator/hnc/internal/reconcilers/anchor.go b/incubator/hnc/internal/reconcilers/anchor.go index 6b7b0cb02..58924032d 100644 --- a/incubator/hnc/internal/reconcilers/anchor.go +++ b/incubator/hnc/internal/reconcilers/anchor.go @@ -128,7 +128,7 @@ func (r *AnchorReconciler) onDeleting(ctx context.Context, log logr.Logger, inst // We handle deletions differently depending on whether _one_ anchor is being deleted (i.e., the // user wants to delete the namespace) or whether the Anchor CRD is being deleted, which usually // means HNC is being uninstalled and we shouldn't delete _any_ namespaces. - deletingCRD, err := isDeletingCRD(ctx, r, api.Anchors) + deletingCRD, err := isDeletingCRD(ctx, api.Anchors) if err != nil { log.Error(err, "Couldn't determine if CRD is being deleted") return false, err diff --git a/incubator/hnc/internal/reconcilers/hierarchy_config.go b/incubator/hnc/internal/reconcilers/hierarchy_config.go index 40de66867..2267c6c28 100644 --- a/incubator/hnc/internal/reconcilers/hierarchy_config.go +++ b/incubator/hnc/internal/reconcilers/hierarchy_config.go @@ -625,7 +625,7 @@ func (r *HierarchyConfigReconciler) getSingleton(ctx context.Context, nm string) deletingCRD := false if inst.CreationTimestamp.IsZero() || !inst.DeletionTimestamp.IsZero() { var err error - deletingCRD, err = isDeletingCRD(ctx, r, api.HierarchyConfigurations) + deletingCRD, err = isDeletingCRD(ctx, api.HierarchyConfigurations) if err != nil { return nil, false, err } diff --git a/incubator/hnc/internal/reconcilers/hnc_config.go b/incubator/hnc/internal/reconcilers/hnc_config.go index a14f19877..875a0e9ed 100644 --- a/incubator/hnc/internal/reconcilers/hnc_config.go +++ b/incubator/hnc/internal/reconcilers/hnc_config.go @@ -235,7 +235,7 @@ func (r *ConfigReconciler) validateSingleton(inst *api.HNCConfiguration) { func (r *ConfigReconciler) writeSingleton(ctx context.Context, inst *api.HNCConfiguration) error { if inst.CreationTimestamp.IsZero() { // No point creating it if the CRD's being deleted - if isDeleted, err := isDeletingCRD(ctx, r, api.HNCConfigSingletons); isDeleted || err != nil { + if isDeleted, err := isDeletingCRD(ctx, api.HNCConfigSingletons); isDeleted || err != nil { r.Log.Info("CRD is being deleted (or CRD deletion status couldn't be determined); skip update") return err } diff --git a/incubator/hnc/internal/reconcilers/setup.go b/incubator/hnc/internal/reconcilers/setup.go index f690fa054..b60e29041 100644 --- a/incubator/hnc/internal/reconcilers/setup.go +++ b/incubator/hnc/internal/reconcilers/setup.go @@ -9,25 +9,57 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/config" "sigs.k8s.io/controller-runtime/pkg/event" api "sigs.k8s.io/multi-tenancy/incubator/hnc/api/v1alpha2" "sigs.k8s.io/multi-tenancy/incubator/hnc/internal/forest" ) +// FakeDeleteCRDClient is a "fake" client used for testing only +type FakeDeleteCRDClient struct{} + +// FakeDeleteCRDClient doesn't return any err on Get() because none of the reconciler test performs CRD deletion +func (f FakeDeleteCRDClient) Get(context.Context, types.NamespacedName, runtime.Object) error { + return nil +} + var crds = map[string]bool{ "hncconfigurations.hnc.x-k8s.io": true, "subnamespaceanchors.hnc.x-k8s.io": true, "hierarchyconfigurations.hnc.x-k8s.io": true, } +// deleteCRDClientType could be either a real client or FakeDeleteCRDClient +type deleteCRDClientType interface { + Get(context.Context, types.NamespacedName, runtime.Object) error +} + +// deleteCRDClient is an uncached client for checking CRD deletion +var deleteCRDClient deleteCRDClientType + // Create creates all reconcilers. // // This function is called both from main.go as well as from the integ tests. -func Create(mgr ctrl.Manager, f *forest.Forest, maxReconciles int) error { +func Create(mgr ctrl.Manager, f *forest.Forest, maxReconciles int, useFakeClient bool) error { hcChan := make(chan event.GenericEvent) anchorChan := make(chan event.GenericEvent) + // Create uncached client for CRD deletion check + if !useFakeClient { + var err error + deleteCRDClient, err = client.New(config.GetConfigOrDie(), client.Options{ + Scheme: mgr.GetScheme(), + // I'm not sure if this mapper is needed - @ginnyji Dec2020 + Mapper: mgr.GetRESTMapper(), + }) + if err != nil { + return fmt.Errorf("cannot create deleteCRDClient: %s", err.Error()) + } + } else { + deleteCRDClient = FakeDeleteCRDClient{} + } // Create AnchorReconciler. sar := &AnchorReconciler{ Client: mgr.GetClient(), @@ -74,10 +106,10 @@ type crdClient interface { // isDeletingCRD returns true if the specified HNC CRD is being or has been deleted. The argument // expected is the CRD name minus the HNC suffix, e.g. "hierarchyconfigurations". -func isDeletingCRD(ctx context.Context, c crdClient, nm string) (bool, error) { +func isDeletingCRD(ctx context.Context, nm string) (bool, error) { crd := &apiextensions.CustomResourceDefinition{} nsn := types.NamespacedName{Name: nm + "." + api.MetaGroup} - if err := c.Get(ctx, nsn, crd); err != nil { + if err := deleteCRDClient.Get(ctx, nsn, crd); err != nil { if apierrors.IsNotFound(err) { return true, nil } diff --git a/incubator/hnc/internal/reconcilers/suite_test.go b/incubator/hnc/internal/reconcilers/suite_test.go index e998c9fbb..8b4eded7a 100644 --- a/incubator/hnc/internal/reconcilers/suite_test.go +++ b/incubator/hnc/internal/reconcilers/suite_test.go @@ -35,6 +35,7 @@ import ( // +kubebuilder:scaffold:imports + _ "k8s.io/client-go/plugin/pkg/client/auth" api "sigs.k8s.io/multi-tenancy/incubator/hnc/api/v1alpha2" "sigs.k8s.io/multi-tenancy/incubator/hnc/internal/forest" "sigs.k8s.io/multi-tenancy/incubator/hnc/internal/reconcilers" @@ -99,7 +100,7 @@ var _ = BeforeSuite(func(done Done) { Scheme: scheme.Scheme, }) Expect(err).ToNot(HaveOccurred()) - err = reconcilers.Create(k8sManager, forest.NewForest(), 100) + err = reconcilers.Create(k8sManager, forest.NewForest(), 100, true) Expect(err).ToNot(HaveOccurred()) k8sClient = k8sManager.GetClient()