Skip to content
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.

Sign up for GitHub

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

Jump to bottom

List Grafana dashboard UIDs, enforce them, and use them in MetalK8s UI links #3483

Merged
merged 3 commits into from
Aug 5, 2021
Merged

List Grafana dashboard UIDs, enforce them, and use them in MetalK8s UI links #3483

merged 3 commits into from
Aug 5, 2021

Conversation

gdemonet
Copy link
Contributor

@gdemonet gdemonet commented Aug 5, 2021 •
edited
Loading

What does this PR change?

  • it introduces a simple listing of Grafana dashboard UIDs (charts/grafana_dashboard_uids.json), which maps dashboard titles to their UID
  • it enforces that rendered Helm charts use the same UIDs (if unspecified in the chart, we inject it at render-time)
  • it ensures the UI links are using the UID-based URL format (instead of slug-based)
  • it ensures the listing under charts/ is always up-to-date with what we deploy, through a post-install E2E test

What's missing?:

  • a clean method for ensuring that UI links are in-sync with the listing under charts/ (will be tackled in a later PR)

Closes: #3475
Closes: #2820

@bert-e
Copy link
Contributor

bert-e commented Aug 5, 2021

Hello gdemonet,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Status report is not available.

@bert-e
Copy link
Contributor

bert-e commented Aug 5, 2021

Conflict

A conflict has been raised during the creation of
integration branch w/2.11/bugfix/3475-use-dashboard-uids-in-links with contents from bugfix/3475-use-dashboard-uids-in-links
and development/2.11.

I have not created the integration branch.

Here are the steps to resolve this conflict:

 $ git fetch
 $ git checkout -B w/2.11/bugfix/3475-use-dashboard-uids-in-links origin/development/2.11
 $ git merge origin/bugfix/3475-use-dashboard-uids-in-links
 $ # <intense conflict resolution>
 $ git commit
 $ git push -u origin w/2.11/bugfix/3475-use-dashboard-uids-in-links

