From 133a67ce92d4bbf1472aed5ea02fa053f1bfc919 Mon Sep 17 00:00:00 2001 From: "Roland.Ma" Date: Tue, 6 Apr 2021 02:38:04 +0000 Subject: [PATCH 1/3] feat: optional cascade delete resources when deleting workspace Signed-off-by: Roland.Ma --- .../namespace/namespace_controller.go | 21 ++++-- .../workspacetemplate_controller.go | 72 ++++++++++++++++--- pkg/kapis/tenant/v1alpha2/handler.go | 10 ++- pkg/models/tenant/tenant.go | 22 +++++- pkg/utils/k8sutil/k8sutil.go | 11 +++ 5 files changed, 117 insertions(+), 19 deletions(-) diff --git a/pkg/controller/namespace/namespace_controller.go b/pkg/controller/namespace/namespace_controller.go index ecbdd3771..0ca9ad7be 100644 --- a/pkg/controller/namespace/namespace_controller.go +++ b/pkg/controller/namespace/namespace_controller.go @@ -170,6 +170,10 @@ func (r *Reconciler) bindWorkspace(ctx context.Context, logger logr.Logger, name // skip if workspace not found return client.IgnoreNotFound(err) } + // workspace has been deleted + if !workspace.ObjectMeta.DeletionTimestamp.IsZero() { + return r.unbindWorkspace(ctx, logger, namespace) + } // owner reference not match workspace label if !metav1.IsControlledBy(namespace, workspace) { namespace := namespace.DeepCopy() @@ -188,11 +192,18 @@ func (r *Reconciler) bindWorkspace(ctx context.Context, logger logr.Logger, name } func (r *Reconciler) unbindWorkspace(ctx context.Context, logger logr.Logger, namespace *corev1.Namespace) error { - if k8sutil.IsControlledBy(namespace.OwnerReferences, tenantv1alpha1.ResourceKindWorkspace, "") { - namespace := namespace.DeepCopy() - namespace.OwnerReferences = k8sutil.RemoveWorkspaceOwnerReference(namespace.OwnerReferences) - logger.V(4).Info("remove owner reference", "workspace", namespace.Labels[constants.WorkspaceLabelKey]) - if err := r.Update(ctx, namespace); err != nil { + if k8sutil.IsControlledBy(namespace.OwnerReferences, tenantv1alpha1.ResourceKindWorkspace, "") || len(namespace.Labels) > 0 { + ns := namespace.DeepCopy() + + wsName := k8sutil.GetWorkspaceOwnerName(ns.OwnerReferences) + if _, ok := namespace.Labels[tenantv1alpha1.WorkspaceLabel]; ok { + wsName = namespace.Labels[tenantv1alpha1.WorkspaceLabel] + } + + delete(ns.Labels, constants.WorkspaceLabelKey) + ns.OwnerReferences = k8sutil.RemoveWorkspaceOwnerReference(ns.OwnerReferences) + logger.V(4).Info("remove owner reference and label", "namespace", ns.Name, "workspace", wsName) + if err := r.Update(ctx, ns); err != nil { logger.Error(err, "update owner reference failed") return err } diff --git a/pkg/controller/workspacetemplate/workspacetemplate_controller.go b/pkg/controller/workspacetemplate/workspacetemplate_controller.go index d7194d779..07e27854f 100644 --- a/pkg/controller/workspacetemplate/workspacetemplate_controller.go +++ b/pkg/controller/workspacetemplate/workspacetemplate_controller.go @@ -30,7 +30,6 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/yaml" - "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -48,8 +47,11 @@ import ( ) const ( - controllerName = "workspacetemplate-controller" - workspaceTemplateFinalizer = "finalizers.workspacetemplate.kubesphere.io" + controllerName = "workspacetemplate-controller" + workspaceTemplateFinalizer = "finalizers.workspacetemplate.kubesphere.io" + orphanFinalizer = "orphan.finalizers.kubesphere.io" + orphanDeleteOptionAnnotationKey = "kubefed.io/deleteoption" + orphanDeleteOptionAnnotation = "{\"propagationPolicy\":\"Orphan\"}" ) // Reconciler reconciles a WorkspaceRoleBinding object @@ -105,16 +107,30 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { } } else { // The object is being deleted - if sliceutil.HasString(workspaceTemplate.ObjectMeta.Finalizers, workspaceTemplateFinalizer) { + if sliceutil.HasString(workspaceTemplate.ObjectMeta.Finalizers, workspaceTemplateFinalizer) || + sliceutil.HasString(workspaceTemplate.ObjectMeta.Finalizers, orphanFinalizer) { if err := r.deleteOpenPitrixResourcesInWorkspace(rootCtx, workspaceTemplate.Name); err != nil { logger.Error(err, "delete resource in workspace template failed") return ctrl.Result{}, err } + if err := r.deleteWorkspace(rootCtx, workspaceTemplate); err != nil { + if errors.IsNotFound(err) { + logger.V(4).Info("workspace not found", "workspacerole", workspaceTemplate.Name) + } else { + logger.Error(err, "failed delete workspaces") + return ctrl.Result{}, nil + } + } + // remove our finalizer from the list and update it. workspaceTemplate.ObjectMeta.Finalizers = sliceutil.RemoveString(workspaceTemplate.ObjectMeta.Finalizers, func(item string) bool { return item == workspaceTemplateFinalizer }) + workspaceTemplate.ObjectMeta.Finalizers = sliceutil.RemoveString(workspaceTemplate.ObjectMeta.Finalizers, func(item string) bool { + return item == orphanFinalizer + }) + logger.V(4).Info("update workspace template") if err := r.Update(rootCtx, workspaceTemplate); err != nil { logger.Error(err, "update workspace template failed") @@ -224,9 +240,6 @@ func newFederatedWorkspace(template *tenantv1alpha2.WorkspaceTemplate) (*typesv1 }, Spec: template.Spec, } - if err := controllerutil.SetControllerReference(template, federatedWorkspace, scheme.Scheme); err != nil { - return nil, err - } return federatedWorkspace, nil } @@ -238,12 +251,51 @@ func newWorkspace(template *tenantv1alpha2.WorkspaceTemplate) (*tenantv1alpha1.W }, Spec: template.Spec.Template.Spec, } - if err := controllerutil.SetControllerReference(template, workspace, scheme.Scheme); err != nil { - return nil, err - } + return workspace, nil } +func (r *Reconciler) deleteWorkspace(ctx context.Context, template *tenantv1alpha2.WorkspaceTemplate) error { + if r.MultiClusterEnabled { + federatedWorkspace := &typesv1beta1.FederatedWorkspace{} + if err := r.Client.Get(ctx, types.NamespacedName{Name: template.Name}, federatedWorkspace); err != nil { + return err + } + // Workspace will be deleted with Orphan Option when it has a orphan finalizer. + // Reousrces that owned by the Workspace will not be deleted. + if sliceutil.HasString(template.ObjectMeta.Finalizers, orphanFinalizer) { + if federatedWorkspace.Annotations == nil { + federatedWorkspace.Annotations = make(map[string]string, 1) + } + federatedWorkspace.Annotations[orphanDeleteOptionAnnotationKey] = orphanDeleteOptionAnnotation + if err := r.Update(ctx, federatedWorkspace); err != nil { + return err + } + } + if err := r.Delete(ctx, federatedWorkspace); err != nil { + return err + } + } + + opt := &client.DeleteOptions{} + // Dependents won't be deleted when it's has a orphanFinalizer + if sliceutil.HasString(template.ObjectMeta.Finalizers, orphanFinalizer) { + orphan := metav1.DeletePropagationOrphan + opt = &client.DeleteOptions{PropagationPolicy: &orphan} + } + + ws := &tenantv1alpha1.Workspace{ + ObjectMeta: metav1.ObjectMeta{ + Name: template.Name, + }, + } + if err := r.Delete(ctx, ws, opt); err != nil { + return err + } + + return nil +} + func (r *Reconciler) ensureNotControlledByKubefed(ctx context.Context, logger logr.Logger, workspaceTemplate *tenantv1alpha2.WorkspaceTemplate) error { if workspaceTemplate.Labels[constants.KubefedManagedLabel] != "false" { if workspaceTemplate.Labels == nil { diff --git a/pkg/kapis/tenant/v1alpha2/handler.go b/pkg/kapis/tenant/v1alpha2/handler.go index 70611d683..8c5b99774 100644 --- a/pkg/kapis/tenant/v1alpha2/handler.go +++ b/pkg/kapis/tenant/v1alpha2/handler.go @@ -23,6 +23,7 @@ import ( "github.com/emicklei/go-restful" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/client-go/kubernetes" "k8s.io/klog" @@ -219,7 +220,14 @@ func (h *tenantHandler) CreateWorkspace(request *restful.Request, response *rest func (h *tenantHandler) DeleteWorkspace(request *restful.Request, response *restful.Response) { workspace := request.PathParameter("workspace") - err := h.tenant.DeleteWorkspace(workspace) + opts := metav1.DeleteOptions{} + + err := request.ReadEntity(&opts) + if err != nil { + opts = *metav1.NewDeleteOptions(0) + } + + err = h.tenant.DeleteWorkspace(workspace, opts) if err != nil { klog.Error(err) diff --git a/pkg/models/tenant/tenant.go b/pkg/models/tenant/tenant.go index 1b9001acc..f4e1a502a 100644 --- a/pkg/models/tenant/tenant.go +++ b/pkg/models/tenant/tenant.go @@ -67,6 +67,8 @@ import ( "kubesphere.io/kubesphere/pkg/utils/stringutils" ) +const orphanFinalizer = "orphan.finalizers.kubesphere.io" + type Interface interface { ListWorkspaces(user user.Info, query *query.Query) (*api.ListResult, error) ListNamespaces(user user.Info, workspace string, query *query.Query) (*api.ListResult, error) @@ -74,7 +76,7 @@ type Interface interface { ListFederatedNamespaces(info user.Info, workspace string, param *query.Query) (*api.ListResult, error) CreateNamespace(workspace string, namespace *corev1.Namespace) (*corev1.Namespace, error) CreateWorkspace(workspace *tenantv1alpha2.WorkspaceTemplate) (*tenantv1alpha2.WorkspaceTemplate, error) - DeleteWorkspace(workspace string) error + DeleteWorkspace(workspace string, opts metav1.DeleteOptions) error UpdateWorkspace(workspace *tenantv1alpha2.WorkspaceTemplate) (*tenantv1alpha2.WorkspaceTemplate, error) DescribeWorkspace(workspace string) (*tenantv1alpha2.WorkspaceTemplate, error) ListWorkspaceClusters(workspace string) (*api.ListResult, error) @@ -538,8 +540,22 @@ func (t *tenantOperator) ListClusters(user user.Info) (*api.ListResult, error) { return &api.ListResult{Items: items, TotalItems: len(items)}, nil } -func (t *tenantOperator) DeleteWorkspace(workspace string) error { - return t.ksclient.TenantV1alpha2().WorkspaceTemplates().Delete(context.Background(), workspace, *metav1.NewDeleteOptions(0)) +func (t *tenantOperator) DeleteWorkspace(workspace string, opts metav1.DeleteOptions) error { + + if opts.PropagationPolicy != nil && *opts.PropagationPolicy == metav1.DeletePropagationOrphan { + wsp, err := t.DescribeWorkspace(workspace) + if err != nil { + klog.Error(err) + return err + } + wsp.Finalizers = append(wsp.Finalizers, orphanFinalizer) + _, err = t.ksclient.TenantV1alpha2().WorkspaceTemplates().Update(context.Background(), wsp, metav1.UpdateOptions{}) + if err != nil { + klog.Error(err) + return err + } + } + return t.ksclient.TenantV1alpha2().WorkspaceTemplates().Delete(context.Background(), workspace, opts) } // listIntersectedNamespaces returns a list of namespaces that MUST meet ALL the following filters: diff --git a/pkg/utils/k8sutil/k8sutil.go b/pkg/utils/k8sutil/k8sutil.go index 785e142e3..8b6e6b066 100644 --- a/pkg/utils/k8sutil/k8sutil.go +++ b/pkg/utils/k8sutil/k8sutil.go @@ -44,3 +44,14 @@ func RemoveWorkspaceOwnerReference(ownerReferences []metav1.OwnerReference) []me } return tmp } + +// GetWorkspaceOwnerName return workspace kind owner name +func GetWorkspaceOwnerName(ownerReferences []metav1.OwnerReference) string { + for _, owner := range ownerReferences { + if owner.Kind == tenantv1alpha1.ResourceKindWorkspace || + owner.Kind == tenantv1alpha2.ResourceKindWorkspaceTemplate { + return owner.Name + } + } + return "" +} From c3723a37381d7f0da6e7bcbd6b4a9af1a0b590ab Mon Sep 17 00:00:00 2001 From: "Roland.Ma" Date: Tue, 6 Apr 2021 07:32:53 +0000 Subject: [PATCH 2/3] refined e2e testing Signed-off-by: Roland.Ma --- .../workspacetemplate_controller.go | 2 + ...workspacetemplate_controller_suite_test.go | 62 +------- .../workspacetemplate_controller_test.go | 138 ++++++++++-------- 3 files changed, 88 insertions(+), 114 deletions(-) diff --git a/pkg/controller/workspacetemplate/workspacetemplate_controller.go b/pkg/controller/workspacetemplate/workspacetemplate_controller.go index 07e27854f..5135b8fcb 100644 --- a/pkg/controller/workspacetemplate/workspacetemplate_controller.go +++ b/pkg/controller/workspacetemplate/workspacetemplate_controller.go @@ -172,6 +172,7 @@ func (r *Reconciler) singleClusterSync(ctx context.Context, logger logr.Logger, logger.Error(err, "create workspace failed") return err } + return nil } } logger.Error(err, "get workspace failed") @@ -207,6 +208,7 @@ func (r *Reconciler) multiClusterSync(ctx context.Context, logger logr.Logger, w logger.Error(err, "create federated workspace failed") return err } + return nil } } logger.Error(err, "get federated workspace failed") diff --git a/pkg/controller/workspacetemplate/workspacetemplate_controller_suite_test.go b/pkg/controller/workspacetemplate/workspacetemplate_controller_suite_test.go index 4a79c7196..fab1724d4 100644 --- a/pkg/controller/workspacetemplate/workspacetemplate_controller_suite_test.go +++ b/pkg/controller/workspacetemplate/workspacetemplate_controller_suite_test.go @@ -17,35 +17,21 @@ limitations under the License. package workspacetemplate import ( - "os" - "path/filepath" "testing" - "time" - "github.com/onsi/gomega/gexec" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/kubernetes/scheme" "k8s.io/klog/klogr" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" "kubesphere.io/kubesphere/pkg/apis" helmappscheme "kubesphere.io/kubesphere/pkg/apis/application/v1alpha1" + typesv1beta1 "kubesphere.io/kubesphere/pkg/apis/types/v1beta1" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "sigs.k8s.io/controller-runtime/pkg/envtest" "sigs.k8s.io/controller-runtime/pkg/envtest/printer" ) -// These tests use Ginkgo (BDD-style Go testing framework). Refer to -// http://onsi.github.io/ginkgo/ to learn more about Ginkgo. - -var k8sClient client.Client -var k8sManager ctrl.Manager -var testEnv *envtest.Environment - func TestWorkspaceTemplateController(t *testing.T) { RegisterFailHandler(Fail) RunSpecsWithDefaultAndCustomReporters(t, @@ -56,51 +42,13 @@ func TestWorkspaceTemplateController(t *testing.T) { var _ = BeforeSuite(func(done Done) { logf.SetLogger(klogr.New()) - By("bootstrapping test environment") - t := true - if os.Getenv("TEST_USE_EXISTING_CLUSTER") == "true" { - testEnv = &envtest.Environment{ - UseExistingCluster: &t, - } - } else { - testEnv = &envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "config", "crds")}, - AttachControlPlaneOutput: false, - } - } - - cfg, err := testEnv.Start() - Expect(err).ToNot(HaveOccurred()) - Expect(cfg).ToNot(BeNil()) - + err := helmappscheme.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) err = apis.AddToScheme(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) - k8sManager, err = ctrl.NewManager(cfg, ctrl.Options{ - Scheme: scheme.Scheme, - MetricsBindAddress: "0", - }) - Expect(err).ToNot(HaveOccurred()) - - utilruntime.Must(helmappscheme.AddToScheme(k8sManager.GetScheme())) - - err = (&Reconciler{}).SetupWithManager(k8sManager) - Expect(err).ToNot(HaveOccurred()) - - go func() { - err = k8sManager.Start(ctrl.SetupSignalHandler()) - Expect(err).ToNot(HaveOccurred()) - }() - - k8sClient = k8sManager.GetClient() - Expect(k8sClient).ToNot(BeNil()) + err = typesv1beta1.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) close(done) }, 60) - -var _ = AfterSuite(func() { - By("tearing down the test environment") - gexec.KillAndWait(5 * time.Second) - err := testEnv.Stop() - Expect(err).ToNot(HaveOccurred()) -}) diff --git a/pkg/controller/workspacetemplate/workspacetemplate_controller_test.go b/pkg/controller/workspacetemplate/workspacetemplate_controller_test.go index 481f927bf..e574079e8 100644 --- a/pkg/controller/workspacetemplate/workspacetemplate_controller_test.go +++ b/pkg/controller/workspacetemplate/workspacetemplate_controller_test.go @@ -26,26 +26,38 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" iamv1alpha2 "kubesphere.io/kubesphere/pkg/apis/iam/v1alpha2" tenantv1alpha1 "kubesphere.io/kubesphere/pkg/apis/tenant/v1alpha1" tenantv1alpha2 "kubesphere.io/kubesphere/pkg/apis/tenant/v1alpha2" ) +var reconciler *Reconciler var _ = Describe("WorkspaceTemplate", func() { const timeout = time.Second * 30 const interval = time.Second * 1 BeforeEach(func() { + + reconciler = &Reconciler{ + Client: fake.NewFakeClientWithScheme(scheme.Scheme), + Logger: ctrl.Log.WithName("controllers").WithName("acrpullbinding-controller"), + Recorder: record.NewFakeRecorder(5), + } + workspaceAdmin := newWorkspaceAdmin() - err := k8sClient.Create(context.Background(), &workspaceAdmin) + err := reconciler.Create(context.Background(), &workspaceAdmin) Expect(err).NotTo(HaveOccurred()) admin := iamv1alpha2.User{ObjectMeta: metav1.ObjectMeta{Name: "admin"}} - err = k8sClient.Create(context.Background(), &admin) + err = reconciler.Create(context.Background(), &admin) Expect(err).NotTo(HaveOccurred()) }) @@ -54,70 +66,82 @@ var _ = Describe("WorkspaceTemplate", func() { // Avoid adding tests for vanilla CRUD operations because they would // test Kubernetes API server, which isn't the goal here. Context("WorkspaceTemplate Controller", func() { - It("Should create successfully", func() { - key := types.NamespacedName{ - Name: "workspace-template", - } + for _, multiCluster := range []bool{true, false} { + enalbed := multiCluster + It("Should create successfully", func() { + reconciler.MultiClusterEnabled = enalbed + key := types.NamespacedName{ + Name: "workspace-template", + } - created := &tenantv1alpha2.WorkspaceTemplate{ - ObjectMeta: metav1.ObjectMeta{ - Name: key.Name, - }, - } + created := &tenantv1alpha2.WorkspaceTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: key.Name, + }, + } - // Create - Expect(k8sClient.Create(context.Background(), created)).Should(Succeed()) + // Create + Expect(reconciler.Create(context.Background(), created)).Should(Succeed()) - By("Expecting to create workspace template successfully") - Eventually(func() bool { - f := &tenantv1alpha2.WorkspaceTemplate{} - k8sClient.Get(context.Background(), key, f) - return !f.CreationTimestamp.IsZero() - }, timeout, interval).Should(BeTrue()) + req := ctrl.Request{ + NamespacedName: key, + } + _, err := reconciler.Reconcile(req) + Expect(err).To(BeNil()) - By("Expecting to create workspace successfully") - Eventually(func() bool { - f := &tenantv1alpha1.Workspace{} - k8sClient.Get(context.Background(), key, f) - return !f.CreationTimestamp.IsZero() - }, timeout, interval).Should(BeTrue()) + By("Expecting to create workspace template successfully") + Expect(func() *tenantv1alpha2.WorkspaceTemplate { + f := &tenantv1alpha2.WorkspaceTemplate{} + reconciler.Get(context.Background(), key, f) + return f + }()).ShouldNot(BeNil()) - // List workspace roles - By("Expecting to create workspace role successfully") - Eventually(func() bool { - f := &iamv1alpha2.WorkspaceRoleList{} - k8sClient.List(context.Background(), f, &client.ListOptions{LabelSelector: labels.SelectorFromSet(labels.Set{tenantv1alpha1.WorkspaceLabel: key.Name})}) - return len(f.Items) == 1 - }, timeout, interval).Should(BeTrue()) + By("Expecting to create workspace successfully") + Expect(func() *tenantv1alpha1.Workspace { + f := &tenantv1alpha1.Workspace{} + reconciler.Get(context.Background(), key, f) + return f + }()).ShouldNot(BeNil()) - // Update - updated := &tenantv1alpha2.WorkspaceTemplate{} - Expect(k8sClient.Get(context.Background(), key, updated)).Should(Succeed()) - updated.Spec.Template.Spec.Manager = "admin" - Expect(k8sClient.Update(context.Background(), updated)).Should(Succeed()) + // List workspace roles + By("Expecting to create workspace role successfully") + Eventually(func() bool { + f := &iamv1alpha2.WorkspaceRoleList{} + reconciler.List(context.Background(), f, &client.ListOptions{LabelSelector: labels.SelectorFromSet(labels.Set{tenantv1alpha1.WorkspaceLabel: key.Name})}) + return len(f.Items) == 1 + }, timeout, interval).Should(BeTrue()) - // List workspace role bindings - By("Expecting to create workspace manager role binding successfully") - Eventually(func() bool { - f := &iamv1alpha2.WorkspaceRoleBindingList{} - k8sClient.List(context.Background(), f, &client.ListOptions{LabelSelector: labels.SelectorFromSet(labels.Set{tenantv1alpha1.WorkspaceLabel: key.Name})}) - return len(f.Items) == 1 - }, timeout, interval).Should(BeTrue()) + // Update + updated := &tenantv1alpha2.WorkspaceTemplate{} + Expect(reconciler.Get(context.Background(), key, updated)).Should(Succeed()) + updated.Spec.Template.Spec.Manager = "admin" + Expect(reconciler.Update(context.Background(), updated)).Should(Succeed()) - // Delete - By("Expecting to delete workspace successfully") - Eventually(func() error { - f := &tenantv1alpha2.WorkspaceTemplate{} - k8sClient.Get(context.Background(), key, f) - return k8sClient.Delete(context.Background(), f) - }, timeout, interval).Should(Succeed()) + _, err = reconciler.Reconcile(req) + Expect(err).To(BeNil()) - By("Expecting to delete workspace finish") - Eventually(func() error { - f := &tenantv1alpha2.WorkspaceTemplate{} - return k8sClient.Get(context.Background(), key, f) - }, timeout, interval).ShouldNot(Succeed()) - }) + // List workspace role bindings + By("Expecting to create workspace manager role binding successfully") + Eventually(func() bool { + f := &iamv1alpha2.WorkspaceRoleBindingList{} + reconciler.List(context.Background(), f, &client.ListOptions{LabelSelector: labels.SelectorFromSet(labels.Set{tenantv1alpha1.WorkspaceLabel: key.Name})}) + return len(f.Items) == 1 + }, timeout, interval).Should(BeTrue()) + + // Delete + By("Expecting to finalize workspace successfully") + Eventually(func() error { + f := &tenantv1alpha2.WorkspaceTemplate{} + reconciler.Get(context.Background(), key, f) + now := metav1.NewTime(time.Now()) + f.DeletionTimestamp = &now + return reconciler.Update(context.Background(), f) + }, timeout, interval).Should(Succeed()) + + _, err = reconciler.Reconcile(req) + Expect(err).To(BeNil()) + }) + } }) }) From 68e83274531215f4a41ab7f3a6ada571e7b41326 Mon Sep 17 00:00:00 2001 From: "Roland.Ma" Date: Wed, 7 Apr 2021 03:53:05 +0000 Subject: [PATCH 3/3] refine code Signed-off-by: Roland.Ma --- pkg/controller/namespace/namespace_controller.go | 5 +++-- .../workspacetemplate/workspacetemplate_controller.go | 5 +---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/pkg/controller/namespace/namespace_controller.go b/pkg/controller/namespace/namespace_controller.go index 0ca9ad7be..af2a2c331 100644 --- a/pkg/controller/namespace/namespace_controller.go +++ b/pkg/controller/namespace/namespace_controller.go @@ -192,11 +192,12 @@ func (r *Reconciler) bindWorkspace(ctx context.Context, logger logr.Logger, name } func (r *Reconciler) unbindWorkspace(ctx context.Context, logger logr.Logger, namespace *corev1.Namespace) error { - if k8sutil.IsControlledBy(namespace.OwnerReferences, tenantv1alpha1.ResourceKindWorkspace, "") || len(namespace.Labels) > 0 { + _, hasWorkspaceLabel := namespace.Labels[tenantv1alpha1.WorkspaceLabel] + if hasWorkspaceLabel || k8sutil.IsControlledBy(namespace.OwnerReferences, tenantv1alpha1.ResourceKindWorkspace, "") { ns := namespace.DeepCopy() wsName := k8sutil.GetWorkspaceOwnerName(ns.OwnerReferences) - if _, ok := namespace.Labels[tenantv1alpha1.WorkspaceLabel]; ok { + if hasWorkspaceLabel { wsName = namespace.Labels[tenantv1alpha1.WorkspaceLabel] } diff --git a/pkg/controller/workspacetemplate/workspacetemplate_controller.go b/pkg/controller/workspacetemplate/workspacetemplate_controller.go index 5135b8fcb..a4a6a9879 100644 --- a/pkg/controller/workspacetemplate/workspacetemplate_controller.go +++ b/pkg/controller/workspacetemplate/workspacetemplate_controller.go @@ -125,10 +125,7 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { // remove our finalizer from the list and update it. workspaceTemplate.ObjectMeta.Finalizers = sliceutil.RemoveString(workspaceTemplate.ObjectMeta.Finalizers, func(item string) bool { - return item == workspaceTemplateFinalizer - }) - workspaceTemplate.ObjectMeta.Finalizers = sliceutil.RemoveString(workspaceTemplate.ObjectMeta.Finalizers, func(item string) bool { - return item == orphanFinalizer + return item == workspaceTemplateFinalizer || item == orphanFinalizer }) logger.V(4).Info("update workspace template")