Fix #5191: Use metav1.Condition for CR statuses - #6796
Draft
Ganny-lp wants to merge 1 commit into
Draft
Conversation
Replace Camel K's own condition structs (IntegrationCondition, PipeCondition, BuildCondition, IntegrationKitCondition, IntegrationPlatformCondition, KameletCondition, CamelCatalogCondition) with the standard metav1.Condition type across all affected CRDs, backed by the k8s.io/apimachinery/pkg/api/meta helpers (FindStatusCondition, SetStatusCondition, RemoveStatusCondition). - FirstTruthyTime (per-condition) migrated to FirstReadyTimestamp (top-level status field), set only on the first transition to Ready=True. - DeprecatedPods promoted from a per-condition nested field to a top-level status field. - CRDs updated with x-kubernetes-list-type: map / list-map-keys: [type] on the conditions list, per standard Kubernetes conventions. - A conditionAdapter preserves the legacy ResourceCondition interface for existing call sites. - Regenerated: zz_generated.deepcopy.go, pkg/client/camel/applyconfiguration/**, all CRD YAMLs (pkg/resources/config/crd/bases and the Helm CRD bundle), and docs/modules/ROOT/partials/apis/camel-k-crds.adoc via script/gen_doc.sh. - Added unit tests for the migration logic and updated pre-existing assertions across trait/controller tests that compared .Status/.Type against the old typed condition constants. - Fixed isIntegrationUpdated(), which required FirstReadyTimestamp to already be set to detect a transition to Ready=True -- backwards, since that field is exactly what gets set as a result of the transition. The nil/zero guard was moved to the call site in integrationUpdateFunc, where FirstReadyTimestamp.Sub(startTime) is actually dereferenced. IntegrationProfile is intentionally left untouched: its condition field was already marked DeprecatedIntegrationProfileCondition prior to this change. Fixes apache#5191 Co-authored-by: Ganny-lp <geovanny.piedade@usp.br>
Ganny-lp
marked this pull request as draft
September 6, 2026 00:21
Ganny-lp
marked this pull request as ready for review
September 6, 2026 00:43
squakez
reviewed
Sep 7, 2026
squakez
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the work. However, the changes proposed are introducing backward incompatibilities as it massively changes the API CRD. We could try to rework this by adapting the API to the new metav1 Condition logic, or just postpone it when we can tackle API changes (likely in major version 3.x release).
squakez
marked this pull request as draft
September 7, 2026 12:20
Contributor
|
✔️ Unit test coverage report - coverage increased from 63.2% to 63.4% (+0.2%) |
Contributor
|
Although in draft it would be interesting to have a round of test. Please rebase, there was some github action error fixed in main. |
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
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.
[WORK PARTIALLY DONE USING AI] Use metav1.Condition for CR statuses
Fixes #5191
Camel K currently maintains its own condition structs per CR (IntegrationCondition, PipeCondition, BuildCondition, IntegrationKitCondition, IntegrationPlatformCondition, KameletCondition, CamelCatalogCondition), duplicating logic that the Kubernetes ecosystem already standardizes through metav1.Condition. This divergence makes the CRDs less idiomatic, complicates interoperability with generic tooling that expects the standard condition shape, and forces us to maintain bespoke transition/lookup logic instead of relying on the well-tested k8s.io/apimachinery/pkg/api/meta helpers.
Replace all per-CR condition structs with the standard metav1.Condition type, backed by FindStatusCondition, SetStatusCondition, and RemoveStatusCondition from k8s.io/apimachinery/pkg/api/meta.
Migrate FirstTruthyTime (previously per-condition) to FirstReadyTimestamp, a top-level status field set only on the first transition to Ready=True.
Promote DeprecatedPods from a per-condition nested field to a top-level status field.
Update CRDs with x-kubernetes-list-type: map / list-map-keys: [type] on the conditions list, following standard Kubernetes conventions.
Introduce a conditionAdapter to preserve the legacy ResourceCondition interface for existing call sites, minimizing churn outside the condition-handling code itself.
Regenerate zz_generated.deepcopy.go, pkg/client/camel/applyconfiguration/**, all CRD YAMLs (pkg/resources/config/crd/bases and the Helm CRD bundle), and docs/modules/ROOT/partials/apis/camel-k-crds.adoc via script/gen_doc.sh.
Add unit tests covering the migration logic, and update pre-existing assertions across trait/controller tests that compared .Status/.Type against the old typed condition constants.
Fix isIntegrationUpdated(), which incorrectly required FirstReadyTimestamp to already be set in order to detect a transition to Ready=True — backwards, since that field is exactly what gets set as a result of the transition. The nil/zero guard was moved to the call site in integrationUpdateFunc, where FirstReadyTimestamp.Sub(startTime) is actually dereferenced.
IntegrationProfile is intentionally left untouched, since its condition field was already marked DeprecatedIntegrationProfileCondition prior to this change.
Replace Camel K's custom per-CR condition structs with the standard
metav1.Conditiontype