Skip to content

Fix #5191: Use metav1.Condition for CR statuses - #6796

Draft
Ganny-lp wants to merge 1 commit into
apache:mainfrom
Ganny-lp:feature/5191-metav1-conditions
Draft

Fix #5191: Use metav1.Condition for CR statuses#6796
Ganny-lp wants to merge 1 commit into
apache:mainfrom
Ganny-lp:feature/5191-metav1-conditions

Conversation

@Ganny-lp

@Ganny-lp Ganny-lp commented Sep 6, 2026

Copy link
Copy Markdown

[WORK PARTIALLY DONE USING AI] Use metav1.Condition for CR statuses

Fixes #5191


  1. Motivation

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.

  1. Modifications

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.

  1. Release Note

Replace Camel K's custom per-CR condition structs with the standard metav1.Condition type

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
Ganny-lp marked this pull request as draft September 6, 2026 00:21
@Ganny-lp
Ganny-lp marked this pull request as ready for review September 6, 2026 00:43

@squakez squakez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
squakez marked this pull request as draft September 7, 2026 12:20
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

✔️ Unit test coverage report - coverage increased from 63.2% to 63.4% (+0.2%)

@squakez

squakez commented Sep 7, 2026

Copy link
Copy Markdown
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider using metav1.Condition instead of own condition structs

2 participants