Skip to content

Wait for each object's own manifest-id #106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ps-pufferfish merged 2 commits into from
Sep 28, 2026
Merged

Wait for each object's own manifest-id #106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8dd451b
Wait for each object's own manifest-id
ps-pufferfish Sep 24, 2026
0079b98
Stop naming a change by its deploy-id
ps-pufferfish Sep 24, 2026
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
24 changes: 12 additions & 12 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ The `200` response body reports the diff and the comment:
}
```

Every generated manifest carries a deploy-id annotation, and a change to a shared
label or annotation rewrites every manifest that has it, so the comment
summarizes those repeated metadata-only changes and leaves them out of the diff it
shows. `diffs[].diff` is always the unabridged diff, which is where the comment
Every generated manifest carries a manifest-id annotation that changes whenever
the manifest does, and a change to a shared label or annotation rewrites every
manifest that has it, so the comment leaves the ids out of the diff it shows and
summarizes those repeated metadata-only changes instead of showing them. `diffs[].diff` is always the unabridged diff, which is where the comment
points a reader who needs the part it left out.

`Accept: text/event-stream` works here too, with the same event shapes as
Expand Down Expand Up @@ -287,8 +287,8 @@ The stream carries four kinds of event:

Every event carries a `message` written to be read on its own, so a client can
print the stream as it arrives without knowing any payload shape. What a message
leaves out is in `detail`: the temporary workspace, the full commit hashes, the
deploy-id, every changed object where the message named the first few. A step
leaves out is in `detail`: the temporary workspace, the full commit hashes,
every changed object where the message named the first few. A step
whose only interest is to whoever debugs relcoord — the temporary directory it
works in, the manifests commit named by the push lines either side of it — is
logged rather than streamed, which is why there is no `workspace` or `commit`
Expand Down Expand Up @@ -439,15 +439,15 @@ change has actually materialised there, and logs it when it has.

What it waits for comes from `manifest-builder`, which reports the Kubernetes
objects each commit touched — kind, namespace where the object has one, and
name — and stamps every manifest it wrote with a `noa.re/deploy-id` annotation
identifying that generation. relcoord reports both in the `changed-objects`
progress event and in the `outputs` of the change response, and then waits for
each created or modified object to carry that deploy-id and each removed object
to be gone. Objects are waited for with a list narrowed to the object's name
name — and stamps every manifest it wrote with a `noa.re/manifest-id`
annotation, a hash of that object's content. relcoord reports the objects in the
`changed-objects` progress event and in the `outputs` of the change response,
and then waits for each created or modified object to carry its manifest-id and
each removed object to be gone. Objects are waited for with a list narrowed to the object's name
followed by a watch, so a rollout is observed as it happens rather than polled
for.

Carrying the deploy-id says the write landed, not that it took effect, so for the
Carrying the manifest-id says the write landed, not that it took effect, so for the
kinds that roll a write out to pods the wait goes further. A Deployment has to
have had its new generation observed, its new ReplicaSet scaled up, the old ones
drained and the new pods become available; a StatefulSet has to have had its new
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ dependencies = [
"dulwich[merge]",
"httpx",
"hypercorn",
"manifest-builder==0.7.14",
"manifest-builder==0.8.0",
"pyjwt[crypto]",
"pyyaml",
"starlette",
Expand Down
1 change: 0 additions & 1 deletion src/relcoord/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,6 @@ def _change_result_payload(result: object) -> dict[str, Any]:
"directory": str(output.directory),
"generated": output.generated_count,
"cluster": output.cluster,
"deploy_id": output.deploy_id,
"created_or_modified": object_ref_payloads(output.created_or_modified),
"removed": object_ref_payloads(output.removed),
"rollout": getattr(output, "rollout", None),
Expand Down
124 changes: 33 additions & 91 deletions src/relcoord/change.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import tempfile
import threading
import time
from collections.abc import Callable, Iterable, Sequence
from collections.abc import Callable, Iterable, Mapping, Sequence
from dataclasses import dataclass, field
from io import BytesIO
from pathlib import Path
Expand Down Expand Up @@ -157,8 +157,7 @@ class DeploymentDetector(Protocol):
def wait_for_success(
self,
*,
deploy_id: str,
created_or_modified: set[Any],
created_or_modified: Mapping[Any, str],
removed: set[Any],
) -> None: ...

Expand Down Expand Up @@ -191,7 +190,6 @@ class ChangeResult:
deploy_config: Path
manifests_checkout: Path
generated_count: int
deploy_id: str | None = None
outputs: tuple[OutputResult, ...] = ()


Expand All @@ -201,17 +199,16 @@ class OutputResult:

``created_or_modified`` and ``removed`` name the Kubernetes objects the
commit touched, which manifest-builder reads back out of the manifests it
wrote, and ``deploy_id`` is the value of the noa.re/deploy-id annotation it
stamped on each of them. Together they are what deployment detection waits
for, and what a caller needs to follow the change into ``cluster``.
wrote. Deployment detection waits for each created or modified object to
carry the noa.re/manifest-id manifest-builder stamped on it, and for each
removed one to be gone.
"""

name: str
repository: str
directory: Path
manifests_checkout: Path
generated_count: int
deploy_id: str | None = None
cluster: str | None = None
created_or_modified: tuple[KubernetesObjectRef, ...] = ()
removed: tuple[KubernetesObjectRef, ...] = ()
Expand Down Expand Up @@ -400,7 +397,6 @@ def report(phase: str, message: str, **detail: Any) -> None:
deploy_config=deploy_config,
manifests_checkout=output_results[0].manifests_checkout,
generated_count=total_generated,
deploy_id=output_results[0].deploy_id,
outputs=tuple(output_results),
)
except ChangeProcessingError:
Expand Down Expand Up @@ -516,12 +512,6 @@ def _generate_output(
**selection,
)
generated = _written_paths(generation_result)
deploy_id = _deploy_id(generation_result)
if self.detect_deployment and deploy_id is None:
raise DeploymentDetectionError(
"manifest-builder did not return a deploy_id; "
"deployment detection requires git-backed generation"
)
created_or_modified = _sorted_refs(generation_result.created_or_modified)
removed_refs = _sorted_refs(generation_result.removed)
logger.info(
Expand All @@ -542,21 +532,19 @@ def _generate_output(
)
if created_or_modified or removed_refs:
logger.info(
"change step 5/7: output %s in cluster %s changed %s (deploy-id %s)",
"change step 5/7: output %s in cluster %s changed %s",
output.name,
_cluster_name(output) or "<none>",
", ".join(
_format_ref(ref) for ref in (*created_or_modified, *removed_refs)
),
deploy_id or "<none>",
)
report(
"changed-objects",
_changed_objects_message(output, created_or_modified, removed_refs),
output=output.name,
repository=output.repository,
cluster=_cluster_name(output),
deploy_id=deploy_id,
created_or_modified=object_ref_payloads(created_or_modified),
removed=object_ref_payloads(removed_refs),
)
Expand All @@ -567,7 +555,6 @@ def _generate_output(
directory=output.directory,
manifests_checkout=manifests_checkout,
generated_count=len(generated),
deploy_id=deploy_id,
cluster=_cluster_name(output),
created_or_modified=created_or_modified,
removed=removed_refs,
Expand Down Expand Up @@ -649,27 +636,25 @@ def _detect_stage_deployments(
]
observed: list[tuple[str, float]] = []
for output, generation_result in deployed:
deploy_id = _deploy_id(generation_result)
connection = self._connection_for(output)
report(
"deployment-detection",
f"waiting for {output.name} to pick up the change",
deploy_id=deploy_id,
output=output.name,
cluster=output.name,
)
if stage.rollout is None:
_start_deployment_detection(
generation_result,
deploy_id,
output.name,
connection,
self.deployment_detector,
)
continue
started = time.monotonic()
_await_deployment_detection(
generation_result,
deploy_id,
output.name,
connection,
self.deployment_detector,
)
Expand Down Expand Up @@ -747,7 +732,7 @@ class DiffCommentProcessor:
The work is the same as :class:`ChangeProcessor` up to and including the
manifest commit, and then stops: nothing is pushed. The commit is still made,
in the throwaway checkout, because that is what makes the diff the one a
change would produce, cleanups and deploy-id annotations included.
change would produce, cleanups and manifest-id annotations included.

Every configured output is generated, because which of them a commit affects
is not something the commit says: it is what generating shows. The comment
Expand Down Expand Up @@ -1378,11 +1363,6 @@ def _written_paths(generation_result: object) -> set[Path]:
return cast(set[Path], written_paths)


def _deploy_id(generation_result: object) -> str | None:
deploy_id = getattr(generation_result, "deploy_id", None)
return deploy_id if isinstance(deploy_id, str) else None


def _checkout_paths_by_repository(
workdir: Path, outputs: Sequence[OutputSettings]
) -> dict[str, Path]:
Expand Down Expand Up @@ -1728,29 +1708,18 @@ def _cluster_name(output: OutputSettings) -> str | None:

def _start_deployment_detection(
generation_result: GenerationResult,
deploy_id: str | None,
cluster: str,
connection: OutputSettings | None,
detector: DeploymentDetector | None,
) -> None:
if deploy_id is None:
raise DeploymentDetectionError(
"manifest-builder did not return a deploy_id; "
"deployment detection requires git-backed generation"
)
created_or_modified = set(generation_result.created_or_modified)
removed = set(generation_result.removed)
logger.info(
"starting deployment detection for manifest-builder deploy-id %s in cluster %s",
deploy_id,
connection.name if connection is not None else "<injected detector>",
)
logger.info("starting deployment detection in cluster %s", cluster)
thread = threading.Thread(
target=_run_deployment_detection,
name=f"relcoord-deployment-detection-{deploy_id}",
name=f"relcoord-deployment-detection-{cluster}",
kwargs={
"deploy_id": deploy_id,
"created_or_modified": created_or_modified,
"removed": removed,
"created_or_modified": dict(generation_result.manifest_ids),
"removed": set(generation_result.removed),
"cluster": cluster,
"connection": connection,
"detector": detector,
},
Expand All @@ -1761,7 +1730,7 @@ def _start_deployment_detection(

def _await_deployment_detection(
generation_result: GenerationResult,
deploy_id: str | None,
cluster: str,
connection: OutputSettings | None,
detector: DeploymentDetector | None,
) -> None:
Expand All @@ -1771,108 +1740,81 @@ def _await_deployment_detection(
detection of a change without one, a failure is reported to the caller: it
is what stops the stages after this one from being pushed.
"""
if deploy_id is None:
raise DeploymentDetectionError(
"manifest-builder did not return a deploy_id; "
"deployment detection requires git-backed generation"
)
owned_detector: KubernetesDeploymentDetector | None = None
active_detector: DeploymentDetector
if detector is not None:
active_detector = detector
else:
if connection is None:
raise DeploymentDetectionError(
f"deployment detection for manifest-builder deploy-id {deploy_id} "
"has no cluster and no detector to observe it with"
f"deployment detection for {cluster} has no cluster connection "
"and no detector to observe it with"
)
try:
owned_detector = KubernetesDeploymentDetector.for_output(connection)
except Exception as exc:
raise RolloutStageError(
f"could not connect to cluster {connection.name} to detect the "
f"deployment of manifest-builder deploy-id {deploy_id}: {exc}"
f"deployment: {exc}"
) from exc
active_detector = owned_detector
logger.info(
"waiting for deployment of manifest-builder deploy-id %s in cluster %s",
deploy_id,
connection.name if connection is not None else "<injected detector>",
)
logger.info("waiting for deployment in cluster %s", cluster)
try:
active_detector.wait_for_success(
deploy_id=deploy_id,
created_or_modified=set(generation_result.created_or_modified),
created_or_modified=dict(generation_result.manifest_ids),
removed=set(generation_result.removed),
)
except Exception as exc:
raise RolloutStageError(
f"deployment of manifest-builder deploy-id {deploy_id} was not "
f"observed: {exc}"
f"deployment in cluster {cluster} was not observed: {exc}"
) from exc
else:
logger.info(
"deployment detected for manifest-builder deploy-id %s",
deploy_id,
)
logger.info("deployment detected in cluster %s", cluster)
finally:
if owned_detector is not None:
owned_detector.close()


def _run_deployment_detection(
*,
deploy_id: str,
created_or_modified: set[Any],
created_or_modified: Mapping[Any, str],
removed: set[Any],
cluster: str,
connection: OutputSettings | None,
detector: DeploymentDetector | None,
) -> None:
owned_detector: KubernetesDeploymentDetector | None = None
if detector is None:
if connection is None:
logger.error(
"deployment detection failed for manifest-builder deploy-id %s: "
"no cluster and no detector to observe it with",
deploy_id,
"deployment detection failed for %s: no cluster connection and "
"no detector to observe it with",
cluster,
)
return
try:
owned_detector = KubernetesDeploymentDetector.for_output(connection)
except Exception:
logger.exception(
"deployment detection failed for manifest-builder deploy-id %s: "
"could not connect to cluster %s",
deploy_id,
connection.name,
"deployment detection failed: could not connect to cluster %s",
cluster,
)
return
active_detector: DeploymentDetector | None = (
owned_detector if owned_detector is not None else detector
)
if active_detector is None:
logger.error(
"deployment detection failed for manifest-builder deploy-id %s: "
"no detector",
deploy_id,
)
logger.error("deployment detection failed in cluster %s: no detector", cluster)
return
try:
active_detector.wait_for_success(
deploy_id=deploy_id,
created_or_modified=created_or_modified,
removed=removed,
)
except Exception:
logger.exception(
"deployment detection failed for manifest-builder deploy-id %s",
deploy_id,
)
logger.exception("deployment detection failed in cluster %s", cluster)
else:
logger.info(
"deployment detected for manifest-builder deploy-id %s",
deploy_id,
)
logger.info("deployment detected in cluster %s", cluster)
finally:
if owned_detector is not None:
owned_detector.close()
Expand Down
Loading
Loading