Ship Grafana 12.4.5 rather than 13.1.0, and gate the openid restart#679
Merged
Conversation
The restart loop hardcoded 'for D in mqtt openid', while every other openid reference in this file is wrapped in an openid.enabled guard. With openid.enabled=false - which the v6 release notes document as supported - the deployment does not exist, so the rollout restart exits 1. The Job is restartPolicy OnFailure with backoffLimit 9999, and each retry starts by restarting mqtt again, so the broker is bounced every few minutes for weeks, every edge agent reconnect-loops, and the Job never completes. helm upgrade reports success throughout, because the Job is a plain resource rather than a hook.
Grafana 13 deleted the sqlstore migration that adds playlist.created_at/updated_at, which existed from 10.2.0 to 12.4.x, while still running a unified-storage playlist migration whose SQL requires them. Any database created before 10.2.0 - including every ACS v5.1.0 site, which shipped Grafana 10.0.1 - cannot start under 13. Skipping that migration, as the previous fix did, is not an option: Grafana 13 removed the legacy playlist service entirely and serves playlists from unified storage, so skipping it makes every existing playlist disappear from the UI. ACS installations do use playlists, to rotate dashboards on wall displays. 12.4.x still ships playlist_mig.go, so Grafana's own framework adds the columns from whatever schema a site starts on, then completes the playlist migration and records it in unifiedstorage_migration_log. A later move to 13 is safe, because 13 skips the migration on that row. Everything the 13 bump was for survives: 10.x is still two majors behind on security fixes, and auth.jwt with its dev-mode JWKS exemption is present and identical at 12.4.5. Also pin autoMigrationThreshold for folders and dashboards. Grafana 12.4.x auto-migrates both into unified storage when a site has fewer dashboards than the default threshold of 10, one-way. ACS provisions five, so without this a fresh install takes a migration an established site does not - splitting the estate on a row count and leaving CI exercising a different path from production.
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
1 participant
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.
Supersedes #678, which is reverted here. rc.5 is marked DO NOT USE.
Why 13 cannot ship
Grafana 13 deleted the sqlstore migration that adds
playlist.created_at/updated_at(playlist_mig.gois 404 at v13.0.0+;migrations.gohas zero occurrences of "playlist"). Those columns arrived in 10.2.0 and exist through 12.4.x. Grafana 13 nevertheless still runs a unified-storage playlist migration, enabled by default, whose SQL selectsp.created_at— so any database created before 10.2.0 cannot start under 13. ACS v5.1.0 ships Grafana 10.0.1, so this is every upgrading site. Fresh installs are fine because v13 seeds the columns from its schema snapshot, which is why this reached rc.4 undetected.Reproduced on amrc-fpd-jahx upgrading v5.1.0 -> v6.0.0-rc.4, and reproduced again locally against a v10.0.1-shaped SQLite database using v13's own
query_playlists.sql.Why #678 was wrong
#678 set
enableMigration: falseon the assumption that ACS does not use playlists. It does — playlists rotate dashboards on wall displays. Grafana 13 removed the legacy playlist service entirely (pkg/services/playlistis 404 at v13.1.0, 200 at v12.4.5) and serves/api/playliststhrough a k8s client against unified storage, so with the migration skipped nothing can read the surviving legacy rows — every playlist silently disappears from the UI.Worse, it is a trap for anyone who ran rc.5: playlists recreated there live only in unified storage, and the later 12.4.5 migration does a full collection rebuild from the legacy table, destroying them. Grafana's own guard against that (
isAlreadyOnUnifiedStorage, commented "would wipe unistore and repopulate from sql, resulting in data loss") only fires for folders/dashboards, never playlists.The fix
Pin 12.4.5. It still ships
playlist_mig.go, so Grafana's own framework adds the columns from whatever schema a site starts on — no custom DDL, no assumptions about the starting version. It then runs the playlist migration itself and records it inunifiedstorage_migration_log. A later move to 13 is safe: 13 skips on that row's existence (pkg/services/sqlstore/migrator/migrator.go:279-287). This is a permanent fix, not a deferral.Everything #670 wanted from 13 survives: 10.x left support in 2024 so 12.4.5 is still two majors of security fixes, and
auth.jwt(url_login,jwk_set_url, and theCfg.Env != setting.DevJWKS-over-http exemption) is present and identical at 12.4.5.Also: pin autoMigrationThreshold
12.4.x auto-migrates folders and dashboards into unified storage — one-way — when a site has fewer dashboards than
DefaultAutoMigrationThreshold(10). ACS provisions five, so a fresh install would silently take a migration an established site does not, splitting the estate into two storage modes on a row count and leaving CI exercising a different path from production. Setting the threshold to 1 makes every site behave identically.Also: gate the openid restart
service-setuphardcodedfor D in mqtt openid, while every other openid reference in that file is gated onopenid.enabled. Withopenid.enabled=false— documented as supported — the rollout restart exits 1, the Job (OnFailure,backoffLimit: 9999) retries, and each retry restarts mqtt first. The broker bounces every few minutes for weeks, every edge agent reconnect-loops, andhelm upgradereports success throughout.How to test
SELECT count(*) FROM playlistingrafana.dbbefore upgrading. On a cluster already running an rc,/api/playlistsreturns 0 and would give a false green.kubectl logs deploy/acs-grafana -c grafana | grep -iE 'Rejected item|Count validation'— require zero rejections. The validator adds rejected items back into its expected total, so a partial migration reports success, commits the log row, and every future 13.x boot skips it permanently.helm template --set openid.enabled=falserendersfor D in mqtt;.Release notes
The bundled Grafana now moves from 10.0.1 to 12.4.5 rather than 13.1.0. Grafana 13 cannot start against a database created before Grafana 10.2.0, which includes every ACS v5.1.0 installation. Playlists are migrated into Grafana's unified storage on first boot; this is one-way, so snapshot the Grafana volume first and check your playlists are all present afterwards.
Fixes an MQTT restart loop when
openid.enabledis false.