From c022f2268288ebccc71365eec7981b81e2624fc2 Mon Sep 17 00:00:00 2001 From: free6om Date: Tue, 23 Apr 2024 18:06:25 +0800 Subject: [PATCH] chore: move IsOwnedByInstanceSet from pkg instanceset to apps --- controllers/apps/transform_utils.go | 11 ++++++ controllers/apps/transform_utils_test.go | 24 +++++++++++++ .../apps/transformer_cluster_deletion.go | 3 +- .../apps/transformer_component_deletion.go | 3 +- pkg/controller/instanceset/utils.go | 18 ++-------- pkg/controller/instanceset/utils_test.go | 35 +------------------ 6 files changed, 41 insertions(+), 53 deletions(-) diff --git a/controllers/apps/transform_utils.go b/controllers/apps/transform_utils.go index 59ae7a39eb0..0d2f33e7090 100644 --- a/controllers/apps/transform_utils.go +++ b/controllers/apps/transform_utils.go @@ -23,6 +23,7 @@ import ( "context" "encoding/json" "errors" + workloads "github.com/apecloud/kubeblocks/apis/workloads/v1alpha1" "reflect" "time" @@ -247,3 +248,13 @@ func isOwnedByComp(obj client.Object) bool { } return false } + +// isOwnedByInstanceSet is used to judge if the obj is owned by the InstanceSet controller +func isOwnedByInstanceSet(obj client.Object) bool { + for _, ref := range obj.GetOwnerReferences() { + if ref.Kind == workloads.Kind && ref.Controller != nil && *ref.Controller { + return true + } + } + return false +} diff --git a/controllers/apps/transform_utils_test.go b/controllers/apps/transform_utils_test.go index 345828a0a2a..71927274d9d 100644 --- a/controllers/apps/transform_utils_test.go +++ b/controllers/apps/transform_utils_test.go @@ -29,11 +29,14 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/discovery" + "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1" + workloads "github.com/apecloud/kubeblocks/apis/workloads/v1alpha1" ) func TestReflect(t *testing.T) { @@ -120,3 +123,24 @@ func TestIsPolicyV1DiscoveryNotFoundError(t *testing.T) { // wrapped policy/v1 not-found error assert.True(t, isPolicyV1DiscoveryNotFoundError(fmt.Errorf("%w", discoveryErr))) } + +func TestIsOwnedByInstanceSet(t *testing.T) { + its := &workloads.InstanceSet{} + assert.False(t, isOwnedByInstanceSet(its)) + + its.OwnerReferences = []metav1.OwnerReference{ + { + Kind: workloads.Kind, + Controller: pointer.Bool(true), + }, + } + assert.True(t, isOwnedByInstanceSet(its)) + + its.OwnerReferences = []metav1.OwnerReference{ + { + Kind: reflect.TypeOf(appsv1alpha1.Cluster{}).Name(), + Controller: pointer.Bool(true), + }, + } + assert.False(t, isOwnedByInstanceSet(its)) +} diff --git a/controllers/apps/transformer_cluster_deletion.go b/controllers/apps/transformer_cluster_deletion.go index e937e538294..48d542d2603 100644 --- a/controllers/apps/transformer_cluster_deletion.go +++ b/controllers/apps/transformer_cluster_deletion.go @@ -37,7 +37,6 @@ import ( dpv1alpha1 "github.com/apecloud/kubeblocks/apis/dataprotection/v1alpha1" "github.com/apecloud/kubeblocks/pkg/constant" "github.com/apecloud/kubeblocks/pkg/controller/graph" - "github.com/apecloud/kubeblocks/pkg/controller/instanceset" "github.com/apecloud/kubeblocks/pkg/controller/model" intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil" ) @@ -123,7 +122,7 @@ func (t *clusterDeletionTransformer) Transform(ctx graph.TransformContext, dag * for _, o := range delObjs { // skip the objects owned by the component and rsm controller - if shouldSkipObjOwnedByComp(o, *cluster) || instanceset.IsOwnedByRsm(o) { + if shouldSkipObjOwnedByComp(o, *cluster) || isOwnedByInstanceSet(o) { continue } graphCli.Delete(dag, o, inUniversalContext4G()) diff --git a/controllers/apps/transformer_component_deletion.go b/controllers/apps/transformer_component_deletion.go index 280fb30c1b4..cc299c515db 100644 --- a/controllers/apps/transformer_component_deletion.go +++ b/controllers/apps/transformer_component_deletion.go @@ -38,7 +38,6 @@ import ( "github.com/apecloud/kubeblocks/pkg/constant" "github.com/apecloud/kubeblocks/pkg/controller/component" "github.com/apecloud/kubeblocks/pkg/controller/graph" - "github.com/apecloud/kubeblocks/pkg/controller/instanceset" "github.com/apecloud/kubeblocks/pkg/controller/model" intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil" ) @@ -140,7 +139,7 @@ func (t *componentDeletionTransformer) deleteCompResources(transCtx *componentTr if len(snapshot) > 0 { // delete the sub-resources owned by the component before deleting the component for _, object := range snapshot { - if instanceset.IsOwnedByRsm(object) { + if isOwnedByInstanceSet(object) { continue } graphCli.Delete(dag, object) diff --git a/pkg/controller/instanceset/utils.go b/pkg/controller/instanceset/utils.go index 47267b6d0fd..ffed05519c2 100644 --- a/pkg/controller/instanceset/utils.go +++ b/pkg/controller/instanceset/utils.go @@ -23,14 +23,12 @@ import ( "fmt" "strings" - "golang.org/x/exp/slices" - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - workloads "github.com/apecloud/kubeblocks/apis/workloads/v1alpha1" "github.com/apecloud/kubeblocks/pkg/constant" "github.com/apecloud/kubeblocks/pkg/controller/builder" + "golang.org/x/exp/slices" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" ) const ( @@ -204,16 +202,6 @@ func GetEnvConfigMapName(rsmName string) string { return fmt.Sprintf("%s-its-env", rsmName) } -// IsOwnedByRsm is used to judge if the obj is owned by rsm -func IsOwnedByRsm(obj client.Object) bool { - for _, ref := range obj.GetOwnerReferences() { - if ref.Kind == workloads.Kind && ref.Controller != nil && *ref.Controller { - return true - } - } - return false -} - func composeRoleMap(rsm workloads.InstanceSet) map[string]workloads.ReplicaRole { roleMap := make(map[string]workloads.ReplicaRole) for _, role := range rsm.Spec.Roles { diff --git a/pkg/controller/instanceset/utils_test.go b/pkg/controller/instanceset/utils_test.go index 79598fb89de..3cf160ff025 100644 --- a/pkg/controller/instanceset/utils_test.go +++ b/pkg/controller/instanceset/utils_test.go @@ -21,18 +21,12 @@ package instanceset import ( "fmt" - "reflect" - . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/apecloud/kubeblocks/pkg/controller/builder" "golang.org/x/exp/slices" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/apecloud/kubeblocks/apis/apps/v1alpha1" - workloads "github.com/apecloud/kubeblocks/apis/workloads/v1alpha1" - "github.com/apecloud/kubeblocks/pkg/controller/builder" ) var _ = Describe("utils test", func() { @@ -226,31 +220,4 @@ var _ = Describe("utils test", func() { Expect(annotations["foo"]).Should(Equal(headlessK)) }) }) - - Context("IsOwnedByRsm function", func() { - It("should work well", func() { - By("call without ownerReferences") - rsm := &workloads.InstanceSet{} - Expect(IsOwnedByRsm(rsm)).Should(BeFalse()) - - By("call with ownerReference's kind is rsm") - t := true - rsm.OwnerReferences = []metav1.OwnerReference{ - { - Kind: workloads.Kind, - Controller: &t, - }, - } - Expect(IsOwnedByRsm(rsm)).Should(BeTrue()) - - By("call with ownerReference's is not rsm") - rsm.OwnerReferences = []metav1.OwnerReference{ - { - Kind: reflect.TypeOf(v1alpha1.Cluster{}).Name(), - Controller: &t, - }, - } - Expect(IsOwnedByRsm(rsm)).Should(BeFalse()) - }) - }) })