Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions architecture/supported-subset.md
Original file line number Diff line number Diff line change
Expand Up @@ -718,15 +718,20 @@ boolean-typed exception in this group, validated as an actual bool like
string raise, matching `docker compose config` v5.1.2 — these used to be
silently accepted (`is_number`/`interval_seconds`'s bare-number fallback),
producing a script for a file Docker itself refuses.
`interval` is parsed to whole seconds by `interval_seconds`, which floors
at 1 second: the interval only paces the script's `podman healthcheck run`
polling loop, which has no sub-second resolution, so `"500ms"` and `"0"`
both poll once a second. `interval` additionally rejects compound
durations (`"1h30m"`) and the hour unit (`"1h"`) that `timeout` and
`start_period` accept — a deliberate, deferred limitation
(`planning/deferred.md`), not a grammar gap: each raises rather than being
silently truncated or misinterpreted. An explicit `null` (or an absent
`interval`) defaults to 1 second.
`interval` is parsed to whole seconds by `interval_seconds`, which never
reaches podman (it only paces compose2pod's own `wait_healthy` polling
loop), so it honors the full compose-go duration grammar rather than the
narrower one `timeout`/`start_period` forward to podman's `--health-*`
flags: a signed sequence of `<number><unit>` components, units
`ns`/`us`/`µs`/`ms`/`s`/`m`/`h`/`d`/`w` (`d` = 86400s, `w` = 604800s —
compose-go additions over Go's `time.ParseDuration`), compound
(`"1h30m"`), fractional (`"1.5d"`), and signed (`"-1h"`) forms — measured
against `docker compose config` v5.1.2. Whitespace and uppercase units are
refused, matching Docker (`" 1h "`, `"1H"` both raise). It floors at 1
second: the polling loop has no sub-second resolution, so `"500ms"` and
`"0"` both poll once a second, and a negative result (`"-1h"`) also floors
to 1 rather than producing a nonsensical negative interval. An explicit
`null` (or an absent `interval`) defaults to 1 second.
- **`retries`:** an int64 count (`values.validate_count`): a native number
casts leniently (`0.5` is accepted), but a string form must be a strict
integer — no decimal point, no exponent (`"1.5"`, `"30s"` raise). A mapping
Expand Down
62 changes: 41 additions & 21 deletions compose2pod/healthcheck.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
"""Healthcheck translation: compose healthcheck -> podman --health-* values."""

import json
import math
import re
from typing import Any

from compose2pod.exceptions import UnsupportedComposeError
Expand Down Expand Up @@ -39,32 +41,50 @@ def health_cmd(test: object) -> str | None:
raise UnsupportedComposeError(msg)


# compose-go's duration grammar (measured vs `docker compose config` v5.1.2): a
# signed sequence of <number><unit> components. Broader than Go's
# time.ParseDuration -- compose-go adds `d` (days) and `w` (weeks). `interval`
# is converted to seconds to pace the wait_healthy loop and never reaches podman,
# so compose2pod can honor the full set -- unlike timeout/start_period, which
# flow to podman's Go-parser --health-* flags (see values._DURATION).
_UNITS = "ns|us|µs|ms|s|m|h|d|w"
_INTERVAL_DURATION = re.compile(rf"^[+-]?(?:[0-9]+(?:\.[0-9]+)?(?:{_UNITS}))+\Z")
_DURATION_COMPONENT = re.compile(rf"([0-9]+(?:\.[0-9]+)?)({_UNITS})")
_UNIT_SECONDS: dict[str, float] = {
"ns": 1e-9,
"us": 1e-6,
"µs": 1e-6,
"ms": 1e-3,
"s": 1.0,
"m": 60.0,
"h": 3600.0,
"d": 86400.0,
"w": 604800.0,
}


def interval_seconds(duration: object) -> int:
"""Compose duration ('1s', '2m', '500ms', '0') to whole seconds, minimum 1.
"""Compose healthcheck `interval` to whole seconds, minimum 1.

Docker's own field is a Go duration *string*: a native number and a
unitless string ('30') are both refused ("missing unit in duration"),
except the literal '0' zero-duration special case -- measured against
`docker compose config` v5.1.2. Compound durations ('1h30m') and the hour
unit ('1h') are refused here even though Docker accepts them; that is a
deliberate, deferred limitation (planning/deferred.md), not part of the
grammar this function otherwise enforces.
The interval paces compose2pod's `wait_healthy` polling loop and never
reaches podman, so it accepts the full compose-go duration grammar (all
units incl. `d`/`w`, compound like `1h30m`, fractional, sign) -- measured
against `docker compose config` v5.1.2. Whitespace and uppercase units are
refused, as Docker refuses them. `None` and the literal `"0"` default to 1;
a native number, a unitless string, or a value overflowing to infinity raises.
"""
if duration is None:
return 1
msg = f"unsupported healthcheck interval {duration!r} (use forms like '30s', '2m', '500ms')"
msg = f"unsupported healthcheck interval {duration!r} (use forms like '30s', '2m', '1h30m', '500ms')"
if not isinstance(duration, str):
raise UnsupportedComposeError(msg)
text = duration.strip()
if text == "0":
if duration == "0":
return 1
try:
if text.endswith("ms"):
return max(int(float(text[:-2]) / 1000), 1)
if text.endswith("m"):
return max(int(float(text[:-1])) * 60, 1)
if text.endswith("s"):
return max(int(float(text.removesuffix("s"))), 1)
except (ValueError, OverflowError):
raise UnsupportedComposeError(msg) from None
raise UnsupportedComposeError(msg)
if not _INTERVAL_DURATION.match(duration):
raise UnsupportedComposeError(msg)
total = sum(float(num) * _UNIT_SECONDS[unit] for num, unit in _DURATION_COMPONENT.findall(duration))
if not math.isfinite(total):
raise UnsupportedComposeError(msg)
if duration.startswith("-"):
total = -total
return max(int(total), 1)
112 changes: 112 additions & 0 deletions planning/changes/2026-07-16.03-healthcheck-duration-grammar.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
---
summary: Rewrite `healthcheck.interval_seconds` to parse the full compose-go duration grammar (all units incl. `d`/`w`, compound, fractional, sign) — accepting `interval: 1h`/`1h30m`/`1d` the way `docker compose config` v5.1.2 does — and drop its `.strip()`, closing a latent hard-rule violation where padded/whitespace durations (`" 1h "`) were accepted although Docker rejects them.
---

# Design: full compose-go duration grammar for interval_seconds

## Summary

`healthcheck.interval_seconds` parses a Compose healthcheck `interval` to whole
seconds (it paces compose2pod's own `wait_healthy` polling loop and is never
passed to podman). Today it only understands `ms`/`m`/`s`, so `interval: 1h` and
`1h30m` raise even though Docker accepts them (a cataloged over-reject). This
rewrites it to parse the complete compose-go duration grammar → seconds, and
**drops its `.strip()`**, which closes a pre-existing hard-rule violation:
padded durations (`interval: " 1h "`) were accepted although Docker rejects all
surrounding/internal whitespace.

## Motivation

Compound and hour healthcheck durations are a cataloged over-reject
(`planning/deferred.md`; conformance corpus `healthcheck_compound_duration`,
`healthcheck_hour_duration`). Measured against `docker compose config` v5.1.2,
`interval`'s grammar is compose-go's duration parser — broader than Go's
`time.ParseDuration`: it adds `d` (days, 24h) and `w` (weeks, 168h) on top of
`ns/us/µs/ms/s/m/h`, plus compound (`1h30m`), fractional (`1.5d`), and sign
(`-1h`). `interval` is converted to seconds internally, so compose2pod can honor
the whole set.

Separately measured: Docker rejects `" 1h "`, `"1h "`, `" 1h"`, `"1 h"`,
`"1h 30m"`, and `"1H"` — any whitespace or uppercase. The current function calls
`duration.strip()` before parsing, so `interval: " 5s "` is accepted today while
Docker rejects it. Because `interval` is nested under `healthcheck`, the flat
conformance matrix (top-level keys only) never probed it, so this false green —
a document compose2pod accepts and Docker refuses, violating the hard rule
`accepted(compose2pod) ⊆ accepted(docker)` — went uncaught.

## Design

**`compose2pod/healthcheck.py`** — rewrite `interval_seconds`. One units
alternation is single-sourced into both the anchored validator regex and the
component parser, beside a `unit → seconds` map:

```python
_UNITS = "ns|us|µs|ms|s|m|h|d|w"
_INTERVAL_DURATION = re.compile(rf"^[+-]?(?:[0-9]+(?:\.[0-9]+)?(?:{_UNITS}))+$")
_DURATION_COMPONENT = re.compile(rf"([0-9]+(?:\.[0-9]+)?)({_UNITS})")
_UNIT_SECONDS = {"ns": 1e-9, "us": 1e-6, "µs": 1e-6, "ms": 1e-3,
"s": 1.0, "m": 60.0, "h": 3600.0, "d": 86400.0, "w": 604800.0}
```

`interval_seconds`:
- `None` → 1 (unchanged); non-string → raise (unchanged).
- **No `.strip()`** — the raw value is matched, so any whitespace fails the
anchored regex, as Docker does.
- `"0"` → 1 (Docker normalizes `"0"` → `0s`; the zero special case, unchanged).
- Otherwise require `_INTERVAL_DURATION.match`; sum
`float(number) * _UNIT_SECONDS[unit]` over `_DURATION_COMPONENT.findall`;
negate if the string starts with `-`; return `max(int(total), 1)`.
- The refusal message gains a `1h30m` example.

Lowercase-only units (no `H`) and no whitespace fall out of the anchored regex,
matching Docker. A negative interval floors to 1 via `max` (Docker accepts it;
the value is nonsensical as a poll interval, and flooring is the same behavior
sub-second durations already get).

## Non-goals

- **`timeout`/`start_period`** — unchanged. They are emitted verbatim to podman's
`--health-timeout`/`--health-start-period`, whose Go `ParseDuration` has no
`d`/`w`, so their narrower `values._DURATION` grammar is a *legitimate* refusal
under rule two (`decisions/2026-07-14-docker-rejection-parity.md`), not this
over-reject.
- **The `wait_healthy` trailing `sleep`** — a large interval (e.g. `1w`) makes the
loop sleep a long time after a final failed check. This is pre-existing (already
reachable via `interval: 3000m`) and orthogonal to accepting the grammar; not
touched here.
- **Extending `_DURATION`** (timeout/start_period) to `d`/`w` — would over-accept
what podman refuses; see the first non-goal.

## Testing

TDD, Docker as oracle.

- `tests/test_healthcheck.py`: `interval_seconds` returns correct seconds for
`"1h"`→3600, `"1h30m"`→5400, `"1d"`→86400, `"1w"`→604800, `"1.5d"`→129600,
`"2h45m30s500ms"`, `"-1h"`→1 (floored), `"500ms"`→1, `"5s"`→5, `"2m"`→120;
`None`→1; `"0"`→1. Rejects `" 1h "`, `"1h "`, `" 1h"`, `"1 h"`, `"1h 30m"`,
`"1H"`, `"somevalue"`, `"30"` (unitless), and a bare int / non-string.
- **The false-green guard**: a new conformance corpus document
`healthcheck_interval_padded_whitespace.yaml` with `interval: " 1h "` — it goes
red against the hard rule *before* the fix (Docker rejects, compose2pod
accepts) and green after (both reject). Confirms the fix closes the violation
and guards against regression.
- Conformance: `healthcheck_compound_duration` and `healthcheck_hour_duration`
flip over-reject → both-accept.
- Promote `architecture/supported-subset.md` (interval grammar) and remove the
compound/hour bullet from `planning/deferred.md`.
- `just test-ci` @ 100% coverage, `just lint-ci`, `just check-planning`,
`just test-conformance`.

## Risk

- **A valid document breaks because padding is now rejected** (low × low): only
a document Docker *also* rejects (a padded interval) changes verdict — that is
the false-green fix, not a regression. No Docker-valid interval is refused.
- **Unit-alternation ordering** (`ms` vs `s`, `us` vs `µs`) mis-tokenizes
(low × med): the alternation order mirrors the existing `values._DURATION`
(which already ships `ns|us|µs|ms|s|m|h`); `d`/`w` are appended and collide
with nothing. Covered by the per-unit unit tests.
- **`wait_healthy` sleeps long on a huge accepted interval** (low × low):
pre-existing, documented above as a non-goal; the value pacing the loop is the
intended meaning of a large interval.
12 changes: 3 additions & 9 deletions planning/deferred.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,6 @@ subset, not a bug and not a design position. Each item below is a **form** of a
capability compose2pod already supports, refused only because the parser was
never written. Every one was measured against `docker compose config` v5.1.2.

- **Compound and hour healthcheck durations.** `interval: 1h30m` and `1h` raise;
Docker accepts both. `1h30m` is 5400 seconds and the value only paces the
script's polling loop, so podman can honor it. `architecture/supported-subset.md`
presents this refusal as a safety choice ("rather than being silently
truncated"); under rule two it is simply an unfinished parser.
- **Long-form `volumes`.** The mapping form raises; podman expresses it with
`--mount`.
- **`volumes: ["a"]`** — a colon-less relative entry. Docker accepts it;
Expand All @@ -39,10 +34,9 @@ never written. Every one was measured against `docker compose config` v5.1.2.
tilde-bind-mount fix that discovered it.

**Revisit trigger:** a user reports a compose file that `docker compose` runs and
compose2pod refuses — most likely the compound/hour healthcheck duration or
long-form `volumes`, since both are common in hand-written and generated compose
files alike. The conformance harness reports these as `over-reject`, so they
stay visible rather than forgotten.
compose2pod refuses — most likely long-form `volumes`, a common form in
hand-written and generated compose files alike. The conformance harness reports
these as `over-reject`, so they stay visible rather than forgotten.

## Non-target `depends_on` graph is not validated outside the target's closure

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
services:
app:
image: nginx
healthcheck:
test: ["CMD", "true"]
interval: " 5s "
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
services:
app:
image: nginx
healthcheck:
test: ["CMD", "true"]
interval: |
5s
30 changes: 30 additions & 0 deletions tests/conformance/test_corpus.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,33 @@ def test_volume_tilde_bind_is_no_longer_an_over_rejection(
"""
path = Path(__file__).parent / "corpus" / "volume_tilde_bind_no_declaration.yaml"
assert assert_rule(yaml.safe_load(path.read_text())) == "both-accept"


def test_healthcheck_hour_duration_is_no_longer_an_over_rejection(
assert_rule: Callable[[dict[str, Any]], str],
) -> None:
"""`interval_seconds` now parses the full compose-go duration grammar, including `h`.

Same reasoning as the over-rejection tests above: the generic corpus run alone
would stay green even pre-fix, filing `healthcheck_hour_duration` under the
allowed 'over-reject' verdict instead of catching a regression. The stronger
claim -- both oracles ACCEPT `interval: 1h` -- needs this dedicated assertion
on the verdict itself.
"""
path = Path(__file__).parent / "corpus" / "healthcheck_hour_duration.yaml"
assert assert_rule(yaml.safe_load(path.read_text())) == "both-accept"


def test_healthcheck_compound_duration_is_no_longer_an_over_rejection(
assert_rule: Callable[[dict[str, Any]], str],
) -> None:
"""`interval_seconds` now parses compound compose-go durations such as `1h30m`.

Same reasoning as the over-rejection tests above: the generic corpus run alone
would stay green even pre-fix, filing `healthcheck_compound_duration` under the
allowed 'over-reject' verdict instead of catching a regression. The stronger
claim -- both oracles ACCEPT `interval: 1h30m` -- needs this dedicated assertion
on the verdict itself.
"""
path = Path(__file__).parent / "corpus" / "healthcheck_compound_duration.yaml"
assert assert_rule(yaml.safe_load(path.read_text())) == "both-accept"
42 changes: 41 additions & 1 deletion tests/test_healthcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@

_FIVE_SECONDS = 5
_ONE_HUNDRED_TWENTY_SECONDS = 120
_ONE_HOUR_SECONDS = 3600
_ONE_HOUR_THIRTY_MINUTES_SECONDS = 5400
_ONE_DAY_SECONDS = 86400
_ONE_WEEK_SECONDS = 604800
_ONE_AND_A_HALF_DAYS_SECONDS = 129600
_COMPOUND_DURATION_SECONDS = 2 * _ONE_HOUR_SECONDS + 45 * 60 + 30


class TestHealthCmd:
Expand Down Expand Up @@ -75,10 +81,44 @@ def test_bare_zero_string_floors_to_one(self) -> None:
assert interval_seconds("0") == 1

def test_unparseable_interval_raises(self) -> None:
for bad in ("1h30m", "abc", "5x"):
for bad in ("abc", "5x"):
with pytest.raises(UnsupportedComposeError, match="unsupported healthcheck interval"):
interval_seconds(bad)

def test_hour_and_compound_and_larger_units(self) -> None:
assert interval_seconds("1h") == _ONE_HOUR_SECONDS
assert interval_seconds("1h30m") == _ONE_HOUR_THIRTY_MINUTES_SECONDS
assert interval_seconds("1d") == _ONE_DAY_SECONDS
assert interval_seconds("1w") == _ONE_WEEK_SECONDS
assert interval_seconds("1.5d") == _ONE_AND_A_HALF_DAYS_SECONDS
assert interval_seconds("2h45m30s500ms") == _COMPOUND_DURATION_SECONDS

def test_negative_interval_floors_to_one(self) -> None:
# Docker accepts `-1h`; a negative poll interval is nonsensical, floored to 1.
assert interval_seconds("-1h") == 1

def test_whitespace_and_uppercase_rejected(self) -> None:
# Measured: Docker rejects any whitespace or uppercase in a duration.
# " 5s "/"5s "/" 2m " are the real strip-defect demonstrators: the old
# `.strip()`-based parser accepted them (strips to "5s"/"2m", both valid
# units), silently diverging from Docker, which refuses any whitespace.
for bad in (" 1h ", "1h ", " 1h", "1 h", "1h 30m", "1H", " 5s ", "5s ", " 2m "):
with pytest.raises(UnsupportedComposeError, match="unsupported healthcheck interval"):
interval_seconds(bad)

def test_trailing_newline_rejected(self) -> None:
# A YAML block scalar (`interval: |\n 5s\n`) parses to "5s\n". Python's
# `$` (non-MULTILINE) matches before a trailing newline, so a naive regex
# wrongly accepts it; Docker refuses it ('unknown unit "s\n"').
for bad in ("5s\n", "1h\n", "1h30m\n"):
with pytest.raises(UnsupportedComposeError, match="unsupported healthcheck interval"):
interval_seconds(bad)

def test_overflowing_duration_raises_not_crashes(self) -> None:
# A giant integer floats to inf; must raise cleanly, not OverflowError.
with pytest.raises(UnsupportedComposeError, match="unsupported healthcheck interval"):
interval_seconds("1" + "0" * 400 + "s")

def test_non_finite_interval_raises(self) -> None:
# inf/nan are valid YAML floats; they must be refused, not crash raw.
for bad in (float("inf"), float("nan"), "1e400"):
Expand Down
2 changes: 1 addition & 1 deletion tests/test_parsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ def test_non_mapping_healthcheck_raises(self) -> None:
validate({"services": {"app": {"image": "x", "healthcheck": ["CMD", "true"]}}})

def test_unparseable_healthcheck_interval_raises(self) -> None:
compose = {"services": {"app": {"image": "x", "healthcheck": {"test": "true", "interval": "1h30m"}}}}
compose = {"services": {"app": {"image": "x", "healthcheck": {"test": "true", "interval": "abc"}}}}
with pytest.raises(UnsupportedComposeError, match="unsupported healthcheck interval"):
validate(compose)

Expand Down