@gdemonet gdemonet added complexity:medium Something that requires one or few days to fix javascript Pull requests that update Javascript code kind:bug Something isn't working python Pull requests that update Python code topic:monitoring Everything related to monitoring of services in a running cluster topic:ui UI-related issues labels Aug 5, 2021
@gdemonet
charts, salt: List and check dashboard UIDs
a6f8b5f 
The dashboards we provision with MetalK8s have (or should have, which is
why some of this commit's changes are setting some UIDs) a UID. Starting
with Grafana v8.0, linking dashboards using their slug is not available
anymore (see
https://grafana.com/docs/grafana/latest/release-notes/release-notes-8-0-0/#breaking-changes
).

To make sure we can use these UIDs safely in links (mostly from MetalK8s
UI), we start by listing all of them in a JSON file, and enforce that
they are correct when rendering Helm charts.

Verifying the final deployed dashboards will be done in a subsequent
commit, through a post-install E2E test.

See: #3475
@gdemonet
ui: Use dashboard UIDs for stable links
fc0fa3b 
TODO: add a simple unit-test to verify the UIDs are aligned with
`charts/grafana_dashboard_uids.json`

Fixes: #3475
@gdemonet
tests: Ensure expected Grafana dashboards are available
18aff8e 
We add a test which compares the deployed Grafana dashboards with what
is declared in `charts/grafana_dashboard_uids.json`.

It will detect:
- missing and extra dashboards
- UID mismatches

This will ensure we can trust the listing in this file, and use it for
documenation and/or UI links.

See: #3475
@gdemonet gdemonet force-pushed the bugfix/3475-use-dashboard-uids-in-links branch from cb690ff to 18aff8e Compare August 5, 2021 12:18
@bert-e
Copy link
Contributor

bert-e commented Aug 5, 2021

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

@gdemonet gdemonet marked this pull request as ready for review August 5, 2021 14:01
@gdemonet gdemonet requested a review from a team as a code owner August 5, 2021 14:01
@gdemonet
Copy link
Contributor Author

gdemonet commented Aug 5, 2021

/approve

@bert-e
Copy link
Contributor

bert-e commented Aug 5, 2021

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

  • ✔️ development/2.10

  • ✔️ development/2.11

The following branches will NOT be impacted:

  • development/2.0
  • development/2.1
  • development/2.2
  • development/2.3
  • development/2.4
  • development/2.5
  • development/2.6
  • development/2.7
  • development/2.8
  • development/2.9

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

  • Any commit you add on the source branch will trigger a new cycle after the
    current queue is merged.
  • Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

@bert-e
Copy link
Contributor

bert-e commented Aug 5, 2021

I have successfully merged the changeset of this pull request
into targetted development branches:

  • ✔️ development/2.10

  • ✔️ development/2.11

The following branches have NOT changed:

  • development/2.0
  • development/2.1
  • development/2.2
  • development/2.3
  • development/2.4
  • development/2.5
  • development/2.6
  • development/2.7
  • development/2.8
  • development/2.9

Please check the status of the associated issue None.

Goodbye gdemonet.

@bert-e bert-e merged commit c87be34 into development/2.10 Aug 5, 2021
@bert-e bert-e deleted the bugfix/3475-use-dashboard-uids-in-links branch August 5, 2021 19:59
gdemonet added a commit that referenced this pull request Aug 5, 2021
@gdemonet
changelog: Update for 2.10.1 release
60f8cb3 
Refs: #3469
Refs: #3477
Refs: #3481
Refs: #3479
Refs: #3484
Refs: #3483
gdemonet added a commit that referenced this pull request Aug 12, 2021
@gdemonet
charts, tests: Rework patching of Grafana dashboards
2caa241 
In the previous approach (see #3483), we used
`charts/grafana_dashboard_uids.json` as a way to patch and check UIDs
(both at render-time and run-time).

The new approach distinguishes between patching and checking:
- `charts/grafana_dashboard_patches.json` stores a map of patches to
  apply to some dashboards included in charts
- `tests/post/steps/files/grafana_dashboard_uids.json` contain the full
  map of dashboard UIDs for run-time validation

We lose the render-time check, but clarify the role of each file.
gdemonet added a commit that referenced this pull request Aug 12, 2021
@gdemonet
charts, tests: Rework patching of Grafana dashboards
ec357ac 
In the previous approach (see #3483), we used
`charts/grafana_dashboard_uids.json` as a way to patch and check UIDs
(both at render-time and run-time).

The new approach distinguishes between patching and checking:
- `charts/grafana_dashboard_patches.json` stores a map of patches to
  apply to some dashboards included in charts
- `tests/post/steps/files/grafana_dashboard_uids.json` contain the full
  map of dashboard UIDs for run-time validation

We lose the render-time check, but clarify the role of each file.
gdemonet added a commit that referenced this pull request Aug 12, 2021
@gdemonet
charts, tests: Rework patching of Grafana dashboards
ec62b1a 
In the previous approach (see #3483), we used
`charts/grafana_dashboard_uids.json` as a way to patch and check UIDs
(both at render-time and run-time).

The new approach distinguishes between patching and checking:
- `charts/grafana_dashboard_patches.json` stores a map of patches to
  apply to some dashboards included in charts
- `tests/post/steps/files/grafana_dashboard_uids.json` contain the full
  map of dashboard UIDs for run-time validation

We lose the render-time check, but clarify the role of each file.
gdemonet added a commit that referenced this pull request Aug 13, 2021
@gdemonet
charts, tests: Rework patching of Grafana dashboards
121d328 
In the previous approach (see #3483), we used
`charts/grafana_dashboard_uids.json` as a way to patch and check UIDs
(both at render-time and run-time).

The new approach distinguishes between patching and checking:
- `charts/grafana_dashboard_patches.json` stores a map of patches to
  apply to some dashboards included in charts
- `tests/post/steps/files/grafana_dashboard_uids.json` contain the full
  map of dashboard UIDs for run-time validation

We lose the render-time check, but clarify the role of each file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexandre-allard alexandre-allard alexandre-allard approved these changes

Assignees
No one assigned
Labels
complexity:medium Something that requires one or few days to fix javascript Pull requests that update Javascript code kind:bug Something isn't working python Pull requests that update Python code topic:monitoring Everything related to monitoring of services in a running cluster topic:ui UI-related issues
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gdemonet @bert-e @alexandre-allard