Skip to content

Commit

Permalink
chore: remove the volume protection and its RBAC resources (#8206)
Browse files Browse the repository at this point in the history
  • Loading branch information
leon-inf authored Sep 26, 2024
1 parent a382a84 commit 665e033
Show file tree
Hide file tree
Showing 31 changed files with 56 additions and 598 deletions.
14 changes: 0 additions & 14 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -818,20 +818,6 @@ rules:
- get
- patch
- update
- apiGroups:
- rbac.authorization.k8s.io
resources:
- clusterrolebindings
verbs:
- get
- list
- watch
- apiGroups:
- rbac.authorization.k8s.io
resources:
- clusterrolebindings/status
verbs:
- get
- apiGroups:
- rbac.authorization.k8s.io
resources:
Expand Down
12 changes: 3 additions & 9 deletions controllers/apps/component_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,6 @@ type ComponentReconciler struct {
// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings,verbs=get;list;watch
// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings/status,verbs=get

// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=get;list;watch
// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings/status,verbs=get

// +kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get;list;watch

// Reconcile is part of the main kubernetes reconciliation loop which aims to
Expand Down Expand Up @@ -227,12 +224,10 @@ func (r *ComponentReconciler) setupWithManager(mgr ctrl.Manager) error {
Watches(&appsv1alpha1.Configuration{}, handler.EnqueueRequestsFromMapFunc(r.configurationEventHandler))

if viper.GetBool(constant.EnableRBACManager) {
b.Owns(&rbacv1.ClusterRoleBinding{}).
Owns(&rbacv1.RoleBinding{}).
b.Owns(&rbacv1.RoleBinding{}).
Owns(&corev1.ServiceAccount{})
} else {
b.Watches(&rbacv1.ClusterRoleBinding{}, handler.EnqueueRequestsFromMapFunc(r.filterComponentResources)).
Watches(&rbacv1.RoleBinding{}, handler.EnqueueRequestsFromMapFunc(r.filterComponentResources)).
b.Watches(&rbacv1.RoleBinding{}, handler.EnqueueRequestsFromMapFunc(r.filterComponentResources)).
Watches(&corev1.ServiceAccount{}, handler.EnqueueRequestsFromMapFunc(r.filterComponentResources))
}

Expand All @@ -256,8 +251,7 @@ func (r *ComponentReconciler) setupWithMultiClusterManager(mgr ctrl.Manager, mul
Watch(b, &corev1.ConfigMap{}, eventHandler).
Watch(b, &corev1.PersistentVolumeClaim{}, eventHandler).
Watch(b, &corev1.ServiceAccount{}, eventHandler).
Watch(b, &rbacv1.RoleBinding{}, eventHandler).
Watch(b, &rbacv1.ClusterRoleBinding{}, eventHandler)
Watch(b, &rbacv1.RoleBinding{}, eventHandler)

return b.Complete(r)
}
Expand Down
12 changes: 1 addition & 11 deletions controllers/apps/component_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1362,23 +1362,13 @@ var _ = Describe("Component Controller", func() {
Namespace: compObj.Namespace,
Name: saName,
}
crbKey := types.NamespacedName{
Namespace: compObj.Namespace,
Name: saName,
}
Eventually(testapps.CheckObjExists(&testCtx, saKey, &corev1.ServiceAccount{}, expectExisted)).Should(Succeed())
Eventually(testapps.CheckObjExists(&testCtx, rbKey, &rbacv1.RoleBinding{}, expectExisted)).Should(Succeed())
Eventually(testapps.CheckObjExists(&testCtx, crbKey, &rbacv1.ClusterRoleBinding{}, expectExisted)).Should(Succeed())
}

testCompRBAC := func(compName, compDefName, saName string) {
By("update comp definition to enable volume protection")
By("update comp definition to enable lifecycle actions")
Expect(testapps.GetAndChangeObj(&testCtx, client.ObjectKeyFromObject(compDefObj), func(compDef *kbappsv1.ComponentDefinition) {
for i, vol := range compDef.Spec.Volumes {
if vol.HighWatermark <= 0 || vol.HighWatermark >= 100 {
compDef.Spec.Volumes[i].HighWatermark = 85
}
}
compDef.Spec.LifecycleActions.Readonly = testapps.NewLifecycleAction("readonly")
compDef.Spec.LifecycleActions.Readwrite = testapps.NewLifecycleAction("readwrite")
})()).Should(Succeed())
Expand Down
13 changes: 0 additions & 13 deletions controllers/apps/componentdefinition_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,19 +253,6 @@ func (r *ComponentDefinitionReconciler) validateVolumes(cli client.Client, rctx
if !checkUniqueItemWithValue(cmpd.Spec.Volumes, "Name", nil) {
return fmt.Errorf("duplicate volume names are not allowed")
}

hasVolumeToProtect := false
for _, vol := range cmpd.Spec.Volumes {
if vol.HighWatermark > 0 && vol.HighWatermark < 100 {
hasVolumeToProtect = true
break
}
}
if hasVolumeToProtect {
if cmpd.Spec.LifecycleActions == nil || cmpd.Spec.LifecycleActions.Readonly == nil || cmpd.Spec.LifecycleActions.Readwrite == nil {
return fmt.Errorf("the Readonly and Readwrite actions are needed to protect volumes")
}
}
return nil
}

Expand Down
9 changes: 4 additions & 5 deletions controllers/apps/componentdefinition_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,23 +102,22 @@ var _ = Describe("ComponentDefinition Controller", func() {
})

Context("volumes", func() {
It("enable volume protection w/o actions set", func() {
It("duplicate volumes", func() {
By("create a ComponentDefinition obj")
componentDefObj := testapps.NewComponentDefinitionFactory(componentDefName).
SetRuntime(nil).
AddVolume("default", true, 85).
AddVolume("default", true, 0).
AddVolume("default", true, 0).
Create(&testCtx).GetObject()

checkObjectStatus(componentDefObj, kbappsv1.UnavailablePhase)
})

It("enable volume protection w/ actions set", func() {
It("set volume high watermark", func() {
By("create a ComponentDefinition obj")
componentDefObj := testapps.NewComponentDefinitionFactory(componentDefName).
SetRuntime(nil).
AddVolume("default", true, 85).
SetLifecycleAction("Readonly", defaultActionHandler).
SetLifecycleAction("Readwrite", defaultActionHandler).
Create(&testCtx).GetObject()

checkObjectStatus(componentDefObj, kbappsv1.AvailablePhase)
Expand Down
5 changes: 0 additions & 5 deletions controllers/apps/transformer_cluster_ownership.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
package apps

import (
rbacv1 "k8s.io/api/rbac/v1"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1"
Expand All @@ -44,10 +43,6 @@ func (f *clusterOwnershipTransformer) Transform(ctx graph.TransformContext, dag

controllerutil.AddFinalizer(cluster, constant.DBClusterFinalizerName)
for _, object := range objects {
// TODO: skip to set ownership for ClusterRoleBinding which is a cluster-scoped object.
if _, ok := object.(*rbacv1.ClusterRoleBinding); ok {
continue
}
if err := intctrlutil.SetOwnership(cluster, object, rscheme, constant.DBClusterFinalizerName); err != nil {
if _, ok := err.(*controllerutil.AlreadyOwnedError); ok {
continue
Expand Down
71 changes: 29 additions & 42 deletions controllers/apps/transformer_component_deletion.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,28 +82,27 @@ func (t *componentDeletionTransformer) Transform(ctx graph.TransformContext, dag
// handleCompDeleteWhenScaleIn handles the component deletion when scale-in, this scenario will delete all the sub-resources owned by the component by default.
func (t *componentDeletionTransformer) handleCompDeleteWhenScaleIn(transCtx *componentTransformContext, graphCli model.GraphClient,
dag *graph.DAG, comp *appsv1.Component, matchLabels map[string]string) error {
namespacedKinds, nonNamespacedKinds := kindsForCompWipeOut()
return t.deleteCompResources(transCtx, graphCli, dag, comp, matchLabels, namespacedKinds, nonNamespacedKinds)
return t.deleteCompResources(transCtx, graphCli, dag, comp, matchLabels, kindsForCompWipeOut())
}

// handleCompDeleteWhenClusterDelete handles the component deletion when the cluster is being deleted, the sub-resources owned by the component depends on the cluster's TerminationPolicy.
func (t *componentDeletionTransformer) handleCompDeleteWhenClusterDelete(transCtx *componentTransformContext, graphCli model.GraphClient,
dag *graph.DAG, cluster *appsv1.Cluster, comp *appsv1.Component, matchLabels map[string]string) error {
var namespacedKinds, nonNamespacedKinds []client.ObjectList
var kinds []client.ObjectList
switch cluster.Spec.TerminationPolicy {
case appsv1.Delete:
namespacedKinds, nonNamespacedKinds = kindsForCompDelete()
kinds = kindsForCompDelete()
case appsv1.WipeOut:
namespacedKinds, nonNamespacedKinds = kindsForCompWipeOut()
kinds = kindsForCompWipeOut()
}
return t.deleteCompResources(transCtx, graphCli, dag, comp, matchLabels, namespacedKinds, nonNamespacedKinds)
return t.deleteCompResources(transCtx, graphCli, dag, comp, matchLabels, kinds)
}

func (t *componentDeletionTransformer) deleteCompResources(transCtx *componentTransformContext, graphCli model.GraphClient,
dag *graph.DAG, comp *appsv1.Component, matchLabels map[string]string, namespacedKinds, nonNamespacedKinds []client.ObjectList) error {
dag *graph.DAG, comp *appsv1.Component, matchLabels map[string]string, kinds []client.ObjectList) error {

// firstly, delete the workloads owned by the component
workloads, err := model.ReadCacheSnapshot(transCtx, comp, matchLabels, true, compOwnedWorkloadKinds()...)
workloads, err := model.ReadCacheSnapshot(transCtx, comp, matchLabels, compOwnedWorkloadKinds()...)
if err != nil {
return newRequeueError(requeueDuration, err.Error())
}
Expand All @@ -117,23 +116,17 @@ func (t *componentDeletionTransformer) deleteCompResources(transCtx *componentTr
}

// secondly, delete the other sub-resources owned by the component
namespacedObjs, err1 := model.ReadCacheSnapshot(transCtx, comp, matchLabels, true, namespacedKinds...)
snapshot, err1 := model.ReadCacheSnapshot(transCtx, comp, matchLabels, kinds...)
if err1 != nil {
return newRequeueError(requeueDuration, err1.Error())
}
nonNamespacedObjs, err2 := model.ReadCacheSnapshot(transCtx, comp, matchLabels, false, nonNamespacedKinds...)
if err2 != nil {
return newRequeueError(requeueDuration, err2.Error())
}
if len(namespacedObjs) > 0 || len(nonNamespacedObjs) > 0 {
if len(snapshot) > 0 {
// delete the sub-resources owned by the component before deleting the component
for _, snapshot := range []model.ObjectSnapshot{namespacedObjs, nonNamespacedObjs} {
for _, object := range snapshot {
if isOwnedByInstanceSet(object) {
continue
}
graphCli.Delete(dag, object)
for _, object := range snapshot {
if isOwnedByInstanceSet(object) {
continue
}
graphCli.Delete(dag, object)
}
graphCli.Status(dag, comp, transCtx.Component)
return newRequeueError(time.Second*1, "not all component sub-resources deleted")
Expand Down Expand Up @@ -170,19 +163,16 @@ func compOwnedWorkloadKinds() []client.ObjectList {
}
}

func compOwnedKinds() ([]client.ObjectList, []client.ObjectList) {
func compOwnedKinds() []client.ObjectList {
return []client.ObjectList{
&workloads.InstanceSetList{},
&corev1.ServiceList{},
&dpv1alpha1.BackupList{},
&dpv1alpha1.RestoreList{},
&appsv1alpha1.ConfigurationList{},
&corev1.ServiceAccountList{},
&rbacv1.RoleBindingList{},
},
[]client.ObjectList{
&rbacv1.ClusterRoleBindingList{},
}
&workloads.InstanceSetList{},
&corev1.ServiceList{},
&dpv1alpha1.BackupList{},
&dpv1alpha1.RestoreList{},
&appsv1alpha1.ConfigurationList{},
&corev1.ServiceAccountList{},
&rbacv1.RoleBindingList{},
}
}

func compOwnedPreserveKinds() []client.ObjectList {
Expand All @@ -193,21 +183,18 @@ func compOwnedPreserveKinds() []client.ObjectList {
}
}

func kindsForCompDoNotTerminate() ([]client.ObjectList, []client.ObjectList) {
return []client.ObjectList{}, []client.ObjectList{}
func kindsForCompDoNotTerminate() []client.ObjectList {
return []client.ObjectList{}
}

func kindsForCompHalt() ([]client.ObjectList, []client.ObjectList) {
namespacedKinds, nonNamespacedKinds := kindsForCompDoNotTerminate()
namespacedKindsPlus, nonNamespacedKindsPlus := compOwnedKinds()
return append(namespacedKinds, namespacedKindsPlus...), append(nonNamespacedKinds, nonNamespacedKindsPlus...)
func kindsForCompHalt() []client.ObjectList {
return append(kindsForCompDoNotTerminate(), compOwnedKinds()...)
}

func kindsForCompDelete() ([]client.ObjectList, []client.ObjectList) {
namespacedKinds, nonNamespacedKinds := kindsForCompHalt()
return append(namespacedKinds, compOwnedPreserveKinds()...), nonNamespacedKinds
func kindsForCompDelete() []client.ObjectList {
return append(kindsForCompHalt(), compOwnedPreserveKinds()...)
}

func kindsForCompWipeOut() ([]client.ObjectList, []client.ObjectList) {
func kindsForCompWipeOut() []client.ObjectList {
return kindsForCompDelete()
}
Loading

0 comments on commit 665e033

Please sign in to comment.