Skip to content

Commit

Permalink
make leaveMember4ScaleIn cleaner
Browse files Browse the repository at this point in the history
  • Loading branch information
cjc7373 committed Nov 14, 2024
1 parent 2e630a9 commit 98bfd03
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 22 deletions.
43 changes: 22 additions & 21 deletions controllers/apps/transformer_component_workload.go
Original file line number Diff line number Diff line change
Expand Up @@ -619,12 +619,10 @@ func (r *componentWorkloadOps) leaveMember4ScaleIn() error {
return false
}

tryToSwitchover := func(lfa lifecycle.Lifecycle, pod *corev1.Pod) error {
// if pod is not leader/primary, no need to switchover
trySwitchover := func(lfa lifecycle.Lifecycle, pod *corev1.Pod) error {
if !needSwitchover(pod) {
return nil
}
// if HA functionality is not enabled, no need to switchover
err := lfa.Switchover(r.reqCtx.Ctx, r.cli, nil, "")
if err != nil && errors.Is(err, lifecycle.ErrActionNotDefined) {
return nil
Expand All @@ -635,6 +633,20 @@ func (r *componentWorkloadOps) leaveMember4ScaleIn() error {
return err
}

tryMemberLeave := func(lfa lifecycle.Lifecycle) error {
if r.synthesizeComp.LifecycleActions == nil || r.synthesizeComp.LifecycleActions.MemberLeave == nil {
return nil
}

if err := lfa.MemberLeave(r.reqCtx.Ctx, r.cli, nil); err != nil {
if !errors.Is(err, lifecycle.ErrActionNotDefined) {
return err
}
}

return nil
}

// TODO: Move memberLeave to the ITS controller. Instead of performing a switchover, we can directly scale down the non-leader nodes. This is because the pod ordinal is not guaranteed to be continuous.
podsToMemberLeave := make([]*corev1.Pod, 0)
for _, pod := range pods {
Expand All @@ -645,28 +657,17 @@ func (r *componentWorkloadOps) leaveMember4ScaleIn() error {
podsToMemberLeave = append(podsToMemberLeave, pod)
}
for _, pod := range podsToMemberLeave {
if !(needSwitchover(pod) || // if the pod is leader, it needs to call switchover
(r.synthesizeComp.LifecycleActions != nil && r.synthesizeComp.LifecycleActions.MemberLeave != nil)) { // if the memberLeave action is defined, it needs to call it
continue
}

lfa, err1 := lifecycle.New(r.synthesizeComp, pod, pods...)
if err1 != nil {
if err == nil {
err = err1
}
continue
lfa, err := lifecycle.New(r.synthesizeComp, pod, pods...)
if err != nil {
return err
}

// switchover if the leaving pod is leader
if switchoverErr := tryToSwitchover(lfa, pod); switchoverErr != nil {
return switchoverErr
if err := trySwitchover(lfa, pod); err != nil {
return err
}

if err2 := lfa.MemberLeave(r.reqCtx.Ctx, r.cli, nil); err2 != nil {
if !errors.Is(err2, lifecycle.ErrActionNotDefined) && err == nil {
err = err2
}
if err := tryMemberLeave(lfa); err != nil {
return err
}
}
return err // TODO: use requeue-after
Expand Down
2 changes: 1 addition & 1 deletion pkg/testutil/apps/cluster_instance_set_test_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ func MockInstanceSetStatus(testCtx testutil.TestContext, cluster *appsv1.Cluster
if _, ok := pod.Labels[constant.RoleLabelKey]; !ok {
continue
}
role := &workloads.ReplicaRole{}
var role *workloads.ReplicaRole
for _, r := range its.Spec.Roles {
if r.Name == pod.Labels[constant.RoleLabelKey] {
role = r.DeepCopy()
Expand Down

0 comments on commit 98bfd03

Please sign in to comment.