)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"ab929911e9db9d4b3986d549183838c2ccabf88a","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Parent:     6f0e1fb4 (Add 2027.1 spec structure)"},{"line_number":2,"context_line":"Author:     melanie witt \u003cmelwittt@gmail.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2026-08-23 19:26:30 -0700"},{"line_number":4,"context_line":"Commit:     melanie witt \u003cmelwittt@gmail.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"4c57a22b_d62a751d","line":1,"updated":"2026-09-09 23:33:39.000000000","message":"The template for this repository (specs/2027.1-template.rst) states that any specification proposing changes to the Cyborg REST API must carry the APIImpact flag in the commit message so such changes can be found via the documented Gerrit query. This spec adds three new REST methods (GET /v2/services, GET /v2/services/{uuid}, DELETE /v2/services/{uuid}) behind a new microversion, but the commit message (verified via git log of commit ad2ad4e) contains no \u0027APIImpact\u0027 footer.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: The change cannot be found by the project\u0027s documented APIImpact Gerrit query, so API reviewers and downstream API-documentation tracking may miss this new /v2/services API.\n\n**Suggestion**:\nAmend the commit message to include an \u0027APIImpact\u0027 footer line (e.g. \u0027APIImpact: adds /v2/services list, show, and delete endpoints behind a new microversion\u0027) before merging.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"419c250c32d337b8a2df56344b461fe06bb40fd0","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Parent:     6f0e1fb4 (Add 2027.1 spec structure)"},{"line_number":2,"context_line":"Author:     melanie witt \u003cmelwittt@gmail.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2026-08-23 19:26:30 -0700"},{"line_number":4,"context_line":"Commit:     melanie witt \u003cmelwittt@gmail.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"94e09425_d11701fb","line":1,"updated":"2026-09-22 21:34:43.000000000","message":"The project spec template states that any specification proposing changes to the Cyborg REST API must add the APIImpact flag to the commit message. This change introduces a new REST API (GET /v2/services, GET /v2/services/{uuid}, DELETE /v2/services/{uuid}) plus a new microversion, but the commit message contains no APIImpact flag anywhere in its body or footers.\n\n**Severity**: WARNING | **Confidence**: 0.85\n\n**Impact**: Cross-project reviewers and tooling locate API-affecting spec changes via the APIImpact search the template links to; without the flag, this API-affecting spec is invisible to that query and the change fails an explicit repository requirement, weakening the API review contract the template documents.\n\n**Suggestion**:\nAmend the commit message to add an APIImpact footer line, e.g. \"APIImpact: Add /v2/services list, show and delete endpoints (admin-only) behind a new microversion\", placed with the other footers before Change-Id.","commit_id":"709ce6ed53e9baefd073b9f59a013c89ae4d8632"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"c2fec7e0_3f906873","updated":"2026-09-11 14:39:57.000000000","message":"this is a good start thasnks for propsoing it","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"2a1efbee7b6d512fc8a095517bc89a5b0f897d40","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"8fc4a7a0_62ae31a3","updated":"2026-10-06 01:53:25.000000000","message":"Thanks for looking!","commit_id":"a6981650e3dd12564dbdfb641c9e041a32987010"}],"specs/2027.1/approved/services-api.rst":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"b1ddacbd30a213a9ef7c019e5955a66163d4db9e","unresolved":false,"context_lines":[{"line_number":160,"context_line":"designed for pluggable storage backends, but no OpenStack project"},{"line_number":161,"context_line":"has ever shipped a second backend."},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"Nova recognized this and moved to placing SQLAlchemy queries"},{"line_number":164,"context_line":"directly in versioned objects, eliminating the redundant layer."},{"line_number":165,"context_line":"Cyborg is small enough today (~40 DB API methods) that establishing"},{"line_number":166,"context_line":"this direction now is low-cost. Waiting until after services,"}],"source_content_type":"text/x-rst","patch_set":1,"id":"54e20619_7f028720","line":163,"updated":"2026-08-24 03:00:00.000000000","message":"The spec\u0027s key architectural decision — placing SQLAlchemy queries directly in the Service versioned object and setting that as the pattern for all future Cyborg objects — is justified by the claim that \u0027Nova recognized this and moved to placing SQLAlchemy queries directly in versioned objects, eliminating the redundant layer.\u0027 That is not Nova\u0027s practice: Nova retains nova/db/api.py and nova/db/main/api.py as its DB API layer, and nova/objects/*.py (including nova/objects/service.py, which the spec itself cites as a reference) call those db.api functions rather than embedding SQLAlchemy. The spec even cites nova/db/main/api.py lines 574-576 as its reference implementation for last_seen_up, i.e., citing the very layer it says Nova eliminated. Changing Cyborg\u0027s established DB access pattern (CyborgObject -\u003e cyborg/db/api.py -\u003e sqlalchemy/api.py) is a significant, hard-to-reverse direction and should stand on accurate reasoning or real precedent (the closest is Placement\u0027s repository pattern, not objects with embedded SQLAlchemy).\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: The spec sets a precedent for every future Cyborg object based on a premise reviewers can falsify. If merged as-is, the Developer impact section directs contributors away from the project\u0027s existing DB API layer on grounds that do not hold, and reversing course after several objects embed SQLAlchemy would be costly.\n\n**Suggestion**:\nCorrect the justification: state that this is a deliberate Cyborg decision to skip the abstraction (optionally citing the pluggable-backend removal discussion or Placement\u0027s repository approach), drop the false Nova precedent, and note that existing objects are unaffected so the direction can be revisited. Also avoid pinning a reference to mutable upstream line numbers.","commit_id":"70dd3a98df373ba46bf2d94173561fc285d1e966"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"b1ddacbd30a213a9ef7c019e5955a66163d4db9e","unresolved":false,"context_lines":[{"line_number":243,"context_line":""},{"line_number":244,"context_line":"Computed at API response time (not stored):"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"* ``state \u003d \"up\"`` if"},{"line_number":247,"context_line":"  ``(now - service.last_seen_up) \u003c service_down_time``"},{"line_number":248,"context_line":"* ``state \u003d \"down\"`` otherwise"},{"line_number":249,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"6fe7152a_798bab54","line":246,"updated":"2026-08-24 03:00:00.000000000","message":"The spec defines the table column last_seen_up as nullable and \u0027set only when report_count increases\u0027, then defines liveness purely as `state \u003d \"up\" if (now - service.last_seen_up) \u003c service_down_time`. For any row where last_seen_up is NULL, this computation is undefined: a newly registered RPC service has no heartbeat yet (report_count defaults to 0 and last_seen_up is NULL until the first _report_state run up to report_interval later), and GET /v2/services/{uuid} deliberately returns WSGI services that never heartbeat. The spec even acknowledges \u0027state is not meaningful\u0027 for WSGI services without defining what value the API returns. Nova avoids this by falling back to updated_at/created_at in service_is_up().\n\n**Severity**: WARNING | **Confidence**: 0.85\n\n**Impact**: An implementer following the spec literally gets a TypeError or an arbitrary state value for freshly registered services (which appear down or error until the first heartbeat, up to report_interval seconds) and for every cyborg-api row fetched via GET /v2/services/{uuid}. Because the response schema is part of a permanent API microversion, an under-specified state field would have to be fixed in another microversion later.\n\n**Suggestion**:\nSpecify the NULL case explicitly in the Liveness detection section: either follow Nova\u0027s fallback (`last_seen_up or updated_at or created_at`) or define that a row with last_seen_up NULL reports state \u003d \"down\" / state omitted, and state exactly what the GET /v2/services/{uuid} response contains for WSGI services (e.g., \"state\": null or the field omitted).","commit_id":"70dd3a98df373ba46bf2d94173561fc285d1e966"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"b1ddacbd30a213a9ef7c019e5955a66163d4db9e","unresolved":false,"context_lines":[{"line_number":266,"context_line":""},{"line_number":267,"context_line":"Example response::"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    {"},{"line_number":270,"context_line":"        \"services\": ["},{"line_number":271,"context_line":"            {"},{"line_number":272,"context_line":"                \"uuid\": \"d9d2f0c6-3e4a-4b8c-9f1a-2b3c4d5e6f7a\","}],"source_content_type":"text/x-rst","patch_set":1,"id":"aa4db7d7_553bf092","line":269,"updated":"2026-08-24 03:00:00.000000000","message":"The spec states a use case: \u0027As an operator performing a rolling upgrade, I want to see which service version each host is running\u0027, adds a mandatory `version` column to the services table, and justifies registering cyborg-api rows specifically \u0027for version tracking ... important for rolling upgrade version checks\u0027. However, the specified GET /v2/services response schema (the example payload) contains only uuid, host, binary, state, report_count, updated_at, and created_at — no `version` field — and GET /v2/services/{uuid} is described as \u0027returns a single service object\u0027 with no schema that adds it. As specified, the API cannot satisfy its own rolling-upgrade use case, and nothing else in the spec exposes the version to operators.\n\n**Severity**: WARNING | **Confidence**: 0.85\n\n**Impact**: If implemented as written, the primary operator benefit claimed for the version groundwork is unreachable through any API, and exposing it later requires another microversion and API-contract churn — costly given that the template requires API changes to be specified carefully upfront because they must be supported forever.\n\n**Suggestion**:\nAdd `version` to the GET /v2/services and GET /v2/services/{uuid} response schemas (or explicitly document that version exposure is deferred and remove/adjust the rolling-upgrade operator use case accordingly).","commit_id":"70dd3a98df373ba46bf2d94173561fc285d1e966"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"4b8ffb538ae1813096c9427601d2861a89fcdb8a","unresolved":false,"context_lines":[{"line_number":286,"context_line":""},{"line_number":287,"context_line":"Optional query parameters: ``binary``, ``host``."},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"Example response::"},{"line_number":290,"context_line":""},{"line_number":291,"context_line":"    {"},{"line_number":292,"context_line":"        \"services\": ["}],"source_content_type":"text/x-rst","patch_set":2,"id":"c2d27316_69610c2e","line":289,"updated":"2026-08-24 03:37:01.000000000","message":"The problem description states operators need to answer \u0027Which hosts are running cyborg-agent and when did they last report in?\u0027 and the use case section repeats the health-visibility goal. However, the GET /v2/services response schema contains only state, report_count, created_at, and updated_at. The last_seen_up timestamp, which the spec carefully introduces as the only reliable liveness datum, is never exposed, so the API can answer up/down but not \u0027when did they last report in\u0027.\n\n**Severity**: WARNING | **Confidence**: 0.85\n\n**Impact**: The primary operator question the spec sets out to answer is only half-answered. Operators debugging flapping agents get a boolean state and a monotonically increasing counter but no way to see how stale the heartbeat is (e.g., 55 seconds vs. 5 seconds since last report), which is the actual diagnostic value.\n\n**Suggestion**:\nAdd last_seen_up (e.g., exposed as last_heartbeat or last_seen_up in the response, null-omitted for WSGI services like state) to the GET /v2/services and GET /v2/services/{uuid} response schemas, or explicitly narrow the stated use case wording if the timestamp is intentionally omitted.","commit_id":"e703257517f6a9bd2b26189dba3ba98af6091fc8"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"4b8ffb538ae1813096c9427601d2861a89fcdb8a","unresolved":false,"context_lines":[{"line_number":324,"context_line":"UUID; the ``state`` field is omitted from the response since they"},{"line_number":325,"context_line":"do not heartbeat."},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"**DELETE /v2/services/{uuid}** (delete stale service entry)"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"Response code: 204"},{"line_number":330,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"aa841522_bfe0e703","line":327,"updated":"2026-08-24 03:37:01.000000000","message":"The spec adds a periodic _report_state() task that increments report_count and calls service.save() every report_interval, and separately adds DELETE /v2/services/{uuid}. It never defines what the running service does when its row has been deleted: save() against a missing row will fail (or silently recreate it depending on implementation), producing either a crash loop in the periodic task or resurrection of the entry the operator just removed. Nova\u0027s equivalent _report_state() handles this explicitly by catching ServiceNotFound and re-registering, plus retry/dead-service handling for transient DB errors; the spec cites Nova\u0027s pattern elsewhere but omits this error path.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: An operator who deletes an entry for a host whose agent is actually alive (misjudged staleness, wrong UUID, or a race during decommissioning) gets undefined behavior: either the entry instantly reappears in GET /v2/services (making DELETE appear broken) or the agent\u0027s periodic task raises every 10 seconds until restart. Both are operator-facing failure modes of the new API.\n\n**Suggestion**:\nSpecify the DELETE/heartbeat interaction: follow Nova\u0027s report_state behavior, i.e. on ServiceNotFound during save(), re-register the service row (and log), and tolerate transient DB errors with a bounded retry. Also state whether DELETE should be rejected (or warned about) for a service whose state is currently \u0027up\u0027, since the stated use case is stale entries for decommissioned hosts.","commit_id":"e703257517f6a9bd2b26189dba3ba98af6091fc8"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"4b8ffb538ae1813096c9427601d2861a89fcdb8a","unresolved":false,"context_lines":[{"line_number":342,"context_line":""},{"line_number":343,"context_line":"::"},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"    service_policies \u003d ["},{"line_number":346,"context_line":"        policy.RuleDefault(\u0027cyborg:service:get_all\u0027,"},{"line_number":347,"context_line":"                           \u0027rule:admin_api\u0027),"},{"line_number":348,"context_line":"        policy.RuleDefault(\u0027cyborg:service:get_one\u0027,"}],"source_content_type":"text/x-rst","patch_set":2,"id":"3d5908fd_08f38c29","line":345,"updated":"2026-08-24 03:37:01.000000000","message":"The spec proposes service policies as plain policy.RuleDefault entries with check string \u0027rule:admin_api\u0027 in a rule list. The approved 2026.2 consistent-and-secure-rbac spec migrates Cyborg policy to per-resource modules under cyborg/policies/ using DocumentedRuleDefault with scope_types and deprecated bridges for every endpoint, and removes legacy rule lists from cyborg/common/policy.py. New policies added after that migration should follow the new pattern, or they will re-introduce exactly the legacy shape the RBAC spec eliminates.\n\n**Severity**: WARNING | **Confidence**: 0.85\n\n**Impact**: If implemented as specified, the new cyborg:service:* policies would be the only post-migration policies without scope_types or documented defaults. Operators using enforce_new_defaults would get policies invisible to oslo.policy back-compat tooling, and the services endpoints would diverge from the persona model (e.g., no reader persona decision) applied to the comparable admin-only inventory endpoints.\n\n**Suggestion**:\nUpdate the policy section to define the three rules as DocumentedRuleDefault entries with scope_types in a cyborg/policies/services.py module, with rule:admin_api as the deprecated bridge (matching the hardware-inventory mapping in the RBAC spec), and decide the persona for the new defaults explicitly.","commit_id":"e703257517f6a9bd2b26189dba3ba98af6091fc8"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"a8faf6e12b5092a16862e542a0918ad506597320","unresolved":false,"context_lines":[{"line_number":19,"context_line":"cyborg-agent on compute-7 alive?\""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Problem description"},{"line_number":23,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Cyborg runs three service binaries: two RPC services --"}],"source_content_type":"text/x-rst","patch_set":3,"id":"8de92d9e_4fc5dd33","line":22,"updated":"2026-08-24 03:57:34.000000000","message":"The review base commit 6f0e1fb (\u0027Add 2027.1 spec structure\u0027) explicitly changed all 2027.1 template section headers to title case. This spec uses sentence case for the corresponding sections and is even internally inconsistent: \u0027Documentation Impact\u0027 (line 676) is title case while \u0027Problem description\u0027 (line 22), \u0027Proposed change\u0027 (line 67), \u0027Data model impact\u0027 (line 475), \u0027REST API impact\u0027 (line 481), \u0027Security impact\u0027 (line 494) and others are sentence case.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: Cosmetic inconsistency across the 2027.1 spec directory plus mixed casing within a single document, undoing the uniformity the base commit deliberately introduced. No behavior impact; does not block merge.\n\n**Recommendation**:\nRename the sentence-case section headings to the template\u0027s title case (Problem Description, Proposed Change, Data Model Impact, REST API Impact, Security Impact, Notifications Impact, Other End User Impact, Performance Impact, Other Deployer Impact, Developer Impact, Upgrade Impact).","commit_id":"d6cb543371c51dd7bec7daf36d4800d6864e5a70"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"a8faf6e12b5092a16862e542a0918ad506597320","unresolved":false,"context_lines":[{"line_number":313,"context_line":"        ]"},{"line_number":314,"context_line":"    }"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"WSGI services (``cyborg-api``) are filtered out of the list"},{"line_number":317,"context_line":"response because they do not heartbeat and ``state`` is not"},{"line_number":318,"context_line":"meaningful for them."},{"line_number":319,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"de716765_72202944","line":316,"updated":"2026-08-24 03:57:34.000000000","message":"The spec filters WSGI services (topic\u003dNone, i.e. cyborg-api) out of GET /v2/services responses because they never heartbeat and would always show state\u003ddown. However, DELETE /v2/services/{uuid} is the spec\u0027s only cleanup mechanism and requires an already-known UUID, and the re-registration safety net only exists for RPC services via the _report_state() periodic task. cyborg-api never heartbeats and its row never re-registers, so once a controller host running cyborg-api is decommissioned, its row is permanently invisible in the list API and cannot be cleaned up through the API this spec introduces.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Use Case 2 is not satisfiable through the API for the service binary most likely to leave orphaned rows (an API service on every controller that never reports and never disappears). Operators must fall back to direct database access, the exact external-tooling dependency the spec argues against. Defect is limited to operational handling of an edge population, so warning severity, not higher.\n\n**Suggestion**:\nState how stale cyborg-api rows are managed: either permit an admin-only binary/host filter that can surface WSGI entries in the list (so their UUIDs are discoverable for DELETE), or document that WSGI rows must be pruned via a documented operational procedure, or defer cyborg-api registration until the version-tracking consumer exists.","commit_id":"d6cb543371c51dd7bec7daf36d4800d6864e5a70"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"a8faf6e12b5092a16862e542a0918ad506597320","unresolved":false,"context_lines":[{"line_number":628,"context_line":"* **Tests**: Unit tests for the ``Service`` object and REST"},{"line_number":629,"context_line":"  controller. Functional/API tests."},{"line_number":630,"context_line":""},{"line_number":631,"context_line":"* **Client updates** (separate repo): New ``openstack accelerator"},{"line_number":632,"context_line":"  service list`` and ``openstack accelerator service show``"},{"line_number":633,"context_line":"  commands in ``python-cyborgclient``."},{"line_number":634,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"400b30f0_6a073087","line":631,"updated":"2026-08-24 03:57:34.000000000","message":"\u0027Other end user impact\u0027 says python-cyborgclient needs three new commands: openstack accelerator service list, service show, and service delete, matching the three endpoints this spec adds. The Work Items client bullet, however, lists only \u0027service list\u0027 and \u0027service show\u0027, dropping the delete command without the explicit deferral note used for \u0027service set\u0027.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: The spec\u0027s only operator-facing cleanup mechanism (deleting stale entries, Use Case 2) loses its CLI work item, so the delete capability could ship API-only or be forgotten during implementation. Low-impact documentation omission with a one-line remedy.\n\n**Recommendation**:\nAdd \u0027openstack accelerator service delete\u0027 to the client-updates work item, or state explicitly that it is deferred alongside \u0027service set\u0027 until disable semantics are defined.","commit_id":"d6cb543371c51dd7bec7daf36d4800d6864e5a70"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":22,"context_line":"Problem Description"},{"line_number":23,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Cyborg runs three service binaries: two RPC services --"},{"line_number":26,"context_line":"``cyborg-conductor`` and ``cyborg-agent`` -- which start via"},{"line_number":27,"context_line":"``RPCService`` in ``cyborg/common/service.py``, and a WSGI service --"},{"line_number":28,"context_line":"``cyborg-api`` -- which starts via ``WSGIService``. None of them"},{"line_number":29,"context_line":"register in a database. There is no service database model, no"},{"line_number":30,"context_line":"versioned service object, and no heartbeat tracking. Cyborg persists"},{"line_number":31,"context_line":"state about accelerator devices, deployables, and accelerator"},{"line_number":32,"context_line":"requests, but has no record of what services are running or their"},{"line_number":33,"context_line":"health."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"Without a services API, operators cannot answer basic questions: \"Is"},{"line_number":36,"context_line":"the cyborg-agent on compute-7 alive?\" \"Which hosts are running"}],"source_content_type":"text/x-rst","patch_set":5,"id":"09945c9d_7f93ce30","line":33,"range":{"start_line":25,"start_character":0,"end_line":33,"end_character":7},"updated":"2026-09-11 14:39:57.000000000","message":"reflowing to remove the llmisums\n```suggestion\nCyborg runs three service binaries:\ntwo RPC services ``cyborg-conductor`` and ``cyborg-agent``,\nwhich start via ``RPCService`` in ``cyborg/common/service.py``,\nand a WSGI service ``cyborg-api`` which starts via ``WSGIService``.\n\nNone of them register in a database. There is no service database model,\nno versioned service object, and no heartbeat tracking. Cyborg persists\nstate about accelerator devices, deployables, and accelerator requests,\nbut has no record of what services are running or their health.\n```","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":22,"context_line":"Problem Description"},{"line_number":23,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Cyborg runs three service binaries: two RPC services --"},{"line_number":26,"context_line":"``cyborg-conductor`` and ``cyborg-agent`` -- which start via"},{"line_number":27,"context_line":"``RPCService`` in ``cyborg/common/service.py``, and a WSGI service --"},{"line_number":28,"context_line":"``cyborg-api`` -- which starts via ``WSGIService``. None of them"},{"line_number":29,"context_line":"register in a database. There is no service database model, no"},{"line_number":30,"context_line":"versioned service object, and no heartbeat tracking. Cyborg persists"},{"line_number":31,"context_line":"state about accelerator devices, deployables, and accelerator"},{"line_number":32,"context_line":"requests, but has no record of what services are running or their"},{"line_number":33,"context_line":"health."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"Without a services API, operators cannot answer basic questions: \"Is"},{"line_number":36,"context_line":"the cyborg-agent on compute-7 alive?\" \"Which hosts are running"}],"source_content_type":"text/x-rst","patch_set":5,"id":"d98713cc_a76ea296","line":33,"range":{"start_line":25,"start_character":0,"end_line":33,"end_character":7},"in_reply_to":"09945c9d_7f93ce30","updated":"2026-09-22 21:19:05.000000000","message":"Done","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":32,"context_line":"requests, but has no record of what services are running or their"},{"line_number":33,"context_line":"health."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"Without a services API, operators cannot answer basic questions: \"Is"},{"line_number":36,"context_line":"the cyborg-agent on compute-7 alive?\" \"Which hosts are running"},{"line_number":37,"context_line":"cyborg-agent and when did they last report in?\" This gap is"},{"line_number":38,"context_line":"especially problematic in large deployments where the agent fleet"},{"line_number":39,"context_line":"spans hundreds of compute nodes. It also blocks future work on"},{"line_number":40,"context_line":"rolling upgrades, which require service version tracking."},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"Use Cases"},{"line_number":43,"context_line":"---------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"65033ca9_0ecec0f5","line":40,"range":{"start_line":35,"start_character":0,"end_line":40,"end_character":57},"updated":"2026-09-11 14:39:57.000000000","message":"ditto\n```suggestion\nWithout a services API, operators cannot answer basic questions:\n* Is the cyborg-agent on compute-7 alive?\n* Which hosts are running cyborg-agent and when did they last report in?\n\nThis gap is especially problematic in large deployments where the agent\nfleet spans hundreds of compute nodes. It also blocks future work on\nrolling upgrades, which require service version tracking.\n```","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":32,"context_line":"requests, but has no record of what services are running or their"},{"line_number":33,"context_line":"health."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"Without a services API, operators cannot answer basic questions: \"Is"},{"line_number":36,"context_line":"the cyborg-agent on compute-7 alive?\" \"Which hosts are running"},{"line_number":37,"context_line":"cyborg-agent and when did they last report in?\" This gap is"},{"line_number":38,"context_line":"especially problematic in large deployments where the agent fleet"},{"line_number":39,"context_line":"spans hundreds of compute nodes. It also blocks future work on"},{"line_number":40,"context_line":"rolling upgrades, which require service version tracking."},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"Use Cases"},{"line_number":43,"context_line":"---------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"82af9562_c9c199b0","line":40,"range":{"start_line":35,"start_character":0,"end_line":40,"end_character":57},"in_reply_to":"65033ca9_0ecec0f5","updated":"2026-09-22 21:19:05.000000000","message":"Done","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":42,"context_line":"Use Cases"},{"line_number":43,"context_line":"---------"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"As an **operator**, I want to list all running Cyborg services and"},{"line_number":46,"context_line":"their health so that I can identify which hosts have agents that are"},{"line_number":47,"context_line":"not reporting in. Today I must rely on external monitoring or SSH"},{"line_number":48,"context_line":"into each compute node to check process status."}],"source_content_type":"text/x-rst","patch_set":5,"id":"56fe1fc0_036ce9b3","line":45,"range":{"start_line":45,"start_character":6,"end_line":45,"end_character":18},"updated":"2026-09-11 14:39:57.000000000","message":"nit:\n\n```suggestion\nAs an ``operator``, I want to list all running Cyborg services and\n```\nif you can prefer ` `` ` over `**` for ephahsice/bolding i would prefer to use the literals rather then `*` or `**`\n\nwith that siad i woudl prefer if we did not bold the person at all, its distracting form the over all usecase","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":42,"context_line":"Use Cases"},{"line_number":43,"context_line":"---------"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"As an **operator**, I want to list all running Cyborg services and"},{"line_number":46,"context_line":"their health so that I can identify which hosts have agents that are"},{"line_number":47,"context_line":"not reporting in. Today I must rely on external monitoring or SSH"},{"line_number":48,"context_line":"into each compute node to check process status."}],"source_content_type":"text/x-rst","patch_set":5,"id":"ce8f9936_47f378ec","line":45,"range":{"start_line":45,"start_character":6,"end_line":45,"end_character":18},"in_reply_to":"56fe1fc0_036ce9b3","updated":"2026-09-22 21:19:05.000000000","message":"Done","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":46,"context_line":"their health so that I can identify which hosts have agents that are"},{"line_number":47,"context_line":"not reporting in. Today I must rely on external monitoring or SSH"},{"line_number":48,"context_line":"into each compute node to check process status."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"As an **operator**, I want to remove stale service entries for"},{"line_number":51,"context_line":"decommissioned hosts so that the service list reflects the actual"},{"line_number":52,"context_line":"state of the deployment."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"As a **developer implementing rolling upgrades**, I need a"},{"line_number":55,"context_line":"``version`` column in the services table so that services can"}],"source_content_type":"text/x-rst","patch_set":5,"id":"8f97a5b9_c1c6a021","line":52,"range":{"start_line":49,"start_character":1,"end_line":52,"end_character":24},"updated":"2026-09-11 14:39:57.000000000","message":"havign the service entry also allows us to partly adress\n\nhttps://bugs.launchpad.net/openstack-cyborg/+bug/2150086\n\nas with nova when you delete a comptue service we can add the ability to use the conductor to clean up any placement or database entries for the comput-agent so that there is an automated way to do that\n\ntoday you have to first drain the node, then deisable all drivers and restart teh agent then remove the agent after it has reported no resouce and the conductor has updated placement.\n\nthat a pain and not documented but service delete --binary cyborg-agent can properly automate the cleanup without requrieing you to jump through those hoops","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":46,"context_line":"their health so that I can identify which hosts have agents that are"},{"line_number":47,"context_line":"not reporting in. Today I must rely on external monitoring or SSH"},{"line_number":48,"context_line":"into each compute node to check process status."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"As an **operator**, I want to remove stale service entries for"},{"line_number":51,"context_line":"decommissioned hosts so that the service list reflects the actual"},{"line_number":52,"context_line":"state of the deployment."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"As a **developer implementing rolling upgrades**, I need a"},{"line_number":55,"context_line":"``version`` column in the services table so that services can"}],"source_content_type":"text/x-rst","patch_set":5,"id":"4f36e39f_fc386a14","line":52,"range":{"start_line":49,"start_character":1,"end_line":52,"end_character":24},"in_reply_to":"8f97a5b9_c1c6a021","updated":"2026-09-22 21:19:05.000000000","message":"Done","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":51,"context_line":"decommissioned hosts so that the service list reflects the actual"},{"line_number":52,"context_line":"state of the deployment."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"As a **developer implementing rolling upgrades**, I need a"},{"line_number":55,"context_line":"``version`` column in the services table so that services can"},{"line_number":56,"context_line":"negotiate compatible RPC and object versions internally. (The"},{"line_number":57,"context_line":"version field lays groundwork for this; the rolling upgrade"},{"line_number":58,"context_line":"workflow itself is out of scope for this spec. The version is"},{"line_number":59,"context_line":"not exposed through the API -- it is an internal deployment"},{"line_number":60,"context_line":"detail, matching Nova\u0027s pattern.)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"Proposed Change"}],"source_content_type":"text/x-rst","patch_set":5,"id":"3410467a_f913ed2f","line":60,"range":{"start_line":54,"start_character":0,"end_line":60,"end_character":33},"updated":"2026-09-11 14:39:57.000000000","message":"```suggestion\nAs a developer i would like to supprot rolling upgrades,\nTo do this is need service version information to cordiante\nnegotiate compatible RPC and object versions internally.\n\n.. Note::\n\n  While this spec lays the foundation for future roling upgrade and\n  skip level upgrades completing that functionality is out of scope fo this spec\n```","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":51,"context_line":"decommissioned hosts so that the service list reflects the actual"},{"line_number":52,"context_line":"state of the deployment."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"As a **developer implementing rolling upgrades**, I need a"},{"line_number":55,"context_line":"``version`` column in the services table so that services can"},{"line_number":56,"context_line":"negotiate compatible RPC and object versions internally. (The"},{"line_number":57,"context_line":"version field lays groundwork for this; the rolling upgrade"},{"line_number":58,"context_line":"workflow itself is out of scope for this spec. The version is"},{"line_number":59,"context_line":"not exposed through the API -- it is an internal deployment"},{"line_number":60,"context_line":"detail, matching Nova\u0027s pattern.)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"Proposed Change"}],"source_content_type":"text/x-rst","patch_set":5,"id":"f4bddfc0_e380fde3","line":60,"range":{"start_line":54,"start_character":0,"end_line":60,"end_character":33},"in_reply_to":"3410467a_f913ed2f","updated":"2026-09-22 21:19:05.000000000","message":"Done","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":68,"context_line":""},{"line_number":69,"context_line":"A new Alembic migration adds a ``services`` table:"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":".. list-table::"},{"line_number":72,"context_line":"   :header-rows: 1"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"   * - Column"},{"line_number":75,"context_line":"     - Type"},{"line_number":76,"context_line":"     - Notes"},{"line_number":77,"context_line":"   * - ``id``"},{"line_number":78,"context_line":"     - Integer"},{"line_number":79,"context_line":"     - PK, auto-increment"},{"line_number":80,"context_line":"   * - ``uuid``"},{"line_number":81,"context_line":"     - String(36)"},{"line_number":82,"context_line":"     - Unique, not null"},{"line_number":83,"context_line":"   * - ``host``"},{"line_number":84,"context_line":"     - String(255)"},{"line_number":85,"context_line":"     - Not null. Hostname, e.g. ``compute-7``"},{"line_number":86,"context_line":"   * - ``binary``"},{"line_number":87,"context_line":"     - String(255)"},{"line_number":88,"context_line":"     - Not null. E.g. ``cyborg-agent``, ``cyborg-conductor``,"},{"line_number":89,"context_line":"       ``cyborg-api``"},{"line_number":90,"context_line":"   * - ``topic``"},{"line_number":91,"context_line":"     - String(255)"},{"line_number":92,"context_line":"     - Nullable. RPC topic for RPC services"},{"line_number":93,"context_line":"       (``constants.AGENT_TOPIC`` / ``CONDUCTOR_TOPIC``)."},{"line_number":94,"context_line":"       ``None`` for WSGI services (``cyborg-api``)"},{"line_number":95,"context_line":"   * - ``report_count``"},{"line_number":96,"context_line":"     - Integer"},{"line_number":97,"context_line":"     - Default 0, incremented on each heartbeat"},{"line_number":98,"context_line":"   * - ``last_seen_up``"},{"line_number":99,"context_line":"     - DateTime"},{"line_number":100,"context_line":"     - Nullable. Set only when ``report_count`` increases"},{"line_number":101,"context_line":"   * - ``version``"},{"line_number":102,"context_line":"     - Integer"},{"line_number":103,"context_line":"     - Not null. Version of the running service"},{"line_number":104,"context_line":"   * - ``created_at``"},{"line_number":105,"context_line":"     - DateTime"},{"line_number":106,"context_line":"     - Inherited from ``CyborgBase``"},{"line_number":107,"context_line":"   * - ``updated_at``"},{"line_number":108,"context_line":"     - DateTime"},{"line_number":109,"context_line":"     - Inherited from ``CyborgBase``; updated on any write"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"A unique constraint is placed on ``(host, binary)``."},{"line_number":112,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"d5cd46dd_43e871f8","line":109,"range":{"start_line":71,"start_character":0,"end_line":109,"end_character":58},"updated":"2026-09-11 14:39:57.000000000","message":"this would be a liitel easier to review as an ascii table or if you just manually specified the able and column without table formating using code blocks ectra\nbut its fine when rednered its readbale but i prefer to keep specs reabl;e wihtout rednering when practical.\n\nill consolidate my feedback based on the preview \n\nhttps://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_135/openstack/1351fe6d8c7e4fa4a65a7669efce8e72/docs/specs/2027.1/approved/services-api.html#data-model-services-table\n\nto avoid the isseus that nova has i would prefer if we use BigInteger primary keys for all tabels even fi we dont expect to need them\n\nplease do not use the term \"hostname\" for the host colunm\n\nthis is the CONF.host value (which is socket.gethostname) by default but i think we likely want to show both host and perhaps https://docs.openstack.org/cyborg/latest/configuration/config-options.html#agent.resource_provider_name for the agents.\n\nthat woudl make it a littel simpler for the conductor to find the asocated resouce prodiver although we coudl just look up the uuid fo the cyborg managed resouce provdier by getting the deployable managed by the given agent and retirvting the rp uuid form that so its not srictly required\n\nthat brings me to my next point\n\ni think we want to add the service id to the deployable tabel so we can join on that instead of the host as we did in https://specs.openstack.org/openstack/nova-specs/specs/2023.2/implemented/compute-object-ids.html\nand i also think if we are addign this we shoudl also implemnt eh same stable uuid checks as nova to detect host renames\nhttps://specs.openstack.org/openstack/nova-specs/specs/2023.1/implemented/stable-compute-uuid.html\n\nover all however i think this makes sense.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":68,"context_line":""},{"line_number":69,"context_line":"A new Alembic migration adds a ``services`` table:"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":".. list-table::"},{"line_number":72,"context_line":"   :header-rows: 1"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"   * - Column"},{"line_number":75,"context_line":"     - Type"},{"line_number":76,"context_line":"     - Notes"},{"line_number":77,"context_line":"   * - ``id``"},{"line_number":78,"context_line":"     - Integer"},{"line_number":79,"context_line":"     - PK, auto-increment"},{"line_number":80,"context_line":"   * - ``uuid``"},{"line_number":81,"context_line":"     - String(36)"},{"line_number":82,"context_line":"     - Unique, not null"},{"line_number":83,"context_line":"   * - ``host``"},{"line_number":84,"context_line":"     - String(255)"},{"line_number":85,"context_line":"     - Not null. Hostname, e.g. ``compute-7``"},{"line_number":86,"context_line":"   * - ``binary``"},{"line_number":87,"context_line":"     - String(255)"},{"line_number":88,"context_line":"     - Not null. E.g. ``cyborg-agent``, ``cyborg-conductor``,"},{"line_number":89,"context_line":"       ``cyborg-api``"},{"line_number":90,"context_line":"   * - ``topic``"},{"line_number":91,"context_line":"     - String(255)"},{"line_number":92,"context_line":"     - Nullable. RPC topic for RPC services"},{"line_number":93,"context_line":"       (``constants.AGENT_TOPIC`` / ``CONDUCTOR_TOPIC``)."},{"line_number":94,"context_line":"       ``None`` for WSGI services (``cyborg-api``)"},{"line_number":95,"context_line":"   * - ``report_count``"},{"line_number":96,"context_line":"     - Integer"},{"line_number":97,"context_line":"     - Default 0, incremented on each heartbeat"},{"line_number":98,"context_line":"   * - ``last_seen_up``"},{"line_number":99,"context_line":"     - DateTime"},{"line_number":100,"context_line":"     - Nullable. Set only when ``report_count`` increases"},{"line_number":101,"context_line":"   * - ``version``"},{"line_number":102,"context_line":"     - Integer"},{"line_number":103,"context_line":"     - Not null. Version of the running service"},{"line_number":104,"context_line":"   * - ``created_at``"},{"line_number":105,"context_line":"     - DateTime"},{"line_number":106,"context_line":"     - Inherited from ``CyborgBase``"},{"line_number":107,"context_line":"   * - ``updated_at``"},{"line_number":108,"context_line":"     - DateTime"},{"line_number":109,"context_line":"     - Inherited from ``CyborgBase``; updated on any write"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"A unique constraint is placed on ``(host, binary)``."},{"line_number":112,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"28c42b59_da06f01c","line":109,"range":{"start_line":71,"start_character":0,"end_line":109,"end_character":58},"in_reply_to":"d5cd46dd_43e871f8","updated":"2026-09-22 21:19:05.000000000","message":"Done","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":129,"context_line":".. list-table::"},{"line_number":130,"context_line":"   :header-rows: 1"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"   * - Column"},{"line_number":133,"context_line":"     - Type"},{"line_number":134,"context_line":"     - Notes"},{"line_number":135,"context_line":"   * - ``disabled``"},{"line_number":136,"context_line":"     - Boolean"},{"line_number":137,"context_line":"     - Default False"},{"line_number":138,"context_line":"   * - ``disabled_reason``"},{"line_number":139,"context_line":"     - String(255)"},{"line_number":140,"context_line":"     - Nullable"},{"line_number":141,"context_line":"   * - ``forced_down``"},{"line_number":142,"context_line":"     - Boolean"}],"source_content_type":"text/x-rst","patch_set":5,"id":"3faac479_5bbf8030","line":139,"range":{"start_line":132,"start_character":0,"end_line":139,"end_character":18},"updated":"2026-09-11 14:39:57.000000000","message":"this is tricky\n\n\ndisabled exist to prevent scudling\n\nwe could technically provide a filter for the nova scheduler that would check provider traits (it would ideally be a pre-filter but those are not plug-able yet despite my efforts) alternatively we could have the conductor or cyborg-agent set reserved\u003dtotal to have a simialr efffect.\n\nperhasp both, similar to local delete the conductor could do it if the agent is not down after calling a new disabel rpc\n\nthe thing is right now the placement inventoes are manged via the cunductor and i woudl really like us to move that to the cyborg-agent in the future so im not really sure how to implemnet this.\n\nwe niavly coudl refues to bind ARQ to disabled nodes and proably should but that very late for the first rejection of a host so before we supprot disabel via the api we need to figure out hwo to make nova filter out the host.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":true,"context_lines":[{"line_number":129,"context_line":".. list-table::"},{"line_number":130,"context_line":"   :header-rows: 1"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"   * - Column"},{"line_number":133,"context_line":"     - Type"},{"line_number":134,"context_line":"     - Notes"},{"line_number":135,"context_line":"   * - ``disabled``"},{"line_number":136,"context_line":"     - Boolean"},{"line_number":137,"context_line":"     - Default False"},{"line_number":138,"context_line":"   * - ``disabled_reason``"},{"line_number":139,"context_line":"     - String(255)"},{"line_number":140,"context_line":"     - Nullable"},{"line_number":141,"context_line":"   * - ``forced_down``"},{"line_number":142,"context_line":"     - Boolean"}],"source_content_type":"text/x-rst","patch_set":5,"id":"f8788663_50870543","line":139,"range":{"start_line":132,"start_character":0,"end_line":139,"end_character":18},"in_reply_to":"3faac479_5bbf8030","updated":"2026-09-22 21:19:05.000000000","message":"Yeah, it seems clear there isn\u0027t a reasonable way we could provide cyborg-agent \"disable\" functionality, so I think I\u0027ll go ahead and remove this section from the spec. If we wanted to disable services, it would take more design and work, so probably best to punt for now.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":138,"context_line":"   * - ``disabled_reason``"},{"line_number":139,"context_line":"     - String(255)"},{"line_number":140,"context_line":"     - Nullable"},{"line_number":141,"context_line":"   * - ``forced_down``"},{"line_number":142,"context_line":"     - Boolean"},{"line_number":143,"context_line":"     - Default False"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"Versioned object: Service"},{"line_number":146,"context_line":"-------------------------"},{"line_number":147,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"2cb0f98d_0e4722b7","line":144,"range":{"start_line":141,"start_character":0,"end_line":144,"end_character":1},"updated":"2026-09-11 14:39:57.000000000","message":"so i dont think force down is needed as we only have that in nova to overried our evaucate guardrail.\n\nwe do not have the same usecase in cyborg","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":138,"context_line":"   * - ``disabled_reason``"},{"line_number":139,"context_line":"     - String(255)"},{"line_number":140,"context_line":"     - Nullable"},{"line_number":141,"context_line":"   * - ``forced_down``"},{"line_number":142,"context_line":"     - Boolean"},{"line_number":143,"context_line":"     - Default False"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"Versioned object: Service"},{"line_number":146,"context_line":"-------------------------"},{"line_number":147,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"9263f8c6_49615454","line":144,"range":{"start_line":141,"start_character":0,"end_line":144,"end_character":1},"in_reply_to":"2cb0f98d_0e4722b7","updated":"2026-09-22 21:19:05.000000000","message":"Done","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":145,"context_line":"Versioned object: Service"},{"line_number":146,"context_line":"-------------------------"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"A new file ``cyborg/objects/service.py`` defines the ``Service``"},{"line_number":149,"context_line":"versioned object:"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"* Inherits from ``CyborgObject`` (matching ``Device``,"},{"line_number":152,"context_line":"  ``Deployable``, etc.)"},{"line_number":153,"context_line":"* ``VERSION \u003d \u00271.0\u0027``"},{"line_number":154,"context_line":"* Fields mirror the DB columns"},{"line_number":155,"context_line":"* Methods: ``get_by_uuid()``, ``get_by_host_and_binary()``,"},{"line_number":156,"context_line":"  ``list()``, ``create()``, ``save()``, ``destroy()``"},{"line_number":157,"context_line":"* Registered in ``cyborg/objects/__init__.py``"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"DB access pattern"},{"line_number":160,"context_line":"~~~~~~~~~~~~~~~~~"}],"source_content_type":"text/x-rst","patch_set":5,"id":"158f6644_4530f676","line":157,"range":{"start_line":148,"start_character":0,"end_line":157,"end_character":46},"updated":"2026-09-11 14:39:57.000000000","message":"this is just a littel more readable\n\n```suggestion\nA new file ``cyborg/objects/service.py`` defines the ``Service``\nversioned object:\n\nInherits from ``CyborgObject`` (matching ``Device``, ``Deployable``, etc.)\nFields mirror the DB columns\nMethods:\n  ``get_by_uuid()``\n  ``get_by_host_and_binary()``\n  ``list()``\n  ``create()``\n  ``save()``\n  ``destroy()``\n```","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":145,"context_line":"Versioned object: Service"},{"line_number":146,"context_line":"-------------------------"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"A new file ``cyborg/objects/service.py`` defines the ``Service``"},{"line_number":149,"context_line":"versioned object:"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"* Inherits from ``CyborgObject`` (matching ``Device``,"},{"line_number":152,"context_line":"  ``Deployable``, etc.)"},{"line_number":153,"context_line":"* ``VERSION \u003d \u00271.0\u0027``"},{"line_number":154,"context_line":"* Fields mirror the DB columns"},{"line_number":155,"context_line":"* Methods: ``get_by_uuid()``, ``get_by_host_and_binary()``,"},{"line_number":156,"context_line":"  ``list()``, ``create()``, ``save()``, ``destroy()``"},{"line_number":157,"context_line":"* Registered in ``cyborg/objects/__init__.py``"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"DB access pattern"},{"line_number":160,"context_line":"~~~~~~~~~~~~~~~~~"}],"source_content_type":"text/x-rst","patch_set":5,"id":"7eb09df3_4af477f0","line":157,"range":{"start_line":148,"start_character":0,"end_line":157,"end_character":46},"in_reply_to":"158f6644_4530f676","updated":"2026-09-22 21:19:05.000000000","message":"Done","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":162,"context_line":"This section proposes a direction for the ``Service`` object\u0027s"},{"line_number":163,"context_line":"DB access pattern and is subject to reviewer feedback. Reviewers"},{"line_number":164,"context_line":"may prefer to continue with the existing approach of adding"},{"line_number":165,"context_line":"methods to ``cyborg/db/api.py`` and ``cyborg/db/sqlalchemy/api.py``."},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"Existing Cyborg objects access the database through an abstract"},{"line_number":168,"context_line":"DB API layer (``cyborg/db/api.py`` -\u003e"}],"source_content_type":"text/x-rst","patch_set":5,"id":"b6ec2516_03c461d1","line":165,"range":{"start_line":165,"start_character":0,"end_line":165,"end_character":68},"updated":"2026-09-11 14:39:57.000000000","message":"this is fine for now but i think im just going to propse a mini spec or specless bluepint to remvoe the indirection\n\nwe are realsiticly not going to ever supprot a db backeidn other then sqlalchemy so i think im just going to propos a patch to move everything form `cyborg/db/sqlalchemy`` up to  ``cyborg/db/``\n\nright now it just cause maintance overhead for no benifit\n\nlater: ah you are proposing a larger refactor here so cool im ok leaveing this to your spec.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":162,"context_line":"This section proposes a direction for the ``Service`` object\u0027s"},{"line_number":163,"context_line":"DB access pattern and is subject to reviewer feedback. Reviewers"},{"line_number":164,"context_line":"may prefer to continue with the existing approach of adding"},{"line_number":165,"context_line":"methods to ``cyborg/db/api.py`` and ``cyborg/db/sqlalchemy/api.py``."},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"Existing Cyborg objects access the database through an abstract"},{"line_number":168,"context_line":"DB API layer (``cyborg/db/api.py`` -\u003e"}],"source_content_type":"text/x-rst","patch_set":5,"id":"e699fe2b_c57ccc2d","line":165,"range":{"start_line":165,"start_character":0,"end_line":165,"end_character":68},"in_reply_to":"b6ec2516_03c461d1","updated":"2026-09-22 21:19:05.000000000","message":"Acknowledged","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":170,"context_line":"single caller -- the corresponding object method -- making the"},{"line_number":171,"context_line":"abstraction layer largely indirection with no practical benefit."},{"line_number":172,"context_line":"The abstract ``Connection`` base class was designed for pluggable"},{"line_number":173,"context_line":"storage backends, but no OpenStack project ships a second backend."},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"Nova still has its DB API layer (``nova/db/main/api.py``) and"},{"line_number":176,"context_line":"continues to use it. However, Placement -- extracted from Nova --"}],"source_content_type":"text/x-rst","patch_set":5,"id":"ec4fa60c_3894130c","line":173,"updated":"2026-09-11 14:39:57.000000000","message":"yep exactly","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":170,"context_line":"single caller -- the corresponding object method -- making the"},{"line_number":171,"context_line":"abstraction layer largely indirection with no practical benefit."},{"line_number":172,"context_line":"The abstract ``Connection`` base class was designed for pluggable"},{"line_number":173,"context_line":"storage backends, but no OpenStack project ships a second backend."},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"Nova still has its DB API layer (``nova/db/main/api.py``) and"},{"line_number":176,"context_line":"continues to use it. However, Placement -- extracted from Nova --"}],"source_content_type":"text/x-rst","patch_set":5,"id":"f1cde519_b34d436b","line":173,"in_reply_to":"ec4fa60c_3894130c","updated":"2026-09-22 21:19:05.000000000","message":"Acknowledged","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":179,"context_line":"live directly in the objects, decorated with"},{"line_number":180,"context_line":"``@db_api.placement_context_manager.reader/writer``. Cyborg is"},{"line_number":181,"context_line":"closer in size and complexity to Placement than to Nova, making"},{"line_number":182,"context_line":"this a potentially more appropriate model to follow."},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"Cyborg is small enough today (~50 DB API methods) that"},{"line_number":185,"context_line":"establishing this direction now is low-cost. Waiting until after"}],"source_content_type":"text/x-rst","patch_set":5,"id":"742b113b_730e7ac9","line":182,"updated":"2026-09-11 14:39:57.000000000","message":"well even with nova the refactorign could be aprpoate i think size i.e. larger proejct are better served by not having a monolitich file.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":179,"context_line":"live directly in the objects, decorated with"},{"line_number":180,"context_line":"``@db_api.placement_context_manager.reader/writer``. Cyborg is"},{"line_number":181,"context_line":"closer in size and complexity to Placement than to Nova, making"},{"line_number":182,"context_line":"this a potentially more appropriate model to follow."},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"Cyborg is small enough today (~50 DB API methods) that"},{"line_number":185,"context_line":"establishing this direction now is low-cost. Waiting until after"}],"source_content_type":"text/x-rst","patch_set":5,"id":"5e95be83_4b19aca0","line":182,"in_reply_to":"742b113b_730e7ac9","updated":"2026-09-22 21:19:05.000000000","message":"Acknowledged","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":190,"context_line":"The proposed approach is for the ``Service`` object to place its"},{"line_number":191,"context_line":"SQLAlchemy queries directly in the object class methods, with no"},{"line_number":192,"context_line":"new methods added to ``cyborg/db/api.py`` or"},{"line_number":193,"context_line":"``cyborg/db/sqlalchemy/api.py``. Existing objects and the direct"},{"line_number":194,"context_line":"DB API usage in ``quota.py`` would not be migrated as part of"},{"line_number":195,"context_line":"this spec -- that is separate cleanup work."},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"Service registration and heartbeat"}],"source_content_type":"text/x-rst","patch_set":5,"id":"dfe083e0_83e342c0","line":194,"range":{"start_line":193,"start_character":32,"end_line":194,"end_character":28},"updated":"2026-09-11 14:39:57.000000000","message":"i will be deleteing that soon\n\nit is dead code and has been for a long time  https://bugs.launchpad.net/openstack-cyborg/+bug/2143943\n\nif we ever add quota via the api in the future (its unlikely out side of some kind of qos api) it will be based on unified limits so there is no value in keeping any of that code.\n\ni had planend to drop this this cycle i just didnt get around to it but i will do it before the end of the year so dont worry about this file for your spec","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":true,"context_lines":[{"line_number":190,"context_line":"The proposed approach is for the ``Service`` object to place its"},{"line_number":191,"context_line":"SQLAlchemy queries directly in the object class methods, with no"},{"line_number":192,"context_line":"new methods added to ``cyborg/db/api.py`` or"},{"line_number":193,"context_line":"``cyborg/db/sqlalchemy/api.py``. Existing objects and the direct"},{"line_number":194,"context_line":"DB API usage in ``quota.py`` would not be migrated as part of"},{"line_number":195,"context_line":"this spec -- that is separate cleanup work."},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"Service registration and heartbeat"}],"source_content_type":"text/x-rst","patch_set":5,"id":"397e5a13_67a65d38","line":194,"range":{"start_line":193,"start_character":32,"end_line":194,"end_character":28},"in_reply_to":"dfe083e0_83e342c0","updated":"2026-09-22 21:19:05.000000000","message":"Cool, sounds good.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":192,"context_line":"new methods added to ``cyborg/db/api.py`` or"},{"line_number":193,"context_line":"``cyborg/db/sqlalchemy/api.py``. Existing objects and the direct"},{"line_number":194,"context_line":"DB API usage in ``quota.py`` would not be migrated as part of"},{"line_number":195,"context_line":"this spec -- that is separate cleanup work."},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"Service registration and heartbeat"},{"line_number":198,"context_line":"----------------------------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"5165bd04_2a7ec091","line":195,"updated":"2026-09-11 14:39:57.000000000","message":"ok im not agaist puting th fully migration outside of this spec but i may propsoe it sperately then\n\nas noted above at a minim i want to mvoe everything form \n\n``cyborg/db/sqlalchemy/`` up to ``cyborg/db/``\n\nremovaing the indreiction layer\n\ni had not condier movign the queies to the object but if we esablish that patter with the service object i would like to normalise the other obejct as a follow up.\n\nthat may be the split here.\nill remvoe the abstaction layer over the backend and you can provide the refence example of the new pattern via the service object then we can migrate the rest over the next cycle or two.\n\nif we work on unifiy the driver and non driver verions of the object i think that woudl be a good opertunity to do the db method migration.\n\ndepending on other priorities we may not start that unifcation until next cycles so i dont want that to be a blocker for this spec.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":true,"context_lines":[{"line_number":192,"context_line":"new methods added to ``cyborg/db/api.py`` or"},{"line_number":193,"context_line":"``cyborg/db/sqlalchemy/api.py``. Existing objects and the direct"},{"line_number":194,"context_line":"DB API usage in ``quota.py`` would not be migrated as part of"},{"line_number":195,"context_line":"this spec -- that is separate cleanup work."},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"Service registration and heartbeat"},{"line_number":198,"context_line":"----------------------------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"23ef06a8_1a25f0eb","line":195,"in_reply_to":"5165bd04_2a7ec091","updated":"2026-09-22 21:19:05.000000000","message":"TBH I wasn\u0027t thinking of migrating existing methods but rather just not introducing new ones and go directly into objects from now on. I\u0027m not strongly opposed to migrating them either if it seems worthwhile.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":202,"context_line":""},{"line_number":203,"context_line":"Modify ``RPCService.start()`` in ``cyborg/common/service.py``:"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"* After ``self.manager.init_host()``, call"},{"line_number":206,"context_line":"  ``_register_service()``:"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"  * Try ``Service.get_by_host_and_binary(host, binary)``"}],"source_content_type":"text/x-rst","patch_set":5,"id":"2adaca78_0dbb05b9","line":205,"range":{"start_line":205,"start_character":15,"end_line":205,"end_character":22},"updated":"2026-09-11 14:39:57.000000000","message":"as an aside our makger object done inherit form a singel common base class today\n\nhttps://github.com/openstack/cyborg/blob/master/cyborg/agent/manager.py#L40\nhttps://github.com/openstack/cyborg/blob/master/cyborg/conductor/manager.py\n\nwe shoudl proably fix that so the they have a common interface\n\nwe have added init_host to both manger and tied to keep the interface constiat but it woudl be nice to use an protocol (pythons equivlent of an interface)\nhttps://typing.python.org/en/latest/spec/protocol.html\nor abstract base class to tighten this up a bit.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":true,"context_lines":[{"line_number":202,"context_line":""},{"line_number":203,"context_line":"Modify ``RPCService.start()`` in ``cyborg/common/service.py``:"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"* After ``self.manager.init_host()``, call"},{"line_number":206,"context_line":"  ``_register_service()``:"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"  * Try ``Service.get_by_host_and_binary(host, binary)``"}],"source_content_type":"text/x-rst","patch_set":5,"id":"07aa5eec_5b0bc58d","line":205,"range":{"start_line":205,"start_character":15,"end_line":205,"end_character":22},"in_reply_to":"2adaca78_0dbb05b9","updated":"2026-09-22 21:19:05.000000000","message":"Agreed they should have a common base class and interface.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":208,"context_line":"  * Try ``Service.get_by_host_and_binary(host, binary)``"},{"line_number":209,"context_line":"  * If found, increment ``report_count`` and update"},{"line_number":210,"context_line":"    ``version`` (treated as the initial heartbeat)"},{"line_number":211,"context_line":"  * If not found, create a new ``Service`` record"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"* Add periodic task ``_report_state()`` (every"},{"line_number":214,"context_line":"  ``report_interval`` seconds):"}],"source_content_type":"text/x-rst","patch_set":5,"id":"a7230f0d_78614e67","line":211,"updated":"2026-09-11 14:39:57.000000000","message":"so here for the cyborg-agent specificlly  i would extend this to also suprpot the stable uuid feature and have the agent write the uuid ot a state file on disk so we can detch comptue host renames and stop the agent start.\n\nhttps://specs.openstack.org/openstack/nova-specs/specs/2023.1/implemented/stable-compute-uuid.html\n\notherwise this looks good to me.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":208,"context_line":"  * Try ``Service.get_by_host_and_binary(host, binary)``"},{"line_number":209,"context_line":"  * If found, increment ``report_count`` and update"},{"line_number":210,"context_line":"    ``version`` (treated as the initial heartbeat)"},{"line_number":211,"context_line":"  * If not found, create a new ``Service`` record"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"* Add periodic task ``_report_state()`` (every"},{"line_number":214,"context_line":"  ``report_interval`` seconds):"}],"source_content_type":"text/x-rst","patch_set":5,"id":"019e8245_9f80ae1b","line":211,"in_reply_to":"a7230f0d_78614e67","updated":"2026-09-22 21:19:05.000000000","message":"Done","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":210,"context_line":"    ``version`` (treated as the initial heartbeat)"},{"line_number":211,"context_line":"  * If not found, create a new ``Service`` record"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"* Add periodic task ``_report_state()`` (every"},{"line_number":214,"context_line":"  ``report_interval`` seconds):"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"  * Increment ``report_count``"},{"line_number":217,"context_line":"  * Call ``service.save()``"},{"line_number":218,"context_line":"  * ``last_seen_up`` is set in ``save()`` only when"},{"line_number":219,"context_line":"    ``report_count`` increases"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"New config options:"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"* ``[DEFAULT] report_interval`` -- Integer, default 10 seconds."}],"source_content_type":"text/x-rst","patch_set":5,"id":"76473065_dda60b9e","line":220,"range":{"start_line":213,"start_character":1,"end_line":220,"end_character":1},"updated":"2026-09-11 14:39:57.000000000","message":"this alreayd kind of exists of for the cybrog agent via update_available_resource\n\nhttps://github.com/openstack/cyborg/blob/master/cyborg/agent/manager.py#L191-L202\n\nbut sure i think we can add a second perodic \n\nthis woudl lean me more in the direction of making Manger be a base class so we can share this between both managers.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":210,"context_line":"    ``version`` (treated as the initial heartbeat)"},{"line_number":211,"context_line":"  * If not found, create a new ``Service`` record"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"* Add periodic task ``_report_state()`` (every"},{"line_number":214,"context_line":"  ``report_interval`` seconds):"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"  * Increment ``report_count``"},{"line_number":217,"context_line":"  * Call ``service.save()``"},{"line_number":218,"context_line":"  * ``last_seen_up`` is set in ``save()`` only when"},{"line_number":219,"context_line":"    ``report_count`` increases"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"New config options:"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"* ``[DEFAULT] report_interval`` -- Integer, default 10 seconds."}],"source_content_type":"text/x-rst","patch_set":5,"id":"cd5901c5_c6519a53","line":220,"range":{"start_line":213,"start_character":1,"end_line":220,"end_character":1},"in_reply_to":"76473065_dda60b9e","updated":"2026-09-22 21:19:05.000000000","message":"Acknowledged","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":220,"context_line":""},{"line_number":221,"context_line":"New config options:"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"* ``[DEFAULT] report_interval`` -- Integer, default 10 seconds."},{"line_number":224,"context_line":"  How often each service reports its state to the database."},{"line_number":225,"context_line":"* ``[DEFAULT] service_down_time`` -- Integer, default 60 seconds."},{"line_number":226,"context_line":"  Elapsed time since ``last_seen_up`` after which a service is"}],"source_content_type":"text/x-rst","patch_set":5,"id":"91140a66_354da9d5","line":223,"range":{"start_line":223,"start_character":52,"end_line":223,"end_character":54},"updated":"2026-09-11 14:39:57.000000000","message":"hum i was thinking more like 300\n\ni dont think this should be sent on a high cadence like 10 seconds","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":220,"context_line":""},{"line_number":221,"context_line":"New config options:"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"* ``[DEFAULT] report_interval`` -- Integer, default 10 seconds."},{"line_number":224,"context_line":"  How often each service reports its state to the database."},{"line_number":225,"context_line":"* ``[DEFAULT] service_down_time`` -- Integer, default 60 seconds."},{"line_number":226,"context_line":"  Elapsed time since ``last_seen_up`` after which a service is"}],"source_content_type":"text/x-rst","patch_set":5,"id":"0dc5cc17_a9a05e7e","line":223,"range":{"start_line":223,"start_character":52,"end_line":223,"end_character":54},"in_reply_to":"91140a66_354da9d5","updated":"2026-09-22 21:19:05.000000000","message":"Done","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":222,"context_line":""},{"line_number":223,"context_line":"* ``[DEFAULT] report_interval`` -- Integer, default 10 seconds."},{"line_number":224,"context_line":"  How often each service reports its state to the database."},{"line_number":225,"context_line":"* ``[DEFAULT] service_down_time`` -- Integer, default 60 seconds."},{"line_number":226,"context_line":"  Elapsed time since ``last_seen_up`` after which a service is"},{"line_number":227,"context_line":"  considered down."},{"line_number":228,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"3e1e0ed9_3a5edcc4","line":225,"range":{"start_line":225,"start_character":54,"end_line":225,"end_character":56},"updated":"2026-09-11 14:39:57.000000000","message":"and this i woudl set to 900s\n\nthe liveness is really the sec ondary use case for this feature\nif that was the only reason to intoduce it i would proabl say the api is not needed its not needed.\n\nknowing whre agent are expect and the version they are runing is the primary reason to intoduce this.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":222,"context_line":""},{"line_number":223,"context_line":"* ``[DEFAULT] report_interval`` -- Integer, default 10 seconds."},{"line_number":224,"context_line":"  How often each service reports its state to the database."},{"line_number":225,"context_line":"* ``[DEFAULT] service_down_time`` -- Integer, default 60 seconds."},{"line_number":226,"context_line":"  Elapsed time since ``last_seen_up`` after which a service is"},{"line_number":227,"context_line":"  considered down."},{"line_number":228,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"eebb7e97_c878db63","line":225,"range":{"start_line":225,"start_character":54,"end_line":225,"end_character":56},"in_reply_to":"3e1e0ed9_3a5edcc4","updated":"2026-09-22 21:19:05.000000000","message":"Done","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":233,"context_line":"listen on a message bus topic and cannot heartbeat through the same"},{"line_number":234,"context_line":"periodic task mechanism as the RPC services."},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"Nova handles this identically: ``nova-osapi_compute`` and"},{"line_number":237,"context_line":"``nova-metadata`` register a ``Service`` row on startup (with"},{"line_number":238,"context_line":"``topic\u003dNone``, ``report_count\u003d0``) but never heartbeat. Because"},{"line_number":239,"context_line":"they never heartbeat, they would always appear as"},{"line_number":240,"context_line":"``state \u003d \"down\"``, so Nova\u0027s os-services API explicitly filters"},{"line_number":241,"context_line":"them out of GET responses (see `Nova bug 1543625`_)."},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"Cyborg follows the same pattern:"},{"line_number":244,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"0da4b0c3_fc1899a4","line":241,"range":{"start_line":236,"start_character":0,"end_line":241,"end_character":52},"updated":"2026-09-11 14:39:57.000000000","message":"ya i dont htink the filter in in nova is correct i dislike that we do that in nova today and would condiser the curernt behwvior to be less correct then the bug that was fixed i have wanted to revert that patch in the past so i would prefer not to ot use the same approch.\n\nit breaks the abliyt to know if all your api services are upgraded and where they are running without going to the db which undermins the reason for this api to exist.\n\ninstead i would prefer to return the apis from the service list and have a sentenial value for the up state or not register them at all.\n\nmy prefence is to register them amd  make up nullable\n\nalternative we can have the API update there last up on each applcaition start.\nand perodiclly on every reporting interval by usign a monitomic timer.\n\ni.e. when the wsgi app is spawned by a new request being recived ti woudl update its last up before serving the request, and if tis long running it woudl check a monatomic time since last update on every request and update the uptim in the db when that exceeds the reporting interval.\n\nthis can be ma module level variable so that ti will be shared by all threads in the saem application.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":233,"context_line":"listen on a message bus topic and cannot heartbeat through the same"},{"line_number":234,"context_line":"periodic task mechanism as the RPC services."},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"Nova handles this identically: ``nova-osapi_compute`` and"},{"line_number":237,"context_line":"``nova-metadata`` register a ``Service`` row on startup (with"},{"line_number":238,"context_line":"``topic\u003dNone``, ``report_count\u003d0``) but never heartbeat. Because"},{"line_number":239,"context_line":"they never heartbeat, they would always appear as"},{"line_number":240,"context_line":"``state \u003d \"down\"``, so Nova\u0027s os-services API explicitly filters"},{"line_number":241,"context_line":"them out of GET responses (see `Nova bug 1543625`_)."},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"Cyborg follows the same pattern:"},{"line_number":244,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"0092b6bb_f69d6a44","line":241,"range":{"start_line":236,"start_character":0,"end_line":241,"end_character":52},"in_reply_to":"0da4b0c3_fc1899a4","updated":"2026-09-22 21:19:05.000000000","message":"Understood and done.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":254,"context_line":"* The row exists for internal version tracking, which is"},{"line_number":255,"context_line":"  important for rolling upgrade version negotiation later"},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"Liveness of ``cyborg-api`` is checked through the"},{"line_number":258,"context_line":"``/healthcheck`` HTTP endpoint (see `Related work`_ below),"},{"line_number":259,"context_line":"not through the services API. This mirrors how operators monitor"},{"line_number":260,"context_line":"Nova\u0027s API service in practice."},{"line_number":261,"context_line":""},{"line_number":262,"context_line":".. _Nova bug 1543625: https://bugs.launchpad.net/nova/+bug/1543625"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"Liveness detection"}],"source_content_type":"text/x-rst","patch_set":5,"id":"dc0b8ab7_bcdfc291","line":261,"range":{"start_line":257,"start_character":0,"end_line":261,"end_character":1},"updated":"2026-09-11 14:39:57.000000000","message":"that not entirlly ture\n\nthey monintor that via the micorversion endpoint `/`\n\nwe intetioanly do not enabel the oslo middelware for nova api upstream or in our downstream product.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":254,"context_line":"* The row exists for internal version tracking, which is"},{"line_number":255,"context_line":"  important for rolling upgrade version negotiation later"},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"Liveness of ``cyborg-api`` is checked through the"},{"line_number":258,"context_line":"``/healthcheck`` HTTP endpoint (see `Related work`_ below),"},{"line_number":259,"context_line":"not through the services API. This mirrors how operators monitor"},{"line_number":260,"context_line":"Nova\u0027s API service in practice."},{"line_number":261,"context_line":""},{"line_number":262,"context_line":".. _Nova bug 1543625: https://bugs.launchpad.net/nova/+bug/1543625"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"Liveness detection"}],"source_content_type":"text/x-rst","patch_set":5,"id":"5aaec911_25cfaf3c","line":261,"range":{"start_line":257,"start_character":0,"end_line":261,"end_character":1},"in_reply_to":"dc0b8ab7_bcdfc291","updated":"2026-09-22 21:19:05.000000000","message":"Done","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":278,"context_line":"  ``(now - last_heartbeat) \u003c service_down_time``"},{"line_number":279,"context_line":"* ``state \u003d \"down\"`` otherwise"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"When ``forced_down`` is eventually exposed through the API, it"},{"line_number":282,"context_line":"would also force ``state \u003d \"down\"`` regardless of the heartbeat"},{"line_number":283,"context_line":"timestamp, as Nova does."},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"For WSGI services (``cyborg-api``): WSGI services do not"},{"line_number":286,"context_line":"heartbeat, so ``last_seen_up`` is always NULL and ``state`` is"},{"line_number":287,"context_line":"not meaningful. WSGI services are filtered out of the default"}],"source_content_type":"text/x-rst","patch_set":5,"id":"9d886404_55bdb70d","line":284,"range":{"start_line":281,"start_character":0,"end_line":284,"end_character":1},"updated":"2026-09-11 14:39:57.000000000","message":"sure but i dont see a use case to ever support that for cybrog so unless you have one lest remove this fro now. we can alwasy add this if we ever have a reson to in the futre","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":278,"context_line":"  ``(now - last_heartbeat) \u003c service_down_time``"},{"line_number":279,"context_line":"* ``state \u003d \"down\"`` otherwise"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"When ``forced_down`` is eventually exposed through the API, it"},{"line_number":282,"context_line":"would also force ``state \u003d \"down\"`` regardless of the heartbeat"},{"line_number":283,"context_line":"timestamp, as Nova does."},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"For WSGI services (``cyborg-api``): WSGI services do not"},{"line_number":286,"context_line":"heartbeat, so ``last_seen_up`` is always NULL and ``state`` is"},{"line_number":287,"context_line":"not meaningful. WSGI services are filtered out of the default"}],"source_content_type":"text/x-rst","patch_set":5,"id":"24a1e3b1_3c6fb02d","line":284,"range":{"start_line":281,"start_character":0,"end_line":284,"end_character":1},"in_reply_to":"9d886404_55bdb70d","updated":"2026-09-22 21:19:05.000000000","message":"I removed this along with the other \"disabled\" text.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":282,"context_line":"would also force ``state \u003d \"down\"`` regardless of the heartbeat"},{"line_number":283,"context_line":"timestamp, as Nova does."},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"For WSGI services (``cyborg-api``): WSGI services do not"},{"line_number":286,"context_line":"heartbeat, so ``last_seen_up`` is always NULL and ``state`` is"},{"line_number":287,"context_line":"not meaningful. WSGI services are filtered out of the default"},{"line_number":288,"context_line":"GET ``/v2/services`` list response. When retrieved via"},{"line_number":289,"context_line":"``binary\u003dcyborg-api`` or GET ``/v2/services/{uuid}``, the"},{"line_number":290,"context_line":"``state`` and ``last_seen_up`` fields are omitted from the"},{"line_number":291,"context_line":"response."},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"REST API: /v2/services"},{"line_number":294,"context_line":"----------------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"a87ee66e_93317eab","line":291,"range":{"start_line":285,"start_character":3,"end_line":291,"end_character":8},"updated":"2026-09-11 14:39:57.000000000","message":"again im -1 on filtering\n\ni woudl prefer to remove state entirly and the heatbibng then do that honstly","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":282,"context_line":"would also force ``state \u003d \"down\"`` regardless of the heartbeat"},{"line_number":283,"context_line":"timestamp, as Nova does."},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"For WSGI services (``cyborg-api``): WSGI services do not"},{"line_number":286,"context_line":"heartbeat, so ``last_seen_up`` is always NULL and ``state`` is"},{"line_number":287,"context_line":"not meaningful. WSGI services are filtered out of the default"},{"line_number":288,"context_line":"GET ``/v2/services`` list response. When retrieved via"},{"line_number":289,"context_line":"``binary\u003dcyborg-api`` or GET ``/v2/services/{uuid}``, the"},{"line_number":290,"context_line":"``state`` and ``last_seen_up`` fields are omitted from the"},{"line_number":291,"context_line":"response."},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"REST API: /v2/services"},{"line_number":294,"context_line":"----------------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"7036bec1_aad8e0ec","line":291,"range":{"start_line":285,"start_character":3,"end_line":291,"end_character":8},"in_reply_to":"a87ee66e_93317eab","updated":"2026-09-22 21:19:05.000000000","message":"Done","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":392,"context_line":"            \"created_at\": \"2026-07-15T08:00:00Z\""},{"line_number":393,"context_line":"        }"},{"line_number":394,"context_line":"    }"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"**DELETE /v2/services/{uuid}** (delete stale service entry)"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"Normal response code: 204"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"Error response codes: 404"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"Removes a service entry from the database. Intended for"},{"line_number":403,"context_line":"decommissioned hosts whose service entries are stale. Returns"},{"line_number":404,"context_line":"404 if the UUID does not match a known service."},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"If the service is still running when its row is deleted, the next"},{"line_number":407,"context_line":"``_report_state()`` call will fail to find the row. The periodic"},{"line_number":408,"context_line":"task catches the ``ServiceNotFound`` exception (a new addition"},{"line_number":409,"context_line":"to ``cyborg/exception.py``) and re-registers the service,"},{"line_number":410,"context_line":"creating a new row with a new UUID. This means deleting a live"},{"line_number":411,"context_line":"service\u0027s entry is harmless but ineffective -- the entry"},{"line_number":412,"context_line":"reappears on the next heartbeat. This is the expected behavior:"},{"line_number":413,"context_line":"DELETE is for cleaning up entries for services that are no longer"},{"line_number":414,"context_line":"running, not for removing live services. (Nova handles this"},{"line_number":415,"context_line":"differently -- it logs an error and sets a disconnected flag,"},{"line_number":416,"context_line":"requiring a service restart. The self-healing re-registration"},{"line_number":417,"context_line":"approach is simpler for operators.) Transient DB errors during"},{"line_number":418,"context_line":"``_report_state()`` are handled with bounded retry."},{"line_number":419,"context_line":""},{"line_number":420,"context_line":"All endpoints are admin-only. A PUT endpoint for administrative"},{"line_number":421,"context_line":"actions is not part of this spec; it would depend on the"}],"source_content_type":"text/x-rst","patch_set":5,"id":"8b8e42b5_315bcada","line":418,"range":{"start_line":395,"start_character":1,"end_line":418,"end_character":51},"updated":"2026-09-11 14:39:57.000000000","message":"so this should likely be async and dispatch to the conductor to do the actual clean up.\n\nI think when we are deleting we need to also ensure the service is down, because we need to clean up the placmeent resouce providers and deployable if the binary is cyborg-agent. we coudl supprot --force to overried this check or require disablinging first to bypass the downtime but we need to clean up the deployable and rps as part of the service delete\n\ni dont think its ok to allow deleting a live service and rely on self heling via re regsistraion.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":392,"context_line":"            \"created_at\": \"2026-07-15T08:00:00Z\""},{"line_number":393,"context_line":"        }"},{"line_number":394,"context_line":"    }"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"**DELETE /v2/services/{uuid}** (delete stale service entry)"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"Normal response code: 204"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"Error response codes: 404"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"Removes a service entry from the database. Intended for"},{"line_number":403,"context_line":"decommissioned hosts whose service entries are stale. Returns"},{"line_number":404,"context_line":"404 if the UUID does not match a known service."},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"If the service is still running when its row is deleted, the next"},{"line_number":407,"context_line":"``_report_state()`` call will fail to find the row. The periodic"},{"line_number":408,"context_line":"task catches the ``ServiceNotFound`` exception (a new addition"},{"line_number":409,"context_line":"to ``cyborg/exception.py``) and re-registers the service,"},{"line_number":410,"context_line":"creating a new row with a new UUID. This means deleting a live"},{"line_number":411,"context_line":"service\u0027s entry is harmless but ineffective -- the entry"},{"line_number":412,"context_line":"reappears on the next heartbeat. This is the expected behavior:"},{"line_number":413,"context_line":"DELETE is for cleaning up entries for services that are no longer"},{"line_number":414,"context_line":"running, not for removing live services. (Nova handles this"},{"line_number":415,"context_line":"differently -- it logs an error and sets a disconnected flag,"},{"line_number":416,"context_line":"requiring a service restart. The self-healing re-registration"},{"line_number":417,"context_line":"approach is simpler for operators.) Transient DB errors during"},{"line_number":418,"context_line":"``_report_state()`` are handled with bounded retry."},{"line_number":419,"context_line":""},{"line_number":420,"context_line":"All endpoints are admin-only. A PUT endpoint for administrative"},{"line_number":421,"context_line":"actions is not part of this spec; it would depend on the"}],"source_content_type":"text/x-rst","patch_set":5,"id":"25d82ae0_3e9c199e","line":418,"range":{"start_line":395,"start_character":1,"end_line":418,"end_character":51},"in_reply_to":"8b8e42b5_315bcada","updated":"2026-09-22 21:19:05.000000000","message":"Makes sense, done.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":420,"context_line":"All endpoints are admin-only. A PUT endpoint for administrative"},{"line_number":421,"context_line":"actions is not part of this spec; it would depend on the"},{"line_number":422,"context_line":"``disabled`` / ``forced_down`` semantics being resolved first --"},{"line_number":423,"context_line":"see the open design questions below."},{"line_number":424,"context_line":""},{"line_number":425,"context_line":"Policy rules"},{"line_number":426,"context_line":"~~~~~~~~~~~~"}],"source_content_type":"text/x-rst","patch_set":5,"id":"15ffd591_43277f8e","line":423,"updated":"2026-09-11 14:39:57.000000000","message":"ack i think disabel does make sesne but we dont need to supprot POST to create service via the api.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":420,"context_line":"All endpoints are admin-only. A PUT endpoint for administrative"},{"line_number":421,"context_line":"actions is not part of this spec; it would depend on the"},{"line_number":422,"context_line":"``disabled`` / ``forced_down`` semantics being resolved first --"},{"line_number":423,"context_line":"see the open design questions below."},{"line_number":424,"context_line":""},{"line_number":425,"context_line":"Policy rules"},{"line_number":426,"context_line":"~~~~~~~~~~~~"}],"source_content_type":"text/x-rst","patch_set":5,"id":"16f995a6_e2de1bf9","line":423,"in_reply_to":"15ffd591_43277f8e","updated":"2026-09-22 21:19:05.000000000","message":"Acknowledged","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":false,"context_lines":[{"line_number":471,"context_line":"    ]"},{"line_number":472,"context_line":""},{"line_number":473,"context_line":"A ``cyborg:service:update`` policy would be added if a PUT"},{"line_number":474,"context_line":"endpoint is implemented in a future spec."},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"Open design questions"},{"line_number":477,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"98f86db9_346ff000","line":474,"updated":"2026-09-11 14:39:57.000000000","message":"ya so the time line in https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_135/openstack/1351fe6d8c7e4fa4a65a7669efce8e72/docs/specs/2026.2/implemented/consistent-and-secure-rbac.html#other-deployer-impact\n\nis \n```\n2026.2 — Cyborg will override enforce_new_defaults to False. Both old and new authorization paths are accepted. Operators may opt in early by setting enforce_new_defaults \u003d True.\n\n2027.1 — The override will be removed and new defaults will be enforced by default. The legacy rules will be formally deprecated with warnings announcing their removal. Operators not yet ready can set enforce_new_defaults \u003d False explicitly for one more cycle.\n\n2027.2 — The deprecated legacy rules and bridges will be removed.\n```\n\nso for 2027.1 on i think we shoudl only use the new rbac rules for new apis\n\n\nso this is consitent with that","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":510,"context_line":"to the schema."},{"line_number":511,"context_line":"Several options exist:"},{"line_number":512,"context_line":""},{"line_number":513,"context_line":"1. **Stop discovery reporting**: A disabled agent skips its periodic"},{"line_number":514,"context_line":"   ``update_available_resource`` task. Existing devices remain in the"},{"line_number":515,"context_line":"   DB but stop being refreshed. This is the closest analogue to"},{"line_number":516,"context_line":"   Nova\u0027s behavior -- the host\u0027s accelerators go stale and"},{"line_number":517,"context_line":"   eventually become unavailable."},{"line_number":518,"context_line":""},{"line_number":519,"context_line":"2. **Remove Placement resource providers**: When disabled, the agent"},{"line_number":520,"context_line":"   (or conductor on its behalf) deletes or zeros out the resource"},{"line_number":521,"context_line":"   providers for that host\u0027s accelerators. Nova\u0027s scheduler would"},{"line_number":522,"context_line":"   then stop placing accelerator-needing instances there. This is"},{"line_number":523,"context_line":"   more aggressive but gives a cleaner signal."},{"line_number":524,"context_line":""},{"line_number":525,"context_line":"3. **Informational only**: The flags exist for operator visibility"},{"line_number":526,"context_line":"   and coordination (\"I\u0027m about to take this host down\") with no"},{"line_number":527,"context_line":"   Cyborg-enforced behavior. Operators would need to separately"},{"line_number":528,"context_line":"   fence the host in Nova to prevent scheduling."},{"line_number":529,"context_line":""},{"line_number":530,"context_line":"These columns are proposed here for discussion. Whether to"},{"line_number":531,"context_line":"include them in the initial migration, defer them to a follow-up"}],"source_content_type":"text/x-rst","patch_set":5,"id":"0884356b_6a9a8c94","line":528,"range":{"start_line":513,"start_character":0,"end_line":528,"end_character":48},"updated":"2026-09-11 14:39:57.000000000","message":"i think none the above\n\nmy preference woudl be as follows\n\ndisable nodes should reject binding new ARQs\ndisables nodes shoudl add a triat to the RP and set resteved\u003dtotal\n\nwe may (likely will) need to start adding nova scudler filter and weigher to cybrog in the coming relase and one of the first woudl pritsie woudl eb a disable host filter, if we set resteved\u003dtotal we can skip that but i think if we supprot deisabel we need to to have it prevent sccdulign new workloads\n\nremoving rps is problamatic as that wioudl impact existign allcoations which we shoudl not be altering.\n\n\nin terms fo discovery really the discover should only happen on agent start up rather then periodically\n\nwe generally try to cache device discovery in nova.\nits actully cached in nova and in libvirt.\n\ni don\u0027t really want to support adding and removing device while the cyborg agent is running at least not in the near term that feels problematic on the placement side especially the removal until https://bugs.launchpad.net/openstack-cyborg/+bug/2017513 is fixed.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":true,"context_lines":[{"line_number":510,"context_line":"to the schema."},{"line_number":511,"context_line":"Several options exist:"},{"line_number":512,"context_line":""},{"line_number":513,"context_line":"1. **Stop discovery reporting**: A disabled agent skips its periodic"},{"line_number":514,"context_line":"   ``update_available_resource`` task. Existing devices remain in the"},{"line_number":515,"context_line":"   DB but stop being refreshed. This is the closest analogue to"},{"line_number":516,"context_line":"   Nova\u0027s behavior -- the host\u0027s accelerators go stale and"},{"line_number":517,"context_line":"   eventually become unavailable."},{"line_number":518,"context_line":""},{"line_number":519,"context_line":"2. **Remove Placement resource providers**: When disabled, the agent"},{"line_number":520,"context_line":"   (or conductor on its behalf) deletes or zeros out the resource"},{"line_number":521,"context_line":"   providers for that host\u0027s accelerators. Nova\u0027s scheduler would"},{"line_number":522,"context_line":"   then stop placing accelerator-needing instances there. This is"},{"line_number":523,"context_line":"   more aggressive but gives a cleaner signal."},{"line_number":524,"context_line":""},{"line_number":525,"context_line":"3. **Informational only**: The flags exist for operator visibility"},{"line_number":526,"context_line":"   and coordination (\"I\u0027m about to take this host down\") with no"},{"line_number":527,"context_line":"   Cyborg-enforced behavior. Operators would need to separately"},{"line_number":528,"context_line":"   fence the host in Nova to prevent scheduling."},{"line_number":529,"context_line":""},{"line_number":530,"context_line":"These columns are proposed here for discussion. Whether to"},{"line_number":531,"context_line":"include them in the initial migration, defer them to a follow-up"}],"source_content_type":"text/x-rst","patch_set":5,"id":"6b8dedf9_a71d2e7b","line":528,"range":{"start_line":513,"start_character":0,"end_line":528,"end_character":48},"in_reply_to":"0884356b_6a9a8c94","updated":"2026-09-22 21:19:05.000000000","message":"Agreed it feels like we really need a Nova scheduler filter to support disable properly.\n\nI have removed sections about disabling a service and mention it as future work at the end of the REST API section.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":530,"context_line":"These columns are proposed here for discussion. Whether to"},{"line_number":531,"context_line":"include them in the initial migration, defer them to a follow-up"},{"line_number":532,"context_line":"spec, or drop them entirely is a question for reviewers."},{"line_number":533,"context_line":""},{"line_number":534,"context_line":"Alternatives"},{"line_number":535,"context_line":"------------"},{"line_number":536,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"f817e58d_9f9ebd0a","line":533,"updated":"2026-09-11 14:39:57.000000000","message":"so your not plannign to propose the actual per-binary healthcheck endpoint that fine we can do that in a sperate spec.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":true,"context_lines":[{"line_number":530,"context_line":"These columns are proposed here for discussion. Whether to"},{"line_number":531,"context_line":"include them in the initial migration, defer them to a follow-up"},{"line_number":532,"context_line":"spec, or drop them entirely is a question for reviewers."},{"line_number":533,"context_line":""},{"line_number":534,"context_line":"Alternatives"},{"line_number":535,"context_line":"------------"},{"line_number":536,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"c2aebacd_a84c81d5","line":533,"in_reply_to":"f817e58d_9f9ebd0a","updated":"2026-09-22 21:19:05.000000000","message":"I was thinking the health check would be large enough to be its own spec and this spec is already quite large.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":534,"context_line":"Alternatives"},{"line_number":535,"context_line":"------------"},{"line_number":536,"context_line":""},{"line_number":537,"context_line":"* **External monitoring only (Prometheus/Nagios)**: Does not provide"},{"line_number":538,"context_line":"  service status through the OpenStack API. Operators expect this"},{"line_number":539,"context_line":"  from OpenStack services."},{"line_number":540,"context_line":"* **Reuse Nova\u0027s servicegroup**: Creates a cross-service database"},{"line_number":541,"context_line":"  dependency. Each project should track its own services."},{"line_number":542,"context_line":"* **Poll the RPC bus directly**: Unreliable when the message bus is"}],"source_content_type":"text/x-rst","patch_set":5,"id":"88be7291_14ae4b86","line":539,"range":{"start_line":537,"start_character":0,"end_line":539,"end_character":26},"updated":"2026-09-11 14:39:57.000000000","message":"this is hwoe they shoudl be the primary way we to do health moniroting\n\nwe can provide basic api expsoe on which host each serviec is deploy and a very basic heatbeat but anything else shoudl not  be in the api.\n\nthe new service api is not a monitoring API and i don\u0027t think we shoudl add one in the future.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":534,"context_line":"Alternatives"},{"line_number":535,"context_line":"------------"},{"line_number":536,"context_line":""},{"line_number":537,"context_line":"* **External monitoring only (Prometheus/Nagios)**: Does not provide"},{"line_number":538,"context_line":"  service status through the OpenStack API. Operators expect this"},{"line_number":539,"context_line":"  from OpenStack services."},{"line_number":540,"context_line":"* **Reuse Nova\u0027s servicegroup**: Creates a cross-service database"},{"line_number":541,"context_line":"  dependency. Each project should track its own services."},{"line_number":542,"context_line":"* **Poll the RPC bus directly**: Unreliable when the message bus is"}],"source_content_type":"text/x-rst","patch_set":5,"id":"a9fbb0a5_5b07bcec","line":539,"range":{"start_line":537,"start_character":0,"end_line":539,"end_character":26},"in_reply_to":"88be7291_14ae4b86","updated":"2026-09-22 21:19:05.000000000","message":"Acknowledged","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":539,"context_line":"  from OpenStack services."},{"line_number":540,"context_line":"* **Reuse Nova\u0027s servicegroup**: Creates a cross-service database"},{"line_number":541,"context_line":"  dependency. Each project should track its own services."},{"line_number":542,"context_line":"* **Poll the RPC bus directly**: Unreliable when the message bus is"},{"line_number":543,"context_line":"  partially available. Also not accessible to operators through a"},{"line_number":544,"context_line":"  standard API."},{"line_number":545,"context_line":""},{"line_number":546,"context_line":"Data Model Impact"},{"line_number":547,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"228f5d02_0300f0af","line":544,"range":{"start_line":542,"start_character":0,"end_line":544,"end_character":15},"updated":"2026-09-11 14:39:57.000000000","message":"its also an internaly interface, we do not supprot wrting to the rpc bus in cyborg or nova so you cant poll it in the normal sence of injecting a request and geting a reply out side of calling the reste api and depeing on it to do it as a side effect of the call.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":true,"context_lines":[{"line_number":539,"context_line":"  from OpenStack services."},{"line_number":540,"context_line":"* **Reuse Nova\u0027s servicegroup**: Creates a cross-service database"},{"line_number":541,"context_line":"  dependency. Each project should track its own services."},{"line_number":542,"context_line":"* **Poll the RPC bus directly**: Unreliable when the message bus is"},{"line_number":543,"context_line":"  partially available. Also not accessible to operators through a"},{"line_number":544,"context_line":"  standard API."},{"line_number":545,"context_line":""},{"line_number":546,"context_line":"Data Model Impact"},{"line_number":547,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"01a44e01_bb4dd9e1","line":544,"range":{"start_line":542,"start_character":0,"end_line":544,"end_character":15},"in_reply_to":"228f5d02_0300f0af","updated":"2026-09-22 21:19:05.000000000","message":"Yeah, I think if anything it would be a \"passive\" sort of monitor on \"when is the last time a RPC message was received\". Also would not consider cloud \"quiet\" times when the cloud is legitimately not very active if callers are just not calling the API that much.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":576,"context_line":""},{"line_number":577,"context_line":"None. Versioned notifications for service lifecycle events (service"},{"line_number":578,"context_line":"up, service down) are future work that would build on this"},{"line_number":579,"context_line":"infrastructure."},{"line_number":580,"context_line":""},{"line_number":581,"context_line":"Other End User Impact"},{"line_number":582,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"33d7f205_2ba31d81","line":579,"updated":"2026-09-11 14:39:57.000000000","message":"ack, we dont really have any notificaon supprot in cybrog today\n\nwe have the basic foncig option and olso messaging infra\nhttps://docs.openstack.org/cyborg/latest/configuration/config-options.html#oslo-messaging-notifications\nhttps://github.com/openstack/cyborg/blob/aef8c27ebd4e2e276bebd65d3635247f1812d5bc/cyborg/common/rpc.py#L44-L46\n\nbut there are no notificions in cyborg today\n\nwe shoudl have a seperate spec to cover addign those in general when we have time\n\ntoday what exits is just left over form the copy paste of the rpc.py form nova\nhttps://github.com/openstack/cyborg/commit/c7b24fda7fbf2c117bcd061be2c6dd9ae88acd56#diff-2487fe2c73a90f69b5d45e37b5f6ae1cd45bae0a3d57eef3722521d3a8e40d85\n\nit was never implemtned","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":576,"context_line":""},{"line_number":577,"context_line":"None. Versioned notifications for service lifecycle events (service"},{"line_number":578,"context_line":"up, service down) are future work that would build on this"},{"line_number":579,"context_line":"infrastructure."},{"line_number":580,"context_line":""},{"line_number":581,"context_line":"Other End User Impact"},{"line_number":582,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"d0b322d9_31652575","line":579,"in_reply_to":"33d7f205_2ba31d81","updated":"2026-09-22 21:19:05.000000000","message":"Acknowledged","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":600,"context_line":"One DB write per service per ``report_interval`` (default 10"},{"line_number":601,"context_line":"seconds). At 500 agents + a few conductors, this is approximately"},{"line_number":602,"context_line":"50 writes/second -- negligible for any modern database. The services"},{"line_number":603,"context_line":"table has at most one row per ``(host, binary)`` pair."},{"line_number":604,"context_line":""},{"line_number":605,"context_line":"Other Deployer Impact"},{"line_number":606,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"9caa19f2_44e5c58f","line":603,"updated":"2026-09-11 14:39:57.000000000","message":"I agree that 50 simple DB updates/second is not a significant load for a\nmodern database by itself, but I don\u0027t think that is the complete performance\nimpact here.\n\nEach heartbeat is an RPC from the cyborg-agent to the conductor, which then\nhas to process the request, acquire a DB connection, perform the update and\ncommit it, and return the RPC reply.\n\nAt the proposed default 10 second interval that means:\n\n```\n500 agents  -\u003e  50 heartbeat RPCs/second\n1000 agents -\u003e 100 heartbeat RPCs/second\n```\n\nThat is a permanent baseline RPC workload on the conductor, in addition to\nthe normal work it already performs.\n\nWe have been bitten by this kind of scaling before. For example, the Nova\n`heal_instance_info_cache_interval` periodic caused significant sustained\nload in production:\n\nhttps://launchpad.net/bugs/1996094\nhttps://redhat.atlassian.net/browse/OSPRH-10744\n\nThat case was more expensive because each periodic operation resulted in\nmultiple Neutron requests per Nova server on each compute host, so the load\nscaled with the number of instances rather than purely with the number of\nagents. In the Cyborg case the scaling is simpler, one heartbeat per agent\nper interval, but it is still a permanent workload that scales directly\nwith the size of the deployment.\n\n\nso i had ai create a mini bench mark to build some intuitiohn around this\n\nhttps://termbin.com/f9gd4\n\n```\n uv run heartbeat_bench.py\nCreating database: cyborg_heartbeat_benchmark_ddb11827\nCreating services table\nPopulating 1000 service records\nReading service UUIDs from database\nLoaded and shuffled 1000 UUIDs\n\nBenchmarking one UPDATE + COMMIT per service\n\nResults\n-------\ntransactions: 1000\ntotal time:   2.075 s\nthroughput:   481.9 txn/s\n\nmin:          0.920 ms\nmean:         2.072 ms\nmedian/p50:   1.779 ms\np90:          2.673 ms\np95:          3.098 ms\np99:          10.562 ms\nmax:          31.792 ms\n\nDropping benchmark database: cyborg_heartbeat_benchmark_ddb11827\n```\n\n\n\nCyborg does not curently have any concurrency beyond the oslo.messaging RPC executor. \nThe default executor pool is 64 workers. At 1000 agents, 100 heartbeat RPCs/sec\nwould therefore consume, on average:\n\n```\n100 ms/request -\u003e 10 workers\n200 ms/request -\u003e 20 workers\n500 ms/request -\u003e 50 workers\n```\n\nAt ~ 10ms on a idel baremetal devstack the default you are suggetion of 10s\nat that scale is probaly going to consume 1-2 rpc workers constatnly to service the heatbeats\nso that is not wildly problmeatic just someting we shoudl be awrae of althought\nthat is before accounting for any other conductor RPC traffic. The average\nrate also assumes the agent periodic tasks are well distributed; if their\ntimers become aligned, the conductor can see significantly larger bursts\nevery heartbeat interval alhtough oslo.service has some skew built in.\n\nSo I would be cautious about describing this as negligible based only on the\nDB write rate.\n\nI also don\u0027t think we need a 10 second heartbeat for this use case. Cyborg\nservice availability does not normally require failure detection on that\ntimescale.\n\nI would prefer a heartbeat interval somewhere in the range of 60-300 seconds,\nwith a service only considered down after roughly 300-900 seconds. For\nexample, a 60 second heartbeat with a 300 second down threshold would tolerate\nfive missed heartbeats while reducing the steady-state load by a factor of\nsix:\n\n```\n500 agents  -\u003e ~8.3 heartbeat RPCs/second\n1000 agents -\u003e ~16.7 heartbeat RPCs/second\n```\n\nA 300 second heartbeat would reduce that further but that is proably not required\nfor perfoamnce and at that point you start having tradeoffs with the usefulness of\nthe data for making any decsions.\n\n\nI\u0027m not opposed to implementing the heartbeat through the conductor, but I\nthink the default interval should reflect the actual failure-detection\nrequirement rather than using `report_interval\u003d10` simply because that is\nalready the agent periodic interval.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":true,"context_lines":[{"line_number":600,"context_line":"One DB write per service per ``report_interval`` (default 10"},{"line_number":601,"context_line":"seconds). At 500 agents + a few conductors, this is approximately"},{"line_number":602,"context_line":"50 writes/second -- negligible for any modern database. The services"},{"line_number":603,"context_line":"table has at most one row per ``(host, binary)`` pair."},{"line_number":604,"context_line":""},{"line_number":605,"context_line":"Other Deployer Impact"},{"line_number":606,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"625a2026_aba95c9e","line":603,"in_reply_to":"9caa19f2_44e5c58f","updated":"2026-09-22 21:19:05.000000000","message":"Fair point and I had taken the default for granted. I have adjusted them to 300 seconds and 900 seconds respectively as you suggested.\n\nIn practice, in my experience, I have to say that even 60 seconds was considered \"fast\" for heartbeating in a larger number of compute hosts, so 300 seconds seems reasonable to me.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":650,"context_line":"Indri+1 conductor and API talking to Indri agents during the"},{"line_number":651,"context_line":"upgrade window."},{"line_number":652,"context_line":""},{"line_number":653,"context_line":"**Must land in Indri (this release):**"},{"line_number":654,"context_line":""},{"line_number":655,"context_line":"* **Services table with version tracking** (this spec). The"},{"line_number":656,"context_line":"  services table must exist and be populated for a full"}],"source_content_type":"text/x-rst","patch_set":5,"id":"096e8228_d41e8482","line":653,"range":{"start_line":653,"start_character":15,"end_line":653,"end_character":35},"updated":"2026-09-11 14:39:57.000000000","message":"```suggestion\n**Must land in 2027.1 (Indri):**\n```","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":650,"context_line":"Indri+1 conductor and API talking to Indri agents during the"},{"line_number":651,"context_line":"upgrade window."},{"line_number":652,"context_line":""},{"line_number":653,"context_line":"**Must land in Indri (this release):**"},{"line_number":654,"context_line":""},{"line_number":655,"context_line":"* **Services table with version tracking** (this spec). The"},{"line_number":656,"context_line":"  services table must exist and be populated for a full"}],"source_content_type":"text/x-rst","patch_set":5,"id":"37ce979c_0b828b6f","line":653,"range":{"start_line":653,"start_character":15,"end_line":653,"end_character":35},"in_reply_to":"096e8228_d41e8482","updated":"2026-09-22 21:19:05.000000000","message":"Done","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":657,"context_line":"  deployment cycle so that Indri+1 code can query"},{"line_number":658,"context_line":"  ``min(version)`` across the agent fleet. Without this, the"},{"line_number":659,"context_line":"  Indri+1 upgrade has no programmatic way to determine what"},{"line_number":660,"context_line":"  agent versions are running."},{"line_number":661,"context_line":""},{"line_number":662,"context_line":"* **Object versioning audit** (Jira spike, not a spec). All"},{"line_number":663,"context_line":"  versioned objects that cross the conductor-agent RPC boundary"}],"source_content_type":"text/x-rst","patch_set":5,"id":"07b2ac7d_6a283466","line":660,"updated":"2026-09-11 14:39:57.000000000","message":"one thing to condier is shoudl we have version numnber fo feature flags","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":true,"context_lines":[{"line_number":657,"context_line":"  deployment cycle so that Indri+1 code can query"},{"line_number":658,"context_line":"  ``min(version)`` across the agent fleet. Without this, the"},{"line_number":659,"context_line":"  Indri+1 upgrade has no programmatic way to determine what"},{"line_number":660,"context_line":"  agent versions are running."},{"line_number":661,"context_line":""},{"line_number":662,"context_line":"* **Object versioning audit** (Jira spike, not a spec). All"},{"line_number":663,"context_line":"  versioned objects that cross the conductor-agent RPC boundary"}],"source_content_type":"text/x-rst","patch_set":5,"id":"9eca94a2_e0adae0e","line":660,"in_reply_to":"07b2ac7d_6a283466","updated":"2026-09-22 21:19:05.000000000","message":"I like the idea of doing that, I feel it works well for Nova for example.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":659,"context_line":"  Indri+1 upgrade has no programmatic way to determine what"},{"line_number":660,"context_line":"  agent versions are running."},{"line_number":661,"context_line":""},{"line_number":662,"context_line":"* **Object versioning audit** (Jira spike, not a spec). All"},{"line_number":663,"context_line":"  versioned objects that cross the conductor-agent RPC boundary"},{"line_number":664,"context_line":"  must have correct ``VERSION`` declarations in Indri, since"},{"line_number":665,"context_line":"  those become the compatibility target that Indri+1\u0027s"}],"source_content_type":"text/x-rst","patch_set":5,"id":"f7e5b2a5_2f0fd9af","line":662,"range":{"start_line":662,"start_character":30,"end_line":662,"end_character":55},"updated":"2026-09-11 14:39:57.000000000","message":"there shoudl be no mention of jira or unofficial third party planning tools in the specs.\n\ni agree that that is not w work item of this spec persay and the audit itself does not need its own spec, blueprint or tracker upstream although it we be good to report on the finding in the irc meetings so we may add it to one fo our working etherpads (https://etherpad.opendev.org/p/cyborg-2027.1-status or","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":true,"context_lines":[{"line_number":659,"context_line":"  Indri+1 upgrade has no programmatic way to determine what"},{"line_number":660,"context_line":"  agent versions are running."},{"line_number":661,"context_line":""},{"line_number":662,"context_line":"* **Object versioning audit** (Jira spike, not a spec). All"},{"line_number":663,"context_line":"  versioned objects that cross the conductor-agent RPC boundary"},{"line_number":664,"context_line":"  must have correct ``VERSION`` declarations in Indri, since"},{"line_number":665,"context_line":"  those become the compatibility target that Indri+1\u0027s"}],"source_content_type":"text/x-rst","patch_set":5,"id":"d9aa0970_3495c24f","line":662,"range":{"start_line":662,"start_character":30,"end_line":662,"end_character":55},"in_reply_to":"f7e5b2a5_2f0fd9af","updated":"2026-09-22 21:19:05.000000000","message":"Sorry 😣 I missed this reference.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":666,"context_line":"  ``obj_make_compatible()`` methods downlevel to. Some objects"},{"line_number":667,"context_line":"  already have this (``ExtARQ``, ``DeviceProfile``); the audit"},{"line_number":668,"context_line":"  verifies completeness and flags any gaps. This should happen"},{"line_number":669,"context_line":"  early in the cycle so that fixes can land before release."},{"line_number":670,"context_line":""},{"line_number":671,"context_line":"**Lands in Indri+1 (code that drives the upgrade):**"},{"line_number":672,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"f2d77748_eeda35f9","line":669,"updated":"2026-09-11 14:39:57.000000000","message":"i wonder i we also want to store the configured rpc version in the db in the service table as well.\n\nmy thinking is that that will allow use to know in advance what max rpc version the target service is capable of suprpoting.\n\nthree is some generic runtime fucntionatly to do that in oslo but i think that involes the client actully checkign with server if the version is supproted\nat least for the conductor it may be simpler to check in the db.\n\ntoughts?","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":true,"context_lines":[{"line_number":666,"context_line":"  ``obj_make_compatible()`` methods downlevel to. Some objects"},{"line_number":667,"context_line":"  already have this (``ExtARQ``, ``DeviceProfile``); the audit"},{"line_number":668,"context_line":"  verifies completeness and flags any gaps. This should happen"},{"line_number":669,"context_line":"  early in the cycle so that fixes can land before release."},{"line_number":670,"context_line":""},{"line_number":671,"context_line":"**Lands in Indri+1 (code that drives the upgrade):**"},{"line_number":672,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"d6bcca52_50baf739","line":669,"in_reply_to":"f2d77748_eeda35f9","updated":"2026-09-22 21:19:05.000000000","message":"Sorry, I think I don\u0027t understand what is the main benefit you\u0027re thinking by having the rpc_version in the services table ... is it to avoid some kind of version negotiation in oslo.messaging?\n\nIf I understand correctly, the way Nova does it with the SERVICE_VERSION_HISTORY, it will look up the RPC version there and similarly avoid any version negotiation in oslo.messaging. And also does not require an additional DB lookup.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":668,"context_line":"  verifies completeness and flags any gaps. This should happen"},{"line_number":669,"context_line":"  early in the cycle so that fixes can land before release."},{"line_number":670,"context_line":""},{"line_number":671,"context_line":"**Lands in Indri+1 (code that drives the upgrade):**"},{"line_number":672,"context_line":""},{"line_number":673,"context_line":"* **``obj_make_compatible()`` methods** for any versioned objects"},{"line_number":674,"context_line":"  bumped in Indri+1, so the Indri+1 conductor can downlevel"}],"source_content_type":"text/x-rst","patch_set":5,"id":"81ef0034_a61300bf","line":671,"range":{"start_line":671,"start_character":11,"end_line":671,"end_character":18},"updated":"2026-09-11 14:39:57.000000000","message":"`2027.2` we shoudl prefer the offial release number over the code names espiclaly when they are not chosen yet.\n\n\n```suggestion\n**Lands in 2027.2 (release n+1):**\n```","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":668,"context_line":"  verifies completeness and flags any gaps. This should happen"},{"line_number":669,"context_line":"  early in the cycle so that fixes can land before release."},{"line_number":670,"context_line":""},{"line_number":671,"context_line":"**Lands in Indri+1 (code that drives the upgrade):**"},{"line_number":672,"context_line":""},{"line_number":673,"context_line":"* **``obj_make_compatible()`` methods** for any versioned objects"},{"line_number":674,"context_line":"  bumped in Indri+1, so the Indri+1 conductor can downlevel"}],"source_content_type":"text/x-rst","patch_set":5,"id":"231e0aa1_5017a3a7","line":671,"range":{"start_line":671,"start_character":11,"end_line":671,"end_character":18},"in_reply_to":"81ef0034_a61300bf","updated":"2026-09-22 21:19:05.000000000","message":"Done","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":674,"context_line":"  bumped in Indri+1, so the Indri+1 conductor can downlevel"},{"line_number":675,"context_line":"  objects when talking to Indri agents."},{"line_number":676,"context_line":""},{"line_number":677,"context_line":"* **RPC version pinning**: ``VERSION_ALIASES`` dicts and an"},{"line_number":678,"context_line":"  ``[upgrade_levels]`` config option with ``auto`` mode. The"},{"line_number":679,"context_line":"  conductor reads ``min(version)`` from the services table"},{"line_number":680,"context_line":"  (populated during the Indri deployment) to automatically cap"},{"line_number":681,"context_line":"  the RPC API version during the upgrade window. This is the"},{"line_number":682,"context_line":"  systematic version of the per-method"},{"line_number":683,"context_line":"  ``can_send_version()`` guards already established by the"},{"line_number":684,"context_line":"  NVMe driver spec."},{"line_number":685,"context_line":""},{"line_number":686,"context_line":"* **``cyborg-status upgrade check``**: queries the services"},{"line_number":687,"context_line":"  table to verify all agents have been upgraded before enabling"}],"source_content_type":"text/x-rst","patch_set":5,"id":"ec3192fe_3ba9cadb","line":684,"range":{"start_line":677,"start_character":0,"end_line":684,"end_character":19},"updated":"2026-09-11 14:39:57.000000000","message":"ya so im not entirely sure if we really want to follow nova\u0027s lead on that entrily\n\nwe knwo it works but if we are reading the services tabel anyway why not keep 2 version colums\n\none for the compute service version of the service, and a second for the configured/selectied rpc version.\n\nwe can keep the aliases for the config ideally using the ofilaly release number `2027.1` as the keys but i dont knwo that we need to do \"``min(version)`` from the services\" wehn we can jsut do Service.get_by_host_and_binary(host, binary).rpc_version","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":true,"context_lines":[{"line_number":674,"context_line":"  bumped in Indri+1, so the Indri+1 conductor can downlevel"},{"line_number":675,"context_line":"  objects when talking to Indri agents."},{"line_number":676,"context_line":""},{"line_number":677,"context_line":"* **RPC version pinning**: ``VERSION_ALIASES`` dicts and an"},{"line_number":678,"context_line":"  ``[upgrade_levels]`` config option with ``auto`` mode. The"},{"line_number":679,"context_line":"  conductor reads ``min(version)`` from the services table"},{"line_number":680,"context_line":"  (populated during the Indri deployment) to automatically cap"},{"line_number":681,"context_line":"  the RPC API version during the upgrade window. This is the"},{"line_number":682,"context_line":"  systematic version of the per-method"},{"line_number":683,"context_line":"  ``can_send_version()`` guards already established by the"},{"line_number":684,"context_line":"  NVMe driver spec."},{"line_number":685,"context_line":""},{"line_number":686,"context_line":"* **``cyborg-status upgrade check``**: queries the services"},{"line_number":687,"context_line":"  table to verify all agents have been upgraded before enabling"}],"source_content_type":"text/x-rst","patch_set":5,"id":"80cec7fe_45903831","line":684,"range":{"start_line":677,"start_character":0,"end_line":684,"end_character":19},"in_reply_to":"ec3192fe_3ba9cadb","updated":"2026-09-22 21:19:05.000000000","message":"So the benefit you\u0027re thinking would be to avoid calculating the min service version. For RPC version capping this is done once (DB lookup) in Nova at service startup time.\n\nBut if we had a rpc_version column, we would be doing a DB lookup at every RPC call ... which could be cached to avoid it being heavier than the min service version check? And avoids needing the SERVICE_VERSION_HISTORY map.\n\nDownside is the version cap becomes scattered among all of the services records rather than central in the SERVICE_VERISON_HISTORY.\n\nTBH the Nova way seems a lot simpler and I\u0027m not sure we would gain much by going with a rpc_version column instead.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":683,"context_line":"  ``can_send_version()`` guards already established by the"},{"line_number":684,"context_line":"  NVMe driver spec."},{"line_number":685,"context_line":""},{"line_number":686,"context_line":"* **``cyborg-status upgrade check``**: queries the services"},{"line_number":687,"context_line":"  table to verify all agents have been upgraded before enabling"},{"line_number":688,"context_line":"  new features. Same pattern as ``nova-status upgrade check``."},{"line_number":689,"context_line":""},{"line_number":690,"context_line":"* **Online data migration infrastructure**: a consistent entry"},{"line_number":691,"context_line":"  point (analogous to"}],"source_content_type":"text/x-rst","patch_set":5,"id":"d7ffca5c_79047f7b","line":688,"range":{"start_line":686,"start_character":1,"end_line":688,"end_character":62},"updated":"2026-09-11 14:39:57.000000000","message":"so that not quite what nova status does\n\nnova status has no impact on if a feature is enabeld  or not\n\nwhat it does do is check that you have appleid al the db schema migration and online data migrations that are requried.\n\nit can have extra checsk for specific upgrade cosndieration but it has no part in determining of a feature is aviable\n\nthat is done by a seperate min service version version theck at runtime and i dont belive nova-status has any awareness of rpc versionl level it just the db it think.\n\nso that not really related to obejct or rpc versioning.\n\nThe closet we get to that is the _check_old_computes check\n\nhttps://github.com/openstack/nova/blob/master/nova/cmd/status.py#L255\nhttps://github.com/openstack/nova/blob/master/nova/cmd/status.py#L307\n\ni assume your propsoing we would ahve a cyborg-agent too old check?\n\ni tried to codify some upgrade expections in 2026.1 \nhttps://github.com/openstack/cyborg/blob/master/doc/source/admin/upgrade.rst\nmainbly based on https://opendev.org/openstack/grenade#Theory%20of%20Upgrade\n\nand the highlevle workflow but we will need to extend that doc as part fo this work","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":true,"context_lines":[{"line_number":683,"context_line":"  ``can_send_version()`` guards already established by the"},{"line_number":684,"context_line":"  NVMe driver spec."},{"line_number":685,"context_line":""},{"line_number":686,"context_line":"* **``cyborg-status upgrade check``**: queries the services"},{"line_number":687,"context_line":"  table to verify all agents have been upgraded before enabling"},{"line_number":688,"context_line":"  new features. Same pattern as ``nova-status upgrade check``."},{"line_number":689,"context_line":""},{"line_number":690,"context_line":"* **Online data migration infrastructure**: a consistent entry"},{"line_number":691,"context_line":"  point (analogous to"}],"source_content_type":"text/x-rst","patch_set":5,"id":"9f765a49_b0d4b6c3","line":688,"range":{"start_line":686,"start_character":1,"end_line":688,"end_character":62},"in_reply_to":"d7ffca5c_79047f7b","updated":"2026-09-22 21:19:05.000000000","message":"\u003e i assume your propsoing we would ahve a cyborg-agent too old check?\n\nNo, sorry, this was just meant to say having a `cyborg-status upgrade check` CLI would be a piece of the rolling upgrades picture. I\u0027ll rewrite it.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"90849609e1be6d1bd84ace70fdc03aa54a24e67d","unresolved":true,"context_lines":[{"line_number":718,"context_line":"failing, and the orchestrator stops routing traffic before the"},{"line_number":719,"context_line":"process is stopped."},{"line_number":720,"context_line":""},{"line_number":721,"context_line":"Adding this to Cyborg is a minimal change (a few lines in"},{"line_number":722,"context_line":"``etc/cyborg/api-paste.ini``, no code) and can land independently"},{"line_number":723,"context_line":"of this spec as a standalone patch. It is noted here for"},{"line_number":724,"context_line":"completeness -- together with the services API, it provides the"},{"line_number":725,"context_line":"full operability picture: ``/healthcheck`` for infrastructure-level"},{"line_number":726,"context_line":"probes, ``/v2/services`` for operator-level visibility and control."},{"line_number":727,"context_line":""},{"line_number":728,"context_line":""},{"line_number":729,"context_line":"Implementation"},{"line_number":730,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":5,"id":"9658c9de_32b86565","line":727,"range":{"start_line":721,"start_character":1,"end_line":727,"end_character":1},"updated":"2026-09-11 14:39:57.000000000","message":"it is but i dont want to do that\n\nit add no value over hittign the microversion discovery endpoint `/` \nand it make it harder to eventuly supprot real /healthcheck endpoint so \n-1 on addign this. \n\nas much as i do not like using past-deploy to allwo configureabl middleware\nif some one really want to sue this today hte can do that already via api-paste.ini \nso i do not want to add this to the default one.\nhttps://github.com/openstack/cyborg/blob/master/etc/cyborg/api-paste.ini","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"861710076bcb3ab503f720f1ef0a1b060693a214","unresolved":false,"context_lines":[{"line_number":718,"context_line":"failing, and the orchestrator stops routing traffic before the"},{"line_number":719,"context_line":"process is stopped."},{"line_number":720,"context_line":""},{"line_number":721,"context_line":"Adding this to Cyborg is a minimal change (a few lines in"},{"line_number":722,"context_line":"``etc/cyborg/api-paste.ini``, no code) and can land independently"},{"line_number":723,"context_line":"of this spec as a standalone patch. It is noted here for"},{"line_number":724,"context_line":"completeness -- together with the services API, it provides the"},{"line_number":725,"context_line":"full operability picture: ``/healthcheck`` for infrastructure-level"},{"line_number":726,"context_line":"probes, ``/v2/services`` for operator-level visibility and control."},{"line_number":727,"context_line":""},{"line_number":728,"context_line":""},{"line_number":729,"context_line":"Implementation"},{"line_number":730,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":5,"id":"1063827a_0fe3e245","line":727,"range":{"start_line":721,"start_character":1,"end_line":727,"end_character":1},"in_reply_to":"9658c9de_32b86565","updated":"2026-09-22 21:19:05.000000000","message":"That\u0027s fair, I have removed this.","commit_id":"ad2ad4eb1e0f419bbc683b3d371d24d8d50b33cb"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"419c250c32d337b8a2df56344b461fe06bb40fd0","unresolved":false,"context_lines":[{"line_number":233,"context_line":"``Service`` record on startup with ``binary \u003d \"cyborg-api\"``, ``topic \u003d None``,"},{"line_number":234,"context_line":"``report_count \u003d 0``."},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"Unlike Nova, which filters WSGI services out of its os-services list response,"},{"line_number":237,"context_line":"Cyborg includes them. Hiding WSGI services prevents operators from seeing where"},{"line_number":238,"context_line":"all API services are deployed and whether they are all running the same"},{"line_number":239,"context_line":"version, which undermines the purpose of the services API. WSGI service rows"}],"source_content_type":"text/x-rst","patch_set":6,"id":"8731c18a_c42f573a","line":236,"updated":"2026-09-22 21:34:43.000000000","message":"The spec justifies including WSGI (cyborg-api) services in GET /v2/services by saying that hiding them prevents operators from seeing \"whether they are all running the same version\". However, the response schema section explicitly excludes the ``version`` column from the API response as \"internal to rolling upgrade negotiation\", so the proposed API exposes no version information for any service and the stated operator goal cannot be met.\n\n**Severity**: WARNING | **Confidence**: 0.85\n\n**Impact**: The documented operator use case (comparing service versions across hosts) is not satisfiable through the proposed API, so reviewers and implementers would build to a rationale the API contradicts; adding the field later would require another API/microversion change instead of getting the schema right now.\n\n**Suggestion**:\nEither include ``version`` in the GET /v2/services and GET /v2/services/{uuid} responses (matching the stated rationale and Nova\u0027s services API), or reword the WSGI rationale to claim only deployment-location visibility rather than version visibility.","commit_id":"709ce6ed53e9baefd073b9f59a013c89ae4d8632"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"419c250c32d337b8a2df56344b461fe06bb40fd0","unresolved":false,"context_lines":[{"line_number":559,"context_line":"Road to rolling upgrade support"},{"line_number":560,"context_line":"~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"},{"line_number":561,"context_line":""},{"line_number":562,"context_line":"This spec is the first of several steps needed to support rolling upgrades"},{"line_number":563,"context_line":"from 2027.1 (Indri) onwards. Since 2027.1 is the first product release,"},{"line_number":564,"context_line":"there is no prior release to upgrade from. The upgrade path this work"},{"line_number":565,"context_line":"enables is 2027.1 to 2027.2: a 2027.2 conductor and API talking to 2027.1"}],"source_content_type":"text/x-rst","patch_set":6,"id":"805a5eed_ff52609c","line":562,"updated":"2026-09-22 21:34:43.000000000","message":"The upgrade roadmap states \"Since 2027.1 is the first product release, there is no prior release to upgrade from.\" Cyborg has shipped many releases before 2027.1: this repository contains specs from Pike through 2026.2, and the commit immediately preceding this change moved the 2026.2 consistent-and-secure-rbac spec to implemented/ because it \"has been implemented in the 2026.2 release\". The defensible rationale is that the services table does not exist before 2027.1, not that 2027.1 is the first release.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: A factually wrong claim in the permanent spec misleads readers about Cyborg\u0027s upgrade story; it suggests releases prior to 2027.1 do not exist or are not upgradable, obscuring the real reason the first supported rolling-upgrade window is 2027.1 -\u003e 2027.2 (no services table/versions recorded before 2027.1).\n\n**Suggestion**:\nReword to state the actual constraint, e.g. \"Because the services table does not exist before 2027.1, there is no recorded service/version data from a prior release, so the first supported rolling-upgrade window is 2027.1 to 2027.2.\"","commit_id":"709ce6ed53e9baefd073b9f59a013c89ae4d8632"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"419c250c32d337b8a2df56344b461fe06bb40fd0","unresolved":false,"context_lines":[{"line_number":709,"context_line":"  heartbeat"},{"line_number":710,"context_line":"* Neutron agents API: ``neutron/db/agents_db.py``,"},{"line_number":711,"context_line":"  ``neutron/objects/agent.py``, ``neutron_lib/api/definitions/agent.py``"},{"line_number":712,"context_line":"* Cyborg consistent-and-secure-rbac spec (2026.2):"},{"line_number":713,"context_line":"  ``specs/2026.2/approved/consistent-and-secure-rbac.rst`` (establishes the"},{"line_number":714,"context_line":"  policy authoring pattern used here)"},{"line_number":715,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"ac2850fa_212b9d4c","line":712,"updated":"2026-09-22 21:34:43.000000000","message":"The References section points readers to ``specs/2026.2/approved/consistent-and-secure-rbac.rst``, but that file was moved to ``specs/2026.2/implemented/consistent-and-secure-rbac.rst`` by commit 7f137a0, which is the base commit of this review. The cited path does not exist in the repository state this change would merge into.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Impact**: Readers following the reference (in the rendered docs or the repository) land on a nonexistent path and cannot find the spec that defines the policy authoring pattern this spec relies on.\n\n**Recommendation**:\nUpdate the path to ``specs/2026.2/implemented/consistent-and-secure-rbac.rst``.","commit_id":"709ce6ed53e9baefd073b9f59a013c89ae4d8632"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"9c5dd922f406da274f1eb213c2de72a7eee63c4e","unresolved":false,"context_lines":[{"line_number":229,"context_line":"message bus topic and cannot heartbeat through the same periodic task mechanism"},{"line_number":230,"context_line":"as the RPC services."},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"Modify ``WSGIService.start()`` in ``cyborg/common/service.py`` to register a"},{"line_number":233,"context_line":"``Service`` record on startup with ``binary \u003d \"cyborg-api\"``, ``topic \u003d None``,"},{"line_number":234,"context_line":"``report_count \u003d 0``."},{"line_number":235,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"5045a830_bd3353bb","line":232,"updated":"2026-09-23 15:35:37.000000000","message":"For cyborg-agent, the find-or-create flow is defined, but two entry points into it are not: (a) the local service_uuid file exists yet no row matches that UUID (database re-created, or the row deleted through the new DELETE endpoint) — the flow falls to an ambiguous \u0027Otherwise\u0027 branch and \u0027create a new Service record and write its UUID\u0027 does not say whether the persisted UUID is reused; (b) get_by_host_and_binary finds a row while the local UUID file is missing — adopting vs. updating that row is unspecified, and a naive create violates the (host, binary) unique constraint. For cyborg-api, \u0027register a Service record on startup\u0027 never says what happens when the row already exists, which occurs on every API restart and on any multi-worker deployment.\n\n**Severity**: WARNING | **Confidence**: 0.82\n\n**Impact**: Implementers must guess unspecified branches. A naive implementation hits DBDuplicateEntry on the (host, binary) unique constraint on API restart or second worker, or silently overwrites/recreates rows with a different UUID than the persisted file, recreating exactly the stale-row/orphaned-FK problem the stable-UUID mechanism exists to prevent.\n\n**Suggestion**:\nMake registration explicitly find-or-create for all binaries: on startup, get_by_host_and_binary first; if found, update topic/version (and adopt it if no local UUID exists); only create when no row exists. State that cyborg-api reuses its existing row on restart, and define the agent\u0027s behavior when the UUID lookup misses but the host/binary row exists (reuse row and rewrite the file with its UUID).","commit_id":"8dbf8ff957eb6e4f672556d3cc66a67d6f6db764"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"9c5dd922f406da274f1eb213c2de72a7eee63c4e","unresolved":false,"context_lines":[{"line_number":242,"context_line":"Liveness detection"},{"line_number":243,"context_line":"------------------"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"The ``state`` field is computed at API response time (not stored) and takes one"},{"line_number":246,"context_line":"of two values: ``\"up\"`` or ``\"down\"``. For RPC services:"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"* Use ``last_seen_up`` if not NULL, otherwise fall back to ``created_at``. This"}],"source_content_type":"text/x-rst","patch_set":7,"id":"68430b23_7433b115","line":245,"updated":"2026-09-23 15:35:37.000000000","message":"The Liveness detection section opens with an unqualified statement that the computed state field \u0027takes one of two values: \"up\" or \"down\"\u0027, but the same section and both response examples return state: null for WSGI cyborg-api services. API reference writers and client/SDK authors following the opening sentence will model a two-value enum and mishandle the third, null case the examples show.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: An inaccurate field contract propagates into the API reference and client code: python-cyborgclient and openstacksdk consumers parsing state into an up/down enum fail on cyborg-api rows. Documentation fix only; no runtime behavior in this change is affected.\n\n**Recommendation**:\nQualify the opening sentence, e.g. \u0027For RPC services state is up or down; for WSGI services state is null\u0027, so the field contract matches the examples and the REST API Impact section.","commit_id":"8dbf8ff957eb6e4f672556d3cc66a67d6f6db764"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"9c5dd922f406da274f1eb213c2de72a7eee63c4e","unresolved":false,"context_lines":[{"line_number":461,"context_line":"Data Model Impact"},{"line_number":462,"context_line":"-----------------"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"One new table (``services``), one Alembic migration. No changes to existing"},{"line_number":465,"context_line":"tables."},{"line_number":466,"context_line":""},{"line_number":467,"context_line":"REST API Impact"}],"source_content_type":"text/x-rst","patch_set":7,"id":"abedac5f_a3de9b6e","line":464,"updated":"2026-09-23 15:35:37.000000000","message":"The Proposed Change adds a BigInteger service_id FK column to the existing deployables table, and DELETE cleanup destroys deployables \u0027looked up via the service_id FK\u0027. Yet Data Model Impact claims \u0027No changes to existing tables\u0027 and Upgrade Impact claims \u0027No data migration for existing tables is required\u0027. Both statements are false given the proposed FK. The sections also omit the backfill needed for rows that exist before the column is added: every pre-existing deployable would have NULL service_id, so the FK-based cleanup lookup silently skips them.\n\n**Severity**: WARNING | **Confidence**: 0.92\n\n**Impact**: An implementer trusting Data Model Impact/Upgrade Impact will skip the deployables migration and backfill. DELETE cleanup then finds no deployables via service_id for any accelerator registered before the upgrade, so deployables and Placement resource providers of decommissioned hosts are silently left behind, and upgrade planning understates the required work.\n\n**Suggestion**:\nCorrect Data Model Impact to list the deployables.service_id column as a second schema change (its own Alembic migration) and specify how existing deployable rows are backfilled with the right services.id (e.g., join on host/binary after services populate), or explicitly state the column is nullable and legacy rows are reconciled by X. Amend Upgrade Impact accordingly.","commit_id":"8dbf8ff957eb6e4f672556d3cc66a67d6f6db764"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"0f1b2b4a123cb6d79831fb16cda7132f3be08736","unresolved":false,"context_lines":[{"line_number":175,"context_line":"RPC services (cyborg-conductor, cyborg-agent)"},{"line_number":176,"context_line":"~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"Stable service UUID"},{"line_number":179,"context_line":"~~~~~~~~~~~~~~~~~~~"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"To detect host renames and prevent duplicate service rows, each"}],"source_content_type":"text/x-rst","patch_set":8,"id":"50f9c54d_c59e0c3f","line":178,"updated":"2026-09-23 18:39:36.000000000","message":"\u0027RPC services (cyborg-conductor, cyborg-agent)\u0027 (line 175) and \u0027Stable service UUID\u0027 (line 178) both use tilde underlines, so reStructuredText renders them as sibling sections under \u0027Service registration and heartbeat\u0027. The \u0027RPC services\u0027 section therefore has no body, and the RPCService.start() find-or-create registration and heartbeat flow (lines 198-231), which applies to both RPC services, renders inside \u0027Stable service UUID\u0027 instead of under its intended parent.\n\n**Severity**: SUGGESTION | **Confidence**: 0.95\n\n**Impact**: The rendered spec and the sphinx toctree show an empty \u0027RPC services (cyborg-conductor, cyborg-agent)\u0027 section and place the startup registration and heartbeat procedure under \u0027Stable service UUID\u0027, misrepresenting the design structure for readers of this long-lived design document.\n\n**Recommendation**:\nGive \u0027Stable service UUID\u0027 a deeper adornment (e.g. triple-quote) so it nests under \u0027RPC services (cyborg-conductor, cyborg-agent)\u0027 at the same level as intended, or move the \u0027Modify RPCService.start()\u0027 registration content up into the \u0027RPC services\u0027 section body.","commit_id":"58ec1b12b01baea1ddfda80a9be2177b60f41234"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"0f1b2b4a123cb6d79831fb16cda7132f3be08736","unresolved":false,"context_lines":[{"line_number":579,"context_line":"* During a rolling upgrade, old-code services simply do not appear in the"},{"line_number":580,"context_line":"  services list until they are restarted with new code. This is acceptable"},{"line_number":581,"context_line":"  because the services API is new -- there is no prior behavior to preserve."},{"line_number":582,"context_line":"* Existing deployable rows require a one-time backfill migration in 2027.1 to"},{"line_number":583,"context_line":"  populate ``service_id`` by resolving the matching service row (for example"},{"line_number":584,"context_line":"  via ``host`` and agent ``binary``). Until backfilled, those rows do not"},{"line_number":585,"context_line":"  participate in service-id-based cleanup."}],"source_content_type":"text/x-rst","patch_set":8,"id":"7e9eba1b_14098b5b","line":582,"updated":"2026-09-23 18:39:36.000000000","message":"The Upgrade Impact requires a \u0027one-time backfill migration in 2027.1\u0027 to populate deployables.service_id \u0027by resolving the matching service row (for example via host and agent binary)\u0027. But the services table is created empty by this same upgrade, and rows only appear \u0027automatically on restart with new code\u0027, which happens after DB migrations in a rolling upgrade (the spec itself notes old-code services \u0027do not appear in the services list until they are restarted with new code\u0027). At migration time there are no service rows to resolve, so a one-time migration backfills nothing. The online-data-migration infrastructure that could re-run such a backfill is explicitly deferred to 2027.2.\n\n**Severity**: WARNING | **Confidence**: 0.85\n\n**Impact**: Nothing re-runs the one-time migration after services re-register, so pre-existing deployable rows permanently keep service_id \u003d NULL. The spec states NULL rows \u0027are not included in service-id-based delete cleanup\u0027 (lines 398-401), so the decommissioning workflow that motivates this spec (bug 2150086) silently skips every deployable that existed before the upgrade.\n\n**Suggestion**:\nPopulate service_id at registration time instead: when an agent adopts its service row in RPCService.start(), set service_id on the deployables it manages; or specify the backfill as a repeatable online data migration that operators run after services have re-registered. Update the Upgrade Impact, \u0027Related work\u0027 wording (\u0027handled after backfill completes\u0027), and Work Items to match the chosen mechanism.","commit_id":"58ec1b12b01baea1ddfda80a9be2177b60f41234"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"c3aad2094da2dd8443ca8934b243ac3715123f86","unresolved":false,"context_lines":[{"line_number":390,"context_line":"For ``cyborg-agent`` services, the API dispatches cleanup to the conductor"},{"line_number":391,"context_line":"asynchronously. The conductor:"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"1. Destroys all deployables associated with the service (looked up via the"},{"line_number":394,"context_line":"   ``service_id`` FK on the ``deployables`` table)"},{"line_number":395,"context_line":"2. Deletes the Placement resource providers for those deployables"},{"line_number":396,"context_line":"3. Deletes the service row from the database"}],"source_content_type":"text/x-rst","patch_set":9,"id":"1b490cba_83e807c7","line":393,"updated":"2026-09-23 19:03:14.000000000","message":"The DELETE /v2/services/{uuid} flow for cyborg-agent services has the conductor destroy all deployables associated with the service, delete their Placement resource providers, then delete the service row. Deployables are not free-standing rows: accelerator requests bind to deployables (ExtARQ.bind()), deployables form parent/root trees, and attributes/attach handles reference a deployable_id. The spec never says what happens to accelerator requests (ExtARQs) bound to deployables that get destroyed, nor what happens if the deployable destroy step itself fails (only Placement-cleanup errors are defined as non-blocking).\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: An implementation following the spec literally could destroy deployables that have bound accelerator requests, leaving ARQs referencing destroyed deployables (FK violations or requests orphaned on a decommissioned host), or abort mid-cleanup with undefined retry/partial state. This weakens the spec\u0027s headline single-command decommissioning workflow and leaves the destructive API contract incomplete.\n\n**Suggestion**:\nSpecify ARQ handling in the cleanup flow: state whether deletion is refused when bound ARQs exist (e.g., 409), requires prior deallocation, or unbinds/deletes the ExtARQs as part of the flow; state whether child deployables in a parent/root tree are destroyed; and define failure/retry behavior when the deployable destroy step fails, analogous to the documented Placement-error handling.","commit_id":"8fca1c2ae8e1f55ca2c6dea5a4f9b4b28c0a141f"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"c3aad2094da2dd8443ca8934b243ac3715123f86","unresolved":false,"context_lines":[{"line_number":634,"context_line":"  are still registered (a \"too old agent\" check, same as Nova\u0027s"},{"line_number":635,"context_line":"  ``_check_old_computes``)."},{"line_number":636,"context_line":""},{"line_number":637,"context_line":"* **Online data migration infrastructure**: a consistent entry point (analogous"},{"line_number":638,"context_line":"  to ``nova-manage db online_data_migrations``) for future N+1 schema changes"},{"line_number":639,"context_line":"  that must be applied without downtime. The 2027.1 ``deployables.service_id``"},{"line_number":640,"context_line":"  backfill is a one-time migration for this spec."}],"source_content_type":"text/x-rst","patch_set":9,"id":"65466ca4_1a4f3e48","line":637,"updated":"2026-09-23 19:03:14.000000000","message":"Upgrade Impact commits a 2027.1 online data migration run \u0027via cyborg-dbsync online_data_migrations\u0027, and Work Items says to \u0027extend cyborg-dbsync online_data_migrations with a 2027.1 backfill\u0027. The rolling-upgrade roadmap, however, lists \u0027Online data migration infrastructure: a consistent entry point (analogous to nova-manage db online_data_migrations)\u0027 under \u0027Lands in 2027.2 (release N+1)\u0027. The spec thus contradicts itself about which release delivers the entry point that its own 2027.1 upgrade procedure and cleanup semantics depend on.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Deployers and implementers cannot tell whether cyborg-dbsync online_data_migrations must exist (or be extended) in 2027.1 for the documented backfill to work. If the entry point really only lands in 2027.2, the 2027.1 backfill procedure -- which gates service-id-based cleanup of legacy deployables -- has no delivery vehicle.\n\n**Suggestion**:\nReconcile the sections: either move the online data migration entry point into the 2027.1 must-haves because this spec\u0027s backfill depends on it, or clarify that the 2027.1 backfill extends the entry point already proposed for 2026.2 and reword the 2027.2 bullet to cover only the N+1 generalization.","commit_id":"8fca1c2ae8e1f55ca2c6dea5a4f9b4b28c0a141f"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"05e4ed6469cb69ae465da69ad1a2633a4418a2b4","unresolved":false,"context_lines":[{"line_number":267,"context_line":"  means a freshly registered service that has not yet heartbeated is considered"},{"line_number":268,"context_line":"  \"up\" for ``service_down_time`` seconds after creation, rather than"},{"line_number":269,"context_line":"  immediately appearing \"down\"."},{"line_number":270,"context_line":"* ``state \u003d \"up\"`` if"},{"line_number":271,"context_line":"  ``(now - last_heartbeat) \u003c service_down_time``"},{"line_number":272,"context_line":"* ``state \u003d \"down\"`` otherwise"},{"line_number":273,"context_line":""}],"source_content_type":"text/x-rst","patch_set":10,"id":"f1ab0b68_2354a8ea","line":270,"updated":"2026-09-23 19:37:59.000000000","message":"The normative liveness formula is written as \u0027state \u003d \"up\" if (now - last_heartbeat) \u003c service_down_time\u0027, but no field named last_heartbeat exists anywhere in the data model; the column is last_seen_up, with a created_at fallback defined two bullets earlier. The formula introduces a second, undefined name for the same concept.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: An implementer transcribing the formula may look for or invent a last_heartbeat field or differently-named helper, producing code or API documentation that diverges from the actual column name. Low impact, but it is a normative formula in the section implementers will copy.\n\n**Recommendation**:\nUse the real column name in the formula, e.g. state \u003d \"up\" if (now - COALESCE(last_seen_up, created_at)) \u003c service_down_time, matching the fallback rule above it.","commit_id":"a6981650e3dd12564dbdfb641c9e041a32987010"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"5e0d558bc6247335ebfe4166ba3bfd2ba42dc0ad","unresolved":true,"context_lines":[{"line_number":404,"context_line":"``cyborg-dbsync online_data_migrations`` has populated"},{"line_number":405,"context_line":"``deployables.service_id``."},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"If deployable deletion fails, the service row is not deleted. The failure is"},{"line_number":408,"context_line":"logged and the operator can retry deletion after remediation. This spec does"},{"line_number":409,"context_line":"not introduce periodic cleanup tasks; cleanup runs only when an operator"},{"line_number":410,"context_line":"explicitly issues service deletion."}],"source_content_type":"text/x-rst","patch_set":10,"id":"16f124ce_149253d1","line":407,"updated":"2026-10-02 11:01:39.000000000","message":"In this case of failure the api returns a 404?","commit_id":"a6981650e3dd12564dbdfb641c9e041a32987010"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"2a1efbee7b6d512fc8a095517bc89a5b0f897d40","unresolved":true,"context_lines":[{"line_number":404,"context_line":"``cyborg-dbsync online_data_migrations`` has populated"},{"line_number":405,"context_line":"``deployables.service_id``."},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"If deployable deletion fails, the service row is not deleted. The failure is"},{"line_number":408,"context_line":"logged and the operator can retry deletion after remediation. This spec does"},{"line_number":409,"context_line":"not introduce periodic cleanup tasks; cleanup runs only when an operator"},{"line_number":410,"context_line":"explicitly issues service deletion."}],"source_content_type":"text/x-rst","patch_set":10,"id":"95111ebc_82c134c3","line":407,"in_reply_to":"16f124ce_149253d1","updated":"2026-10-06 01:53:25.000000000","message":"No, the proposal is to have service delete be an async action, so from the REST API caller perspective if pre-checks succeed, they will see a 204 and if we later fail to delete the deployable at cyborg-conductor, an error will be logged and we will not delete the service record yet. The caller will only be able to see that the service is not yet deleted.\n\nThis is a tradeoff between making service delete async vs sync. There are multiple steps involved in cleanup for a service deletion so we hesitate to do it synchronously (calling Placement API etc).\n\nI will update the text here to hopefully make it clearer.","commit_id":"a6981650e3dd12564dbdfb641c9e041a32987010"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"05e4ed6469cb69ae465da69ad1a2633a4418a2b4","unresolved":false,"context_lines":[{"line_number":505,"context_line":"* ``GET /v2/services`` -- 200 on success, 403 if not admin"},{"line_number":506,"context_line":"* ``GET /v2/services/{uuid}`` -- 200 on success, 403 if not admin, 404 if not"},{"line_number":507,"context_line":"  found"},{"line_number":508,"context_line":"* ``DELETE /v2/services/{uuid}`` -- 204 on success, 403 if not admin, 404 if"},{"line_number":509,"context_line":"  not found, 409 if the service is still up"},{"line_number":510,"context_line":""},{"line_number":511,"context_line":"All methods require admin privileges."}],"source_content_type":"text/x-rst","patch_set":10,"id":"60fb5f8b_7c66665e","line":508,"updated":"2026-09-23 19:37:59.000000000","message":"The Proposed Change defines two DELETE preconditions: the API returns 409 if the service state is \u0027up\u0027 OR if bound ARQs are still present. The REST API Impact section summarizes the DELETE response codes as \u0027409 if the service is still up\u0027, dropping the bound-ARQ cause entirely.\n\n**Severity**: WARNING | **Confidence**: 0.88\n\n**Impact**: An implementer or API-reference author relying on the summary may implement or document 409 only for the still-up case, silently omitting the bound-ARQ rejection. Client and tooling authors would not handle 409-on-bound-ARQs, and operators would get an undocumented failure mode during decommissioning.\n\n**Suggestion**:\nUpdate the DELETE bullet in REST API Impact to read \u0027409 if the service is still up or still has bound ARQs\u0027 so the summary matches the normative preconditions in Proposed Change.","commit_id":"a6981650e3dd12564dbdfb641c9e041a32987010"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"05e4ed6469cb69ae465da69ad1a2633a4418a2b4","unresolved":false,"context_lines":[{"line_number":726,"context_line":"* Periodic heartbeat reporting"},{"line_number":727,"context_line":"* GET ``/v2/services`` with and without query filters"},{"line_number":728,"context_line":"* DELETE of a down service (verify resource cleanup)"},{"line_number":729,"context_line":"* DELETE of a live service (verify 409 rejection)"},{"line_number":730,"context_line":"* Microversion gating (endpoint absent at older microversions)"},{"line_number":731,"context_line":""},{"line_number":732,"context_line":"No specialized hardware is required. All tests can run in the"}],"source_content_type":"text/x-rst","patch_set":10,"id":"05d5f7cb_ca4123bf","line":729,"updated":"2026-09-23 19:37:59.000000000","message":"The Testing section lists DELETE of a down service (verify cleanup) and DELETE of a live service (verify 409), but no test for the second specified precondition: DELETE returning 409 when bound ARQs exist. This is the safety check that prevents the conductor from destroying deployables with bound accelerator requests during decommissioning.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: Without a planned test, a regression in the bound-ARQ precheck could pass CI and allow cleanup of a service whose deployables still have bound ARQs, destroying accelerator state that workloads depend on. The missing test guards the exact data-integrity property the delete workflow advertises.\n\n**Recommendation**:\nAdd a functional-test bullet: \u0027DELETE of a down service with bound ARQs (verify 409 rejection and that deployables and the service row remain intact)\u0027.","commit_id":"a6981650e3dd12564dbdfb641c9e041a32987010"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"843cd3861761dbfcce08a9d194fd90485598b9d2","unresolved":false,"context_lines":[{"line_number":423,"context_line":"For ``cyborg-conductor`` and ``cyborg-api`` services, no associated resources"},{"line_number":424,"context_line":"need cleanup -- the service row is deleted directly."},{"line_number":425,"context_line":""},{"line_number":426,"context_line":"Errors during Placement cleanup are logged but do not prevent the service row"},{"line_number":427,"context_line":"from being deleted, matching Nova\u0027s approach. Operators can reconcile Placement"},{"line_number":428,"context_line":"manually if needed."},{"line_number":429,"context_line":""}],"source_content_type":"text/x-rst","patch_set":11,"id":"0823d663_3ebcd425","line":426,"updated":"2026-10-06 02:07:40.000000000","message":"The DELETE /v2/services/{uuid} design states two irreconcilable failure behaviors in the same section. Lines 411-416 say that if the conductor\u0027s cleanup fails (e.g. a Placement error), the service row is NOT deleted, the failure is logged, the service remains visible in GET /v2/services, and the operator can retry deletion. Lines 426-428 say the opposite: errors during Placement cleanup are logged but \u0027do not prevent the service row from being deleted\u0027. A spec must define one normative behavior; as written, two implementers can build opposite delete semantics for a destructive operation that destroys deployables and Placement resource providers.\n\n**Severity**: HIGH | **Confidence**: 0.95\n\n**Impact**: The normative design for a destructive admin operation is ambiguous. One reading leaves the service row intact so the operator can remediate and retry; the other deletes the row even when Placement cleanup failed, orphaning resource providers. Implementers cannot determine which conductor error-handling and API behavior to build, and the documented \u0027retry deletion\u0027 workflow only exists under one of the two readings.\n\n**Priority**: Before merge\n**Recommendation**:\nPick one semantic and reconcile the text. The row-not-deleted/retry semantics (lines 411-416) fits the numbered conductor steps and the asynchronous 204-via-cast flow; either remove the final paragraph or rewrite it to state that cleanup steps are idempotent so a failed deletion can be retried, and clarify the outcome when some steps already completed before the failure.","commit_id":"d3b489ad17cebfa5a0a9d8ff7c8241fb4344e301"}]}