HYPERFLEET-1052 - chore: remove deprecated Ready condition type#45
Conversation
📝 WalkthroughWalkthroughThis PR removes the deprecated Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@CHANGELOG.md`:
- Around line 14-20: Release notes claim `Ready` condition is
deprecated/migrated to `Reconciled`, but other repos still reference `Ready` and
related constants causing drift; update linked repos or roll back the notes.
Search for and replace or reconcile usages of `Ready`, `ExampleReadyReason`, and
`ExampleReadyMessage` in dependent repos (examples, docs, tests, comments) to
use `Reconciled`/`LastKnownReconciled` and update any `ConditionType` references
and `postCluster` documentation to list only `LastKnownReconciled` and
`Reconciled`; if immediate updates across repos aren’t possible, revert the
CHANGELOG wording for 1.0.14 to avoid declaring the migration complete until all
linked repositories (e.g., hyperfleet-api and architecture docs) are
synchronized.
In `@models/common/model.tsp`:
- Line 148: The spec was changed to use status.conditions.Reconciled instead of
status.conditions.Ready, but external docs/tests still reference Ready; update
all cross-repo references and tests to match the new symbol
(status.conditions.Reconciled) to avoid contract drift: search for occurrences
of "status.conditions.Ready" and "Ready" in query examples, API docs, and test
fixtures in the linked repositories (e.g., architecture/docs and hyperfleet-api
tests), replace them with "status.conditions.Reconciled" and adjust any sample
queries (e.g., selectors like status.conditions.Reconciled='True'), then
run/adjust unit/integration tests and contract validation to ensure they conform
to the HyperFleet architecture standards.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 75d1439b-f52f-4fc7-9cc5-d92d8438b8c7
📒 Files selected for processing (14)
CHANGELOG.mdmain.tspmodels-core/cluster/example_cluster.tspmodels-core/nodepool/example_nodepool.tspmodels-gcp/cluster/example_cluster.tspmodels-gcp/nodepool/example_nodepool.tspmodels/clusters/model.tspmodels/common/model.tspmodels/nodepools/model.tspschemas/core/openapi.yamlschemas/core/swagger.yamlschemas/gcp/openapi.yamlschemas/gcp/swagger.yamlservices/clusters.tsp
💤 Files with no reviewable changes (4)
- models-gcp/cluster/example_cluster.tsp
- models-core/cluster/example_cluster.tsp
- models-core/nodepool/example_nodepool.tsp
- models-gcp/nodepool/example_nodepool.tsp
| - Deprecated `Ready` condition type from `ConditionType`, status model documentation, and all examples (HYPERFLEET-1052) | ||
| - `ExampleReadyReason` and `ExampleReadyMessage` constants | ||
|
|
||
| ### Changed | ||
|
|
||
| - Search example updated from `status.conditions.Ready` to `status.conditions.Reconciled` | ||
| - `postCluster` documentation updated to list only `LastKnownReconciled` and `Reconciled` as mandatory conditions |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify remaining references in linked repos
gh search code "status.conditions.Ready repo:openshift-hyperfleet/hyperfleet-api" --limit 100
gh search code "status.conditions.Ready repo:openshift-hyperfleet/architecture" --limit 50Repository: openshift-hyperfleet/hyperfleet-api-spec
Length of output: 2562
🏁 Script executed:
#!/bin/bash
# Inspect CHANGELOG.md lines around 14-20
head -30 CHANGELOG.md | cat -nRepository: openshift-hyperfleet/hyperfleet-api-spec
Length of output: 1399
Coordinate linked-repo updates before publishing 1.0.14 release notes.
The release notes declare the Ready condition fully deprecated and migrated to Reconciled, but linked repos still contain active references:
- hyperfleet-api: docs/search.md (examples), pkg/db/sql_helpers.go (comments), README.md (curl example), test files (test cases)
- architecture: adrs/0010-jsonb-provider-agnostic-spec.md, docs/status-guide.md (documentation)
These outstanding references create documentation drift and will confuse consumers. Update or defer the 1.0.14 release notes until dependent repos are synchronized.
🤖 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 `@CHANGELOG.md` around lines 14 - 20, Release notes claim `Ready` condition is
deprecated/migrated to `Reconciled`, but other repos still reference `Ready` and
related constants causing drift; update linked repos or roll back the notes.
Search for and replace or reconcile usages of `Ready`, `ExampleReadyReason`, and
`ExampleReadyMessage` in dependent repos (examples, docs, tests, comments) to
use `Reconciled`/`LastKnownReconciled` and update any `ConditionType` references
and `postCluster` documentation to list only `LastKnownReconciled` and
`Reconciled`; if immediate updates across repos aren’t possible, revert the
CHANGELOG wording for 1.0.14 to avoid declaring the migration complete until all
linked repositories (e.g., hyperfleet-api and architecture docs) are
synchronized.
There was a problem hiding this comment.
Changes to these repos would be followed up in the current ticket. API Spec works as a prerequisite for Hyperfleet API, hence making changes here first.
There was a problem hiding this comment.
@tirthct, understood — the spec-first, consumers-second sequencing makes sense for this migration. Since HYPERFLEET-1052 will track the follow-up updates to hyperfleet-api and architecture, the drift is intentional and time-bounded.
For completeness, the follow-up work in those repos includes:
- hyperfleet-api:
docs/search.md,pkg/db/sql_helpers.go(comments),README.mdcurl example, integration test cases intest/integration/search_field_mapping_test.go - architecture:
docs/status-guide.md,adrs/0010-jsonb-provider-agnostic-spec.md
Would you like me to open a GitHub issue to track these follow-up items, or are they already captured within the HYPERFLEET-1052 ticket?
📝 Learnings were identified and not saved (knowledge base disabled). Enable
| model SearchParams { | ||
| /** Filter results using TSL (Tree Search Language) query syntax. | ||
| * Examples: `status.conditions.Ready='True'`, `name in ('c1','c2')`, `labels.region='us-east'` */ | ||
| * Examples: `status.conditions.Reconciled='True'`, `name in ('c1','c2')`, `labels.region='us-east'` */ |
There was a problem hiding this comment.
Cross-repo contract docs/tests still reference Ready and now diverge from this spec
This change is correct for the spec, but linked findings show openshift-hyperfleet/architecture and openshift-hyperfleet/hyperfleet-api still documenting/querying status.conditions.Ready. Please land coordinated follow-up updates before release to avoid contract confusion in search/docs/tests.
As per coding guidelines, "Validate changes against HyperFleet architecture standards from the linked architecture repository."
🤖 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 `@models/common/model.tsp` at line 148, The spec was changed to use
status.conditions.Reconciled instead of status.conditions.Ready, but external
docs/tests still reference Ready; update all cross-repo references and tests to
match the new symbol (status.conditions.Reconciled) to avoid contract drift:
search for occurrences of "status.conditions.Ready" and "Ready" in query
examples, API docs, and test fixtures in the linked repositories (e.g.,
architecture/docs and hyperfleet-api tests), replace them with
"status.conditions.Reconciled" and adjust any sample queries (e.g., selectors
like status.conditions.Reconciled='True'), then run/adjust unit/integration
tests and contract validation to ensure they conform to the HyperFleet
architecture standards.
There was a problem hiding this comment.
This will be followed up by a PR in hyperfleet-api and architecture repo
There was a problem hiding this comment.
@tirthct, understood — thanks for confirming. To make sure nothing falls through the cracks, would you like me to open tracking issues in hyperfleet-api and/or architecture repos to capture the remaining status.conditions.Ready → Reconciled references that need updating? Based on the linked findings, the key areas are:
openshift-hyperfleet/hyperfleet-api: integration tests (test/integration/search_field_mapping_test.go), DB helper docs/tests (pkg/db/sql_helpers.go,pkg/db/sql_helpers_test.go),README.md, anddocs/search.md.openshift-hyperfleet/architecture:docs/status-guide.mdandadrs/0010-jsonb-provider-agnostic-spec.md.
Just let me know and I'll create them.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ma-hill The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
Bot
merged commit 2b7675f
into
openshift-hyperfleet:main
2 participants
Summary
Readycondition type fromConditionTypeunion, status model docs, and all examplesExampleReadyReasonandExampleReadyMessageconstantspostClusterdoc to referenceReconciledinstead ofReadyContext
This is step 3 of the phased Ready → Reconciled migration:
All consumers (Sentinel, adapters, E2E, infra) have been on Reconciled since HYPERFLEET-1016 closed. The two mandatory conditions are now Reconciled and LastKnownReconciled —
@minItems(2)isunchanged.
Test plan
npm run build:all— both core and GCP schemas compile successfullygrep -rn "Ready" --include="*.tsp" .— zero matches in sourcegrep "Ready" schemas/core/openapi.yaml schemas/gcp/openapi.yaml— zero matches in generated schemas@minItems(2)unchanged in both ClusterStatus and NodePoolStatus