Skip to content

Conversation

joelanford
Copy link
Collaborator

@joelanford joelanford commented Aug 21, 2025

Description

A few small fixups and comment additions to help remember follow-ups:

  1. Make Reconcile and reconcile more consistent between CE and CER reconcilers
    • update status in one place in Reconcile
    • only log reconciler start/stop if we actually call reconcile

NOTE: There is still one major difference: CER updates finalizers as separate patches and continues reconcile logic. CE updates finalizers and requires re-reconcile to continue beyond. The CER logic may cause issues where the next reconcile sees a stale resource version (from finalizer patch) because the status update event isn't seen by our informer in time. This ultimately leads to reconciling a stale CER that manifests as a conflict error in our logs that is often a red herring for users trying to troubleshoot issues in controllers.

Another noticeable difference is that CE uses controller-runtime Finalizers helpers, and CER handles things manually.

Changing this broke unit tests, so I decided not to leave that change out to keep the PR scope small. I'll make a separate PR that focuses just on finalizers.

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

@joelanford joelanford force-pushed the centralized-status-handling branch 2 times, most recently from 9fc9ecc to 225ab4f Compare September 4, 2025 06:36
@codecov-commenter
Copy link

codecov-commenter commented Sep 4, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 67.18750% with 21 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (poc-boxcutter@01cf0d3). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...controllers/clusterextensionrevision_controller.go 64.40% 18 Missing and 3 partials ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@               Coverage Diff                @@
##             poc-boxcutter       #5   +/-   ##
================================================
  Coverage                 ?   71.78%           
================================================
  Files                    ?       85           
  Lines                    ?     8195           
  Branches                 ?        0           
================================================
  Hits                     ?     5883           
  Misses                   ?     1915           
  Partials                 ?      397           
Flag Coverage Δ
e2e 39.73% <7.81%> (?)
experimental-e2e 44.72% <59.37%> (?)
unit 56.41% <54.68%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@joelanford joelanford force-pushed the centralized-status-handling branch from 225ab4f to 9551542 Compare September 4, 2025 11:59
@thetechnick thetechnick force-pushed the poc-boxcutter branch 3 times, most recently from a0adcc0 to c175077 Compare September 5, 2025 14:43
dependabot bot and others added 8 commits September 5, 2025 17:05
Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.23.1 to 1.23.2.
- [Release notes](https://github.com/prometheus/client_golang/releases)
- [Changelog](https://github.com/prometheus/client_golang/blob/v1.23.2/CHANGELOG.md)
- [Commits](prometheus/client_golang@v1.23.1...v1.23.2)

---
updated-dependencies:
- dependency-name: github.com/prometheus/client_golang
  dependency-version: 1.23.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Update boxcutter library to branch with latest k8s and controller-runtime libs

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

* Update go.mod

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

* Add ClusterExtensionRevisionAPI

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

* Add BoxcutterRuntime featuregate

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

* Add Boxcutter applier

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

* Add ClusterExtensionRevision controller

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

* Add Boxcutter runtime to main

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

* Remove ClusterExtensionRevision from crd-docs

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

* Update hack/tools/update-crds.sh for ClusterExtensionRevision API

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

* Generate manifests

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

* Remove access manager and dynamic cache

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

* Update boxcutter to v0.3.0, add TrackingCache to Runnables

* boxcutter webhook support

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

* add BoxcutterRuntime feature gate to experimental release

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

* add boxcutter cluster-admin cluster role binding in boxcutter's feature component

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

* Boxcutter Preflight

Signed-off-by: Todd Short <tshort@redhat.com>

* Boxcutter Preflight mock cleanup

Signed-off-by: Todd Short <tshort@redhat.com>

* Use new TrackingCache Watch/Free.

Ensure informers are started before reconciling and stopped before
removing the finalizer.

* add BoxcutterInstalledBundleGetter, plumb bundle metadata into revision annotations

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

* InstalledBundleGetter -> RevisionStatesGetter

This change accommodates the possibility of a revision that is currently
rolling out, which is possible for appliers that perform rollouts
asynchronously.

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

* refactor Applier interface and improve status reporting

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

* fixup tests for applier and installedbundlegetter changes

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

* resolve linter issues

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

* set status for other failure modes during ClusterExtensionRevision reconciliation

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

* TODO: fail upgrade-e2e if revision storage is unmigrated

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

* fixing broken tests after rebase

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

* Boxcutter Phases

Defines a set of phases which facilitate a smoother installation vs applying every resource in the bundle all at once.

Signed-off-by: Daniel Franz <dfranz@redhat.com>

* Const Cleanup

Captures conditions and reasons used by ClusterExtensionRevision into consts.

Signed-off-by: Daniel Franz <dfranz@redhat.com>

* Add migration from helm to boxcutter revision

---------

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
Signed-off-by: Todd Short <tshort@redhat.com>
Signed-off-by: Daniel Franz <dfranz@redhat.com>
Co-authored-by: Per Goncalves da Silva <pegoncal@redhat.com>
Co-authored-by: Joe Lanford <joe.lanford@gmail.com>
Co-authored-by: Todd Short <tshort@redhat.com>
Co-authored-by: Daniel Franz <dfranz@redhat.com>
…or-framework#2196)

Bumps [pkg.package-operator.run/boxcutter](https://github.com/package-operator/boxcutter) from 0.5.1 to 0.6.0.
- [Release notes](https://github.com/package-operator/boxcutter/releases)
- [Commits](package-operator/boxcutter@v0.5.1...v0.6.0)

---
updated-dependencies:
- dependency-name: pkg.package-operator.run/boxcutter
  dependency-version: 0.6.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [golang.org/x/mod](https://github.com/golang/mod) from 0.27.0 to 0.28.0.
- [Commits](golang/mod@v0.27.0...v0.28.0)

---
updated-dependencies:
- dependency-name: golang.org/x/mod
  dependency-version: 0.28.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [mkdocs-material](https://github.com/squidfunk/mkdocs-material) from 9.6.18 to 9.6.19.
- [Release notes](https://github.com/squidfunk/mkdocs-material/releases)
- [Changelog](https://github.com/squidfunk/mkdocs-material/blob/master/CHANGELOG)
- [Commits](squidfunk/mkdocs-material@9.6.18...9.6.19)

---
updated-dependencies:
- dependency-name: mkdocs-material
  dependency-version: 9.6.19
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [golang.org/x/sync](https://github.com/golang/sync) from 0.16.0 to 0.17.0.
- [Commits](golang/sync@v0.16.0...v0.17.0)

---
updated-dependencies:
- dependency-name: golang.org/x/sync
  dependency-version: 0.17.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
Co-authored-by: Per Goncalves da Silva <pegoncal@redhat.com>
Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
@joelanford
Copy link
Collaborator Author

Re-opened here: operator-framework#2200

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.

4 participants