HYPERFLEET-1017 - feat: rename aggregated condition Available to LastKnownReconciled#38
Conversation
|
[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 |
|
Caution Review failedFailed to post review comments WalkthroughThis PR replaces the "Available" status condition type with "LastKnownReconciled" across models, examples, documentation, and OpenAPI/Swagger schemas. It adds Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
schemas/core/openapi.yaml (1)
1260-1267:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlign the schema with the new “three mandatory conditions” contract.
This block now documents 3 mandatory conditions, but
minItemsis still2here (and the same mismatch remains inNodePoolStatusat Line 1626). That leaves the published contract looser than the docs and lets clients/codegen accept incomplete statuses.Suggested fix
conditions: type: array items: $ref: '#/components/schemas/ResourceCondition' - minItems: 2 + minItems: 3Apply the same change to
NodePoolStatusin the source model and regenerate the schema variants.🤖 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 `@schemas/core/openapi.yaml` around lines 1260 - 1267, Update the schema to reflect the new three-mandatory-conditions contract by changing minItems from 2 to 3 for the conditions arrays: specifically update ClusterStatus.conditions (the block currently showing minItems: 2 and the three documented conditions Ready/Reconciled/LastKnownReconciled) and likewise update NodePoolStatus.conditions to minItems: 3 in the source model, then regenerate the OpenAPI/schema variants so the published schema enforces the three required conditions.
🧹 Nitpick comments (1)
models-gcp/nodepool/example_nodepool.tsp (1)
61-70: 💤 Low valueConsider using shared constants for consistency.
The nodepool examples use hardcoded strings (
"ResourceLastKnownReconciled") for the reason, while cluster examples use the sharedExampleLastKnownReconciledReasonconstant. This inconsistency could cause maintenance overhead if the reason text needs to change.🤖 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-gcp/nodepool/example_nodepool.tsp` around lines 61 - 70, Replace the hardcoded reason string "ResourceLastKnownReconciled" used in the nodepool example Condition object with the shared constant ExampleLastKnownReconciledReason so examples stay consistent; locate the Condition entry where ConditionType.LastKnownReconciled and ResourceConditionStatus.True are set and change the reason field to ExampleLastKnownReconciledReason, adding/importing that constant if needed so the example compiles and matches cluster examples.
🤖 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 `@schemas/core/openapi.yaml`:
- Around line 1260-1267: Update the schema to reflect the new
three-mandatory-conditions contract by changing minItems from 2 to 3 for the
conditions arrays: specifically update ClusterStatus.conditions (the block
currently showing minItems: 2 and the three documented conditions
Ready/Reconciled/LastKnownReconciled) and likewise update
NodePoolStatus.conditions to minItems: 3 in the source model, then regenerate
the OpenAPI/schema variants so the published schema enforces the three required
conditions.
---
Nitpick comments:
In `@models-gcp/nodepool/example_nodepool.tsp`:
- Around line 61-70: Replace the hardcoded reason string
"ResourceLastKnownReconciled" used in the nodepool example Condition object with
the shared constant ExampleLastKnownReconciledReason so examples stay
consistent; locate the Condition entry where ConditionType.LastKnownReconciled
and ResourceConditionStatus.True are set and change the reason field to
ExampleLastKnownReconciledReason, adding/importing that constant if needed so
the example compiles and matches cluster examples.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 7e279f64-f604-4e19-9537-50c4d0da2063
📒 Files selected for processing (12)
models-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
rafabene
force-pushed
the
HYPERFLEET-1017-rename-available-to-last-known-reconciled
branch
2 times, most recently
from
b4991c6 to
8b2c696
Compare
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)
schemas/gcp/swagger.yaml (1)
1404-1412:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate
minItemsto match the 3 mandatory conditionsLine 1404/Line 1918 document three mandatory conditions, but Line 1411 and Line 1925 still permit only two entries. This leaves the schema less strict than its own contract.
Proposed fix
- minItems: 2 + minItems: 3 ... - minItems: 2 + minItems: 3Also applies to: 1918-1926
🤖 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 `@schemas/gcp/swagger.yaml` around lines 1404 - 1412, The schema currently defines the resource conditions array (items: $ref: '#/definitions/ResourceCondition') with minItems: 2 but the spec/doc text requires three mandatory conditions; update every occurrence of minItems: 2 to minItems: 3 for the conditions array so the OpenAPI schema enforces the three required conditions (ensure you change both places mentioned around the ResourceCondition definition in swagger.yaml).schemas/core/openapi.yaml (1)
1266-1274:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlign schema cardinality with documented mandatory conditions
Line 1270 and Line 1636 describe 3 mandatory conditions, but Line 1266 and Line 1632 still allow only 2. This weakens the API contract and validation for status payloads.
Proposed fix
- minItems: 2 + minItems: 3 ... - minItems: 2 + minItems: 3Also applies to: 1632-1640
🤖 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 `@schemas/core/openapi.yaml` around lines 1266 - 1274, Update the OpenAPI schema for the cluster status "conditions" array to require three items instead of two: change the minItems value from 2 to 3 for the schema that contains the "List of status conditions for the cluster" description (the "conditions" array where the description lists the three mandatory conditions: Ready, Reconciled, LastKnownReconciled), and make the same change for the second occurrence of that schema later in the file so both locations enforce minItems: 3.
🤖 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 `@schemas/core/openapi.yaml`:
- Around line 1266-1274: Update the OpenAPI schema for the cluster status
"conditions" array to require three items instead of two: change the minItems
value from 2 to 3 for the schema that contains the "List of status conditions
for the cluster" description (the "conditions" array where the description lists
the three mandatory conditions: Ready, Reconciled, LastKnownReconciled), and
make the same change for the second occurrence of that schema later in the file
so both locations enforce minItems: 3.
In `@schemas/gcp/swagger.yaml`:
- Around line 1404-1412: The schema currently defines the resource conditions
array (items: $ref: '#/definitions/ResourceCondition') with minItems: 2 but the
spec/doc text requires three mandatory conditions; update every occurrence of
minItems: 2 to minItems: 3 for the conditions array so the OpenAPI schema
enforces the three required conditions (ensure you change both places mentioned
around the ResourceCondition definition in swagger.yaml).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 55123c3a-35b2-4886-9821-d834ac2dfdce
📒 Files selected for processing (12)
models-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 skipped from review due to trivial changes (1)
- services/clusters.tsp
🚧 Files skipped from review as they are similar to previous changes (1)
- models-core/cluster/example_cluster.tsp
rafabene
force-pushed
the
HYPERFLEET-1017-rename-available-to-last-known-reconciled
branch
from
8b2c696 to
a23333d
Compare
| Available: "Available", | ||
| Ready: "Ready", | ||
| Reconciled: "Reconciled" | ||
| Reconciled: "Reconciled", | ||
| LastKnownReconciled: "LastKnownReconciled" |
There was a problem hiding this comment.
Should we remove Available from here?
ConditionType.Available appears in example_adapter_status.tsp but it may be better defined as a new AdapterConditionType and define also Applied, Health, Finalized for reuse
|
One required change is to update the version at |
2 participants
Summary
LastKnownReconciledto theConditionTypeunion in the shared model (keepsAvailablefor adapter-level use)AvailabletoLastKnownReconciledin all cluster and nodepool status examples (core + GCP)The adapter-level
Availablecondition (reported by adapters) is not renamed — only the API-computed aggregated condition changes.Test plan
./build-schema.sh gcpcompiles successfully./build-schema.sh gcp --swaggercompiles successfully./build-schema.sh corecompiles successfully./build-schema.sh core --swaggercompiles successfullyLastKnownReconciledfor aggregated conditionsAvailablefor adapter-level conditionsmodels/statuses/example_adapter_status.tspis unchangedRelates to: https://redhat.atlassian.net/browse/HYPERFLEET-1017
Summary by CodeRabbit
New Features
Documentation