diff --git a/apis/apps/v1alpha1/pod_probe_marker_types.go b/apis/apps/v1alpha1/pod_probe_marker_types.go index 722efb48b8..a241970c13 100644 --- a/apis/apps/v1alpha1/pod_probe_marker_types.go +++ b/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 { diff --git a/config/crd/bases/apps.kruise.io_podprobemarkers.yaml b/config/crd/bases/apps.kruise.io_podprobemarkers.yaml index a1f88a76cc..4b8c0deac3 100644 --- a/config/crd/bases/apps.kruise.io_podprobemarkers.yaml +++ b/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: diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index add4fca95d..7cd14510e7 100644 --- a/config/webhook/manifests.yaml +++ b/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 diff --git a/pkg/controller/nodepodprobe/node_pod_probe_controller.go b/pkg/controller/nodepodprobe/node_pod_probe_controller.go index 1eca44383a..71302ad957 100644 --- a/pkg/controller/nodepodprobe/node_pod_probe_controller.go +++ b/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,6 +236,7 @@ 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 @@ -242,23 +244,8 @@ func (r *ReconcileNodePodProbe) updatePodProbeStatus(pod *corev1.Pod, status app 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,24 +259,62 @@ 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 @@ -297,20 +322,10 @@ func (r *ReconcileNodePodProbe) updatePodProbeStatus(pod *corev1.Pod, status app 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 } diff --git a/pkg/controller/nodepodprobe/node_pod_probe_controller_test.go b/pkg/controller/nodepodprobe/node_pod_probe_controller_test.go index 9ba4198ae2..bc3cca698d 100644 --- a/pkg/controller/nodepodprobe/node_pod_probe_controller_test.go +++ b/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 { diff --git a/pkg/controller/podprobemarker/pod_probe_marker_controller_test.go b/pkg/controller/podprobemarker/pod_probe_marker_controller_test.go index c1fe12314e..97b5317432 100644 --- a/pkg/controller/podprobemarker/pod_probe_marker_controller_test.go +++ b/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, diff --git a/pkg/webhook/add_podprobemarker.go b/pkg/webhook/add_podprobemarker.go new file mode 100644 index 0000000000..9118e79948 --- /dev/null +++ b/pkg/webhook/add_podprobemarker.go @@ -0,0 +1,25 @@ +/* +Copyright 2022 The Kruise Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package webhook + +import ( + "github.com/openkruise/kruise/pkg/webhook/podprobemarker/validating" +) + +func init() { + addHandlers(validating.HandlerMap) +} diff --git a/pkg/webhook/podprobemarker/validating/probe_create_update_handler.go b/pkg/webhook/podprobemarker/validating/probe_create_update_handler.go new file mode 100644 index 0000000000..c01cfda564 --- /dev/null +++ b/pkg/webhook/podprobemarker/validating/probe_create_update_handler.go @@ -0,0 +1,247 @@ +/* +Copyright 2022 The Kruise Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validating + +import ( + "context" + "fmt" + "net/http" + "reflect" + "regexp" + + "github.com/openkruise/kruise/apis/apps/pub" + appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" + admissionv1 "k8s.io/api/admission/v1" + corev1 "k8s.io/api/core/v1" + genericvalidation "k8s.io/apimachinery/pkg/api/validation" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metavalidation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" + "k8s.io/apimachinery/pkg/util/sets" + validationutil "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/kubernetes/pkg/apis/core/validation" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/runtime/inject" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +const ( + nameMaxLen = 63 +) + +var ( + validateNameMsg = "podProbeMarker name must consist of alphanumeric characters or '-'" + validateNameRegex = regexp.MustCompile(validNameFmt) + validNameFmt = `^[a-zA-Z0-9\-]+$` + // k8s native pod condition type + k8sNativePodConditions = sets.NewString(string(corev1.PodScheduled), string(corev1.PodInitialized), string(corev1.PodReady), + string(corev1.ContainersReady), string(pub.KruisePodReadyConditionType), string(pub.InPlaceUpdateReady)) +) + +// PodProbeMarkerCreateUpdateHandler handles PodProbeMarker +type PodProbeMarkerCreateUpdateHandler struct { + // Decoder decodes objects + Decoder *admission.Decoder +} + +var _ admission.Handler = &PodProbeMarkerCreateUpdateHandler{} + +// Handle handles admission requests. +func (h *PodProbeMarkerCreateUpdateHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + obj := &appsv1alpha1.PodProbeMarker{} + err := h.Decoder.Decode(req, obj) + if err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + var old *appsv1alpha1.PodProbeMarker + //when Operation is update, decode older object + if req.AdmissionRequest.Operation == admissionv1.Update { + old = new(appsv1alpha1.PodProbeMarker) + if err := h.Decoder.Decode( + admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Object: req.AdmissionRequest.OldObject}}, + old); err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + } + allErrs := h.validatingPodProbeMarkerFn(obj, old) + if len(allErrs) != 0 { + return admission.Errored(http.StatusBadRequest, allErrs.ToAggregate()) + } + return admission.ValidationResponse(true, "") +} + +func (h *PodProbeMarkerCreateUpdateHandler) validatingPodProbeMarkerFn(obj, old *appsv1alpha1.PodProbeMarker) field.ErrorList { + //validate ppm.Spec + allErrs := validatePodProbeMarkerSpec(obj, field.NewPath("spec")) + // when operation is update, validating whether old and new pps conflict + if old != nil { + if !reflect.DeepEqual(old.Spec.Selector, obj.Spec.Selector) { + allErrs = append(allErrs, field.Invalid(field.NewPath("selector"), obj.Spec.Selector, "selector must be immutable")) + } + } + return allErrs +} + +func validatePodProbeMarkerSpec(obj *appsv1alpha1.PodProbeMarker, fldPath *field.Path) field.ErrorList { + spec := &obj.Spec + allErrs := field.ErrorList{} + // selector + if spec.Selector == nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("selector"), spec.Selector, "empty Selector is not valid for PodProbeMarker.")) + } else { + allErrs = append(allErrs, validateSelector(spec.Selector, fldPath.Child("selector"))...) + } + // containerProbe + if len(spec.Probes) == 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("probes"), spec.Selector, "empty probes is not valid for PodProbeMarker.")) + return allErrs + } + uniqueProbe := sets.NewString() + uniqueConditionType := sets.NewString() + for _, probe := range spec.Probes { + if probe.Name == "" || probe.ContainerName == "" { + allErrs = append(allErrs, field.Invalid(fldPath.Child("probes"), spec.Selector, "probe name and containerName can't be empty in PodProbeMarker.")) + return allErrs + } + if k8sNativePodConditions.Has(probe.Name) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("probes"), spec.Selector, fmt.Sprintf("probe name can't be %s", probe.Name))) + return allErrs + } + if uniqueProbe.Has(probe.Name) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("probes"), spec.Selector, "probe name must be unique in PodProbeMarker.")) + return allErrs + } + uniqueProbe.Insert(probe.Name) + allErrs = append(allErrs, validateProbe(&probe.Probe, fldPath.Child("probe"))...) + if probe.PodConditionType == "" && len(probe.MarkerPolicy) == 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("probes"), spec.Selector, "podConditionType and markerPolicy cannot be empty at the same time")) + return allErrs + } + if probe.PodConditionType != "" && uniqueConditionType.Has(probe.PodConditionType) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("probes"), spec.Selector, fmt.Sprintf("podConditionType %s must be unique in podProbeMarker", probe.PodConditionType))) + return allErrs + } else if probe.PodConditionType != "" { + uniqueConditionType.Insert(probe.PodConditionType) + allErrs = append(allErrs, metavalidation.ValidateLabelName(probe.PodConditionType, fldPath)...) + } + uniquePolicy := sets.NewString() + for _, policy := range probe.MarkerPolicy { + if uniquePolicy.Has(string(policy.State)) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("markerPolicy"), spec.Selector, "marker policy state must be unique.")) + return allErrs + } + uniquePolicy.Insert(string(policy.State)) + allErrs = append(allErrs, validateProbeMarkerPolicy(&policy, fldPath.Child("markerPolicy"))...) + } + } + + return allErrs +} + +func validateSelector(selector *metav1.LabelSelector, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + allErrs = append(allErrs, metavalidation.ValidateLabelSelector(selector, fldPath)...) + if len(selector.MatchLabels)+len(selector.MatchExpressions) == 0 { + allErrs = append(allErrs, field.Invalid(fldPath, selector, "empty selector is not valid for podProbeMarker.")) + } + _, err := metav1.LabelSelectorAsSelector(selector) + if err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("selector"), selector, "")) + } + return allErrs +} + +func validateProbe(probe *appsv1alpha1.ContainerProbeSpec, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if probe == nil { + allErrs = append(allErrs, field.Invalid(fldPath, probe, "probe can't be empty in podProbeMarker.")) + return allErrs + } + allErrs = append(allErrs, validateHandler(&probe.Handler, fldPath)...) + allErrs = append(allErrs, validation.ValidateNonnegativeField(int64(probe.InitialDelaySeconds), fldPath.Child("initialDelaySeconds"))...) + allErrs = append(allErrs, validation.ValidateNonnegativeField(int64(probe.TimeoutSeconds), fldPath.Child("timeoutSeconds"))...) + allErrs = append(allErrs, validation.ValidateNonnegativeField(int64(probe.PeriodSeconds), fldPath.Child("periodSeconds"))...) + allErrs = append(allErrs, validation.ValidateNonnegativeField(int64(probe.SuccessThreshold), fldPath.Child("successThreshold"))...) + allErrs = append(allErrs, validation.ValidateNonnegativeField(int64(probe.FailureThreshold), fldPath.Child("failureThreshold"))...) + return allErrs +} + +func validateHandler(handler *corev1.Handler, fldPath *field.Path) field.ErrorList { + numHandlers := 0 + allErrors := field.ErrorList{} + if handler.Exec != nil { + numHandlers++ + allErrors = append(allErrors, validateExecAction(handler.Exec, fldPath.Child("exec"))...) + } + if handler.HTTPGet != nil || handler.TCPSocket != nil { + numHandlers++ + allErrors = append(allErrors, field.Forbidden(fldPath.Child("probe"), "current only support exec probe")) + } + if numHandlers == 0 { + allErrors = append(allErrors, field.Required(fldPath, "must specify a handler type")) + } + return allErrors +} + +func validateExecAction(exec *corev1.ExecAction, fldPath *field.Path) field.ErrorList { + allErrors := field.ErrorList{} + if len(exec.Command) == 0 { + allErrors = append(allErrors, field.Required(fldPath.Child("command"), "")) + } + return allErrors +} + +func validateProbeMarkerPolicy(policy *appsv1alpha1.ProbeMarkerPolicy, fldPath *field.Path) field.ErrorList { + allErrors := field.ErrorList{} + if policy.State != appsv1alpha1.ProbeSucceeded && policy.State != appsv1alpha1.ProbeFailed { + allErrors = append(allErrors, field.Required(fldPath.Child("state"), "marker policy state must be 'True' or 'False'")) + return allErrors + } + if len(policy.Labels) == 0 && len(policy.Annotations) == 0 { + allErrors = append(allErrors, field.Required(fldPath, "marker policy annotations or labels can't be both empty")) + return allErrors + } + metadata := metav1.ObjectMeta{Annotations: policy.Annotations, Labels: policy.Labels, Name: "fake-name"} + allErrors = append(allErrors, genericvalidation.ValidateObjectMeta(&metadata, false, validatePodProbeMarkerName, fldPath)...) + + return allErrors +} + +func validatePodProbeMarkerName(name string, prefix bool) (allErrs []string) { + if !validateNameRegex.MatchString(name) { + allErrs = append(allErrs, validationutil.RegexError(validateNameMsg, validNameFmt, "example-com")) + } + if len(name) > nameMaxLen { + allErrs = append(allErrs, validationutil.MaxLenError(nameMaxLen)) + } + return allErrs +} + +var _ inject.Client = &PodProbeMarkerCreateUpdateHandler{} + +// InjectClient injects the client into the PodProbeMarkerCreateUpdateHandler +func (h *PodProbeMarkerCreateUpdateHandler) InjectClient(c client.Client) error { + return nil +} + +var _ admission.DecoderInjector = &PodProbeMarkerCreateUpdateHandler{} + +// InjectDecoder injects the decoder into the PodProbeMarkerCreateUpdateHandler +func (h *PodProbeMarkerCreateUpdateHandler) InjectDecoder(d *admission.Decoder) error { + h.Decoder = d + return nil +} diff --git a/pkg/webhook/podprobemarker/validating/probe_validating_test.go b/pkg/webhook/podprobemarker/validating/probe_validating_test.go new file mode 100644 index 0000000000..448c11e663 --- /dev/null +++ b/pkg/webhook/podprobemarker/validating/probe_validating_test.go @@ -0,0 +1,236 @@ +/* +Copyright 2022 The Kruise Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validating + +import ( + "testing" + + appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +func init() { + scheme = runtime.NewScheme() + _ = appsv1alpha1.AddToScheme(scheme) +} + +var ( + scheme *runtime.Scheme + + ppmDemo = appsv1alpha1.PodProbeMarker{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ppm-test", + }, + Spec: appsv1alpha1.PodProbeMarkerSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "test", + }, + }, + Probes: []appsv1alpha1.PodContainerProbe{ + { + Name: "healthy", + ContainerName: "main", + Probe: appsv1alpha1.ContainerProbeSpec{ + Probe: corev1.Probe{ + Handler: corev1.Handler{ + Exec: &corev1.ExecAction{ + Command: []string{"/bin/sh", "-c", "/healthy.sh"}, + }, + }, + }, + }, + PodConditionType: "game.kruise.io/healthy", + MarkerPolicy: []appsv1alpha1.ProbeMarkerPolicy{ + { + State: appsv1alpha1.ProbeSucceeded, + Annotations: map[string]string{ + "controller.kubernetes.io/pod-deletion-cost": "10", + }, + Labels: map[string]string{ + "server-healthy": "true", + }, + }, + { + State: appsv1alpha1.ProbeFailed, + Annotations: map[string]string{ + "controller.kubernetes.io/pod-deletion-cost": "-10", + }, + Labels: map[string]string{ + "server-healthy": "false", + }, + }, + }, + }, + }, + }, + } +) + +func TestValidatingPodProbeMarker(t *testing.T) { + cases := []struct { + name string + getPpm func() *appsv1alpha1.PodProbeMarker + expectErrList int + }{ + { + name: "test1, invalid ppm", + getPpm: func() *appsv1alpha1.PodProbeMarker { + ppm := ppmDemo.DeepCopy() + ppm.Spec.Selector = nil + return ppm + }, + expectErrList: 1, + }, + { + name: "test2, invalid ppm", + getPpm: func() *appsv1alpha1.PodProbeMarker { + ppm := ppmDemo.DeepCopy() + ppm.Spec.Probes = nil + return ppm + }, + expectErrList: 1, + }, + { + name: "test3, invalid ppm", + getPpm: func() *appsv1alpha1.PodProbeMarker { + ppm := ppmDemo.DeepCopy() + ppm.Spec.Probes = append(ppm.Spec.Probes, appsv1alpha1.PodContainerProbe{ + Name: "healthy", + ContainerName: "other", + }) + return ppm + }, + expectErrList: 1, + }, + { + name: "test4, invalid ppm", + getPpm: func() *appsv1alpha1.PodProbeMarker { + ppm := ppmDemo.DeepCopy() + ppm.Spec.Probes = append(ppm.Spec.Probes, appsv1alpha1.PodContainerProbe{ + Name: "check", + ContainerName: "other", + Probe: appsv1alpha1.ContainerProbeSpec{ + Probe: corev1.Probe{ + Handler: corev1.Handler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(80), + }, + }, + }, + }, + PodConditionType: "game.kruise.io/check", + }) + return ppm + }, + expectErrList: 1, + }, + { + name: "test5, invalid ppm", + getPpm: func() *appsv1alpha1.PodProbeMarker { + ppm := ppmDemo.DeepCopy() + ppm.Spec.Probes = append(ppm.Spec.Probes, appsv1alpha1.PodContainerProbe{ + Name: "check", + ContainerName: "other", + Probe: appsv1alpha1.ContainerProbeSpec{ + Probe: corev1.Probe{ + Handler: corev1.Handler{ + Exec: &corev1.ExecAction{}, + }, + }, + }, + PodConditionType: "game.kruise.io/check", + }) + return ppm + }, + expectErrList: 1, + }, + { + name: "test6, invalid ppm", + getPpm: func() *appsv1alpha1.PodProbeMarker { + ppm := ppmDemo.DeepCopy() + ppm.Spec.Probes[0].MarkerPolicy = []appsv1alpha1.ProbeMarkerPolicy{ + { + State: appsv1alpha1.ProbeUnknown, + Annotations: map[string]string{ + "controller.kubernetes.io/pod-deletion-cost": "10", + }, + Labels: map[string]string{ + "server-healthy": "true", + }, + }, + } + return ppm + }, + expectErrList: 1, + }, + { + name: "test7, invalid ppm", + getPpm: func() *appsv1alpha1.PodProbeMarker { + ppm := ppmDemo.DeepCopy() + ppm.Spec.Probes[0].MarkerPolicy = []appsv1alpha1.ProbeMarkerPolicy{ + { + State: appsv1alpha1.ProbeSucceeded, + Annotations: map[string]string{ + "controller.kubernetes.io/pod-deletion-cost": "10", + }, + Labels: map[string]string{ + "server-/$healthy": "true", + }, + }, + } + return ppm + }, + expectErrList: 2, + }, + { + name: "test8, invalid ppm", + getPpm: func() *appsv1alpha1.PodProbeMarker { + ppm := ppmDemo.DeepCopy() + ppm.Spec.Probes[0].Name = string(corev1.PodInitialized) + return ppm + }, + expectErrList: 1, + }, + { + name: "test9, invalid ppm", + getPpm: func() *appsv1alpha1.PodProbeMarker { + ppm := ppmDemo.DeepCopy() + ppm.Spec.Probes[0].PodConditionType = "#5invalid" + return ppm + }, + expectErrList: 1, + }, + } + + decoder, _ := admission.NewDecoder(scheme) + perHandler := PodProbeMarkerCreateUpdateHandler{ + Decoder: decoder, + } + for _, cs := range cases { + t.Run(cs.name, func(t *testing.T) { + errList := perHandler.validatingPodProbeMarkerFn(cs.getPpm(), nil) + if len(errList) != cs.expectErrList { + t.Fatalf("expect errList(%d) but get(%d) error: %s", cs.expectErrList, len(errList), errList.ToAggregate().Error()) + } + }) + } +} diff --git a/pkg/webhook/podprobemarker/validating/webhooks.go b/pkg/webhook/podprobemarker/validating/webhooks.go new file mode 100644 index 0000000000..2cbe717de9 --- /dev/null +++ b/pkg/webhook/podprobemarker/validating/webhooks.go @@ -0,0 +1,30 @@ +/* +Copyright 2022 The Kruise Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validating + +import ( + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// +kubebuilder:webhook:path=/validate-apps-kruise-io-podprobemarker,mutating=false,failurePolicy=fail,sideEffects=None,admissionReviewVersions=v1;v1beta1,groups=apps.kruise.io,resources=podprobemarkers,verbs=create;update,versions=v1alpha1,name=vpodprobemarker.kb.io + +var ( + // HandlerMap contains admission webhook handlers + HandlerMap = map[string]admission.Handler{ + "validate-apps-kruise-io-podprobemarker": &PodProbeMarkerCreateUpdateHandler{}, + } +) diff --git a/test/e2e/policy/podunavailablebudget.go b/test/e2e/policy/podunavailablebudget.go index c438170dea..fab100a5e6 100644 --- a/test/e2e/policy/podunavailablebudget.go +++ b/test/e2e/policy/podunavailablebudget.go @@ -110,13 +110,13 @@ var _ = SIGDescribe("PodUnavailableBudget", func() { } ginkgo.By(fmt.Sprintf("Creating PodUnavailableBudget(%s/%s)", pub.Namespace, pub.Name)) tester.CreatePub(pub) + time.Sleep(time.Second * 3) // create deployment deployment := tester.NewBaseDeployment(ns) deployment.Spec.Replicas = utilpointer.Int32Ptr(1) ginkgo.By(fmt.Sprintf("Creating Deployment(%s/%s)", deployment.Namespace, deployment.Name)) tester.CreateDeployment(deployment) - time.Sleep(time.Second * 3) ginkgo.By(fmt.Sprintf("check PodUnavailableBudget(%s/%s) Status", pub.Namespace, pub.Name)) expectStatus := &policyv1alpha1.PodUnavailableBudgetStatus{