Repository navigation
feat(observability): label a task whose pods were never created - #413
Draft
yuechao-qin wants to merge 1 commit into
Draft
yuechao-qin wants to merge 1 commit into
yuechao-qin wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A task whose pods are refused at admission now says so, instead of looking identical to
a task nobody has told us anything about.
The
execution.pending_reasonlabel gains a sixth value,no_pod_created, and the twoscenarios it separates are:
The second is a tenant's own namespace quota, RBAC rule or pod security policy. It
should never page the on-call for Tangle capacity.
Why
The existing five values all come from reading a pod. When admission refuses every pod,
there is no pod to read, and the label falls back to
none— the same value a task getswhen the launcher is not Kubernetes at all. A four-hour wait caused by an exhausted quota
is therefore indistinguishable from a four-hour wait we have no information about.
Which admission rule refused the pod is not knowable from the data we hold. Kubernetes
records that as a
FailedCreateEvent, andlauncher_dataserialises the Job and itspods, never Events. This change reports that nothing was scheduled, not why — enough to
route the alert, and the operator reads Events to fix it.
Changes
What the new value means
no_pod_createdis returned when the launcher is Kubernetes and the serialised pod mapis empty. It is last-stamp-wins, not an observation: a Job is launched with an empty pod
map, so every task is briefly in this state, and a healthy one is overwritten within
seconds by the first poll that sees a pod. It survives to be read only when no pod ever
appears.
A guard that was missing
pending_reasonpreviously collapsed a non-Kubernetes launcher into an empty pod map.That was harmless while an empty map meant "say nothing", and would now label every
local-process task as a refused Kubernetes pod. The launcher key is checked explicitly.
Why there are now two listeners
The orchestrator supplies the first
launcher_dataas a constructor argument. That doesreach the existing
setlistener, which computes the right reason — and__init__thenthrows it away, because
extra_datais declared afterlauncher_dataand gets assignedits own default of
None. Until now that destroyed nothing, since a Job at launch has anempty pod map and no reason existed to lose.
no_pod_createdis the first reason thatcan exist at construction time, so the clobber starts to matter here.
A
before_inserthook stamps the row at flush, once the object is fully built, where nodeclaration order can reach it. The write rides the same
INSERTand costs no secondstatement.
Neither listener can replace the other. The metric reads the reason at the session's
before_commit, and every mapper flush hook runs after that, so a reason stamped atbefore_updatewould be read a commit too late — thesetlistener has to stay forpolls. The
setlistener in turn cannot survive__init__— the insert hook has to stayfor the first write. Both docstrings say so, and the stamping body is shared rather than
duplicated.
Tests
setlistener alone would lose the constructor's reason, which fails loudly ifextra_datais ever declared abovelauncher_data661 passing. Mutation probes: dropping the no-pod branch turns 3 red, dropping the
not-Kubernetes guard turns 16 red, moving the insert hook to
before_updateturns 2 red,moving the
setlistener tobefore_updateturns 15 red, and declaringextra_dataabove
launcher_dataturns the new ordering test red on its own.