Skip to content

Commit

Permalink
chore: move IsOwnedByInstanceSet from pkg instanceset to apps (#7137)
Browse files Browse the repository at this point in the history
(cherry picked from commit 773ca0c)
  • Loading branch information
free6om committed Apr 23, 2024
1 parent 87a3e5b commit cfeea06
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 46 deletions.
11 changes: 11 additions & 0 deletions controllers/apps/transform_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1"
workloads "github.com/apecloud/kubeblocks/apis/workloads/v1alpha1"
"github.com/apecloud/kubeblocks/pkg/constant"
"github.com/apecloud/kubeblocks/pkg/controller/graph"
"github.com/apecloud/kubeblocks/pkg/controller/model"
Expand Down Expand Up @@ -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
}
24 changes: 24 additions & 0 deletions controllers/apps/transform_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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))
}
3 changes: 1 addition & 2 deletions controllers/apps/transformer_cluster_deletion.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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())
Expand Down
3 changes: 1 addition & 2 deletions controllers/apps/transformer_component_deletion.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
Expand Down
11 changes: 0 additions & 11 deletions pkg/controller/instanceset/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (

"golang.org/x/exp/slices"
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"
Expand Down Expand Up @@ -178,16 +177,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 {
Expand Down
31 changes: 0 additions & 31 deletions pkg/controller/instanceset/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,13 @@ package instanceset

import (
"fmt"
"reflect"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

"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"
)

Expand Down Expand Up @@ -226,31 +222,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())
})
})
})

0 comments on commit cfeea06

Please sign in to comment.