From 02c50832ac10b950c241f84eb273160c64ac9ba8 Mon Sep 17 00:00:00 2001 From: "jinle.xjl" Date: Fri, 27 Oct 2023 19:31:43 +0800 Subject: [PATCH 1/2] fix update resource conflicts and add some comments --- .../api/v1alpha1/module_types.go | 1 + .../api/v1alpha1/moduledeployment_types.go | 6 +- .../api/v1alpha1/modulereplicaset_types.go | 1 + ...rverless.alipay.com_moduledeployments.yaml | 7 +- ...rverless.alipay.com_modulereplicasets.yaml | 2 + .../bases/serverless.alipay.com_modules.yaml | 2 + .../internal/controller/module_controller.go | 10 +- .../controller/moduledeployment_controller.go | 101 +++++++++--------- .../controller/modulereplicaset_controller.go | 30 +++++- .../internal/utils/controller_utils.go | 27 ++++- 10 files changed, 122 insertions(+), 65 deletions(-) diff --git a/module-controller/api/v1alpha1/module_types.go b/module-controller/api/v1alpha1/module_types.go index 4a91a96fc..a644c743f 100644 --- a/module-controller/api/v1alpha1/module_types.go +++ b/module-controller/api/v1alpha1/module_types.go @@ -89,6 +89,7 @@ type ModuleStatus struct { //+kubebuilder:object:root=true //+kubebuilder:subresource:status +//+kubebuilder:resource:shortName=md // Module is the Schema for the modules API type Module struct { diff --git a/module-controller/api/v1alpha1/moduledeployment_types.go b/module-controller/api/v1alpha1/moduledeployment_types.go index d3809ae80..34ebb55f9 100644 --- a/module-controller/api/v1alpha1/moduledeployment_types.go +++ b/module-controller/api/v1alpha1/moduledeployment_types.go @@ -83,10 +83,13 @@ type ReleaseStatus struct { // +optional OriginalDeltaReplicas int32 `json:"originalDeltaReplicas,omitempty"` - // The phase current release reach + // The phase current whole release reach // +optional Progress ReleaseProgress `json:"progress,omitempty"` + // the phase current batch release reach + BatchProgress ReleaseProgress `json:"batchProgress,omitempty"` + // Last time the release transitioned from one status to another. LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"` @@ -198,6 +201,7 @@ type ModuleDeploymentStatus struct { //+kubebuilder:object:root=true //+kubebuilder:subresource:status +//+kubebuilder:resource:shortName=mddeploy // ModuleDeployment is the Schema for the moduledeployments API type ModuleDeployment struct { diff --git a/module-controller/api/v1alpha1/modulereplicaset_types.go b/module-controller/api/v1alpha1/modulereplicaset_types.go index 8498582e7..a1055cc7c 100644 --- a/module-controller/api/v1alpha1/modulereplicaset_types.go +++ b/module-controller/api/v1alpha1/modulereplicaset_types.go @@ -71,6 +71,7 @@ type ModuleReplicaSetStatus struct { //+kubebuilder:object:root=true //+kubebuilder:subresource:status +//+kubebuilder:resource:shortName=mdrs // ModuleReplicaSet is the Schema for the modulereplicasets API type ModuleReplicaSet struct { diff --git a/module-controller/config/crd/bases/serverless.alipay.com_moduledeployments.yaml b/module-controller/config/crd/bases/serverless.alipay.com_moduledeployments.yaml index cf244c8c5..732782562 100644 --- a/module-controller/config/crd/bases/serverless.alipay.com_moduledeployments.yaml +++ b/module-controller/config/crd/bases/serverless.alipay.com_moduledeployments.yaml @@ -11,6 +11,8 @@ spec: kind: ModuleDeployment listKind: ModuleDeploymentList plural: moduledeployments + shortNames: + - mddeploy singular: moduledeployment scope: Namespaced versions: @@ -224,6 +226,9 @@ spec: type: integer releaseStatus: properties: + batchProgress: + description: the phase current bath release reach + type: string currentBatch: description: Records the current batch serial number. format: int32 @@ -241,7 +246,7 @@ spec: format: int32 type: integer progress: - description: The phase current release reach + description: The phase current whole release reach type: string realBatchCount: description: Records the real batch count diff --git a/module-controller/config/crd/bases/serverless.alipay.com_modulereplicasets.yaml b/module-controller/config/crd/bases/serverless.alipay.com_modulereplicasets.yaml index 0d2c8e292..2c6cc0e3b 100644 --- a/module-controller/config/crd/bases/serverless.alipay.com_modulereplicasets.yaml +++ b/module-controller/config/crd/bases/serverless.alipay.com_modulereplicasets.yaml @@ -11,6 +11,8 @@ spec: kind: ModuleReplicaSet listKind: ModuleReplicaSetList plural: modulereplicasets + shortNames: + - mdrs singular: modulereplicaset scope: Namespaced versions: diff --git a/module-controller/config/crd/bases/serverless.alipay.com_modules.yaml b/module-controller/config/crd/bases/serverless.alipay.com_modules.yaml index d3b6e082c..ad4309922 100644 --- a/module-controller/config/crd/bases/serverless.alipay.com_modules.yaml +++ b/module-controller/config/crd/bases/serverless.alipay.com_modules.yaml @@ -11,6 +11,8 @@ spec: kind: Module listKind: ModuleList plural: modules + shortNames: + - md singular: module scope: Namespaced versions: diff --git a/module-controller/internal/controller/module_controller.go b/module-controller/internal/controller/module_controller.go index 177b64885..d47d038e6 100644 --- a/module-controller/internal/controller/module_controller.go +++ b/module-controller/internal/controller/module_controller.go @@ -129,7 +129,7 @@ func (r *ModuleReconciler) parseModuleInstanceStatus(ctx context.Context, module log.Log.Info(fmt.Sprintf("%s%s", "module status change to ", moduleInstanceStatus), "moduleName", module.Name) err := r.Status().Update(ctx, module) if err != nil { - return ctrl.Result{}, err + return ctrl.Result{}, utils.Error(err, "update module status failed") } return ctrl.Result{}, nil } @@ -287,7 +287,7 @@ func (r *ModuleReconciler) handlePendingModuleInstance(ctx context.Context, modu log.Log.Info(fmt.Sprintf("%s%s", "module status change to ", v1alpha1.ModuleInstanceStatusPrepare), "moduleName", module.Name) err := r.Status().Update(ctx, module) if err != nil { - return ctrl.Result{}, err + return ctrl.Result{}, utils.Error(err, "update module status from pending to prepare failed") } return ctrl.Result{}, nil } @@ -360,7 +360,7 @@ func (r *ModuleReconciler) handlePrepareModuleInstance(ctx context.Context, modu log.Log.Info(fmt.Sprintf("%s%s", "module status change to ", v1alpha1.ModuleInstanceStatusUpgrading), "moduleName", module.Name) err := r.Status().Update(ctx, module) if err != nil { - return ctrl.Result{}, err + return ctrl.Result{}, utils.Error(err, "update module from prepare to upgrading failed") } return ctrl.Result{}, nil } @@ -385,7 +385,7 @@ func (r *ModuleReconciler) handleUpgradingModuleInstance(ctx context.Context, mo log.Log.Info(fmt.Sprintf("%s%s", "module status change to ", v1alpha1.ModuleInstanceStatusCompleting), "moduleName", module.Name) err = r.Status().Update(ctx, module) if err != nil { - return ctrl.Result{}, err + return ctrl.Result{}, utils.Error(err, "update module from upgrading to completing failed") } return ctrl.Result{}, nil } @@ -399,7 +399,7 @@ func (r *ModuleReconciler) handleCompletingModuleInstance(ctx context.Context, m log.Log.Info(fmt.Sprintf("%s%s", "module status change to ", v1alpha1.ModuleInstanceStatusAvailable), "moduleName", module.Name) err := r.Status().Update(ctx, module) if err != nil { - return ctrl.Result{}, err + return ctrl.Result{}, utils.Error(err, "update module from completing to available failed") } return ctrl.Result{}, nil } diff --git a/module-controller/internal/controller/moduledeployment_controller.go b/module-controller/internal/controller/moduledeployment_controller.go index f0649bafc..d42cfe18f 100644 --- a/module-controller/internal/controller/moduledeployment_controller.go +++ b/module-controller/internal/controller/moduledeployment_controller.go @@ -95,6 +95,15 @@ func (r *ModuleDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req return ctrl.Result{}, nil } + // update moduleDeployment owner reference + if utils.HasOwnerReference(&moduleDeployment.ObjectMeta, moduleDeployment.Spec.BaseDeploymentName) { + err = r.updateOwnerReference(ctx, moduleDeployment) + if err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil + } + // create moduleReplicaSet newRS, _, moduleVersionChanged, err := r.createOrGetModuleReplicas(ctx, moduleDeployment) if err != nil { @@ -113,16 +122,18 @@ func (r *ModuleDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req switch releaseStatus.Progress { case v1alpha1.ModuleDeploymentReleaseProgressInit: handleInitModuleDeployment(moduleDeployment, newRS) + log.Log.Info("update moduleDeployment release status", "moduleDeploymentName", moduleDeployment.Name) if err := r.Status().Update(ctx, moduleDeployment); err != nil { - return ctrl.Result{}, err + return ctrl.Result{}, utils.Error(err, "update release status failed when init moduleDeployment") } case v1alpha1.ModuleDeploymentReleaseProgressExecuting: return r.updateModuleReplicaSet(ctx, moduleDeployment, newRS) case v1alpha1.ModuleDeploymentReleaseProgressCompleted: if moduleDeployment.Spec.Replicas != newRS.Spec.Replicas { moduleDeployment.Status.ReleaseStatus.Progress = v1alpha1.ModuleDeploymentReleaseProgressInit + log.Log.Info("update release status progress to init when complete moduleDeployment", "moduleDeploymentName", moduleDeployment.Name) if err := r.Status().Update(ctx, moduleDeployment); err != nil { - return ctrl.Result{}, err + return ctrl.Result{}, utils.Error(err, "update release status progress to init failed when complete moduleDeployment") } } if !moduleVersionChanged && isUrlChange(moduleDeployment.Spec.Template.Spec.Module, newRS.Spec.Template.Spec.Module) { @@ -138,23 +149,19 @@ func (r *ModuleDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req } moduleDeployment.Status.ReleaseStatus.Progress = v1alpha1.ModuleDeploymentReleaseProgressPaused + log.Log.Info("update moduleDeployment releaseStatus progress to paused", "moduleDeploymentName", moduleDeployment.Name) if err := r.Status().Update(ctx, moduleDeployment); err != nil { - return ctrl.Result{}, err + return ctrl.Result{}, utils.Error(err, "update moduleDeployment releaseStatus progress to paused failed") } case v1alpha1.ModuleDeploymentReleaseProgressPaused: if !moduleDeployment.Spec.Pause && time.Since(moduleDeployment.Status.ReleaseStatus.NextReconcileTime.Time) >= 0 { moduleDeployment.Status.ReleaseStatus.Progress = v1alpha1.ModuleDeploymentReleaseProgressExecuting + log.Log.Info("update moduleDeployment progress from paused to executing", "moduleDeploymentName", moduleDeployment.Name) if err := r.Status().Update(ctx, moduleDeployment); err != nil { - return ctrl.Result{}, err + return ctrl.Result{}, utils.Error(err, "update moduleDeployment progress from paused to executing failed") } } } - - // update moduleDeployment owner reference - err = r.updateOwnerReference(ctx, moduleDeployment) - if err != nil { - return ctrl.Result{}, err - } return ctrl.Result{}, nil } @@ -226,34 +233,25 @@ func (r *ModuleDeploymentReconciler) handleDeletingModuleDeployment(ctx context. // update moduleDeployment owner reference func (r *ModuleDeploymentReconciler) updateOwnerReference(ctx context.Context, moduleDeployment *v1alpha1.ModuleDeployment) error { - moduleDeploymentOwnerReferenceExist := false - for _, ownerReference := range moduleDeployment.GetOwnerReferences() { - if moduleDeployment.Spec.BaseDeploymentName == ownerReference.Name { - moduleDeploymentOwnerReferenceExist = true - } + deployment := &v1.Deployment{} + err := r.Client.Get(ctx, types.NamespacedName{Namespace: moduleDeployment.Namespace, Name: moduleDeployment.Spec.BaseDeploymentName}, deployment) + if err != nil { + return utils.Error(err, "Failed to get deployment", "deploymentName", deployment.Name) } - - if !moduleDeploymentOwnerReferenceExist { - deployment := &v1.Deployment{} - err := r.Client.Get(ctx, types.NamespacedName{Namespace: moduleDeployment.Namespace, Name: moduleDeployment.Spec.BaseDeploymentName}, deployment) - if err != nil { - return utils.Error(err, "Failed to get deployment", "deploymentName", deployment.Name) - } - ownerReference := moduleDeployment.GetOwnerReferences() - ownerReference = append(ownerReference, metav1.OwnerReference{ - APIVersion: deployment.APIVersion, - Kind: deployment.Kind, - UID: deployment.UID, - Name: deployment.Name, - BlockOwnerDeletion: pointer.Bool(true), - Controller: pointer.Bool(true), - }) - moduleDeployment.SetOwnerReferences(ownerReference) - utils.AddFinalizer(&moduleDeployment.ObjectMeta, finalizer.ModuleReplicaSetExistedFinalizer) - err = r.Client.Update(ctx, moduleDeployment) - if err != nil { - return utils.Error(err, "Failed to update moduleDeployment", "moduleDeploymentName", moduleDeployment.Name) - } + ownerReference := moduleDeployment.GetOwnerReferences() + ownerReference = append(ownerReference, metav1.OwnerReference{ + APIVersion: deployment.APIVersion, + Kind: deployment.Kind, + UID: deployment.UID, + Name: deployment.Name, + BlockOwnerDeletion: pointer.Bool(true), + Controller: pointer.Bool(true), + }) + moduleDeployment.SetOwnerReferences(ownerReference) + utils.AddFinalizer(&moduleDeployment.ObjectMeta, finalizer.ModuleReplicaSetExistedFinalizer) + err = r.Client.Update(ctx, moduleDeployment) + if err != nil { + return utils.Error(err, "Failed to update moduleDeployment", "moduleDeploymentName", moduleDeployment.Name) } return nil } @@ -352,19 +350,23 @@ func (r *ModuleDeploymentReconciler) updateModuleReplicaSet(ctx context.Context, if deltaReplicas == 0 { moduleDeployment.Status.ReleaseStatus.Progress = v1alpha1.ModuleDeploymentReleaseProgressCompleted moduleDeployment.Status.ReleaseStatus.LastTransitionTime = metav1.Now() - moduleDeployment.Status.Conditions = append(moduleDeployment.Status.Conditions, v1alpha1.ModuleDeploymentCondition{ + moduleDeployment.Status.Conditions = utils.AppendModuleDeploymentCondition(moduleDeployment.Status.Conditions, v1alpha1.ModuleDeploymentCondition{ Type: v1alpha1.DeploymentAvailable, Status: corev1.ConditionTrue, LastTransitionTime: metav1.Now(), Message: "deployment release progress completed", }) - return ctrl.Result{}, r.Status().Update(ctx, moduleDeployment) + log.Log.Info("update moduleDeployment to completed failed", "moduleDeploymentName", moduleDeployment.Name) + return ctrl.Result{}, utils.Error(r.Status().Update(ctx, moduleDeployment), "update moduleDeployment to completed failed") } // wait moduleReplicaset ready - 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 + // TODO wait batch release completed + //if newRS.Spec.Replicas != curReplicas { + if moduleDeployment.Status.ReleaseStatus.BatchProgress == v1alpha1.ModuleDeploymentReleaseProgressExecuting { + log.Log.Info("module replicaset is not ready", "expectReplicas", newRS.Spec.Replicas, "curReplicas", curReplicas, "availableReplicas", newRS.Status.AvailableReplicas) + //return ctrl.Result{Requeue: true, RequeueAfter: utils.GetNextReconcileTime(time.Now())}, nil + return ctrl.Result{}, nil } replicas := int32(0) @@ -395,17 +397,13 @@ func (r *ModuleDeploymentReconciler) updateModuleReplicaSet(ctx context.Context, moduleDeployment.Status.ReleaseStatus.LastTransitionTime = now moduleDeployment.Status.ReleaseStatus.Progress = v1alpha1.ModuleDeploymentReleaseProgressExecuting - moduleDeployment.Status.Conditions = append(moduleDeployment.Status.Conditions, v1alpha1.ModuleDeploymentCondition{ + moduleDeployment.Status.Conditions = utils.AppendModuleDeploymentCondition(moduleDeployment.Status.Conditions, v1alpha1.ModuleDeploymentCondition{ Type: v1alpha1.DeploymentProgressing, Status: corev1.ConditionTrue, LastTransitionTime: now, Message: message, }) - //if len(moduleDeployment.Status.Conditions) > 10 { - // moduleDeployment.Status.Conditions = moduleDeployment.Status.Conditions[] - //} - var grayTime int if curBatch != realBatchCount { // needConfirm = true or batch = 1 and useBeta = true @@ -420,8 +418,15 @@ func (r *ModuleDeploymentReconciler) updateModuleReplicaSet(ctx context.Context, } } } + // TODO update current batch moduleDeployment.Status.ReleaseStatus.CurrentBatch += 1 - return ctrl.Result{Requeue: true, RequeueAfter: time.Duration(grayTime) * time.Second}, r.Status().Update(ctx, moduleDeployment) + moduleDeployment.Status.ReleaseStatus.BatchProgress = v1alpha1.ModuleDeploymentReleaseProgressExecuting + log.Log.Info("update moduleDeployment batch progress to executing", "moduleDeploymentName", moduleDeployment.Name) + err = r.Status().Update(ctx, moduleDeployment) + if err != nil { + return ctrl.Result{}, utils.Error(err, "update moduleDeployment batch progress to executing failed") + } + return ctrl.Result{Requeue: true, RequeueAfter: time.Duration(grayTime) * time.Second}, nil } // generate module replicas diff --git a/module-controller/internal/controller/modulereplicaset_controller.go b/module-controller/internal/controller/modulereplicaset_controller.go index a168658ed..c794ea632 100644 --- a/module-controller/internal/controller/modulereplicaset_controller.go +++ b/module-controller/internal/controller/modulereplicaset_controller.go @@ -110,22 +110,42 @@ func (r *ModuleReplicaSetReconciler) Reconcile(ctx context.Context, req ctrl.Req } // update status.replicas - currentReplicas := int32(0) + currentReplicas := int32(len(sameReplicaSetModules)) + availableReplicas := int32(0) // calculate the modules that have been installed successfully for i := 0; i < len(sameReplicaSetModules); i++ { status := sameReplicaSetModules[i].Status.Status if status == v1alpha1.ModuleInstanceStatusAvailable { - currentReplicas += 1 + availableReplicas += 1 } } - if currentReplicas != moduleReplicaSet.Status.CurrentReplicas { + if currentReplicas != moduleReplicaSet.Status.CurrentReplicas || availableReplicas != moduleReplicaSet.Status.AvailableReplicas { // if current replicas isn't equal to status.replicas, then we need update status moduleReplicaSet.Status.CurrentReplicas = currentReplicas + moduleReplicaSet.Status.AvailableReplicas = availableReplicas + log.Log.Info("update moduleReplicaSet current replicas and available replicas", "moduleReplicaSetName", moduleReplicaSet.Name) err := r.Status().Update(ctx, moduleReplicaSet) + if err != nil { + return ctrl.Result{}, utils.Error(err, "update moduleReplicaSet current replicas and available replicas failed") + } + return ctrl.Result{}, nil + } + + if moduleReplicaSet.ObjectMeta.Generation > 1 && moduleReplicaSet.Status.AvailableReplicas == moduleReplicaSet.Spec.Replicas { + // TODO available replicas equals to expect replicas + moduleDeployment := &v1alpha1.ModuleDeployment{} + err := r.Client.Get(ctx, types.NamespacedName{Namespace: moduleReplicaSet.Namespace, Name: moduleReplicaSet.Labels[label.ModuleDeploymentLabel]}, moduleDeployment) if err != nil { return ctrl.Result{}, err } + // moduleReplicaSet is completed, update moduleDeployment batch progress + moduleDeployment.Status.ReleaseStatus.BatchProgress = v1alpha1.ModuleDeploymentReleaseProgressCompleted + log.Log.Info("update moduleDeployment BatchProgress to completed", "moduleDeploymentName", moduleDeployment.Name) + err = r.Status().Update(ctx, moduleDeployment) + if err != nil { + return ctrl.Result{}, utils.Error(err, "update moduleDeployment BatchProgress failed") + } } // replicas change @@ -135,10 +155,10 @@ func (r *ModuleReplicaSetReconciler) Reconcile(ctx context.Context, req ctrl.Req if deltaReplicas != 0 { if moduleReplicaSet.Status.Replicas != moduleReplicaSet.Spec.Replicas { moduleReplicaSet.Status.Replicas = moduleReplicaSet.Spec.Replicas - moduleReplicaSet.Status.CurrentReplicas = currentReplicas + log.Log.Info("update moduleReplicaSet status Replicas", "moduleReplicaSetName", moduleReplicaSet.Name) err := r.Status().Update(ctx, moduleReplicaSet) if err != nil { - return ctrl.Result{}, err + return ctrl.Result{}, utils.Error(err, "update moduleReplicaSet status Replicas failed") } } else { log.Log.Info("Already try to reconcile to the desired replicas", "moduleReplicaSetName", moduleReplicaSet.Name) diff --git a/module-controller/internal/utils/controller_utils.go b/module-controller/internal/utils/controller_utils.go index 21c14dbd2..266982f10 100644 --- a/module-controller/internal/utils/controller_utils.go +++ b/module-controller/internal/utils/controller_utils.go @@ -2,6 +2,7 @@ package utils import ( "fmt" + "github.com/sofastack/sofa-serverless/api/v1alpha1" "strconv" "strings" "time" @@ -52,7 +53,15 @@ func HasFinalizer(meta *metav1.ObjectMeta, needle string) bool { } } } + return false +} +func HasOwnerReference(meta *metav1.ObjectMeta, needle string) bool { + for _, ownerReference := range meta.GetOwnerReferences() { + if needle == ownerReference.Name { + return true + } + } return false } @@ -60,6 +69,11 @@ func Key(req ctrl.Request) string { return fmt.Sprintf("%s/%s", req.Namespace, req.Name) } +func Error(err error, msg string, keysAndValues ...interface{}) error { + log.Log.Error(err, msg, keysAndValues...) + return err +} + func GetNextReconcileTime(currentTime time.Time) time.Duration { timeDuration := time.Now().Sub(currentTime) var nextDuration time.Duration @@ -84,11 +98,6 @@ func GetModuleCountFromPod(pod *corev1.Pod) (count int) { return count } -func Error(err error, msg string, keysAndValues ...interface{}) error { - log.Log.Error(err, msg, keysAndValues...) - return err -} - func GetModuleInstanceCount(pod corev1.Pod) int { if pod.Labels[label.ModuleInstanceCount] == "" { return 0 @@ -99,3 +108,11 @@ func GetModuleInstanceCount(pod corev1.Pod) int { } return count } + +func AppendModuleDeploymentCondition(conditions []v1alpha1.ModuleDeploymentCondition, condition v1alpha1.ModuleDeploymentCondition) []v1alpha1.ModuleDeploymentCondition { + if len(conditions) < 10 { + return append(conditions, condition) + } else { + return append(conditions[1:], condition) + } +} From e74ff9e44e242727f9d2f942ed004e44352899ee Mon Sep 17 00:00:00 2001 From: "jinle.xjl" Date: Tue, 31 Oct 2023 20:05:20 +0800 Subject: [PATCH 2/2] fix ut --- ...rverless.alipay.com_moduledeployments.yaml | 2 +- .../internal/controller/module_controller.go | 18 +- .../controller/module_controller_suit_test.go | 3 +- .../controller/moduledeployment_controller.go | 24 +- ...ontroller_operation_strategy_suit_test.go} | 5 +- ...ledeployment_controller_scale_suit_test.go | 161 +++++++++++ ...ent_controller_upgrade_policy_suit_test.go | 133 +++++++++ .../controller/modulereplicaset_controller.go | 25 +- .../modulereplicaset_controller_suit_test.go | 266 ------------------ .../internal/controller/pod_controller.go | 4 +- .../controller/pod_controller_suit_test.go | 1 + .../internal/utils/controller_utils.go | 24 ++ .../internal/utils/controller_utils_test.go | 41 +++ .../internal/utils/test_utils.go | 17 +- .../internal/utils/test_utils_test.go | 2 +- 15 files changed, 419 insertions(+), 307 deletions(-) rename module-controller/internal/controller/{moduledeployment_controller_suit_test.go => moduledeployment_controller_operation_strategy_suit_test.go} (98%) create mode 100644 module-controller/internal/controller/moduledeployment_controller_scale_suit_test.go create mode 100644 module-controller/internal/controller/moduledeployment_controller_upgrade_policy_suit_test.go delete mode 100644 module-controller/internal/controller/modulereplicaset_controller_suit_test.go diff --git a/module-controller/config/crd/bases/serverless.alipay.com_moduledeployments.yaml b/module-controller/config/crd/bases/serverless.alipay.com_moduledeployments.yaml index 732782562..27bf6ebd0 100644 --- a/module-controller/config/crd/bases/serverless.alipay.com_moduledeployments.yaml +++ b/module-controller/config/crd/bases/serverless.alipay.com_moduledeployments.yaml @@ -227,7 +227,7 @@ spec: releaseStatus: properties: batchProgress: - description: the phase current bath release reach + description: the phase current batch release reach type: string currentBatch: description: Records the current batch serial number. diff --git a/module-controller/internal/controller/module_controller.go b/module-controller/internal/controller/module_controller.go index d47d038e6..e8f2c4cea 100644 --- a/module-controller/internal/controller/module_controller.go +++ b/module-controller/internal/controller/module_controller.go @@ -127,7 +127,7 @@ func (r *ModuleReconciler) parseModuleInstanceStatus(ctx context.Context, module module.Status.Status = moduleInstanceStatus module.Status.LastTransitionTime = metav1.Now() log.Log.Info(fmt.Sprintf("%s%s", "module status change to ", moduleInstanceStatus), "moduleName", module.Name) - err := r.Status().Update(ctx, module) + err := utils.UpdateStatus(r.Client, ctx, module) if err != nil { return ctrl.Result{}, utils.Error(err, "update module status failed") } @@ -246,7 +246,7 @@ func (r *ModuleReconciler) cleanLabelAndFinalizer(ctx context.Context, module *v // remove finalizer log.Log.Info("start clean module install finalizer", "moduleName", module.Spec.Module.Name, "module", module.Name) utils.RemoveFinalizer(&module.ObjectMeta, finalizer.AllocatePodFinalizer) - err := r.Client.Update(ctx, module) + err := utils.UpdateResource(r.Client, ctx, module) if err != nil { return utils.Error(err, "failed to clean module install finalizer", "moduleName", module.Spec.Module.Name, "module", module.Name) } @@ -268,7 +268,7 @@ func (r *ModuleReconciler) cleanLabelAndFinalizer(ctx context.Context, module *v pod.Labels[label.ModuleInstanceCount] = strconv.Itoa(count - 1) } } - err = r.Client.Update(ctx, pod) + err = utils.UpdateResource(r.Client, ctx, pod) if err != nil { return err } @@ -285,7 +285,7 @@ func (r *ModuleReconciler) handlePendingModuleInstance(ctx context.Context, modu module.Status.Status = v1alpha1.ModuleInstanceStatusPrepare module.Status.LastTransitionTime = metav1.Now() log.Log.Info(fmt.Sprintf("%s%s", "module status change to ", v1alpha1.ModuleInstanceStatusPrepare), "moduleName", module.Name) - err := r.Status().Update(ctx, module) + err := utils.UpdateStatus(r.Client, ctx, module) if err != nil { return ctrl.Result{}, utils.Error(err, "update module status from pending to prepare failed") } @@ -325,7 +325,7 @@ func (r *ModuleReconciler) handlePendingModuleInstance(ctx context.Context, modu return ctrl.Result{RequeueAfter: requeueAfter}, nil } UpdatePodLabelBeforeInstallModule(pod, module.Spec.Module.Name) - err = r.Client.Update(ctx, &pod) + err = utils.UpdateResource(r.Client, ctx, &pod) if err != nil { return ctrl.Result{}, err } @@ -344,7 +344,7 @@ func (r *ModuleReconciler) handlePendingModuleInstance(ctx context.Context, modu } module.SetOwnerReferences(owner) utils.AddFinalizer(&module.ObjectMeta, finalizer.AllocatePodFinalizer) - err = r.Client.Update(ctx, module) + err = utils.UpdateResource(r.Client, ctx, module) if err != nil { return ctrl.Result{}, err } @@ -358,7 +358,7 @@ func (r *ModuleReconciler) handlePrepareModuleInstance(ctx context.Context, modu module.Status.Status = v1alpha1.ModuleInstanceStatusUpgrading module.Status.LastTransitionTime = metav1.Now() log.Log.Info(fmt.Sprintf("%s%s", "module status change to ", v1alpha1.ModuleInstanceStatusUpgrading), "moduleName", module.Name) - err := r.Status().Update(ctx, module) + err := utils.UpdateStatus(r.Client, ctx, module) if err != nil { return ctrl.Result{}, utils.Error(err, "update module from prepare to upgrading failed") } @@ -383,7 +383,7 @@ func (r *ModuleReconciler) handleUpgradingModuleInstance(ctx context.Context, mo module.Status.Status = v1alpha1.ModuleInstanceStatusCompleting module.Status.LastTransitionTime = metav1.Now() log.Log.Info(fmt.Sprintf("%s%s", "module status change to ", v1alpha1.ModuleInstanceStatusCompleting), "moduleName", module.Name) - err = r.Status().Update(ctx, module) + err = utils.UpdateStatus(r.Client, ctx, module) if err != nil { return ctrl.Result{}, utils.Error(err, "update module from upgrading to completing failed") } @@ -397,7 +397,7 @@ func (r *ModuleReconciler) handleCompletingModuleInstance(ctx context.Context, m module.Status.Status = v1alpha1.ModuleInstanceStatusAvailable module.Status.LastTransitionTime = metav1.Now() log.Log.Info(fmt.Sprintf("%s%s", "module status change to ", v1alpha1.ModuleInstanceStatusAvailable), "moduleName", module.Name) - err := r.Status().Update(ctx, module) + err := utils.UpdateStatus(r.Client, ctx, module) if err != nil { return ctrl.Result{}, utils.Error(err, "update module from completing to available failed") } diff --git a/module-controller/internal/controller/module_controller_suit_test.go b/module-controller/internal/controller/module_controller_suit_test.go index d8879f8c0..0efd9c091 100644 --- a/module-controller/internal/controller/module_controller_suit_test.go +++ b/module-controller/internal/controller/module_controller_suit_test.go @@ -20,6 +20,7 @@ import ( ) var _ = Describe("Module Controller", func() { + const timeout = time.Second * 30 const interval = time.Second * 3 moduleName := "test-module-name" @@ -78,7 +79,7 @@ var _ = Describe("Module Controller", func() { module.Labels[label.ModuleReplicasetLabel] = moduleReplicaSetName module.Labels[label.ModuleNameLabel] = "test-module" utils.AddFinalizer(&module.ObjectMeta, finalizer.AllocatePodFinalizer) - moduleReplicaSet := prepareModuleReplicaSet(namespaceName, moduleReplicaSetName) + moduleReplicaSet := prepareModuleReplicaSet(namespaceName, moduleReplicaSetName, "") moduleReplicaSet.Spec.Template.Spec.Module.Url = updateModuleUrl Expect(k8sClient.Create(context.TODO(), &moduleReplicaSet)).Should(Succeed()) Expect(k8sClient.Update(context.TODO(), &module)).Should(Succeed()) diff --git a/module-controller/internal/controller/moduledeployment_controller.go b/module-controller/internal/controller/moduledeployment_controller.go index d42cfe18f..13dd72f02 100644 --- a/module-controller/internal/controller/moduledeployment_controller.go +++ b/module-controller/internal/controller/moduledeployment_controller.go @@ -96,7 +96,7 @@ func (r *ModuleDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req } // update moduleDeployment owner reference - if utils.HasOwnerReference(&moduleDeployment.ObjectMeta, moduleDeployment.Spec.BaseDeploymentName) { + if !utils.HasOwnerReference(&moduleDeployment.ObjectMeta, moduleDeployment.Spec.BaseDeploymentName) { err = r.updateOwnerReference(ctx, moduleDeployment) if err != nil { return ctrl.Result{}, err @@ -123,7 +123,7 @@ func (r *ModuleDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req case v1alpha1.ModuleDeploymentReleaseProgressInit: handleInitModuleDeployment(moduleDeployment, newRS) log.Log.Info("update moduleDeployment release status", "moduleDeploymentName", moduleDeployment.Name) - if err := r.Status().Update(ctx, moduleDeployment); err != nil { + if err := utils.UpdateStatus(r.Client, ctx, moduleDeployment); err != nil { return ctrl.Result{}, utils.Error(err, "update release status failed when init moduleDeployment") } case v1alpha1.ModuleDeploymentReleaseProgressExecuting: @@ -132,13 +132,13 @@ func (r *ModuleDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req if moduleDeployment.Spec.Replicas != newRS.Spec.Replicas { moduleDeployment.Status.ReleaseStatus.Progress = v1alpha1.ModuleDeploymentReleaseProgressInit log.Log.Info("update release status progress to init when complete moduleDeployment", "moduleDeploymentName", moduleDeployment.Name) - if err := r.Status().Update(ctx, moduleDeployment); err != nil { + if err := utils.UpdateStatus(r.Client, ctx, moduleDeployment); err != nil { return ctrl.Result{}, utils.Error(err, "update release status progress to init failed when complete moduleDeployment") } } if !moduleVersionChanged && isUrlChange(moduleDeployment.Spec.Template.Spec.Module, newRS.Spec.Template.Spec.Module) { newRS.Spec.Template.Spec.Module = moduleDeployment.Spec.Template.Spec.Module - if err := r.Client.Update(ctx, newRS); err != nil { + if err := utils.UpdateResource(r.Client, ctx, newRS); err != nil { return ctrl.Result{}, err } } @@ -150,14 +150,14 @@ func (r *ModuleDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req moduleDeployment.Status.ReleaseStatus.Progress = v1alpha1.ModuleDeploymentReleaseProgressPaused log.Log.Info("update moduleDeployment releaseStatus progress to paused", "moduleDeploymentName", moduleDeployment.Name) - if err := r.Status().Update(ctx, moduleDeployment); err != nil { + if err := utils.UpdateStatus(r.Client, ctx, moduleDeployment); err != nil { return ctrl.Result{}, utils.Error(err, "update moduleDeployment releaseStatus progress to paused failed") } case v1alpha1.ModuleDeploymentReleaseProgressPaused: if !moduleDeployment.Spec.Pause && time.Since(moduleDeployment.Status.ReleaseStatus.NextReconcileTime.Time) >= 0 { moduleDeployment.Status.ReleaseStatus.Progress = v1alpha1.ModuleDeploymentReleaseProgressExecuting log.Log.Info("update moduleDeployment progress from paused to executing", "moduleDeploymentName", moduleDeployment.Name) - if err := r.Status().Update(ctx, moduleDeployment); err != nil { + if err := utils.UpdateStatus(r.Client, ctx, moduleDeployment); err != nil { return ctrl.Result{}, utils.Error(err, "update moduleDeployment progress from paused to executing failed") } } @@ -223,7 +223,7 @@ func (r *ModuleDeploymentReconciler) handleDeletingModuleDeployment(ctx context. } else { log.Log.Info("moduleReplicaSet is deleted, remove moduleDeployment finalizer", "moduleDeploymentName", moduleDeployment.Name) utils.RemoveFinalizer(&moduleDeployment.ObjectMeta, finalizer.ModuleReplicaSetExistedFinalizer) - err := r.Client.Update(ctx, moduleDeployment) + err := utils.UpdateResource(r.Client, ctx, moduleDeployment) if err != nil { return ctrl.Result{}, err } @@ -249,7 +249,7 @@ func (r *ModuleDeploymentReconciler) updateOwnerReference(ctx context.Context, m }) moduleDeployment.SetOwnerReferences(ownerReference) utils.AddFinalizer(&moduleDeployment.ObjectMeta, finalizer.ModuleReplicaSetExistedFinalizer) - err = r.Client.Update(ctx, moduleDeployment) + err = utils.UpdateResource(r.Client, ctx, moduleDeployment) if err != nil { return utils.Error(err, "Failed to update moduleDeployment", "moduleDeploymentName", moduleDeployment.Name) } @@ -326,7 +326,7 @@ func (r *ModuleDeploymentReconciler) updateModuleReplicas( log.Log.Info("prepare to update newRS", "moduleReplicaSetName", newRS.Name) newRS.Spec.Replicas = replicas newRS.Spec.Template.Spec.Module = moduleSpec.Module - err := r.Client.Update(ctx, newRS) + err := utils.UpdateResource(r.Client, ctx, newRS) if err != nil { return utils.Error(err, "Failed to update newRS", "moduleReplicaSetName", newRS.Name) } @@ -356,8 +356,8 @@ func (r *ModuleDeploymentReconciler) updateModuleReplicaSet(ctx context.Context, LastTransitionTime: metav1.Now(), Message: "deployment release progress completed", }) - log.Log.Info("update moduleDeployment to completed failed", "moduleDeploymentName", moduleDeployment.Name) - return ctrl.Result{}, utils.Error(r.Status().Update(ctx, moduleDeployment), "update moduleDeployment to completed failed") + log.Log.Info("update moduleDeployment to completed", "moduleDeploymentName", moduleDeployment.Name) + return ctrl.Result{}, utils.Error(utils.UpdateStatus(r.Client, ctx, moduleDeployment), "update moduleDeployment to completed failed") } // wait moduleReplicaset ready @@ -422,7 +422,7 @@ func (r *ModuleDeploymentReconciler) updateModuleReplicaSet(ctx context.Context, moduleDeployment.Status.ReleaseStatus.CurrentBatch += 1 moduleDeployment.Status.ReleaseStatus.BatchProgress = v1alpha1.ModuleDeploymentReleaseProgressExecuting log.Log.Info("update moduleDeployment batch progress to executing", "moduleDeploymentName", moduleDeployment.Name) - err = r.Status().Update(ctx, moduleDeployment) + err = utils.UpdateStatus(r.Client, ctx, moduleDeployment) if err != nil { return ctrl.Result{}, utils.Error(err, "update moduleDeployment batch progress to executing failed") } diff --git a/module-controller/internal/controller/moduledeployment_controller_suit_test.go b/module-controller/internal/controller/moduledeployment_controller_operation_strategy_suit_test.go similarity index 98% rename from module-controller/internal/controller/moduledeployment_controller_suit_test.go rename to module-controller/internal/controller/moduledeployment_controller_operation_strategy_suit_test.go index 51259a1c0..71307cae6 100644 --- a/module-controller/internal/controller/moduledeployment_controller_suit_test.go +++ b/module-controller/internal/controller/moduledeployment_controller_operation_strategy_suit_test.go @@ -17,7 +17,8 @@ import ( "github.com/sofastack/sofa-serverless/internal/constants/label" ) -var _ = Describe("ModuleDeployment Controller", func() { +var _ = Describe("ModuleDeployment Controller OperationStrategy Test", func() { + const timeout = time.Second * 30 const interval = time.Second * 5 @@ -174,7 +175,7 @@ var _ = Describe("ModuleDeployment Controller", func() { It("0. prepare 2 pods", func() { Eventually(func() bool { pod := preparePod(namespace, "fake-pod-3") - pod.Labels[label.ModuleLabelPrefix+"dynamic-provider"] = "true" + //pod.Labels[label.ModuleLabelPrefix+"dynamic-provider"] = "true" if err := k8sClient.Create(context.TODO(), &pod); err != nil { return false } diff --git a/module-controller/internal/controller/moduledeployment_controller_scale_suit_test.go b/module-controller/internal/controller/moduledeployment_controller_scale_suit_test.go new file mode 100644 index 000000000..5ad6c4297 --- /dev/null +++ b/module-controller/internal/controller/moduledeployment_controller_scale_suit_test.go @@ -0,0 +1,161 @@ +package controller + +import ( + "context" + "k8s.io/apimachinery/pkg/labels" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/sofastack/sofa-serverless/api/v1alpha1" + "github.com/sofastack/sofa-serverless/internal/constants/label" + "github.com/sofastack/sofa-serverless/internal/utils" +) + +var _ = Describe("ModuleReplicaSet Controller Scale Test", func() { + + const timeout = time.Second * 30 + const interval = time.Second * 3 + + moduleDeploymentName := "test-module-deployment" + var moduleReplicaSetName string + namespaceName := "scale-test-namespace" + podName1 := "test-pod" + + namespaceObj := prepareNamespace(namespaceName) + deployment := prepareDeployment(namespaceName) + moduleDeployment := utils.PrepareModuleDeployment(namespaceName, moduleDeploymentName) + pod := preparePod(namespaceName, podName1) + Context("create module deployment", func() { + It("prepare deployment and pod", func() { + Expect(k8sClient.Create(context.TODO(), &namespaceObj)).Should(Succeed()) + + Expect(k8sClient.Create(context.TODO(), &deployment)).Should(Succeed()) + + Expect(k8sClient.Create(context.TODO(), &pod)).Should(Succeed()) + + pod.Status.PodIP = "127.0.0.1" + Expect(k8sClient.Status().Update(context.TODO(), &pod)).Should(Succeed()) + }) + + It("create module replicaset", func() { + Expect(k8sClient.Create(context.TODO(), &moduleDeployment)).Should(Succeed()) + + Eventually(func() bool { + set := map[string]string{ + label.ModuleDeploymentLabel: moduleDeployment.Name, + } + replicaSetList := &v1alpha1.ModuleReplicaSetList{} + err := k8sClient.List(context.TODO(), replicaSetList, &client.ListOptions{LabelSelector: labels.SelectorFromSet(set)}, client.InNamespace(moduleDeployment.Namespace)) + if err != nil { + return false + } + if len(replicaSetList.Items) != 1 { + return false + } + moduleReplicaSetName = replicaSetList.Items[0].Name + k8sClient.Get(context.TODO(), types.NamespacedName{Name: moduleDeploymentName, Namespace: namespaceName}, &moduleDeployment) + if moduleDeployment.Status.ReleaseStatus.Progress != v1alpha1.ModuleDeploymentReleaseProgressCompleted { + return false + } + return true + }, timeout, interval).Should(BeTrue()) + }) + }) + + Context("scale up", func() { + It("replica is 1 and create one module", func() { + var newModuleDeployment v1alpha1.ModuleDeployment + k8sClient.Get(context.TODO(), types.NamespacedName{Name: moduleDeploymentName, Namespace: namespaceName}, &newModuleDeployment) + newModuleDeployment.Spec.Replicas = 1 + Expect(k8sClient.Update(context.TODO(), &newModuleDeployment)).Should(Succeed()) + Eventually(func() bool { + // replicas is 1 + key := types.NamespacedName{ + Name: moduleReplicaSetName, + Namespace: namespaceName, + } + moduleReplicaSet := v1alpha1.ModuleReplicaSet{} + err := k8sClient.Get(context.TODO(), key, &moduleReplicaSet) + if err != nil && moduleReplicaSet.Spec.Replicas != 1 { + return false + } + + // module is 1 and allocate to pod1 + selector, err := metav1.LabelSelectorAsSelector(&moduleReplicaSet.Spec.Selector) + modules := &v1alpha1.ModuleList{} + err = k8sClient.List(context.TODO(), modules, &client.ListOptions{Namespace: moduleReplicaSet.Namespace, LabelSelector: selector}) + if err == nil && len(modules.Items) == 1 && modules.Items[0].Labels[label.BaseInstanceIpLabel] == pod.Status.PodIP { + return true + } + return false + }, timeout, interval).Should(BeTrue()) + }) + }) + + Context("scale down", func() { + It("replica is 0 and delete all module", func() { + var newModuleDeployment v1alpha1.ModuleDeployment + k8sClient.Get(context.TODO(), types.NamespacedName{Name: moduleDeploymentName, Namespace: namespaceName}, &newModuleDeployment) + newModuleDeployment.Spec.Replicas = 0 + Expect(k8sClient.Update(context.TODO(), &newModuleDeployment)).Should(Succeed()) + Eventually(func() bool { + + // replicas is 0 + key := types.NamespacedName{ + Name: moduleReplicaSetName, + Namespace: namespaceName, + } + moduleReplicaSet := v1alpha1.ModuleReplicaSet{} + err := k8sClient.Get(context.TODO(), key, &moduleReplicaSet) + if err != nil && moduleReplicaSet.Spec.Replicas != 0 { + return false + } + + // modules are all deleted + selector, err := metav1.LabelSelectorAsSelector(&moduleReplicaSet.Spec.Selector) + modules := &v1alpha1.ModuleList{} + err = k8sClient.List(context.TODO(), modules, &client.ListOptions{Namespace: moduleReplicaSet.Namespace, LabelSelector: selector}) + if err == nil && len(modules.Items) == 0 { + return true + } + return false + }, timeout, interval).Should(BeTrue()) + }) + }) +}) + +func prepareModuleReplicaSet(namespace, moduleReplicaSetName, moduleDeploymentName string) v1alpha1.ModuleReplicaSet { + + moduleReplicaSet := v1alpha1.ModuleReplicaSet{ + Spec: v1alpha1.ModuleReplicaSetSpec{ + Replicas: 1, + Template: v1alpha1.ModuleTemplateSpec{ + Spec: v1alpha1.ModuleSpec{ + Module: v1alpha1.ModuleInfo{ + Name: "dynamic-provider", + Version: "1.0.0", + Url: "http://serverless-opensource.oss-cn-shanghai.aliyuncs.com/module-packages/stable/dynamic-provider-1.0.0-ark-biz.jar", + }, + }, + }, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: moduleReplicaSetName, + Namespace: namespace, + Labels: map[string]string{ + "app": "dynamic-stock", + label.MaxModuleCount: "10", + label.ModuleSchedulingStrategy: string(v1alpha1.Scatter), + label.ModuleDeploymentLabel: moduleDeploymentName, + label.ModuleReplicasetRevisionLabel: "1", + }, + Annotations: map[string]string{}, + }, + } + return moduleReplicaSet +} diff --git a/module-controller/internal/controller/moduledeployment_controller_upgrade_policy_suit_test.go b/module-controller/internal/controller/moduledeployment_controller_upgrade_policy_suit_test.go new file mode 100644 index 000000000..373fcc0e9 --- /dev/null +++ b/module-controller/internal/controller/moduledeployment_controller_upgrade_policy_suit_test.go @@ -0,0 +1,133 @@ +package controller + +import ( + "context" + "k8s.io/apimachinery/pkg/labels" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/sofastack/sofa-serverless/api/v1alpha1" + "github.com/sofastack/sofa-serverless/internal/constants/label" + "github.com/sofastack/sofa-serverless/internal/utils" +) + +var _ = Describe("ModuleReplicaSet Controller Scale Test", func() { + + const timeout = time.Second * 30 + const interval = time.Second * 3 + + moduleDeploymentName := "test-module-deployment-for-upgrade-policy" + var moduleReplicaSetName string + namespaceName := "upgrade-policy-test-namespace" + podName1 := "test-pod-for-upgrade-policy-1" + podName2 := "test-pod-for-upgrade-policy-2" + + namespaceObj := prepareNamespace(namespaceName) + deployment := prepareDeployment(namespaceName) + moduleDeployment := utils.PrepareModuleDeployment(namespaceName, moduleDeploymentName) + moduleDeployment.Spec.Replicas = 1 + moduleDeployment.Spec.OperationStrategy.UpgradePolicy = v1alpha1.ScaleUpThenScaleDownUpgradePolicy + pod := preparePod(namespaceName, podName1) + Context("create module deployment", func() { + It("prepare deployment and pod", func() { + Expect(k8sClient.Create(context.TODO(), &namespaceObj)).Should(Succeed()) + + Expect(k8sClient.Create(context.TODO(), &deployment)).Should(Succeed()) + + Expect(k8sClient.Create(context.TODO(), &pod)).Should(Succeed()) + + pod.Status.PodIP = "127.0.0.1" + Expect(k8sClient.Status().Update(context.TODO(), &pod)).Should(Succeed()) + + }) + + It("create module replicaset", func() { + Expect(k8sClient.Create(context.TODO(), &moduleDeployment)).Should(Succeed()) + + Eventually(func() bool { + set := map[string]string{ + label.ModuleDeploymentLabel: moduleDeployment.Name, + } + replicaSetList := &v1alpha1.ModuleReplicaSetList{} + err := k8sClient.List(context.TODO(), replicaSetList, &client.ListOptions{LabelSelector: labels.SelectorFromSet(set)}, client.InNamespace(moduleDeployment.Namespace)) + if err != nil || len(replicaSetList.Items) != 1 { + return false + } + moduleReplicaSetName = replicaSetList.Items[0].Name + // module is 1 and allocate to pod1 + modules := &v1alpha1.ModuleList{} + err = k8sClient.List(context.TODO(), modules, &client.ListOptions{Namespace: namespaceName, LabelSelector: labels.SelectorFromSet(map[string]string{ + label.ModuleReplicasetLabel: moduleReplicaSetName, + })}) + if err == nil && len(modules.Items) == 1 && modules.Items[0].Labels[label.BaseInstanceIpLabel] == pod.Status.PodIP { + return true + } + return false + }, timeout, interval).Should(BeTrue()) + }) + }) + + Context("create module replicaset with scaleup_then_scaledown upgradePolicy", func() { + It("create module", func() { + pod2 := preparePod(namespaceName, podName2) + Expect(k8sClient.Create(context.TODO(), &pod2)).Should(Succeed()) + pod2.Status.PodIP = "127.0.0.2" + Expect(k8sClient.Status().Update(context.TODO(), &pod2)) + + k8sClient.Get(context.TODO(), types.NamespacedName{Name: moduleDeploymentName, Namespace: namespaceName}, &moduleDeployment) + moduleDeployment.Spec.OperationStrategy.UpgradePolicy = v1alpha1.ScaleUpThenScaleDownUpgradePolicy + moduleDeployment.Spec.Template.Spec.Module.Version = "1.0.1" + k8sClient.Update(context.TODO(), &moduleDeployment) + + Eventually(func() bool { + + // old replicaSet replicas is 0 + key := types.NamespacedName{ + Name: moduleReplicaSetName, + Namespace: namespaceName, + } + var oldModuleReplicaSet v1alpha1.ModuleReplicaSet + k8sClient.Get(context.TODO(), key, &oldModuleReplicaSet) + + replicaSetList := &v1alpha1.ModuleReplicaSetList{} + err := k8sClient.List(context.TODO(), replicaSetList, &client.ListOptions{LabelSelector: labels.SelectorFromSet(map[string]string{label.ModuleDeploymentLabel: moduleDeployment.Name})}, client.InNamespace(moduleDeployment.Namespace)) + + newModuleReplicaSet := v1alpha1.ModuleReplicaSet{} + for _, moduleReplicaSet := range replicaSetList.Items { + if moduleReplicaSet.Name != oldModuleReplicaSet.Name { + newModuleReplicaSet = moduleReplicaSet + } + } + if newModuleReplicaSet.Name == "" { + return false + } + + // module is 1 + modules := &v1alpha1.ModuleList{} + err = k8sClient.List(context.TODO(), modules, &client.ListOptions{Namespace: namespaceName, LabelSelector: labels.SelectorFromSet(map[string]string{ + label.ModuleReplicasetLabel: newModuleReplicaSet.Name, + })}) + if err != nil || len(modules.Items) != 1 { + return false + } + + // pod is available and reallocate to pod2 + if v1alpha1.ModuleInstanceStatusAvailable != modules.Items[0].Status.Status || modules.Items[0].Labels[label.BaseInstanceIpLabel] != pod2.Status.PodIP { + return false + } + + // old moduleReplicaSet replicas is 0 + if oldModuleReplicaSet.Spec.Replicas != 0 { + return false + } + + return true + }, timeout, interval).Should(BeTrue()) + }) + }) + +}) diff --git a/module-controller/internal/controller/modulereplicaset_controller.go b/module-controller/internal/controller/modulereplicaset_controller.go index c794ea632..d6272d2f2 100644 --- a/module-controller/internal/controller/modulereplicaset_controller.go +++ b/module-controller/internal/controller/modulereplicaset_controller.go @@ -125,7 +125,7 @@ func (r *ModuleReplicaSetReconciler) Reconcile(ctx context.Context, req ctrl.Req moduleReplicaSet.Status.CurrentReplicas = currentReplicas moduleReplicaSet.Status.AvailableReplicas = availableReplicas log.Log.Info("update moduleReplicaSet current replicas and available replicas", "moduleReplicaSetName", moduleReplicaSet.Name) - err := r.Status().Update(ctx, moduleReplicaSet) + err := utils.UpdateStatus(r.Client, ctx, moduleReplicaSet) if err != nil { return ctrl.Result{}, utils.Error(err, "update moduleReplicaSet current replicas and available replicas failed") } @@ -133,7 +133,7 @@ func (r *ModuleReplicaSetReconciler) Reconcile(ctx context.Context, req ctrl.Req } if moduleReplicaSet.ObjectMeta.Generation > 1 && moduleReplicaSet.Status.AvailableReplicas == moduleReplicaSet.Spec.Replicas { - // TODO available replicas equals to expect replicas + // available replicas equals to expect replicas moduleDeployment := &v1alpha1.ModuleDeployment{} err := r.Client.Get(ctx, types.NamespacedName{Namespace: moduleReplicaSet.Namespace, Name: moduleReplicaSet.Labels[label.ModuleDeploymentLabel]}, moduleDeployment) if err != nil { @@ -142,10 +142,11 @@ func (r *ModuleReplicaSetReconciler) Reconcile(ctx context.Context, req ctrl.Req // moduleReplicaSet is completed, update moduleDeployment batch progress moduleDeployment.Status.ReleaseStatus.BatchProgress = v1alpha1.ModuleDeploymentReleaseProgressCompleted log.Log.Info("update moduleDeployment BatchProgress to completed", "moduleDeploymentName", moduleDeployment.Name) - err = r.Status().Update(ctx, moduleDeployment) + err = utils.UpdateStatus(r.Client, ctx, moduleDeployment) if err != nil { return ctrl.Result{}, utils.Error(err, "update moduleDeployment BatchProgress failed") } + return ctrl.Result{}, nil } // replicas change @@ -156,7 +157,7 @@ func (r *ModuleReplicaSetReconciler) Reconcile(ctx context.Context, req ctrl.Req if moduleReplicaSet.Status.Replicas != moduleReplicaSet.Spec.Replicas { moduleReplicaSet.Status.Replicas = moduleReplicaSet.Spec.Replicas log.Log.Info("update moduleReplicaSet status Replicas", "moduleReplicaSetName", moduleReplicaSet.Name) - err := r.Status().Update(ctx, moduleReplicaSet) + err := utils.UpdateStatus(r.Client, ctx, moduleReplicaSet) if err != nil { return ctrl.Result{}, utils.Error(err, "update moduleReplicaSet status Replicas failed") } @@ -212,7 +213,7 @@ func (r *ModuleReplicaSetReconciler) compareAndUpdateModule(ctx context.Context, existedModule.Spec.Module.Name = desiredModule.Name existedModule.Spec.Module.Version = desiredModule.Version existedModule.Spec.Module.Url = desiredModule.Url - err := r.Client.Update(ctx, &existedModule) + err := utils.UpdateResource(r.Client, ctx, &existedModule) if err != nil { return utils.Error(err, "Failed to update module", "moduleName", existedModule.Name) } @@ -234,7 +235,7 @@ func (r *ModuleReplicaSetReconciler) handleDeletingModuleReplicaSet(ctx context. // all module is removed, remove module replicaset finalizer log.Log.Info("all modules are deleted, remove moduleReplicaSet finalizer", "moduleReplicaSetName", moduleReplicaSet.Name) utils.RemoveFinalizer(&moduleReplicaSet.ObjectMeta, finalizer.ModuleExistedFinalizer) - err := r.Client.Update(ctx, moduleReplicaSet) + err := utils.UpdateResource(r.Client, ctx, moduleReplicaSet) if err != nil { return ctrl.Result{}, err } @@ -246,7 +247,7 @@ func (r *ModuleReplicaSetReconciler) handleDeletingModuleReplicaSet(ctx context. log.Log.Info("moduleReplicaSet is deleting, delete module", "moduleReplicaSetName", moduleReplicaSet.Name, "module", existedModule.Name, "ip", existedModule.Labels[label.BaseInstanceIpLabel]) if existedModule.Labels[label.DeleteModuleLabel] != "true" { existedModule.Labels[label.DeleteModuleLabel] = "true" - err = r.Client.Update(ctx, &existedModule) + err = utils.UpdateResource(r.Client, ctx, &existedModule) if err != nil { log.Log.Error(err, "Failed to update uninstall module label", "moduleName", existedModule.Name) } @@ -379,7 +380,7 @@ func (r *ModuleReplicaSetReconciler) scaledown(ctx context.Context, existedModul targetPod := moduleToPod[module.Name] if targetPod.Labels[label.DeletePodDirectlyLabel] != "true" { targetPod.Labels[label.DeletePodDirectlyLabel] = "true" - err = r.Client.Update(ctx, targetPod) + err = utils.UpdateResource(r.Client, ctx, targetPod) if err != nil { log.Log.Error(err, "Failed to update delete pod label", "module", module, "podName", targetPod.Name) } @@ -387,7 +388,7 @@ func (r *ModuleReplicaSetReconciler) scaledown(ctx context.Context, existedModul } else { if module.Labels[label.DeleteModuleLabel] != "true" { module.Labels[label.DeleteModuleLabel] = "true" - err = r.Client.Update(ctx, &module) + err = utils.UpdateResource(r.Client, ctx, &module) if err != nil { log.Log.Error(err, "Failed to delete module", "module", module) } @@ -427,7 +428,7 @@ func (r *ModuleReplicaSetReconciler) scaleDownOldPods(ctx context.Context, toAll deleteReplicas -= otherModuleReplicaSet.Spec.Replicas otherModuleReplicaSet.Spec.Replicas = 0 } - if err := r.Client.Update(ctx, otherModuleReplicaSet); err != nil { + if err := utils.UpdateResource(r.Client, ctx, otherModuleReplicaSet); err != nil { return utils.Error(err, "Failed to update other replicaset", "moduleReplicaSetName", otherModuleReplicaSet.Name) } } @@ -470,7 +471,7 @@ func (r *ModuleReplicaSetReconciler) getScaleUpCandidatePods(sameReplicaSetModul if cntStr, ok := pod.Labels[label.ModuleInstanceCount]; !ok { instanceCount = utils.GetModuleCountFromPod(&pod) pod.Labels[label.ModuleInstanceCount] = strconv.Itoa(instanceCount) - if err = r.Client.Update(context.TODO(), &pod); err != nil { + if err = utils.UpdateResource(r.Client, context.TODO(), &pod); err != nil { log.Log.Error(err, fmt.Sprintf("failed to update pod label")) continue } @@ -498,7 +499,7 @@ func (r *ModuleReplicaSetReconciler) doAllocatePod(ctx context.Context, toAlloca var podIps []string for _, pod := range toAllocatePod { UpdatePodLabelBeforeInstallModule(pod, moduleReplicaSet.Spec.Template.Spec.Module.Name) - err := r.Client.Update(ctx, &pod) + err := utils.UpdateResource(r.Client, ctx, &pod) // add pod finalizer if err != nil { return podIps, err diff --git a/module-controller/internal/controller/modulereplicaset_controller_suit_test.go b/module-controller/internal/controller/modulereplicaset_controller_suit_test.go deleted file mode 100644 index e4eb01e9a..000000000 --- a/module-controller/internal/controller/modulereplicaset_controller_suit_test.go +++ /dev/null @@ -1,266 +0,0 @@ -package controller - -import ( - "context" - "k8s.io/apimachinery/pkg/labels" - "time" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/sofastack/sofa-serverless/api/v1alpha1" - "github.com/sofastack/sofa-serverless/internal/constants/finalizer" - "github.com/sofastack/sofa-serverless/internal/constants/label" - "github.com/sofastack/sofa-serverless/internal/utils" -) - -var _ = Describe("ModuleReplicaSet Controller", func() { - const timeout = time.Second * 30 - const interval = time.Second * 3 - - moduleReplicaSetName := "test-module-replicaset" - namespaceName := "module-replicaset-controller-namespace" - podName := "test-pod-for-replicaset" - podName2 := "test-pod-for-replicaset-2" - podName3 := "test-pod-for-replicaset-3" - - Context("create module replicaset", func() { - It("create success and replica is 1", func() { - namespace := prepareNamespace(namespaceName) - Expect(k8sClient.Create(context.TODO(), &namespace)).Should(Succeed()) - pod := preparePod(namespaceName, podName) - Expect(k8sClient.Create(context.TODO(), &pod)).Should(Succeed()) - moduleReplicaSet := prepareModuleReplicaSet(namespaceName, moduleReplicaSetName) - utils.AddFinalizer(&moduleReplicaSet.ObjectMeta, finalizer.ModuleExistedFinalizer) - Expect(k8sClient.Create(context.TODO(), &moduleReplicaSet)).Should(Succeed()) - Eventually(func() bool { - key := types.NamespacedName{ - Name: moduleReplicaSetName, - Namespace: namespaceName, - } - var newModuleReplicaSet v1alpha1.ModuleReplicaSet - k8sClient.Get(context.TODO(), key, &newModuleReplicaSet) - if newModuleReplicaSet.Spec.Replicas == 1 { - selector, err := metav1.LabelSelectorAsSelector(&newModuleReplicaSet.Spec.Selector) - modules := &v1alpha1.ModuleList{} - err = k8sClient.List(context.TODO(), modules, &client.ListOptions{Namespace: newModuleReplicaSet.Namespace, LabelSelector: selector}) - if err == nil && len(modules.Items) == 1 { - return true - } - } - return false - }, timeout, interval).Should(BeTrue()) - }) - }) - - Context("update module version in replicaset", func() { - It("module version is updated", func() { - key := types.NamespacedName{ - Name: moduleReplicaSetName, - Namespace: namespaceName, - } - pod2 := preparePod(namespaceName, podName2) - Expect(k8sClient.Create(context.TODO(), &pod2)).Should(Succeed()) - var newModuleReplicaSet v1alpha1.ModuleReplicaSet - k8sClient.Get(context.TODO(), key, &newModuleReplicaSet) - newModuleReplicaSet.Spec.Template.Spec.Module.Version = "1.0.1" - Eventually(func() bool { - return k8sClient.Update(context.TODO(), &newModuleReplicaSet) == nil - }, timeout, interval).Should(BeTrue()) - Eventually(func() bool { - selector, err := metav1.LabelSelectorAsSelector(&newModuleReplicaSet.Spec.Selector) - modules := &v1alpha1.ModuleList{} - err = k8sClient.List(context.TODO(), modules, &client.ListOptions{Namespace: newModuleReplicaSet.Namespace, LabelSelector: selector}) - if err == nil && len(modules.Items) == 1 { - return modules.Items[0].Spec.Module.Version == "1.0.1" - } - return false - }, timeout, interval).Should(BeTrue()) - }) - }) - - Context("scale down replicaset", func() { - It("replica is 0 and delete all module", func() { - key := types.NamespacedName{ - Name: moduleReplicaSetName, - Namespace: namespaceName, - } - var newModuleReplicaSet v1alpha1.ModuleReplicaSet - k8sClient.Get(context.TODO(), key, &newModuleReplicaSet) - newModuleReplicaSet.Spec.Replicas = 0 - Expect(k8sClient.Update(context.TODO(), &newModuleReplicaSet)).Should(Succeed()) - Eventually(func() bool { - selector, err := metav1.LabelSelectorAsSelector(&newModuleReplicaSet.Spec.Selector) - modules := &v1alpha1.ModuleList{} - err = k8sClient.List(context.TODO(), modules, &client.ListOptions{Namespace: newModuleReplicaSet.Namespace, LabelSelector: selector}) - if err == nil && len(modules.Items) == 0 { - return true - } - return false - }, timeout, interval).Should(BeTrue()) - }) - }) - - Context("scale up replicaset", func() { - It("replica is 1 and create one module", func() { - key := types.NamespacedName{ - Name: moduleReplicaSetName, - Namespace: namespaceName, - } - - oldModuleReplicaSetName := "old" + moduleReplicaSetName - oldModuleReplicaSet := prepareModuleReplicaSet(namespaceName, oldModuleReplicaSetName) - utils.AddFinalizer(&oldModuleReplicaSet.ObjectMeta, finalizer.ModuleExistedFinalizer) - Expect(k8sClient.Create(context.TODO(), &oldModuleReplicaSet)).Should(Succeed()) - - module1 := PrepareModule(namespaceName, "fake-module-1") - module1.Labels[label.ModuleReplicasetLabel] = oldModuleReplicaSetName - Expect(k8sClient.Create(context.TODO(), &module1)).Should(Succeed()) - - var newModuleReplicaSet v1alpha1.ModuleReplicaSet - k8sClient.Get(context.TODO(), key, &newModuleReplicaSet) - newModuleReplicaSet.Spec.Replicas = 1 - Expect(k8sClient.Update(context.TODO(), &newModuleReplicaSet)).Should(Succeed()) - Eventually(func() bool { - k8sClient.Get(context.TODO(), types.NamespacedName{Name: oldModuleReplicaSetName, Namespace: namespaceName}, &oldModuleReplicaSet) - modules := &v1alpha1.ModuleList{} - err := k8sClient.List(context.TODO(), modules, &client.ListOptions{Namespace: newModuleReplicaSet.Namespace, LabelSelector: labels.SelectorFromSet(map[string]string{ - label.ModuleReplicasetLabel: moduleReplicaSetName, - })}) - if err == nil && len(modules.Items) == 1 && oldModuleReplicaSet.Spec.Replicas == 0 { - k8sClient.Delete(context.TODO(), &oldModuleReplicaSet) - k8sClient.Delete(context.TODO(), &module1) - return true - } - return false - }, timeout, interval).Should(BeTrue()) - }) - }) - - Context("delete replicaset", func() { - It("delete one module and clean replicaset", func() { - key := types.NamespacedName{ - Name: moduleReplicaSetName, - Namespace: namespaceName, - } - var newModuleReplicaSet v1alpha1.ModuleReplicaSet - k8sClient.Get(context.TODO(), key, &newModuleReplicaSet) - Expect(k8sClient.Delete(context.TODO(), &newModuleReplicaSet)).Should(Succeed()) - Eventually(func() bool { - selector, err := metav1.LabelSelectorAsSelector(&newModuleReplicaSet.Spec.Selector) - modules := &v1alpha1.ModuleList{} - err = k8sClient.List(context.TODO(), modules, &client.ListOptions{Namespace: newModuleReplicaSet.Namespace, LabelSelector: selector}) - if err == nil && len(modules.Items) == 0 { - err = k8sClient.Get(context.TODO(), key, &newModuleReplicaSet) - if err != nil && errors.IsNotFound(err) { - return true - } - } - return false - }, timeout, interval).Should(BeTrue()) - }) - }) - - Context("create module replicaset with scaleup_then_scaledown upgradePolicy", func() { - It("create module", func() { - - pod3 := preparePod(namespaceName, podName3) - Expect(k8sClient.Create(context.TODO(), &pod3)).Should(Succeed()) - pod3.Status.PodIP = "127.0.0.2" - k8sClient.Status().Update(context.TODO(), &pod3) - moduleReplicaSet := prepareModuleReplicaSet(namespaceName, moduleReplicaSetName) - moduleReplicaSet.Spec.OperationStrategy.UpgradePolicy = v1alpha1.ScaleUpThenScaleDownUpgradePolicy - utils.AddFinalizer(&moduleReplicaSet.ObjectMeta, finalizer.ModuleExistedFinalizer) - Expect(k8sClient.Create(context.TODO(), &moduleReplicaSet)).Should(Succeed()) - key := types.NamespacedName{ - Name: moduleReplicaSetName, - Namespace: namespaceName, - } - Eventually(func() bool { - var newModuleReplicaSet v1alpha1.ModuleReplicaSet - k8sClient.Get(context.TODO(), key, &newModuleReplicaSet) - if newModuleReplicaSet.Spec.Replicas == 1 { - selector, err := metav1.LabelSelectorAsSelector(&newModuleReplicaSet.Spec.Selector) - modules := &v1alpha1.ModuleList{} - err = k8sClient.List(context.TODO(), modules, &client.ListOptions{Namespace: newModuleReplicaSet.Namespace, LabelSelector: selector}) - if err == nil && len(modules.Items) == 1 && v1alpha1.ModuleInstanceStatusAvailable == modules.Items[0].Status.Status { - return true - } - } - return false - }, timeout, interval).Should(BeTrue()) - }) - }) - - //Context("update module version with scaleup_then_scaledown upgradePolicy", func() { - // It("pod and module will scale up then scale down", func() { - // podName3 := "test-pod-for-replicaset-3" - // pod3 := preparePod(namespaceName, podName3) - // Expect(k8sClient.Create(context.TODO(), &pod3)).Should(Succeed()) - // pod3.Status.PodIP = "127.0.0.3" - // k8sClient.Status().Update(context.TODO(), &pod3) - // key := types.NamespacedName{ - // Name: moduleReplicaSetName, - // Namespace: namespaceName, - // } - // var moduleReplicaSet v1alpha1.ModuleReplicaSet - // k8sClient.Get(context.TODO(), key, &moduleReplicaSet) - // moduleReplicaSet.Spec.Template.Spec.Module.Version = "1.0.1" - // k8sClient.Update(context.TODO(), &moduleReplicaSet) - // - // selector, _ := metav1.LabelSelectorAsSelector(&moduleReplicaSet.Spec.Selector) - // modules := &v1alpha1.ModuleList{} - // k8sClient.List(context.TODO(), modules, &client.ListOptions{Namespace: moduleReplicaSet.Namespace, LabelSelector: selector}) - // var moduleNames []string - // for _, moduleName := range modules.Items { - // moduleNames = append(moduleNames, moduleName.Name) - // } - // - // Eventually(func() bool { - // for _, moduleName := range moduleNames { - // module := &v1alpha1.Module{} - // err := k8sClient.Get(context.TODO(), types.NamespacedName{Name: moduleName, Namespace: namespaceName}, module) - // if !errors.IsNotFound(err) { - // return false - // } - // } - // k8sClient.List(context.TODO(), modules, &client.ListOptions{Namespace: moduleReplicaSet.Namespace, LabelSelector: selector}) - // return len(modules.Items) == len(moduleNames) - // }, timeout, interval).Should(BeTrue()) - // }) - //}) - -}) - -func prepareModuleReplicaSet(namespace, moduleReplicaSetName string) v1alpha1.ModuleReplicaSet { - - moduleReplicaSet := v1alpha1.ModuleReplicaSet{ - Spec: v1alpha1.ModuleReplicaSetSpec{ - Replicas: 1, - Template: v1alpha1.ModuleTemplateSpec{ - Spec: v1alpha1.ModuleSpec{ - Module: v1alpha1.ModuleInfo{ - Name: "dynamic-provider", - Version: "1.0.0", - Url: "http://serverless-opensource.oss-cn-shanghai.aliyuncs.com/module-packages/stable/dynamic-provider-1.0.0-ark-biz.jar", - }, - }, - }, - }, - ObjectMeta: metav1.ObjectMeta{ - Name: moduleReplicaSetName, - Namespace: namespace, - Labels: map[string]string{ - "app": "dynamic-stock", - label.MaxModuleCount: "10", - label.ModuleSchedulingStrategy: string(v1alpha1.Scatter), - }, - Annotations: map[string]string{}, - }, - } - return moduleReplicaSet -} diff --git a/module-controller/internal/controller/pod_controller.go b/module-controller/internal/controller/pod_controller.go index 9b4b634a0..a1e100587 100644 --- a/module-controller/internal/controller/pod_controller.go +++ b/module-controller/internal/controller/pod_controller.go @@ -77,7 +77,7 @@ func (r *PodReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R if pod.Labels[label.DeletePodLabel] == "true" { if module.Labels[label.DeleteModuleLabel] != "true" { module.Labels[label.DeleteModuleLabel] = "true" - err = r.Client.Update(ctx, &module) + err = utils.UpdateResource(r.Client, ctx, &module) if err != nil { log.Log.Error(err, "delete module failed when update delete module label", "moduleName", module.Name, "podName", pod.Name) return ctrl.Result{}, err @@ -86,7 +86,7 @@ func (r *PodReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R } else if pod.Labels[label.DeletePodDirectlyLabel] == "true" { if module.Labels[label.DeleteModuleDirectlyLabel] != "true" { module.Labels[label.DeleteModuleDirectlyLabel] = "true" - err = r.Client.Update(ctx, &module) + err = utils.UpdateResource(r.Client, ctx, &module) if err != nil { log.Log.Error(err, "delete module failed when update delete module label", "moduleName", module.Name, "podName", pod.Name) return ctrl.Result{}, err diff --git a/module-controller/internal/controller/pod_controller_suit_test.go b/module-controller/internal/controller/pod_controller_suit_test.go index c702353ed..6aa41a115 100644 --- a/module-controller/internal/controller/pod_controller_suit_test.go +++ b/module-controller/internal/controller/pod_controller_suit_test.go @@ -17,6 +17,7 @@ import ( ) var _ = Describe("Pod Controller", func() { + const timeout = time.Second * 30 const interval = time.Second * 3 namespaceName := "pod-controller-namespace" diff --git a/module-controller/internal/utils/controller_utils.go b/module-controller/internal/utils/controller_utils.go index 266982f10..06b479fba 100644 --- a/module-controller/internal/utils/controller_utils.go +++ b/module-controller/internal/utils/controller_utils.go @@ -3,6 +3,8 @@ package utils import ( "fmt" "github.com/sofastack/sofa-serverless/api/v1alpha1" + "golang.org/x/net/context" + "sigs.k8s.io/controller-runtime/pkg/client" "strconv" "strings" "time" @@ -116,3 +118,25 @@ func AppendModuleDeploymentCondition(conditions []v1alpha1.ModuleDeploymentCondi return append(conditions[1:], condition) } } + +func UpdateResource(client client.Client, ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + resourceName := obj.GetName() + log.Log.Info("start update resource", "resourceName", resourceName) + err := client.Update(ctx, obj, opts...) + if err != nil { + log.Log.Error(err, "update resource failed", "resourceName", resourceName) + return err + } + return nil +} + +func UpdateStatus(client client.Client, ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { + resourceName := obj.GetName() + log.Log.Info("start update status", "resourceName", resourceName) + err := client.Status().Update(ctx, obj, opts...) + if err != nil { + log.Log.Error(err, "update status failed", "resourceName", resourceName) + return err + } + return nil +} diff --git a/module-controller/internal/utils/controller_utils_test.go b/module-controller/internal/utils/controller_utils_test.go index 9a4a0e0c1..815ccf655 100644 --- a/module-controller/internal/utils/controller_utils_test.go +++ b/module-controller/internal/utils/controller_utils_test.go @@ -2,6 +2,8 @@ package utils import ( "fmt" + "github.com/sofastack/sofa-serverless/api/v1alpha1" + "golang.org/x/net/context" "k8s.io/apimachinery/pkg/api/errors" "strconv" "testing" @@ -111,3 +113,42 @@ func TestGetModuleInstanceCount(t *testing.T) { pod.Labels[label.ModuleInstanceCount] = "1" assert.Equal(t, 1, GetModuleInstanceCount(*pod)) } + +func TestAppendModuleDeploymentCondition(t *testing.T) { + moduleDeployment := PrepareModuleDeployment("namespace", "testModuleDeploymentName") + + i := 0 + for i < 11 { + condition := v1alpha1.ModuleDeploymentCondition{ + Type: v1alpha1.DeploymentProgressing, + Status: corev1.ConditionTrue, + Message: "message" + strconv.Itoa(i), + } + moduleDeployment.Status.Conditions = AppendModuleDeploymentCondition(moduleDeployment.Status.Conditions, condition) + i++ + } + assert.Equal(t, 10, len(moduleDeployment.Status.Conditions)) +} + +func TestUpdateResource(t *testing.T) { + moduleDeployment := PrepareModuleDeployment("namespace", "testModuleDeploymentName") + client := MockClient{} + err := UpdateResource(client, context.TODO(), &moduleDeployment) + assert.True(t, err == nil) +} + +func TestUpdateStatus(t *testing.T) { + moduleDeployment := PrepareModuleDeployment("namespace", "testModuleDeploymentName") + client := MockClient{} + err := UpdateStatus(client, context.TODO(), &moduleDeployment) + assert.True(t, err == nil) +} + +// +//type TestClient struct { +// MockClient +//} +// +//func (m MockClient) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { +// return nil +//} diff --git a/module-controller/internal/utils/test_utils.go b/module-controller/internal/utils/test_utils.go index affc15c17..02d0c398c 100644 --- a/module-controller/internal/utils/test_utils.go +++ b/module-controller/internal/utils/test_utils.go @@ -89,9 +89,24 @@ func (m MockClient) DeleteAllOf(ctx context.Context, obj client.Object, opts ... } func (m MockClient) Status() client.SubResourceWriter { - return nil + return MockSubResourceWriter{} } func (m MockClient) SubResource(subResource string) client.SubResourceClient { return nil } + +type MockSubResourceWriter struct { +} + +func (m MockSubResourceWriter) Create(ctx context.Context, obj client.Object, subResource client.Object, opts ...client.SubResourceCreateOption) error { + return nil +} + +func (m MockSubResourceWriter) Update(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { + return nil +} + +func (m MockSubResourceWriter) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.SubResourcePatchOption) error { + return nil +} diff --git a/module-controller/internal/utils/test_utils_test.go b/module-controller/internal/utils/test_utils_test.go index b3741fc28..7e85a10ca 100644 --- a/module-controller/internal/utils/test_utils_test.go +++ b/module-controller/internal/utils/test_utils_test.go @@ -27,6 +27,6 @@ func TestMockClient(t *testing.T) { assert.Equal(t, nil, mockClient.Update(nil, nil)) assert.Equal(t, nil, mockClient.Patch(nil, nil, nil)) assert.Equal(t, nil, mockClient.DeleteAllOf(nil, nil)) - assert.Equal(t, nil, mockClient.Status()) + assert.Equal(t, MockSubResourceWriter{}, mockClient.Status()) assert.Equal(t, nil, mockClient.SubResource("")) }