Skip to content

Design workload creation-failure explanations (#411) - #516

Merged
hellices merged 7 commits into
mainfrom
docs/515-creation-constraints-design
Oct 30, 2026
Merged

hellices merged 7 commits into
mainfrom
docs/515-creation-constraints-design

Conversation

@hellices

@hellices hellices commented Oct 30, 2026 •

Copy link
Copy Markdown
Owner

Closes #515

Summary

The design for #411, before any code: docs/superpowers/specs/2026-10-30-creation-constraints-design.md. Merging it approves the design; each slice then gets its own sub-issue and PR.

A read-only view opened on a Deployment, ReplicaSet, StatefulSet, DaemonSet, Job or CronJob, through :blocked or Explain workload creation failures. It shows the creation failures recorded for the workload next to the namespace's ResourceQuotas and LimitRanges. It keeps four answers apart: a recorded rejection, a possible current constraint, no standard policy observed, and not established.

  • Reads use the Add capability-aware diagnostics with explicit older-cluster fallbacks #409 readers under one run token, in one namespace:
    • the workload (GET, expected UID);
    • its owned ReplicaSets, for a Deployment, or its owned Jobs, for a CronJob;
    • Events, listed and filtered locally to FailedCreate events matched by UID;
    • quotas and LimitRanges.
      Templates are projected at once, so no literal environment value leaves the run.
  • Classification relies only on fixed quota and LimitRanger phrasings. A quota is attributed only by the name its message gives. A LimitRange message is attributed to "a LimitRange in this namespace", because LimitRanger messages don't name one. Anything else is shown verbatim, attributed to nothing.
  • Quantities are shown as written and compared as exact Decimals. Computed amounts keep the limit's unit family and are never rounded.
  • Evidence:
    • stale or missing quota status is downgraded with downgrade_stale_status;
    • "no quota" is said only when the list is conclusive;
    • Events can only confirm a failure, never rule one out;
    • recreated workloads and earlier incarnations' events are kept apart.
  • Delivery:
    • Q1 core;
    • Q1 view, with a version-matrix scenario;
    • Q2: possible constraints, from the template's demand with LimitRange defaults, quota scope applicability and LimitRange checks.

The seven decisions in the spec's "Decisions" section were confirmed by the maintainer. Two changed from the proposal: CronJobs are in scope, and the command is :blocked.

Verification

Documentation only; no code changed. I checked the spec's claims against the code:

  • parse_quantity in k8s/models.py;
  • EVENTS and its retention-limited policy in core/api_versions.py;
  • downgrade_stale_status in core/capability.py;
  • the event time precedence in k8s/events.py;
  • Pulse's ReplicaFailure rule in core/pulse_rules.py;
  • the existing command aliases in ui/command.py.

Checklist

  • This closes an accepted issue in this repository that is assigned to me.
  • Tests cover the change, written to fail first where behavior changed. (No behavior changed.)
  • make check passes locally. (Documentation only; pre-commit hooks ran on commit.)
  • User-facing docs and the README match the change, or nothing user-facing changed.
  • No security invariant is weakened: writes still need a keystroke approval
    and a fail-closed audit entry, and sensitive reads are still masked.

A read-only view on a Deployment, ReplicaSet, StatefulSet, DaemonSet or Job
showing recorded FailedCreate rejections next to the namespace's
ResourceQuotas and LimitRanges, with exact quantity handling, stale-status
downgrades, and possible constraints in a second slice.

Part of #515

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 30, 2026 04:47
@hellices hellices added this to the v0.7.0 milestone Oct 30, 2026
@hellices hellices added enhancement New feature or request concern: architecture Module boundaries, dependency direction, and system structure labels Oct 30, 2026
@hellices hellices self-assigned this Oct 30, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved projection and resource-accounting gaps can miss or falsely report creation constraints, while legacy event handling can lose repetition counts and misorder failures.

7 open issues
What changed in this PR

This documentation-only PR proposes workload creation-failure explanations for #411, building on the shared capability-aware diagnostic readers and lifecycle. No runtime behavior changes.

Changes:

  • Defines a read-only workload view combining UID-matched creation failures with namespace quotas and LimitRanges.
  • Specifies projections, rejection classification, quantity handling, and evidence boundaries.
  • Splits delivery into Q1 core/view work and Q2 possible-constraint calculations, with testing requirements.
File Description
docs/​superpowers/​specs/​2026-10-30-creation-constraints-design.md Adds the proposed design, safety boundaries, delivery slices, and acceptance mapping.

Review effort: Balanced

- Keep init containers' order and restartPolicy, and runtimeClassName.
- Count and date older events by deprecatedCount and
  deprecatedLastTimestamp.
- Conflicting LimitRange defaults, unknown RuntimeClass overhead and
  Pod-level resources per resource leave demand not established or
  override it precisely.
- BestEffort follows the Pod's QoS, and StatefulSet claims are a
  conditional demand.

Part of #515

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@my-reviewer-agent my-reviewer-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이슈 1건을 인라인 코멘트로 남겼습니다.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Unresolved quota-evaluation gaps around admission-assigned defaults, mandatory resource fields, and incomplete usage data can produce incorrect constraint findings or unsupported unconstrained-template conclusions.

No open issues 🎉

4 previously missed issues

In code that hasn't changed since last review

Low severity Downgrade conclusions when quota usage entries are missing

docs/​superpowers/​specs/​2026-10-30-creation-constraints-design.md:273

Even when status.hard matches spec.hard, a missing status.used entry leaves usage unknown and can cause a status unknown for quota rejection. This rule would leave the observation conclusive without an established value for used + demand. Include incomplete usage maps in the downgrade rule and test matching hard maps with a missing used key.

Low severity Treat omitted priority class as unknown for quota applicability

docs/​superpowers/​specs/​2026-10-30-creation-constraints-design.md:329

An omitted priorityClassName does not establish that the created Pod has no class: Kubernetes' Priority admission plugin assigns the global-default PriorityClass before quota evaluation. With a default class gold and a PriorityClass In [gold] quota of zero Pods, this predicate would skip the applicable quota and permit the unconstrained-template conclusion. Since PriorityClasses are cluster-scoped and outside these reads, mark applicability and dependent negative conclusions as not established when the template's class is empty.

Low severity Use CPU and memory keys for required quota fields

docs/​superpowers/​specs/​2026-10-30-creation-constraints-design.md:337

The compute group explicitly includes hugepages, but Kubernetes' quota validationSet requires fields only for the CPU/memory request and limit keys. A quota on requests.hugepages-2Mi does not require an ordinary Pod to request hugepages, whereas this rule would report a constraint. Enumerate the CPU/memory keys instead of using the display grouping; their mandatory-field checks also need to retain per-container missingness rather than relying solely on aggregate Pod demand.

Low severity Preserve omitted versus empty storage class as unknown

docs/​superpowers/​specs/​2026-10-30-creation-constraints-design.md:349

When a claim template omits storageClassName, admission can assign an unread default class, so checking only the template's class can miss an exhausted per-class quota. Conditional claim creation does not resolve that unknown class. Preserve absent versus explicitly empty class names, and mark affected per-class checks and dependent unconstrained-template conclusions as not established.

7 resolved since last review

Review effort: Balanced

- An incomplete status.used map makes quota status stale.
- A PriorityClass selector with no priorityClassName, and a claim
  template that omits storageClassName, are not established: admission
  may assign cluster defaults the view does not read.
- Only CPU and memory keys require a field, checked per container.

Part of #515

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@hellices

Copy link
Copy Markdown
Owner Author

Copilot's "previously missed issues" from the e335bda review are addressed in f3eb872:

  • Missing status.used entries: an incomplete used map now counts as stale status and is downgraded to PARTIAL/STALE_STATUS. The test plan includes matching hard maps with a missing used key.
  • Omitted priority class: a PriorityClass selector is not established without priorityClassName, because admission may assign a global default the view does not read.
  • Required quota fields: only requests.cpu, cpu, limits.cpu, requests.memory, memory and limits.memory require a field. The check is per container, so the projection keeps which container lacks what. Hugepages and extended resources are excluded.
  • Omitted vs. empty storage class: the projection keeps them apart. When the class is omitted, per-class checks and "nothing constrains this template" are not established. An explicitly empty class matches no per-class quota.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The quota rule omits the PodLevelResources exemption and must leave results unestablished when admission behavior is unknown to avoid spurious per-container CPU or memory requirements.

1 open issue

Review effort: Balanced

Part of #515

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed to evaluate whether propagated ReplicaFailure conditions could misclassify Pod deletion failures as creation failures and admission rejections.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

Part of #515

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@hellices

Copy link
Copy Markdown
Owner Author

Copilot's 4ea6123 review asked whether a propagated ReplicaFailure condition could turn a Pod deletion failure into a creation failure. It could: the ReplicaSet controller also sets ReplicaFailure with reason FailedDelete. As of 8e0540b, a condition counts only when its status is True and its reason is FailedCreate. The test plan covers FailedCreate, FailedDelete (not shown) and status False.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed because the specified unconditional stale-status downgrade conflicts with its available-only precondition when quota reads are partial.

No open issues 🎉

Review effort: Balanced

Part of #515

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@hellices

Copy link
Copy Markdown
Owner Author

Copilot's 8e0540b review pointed out that downgrade_stale_status accepts only an AVAILABLE observation. As of 43dd21e, an AVAILABLE quotas observation is downgraded to PARTIAL/STALE_STATUS, and one that is already PARTIAL (e.g. truncated) keeps its own, already inconclusive basis. Either way the stale quota's rows say "status not yet updated". The test plan covers a stale quota in a truncated list.

@hellices
hellices requested a balanced review from Copilot October 30, 2026 05:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Seven unresolved design choices, including workload coverage and namespace-wide Event listing under bounded truncation, require maintainer agreement on scope and evidence tradeoffs before implementation.

No open issues 🎉

Review effort: Balanced

@my-reviewer-agent my-reviewer-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

검토 범위에서 보고할 이슈가 없습니다.

- CronJobs are in scope: the view follows a CronJob's own FailedCreate
  and its owned Jobs' events.
- The command is :blocked.
- The other five decisions stand as proposed.

Part of #515

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@my-reviewer-agent my-reviewer-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

검토 범위에서 보고할 이슈가 없습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

concern: architecture Module boundaries, dependency direction, and system structure enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Design workload creation-failure explanations (#411)

2 participants