labstack / labstack/onebox

fix(doctor): backup check warns about services declared persistence.mode: ephemeral

Open
#151 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Go
Stars
3
Forks
0
Avg merge
2h 40m
Merged PRs (30d)
63

Description

Summary

ob doctor's service backup check keys only off service.Backup == nil, ignoring persistence.mode: ephemeral. A service the author has explicitly declared disposable is reported as unprotected durable data on every run, and the remedy it offers cannot be followed.

Observed

A project declaring:

redis:
  driver: redis
  version: "8.8"
  # Rate-limit counters and nothing else. Losing them costs a window of
  # limiter state, which rebuilds on its own.
  persistence:
    mode: ephemeral

gets, on every ob doctor:

[WARN] backups: durable data is present and Onebox does not back it up
  [WARN] redis/backup: managed service data lives only on this host; declare
         services.redis.backup to copy it off, or accept that one disk is all there is

That single warning also drags the overall report to Onebox doctor: WARNING.

Why it is wrong

The declaration has already answered the question the check is asking, and Onebox acts on it everywhere else:

  • internal/app/services.go:122serviceIsEphemeral exists.
  • internal/app/services.go:531-539 — the renderer consults it and turns persistence off (appendonly: no, save: "" for the redis driver). The comment there is explicit: "The mode owns this decision and a driver setting cannot override it."
  • internal/app/load.go:582 — the loader refuses a service that declares volumes together with mode: ephemeral, because "an ephemeral service owns no durable volume".

So by the time doctor runs, the project has stated the data is disposable, the renderer has removed persistence, and the loader has guaranteed there is no durable volume. Doctor then warns that durable data is unprotected.

The offered remedy is worse than the warning. "Declare services.redis.backup" asks the operator to configure a backup for a service Onebox has already established owns nothing durable to back up. The advice cannot be acted on, so the only way to clear the warning is to stop meaning what the declaration says.

The standard is already stated in this file

cmd/ob/doctor.go:520-524, in the comment immediately above the loop:

// A service that declares backup is not the same as one that does not,
// and this said otherwise for both — it warned that "Onebox takes no
// backups yet" over a database archiving to an off-host repository. A
// doctor that reports a healthy thing as broken is a doctor people stop
// reading.

That is this defect, one case over. The earlier fix taught the check to distinguish "declares backup" from "does not"; it did not teach it to distinguish "has durable data" from "declared it has none."

The workload branch directly above, at doctor.go:505, already reasons about exactly this field — it tells the operator to "declare persistence: {mode: durable} to state this, or mode: ephemeral if the volume is not state." So doctor understands the concept, applies it to workloads, and skips it for services.

Cost

A warning an operator must dismiss on every run is not free. It trains people to skim past a report whose entire value is that its warnings mean something, and it is the same erosion product.md guards against from the other direction — there, silence reading as approval; here, noise reading as nothing.

Proposal

In the service loop at cmd/ob/doctor.go:530, consult the ephemeral declaration before warning. Something like:

  • serviceIsEphemeral(service)doctorPass, message along the lines of "declared ephemeral; Onebox renders no durable volume and takes no backup, which is what the declaration asks for."
  • service.Backup == nil and not ephemeral → the existing warning, unchanged.
  • service.Backup != nil → the existing pass, unchanged.

Reporting it as a pass rather than omitting it keeps the inventory complete, which seems closer to the project's habit of naming what it does not own rather than staying quiet about it.

serviceIsEphemeral is currently unexported in internal/app; whether doctor consults an exported helper or the field directly is a taste call I have no basis for.

Notes

  • Reproduced against runner v2026.8.21 (aafb110), against 8500ab7 of this repository.
  • Same run reports [WARN] qdrant/backup — that one appears correct: qdrant declares no backup and no persistence mode, so nothing has claimed the data is disposable.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at the service loop in cmd/ob/doctor.go:530 and read serviceIsEphemeral in internal/app/services.go, including its use in the renderer and loader. Reproduce the ob doctor output with a service using persistence.mode: ephemeral, then add coverage for the service check. Done means ephemeral services report a pass without the backup warning, while services without persistence mode or backup retain the existing warning.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
cli, devops, infrastructure
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.