Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Commit

Permalink
fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@Charlie17Li
Charlie17Li committed Oct 23, 2023
1 parent 1031ab7 commit cb3096d
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 38 deletions.
5 changes: 4 additions & 1 deletion module-controller/api/v1alpha1/moduledeployment_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ const (
ModuleDeploymentReleaseProgressPaused ReleaseProgress = "Paused"
ModuleDeploymentReleaseProgressCompleted ReleaseProgress = "Completed"
ModuleDeploymentReleaseProgressAborted ReleaseProgress = "Aborted"
ModuleDeploymentReleaseProgressTermed ReleaseProgress = "Terminated"
ModuleDeploymentReleaseProgressTerminating ReleaseProgress = "Terminating"
ModuleDeploymentReleaseProgressTerminated ReleaseProgress = "Terminated"
)

type ModuleUpgradeType string
Expand Down Expand Up @@ -180,6 +181,8 @@ type ModuleDeploymentStatus struct {

ReleaseStatus *ReleaseStatus `json:"releaseStatus,omitempty"`

DoTerminating bool `json:"doTerminating,omitempty"`

ObservedGeneration int64 `json:"observedGeneration,omitempty"`
}

Expand Down
2 changes: 2 additions & 0 deletions module-controller/config/crd/bases/serverless.alipay.com_moduledeployments.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,8 @@ spec:
type: string
type: object
type: array
doTerminating:
type: boolean
observedGeneration:
format: int64
type: integer
Expand Down
63 changes: 40 additions & 23 deletions module-controller/internal/controller/moduledeployment_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,18 @@ func (r *ModuleDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req

if moduleDeployment.DeletionTimestamp != nil {
// delete moduleDeployment
return r.handleDeletingModuleDeployment(ctx, moduleDeployment)
if !utils.HasFinalizer(&moduleDeployment.ObjectMeta, finalizer.ModuleReplicaSetExistedFinalizer) &&
!utils.HasFinalizer(&moduleDeployment.ObjectMeta, finalizer.ModuleExistedFinalizer) {
return ctrl.Result{}, nil
}

if !moduleDeployment.Status.DoTerminating {
moduleDeployment.Status.DoTerminating = true
if moduleDeployment.Status.ReleaseStatus.Progress != moduledeploymentv1alpha1.ModuleDeploymentReleaseProgressTerminating {
moduleDeployment.Status.ReleaseStatus.Progress = moduledeploymentv1alpha1.ModuleDeploymentReleaseProgressTerminating
}
return ctrl.Result{}, r.Status().Update(ctx, moduleDeployment)
}
}

if moduleDeployment.Spec.Pause {
Expand Down Expand Up @@ -112,6 +123,10 @@ func (r *ModuleDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req
case moduledeploymentv1alpha1.ModuleDeploymentReleaseProgressExecuting:
return r.updateModuleReplicaSet(ctx, moduleDeployment, newRS)
case moduledeploymentv1alpha1.ModuleDeploymentReleaseProgressCompleted:
if moduleDeployment.DeletionTimestamp != nil {
moduleDeployment.Status.ReleaseStatus.Progress = moduledeploymentv1alpha1.ModuleDeploymentReleaseProgressTerminating
return ctrl.Result{}, r.Status().Update(ctx, moduleDeployment)
}
if moduleDeployment.Spec.Replicas != newRS.Spec.Replicas {
moduleDeployment.Status.ReleaseStatus.Progress = moduledeploymentv1alpha1.ModuleDeploymentReleaseProgressInit
if err := r.Status().Update(ctx, moduleDeployment); err != nil {
Expand Down Expand Up @@ -141,6 +156,25 @@ func (r *ModuleDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req
return ctrl.Result{}, err
}
}
case moduledeploymentv1alpha1.ModuleDeploymentReleaseProgressTerminating:
// delete modules
if utils.HasFinalizer(&moduleDeployment.ObjectMeta, finalizer.ModuleExistedFinalizer) {
if moduleDeployment.Spec.Replicas != 0 {
moduleDeployment.Spec.Replicas = 0
return ctrl.Result{}, r.Update(ctx, moduleDeployment)
}
if newRS.Status.Replicas != 0 {
handleInitModuleDeployment(moduleDeployment, newRS)
return ctrl.Result{}, r.Status().Update(ctx, moduleDeployment)
}
utils.RemoveFinalizer(&moduleDeployment.ObjectMeta, finalizer.ModuleExistedFinalizer)
return ctrl.Result{}, r.Update(ctx, moduleDeployment)
}

// delete module replicaset
if utils.HasFinalizer(&moduleDeployment.ObjectMeta, finalizer.ModuleReplicaSetExistedFinalizer) {
return r.handleDeletingModuleDeployment(ctx, moduleDeployment)
}
}

// update moduleDeployment owner reference
Expand Down Expand Up @@ -176,10 +210,6 @@ func handleInitModuleDeployment(moduleDeployment *moduledeploymentv1alpha1.Modul

// handle deleting module deployment
func (r *ModuleDeploymentReconciler) handleDeletingModuleDeployment(ctx context.Context, moduleDeployment *moduledeploymentv1alpha1.ModuleDeployment) (ctrl.Result, error) {
if !utils.HasFinalizer(&moduleDeployment.ObjectMeta, finalizer.ModuleReplicaSetExistedFinalizer) {
return ctrl.Result{}, nil
}

existReplicaset := true
set := map[string]string{
label.ModuleDeploymentLabel: moduleDeployment.Name,
Expand All @@ -198,23 +228,10 @@ func (r *ModuleDeploymentReconciler) handleDeletingModuleDeployment(ctx context.
}

if existReplicaset {
batchCount := moduleDeployment.Status.ReleaseStatus.RealBatchCount
deleteNum := int32(math.Ceil(float64(moduleDeployment.Spec.Replicas) / float64(batchCount)))

// delete modules in batches
for i := 0; i < len(replicaSetList.Items); i++ {
if deleteNum < replicaSetList.Items[i].Spec.Replicas {
replicaSetList.Items[i].Spec.Replicas -= deleteNum
err := r.Client.Update(ctx, &replicaSetList.Items[i])
if err != nil {
return ctrl.Result{}, utils.Error(err, "Failed to update moduleReplicaSet", "moduleReplicaSetName", replicaSetList.Items[i].Name)
}
} else {
deleteNum -= replicaSetList.Items[i].Spec.Replicas
err := r.Client.Delete(ctx, &replicaSetList.Items[i])
if err != nil {
return ctrl.Result{}, utils.Error(err, "Failed to delete moduleReplicaSet", "moduleReplicaSetName", replicaSetList.Items[i].Name)
}
err := r.Client.Delete(ctx, &replicaSetList.Items[i])
if err != nil {
return ctrl.Result{}, utils.Error(err, "Failed to delete moduleReplicaSet", "moduleReplicaSetName", replicaSetList.Items[i].Name)
}
}
requeueAfter := utils.GetNextReconcileTime(moduleDeployment.DeletionTimestamp.Time)
Expand Down Expand Up @@ -256,6 +273,7 @@ func (r *ModuleDeploymentReconciler) updateOwnerReference(ctx context.Context, m
})
moduleDeployment.SetOwnerReferences(ownerReference)
utils.AddFinalizer(&moduleDeployment.ObjectMeta, finalizer.ModuleReplicaSetExistedFinalizer)
utils.AddFinalizer(&moduleDeployment.ObjectMeta, finalizer.ModuleExistedFinalizer)
err = r.Client.Update(ctx, moduleDeployment)
if err != nil {
return utils.Error(err, "Failed to update moduleDeployment", "moduleDeploymentName", moduleDeployment.Name)
Expand Down Expand Up @@ -368,8 +386,7 @@ func (r *ModuleDeploymentReconciler) updateModuleReplicaSet(ctx context.Context,
}

// wait moduleReplicaset ready

if newRS.Spec.Replicas > curReplicas+moduleDeployment.Spec.OperationStrategy.MaxUnavailable {
if newRS.Spec.Replicas != curReplicas {
log.Log.Info(fmt.Sprintf("newRs is not ready, expect replicas %v, but got %v", newRS.Spec.Replicas, curReplicas))
return ctrl.Result{Requeue: true, RequeueAfter: utils.GetNextReconcileTime(time.Now())}, nil
}
Expand Down
38 changes: 24 additions & 14 deletions module-controller/internal/controller/moduledeployment_controller_suit_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ package controller
import (
"context"
"fmt"
"sigs.k8s.io/controller-runtime/pkg/log"
"time"

"sigs.k8s.io/controller-runtime/pkg/log"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -99,11 +100,11 @@ var _ = Describe("ModuleDeployment Controller", func() {
newModuleDeployment.Spec.Template.Spec.Module.Version = "1.0.1"
Expect(k8sClient.Update(context.TODO(), &newModuleDeployment)).Should(Succeed())

Eventually(func() bool {
Eventually(func() error {
return checkModuleDeploymentReplicas(
types.NamespacedName{Name: moduleDeploymentName, Namespace: namespace},
newModuleDeployment.Spec.Replicas)
}, timeout, interval).Should(BeTrue())
}, timeout, interval).Should(Succeed())
})
})

Expand Down Expand Up @@ -190,20 +191,20 @@ var _ = Describe("ModuleDeployment Controller", func() {

It("2. check if the replicas is 1", func() {
// todo: we just check deployment.status.replicas rather than modulereplicaset
Eventually(func() bool {
Eventually(func() error {
if err := k8sClient.Get(context.TODO(), nn, &moduleDeployment); err != nil {
return false
return err
}

if !moduleDeployment.Spec.Pause {
return false
return fmt.Errorf("the deployment is not paused")
}

return checkModuleDeploymentReplicas(
types.NamespacedName{
Name: moduleDeploymentName,
Namespace: moduleDeployment.Namespace}, 1)
}, timeout, interval).Should(BeTrue())
}, timeout, interval).Should(Succeed())
})

It("3. resume", func() {
Expand Down Expand Up @@ -261,7 +262,7 @@ var _ = Describe("ModuleDeployment Controller", func() {
})

It("2. check if use Beta strategy", func() {
Eventually(func() bool {
Eventually(func() error {
return checkModuleDeploymentReplicas(nn, 1)
})
})
Expand Down Expand Up @@ -300,22 +301,22 @@ var _ = Describe("ModuleDeployment Controller", func() {
})
})

func checkModuleDeploymentReplicas(nn types.NamespacedName, replicas int32) bool {
func checkModuleDeploymentReplicas(nn types.NamespacedName, replicas int32) error {
set := map[string]string{
label.ModuleDeploymentLabel: nn.Name,
}
replicaSetList := &moduledeploymentv1alpha1.ModuleReplicaSetList{}
err := k8sClient.List(context.TODO(), replicaSetList, &client.ListOptions{LabelSelector: labels.SelectorFromSet(set)}, client.InNamespace(nn.Namespace))
if err != nil || len(replicaSetList.Items) == 0 {
return false
return fmt.Errorf("the replicasetList is empty")
}

maxVersion := 0
var newRS *moduledeploymentv1alpha1.ModuleReplicaSet
for i := 0; i < len(replicaSetList.Items); i++ {
version, err := getRevision(&replicaSetList.Items[i])
if err != nil {
return false
return err
}
if version > maxVersion {
maxVersion = version
Expand All @@ -324,9 +325,18 @@ func checkModuleDeploymentReplicas(nn types.NamespacedName, replicas int32) bool
}

// the replicas of new replicaset must be equal to newModuleDeployment
return newRS != nil &&
newRS.Status.Replicas == newRS.Spec.Replicas &&
newRS.Status.Replicas == replicas
if newRS == nil {
return fmt.Errorf("the replicaset is nil")
}
if newRS.Status.Replicas != newRS.Spec.Replicas {
return fmt.Errorf("the replicaset is not ready, expect replicas is %v, but got %v",
newRS.Spec.Replicas, newRS.Status.ReadyReplicas)
}
if newRS.Spec.Replicas != replicas {
return fmt.Errorf("the deployment is not ready, expect replicas is %v, but got %v",
replicas, newRS.Spec.Replicas)
}
return nil
}

func prepareModuleDeployment(namespace, moduleDeploymentName string) v1alpha1.ModuleDeployment {
Expand Down

0 comments on commit cb3096d

Please sign in to comment.