From bbcb429d5ae16888843455d886ad76cd1c8fbc7b Mon Sep 17 00:00:00 2001 From: shaofan-hs Date: Mon, 25 Mar 2024 17:53:49 +0800 Subject: [PATCH] fix, podopslifecycle webhook waits for more events --- pkg/controllers/utils/pod_utils.go | 15 ++ pkg/controllers/utils/pod_utils_test.go | 25 +++ .../generic/pod/opslifecycle/mutating.go | 37 ++-- .../generic/pod/opslifecycle/validating.go | 6 +- .../generic/pod/opslifecycle/webhook.go | 68 ++----- .../generic/pod/opslifecycle/webhook_test.go | 173 +++++++++++++----- 6 files changed, 206 insertions(+), 118 deletions(-) diff --git a/pkg/controllers/utils/pod_utils.go b/pkg/controllers/utils/pod_utils.go index be50673c..b850762a 100644 --- a/pkg/controllers/utils/pod_utils.go +++ b/pkg/controllers/utils/pod_utils.go @@ -18,6 +18,7 @@ package utils import ( "encoding/json" + "strings" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" @@ -106,6 +107,20 @@ func IsPodServiceAvailable(pod *corev1.Pod) bool { return exist } +func GetProtectionFinalizers(pod *corev1.Pod) []string { + if pod == nil || pod.ObjectMeta.Finalizers == nil || len(pod.ObjectMeta.Finalizers) == 0 { + return nil + } + + var finalizers []string + for _, f := range pod.ObjectMeta.Finalizers { + if strings.HasPrefix(f, v1alpha1.PodOperationProtectionFinalizerPrefix) { + finalizers = append(finalizers, f) + } + } + return finalizers +} + func IsExpectedFinalizerSatisfied(pod *corev1.Pod) (bool, map[string]string, error) { satisfied := true notSatisfiedFinalizers := make(map[string]string) // expected finalizers that are not satisfied diff --git a/pkg/controllers/utils/pod_utils_test.go b/pkg/controllers/utils/pod_utils_test.go index 7cf36e9f..d10cef56 100644 --- a/pkg/controllers/utils/pod_utils_test.go +++ b/pkg/controllers/utils/pod_utils_test.go @@ -18,6 +18,7 @@ package utils import ( "encoding/json" + "fmt" "testing" corev1 "k8s.io/api/core/v1" @@ -235,6 +236,30 @@ func TestIsPodServiceAvailable(t *testing.T) { } } +func TestGetProtectionFinalizers(t *testing.T) { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: corev1.PodSpec{}, + } + + if finalizers := GetProtectionFinalizers(pod); finalizers != nil { + t.Fatalf("expected failure") + } + + protectionFinalizer := fmt.Sprintf("%s/%s", appsv1alpha1.PodOperationProtectionFinalizerPrefix, "finalizer1") + pod.ObjectMeta.Finalizers = []string{ + protectionFinalizer, + "finalizer2", + } + + finalizers := GetProtectionFinalizers(pod) + if len(finalizers) != 1 || finalizers[0] != protectionFinalizer { + t.Fatalf("expected failure") + } +} + func TestIsPodExpectedFinalizerSatisfied(t *testing.T) { pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/webhook/server/generic/pod/opslifecycle/mutating.go b/pkg/webhook/server/generic/pod/opslifecycle/mutating.go index 619957b3..1e1e035e 100644 --- a/pkg/webhook/server/generic/pod/opslifecycle/mutating.go +++ b/pkg/webhook/server/generic/pod/opslifecycle/mutating.go @@ -36,7 +36,7 @@ func (lc *OpsLifecycle) Mutating(ctx context.Context, c client.Client, oldPod, n return nil } - // add readiness gate when pod is created + // Add readiness gate when pod is created if operation == admissionv1.Create { addReadinessGates(newPod, v1alpha1.ReadinessGatePodServiceReady) } @@ -50,14 +50,14 @@ func (lc *OpsLifecycle) Mutating(ctx context.Context, c client.Client, oldPod, n var operatingCount, operateCount, operatedCount, completeCount int var undoTypeToNumsMap = map[string]int{} for id, labels := range newIDToLabelsMap { - if undoOperationType, ok := labels[v1alpha1.PodUndoOperationTypeLabelPrefix]; ok { // operation is canceled + if undoOperationType, ok := labels[v1alpha1.PodUndoOperationTypeLabelPrefix]; ok { // Operation is canceled if _, ok := undoTypeToNumsMap[undoOperationType]; !ok { undoTypeToNumsMap[undoOperationType] = 1 } else { undoTypeToNumsMap[undoOperationType] = undoTypeToNumsMap[undoOperationType] + 1 } - // clean up these labels with id + // Clean up these labels with the ID for _, v := range []string{v1alpha1.PodOperatingLabelPrefix, v1alpha1.PodOperationTypeLabelPrefix, v1alpha1.PodPreCheckLabelPrefix, @@ -76,19 +76,18 @@ func (lc *OpsLifecycle) Mutating(ctx context.Context, c client.Client, oldPod, n operatingCount++ if _, ok := labels[v1alpha1.PodPreCheckedLabelPrefix]; ok { // pre-checked - _, hasPrepare := labels[v1alpha1.PodPreparingLabelPrefix] - _, hasOperate := labels[v1alpha1.PodOperateLabelPrefix] - - if !hasPrepare && !hasOperate { + _, hasPreparing := labels[v1alpha1.PodPreparingLabelPrefix] + if !hasPreparing { delete(newPod.Labels, v1alpha1.PodServiceAvailableLabel) - lc.addLabelWithTime(newPod, fmt.Sprintf("%s/%s", v1alpha1.PodPreparingLabelPrefix, id)) // prepare - } else if !hasOperate { - if ready, _ := lc.readyToUpgrade(newPod); ready { - delete(newPod.Labels, fmt.Sprintf("%s/%s", v1alpha1.PodPreparingLabelPrefix, id)) + lc.addLabelWithTime(newPod, fmt.Sprintf("%s/%s", v1alpha1.PodPreparingLabelPrefix, id)) // preparing + } + + _, hasOperate := labels[v1alpha1.PodOperateLabelPrefix] + if !hasOperate && lc.readyToOperate(newPod) { + delete(newPod.Labels, fmt.Sprintf("%s/%s", v1alpha1.PodPreparingLabelPrefix, id)) - lc.addLabelWithTime(newPod, fmt.Sprintf("%s/%s", v1alpha1.PodOperateLabelPrefix, id)) // operate - } + lc.addLabelWithTime(newPod, fmt.Sprintf("%s/%s", v1alpha1.PodOperateLabelPrefix, id)) // operate } } else { if _, ok := labels[v1alpha1.PodPreCheckLabelPrefix]; !ok { @@ -116,17 +115,17 @@ func (lc *OpsLifecycle) Mutating(ctx context.Context, c client.Client, oldPod, n klog.Infof("pod: %s/%s, numOfIDs: %d, operatingCount: %d, operateCount: %d, operatedCount: %d, completeCount: %d", newPod.Namespace, newPod.Name, numOfIDs, operatingCount, operateCount, operatedCount, completeCount) for t, num := range undoTypeToNumsMap { - if num == typeToNumsMap[t] { // reset the permission with type t if all operating with type t are canceled + if num == typeToNumsMap[t] { // Reset the permission with type t if all operating with type t are canceled delete(newPod.Labels, fmt.Sprintf("%s/%s", v1alpha1.PodOperationPermissionLabelPrefix, t)) } } - if operatingCount != 0 { // when operation is done, controller will remove operating label and operation type label + if operatingCount != 0 { // When operation is done, controller will remove operating label and operation type label return nil } - if completeCount == numOfIDs { // all operations are completed - satisfied, notSatisfiedFinalizers, err := controllerutils.IsExpectedFinalizerSatisfied(newPod) // whether all expected finalizers are satisfied + if completeCount == numOfIDs { // All operations are completed + satisfied, notSatisfiedFinalizers, err := controllerutils.IsExpectedFinalizerSatisfied(newPod) // Whether all expected finalizers are satisfied if err != nil || !satisfied { klog.Infof("pod: %s/%s, satisfied: %v, expectedFinalizer: %v, err: %v", newPod.Namespace, newPod.Name, satisfied, notSatisfiedFinalizers, err) return err @@ -146,7 +145,7 @@ func (lc *OpsLifecycle) Mutating(ctx context.Context, c client.Client, oldPod, n return nil } - if operateCount == numOfIDs { // all operations are going to be done + if operateCount == numOfIDs { // All operations are going to be done oldIdToLabelsMap, _, err := podopslifecycle.PodIDAndTypesMap(oldPod) if err != nil { return err @@ -172,7 +171,7 @@ func (lc *OpsLifecycle) Mutating(ctx context.Context, c client.Client, oldPod, n } } - if operatedCount == numOfIDs { // all operations are done + if operatedCount == numOfIDs { // All operations are done for id, labels := range newIDToLabelsMap { if _, ok := labels[v1alpha1.PodPostCheckLabelPrefix]; !ok { lc.addLabelWithTime(newPod, fmt.Sprintf("%s/%s", v1alpha1.PodPostCheckLabelPrefix, id)) // post-check diff --git a/pkg/webhook/server/generic/pod/opslifecycle/validating.go b/pkg/webhook/server/generic/pod/opslifecycle/validating.go index c33c0373..f4827f15 100644 --- a/pkg/webhook/server/generic/pod/opslifecycle/validating.go +++ b/pkg/webhook/server/generic/pod/opslifecycle/validating.go @@ -41,7 +41,7 @@ func (lc *OpsLifecycle) Validating(ctx context.Context, c client.Client, oldPod, expectedLabels := make(map[string]struct{}) foundLabels := make(map[string]struct{}) for label := range newPod.Labels { - for _, v := range pairLabelPrefixesMap { // labels must exist together and have the same id + for _, v := range pairLabelPrefixesMap { // Labels must exist together and have the same ID if !strings.HasPrefix(label, v) { continue } @@ -62,7 +62,7 @@ func (lc *OpsLifecycle) Validating(ctx context.Context, c client.Client, oldPod, } found := false - for v := range expectedLabels { // try to find the expected another label prefixes + for v := range expectedLabels { // Try to find the expected another label prefixes if strings.HasPrefix(label, v) { foundLabels[v] = struct{}{} found = true @@ -73,7 +73,7 @@ func (lc *OpsLifecycle) Validating(ctx context.Context, c client.Client, oldPod, continue } - for _, v := range coexistingLabelPrefixesMap { // labels must exist together + for _, v := range coexistingLabelPrefixesMap { // Labels must exist together if !strings.HasPrefix(label, v) { continue } diff --git a/pkg/webhook/server/generic/pod/opslifecycle/webhook.go b/pkg/webhook/server/generic/pod/opslifecycle/webhook.go index b8e5cf9a..ea290a29 100644 --- a/pkg/webhook/server/generic/pod/opslifecycle/webhook.go +++ b/pkg/webhook/server/generic/pod/opslifecycle/webhook.go @@ -17,9 +17,7 @@ limitations under the License. package opslifecycle import ( - "encoding/json" "strconv" - "strings" "time" corev1 "k8s.io/api/core/v1" @@ -33,7 +31,7 @@ const ( ) var ( - // some labels must exist together and have the same id, and they are a pair + // Some labels must exist together and have the same ID pairLabelPrefixesMap = map[string]string{ v1alpha1.PodOperatingLabelPrefix: v1alpha1.PodOperationTypeLabelPrefix, v1alpha1.PodOperationTypeLabelPrefix: v1alpha1.PodOperatingLabelPrefix, @@ -42,27 +40,24 @@ var ( v1alpha1.PodDoneOperationTypeLabelPrefix: v1alpha1.PodOperatedLabelPrefix, } - // some labels must exist together + // Some labels must exist together coexistingLabelPrefixesMap = map[string]string{ v1alpha1.PodPreCheckedLabelPrefix: v1alpha1.PodOperationPermissionLabelPrefix, v1alpha1.PodOperationPermissionLabelPrefix: v1alpha1.PodPreCheckedLabelPrefix, } ) -type ReadyToUpgrade func(pod *corev1.Pod) (bool, []string) +type ReadyToOperate func(pod *corev1.Pod) bool type TimeLabelValue func() string -type IsPodReady func(pod *corev1.Pod) bool type OpsLifecycle struct { - readyToUpgrade ReadyToUpgrade // for testing - isPodReady IsPodReady + readyToOperate ReadyToOperate timeLabelValue TimeLabelValue } func New() *OpsLifecycle { return &OpsLifecycle{ - readyToUpgrade: hasNoBlockingFinalizer, - isPodReady: controllerutils.IsPodReady, + readyToOperate: readyToOperate, timeLabelValue: func() string { return strconv.FormatInt(time.Now().UnixNano(), 10) }, @@ -91,56 +86,23 @@ func addReadinessGates(pod *corev1.Pod, conditionType corev1.PodConditionType) { }) } -func hasNoBlockingFinalizer(pod *corev1.Pod) (bool, []string) { +func readyToOperate(pod *corev1.Pod) bool { if pod == nil { - return true, nil + return false } - hasReadinessGate := false - if pod.Spec.ReadinessGates != nil { - for _, readinessGate := range pod.Spec.ReadinessGates { - if readinessGate.ConditionType == v1alpha1.ReadinessGatePodServiceReady { - hasReadinessGate = true - break - } - } - } - if !hasReadinessGate { - // if has no service-ready ReadinessGate, treat it as normal pod. - return true, nil - } - - if pod.ObjectMeta.Finalizers == nil || len(pod.ObjectMeta.Finalizers) == 0 { - return true, nil - } - - var finalizers []string - for _, f := range pod.ObjectMeta.Finalizers { - if strings.HasPrefix(f, v1alpha1.PodOperationProtectionFinalizerPrefix) { - finalizers = append(finalizers, f) - } + index, condition := controllerutils.GetPodCondition(&pod.Status, v1alpha1.ReadinessGatePodServiceReady) + if index == -1 { + return true } + finalizers := controllerutils.GetProtectionFinalizers(pod) if len(finalizers) > 0 { - return false, finalizers - } - - return true, nil -} - -func podAvailableConditions(pod *corev1.Pod) (*v1alpha1.PodAvailableConditions, error) { - if pod.Annotations == nil { - return nil, nil - } - - anno, ok := pod.Annotations[v1alpha1.PodAvailableConditionsAnnotation] - if !ok { - return nil, nil + return false } - availableConditions := &v1alpha1.PodAvailableConditions{} - if err := json.Unmarshal([]byte(anno), availableConditions); err != nil { - return nil, err + if condition.Status == corev1.ConditionFalse { + return true } - return availableConditions, nil + return false } diff --git a/pkg/webhook/server/generic/pod/opslifecycle/webhook_test.go b/pkg/webhook/server/generic/pod/opslifecycle/webhook_test.go index 3af60485..193bc5aa 100644 --- a/pkg/webhook/server/generic/pod/opslifecycle/webhook_test.go +++ b/pkg/webhook/server/generic/pod/opslifecycle/webhook_test.go @@ -32,7 +32,7 @@ import ( func TestValidating(t *testing.T) { inputs := []struct { labels map[string]string - keyWords string // used to check the error message + keyWords string // Used to check the error message }{ { labels: map[string]string{ @@ -117,18 +117,17 @@ func TestValidating(t *testing.T) { func TestMutating(t *testing.T) { inputs := []struct { - notes string - keyWords string // used to check the error message + note string + keyWords string // Used to check the error message oldPodLabels map[string]string newPodLabels map[string]string expectedLabels map[string]string - readyToUpgrade ReadyToUpgrade - isPodReady IsPodReady + readyToOperate ReadyToOperate }{ { - notes: "pre-check", + note: "pre-check", newPodLabels: map[string]string{ fmt.Sprintf("%s/%s", v1alpha1.PodOperatingLabelPrefix, "123"): "1402144848", fmt.Sprintf("%s/%s", v1alpha1.PodOperationTypeLabelPrefix, "123"): "upgrade", @@ -141,7 +140,7 @@ func TestMutating(t *testing.T) { }, }, { - notes: "pre-check", + note: "pre-check", newPodLabels: map[string]string{ fmt.Sprintf("%s/%s", v1alpha1.PodOperatingLabelPrefix, "123"): "1402144848", fmt.Sprintf("%s/%s", v1alpha1.PodOperationTypeLabelPrefix, "123"): "upgrade", @@ -163,7 +162,7 @@ func TestMutating(t *testing.T) { }, { - notes: "prepare", + note: "preparing", newPodLabels: map[string]string{ fmt.Sprintf("%s/%s", v1alpha1.PodOperatingLabelPrefix, "123"): "1402144848", fmt.Sprintf("%s/%s", v1alpha1.PodOperationTypeLabelPrefix, "123"): "upgrade", @@ -181,11 +180,11 @@ func TestMutating(t *testing.T) { fmt.Sprintf("%s/%s", v1alpha1.PodPreparingLabelPrefix, "123"): "1402144848", }, - readyToUpgrade: readyToUpgradeReturnFalse, + readyToOperate: readyToUpgradeReturnFalse, }, { - notes: "prepare, undo", + note: "preparing, undo", newPodLabels: map[string]string{ fmt.Sprintf("%s/%s", v1alpha1.PodOperatingLabelPrefix, "123"): "1402144848", fmt.Sprintf("%s/%s", v1alpha1.PodOperationTypeLabelPrefix, "123"): "upgrade", @@ -199,7 +198,7 @@ func TestMutating(t *testing.T) { }, { - notes: "prepare, operate, undo", + note: "preparing, operate, undo", newPodLabels: map[string]string{ fmt.Sprintf("%s/%s", v1alpha1.PodOperatingLabelPrefix, "123"): "1402144848", fmt.Sprintf("%s/%s", v1alpha1.PodOperationTypeLabelPrefix, "123"): "upgrade", @@ -224,11 +223,11 @@ func TestMutating(t *testing.T) { fmt.Sprintf("%s/%s", v1alpha1.PodPreparingLabelPrefix, "456"): "1402144848", }, - readyToUpgrade: readyToUpgradeReturnFalse, + readyToOperate: readyToUpgradeReturnFalse, }, { - notes: "prepare, pre-check", + note: "preparing, pre-check", newPodLabels: map[string]string{ fmt.Sprintf("%s/%s", v1alpha1.PodOperatingLabelPrefix, "123"): "1402144848", fmt.Sprintf("%s/%s", v1alpha1.PodOperationTypeLabelPrefix, "123"): "upgrade", @@ -254,11 +253,11 @@ func TestMutating(t *testing.T) { fmt.Sprintf("%s/%s", v1alpha1.PodPreCheckLabelPrefix, "456"): "1402144848", }, - readyToUpgrade: readyToUpgradeReturnFalse, + readyToOperate: readyToUpgradeReturnFalse, }, { - notes: "prepare, operate", + note: "preparing, operate", newPodLabels: map[string]string{ fmt.Sprintf("%s/%s", v1alpha1.PodOperatingLabelPrefix, "123"): "1402144848", fmt.Sprintf("%s/%s", v1alpha1.PodOperationTypeLabelPrefix, "123"): "upgrade", @@ -275,10 +274,45 @@ func TestMutating(t *testing.T) { fmt.Sprintf("%s/%s", v1alpha1.PodPreparingLabelPrefix, "123"): "1402144848", }, + readyToOperate: readyToUpgradeReturnFalse, }, { - notes: "operated", + note: "preparing, operate", + newPodLabels: map[string]string{ + fmt.Sprintf("%s/%s", v1alpha1.PodOperatingLabelPrefix, "123"): "1402144848", + fmt.Sprintf("%s/%s", v1alpha1.PodOperationTypeLabelPrefix, "123"): "restart", + fmt.Sprintf("%s/%s", v1alpha1.PodPreCheckLabelPrefix, "123"): "1402144848", + fmt.Sprintf("%s/%s", v1alpha1.PodPreCheckedLabelPrefix, "123"): "1402144848", + fmt.Sprintf("%s/%s", v1alpha1.PodOperationPermissionLabelPrefix, "restart"): "1402144848", + + fmt.Sprintf("%s/%s", v1alpha1.PodOperatingLabelPrefix, "456"): "1402144848", + fmt.Sprintf("%s/%s", v1alpha1.PodOperationTypeLabelPrefix, "456"): "delete", + fmt.Sprintf("%s/%s", v1alpha1.PodPreCheckLabelPrefix, "456"): "1402144848", + fmt.Sprintf("%s/%s", v1alpha1.PodPreCheckedLabelPrefix, "456"): "1402144848", + fmt.Sprintf("%s/%s", v1alpha1.PodOperationPermissionLabelPrefix, "delete"): "1402144848", + }, + expectedLabels: map[string]string{ + fmt.Sprintf("%s/%s", v1alpha1.PodOperatingLabelPrefix, "123"): "1402144848", + fmt.Sprintf("%s/%s", v1alpha1.PodOperationTypeLabelPrefix, "123"): "restart", + fmt.Sprintf("%s/%s", v1alpha1.PodPreCheckLabelPrefix, "123"): "1402144848", + fmt.Sprintf("%s/%s", v1alpha1.PodPreCheckedLabelPrefix, "123"): "1402144848", + fmt.Sprintf("%s/%s", v1alpha1.PodOperationPermissionLabelPrefix, "restart"): "1402144848", + + fmt.Sprintf("%s/%s", v1alpha1.PodOperateLabelPrefix, "123"): "1402144848", + + fmt.Sprintf("%s/%s", v1alpha1.PodOperatingLabelPrefix, "456"): "1402144848", + fmt.Sprintf("%s/%s", v1alpha1.PodOperationTypeLabelPrefix, "456"): "delete", + fmt.Sprintf("%s/%s", v1alpha1.PodPreCheckLabelPrefix, "456"): "1402144848", + fmt.Sprintf("%s/%s", v1alpha1.PodPreCheckedLabelPrefix, "456"): "1402144848", + fmt.Sprintf("%s/%s", v1alpha1.PodOperationPermissionLabelPrefix, "delete"): "1402144848", + + fmt.Sprintf("%s/%s", v1alpha1.PodOperateLabelPrefix, "456"): "1402144848", + }, + }, + + { + note: "operated", oldPodLabels: map[string]string{ fmt.Sprintf("%s/%s", v1alpha1.PodOperatingLabelPrefix, "123"): "1402144848", fmt.Sprintf("%s/%s", v1alpha1.PodOperationTypeLabelPrefix, "123"): "upgrade", @@ -303,7 +337,7 @@ func TestMutating(t *testing.T) { }, { - notes: "post-check, but wait for operating", + note: "post-check, but wait for operating", oldPodLabels: map[string]string{ fmt.Sprintf("%s/%s", v1alpha1.PodOperatingLabelPrefix, "123"): "1402144848", fmt.Sprintf("%s/%s", v1alpha1.PodOperationTypeLabelPrefix, "123"): "upgrade", @@ -324,7 +358,7 @@ func TestMutating(t *testing.T) { }, { - notes: "post-check", + note: "post-check", newPodLabels: map[string]string{ fmt.Sprintf("%s/%s", v1alpha1.PodOperateLabelPrefix, "123"): "1402144848", fmt.Sprintf("%s/%s", v1alpha1.PodOperatedLabelPrefix, "123"): "1402144848", @@ -350,7 +384,7 @@ func TestMutating(t *testing.T) { }, { - notes: "complete", + note: "complete", newPodLabels: map[string]string{ fmt.Sprintf("%s/%s", v1alpha1.PodOperateLabelPrefix, "123"): "1402144848", fmt.Sprintf("%s/%s", v1alpha1.PodOperatedLabelPrefix, "123"): "1402144848", @@ -384,7 +418,7 @@ func TestMutating(t *testing.T) { }, { - notes: "wait for removing finalizers", + note: "wait for removing finalizers", newPodLabels: map[string]string{ fmt.Sprintf("%s/%s", v1alpha1.PodOperatedLabelPrefix, "123"): "1402144848", fmt.Sprintf("%s/%s", v1alpha1.PodDoneOperationTypeLabelPrefix, "123"): "upgrade", @@ -404,7 +438,7 @@ func TestMutating(t *testing.T) { }, { - notes: "all finished", + note: "all finished", newPodLabels: map[string]string{ fmt.Sprintf("%s/%s", v1alpha1.PodOperateLabelPrefix, "123"): "1402144848", fmt.Sprintf("%s/%s", v1alpha1.PodOperatedLabelPrefix, "123"): "1402144848", @@ -426,7 +460,6 @@ func TestMutating(t *testing.T) { }, } - opslifecycle := &OpsLifecycle{} for _, v := range inputs { if v.oldPodLabels != nil { v.oldPodLabels[v1alpha1.ControlledByKusionStackLabelKey] = "true" @@ -450,15 +483,9 @@ func TestMutating(t *testing.T) { }, } - opsLifecycleDefaultFunc(opslifecycle) - if v.readyToUpgrade != nil { - opslifecycle.readyToUpgrade = v.readyToUpgrade - } - if v.isPodReady != nil { - opslifecycle.isPodReady = v.isPodReady - } + opslifecycle := getOpsLifecycleWithFuncs(v.readyToOperate) - t.Logf("notes: %s", v.notes) + t.Logf("note: %s", v.note) err := opslifecycle.Mutating(context.Background(), nil, oldPod, newPod, admissionv1.Update) if v.keyWords == "" { assert.Nil(t, err) @@ -476,27 +503,87 @@ func TestMutating(t *testing.T) { } } -func opsLifecycleDefaultFunc(opslifecycle *OpsLifecycle) { - opslifecycle.timeLabelValue = func() string { - return "1402144848" +func TestReadyToUpgrade(t *testing.T) { + inputs := []struct { + note string + + podStatus corev1.PodStatus + finalizers []string + readyToOperate bool + }{ + { + note: "no finalizers, and no expected conditions", + podStatus: corev1.PodStatus{ + Conditions: []corev1.PodCondition{ + { + Type: "pod.kusionstack.io/test", + Status: corev1.ConditionTrue, + }, + }, + }, + readyToOperate: true, + }, + { + note: "no finalizers, and service-ready is true", + podStatus: corev1.PodStatus{ + Conditions: []corev1.PodCondition{ + { + Type: v1alpha1.ReadinessGatePodServiceReady, + Status: corev1.ConditionTrue, + }, + }, + }, + readyToOperate: false, + }, + { + note: "has protection finalizer, and service-ready is false", + podStatus: corev1.PodStatus{ + Conditions: []corev1.PodCondition{ + { + Type: v1alpha1.ReadinessGatePodServiceReady, + Status: corev1.ConditionFalse, + }, + }, + }, + finalizers: []string{fmt.Sprintf("%s/%s", v1alpha1.PodOperationProtectionFinalizerPrefix, "finalizer1")}, + readyToOperate: false, + }, } + for _, v := range inputs { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + Namespace: "default", + Finalizers: v.finalizers, + }, + Spec: corev1.PodSpec{}, + Status: v.podStatus, + } - opslifecycle.readyToUpgrade = readyToUpgradeReturnTrue - opslifecycle.isPodReady = isPodReadyReturnTrue + if assert.Equal(t, v.readyToOperate, readyToOperate(pod)) { + t.Logf("note: %s", v.note) + } + } } -func readyToUpgradeReturnTrue(pod *corev1.Pod) (bool, []string) { - return true, nil -} +func getOpsLifecycleWithFuncs(readyToOperate ReadyToOperate) *OpsLifecycle { + opslifecycle := &OpsLifecycle{ + timeLabelValue: func() string { + return "1402144848" + }, + readyToOperate: readyToUpgradeReturnTrue, + } -func readyToUpgradeReturnFalse(pod *corev1.Pod) (bool, []string) { - return false, nil + if readyToOperate != nil { + opslifecycle.readyToOperate = readyToOperate + } + return opslifecycle } -func isPodReadyReturnTrue(pod *corev1.Pod) bool { +func readyToUpgradeReturnTrue(pod *corev1.Pod) bool { return true } -func isPodReadyReturnFalse(pod *corev1.Pod) bool { - return true +func readyToUpgradeReturnFalse(pod *corev1.Pod) bool { + return false }