api icon indicating copy to clipboard operation
api copied to clipboard

DO NOT MERGE: ClusterExtensionRevision API Review

Open perdasilva opened this issue 3 months ago • 6 comments

OLMv1 ClusterExtensionRevision API Review

:warning: DO NOT MERGE This PR is only to facilitate API review.

ClusterExtensionRevision API

ClusterExtensionRevision objects are created by the ClusterExtension controller for each install, upgrade, or reconfiguration operation. They are owned by a ClusterExtension. A ClusterExtensionRevision carries the objects of the revision (i.e. the resources of a package at a given version + any user supplied configuration). We don't expect users to create any clusterextensionrevision objects. We only expect users to interact with this API when something goes wrong, or, in the future, in approval flows (i.e. a revision is kept from rolling out until there's some manual intervention by an user).

Lifecycle

  • Active: actively reconciling
  • Archived: inert (only for historical / auditing purposes)

Revision Rollout Strategy

  1. The objects are grouped into phases.
  2. The objects of each phase are applied together and in no particular order.
  3. Rollout will only progress to the next phase once all object of the current phase pass their probes
  4. Probes are attached to kinds and currently come in two flavours:
    1. status condition probes: e.g. Available=True
    2. field equality: e.g. .status.replicas == .status.updatedReplicas
  5. The rollout is complete once all phases are complete

Object ownership is transferred from previous to subsequent revisions.

Once a revision completes, it sets all previous revisions to Archived. When Archived the revision will clean up any objects it still manages.

Collision Control

Phase objects can be be configured to error on existing, adopt orphan resources, or force adopt resources.

Bundle Rollout Strategy Definition

registry+v1 bundles will have their rollout strategy defined by OLM in both phases and probes. If we directly support the Helm format, OLM will likey also define the rollout strategy.

Currently the registry+v1 bundle strategy defined by sorting the bundle manifests into different phases: namespaces, policies, rbac, crds, etc. with default probes, e.g. CRD has Established=True condition, Deployment has Available=True, etc.

Known Upcoming Changes

  • Removal of the Paused lifecycle state from docs and code
  • Removal of the "Migrated" reason on the "Available" condition
  • Moving collisionProtection out of the object and up to the top level

Known Limitations

The objects are currently defined inline in the CRD. In the (not so distant) future we will shard the resources across some kind of container (e.g. ConfigMap or Secret).

Future Work (beyond GA)

  • Revision approval
  • Rollback strategies
  • Paused lifecycle
  • Bundle author provided rollout definition

Open Questions

  • Can we release this API TechPreview without resource sharding and calling out the known limitation. This would require subsequent (possibly breaking) API changes.
  • Should the rollout definition be exposed through the API or just be a clusterextensionrevision controller concern that is opaque to users. Or rather, should we add the probe definitions to the API, or, rather than expose phases just have a "bad of objects"?

perdasilva avatar Dec 02 '25 15:12 perdasilva

Pipeline controller notification This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

openshift-ci-robot avatar Dec 02 '25 15:12 openshift-ci-robot

[!IMPORTANT]

Review skipped

Auto reviews are limited based on label configuration.

:no_entry_sign: Excluded labels (none allowed) (1)
  • do-not-merge/work-in-progress

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Dec 02 '25 15:12 coderabbitai[bot]

Skipping CI for Draft Pull Request. If you want CI signal for your change, please convert it to an actual PR. You can still manually trigger a test run with /test all

openshift-ci[bot] avatar Dec 02 '25 15:12 openshift-ci[bot]

Hello @perdasilva! Some important instructions when contributing to openshift/api: API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

openshift-ci[bot] avatar Dec 02 '25 15:12 openshift-ci[bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please assign joelspeed for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci[bot] avatar Dec 02 '25 15:12 openshift-ci[bot]

Following up here - this is in my queue, but I'm working through reading the associated RFE first to ensure I've got full context before reviewing.

I should have an initial review within the next day or so.

everettraven avatar Dec 08 '25 13:12 everettraven

@everettraven Thank you for the thoughtful review. Imma queue up an issue to address what you've identified! =D

perdasilva avatar Jan 19 '26 13:01 perdasilva

@everettraven Thank you for the thoughtful review. Imma queue up an issue to address what you've identified! =D

Just a note, it may be easiest to get everything to a good state in this temporary PR and then use the diff between what exists in this PR and the upstream APIs for making the changes. That way you don't have to do multiple rounds of iteration.

everettraven avatar Jan 19 '26 14:01 everettraven