HYPERFLEET-1177 - docs: v0.2.0 to v1.0.0 breaking-change checklist#154
Open
tirthct wants to merge 3 commits into
Open
HYPERFLEET-1177 - docs: v0.2.0 to v1.0.0 breaking-change checklist#154tirthct wants to merge 3 commits into
tirthct wants to merge 3 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR updates HyperFleet documentation to replace the "Ready" condition with the "Reconciled" condition across multiple architecture artifacts. The Sentinel design document's default reconciliation logic is rewritten to detect generation mismatches and staleness via Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
hyperfleet/components/sentinel/sentinel.md (2)
351-356:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winExample configs still reference
Readycondition — critical divergence.Lines 351-356, 547-650 show YAML config examples using
condition("Ready")andis_ready/ready_and_staleparams. Per breaking-changes.md item#7,condition("Ready")returns zero-value in v1.0.0, causing silent failure (Sentinel never publishes; clusters stuck). The default param table (lines 285-292) was updated toReconciled, but these example configs were not. Users copying these examples deploy broken configurations.Update all example configs to Reconciled
Lines 351-356:
message_decision: params: - ref_time: 'condition("Ready").last_updated_time' - is_ready: 'condition("Ready").status == "True"' - is_new_resource: '!is_ready && resource.generation == 1' - ready_and_stale: 'is_ready && now - timestamp(ref_time) > duration("30m")' - not_ready_and_debounced: '!is_ready && now - timestamp(ref_time) > duration("10s")' - result: 'is_new_resource || ready_and_stale || not_ready_and_debounced' + ref_time: 'condition("Reconciled").last_updated_time' + is_reconciled: 'condition("Reconciled").status == "True"' + is_new_resource: '!is_reconciled && resource.generation == 1' + generation_mismatch: 'resource.generation > condition("Reconciled").observed_generation' + reconciled_and_stale: 'is_reconciled && now - timestamp(ref_time) > duration("30m")' + not_reconciled_and_debounced: '!is_reconciled && now - timestamp(ref_time) > duration("10s")' + result: 'is_new_resource || generation_mismatch || reconciled_and_stale || not_reconciled_and_debounced'Apply equivalent updates to lines 554-559, 580-584, 606-610.
Also applies to: 547-650
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hyperfleet/components/sentinel/sentinel.md` around lines 351 - 356, Example configs still use condition("Ready") and derived params (is_ready, ready_and_stale, not_ready_and_debounced) which break in v1.0.0; update those references to use condition("Reconciled") and adjust any derived expressions accordingly (e.g., replace condition("Ready").last_updated_time with condition("Reconciled").last_updated_time and change is_ready to 'condition("Reconciled").status == "True"') in the shown parameter blocks and the other example blocks that define is_new_resource/ready_and_stale/not_ready_and_debounced so examples match the default param table and avoid silent failures.
473-473:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExplanatory text still references
Ready— contradicts updated default config.Line 473 states "default configuration relies on the
Readycondition" but the default param table (lines 285-292) was updated toReconciled. Lines 504-512 describe generation-change flow usingReadysemantics. Line 519 cross-references status-guide.md mentioning aggregation intoReadycondition. All contradict the v1.0.0 contract.Update explanatory references
Line 473:
-The Sentinel reads the resource's status conditions to evaluate the message decision rules. The default configuration relies on the `Ready` condition, but custom rules can reference any condition: +The Sentinel reads the resource's status conditions to evaluate the message decision rules. The default configuration relies on the `Reconciled` condition, but custom rules can reference any condition:Lines 504-512:
-When a user changes the cluster spec (e.g., scales nodes), `generation` increments (1 → 2). The API detects that not all adapters have reconciled this generation and sets `Ready` to `False`. The Sentinel's default rules see `Ready != True` and trigger reconciliation — no separate generation check is needed in the Sentinel. +When a user changes the cluster spec (e.g., scales nodes), `generation` increments (1 → 2). The API detects that not all adapters have reconciled this generation and sets `Reconciled` to `False`. The Sentinel's default rules see `Reconciled != True` and trigger reconciliation — no separate generation check is needed in the Sentinel.Line 519:
-- [HyperFleet Status Guide](../../docs/status-guide.md) - Complete documentation of the status contract, including how adapters report `observed_generation` and how the API aggregates it into the `Ready` condition +- [HyperFleet Status Guide](../../docs/status-guide.md) - Complete documentation of the status contract, including how adapters report `observed_generation` and how the API aggregates it into the `Reconciled` conditionAlso applies to: 504-512, 519-519
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hyperfleet/components/sentinel/sentinel.md` at line 473, The documentation still refers to the `Ready` condition even though the default parameter table and v1.0.0 contract use `Reconciled`; update all explanatory text to use `Reconciled` (replace “Ready” mentions in the Sentinel overview, the generation-change flow description, and the cross-reference to status-guide.md) and adjust any wording that implies Ready semantics to describe Reconciled semantics instead (search for occurrences of "Ready" in the Sentinel.md sections around the Sentinel overview, generation-change flow, and the cross-reference and change them to "Reconciled", ensuring the text matches the default param table and the v1.0.0 contract).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@hyperfleet/components/sentinel/sentinel.md`:
- Around line 351-356: Example configs still use condition("Ready") and derived
params (is_ready, ready_and_stale, not_ready_and_debounced) which break in
v1.0.0; update those references to use condition("Reconciled") and adjust any
derived expressions accordingly (e.g., replace
condition("Ready").last_updated_time with
condition("Reconciled").last_updated_time and change is_ready to
'condition("Reconciled").status == "True"') in the shown parameter blocks and
the other example blocks that define
is_new_resource/ready_and_stale/not_ready_and_debounced so examples match the
default param table and avoid silent failures.
- Line 473: The documentation still refers to the `Ready` condition even though
the default parameter table and v1.0.0 contract use `Reconciled`; update all
explanatory text to use `Reconciled` (replace “Ready” mentions in the Sentinel
overview, the generation-change flow description, and the cross-reference to
status-guide.md) and adjust any wording that implies Ready semantics to describe
Reconciled semantics instead (search for occurrences of "Ready" in the
Sentinel.md sections around the Sentinel overview, generation-change flow, and
the cross-reference and change them to "Reconciled", ensuring the text matches
the default param table and the v1.0.0 contract).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 561ab9f1-7a7f-4851-bf54-29d52ae7a250
📒 Files selected for processing (4)
hyperfleet/README.mdhyperfleet/components/sentinel/sentinel.mdhyperfleet/docs/glossary.mdhyperfleet/docs/release/v1.0.0/breaking-changes.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hyperfleet/components/sentinel/sentinel.md (1)
285-292:⚠️ Potential issue | 🟠 MajorFix Sentinel design doc contradiction:
generation_mismatchis an explicit immediate trigger
Key Insight — Why No Hardcoded Generation CheckclaimsReconciled == Falsealready covers the generation-mismatch case (lines 261-263), but the default decision definition explicitly includesgeneration_mismatchand ORs it into the result (lines 288, 292), which makes generation mismatch able to trigger publishing immediately (independent of the debounce path).Test 6is inconsistent with both the design’s defaultresultand Sentinel’s default behavior: it evaluatesnot_reconciled_and_debounced=truebut omitsgeneration_mismatch, and theResultline shown for Test 6 lacks thegeneration_mismatchterm (line 955). Update Test 6 to reflectgeneration_mismatch=truefor a “generation mismatch” scenario, and replace the note that generation mismatch is “handled implicitly” only vianot_reconciled_and_debounced(around lines 944-960).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hyperfleet/components/sentinel/sentinel.md` around lines 285 - 292, The doc contradicts itself: the decision Result explicitly includes generation_mismatch as an immediate trigger but the text and Test 6 claim generation mismatch is handled implicitly via not_reconciled_and_debounced; update the design doc to be consistent by making Test 6 represent a true generation mismatch (set generation_mismatch = true) and adjust the Test 6 Result line to include generation_mismatch in the OR list, and replace the surrounding note that says generation mismatch is only handled implicitly with text that acknowledges generation_mismatch is an explicit immediate trigger in the default Result.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@hyperfleet/components/sentinel/sentinel.md`:
- Around line 285-292: The doc contradicts itself: the decision Result
explicitly includes generation_mismatch as an immediate trigger but the text and
Test 6 claim generation mismatch is handled implicitly via
not_reconciled_and_debounced; update the design doc to be consistent by making
Test 6 represent a true generation mismatch (set generation_mismatch = true) and
adjust the Test 6 Result line to include generation_mismatch in the OR list, and
replace the surrounding note that says generation mismatch is only handled
implicitly with text that acknowledges generation_mismatch is an explicit
immediate trigger in the default Result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: ba685685-b424-408e-b5de-7c9cecc58e61
📒 Files selected for processing (1)
hyperfleet/components/sentinel/sentinel.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
kuudori
reviewed
Jun 11, 2026
| | 1 | **Status report: POST to PUT** for clusters and nodepools | Change HTTP method in all adapter post-actions from POST to PUT | **405 Method Not Allowed.** PUT-only routes confirmed in `plugins/clusters/plugin.go`; no POST route exists | Automatable (1178) | [HYPERFLEET-978](https://redhat.atlassian.net/browse/HYPERFLEET-978) | - | | ||
| | 2 | **Ready condition removed; replaced by Reconciled** | Replace all `Ready` references with `Reconciled` in status queries, scripts, and monitoring | **Silent hang.** API silently accepts `type="Ready"` in status reports but never aggregates it into resource conditions; scripts polling for Ready=True wait forever | Automatable (1178) | [HYPERFLEET-1052](https://redhat.atlassian.net/browse/HYPERFLEET-1052) | [HYPERFLEET-559](https://redhat.atlassian.net/browse/HYPERFLEET-559) | | ||
| | 3 | **Aggregated condition Available renamed to LastKnownReconciled** | Replace `Available` with `LastKnownReconciled` in all resource-level condition queries | **Not found.** Aggregation code produces only `Reconciled` and `LastKnownReconciled`; resource-level `Available` is no longer emitted | Automatable (1178) | [HYPERFLEET-1017](https://redhat.atlassian.net/browse/HYPERFLEET-1017) | - | | ||
| | 4 | **Status value validation** | Ensure all adapter status reports use True, False, or Unknown only | **400 Bad Request.** `isValidAdapterConditionStatus()` rejects empty/invalid values; `PresentAdapterStatus()` also filters invalid conditions from GET responses. v0.2.0 accepted empty status values | Automatable (1178) | [HYPERFLEET-841](https://redhat.atlassian.net/browse/HYPERFLEET-841) | - | |
Contributor
There was a problem hiding this comment.
I see that isValidAdapterConditionStatus was added in 647, not 841.
But maybe it was added in v0.2.0, so it's not a v1.0.0 BC
Contributor
Author
There was a problem hiding this comment.
Ack, you are right. Fixed this
kuudori
reviewed
Jun 11, 2026
| | 8 | **Sentinel Helm: config.hyperfleetApi moved to config.clients.hyperfleetApi** | Move API client config from `config.hyperfleetApi.baseUrl` to `config.clients.hyperfleetApi.baseUrl` in Sentinel Helm values | **Sentinel fails to start.** Helm template references `.Values.config.clients.hyperfleetApi.baseUrl`; old path renders empty; validation fails with "clients.hyperfleet_api.base_url required" | Automatable (1178) | [HYPERFLEET-549](https://redhat.atlassian.net/browse/HYPERFLEET-549), [HYPERFLEET-866](https://redhat.atlassian.net/browse/HYPERFLEET-866) | - | | ||
| | 9 | **Sentinel Helm: messageDecision params changed from map to list** | Restructure `messageDecision.params` from map format (`key: 'expr'`) to list format (`- name: key, expr: 'expr'`) | **Helm template fails to render.** Template iterates with `.name` and `.expr` fields; old map format has no such fields. Also fixes non-deterministic param ordering | Automatable (1178) | [HYPERFLEET-1011](https://redhat.atlassian.net/browse/HYPERFLEET-1011) | - | | ||
| | 10 | **API Helm: jwt.enabled default changed from true to false** | Explicitly set `config.server.jwt.enabled: true` in Helm values if JWT authentication is required | **API accepts unauthenticated requests.** v0.2.0 Helm chart defaulted to `jwt.enabled: true`; v1.0.0 defaults to `false`. Requests that were previously rejected without a valid JWT token are now accepted | Doc-only (1163/1179) | Helm chart values.yaml diff | - | | ||
| | 11 | **JWT identity_claim required when JWT enabled** | Add `server.jwt.identity_claim` pointing to JWT claim for caller identity | **API fails to start** if JWT enabled without identity_claim. Config validation: `if c.IdentityClaim == "" { return fmt.Errorf("server.jwt.identity_claim is required") }`. If claim name is set but does not exist in token, audit fields are silently empty | Doc-only (1163/1179) | [HYPERFLEET-1134](https://redhat.atlassian.net/browse/HYPERFLEET-1134) | [HYPERFLEET-824](https://redhat.atlassian.net/browse/HYPERFLEET-824) | |
Contributor
There was a problem hiding this comment.
audit fields are silently empty
This isn't really true. Write methods are guarded.
See pkg/auth/auth_middleware.go > ResolveCallerIdentity > isMutatingMethod
…pterConditionStatus statement
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
rafabene
reviewed
Jun 12, 2026
| | `not_reconciled_and_debounced` | CEL → bool | `!is_reconciled && now - timestamp(ref_time) > duration("10s")` | Not-reconciled resource, debounce period elapsed | | ||
|
|
||
| **Result**: `is_new_resource || ready_and_stale || not_ready_and_debounced` | ||
| **Result**: `is_new_resource || generation_mismatch || reconciled_and_stale || not_reconciled_and_debounced` |
Contributor
There was a problem hiding this comment.
The generation_mismatch param appears in the parameter table (line 288) and Result expression (line 292), but is missing from all four YAML config examples (lines 356, 559, 585, 611) and all test cases (lines 882–955).
Test 6's note also explicitly says generation mismatch is "handled implicitly" via Reconciled=False, which contradicts having an explicit generation_mismatch param.
Either add generation_mismatch to all YAML examples and update the 7 test cases to use the 4-term result, or remove it from the table and revert the Result to the 3-term form matching the examples.
rafabene
reviewed
Jun 12, 2026
| ```cel | ||
| condition("Ready").status == "True" # check if resource is ready | ||
| condition("Ready").last_updated_time # get timestamp for age calculation | ||
| condition("Reconciled").status == "True" # check if resource is ready |
Contributor
There was a problem hiding this comment.
Stale comment — says "ready" but the condition is now Reconciled:
Suggested change
| condition("Reconciled").status == "True" # check if resource is ready | |
| condition("Reconciled").status == "True" # check if resource is reconciled |
rafabene
reviewed
Jun 12, 2026
| | **Fan-out** | The messaging pattern used in HyperFleet where a single reconciliation event published to one topic is independently delivered to multiple adapter subscriptions simultaneously. Each adapter receives its own copy of the event via a dedicated subscription. | Broker | | ||
| | **GCP Pub/Sub** | Google Cloud Pub/Sub — the primary Message Broker implementation used in GCP-hosted HyperFleet deployments. Configured with `broker.type: googlepubsub` in `broker.yaml`. | Broker | | ||
| | **Generation** | An integer field on HyperFleet API resources that increments each time the resource's spec changes. Adapters include `observed_generation` in their status reports to indicate which version of the spec they reconciled. Sentinel uses generation changes to detect new desired state requiring reconciliation. | API, Sentinel, Adapter Framework | | ||
| | **Finalized (Condition)** | An adapter condition introduced in v1.0.0 for the deletion lifecycle. Adapters report `Finalized: True` after completing cleanup of their managed resources during deletion. The API's hard-delete mechanism checks that all required adapters have reported `Finalized: True` before permanently removing a resource from the database. | Adapter Framework, API | |
Contributor
There was a problem hiding this comment.
Glossary is alphabetically ordered, but Finalized (F) was inserted after Generation (G). It should be between Fan-out and GCP Pub/Sub.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verified checklist of every breaking change between v0.2.0 and v1.0.0, so partner teams can reconfigure and redeploy with no surprises.
Also fixes stale Ready condition references across the repo and adds missing glossary terms.
Changes
hyperfleet/docs/release/v1.0.0/breaking-changes.md— the checklist (18 items)hyperfleet/docs/e2e-testing/README.md— index for e2e testing docshyperfleet/docs/release/README.md— link to v1.0.0 folderhyperfleet/docs/glossary.md— added Reconciled, LastKnownReconciled, Finalized; marked Ready as removedhyperfleet/components/sentinel/sentinel.md— Ready→Reconciled in CEL exampleshyperfleet/README.md— Ready phase→Reconciled statehyperfleet/docs/status-guide.md— added v1.0.0 deprecation notice (full rewrite tracked under HYPERFLEET-1186)hyperfleet/docs/e2e-testing/tier1-nightly-failure-investigation-2026-04-20.md— 5 broken/missing ToC entriesItems evaluated and confirmed not breaking
These were investigated but removed from the checklist because the behavior did not exist in v0.2.0 (new functionality, not a change to existing behavior):
Test Plan
markdownlint-cli2passes on all 7 changed files (0 errors)