Repository navigation
🐛 fix: align self-managed startup probe budgets - #14490
Conversation
Give the self-managed manifest and manual template the hosted ten-minute startup budget and guard their timing values against drift. Hive-Run: hivecommons#14425 Hive-Plan: align-self-managed-startup-budget Hive-Spec: hivecommons#14425#startup-probe-parity Signed-off-by: thatguywob <thatguywob@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@thatguywob is auto-promoted for contributor-lane merging on this PR: the last 5 merged PRs by this author in this repository have no revert or sentinel-alert evidence. CI, reviewer verdict, diff fences, soak, holds and branch protection still apply. |
|
@thatguywob this PR is back in your court ( Failing checks:
Reply here or push a new head to this branch (same sign-off); CI and the review swarm re-run automatically and the PR returns to the normal path. Queued in your relay fix queue (1 waiting): fixes are handed out before new work as soon as you have capacity. |
Signed-off-by: thatguywob <thatguywob@users.noreply.github.com>
Signed-off-by: thatguywob <thatguywob@users.noreply.github.com>
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of No conflicting open PRs or same-file overlaps were found. |
There was a problem hiding this comment.
Looks correct to me. Read against the tree at the merge-base:
src/pkg/hub/spoke/self_startup_probe.go:12-15derivesHostedStartupProbeFailureThreshold = (600 - 10) / 5 = 118, so the new literal118insrc/deploy/k8s/deployment.yaml:206andsrc/docs/manual-provisioning.md:1415matches hosted provisioning exactly, and the10s + 118 x 5s = 600scomments are arithmetically right.- The new
TestSelfManagedStartupProbesMatchHostedConstantsreuses the existingprobeInthelper (saas_provision_startup_probe_test.go:21) and compares against the samehostedStartupProbe*aliases thatsaas_provision.go:2290-2295uses to render the hosted template, so a future budget change fails this test rather than silently drifting again. Both files have exactly onestartupProbe:block, so thestrings.Cutslicing is unambiguous.test (hub 1-4/4)are green on this head.
One scope note for the maintainer, not a defect: commits 215c571 and be87431 change src/deploy/probe_podman_rootless_netadmin.sh (store under /var/tmp, keep podman pull stderr visible). Neither is mentioned in the body or in #14425; they read as CI-probe hardening that rode along. Harmless on their own, but the changelog fragment does not cover them.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1
There was a problem hiding this comment.
The startup-probe change is correct: failureThreshold: 118 in src/deploy/k8s/deployment.yaml:206 and src/docs/manual-provisioning.md:1415 matches spoke.HostedStartupProbeFailureThreshold = (600 − 10) / 5 = 118 (src/pkg/hub/spoke/self_startup_probe.go:12-15). The new TestSelfManagedStartupProbesMatchHostedConstants cuts the single startupProbe: … livenessProbe: block (each file has exactly one occurrence), parses it with gopkg.in/yaml.v3 (already a hub test dependency, e.g. node_health_rbac_reconcile_test.go), and compares against the same hub-side constants the hosted template renders (saas_provision.go:2290-2295), so drift in either direction fails CI. Changelog fragment name follows the fixed-<slug>.md guard. Nothing wrong here.
Scope: two commits in this PR are unrelated to #14425 and the body does not mention them.
- 215c571 (
src/deploy/probe_podman_rootless_netadmin.sh:149) stops discardingpodman pulloutput so pull failures are diagnosable. - be87431 (
src/deploy/probe_podman_rootless_netadmin.sh:96) moves the private rootless store frommktemp -d -tto-p /var/tmp.
Both look reasonable on their own (the rootful probe already uses /var/tmp, as the new comment says), but they change a podman qualification script, not startup probes, and a reader of this PR's title/body — or a future git blame on #14425 — would never find them. Suggestion: either split them into their own PR, or add a short paragraph to the body (and ideally a separate changelog line) describing them so the change set is honest about what it ships. I have not seen an issue that tracks them; if one exists, citing it would be enough.
(Not a defect, so not blocking from my side; flagging for the maintainer who merges to decide whether the extra commits ride along.)
— hive: agent=reviewer backend=copilot model=claude-fable-5.1
|
@thatguywob this PR is back in your court ( Review asks:
Reply here or push a new head to this branch (same sign-off); CI and the review swarm re-run automatically and the PR returns to the normal path. Queued in your relay fix queue (1 waiting): fixes are handed out before new work as soon as you have capacity. |
Signed-off-by: thatguywob <thatguywob@users.noreply.github.com>
022bd1f
|
There was a problem hiding this comment.
The probe-budget change looks correct to me.
Verified against the tree at 022bd1f:
src/pkg/hub/spoke/self_startup_probe.go:11-15—HostedStartupProbeFailureThreshold = (600 − 10) / 5 = 118, sofailureThreshold: 118withinitialDelaySeconds: 10/periodSeconds: 5indeploy/k8s/deployment.yaml:202-206anddocs/manual-provisioning.md:1409-1413gives exactly the hosted ten-minute budget #14425 asks for.TestSelfManagedStartupProbesMatchHostedConstantscompares both templates to thehostedStartupProbe*aliases insaas_provision.go:2290-2295, which are thespokeconstants, so a future budget change breaks the test rather than silently drifting. Each file has exactly onestartupProbe:occurrence, so thestrings.Cutanchor is unambiguous.
One scope note for the maintainer: the two hunks in src/deploy/probe_podman_rootless_netadmin.sh (store under /var/tmp, un-silenced podman pull) are unrelated to #14425 and carry their own un-numbered fragment changelog.d/fixed-rootless-probe-storage.md. They are low-risk and match what probe_podman_rootful_netadmin.sh:65, probe_podman_ipv6_egress.sh:73 and probe_podman_selinux_avc.sh:116 already do, so I would not block on them — but if you prefer one change per PR they split cleanly.
Could not run go test from this host; CI will confirm the new test.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Decision needed: @clubanderson should decide whether the unrelated rootless-probe script changes (private store moved to /var/tmp, pull errors no longer silenced) may land bundled with the #14425 startup-probe budget fix in this PR, or must be split into their own PR.
This PR carries sentinel-alert and edits deploy manifests and a CI probe script, so I am routing it rather than judging it alone. The facts, verified against v5:
The #14425 change is correct. src/pkg/hub/spoke/self_startup_probe.go:12-15 defines HostedStartupBudget = 10m, initial delay 10s, period 5s, and derives failureThreshold = (600-10)/5 = 118. src/deploy/k8s/deployment.yaml:206 and src/docs/manual-provisioning.md:1415 move from 30 to exactly that value, and the new TestSelfManagedStartupProbesMatchHostedConstants parses both files' startupProbe block with yaml.v3 (already in src/go.mod:30) and compares all three fields to the hub's constants, so future drift fails CI. The probeInt helper it uses already exists at saas_provision_startup_probe_test.go:21.
Scope beyond #14425. The PR also changes src/deploy/probe_podman_rootless_netadmin.sh: the private image store moves from mktemp -d -t … to mktemp -d -p /var/tmp … (:94-96) and podman pull stderr is no longer discarded (:149-151). This script is the body of the podman-rootless-lane workflow (.github/workflows/podman-rootless-lane.yml:235). The body says these were carried over from commits 215c571f3/be8743178 after the lane failed on this PR. They are sensible and the lane is green on this head, but they are unrelated to the probe budget and have their own changelog fragment (changelog.d/fixed-rootless-probe-storage.md), so this PR ships two changes under one Fixes #14425.
Author is a Hive contributor agent (author/clanker, role/trusted), not a human adding themselves to anything; no OWNERS, workflow or permission files are touched. I have not changed or closed anything.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Reviewed (intent-alignment) — the fix for #14425 is complete: src/deploy/k8s/deployment.yaml:206 and src/docs/manual-provisioning.md:1415 now set failureThreshold: 118, matching spoke.HostedStartupProbeFailureThreshold (src/pkg/hub/spoke/self_startup_probe.go:12-15, (600−10)/5), and TestSelfManagedStartupProbesMatchHostedConstants pins both templates to the hosted constants.
One advisory, non-blocking note: src/deploy/probe_podman_rootless_netadmin.sh:94-96,149-151 and changelog.d/fixed-rootless-probe-storage.md are unrelated to #14425 (the body says they were kept to get the rootless CI probe green). The change itself matches the sibling probes (probe_podman_rootful_netadmin.sh:65 already uses /var/tmp), so it is low-risk — but it would be cleaner as its own PR so the #14425 fix can be reverted/bisected independently.
— hive: agent=adjudicator backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Hive filed review findings that do not block this PR as backlog items:
Cap: 10 backlog item(s) per PR per day. |
|
Needs a decision, not a merge:
Head: verdict requires_human @022bd1f, 62/62 checks green, DCO yes, sentinel alert, soak 6m0s of 30m0s. How to release it:
Clear the cause and the hive re-evaluates on its next pass; the stage moves to |
clubanderson
left a comment
There was a problem hiding this comment.
/hive decision approve — the deployment.yaml failureThreshold (30→118) and probe-script edits are intended: they align the startup budget with spoke.HostedStartupBudget, keep image layers off /tmp, and surface pull errors. Land as one PR; no split needed. Sentinel alert reviewed and cleared.
|
LGTM label has been added. DetailsGit tree hash: 1ffadd7154874e5cdf1523163113fdea12e4e3ed |
|
Merged by hive contributor-lane auto-merge: CI green for ≥30m, reviewer verdict not required, @thatguywob was auto-promoted after 5 clean merged PRs in this repository. |
Self-managed Kubernetes deployments and the manual provisioning template still allowed only 160 seconds for first boot. Set their startup probe failure threshold to 118, matching the hosted ten-minute budget, and update the timing comments.
Add a regression test comparing both templates' initial delay, period, and failure threshold with the hosted constants, plus a changelog fragment.
The rootless Podman qualification check initially failed while pulling the image. This PR also retains the CI fixes from commits
215c571f3andbe8743178: preserve image-pull diagnostics and move the private image store to/var/tmpto avoid quota-limited/tmpstorage. A separate changelog fragment documents these probe changes.Validation: Go formatting is clean. Tests were not run locally per AGENTS.md; CI runs the suite. The regression test would reject the previous threshold of 30.
Fixes #14425
— hive: backend=codex model=gpt-6.1-sol
🐝 Hive Agent:
contributor| SHA:f5212ffb0