-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Automatically overwrite dashboard UIDs with a prescribed naming pattern, on the provider side #363
base: main
Are you sure you want to change the base?
Conversation
I'd appreciate your review, @Guillaumebeuzeboc, @taurus-forever, @jdkandersson, @cbartz, @camille-rodriguez, @slapcat, @facundofc. |
I'll delegate to @cbartz who has the best understanding of the issue in our team, thank you for preparing the PR! |
6acd42e
to
64ddc56
Compare
9ec7ddb
to
62826cd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This content isn't valid json, so had to modify them for the tests to pass.
62826cd
to
1076450
Compare
9953221
to
e40b35e
Compare
@@ -219,7 +219,7 @@ def __init__(self, *args): | |||
# Increment this PATCH version before using `charmcraft publish-lib` or reset | |||
# to 0 if you are raising the major API version | |||
|
|||
LIBPATCH = 40 | |||
LIBPATCH = 41 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we bump LIBAPI?
On one hand, this change would break existing bookmarks to existing dashboards.
On the other hand, we would like charms to take this fix to UID collision via regular fetch-lib, without needing to manually fetch the next LIBAPI.
Tested with: graph LR
graf --- am
graf --- am2
Where alertmanager was first deployed from edge (with the "old" dashboard lib), then refreshed from path with the new lib from this PR. bundle: kubernetes
applications:
am:
charm: local:alertmanager-k8s-1
resources:
alertmanager-image: 99
scale: 2
trust: true
am2:
charm: local:alertmanager-k8s-2
resources:
alertmanager-image: 99
scale: 1
trust: true
graf:
charm: grafana-k8s
channel: latest/stable
revision: 122
resources:
grafana-image: 70
litestream-image: 45
scale: 1
trust: true
relations:
- - graf:grafana-dashboard
- am:grafana-dashboard
- - graf:grafana-dashboard
- am2:grafana-dashboard |
Issue
Solution
Use shake_256 to generate a prescribed uid, overwriting any pre-existing uid.
In tandem with:
Context
.add_dashboard
method is https://github.com/canonical/cos-registration-server-k8s-operator/blob/6e156f0b1367f7cc6ce12d89f6a367bf491fea7f/src/charm.py#L240.json.loads
them in the provider side.Testing Instructions
First, prepare the charms locally:
--hash
from requirements.txt:Upgrade Notes
dashboard title is not unique in folder
dashboards provisioning provider has no database write permissions because of duplicates
Not saving new dashboard due to restricted database access
To enable automatic uid collision prevention for the dashboards in your charm: