Skip to content

Commit

Permalink
fix: should not check retained backup when deleting cluster (apecloud…
Browse files Browse the repository at this point in the history
  • Loading branch information
lynnleelhl authored Jul 26, 2023
1 parent 8dcea09 commit a65fcfc
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 7 deletions.
14 changes: 14 additions & 0 deletions controllers/apps/cluster_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,13 +315,27 @@ var _ = Describe("Cluster Controller", func() {
Eventually(testapps.GetClusterObservedGeneration(&testCtx, clusterKey)).Should(BeEquivalentTo(1))
Eventually(testapps.GetClusterPhase(&testCtx, clusterKey)).Should(Equal(appsv1alpha1.CreatingClusterPhase))

By("Mocking a retained backup")
backupPolicyName := "test-backup-policy"
backupName := "test-backup"
backup := testapps.NewBackupFactory(testCtx.DefaultNamespace, backupName).
SetBackupPolicyName(backupPolicyName).
SetBackupType(dataprotectionv1alpha1.BackupTypeDataFile).
SetLabels(map[string]string{constant.AppInstanceLabelKey: clusterKey.Name, constant.BackupProtectionLabelKey: constant.BackupRetain}).
WithRandomName().
Create(&testCtx).GetObject()
backupKey := client.ObjectKeyFromObject(backup)

// REVIEW: this test flow

By("Delete the cluster")
testapps.DeleteObject(&testCtx, clusterKey, &appsv1alpha1.Cluster{})

By("Wait for the cluster to terminate")
Eventually(testapps.CheckObjExists(&testCtx, clusterKey, &appsv1alpha1.Cluster{}, false)).Should(Succeed())

By("Checking backup should exist")
Eventually(testapps.CheckObjExists(&testCtx, backupKey, &dataprotectionv1alpha1.Backup{}, true)).Should(Succeed())
}

testDoNotTerminate := func(compName, compDefName string) {
Expand Down
5 changes: 0 additions & 5 deletions controllers/apps/cluster_plan_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"context"
"fmt"
"reflect"
"strings"

"github.com/go-logr/logr"
"github.com/pkg/errors"
Expand Down Expand Up @@ -265,10 +264,6 @@ func (c *clusterPlanBuilder) reconcileObject(node *ictrltypes.LifecycleVertex) e
}
// delete secondary objects
if _, ok := node.Obj.(*appsv1alpha1.Cluster); !ok {
// retain backup for data protection even if the cluster is wiped out.
if strings.EqualFold(node.Obj.GetLabels()[constant.BackupProtectionLabelKey], constant.BackupRetain) {
return nil
}
err := intctrlutil.BackgroundDeleteObject(c.cli, c.transCtx.Context, node.Obj)
// err := c.cli.Delete(c.transCtx.Context, node.obj)
if err != nil && !apierrors.IsNotFound(err) {
Expand Down
17 changes: 15 additions & 2 deletions controllers/apps/transformer_cluster_deletion.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,18 +137,31 @@ func (t *ClusterDeletionTransformer) Transform(ctx graph.TransformContext, dag *
return err
}

toDeleteObjs := func(objs clusterOwningObjects) []client.Object {
var delObjs []client.Object
for _, obj := range objs {
// retain backup for data protection even if the cluster is wiped out.
if strings.EqualFold(obj.GetLabels()[constant.BackupProtectionLabelKey], constant.BackupRetain) {
continue
}
delObjs = append(delObjs, obj)
}
return delObjs
}

// add objects deletion vertex
objs, err := getClusterOwningObjects(transCtx, *cluster, ml, toDeleteKinds...)
if err != nil {
return err
}
for _, o := range objs {
delObjs := toDeleteObjs(objs)
for _, o := range delObjs {
vertex := &ictrltypes.LifecycleVertex{Obj: o, Action: ictrltypes.ActionDeletePtr()}
dag.AddVertex(vertex)
dag.Connect(root, vertex)
}
// set cluster action to noop until all the sub-resources deleted
if len(objs) == 0 {
if len(delObjs) == 0 {
root.Action = ictrltypes.ActionDeletePtr()
} else {
root.Action = ictrltypes.ActionNoopPtr()
Expand Down

0 comments on commit a65fcfc

Please sign in to comment.