villebro opened a new pull request, #173: URL: https://github.com/apache/superset-kubernetes-operator/pull/173
## Summary The operator currently performs a semver comparison between the last-applied and desired lifecycle image tags and hard-blocks any version decrease, setting `status.phase: Blocked` permanently. This strands deployments in a state they cannot escape: if a major upgrade fails mid-migration and the database is restored to a pre-upgrade backup, pinning the image back to the previous version is (correctly) a lower tag — and the operator refuses to reconcile it. There is no supported way to revert. This PR removes downgrade blocking entirely. Any change to the lifecycle image tag now re-runs the migrate task (`superset db upgrade`) regardless of direction. This matches the behavior of the official Apache Superset Helm chart, whose `post-install,post-upgrade` init hook runs `superset db upgrade` on every upgrade with no version-direction logic. Superset does provide `superset db downgrade`, but its down migrations are poorly tested and often break, so the operator never runs them — a lower image tag simply re-runs the forward migration. Blocking the change added no safety, only a dead end. Taking a database backup before an upgrade remains the recommended safety net (and lands as a follow-up feature). **Breaking change** (still v0): the `direction` field is removed from `status.lifecycle.upgrade`, and the `VersionComparisonSkipped` warning event no longer fires. Supervised upgrade approval is unchanged. ## Details - Trim `version.go` to just the `ImageRef` helper; delete `CompareVersions`, `VersionDirection`, and the direction constants. This drops the only direct use of `Masterminds/semver` (now an indirect dependency). - `checkUpgradeGates` no longer inspects direction, blocks downgrades, or emits `VersionComparisonSkipped`. It records the in-flight upgrade context and enforces supervised approval only; migrate re-runs via its existing checksum input, which already hashes the image. With the downgrade branch gone the function only ever gates on approval, so it now returns a single `bool` and the caller produces the wait result. - Remove `UpgradeContext.direction` (and its CEL enum) from the API; regenerate CRDs, Helm CRDs, and the API reference via `make codegen`. - Repurpose the `Blocked` phase docs to its remaining use (invalid `seed.cronSchedule`). - Delete the `FuzzCompareVersions` target and corpus. Drop the now-redundant `downgrade proceeds` and `non-semver tags proceed` unit tests — direction is no longer special-cased, so the general "an image change proceeds and re-runs migrate" test already covers those paths. - Codify this in the testing philosophy: add a **"Don't test non-actions"** guideline to `development-guidelines.md` so we delete a feature's tests rather than inverting them into "X no longer happens" assertions. - Update lifecycle/architecture docs and add an Unreleased changelog entry. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
