Skip to content

Commit 7f9845d

Browse files
shaofan-hsshaofan-hs
committed
fix, podopslifecycle webhook waits for more events
1 parent 72fb78d commit 7f9845d

File tree

7 files changed

+232
-123
lines changed

7 files changed

+232
-123
lines changed

‎pkg/controllers/podopslifecycle/podopslifecycle_controller_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ var _ = Describe("Stage processing", func() {
402402
Expect(err).NotTo(HaveOccurred())
403403
Expect(len(labels)).To(Equal(2))
404404

405-
for k, _ := range idToLabelsMap {
405+
for k := range idToLabelsMap {
406406
key := fmt.Sprintf("%s/%s", v1alpha1.PodPostCheckedLabelPrefix, k)
407407
preChecked, ok := labels[key]
408408
Expect(ok).To(Equal(true))

‎pkg/controllers/utils/pod_utils.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package utils
1818

1919
import (
2020
"encoding/json"
21+
"strings"
2122

2223
corev1 "k8s.io/api/core/v1"
2324
"k8s.io/apimachinery/pkg/util/sets"
@@ -106,6 +107,20 @@ func IsPodServiceAvailable(pod *corev1.Pod) bool {
106107
return exist
107108
}
108109

110+
func GetProtectionFinalizers(pod *corev1.Pod) []string {
111+
if pod == nil || pod.ObjectMeta.Finalizers == nil || len(pod.ObjectMeta.Finalizers) == 0 {
112+
return nil
113+
}
114+
115+
var finalizers []string
116+
for _, f := range pod.ObjectMeta.Finalizers {
117+
if strings.HasPrefix(f, v1alpha1.PodOperationProtectionFinalizerPrefix) {
118+
finalizers = append(finalizers, f)
119+
}
120+
}
121+
return finalizers
122+
}
123+
109124
func IsExpectedFinalizerSatisfied(pod *corev1.Pod) (bool, map[string]string, error) {
110125
satisfied := true
111126
notSatisfiedFinalizers := make(map[string]string) // expected finalizers that are not satisfied

‎pkg/controllers/utils/pod_utils_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package utils
1818

1919
import (
2020
"encoding/json"
21+
"fmt"
2122
"testing"
2223

2324
corev1 "k8s.io/api/core/v1"
@@ -235,6 +236,30 @@ func TestIsPodServiceAvailable(t *testing.T) {
235236
}
236237
}
237238

239+
func TestGetProtectionFinalizers(t *testing.T) {
240+
pod := &corev1.Pod{
241+
ObjectMeta: metav1.ObjectMeta{
242+
Name: "test",
243+
},
244+
Spec: corev1.PodSpec{},
245+
}
246+
247+
if finalizers := GetProtectionFinalizers(pod); finalizers != nil {
248+
t.Fatalf("expected failure")
249+
}
250+
251+
protectionFinalizer := fmt.Sprintf("%s/%s", appsv1alpha1.PodOperationProtectionFinalizerPrefix, "finalizer1")
252+
pod.ObjectMeta.Finalizers = []string{
253+
protectionFinalizer,
254+
"finalizer2",
255+
}
256+
257+
finalizers := GetProtectionFinalizers(pod)
258+
if len(finalizers) != 1 || finalizers[0] != protectionFinalizer {
259+
t.Fatalf("expected failure")
260+
}
261+
}
262+
238263
func TestIsPodExpectedFinalizerSatisfied(t *testing.T) {
239264
pod := &corev1.Pod{
240265
ObjectMeta: metav1.ObjectMeta{

‎pkg/webhook/server/generic/pod/opslifecycle/mutating.go

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ func (lc *OpsLifecycle) Mutating(ctx context.Context, c client.Client, oldPod, n
3636
return nil
3737
}
3838

39-
// add readiness gate when pod is created
39+
// Add readiness gate when pod is created
4040
if operation == admissionv1.Create {
4141
addReadinessGates(newPod, v1alpha1.ReadinessGatePodServiceReady)
4242
}
@@ -50,14 +50,14 @@ func (lc *OpsLifecycle) Mutating(ctx context.Context, c client.Client, oldPod, n
5050
var operatingCount, operateCount, operatedCount, completeCount int
5151
var undoTypeToNumsMap = map[string]int{}
5252
for id, labels := range newIDToLabelsMap {
53-
if undoOperationType, ok := labels[v1alpha1.PodUndoOperationTypeLabelPrefix]; ok { // operation is canceled
53+
if undoOperationType, ok := labels[v1alpha1.PodUndoOperationTypeLabelPrefix]; ok { // Operation is canceled
5454
if _, ok := undoTypeToNumsMap[undoOperationType]; !ok {
5555
undoTypeToNumsMap[undoOperationType] = 1
5656
} else {
5757
undoTypeToNumsMap[undoOperationType] = undoTypeToNumsMap[undoOperationType] + 1
5858
}
5959

60-
// clean up these labels with id
60+
// Clean up these labels with the ID
6161
for _, v := range []string{v1alpha1.PodOperatingLabelPrefix,
6262
v1alpha1.PodOperationTypeLabelPrefix,
6363
v1alpha1.PodPreCheckLabelPrefix,
@@ -76,19 +76,18 @@ func (lc *OpsLifecycle) Mutating(ctx context.Context, c client.Client, oldPod, n
7676
operatingCount++
7777

7878
if _, ok := labels[v1alpha1.PodPreCheckedLabelPrefix]; ok { // pre-checked
79-
_, hasPrepare := labels[v1alpha1.PodPreparingLabelPrefix]
80-
_, hasOperate := labels[v1alpha1.PodOperateLabelPrefix]
81-
82-
if !hasPrepare && !hasOperate {
79+
_, hasPreparing := labels[v1alpha1.PodPreparingLabelPrefix]
80+
if !hasPreparing {
8381
delete(newPod.Labels, v1alpha1.PodServiceAvailableLabel)
8482

85-
lc.addLabelWithTime(newPod, fmt.Sprintf("%s/%s", v1alpha1.PodPreparingLabelPrefix, id)) // prepare
86-
} else if !hasOperate {
87-
if ready, _ := lc.readyToUpgrade(newPod); ready {
88-
delete(newPod.Labels, fmt.Sprintf("%s/%s", v1alpha1.PodPreparingLabelPrefix, id))
83+
lc.addLabelWithTime(newPod, fmt.Sprintf("%s/%s", v1alpha1.PodPreparingLabelPrefix, id)) // preparing
84+
}
85+
86+
_, hasOperate := labels[v1alpha1.PodOperateLabelPrefix]
87+
if !hasOperate && lc.readyToOperate(newPod) {
88+
delete(newPod.Labels, fmt.Sprintf("%s/%s", v1alpha1.PodPreparingLabelPrefix, id))
8989

90-
lc.addLabelWithTime(newPod, fmt.Sprintf("%s/%s", v1alpha1.PodOperateLabelPrefix, id)) // operate
91-
}
90+
lc.addLabelWithTime(newPod, fmt.Sprintf("%s/%s", v1alpha1.PodOperateLabelPrefix, id)) // operate
9291
}
9392
} else {
9493
if _, ok := labels[v1alpha1.PodPreCheckLabelPrefix]; !ok {
@@ -116,17 +115,17 @@ func (lc *OpsLifecycle) Mutating(ctx context.Context, c client.Client, oldPod, n
116115
klog.Infof("pod: %s/%s, numOfIDs: %d, operatingCount: %d, operateCount: %d, operatedCount: %d, completeCount: %d", newPod.Namespace, newPod.Name, numOfIDs, operatingCount, operateCount, operatedCount, completeCount)
117116

118117
for t, num := range undoTypeToNumsMap {
119-
if num == typeToNumsMap[t] { // reset the permission with type t if all operating with type t are canceled
118+
if num == typeToNumsMap[t] { // Reset the permission with type t if all operating with type t are canceled
120119
delete(newPod.Labels, fmt.Sprintf("%s/%s", v1alpha1.PodOperationPermissionLabelPrefix, t))
121120
}
122121
}
123122

124-
if operatingCount != 0 { // when operation is done, controller will remove operating label and operation type label
123+
if operatingCount != 0 { // When operation is done, controller will remove operating label and operation type label
125124
return nil
126125
}
127126

128-
if completeCount == numOfIDs { // all operations are completed
129-
satisfied, notSatisfiedFinalizers, err := controllerutils.IsExpectedFinalizerSatisfied(newPod) // whether all expected finalizers are satisfied
127+
if completeCount == numOfIDs { // All operations are completed
128+
satisfied, notSatisfiedFinalizers, err := controllerutils.IsExpectedFinalizerSatisfied(newPod) // Whether all expected finalizers are satisfied
130129
if err != nil || !satisfied {
131130
klog.Infof("pod: %s/%s, satisfied: %v, expectedFinalizer: %v, err: %v", newPod.Namespace, newPod.Name, satisfied, notSatisfiedFinalizers, err)
132131
return err
@@ -146,7 +145,7 @@ func (lc *OpsLifecycle) Mutating(ctx context.Context, c client.Client, oldPod, n
146145
return nil
147146
}
148147

149-
if operateCount == numOfIDs { // all operations are going to be done
148+
if operateCount == numOfIDs { // All operations are going to be done
150149
oldIdToLabelsMap, _, err := podopslifecycle.PodIDAndTypesMap(oldPod)
151150
if err != nil {
152151
return err
@@ -172,7 +171,7 @@ func (lc *OpsLifecycle) Mutating(ctx context.Context, c client.Client, oldPod, n
172171
}
173172
}
174173

175-
if operatedCount == numOfIDs { // all operations are done
174+
if operatedCount == numOfIDs { // All operations are done
176175
for id, labels := range newIDToLabelsMap {
177176
if _, ok := labels[v1alpha1.PodPostCheckLabelPrefix]; !ok {
178177
lc.addLabelWithTime(newPod, fmt.Sprintf("%s/%s", v1alpha1.PodPostCheckLabelPrefix, id)) // post-check

‎pkg/webhook/server/generic/pod/opslifecycle/validating.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func (lc *OpsLifecycle) Validating(ctx context.Context, c client.Client, oldPod,
4141
expectedLabels := make(map[string]struct{})
4242
foundLabels := make(map[string]struct{})
4343
for label := range newPod.Labels {
44-
for _, v := range pairLabelPrefixesMap { // labels must exist together and have the same id
44+
for _, v := range pairLabelPrefixesMap { // Labels must exist together and have the same ID
4545
if !strings.HasPrefix(label, v) {
4646
continue
4747
}
@@ -62,7 +62,7 @@ func (lc *OpsLifecycle) Validating(ctx context.Context, c client.Client, oldPod,
6262
}
6363

6464
found := false
65-
for v := range expectedLabels { // try to find the expected another label prefixes
65+
for v := range expectedLabels { // Try to find the expected another label prefixes
6666
if strings.HasPrefix(label, v) {
6767
foundLabels[v] = struct{}{}
6868
found = true
@@ -73,7 +73,7 @@ func (lc *OpsLifecycle) Validating(ctx context.Context, c client.Client, oldPod,
7373
continue
7474
}
7575

76-
for _, v := range coexistingLabelPrefixesMap { // labels must exist together
76+
for _, v := range coexistingLabelPrefixesMap { // Labels must exist together
7777
if !strings.HasPrefix(label, v) {
7878
continue
7979
}

‎pkg/webhook/server/generic/pod/opslifecycle/webhook.go

Lines changed: 15 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@ limitations under the License.
1717
package opslifecycle
1818

1919
import (
20-
"encoding/json"
2120
"strconv"
22-
"strings"
2321
"time"
2422

2523
corev1 "k8s.io/api/core/v1"
@@ -28,12 +26,8 @@ import (
2826
controllerutils "kusionstack.io/operating/pkg/controllers/utils"
2927
)
3028

31-
const (
32-
waitingForLifecycleSeconds int64 = 5
33-
)
34-
3529
var (
36-
// some labels must exist together and have the same id, and they are a pair
30+
// Some labels must exist together and have the same ID
3731
pairLabelPrefixesMap = map[string]string{
3832
v1alpha1.PodOperatingLabelPrefix: v1alpha1.PodOperationTypeLabelPrefix,
3933
v1alpha1.PodOperationTypeLabelPrefix: v1alpha1.PodOperatingLabelPrefix,
@@ -42,27 +36,24 @@ var (
4236
v1alpha1.PodDoneOperationTypeLabelPrefix: v1alpha1.PodOperatedLabelPrefix,
4337
}
4438

45-
// some labels must exist together
39+
// Some labels must exist together
4640
coexistingLabelPrefixesMap = map[string]string{
4741
v1alpha1.PodPreCheckedLabelPrefix: v1alpha1.PodOperationPermissionLabelPrefix,
4842
v1alpha1.PodOperationPermissionLabelPrefix: v1alpha1.PodPreCheckedLabelPrefix,
4943
}
5044
)
5145

52-
type ReadyToUpgrade func(pod *corev1.Pod) (bool, []string)
46+
type ReadyToOperate func(pod *corev1.Pod) bool
5347
type TimeLabelValue func() string
54-
type IsPodReady func(pod *corev1.Pod) bool
5548

5649
type OpsLifecycle struct {
57-
readyToUpgrade ReadyToUpgrade // for testing
58-
isPodReady IsPodReady
50+
readyToOperate ReadyToOperate
5951
timeLabelValue TimeLabelValue
6052
}
6153

6254
func New() *OpsLifecycle {
6355
return &OpsLifecycle{
64-
readyToUpgrade: hasNoBlockingFinalizer,
65-
isPodReady: controllerutils.IsPodReady,
56+
readyToOperate: readyToOperate,
6657
timeLabelValue: func() string {
6758
return strconv.FormatInt(time.Now().UnixNano(), 10)
6859
},
@@ -91,56 +82,23 @@ func addReadinessGates(pod *corev1.Pod, conditionType corev1.PodConditionType) {
9182
})
9283
}
9384

94-
func hasNoBlockingFinalizer(pod *corev1.Pod) (bool, []string) {
85+
func readyToOperate(pod *corev1.Pod) bool {
9586
if pod == nil {
96-
return true, nil
97-
}
98-
99-
hasReadinessGate := false
100-
if pod.Spec.ReadinessGates != nil {
101-
for _, readinessGate := range pod.Spec.ReadinessGates {
102-
if readinessGate.ConditionType == v1alpha1.ReadinessGatePodServiceReady {
103-
hasReadinessGate = true
104-
break
105-
}
106-
}
107-
}
108-
if !hasReadinessGate {
109-
// if has no service-ready ReadinessGate, treat it as normal pod.
110-
return true, nil
87+
return false
11188
}
11289

113-
if pod.ObjectMeta.Finalizers == nil || len(pod.ObjectMeta.Finalizers) == 0 {
114-
return true, nil
115-
}
116-
117-
var finalizers []string
118-
for _, f := range pod.ObjectMeta.Finalizers {
119-
if strings.HasPrefix(f, v1alpha1.PodOperationProtectionFinalizerPrefix) {
120-
finalizers = append(finalizers, f)
121-
}
90+
index, condition := controllerutils.GetPodCondition(&pod.Status, v1alpha1.ReadinessGatePodServiceReady)
91+
if index == -1 {
92+
return true
12293
}
12394

95+
finalizers := controllerutils.GetProtectionFinalizers(pod)
12496
if len(finalizers) > 0 {
125-
return false, finalizers
126-
}
127-
128-
return true, nil
129-
}
130-
131-
func podAvailableConditions(pod *corev1.Pod) (*v1alpha1.PodAvailableConditions, error) {
132-
if pod.Annotations == nil {
133-
return nil, nil
134-
}
135-
136-
anno, ok := pod.Annotations[v1alpha1.PodAvailableConditionsAnnotation]
137-
if !ok {
138-
return nil, nil
97+
return false
13998
}
14099

141-
availableConditions := &v1alpha1.PodAvailableConditions{}
142-
if err := json.Unmarshal([]byte(anno), availableConditions); err != nil {
143-
return nil, err
100+
if condition.Status == corev1.ConditionFalse {
101+
return true
144102
}
145-
return availableConditions, nil
103+
return false
146104
}

0 commit comments

Comments
 (0)