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

victoria-metrics-k8s-stack: optional allowCrossNamespaceImport in GrafanaDashboard(s) #788

Merged

Conversation

plevart
Copy link
Contributor

@plevart plevart commented Dec 9, 2023

In victoria-metrics-k8s-stack Helm Chart, when values define grafanaOperatorDashboardsFormat.enabled=true, special GrafanaDashboard CRs get rendered that provide dashboards to Grafana Operator (https://grafana-operator.github.io/grafana-operator/). But those CRs don't get imported into Grafana instance when the Grafana Operator is not deployed to the same k8s namespace as the victoria-metrics-k8s-stack chart is, since GrafanaDashboard CRs are created in this namespace.
The provided patch allows overriding an additional boolean attribute in the chart values: grafanaOperatorDashboardsFormat.allowCrossNamespaceImport which is false by default. When set to true, the created GrafanaDashboard CRs will also contain this attribute and Grafana Operator will import them from a namespace that is different than the Grafana Operator's namespace.

@Haleygo
Copy link
Contributor

Haleygo commented Dec 11, 2023

Thanks for the pull request!
You also need to add this to the dashboard sync script as a part of standard header.
And could you help adding some changelog here?

@plevart
Copy link
Contributor Author

plevart commented Dec 11, 2023

@Haleygo I see. So the GrafanaDashboard resource template embedded in the python script is rendered with the same "values.yaml" as the dashboard templates in the Helm chart? I can just add the same modifications using same references to .Values dictionary? Out of curiosity, when is this script called?

…fanaDashboard CRs for Grafana Operator - also add to sync_grafana_dashboards.py
@plevart
Copy link
Contributor Author

plevart commented Dec 11, 2023

So, here it is, @Haleygo ... I think I understand it now. The python script is used from the top Makefile to generate various Helm template files in the Helm charts from source JSON files located on various other repositories. So for example, if some other source is introduced, its Helm template can be generated by the script.

I ran the following command from top dir: make sync-dashboards to regenerate the dashboard templates and the result was exactly the same - there were no changes to the files already part of git repo:

[peter@sun vm-helm-charts]$ git diff
diff --git a/Makefile b/Makefile
index 131ea0b..e5ff369 100644
--- a/Makefile
+++ b/Makefile
@@ -143,8 +143,7 @@ sync-rules:
 # Synchronize grafana dashboards in charts/victoria-metrics-k8s-stack/templates/grafana/dashboards
 sync-dashboards:
        docker run --rm \
-               --user $(shell id -u):$(shell id -g) \
-               --mount type=bind,src="$(shell pwd)/charts/victoria-metrics-k8s-stack",dst=/k8s-stack \
+               --volume "$(shell pwd)/charts/victoria-metrics-k8s-stack":/k8s-stack:z \
                -w /k8s-stack/hack/ \
                $(PYTHON_IMAGE) sh -c "\
                        pip3 install --no-cache-dir --no-build-isolation -r requirements.txt --user && python3 sync_grafana_dashboards.py \

I just had to change the Makefile temporarily since I'm using podman in place of docker and podman, by default, runs rootles containers so:

  • you do not need (neither want) --user option (uids are mapped and root user in container is mapped to user running the rootless container on the host)
  • you need :z option at the end of --volume option (instead of using --mount) so that rootless container can access mounted directory from host due to SELlinux restrictions for rootless containers (the directory on the host and all files in it are re-labeled to system_u:object_r:container_file_t:s0 so that container process can access it).

Copy link
Contributor

@Haleygo Haleygo left a comment

Choose a reason for hiding this comment

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

LGTM!
Thanks for contribution!

@Haleygo Haleygo merged commit 049a283 into VictoriaMetrics:master Dec 12, 2023
@plevart plevart deleted the vm-stack-gop-allowCrossNamespaceImport branch December 12, 2023 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants