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.

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

fix: Test grafana dashboards in hubble deploy #650

Merged
merged 5 commits into from
Aug 30, 2024

Conversation

whatnick
Copy link
Contributor

@whatnick whatnick commented Aug 24, 2024

Description

Closes #501

Related Issue

The refactored hubble deployment included in grafana dashboard testing. Also renamed the dashboards folder to match the grafana application and the Makefile setup. This make phony is not run in CI and tests are run via make test so this folder issue does not show up.

Checklist

  • I have read the contributing documentation.
  • I signed and signed-off the commits (git commit -S -s ...). See this documentation on signing commits.
  • I have correctly attributed the author(s) of the code.
  • I have tested the changes locally.
  • I have followed the project's style guidelines.
  • I have updated the documentation, if necessary.
  • I have added tests, if applicable.

Screenshots (if applicable) or Testing Completed

go test ./... -tags=dashboard -v
=== RUN   TestDashboardsAreSimplified
    simplify-grafana_test.go:21: verifying that dashboard is simplified: ../../../hubble/grafana/dashboards/clusters.json
    simplify-grafana_test.go:21: verifying that dashboard is simplified: ../../../hubble/grafana/dashboards/dns.json
    simplify-grafana_test.go:21: verifying that dashboard is simplified: ../../../hubble/grafana/dashboards/pod-flows-namespace.json
    simplify-grafana_test.go:21: verifying that dashboard is simplified: ../../../hubble/grafana/dashboards/pod-flows-workload.json
    simplify-grafana_test.go:21: verifying that dashboard is simplified: ../../../legacy/grafana/dashboards/clusters.json
    simplify-grafana_test.go:21: verifying that dashboard is simplified: ../../../legacy/grafana/dashboards/dns.json
    simplify-grafana_test.go:21: verifying that dashboard is simplified: ../../../legacy/grafana/dashboards/pod-level.json
--- PASS: TestDashboardsAreSimplified (0.01s)
PASS
ok      github.com/microsoft/retina/deploy/testutils/grafana/dashboards (cached)
make simplify-dashboards
cd deploy/testutils && go test ./... -tags=dashboard,simplifydashboard -v && cd /home/tisham/dev/retina
=== RUN   TestOverwriteDashboards
    simplify-grafana-overwrite_test.go:20: simplifying/overwriting dashboard: ../../../hubble/grafana/dashboards/clusters.json
    simplify-grafana-overwrite_test.go:20: simplifying/overwriting dashboard: ../../../hubble/grafana/dashboards/dns.json
    simplify-grafana-overwrite_test.go:20: simplifying/overwriting dashboard: ../../../hubble/grafana/dashboards/pod-flows-namespace.json
    simplify-grafana-overwrite_test.go:20: simplifying/overwriting dashboard: ../../../hubble/grafana/dashboards/pod-flows-workload.json
    simplify-grafana-overwrite_test.go:20: simplifying/overwriting dashboard: ../../../legacy/grafana/dashboards/clusters.json
    simplify-grafana-overwrite_test.go:20: simplifying/overwriting dashboard: ../../../legacy/grafana/dashboards/dns.json
    simplify-grafana-overwrite_test.go:20: simplifying/overwriting dashboard: ../../../legacy/grafana/dashboards/pod-level.json
--- PASS: TestOverwriteDashboards (0.01s)
PASS
ok      github.com/microsoft/retina/deploy/testutils/grafana/dashboards 0.013s

Additional Notes

This PR duplicates code if more dashboard folders are added it would perhaps be useful to improve the go test code to receive a list of folders instead.


Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.

@whatnick whatnick requested a review from a team as a code owner August 24, 2024 11:09
@whatnick
Copy link
Contributor Author

whatnick commented Aug 24, 2024

@microsoft-github-policy-service agree

Copy link
Contributor

@huntergregory huntergregory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking up this change @whatnick ! Mainly just had a thought about refactoring

Makefile Outdated Show resolved Hide resolved
Copy link
Contributor

@huntergregory huntergregory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thank you @whatnick ! If you haven't yet, could you just double check that simplifying dashboards works with the new make code? I would suggest locally reverting one of the deploy/hubble/ json files and seeing if it changes again after running make.

@whatnick
Copy link
Contributor Author

This is great, thank you @whatnick ! If you haven't yet, could you just double check that simplifying dashboards works with the new make code? I would suggest locally reverting one of the deploy/hubble/ json files and seeing if it changes again after running make.

Yep manual changes get reverted by the test in overwrite mode.

@nddq nddq enabled auto-merge August 30, 2024 14:13
@nddq nddq added this pull request to the merge queue Aug 30, 2024
Merged via the queue into microsoft:main with commit 3abb947 Aug 30, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dashboard tests are not running
3 participants