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: use non-fqdn url for grafana to support non-standard cluster names #791

Merged
merged 3 commits into from Jul 20, 2022

Conversation

weisdd
Copy link
Contributor

@weisdd weisdd commented Jul 18, 2022

Description

Currently, grafana-operator uses FQDN names for its calls to grafana API (e.g. http://grafana.monitoring.svc.cluster.local:3000), which is incompatible with custom cluster names.
For internal DNS requests, there's no need to rely on FQDN, <svc-name>.<namespace>:<port> format is perfectly fine.
The PR addresses that.

Additional changes:

  • Some code style issues are automatically fixed by gofumpt;
  • Fixed staticcheck complaint about error formatting.

Relevant issues/tickets

Fixes: #592

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • This change requires a documentation update
  • I have added tests that prove my fix is effective or that my feature works
  • I have added a test case that will be used to verify my changes
  • Verified independently on a cluster by reviewer

Verification steps

  1. Deploy grafana instance with any spec
  2. Make sure operator is able to communicate with grafana:
  • For clusters with standard names, operator should work just as before;
  • For clusters with a custom name, operator should start working.

Signed-off-by: Igor Beliakov <demtis.register@gmail.com>
@HubertStefanski
Copy link
Member

HubertStefanski commented Jul 19, 2022

Works fine with preferService: False but I get this error on OpenShift when set to True

	running periodic dashboard resync
2022-07-19T12:27:11.335+0100	ERROR	failed to get or create namespace folder for dashboard	{"folder": "hstefans", "dashboard": "", "error": "Get \"http://admin:***@grafana-service.hstefans:3000/api/folders\": dial tcp: lookup grafana-service.hstefans: no such host"}
github.com/go-logr/zapr.(*zapLogger).Error
	/home/hstefans/go/src/github.com/grafana-operator/grafana-operator/vendor/github.com/go-logr/zapr/zapr.go:132
sigs.k8s.io/controller-runtime/pkg/log.(*DelegatingLogger).Error
	/home/hstefans/go/src/github.com/grafana-operator/grafana-operator/vendor/sigs.k8s.io/controller-runtime/pkg/log/deleg.go:144
github.com/grafana-operator/grafana-operator/v4/controllers/grafanadashboard.(*GrafanaDashboardReconciler).reconcileDashboards
	/home/hstefans/go/src/github.com/grafana-operator/grafana-operator/controllers/grafanadashboard/grafanadashboard_controller.go:294
github.com/grafana-operator/grafana-operator/v4/controllers/grafanadashboard.(*GrafanaDashboardReconciler).Reconcile
	/home/hstefans/go/src/github.com/grafana-operator/grafana-operator/controllers/grafanadashboard/grafanadashboard_controller.go:101
github.com/grafana-operator/grafana-operator/v4/controllers/grafanadashboard.SetupWithManager.func1
	/home/hstefans/go/src/github.com/grafana-operator/grafana-operator/controllers/grafanadashboard/grafanadashboard_controller.go:184
github.com/grafana-operator/grafana-operator/v4/controllers/grafanadashboard.SetupWithManager.func2
	/home/hstefans/go/src/github.com/grafana-operator/grafana-operator/controllers/grafanadashboard/grafanadashboard_controller.go:193
2022-07-19T12:27:11.335+0100	ERROR	error updating dashboard	{"name": "simple-dashboard", "namespace": "hstefans", "error": "Get \"http://admin:***@grafana-service.hstefans:3000/api/folders\": dial tcp: lookup grafana-service.hstefans: no such host"}

the service does exist
image

I'm also not 100% sure this is an issue with your PR specifically.

code looks good other than that

Copy link
Member

@HubertStefanski HubertStefanski left a comment

Approving! I managed to verify this on my Openshift cluster, I must have wrongly configured something with the previous attempt, LGTM

@HubertStefanski HubertStefanski merged commit 4b1c75b into grafana-operator:master Jul 20, 2022
8 checks passed
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.

Cannot create dashboard with custom clustername
2 participants