pod probe marker webhook (#1078)

Signed-off-by: liheng.zms <[email protected]>

Signed-off-by: liheng.zms <[email protected]>

apis/apps/v1alpha1/pod_probe_marker_types.go

@@ -43,7 +43,13 @@ type PodContainerProbe struct {
 	Probe ContainerProbeSpec `json:"probe"`
 	// According to the execution result of ContainerProbe, perform specific actions,
 	// such as: patch Pod labels, annotations, ReadinessGate Condition
+	// It cannot be null at the same time as PodConditionType.
 	MarkerPolicy []ProbeMarkerPolicy `json:"markerPolicy,omitempty"`
+	// If it is not empty, the Probe execution result will be recorded on the Pod condition.
+	// It cannot be null at the same time as MarkerPolicy.
+	// For example PodConditionType=game.kruise.io/healthy, pod.status.condition.type = game.kruise.io/healthy.
+	// When probe is Succeeded, pod.status.condition.status = True. Otherwise, when the probe fails to execute, pod.status.condition.status = False.
+	PodConditionType string `json:"podConditionType,omitempty"`
 }
 
 type ContainerProbeSpec struct {

config/crd/bases/apps.kruise.io_podprobemarkers.yaml

@@ -49,7 +49,8 @@ spec:
                     markerPolicy:
                       description: 'According to the execution result of ContainerProbe,
                         perform specific actions, such as: patch Pod labels, annotations,
-                        ReadinessGate Condition'
+                        ReadinessGate Condition It cannot be null at the same time
+                        as PodConditionType.'
                       items:
                         properties:
                           annotations:
@@ -78,6 +79,15 @@ spec:
                       description: probe name, unique within the Pod(Even between
                         different containers, they cannot be the same)
                       type: string
+                    podConditionType:
+                      description: If it is not empty, the Probe execution result
+                        will be recorded on the Pod condition. It cannot be null at
+                        the same time as MarkerPolicy. For example PodConditionType=game.kruise.io/healthy,
+                        pod.status.condition.type = game.kruise.io/healthy. When probe
+                        is Succeeded, pod.status.condition.status = True. Otherwise,
+                        when the probe fails to execute, pod.status.condition.status
+                        = False.
+                      type: string
                     probe:
                       description: container probe spec
                       properties:

config/webhook/manifests.yaml

@@ -577,6 +577,27 @@ webhooks:
     resources:
     - pods/eviction
   sideEffects: None
+- admissionReviewVersions:
+  - v1
+  - v1beta1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-apps-kruise-io-podprobemarker
+  failurePolicy: Fail
+  name: vpodprobemarker.kb.io
+  rules:
+  - apiGroups:
+    - apps.kruise.io
+    apiVersions:
+    - v1alpha1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - podprobemarkers
+  sideEffects: None
 - admissionReviewVersions:
   - v1
   - v1beta1

pkg/controller/nodepodprobe/node_pod_probe_controller.go

@@ -19,11 +19,12 @@ package nodepodprobe
 import (
 	"context"
 	"flag"
-	"fmt"
 	"reflect"
 	"strings"
 	"time"
 
+	"k8s.io/apimachinery/pkg/util/sets"
+
 	appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1"
 	"github.com/openkruise/kruise/pkg/util"
 	utilclient "github.com/openkruise/kruise/pkg/util/client"
@@ -235,30 +236,16 @@ func (r *ReconcileNodePodProbe) updatePodProbeStatus(pod *corev1.Pod, status app
 	// pod status condition record probe result
 	var probeConditions []corev1.PodCondition
 	var err error
+	validConditionTypes := sets.NewString()
 	for i := range status.ProbeStates {
 		probeState := status.ProbeStates[i]
 		// ignore the probe state
 		if probeState.State == "" || probeState.State == currentConditions[probeState.Name] {
 			continue
 		}
 
-		var conStatus corev1.ConditionStatus
-		if probeState.State == appsv1alpha1.ProbeSucceeded {
-			conStatus = corev1.ConditionTrue
-		} else {
-			conStatus = corev1.ConditionFalse
-		}
+		// fetch podProbeMarker
 		ppmName, probeName := strings.Split(probeState.Name, "#")[0], strings.Split(probeState.Name, "#")[1]
-		probeConditions = append(probeConditions, corev1.PodCondition{
-			// type -> PodProbeMarker#podProbeMarker.Name#probe.Name
-			Type:               corev1.PodConditionType(fmt.Sprintf("PodProbeMarker#%s#%s", ppmName, probeName)),
-			Status:             conStatus,
-			LastProbeTime:      probeState.LastProbeTime,
-			LastTransitionTime: probeState.LastTransitionTime,
-			Message:            probeState.Message,
-		})
-		// marker pod labels & annotations according to probe state
-		// fetch NodePodProbe
 		ppm := &appsv1alpha1.PodProbeMarker{}
 		err = r.Get(context.TODO(), client.ObjectKey{Namespace: pod.Namespace, Name: ppmName}, ppm)
 		if err != nil {
@@ -272,45 +259,73 @@ func (r *ReconcileNodePodProbe) updatePodProbeStatus(pod *corev1.Pod, status app
 			continue
 		}
 		var policy []appsv1alpha1.ProbeMarkerPolicy
+		var conditionType string
 		for _, probe := range ppm.Spec.Probes {
 			if probe.Name == probeName {
 				policy = probe.MarkerPolicy
+				conditionType = probe.PodConditionType
 				break
 			}
 		}
+
+		if conditionType != "" && validConditionTypes.Has(conditionType) {
+			klog.Warningf("NodePodProbe(%s) pod(%s/%s) condition(%s) is conflict", ppmName, pod.Namespace, pod.Name, conditionType)
+			// patch pod condition
+		} else if conditionType != "" {
+			validConditionTypes.Insert(conditionType)
+			var conStatus corev1.ConditionStatus
+			if probeState.State == appsv1alpha1.ProbeSucceeded {
+				conStatus = corev1.ConditionTrue
+			} else {
+				conStatus = corev1.ConditionFalse
+			}
+			probeConditions = append(probeConditions, corev1.PodCondition{
+				Type:               corev1.PodConditionType(conditionType),
+				Status:             conStatus,
+				LastProbeTime:      probeState.LastProbeTime,
+				LastTransitionTime: probeState.LastTransitionTime,
+				Message:            probeState.Message,
+			})
+		}
+
 		if len(policy) == 0 {
 			continue
 		}
-		// patch pod labels & annotations
-		var matchedPolicy *appsv1alpha1.ProbeMarkerPolicy
+
+		// matchedPolicy is when policy.state is equal to probeState.State, otherwise oppositePolicy
+		// 1. If policy[0].state = Succeeded, policy[1].state = Failed. probeState.State = Succeeded.
+		// So policy[0] is matchedPolicy, policy[1] is oppositePolicy
+		// 2. If policy[0].state = Succeeded, and policy[1] does not exist. probeState.State = Succeeded.
+		// So policy[0] is matchedPolicy, oppositePolicy is nil
+		// 3. If policy[0].state = Succeeded, and policy[1] does not exist. probeState.State = Failed.
+		// So policy[0] is oppositePolicy, matchedPolicy is nil
+		var matchedPolicy, oppositePolicy *appsv1alpha1.ProbeMarkerPolicy
 		for j := range policy {
 			if policy[j].State == probeState.State {
 				matchedPolicy = &policy[j]
-				break
+			} else {
+				oppositePolicy = &policy[j]
+			}
+		}
+		if oppositePolicy != nil {
+			for k := range oppositePolicy.Labels {
+				probeMetadata.Labels[k] = nil
+			}
+			for k := range oppositePolicy.Annotations {
+				probeMetadata.Annotations[k] = nil
 			}
 		}
-		// find matched policy
 		if matchedPolicy != nil {
 			for k, v := range matchedPolicy.Labels {
 				probeMetadata.Labels[k] = v
 			}
 			for k, v := range matchedPolicy.Annotations {
 				probeMetadata.Annotations[k] = v
 			}
-			continue
-		}
-		// If only one Marker Policy is defined, for example: only define State=Succeeded, Patch Labels[healthy]='true'.
-		// When the probe execution success, kruise will patch labels[healthy]='true' to pod.
-		// And when the probe execution fails, Label[healthy] will be deleted.
-		for k := range policy[0].Labels {
-			probeMetadata.Labels[k] = nil
-		}
-		for k := range policy[0].Annotations {
-			probeMetadata.Annotations[k] = nil
 		}
 	}
 	// probe condition no changed, continue
-	if len(probeConditions) == 0 {
+	if len(probeConditions) == 0 && len(probeMetadata.Labels) == 0 && len(probeMetadata.Annotations) == 0 {
 		return nil
 	}
 

pkg/controller/nodepodprobe/node_pod_probe_controller_test.go

@@ -64,6 +64,7 @@ var (
 							},
 						},
 					},
+					PodConditionType: "game.kruise.io/healthy",
 					MarkerPolicy: []appsv1alpha1.ProbeMarkerPolicy{
 						{
 							State: appsv1alpha1.ProbeSucceeded,
@@ -249,7 +250,7 @@ func TestSyncNodePodProbe(t *testing.T) {
 						Status: corev1.PodStatus{
 							Conditions: []corev1.PodCondition{
 								{
-									Type:   corev1.PodConditionType("PodProbeMarker#ppm-1#healthy"),
+									Type:   corev1.PodConditionType("game.kruise.io/healthy"),
 									Status: corev1.ConditionTrue,
 								},
 							},
@@ -273,7 +274,7 @@ func TestSyncNodePodProbe(t *testing.T) {
 						Status: corev1.PodStatus{
 							Conditions: []corev1.PodCondition{
 								{
-									Type:   corev1.PodConditionType("PodProbeMarker#ppm-1#healthy"),
+									Type:   corev1.PodConditionType("game.kruise.io/healthy"),
 									Status: corev1.ConditionFalse,
 								},
 							},
@@ -320,11 +321,11 @@ func TestSyncNodePodProbe(t *testing.T) {
 						Status: corev1.PodStatus{
 							Conditions: []corev1.PodCondition{
 								{
-									Type:   corev1.PodConditionType("PodProbeMarker#ppm-1#healthy"),
+									Type:   corev1.PodConditionType("game.kruise.io/healthy"),
 									Status: corev1.ConditionTrue,
 								},
 								{
-									Type:   corev1.PodConditionType("PodProbeMarker#ppm-2#other"),
+									Type:   corev1.PodConditionType("game.kruise.io/other"),
 									Status: corev1.ConditionTrue,
 								},
 							},
@@ -372,11 +373,11 @@ func TestSyncNodePodProbe(t *testing.T) {
 						Status: corev1.PodStatus{
 							Conditions: []corev1.PodCondition{
 								{
-									Type:   corev1.PodConditionType("PodProbeMarker#ppm-1#healthy"),
+									Type:   corev1.PodConditionType("game.kruise.io/healthy"),
 									Status: corev1.ConditionFalse,
 								},
 								{
-									Type:   corev1.PodConditionType("PodProbeMarker#ppm-2#other"),
+									Type:   corev1.PodConditionType("game.kruise.io/other"),
 									Status: corev1.ConditionTrue,
 								},
 							},
@@ -386,6 +387,115 @@ func TestSyncNodePodProbe(t *testing.T) {
 				return pods
 			},
 		},
+		{
+			name: "test3, marker policy",
+			req: ctrl.Request{
+				NamespacedName: types.NamespacedName{
+					Name: demoNodePodProbe.Name,
+				},
+			},
+			getNode: func() []*corev1.Node {
+				nodes := []*corev1.Node{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "node-1",
+						},
+					},
+				}
+				return nodes
+			},
+			getPods: func() []*corev1.Pod {
+				pods := []*corev1.Pod{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "pod-1",
+							UID:  types.UID("pod-1-uid"),
+							Labels: map[string]string{
+								"app":            "test",
+								"server-healthy": "true",
+								"success":        "true",
+							},
+							Annotations: map[string]string{
+								"controller.kubernetes.io/pod-deletion-cost": "10",
+								"success": "true",
+							},
+						},
+						Spec: corev1.PodSpec{
+							NodeName: "node-1",
+						},
+					},
+				}
+				return pods
+			},
+			getPodProbeMarkers: func() []*appsv1alpha1.PodProbeMarker {
+				demo := demoPodProbeMarker.DeepCopy()
+				demo.Spec.Probes[0].PodConditionType = ""
+				demo.Spec.Probes[0].MarkerPolicy = []appsv1alpha1.ProbeMarkerPolicy{
+					{
+						State: appsv1alpha1.ProbeSucceeded,
+						Annotations: map[string]string{
+							"controller.kubernetes.io/pod-deletion-cost": "10",
+							"success": "true",
+						},
+						Labels: map[string]string{
+							"server-healthy": "true",
+							"success":        "true",
+						},
+					},
+					{
+						State: appsv1alpha1.ProbeFailed,
+						Annotations: map[string]string{
+							"controller.kubernetes.io/pod-deletion-cost": "-10",
+							"failed": "true",
+						},
+						Labels: map[string]string{
+							"failed": "true",
+						},
+					},
+				}
+				return []*appsv1alpha1.PodProbeMarker{demo}
+			},
+			getNodePodProbes: func() []*appsv1alpha1.NodePodProbe {
+				demo := demoNodePodProbe.DeepCopy()
+				demo.Status = appsv1alpha1.NodePodProbeStatus{
+					PodProbeStatuses: []appsv1alpha1.PodProbeStatus{
+						{
+							Name: "pod-1",
+							UID:  "pod-1-uid",
+							ProbeStates: []appsv1alpha1.ContainerProbeState{
+								{
+									Name:  "ppm-1#healthy",
+									State: appsv1alpha1.ProbeFailed,
+								},
+							},
+						},
+					},
+				}
+				return []*appsv1alpha1.NodePodProbe{demo}
+			},
+			expectPods: func() []*corev1.Pod {
+				pods := []*corev1.Pod{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "pod-1",
+							UID:  types.UID("pod-1-uid"),
+							Labels: map[string]string{
+								"app":    "test",
+								"failed": "true",
+							},
+							Annotations: map[string]string{
+								"controller.kubernetes.io/pod-deletion-cost": "-10",
+								"failed": "true",
+							},
+						},
+						Spec: corev1.PodSpec{
+							NodeName: "node-1",
+						},
+					},
+				}
+				return pods
+			},
+		},
 	}
 
 	for _, cs := range cases {

pkg/controller/podprobemarker/pod_probe_marker_controller_test.go

@@ -64,6 +64,7 @@ var (
 							},
 						},
 					},
+					PodConditionType: "game.kruise.io/healthy",
 					MarkerPolicy: []appsv1alpha1.ProbeMarkerPolicy{
 						{
 							State: appsv1alpha1.ProbeSucceeded,