tigerquoll commented on code in PR #1060:
URL: https://github.com/apache/yunikorn-k8shim/pull/1060#discussion_r3895186780
##########
pkg/conf/schedulerconf.go:
##########
@@ -97,7 +98,14 @@ const (
DefaultKubeBurst = -1
DefaultKubeEventQPS = 200 // events are discardable
and limited by default: <= 0 means no limiter
DefaultKubeEventBurst = 400
+ DefaultKubeEventLevel = EventLevelNormal
DefaultAMFilteringGenerateUniqueAppIds = false
+
+ // event levels, the value of CMKubeEventLevel: all events, the Warning
events only, or
+ // no events at all
+ EventLevelNormal = "normal"
+ EventLevelWarning = "warning"
+ EventLevelNone = "none"
Review Comment:
Correct, Kubernetes only has `Normal` and `Warning` (`v1.EventTypeNormal` /
`v1.EventTypeWarning`); `none` was borrowed from the audit policy levels and
does not belong here. Dropped: `kubernetes.eventLevel` is now `normal` |
`warning`, mirroring the two event types, and any other value falls back to
`normal` with a warning in the log.
##########
pkg/cache/context.go:
##########
@@ -1249,7 +1249,7 @@ func (ctx *Context) HandleContainerStateUpdate(request
*si.UpdateContainerSchedu
Message: request.Reason,
}) {
events.GetRecorder().Eventf(task.GetTaskPod().DeepCopy(), nil,
- v1.EventTypeNormal, "PodUnschedulable",
"PodUnschedulable",
+ v1.EventTypeWarning,
"PodUnschedulable", "PodUnschedulable",
Review Comment:
Done, every `Eventf` in `pkg/cache` cross-checked against the Kubernetes
semantics (Normal = expected progress, Warning = something the user needs to
look at):
- `TaskFailed` (`beforeTaskFail`) retyped Normal → Warning. Both paths into
it (`postTaskRejected`, `failWithEvent`) already emit a Warning for the cause;
the terminal notification now matches.
- `PodUnschedulable` is Warning at both emit sites
(`HandleContainerStateUpdate` SKIPPED and FAILED) and nothing else emits that
reason; a unit test now asserts the type for both, and for `TaskFailed`.
- Kept as Normal: `Scheduling`, `Scheduled`, `PodBindSuccessful`,
`TaskCompleted`, `NodeAccepted`, `NodeDeleted`, and the gang progress events
(`TaskGroupMatch`, `CreatingPlaceholders`, `PlaceholderAllocated`,
`GangReservationComplete`). Already Warning: `FailedScheduling`,
`TaskRejected`, `NodeRejected`, `ApplicationFailed`, `TaskGroupsError`,
`GangSchedulingFailed`, the duplicate-metadata `Scheduling` warning, and
everything routed through `failWithEvent`.
- Out of scope here: the core records forwarded by `PublishEvents` are all
published as Normal `Informational` regardless of what the core is reporting
(e.g. placeholder timeouts). Pre-existing and needs an SI-level change to carry
a type, so I have left it for a follow-up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]