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

find the true jaeger and grafana URLs to better support different dev evirons #770

Merged
merged 1 commit into from
Jan 7, 2019

Conversation

jmazzitelli
Copy link
Collaborator

This just changes the Makefile, and so only affects devs.

The defaults for the addon URLs are not necessarily correct (for me, they are never correct). This changes the Makefile to more accurately get the Jaeger and Grafana URLs.

… environs.

the defaults are not necessarily correct (for me, they are never correct).
@jmazzitelli
Copy link
Collaborator Author

Note that you can still override the URLs yourself (like you could before) by simply setting the env vars yourself...e.g. this will use your own URLs rather than relying on that "oc" cmd:

JAEGER_URL=http://my-jaeger-is-here:8080 GRAFANA_URL=http://graf:8080 make openshift-deploy

@@ -249,8 +245,14 @@ docker-push:
@$(eval OC ?= $(shell which istiooc 2>/dev/null || which oc))
@${OC} get project ${NAMESPACE} > /dev/null

.openshift-find-addons: .openshift-validate
@$(eval JAEGER_URL ?= $(shell echo http://$$(${OC} get svc tracing -n istio-system -o jsonpath='{.spec.clusterIP}'):80))
Copy link
Contributor

Choose a reason for hiding this comment

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

This should also check if the route is https, like what I did here: https://github.com/kiali/kiali/pull/768/files#diff-5e6fb42f8bf476d6c792f7dcceabf189R147

Copy link
Contributor

@jotak jotak left a comment

Choose a reason for hiding this comment

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

Tested locally, it works for me
And is anyway safe enough to merge as it's only for dev scripts

@jmazzitelli jmazzitelli merged commit 3ca36b5 into kiali:master Jan 7, 2019
@jmazzitelli jmazzitelli deleted the makefile-get-addon-urls branch January 7, 2019 17:08
@hhovsepy
Copy link
Contributor

hhovsepy commented Jan 7, 2019

"make openshift-deploy" causes:
"Makefile:249: *** unterminated call to function eval': missing )'. Stop."

@jmazzitelli
Copy link
Collaborator Author

What version of make are you using? works for me and @jotak ... here's what I have:

$ make --version
GNU Make 3.82

@jmazzitelli
Copy link
Collaborator Author

@hhovsepy see if this workaround helps --- I just replaced the $() notation with `` (backticks). Works still for me.

diff --git a/Makefile b/Makefile
index 9c8f312..e9a9abd 100644
--- a/Makefile
+++ b/Makefile
@@ -246,9 +246,9 @@ docker-push:
        @${OC} get project ${NAMESPACE} > /dev/null
 
 .openshift-find-addons: .openshift-validate
-       @$(eval JAEGER_URL ?= $(shell echo http://$$(${OC} get svc tracing -n istio-system -o jsonpath='{.spec.clusterIP}'):80))
+       @$(eval JAEGER_URL ?= $(shell echo http://`${OC} get svc tracing -n istio-system -o jsonpath='{.spec.clusterIP}'`:80))
        @echo "Found Jaeger at: ${JAEGER_URL}"
-       @$(eval GRAFANA_URL ?= $(shell echo http://$$(${OC} get svc grafana -n istio-system -o jsonpath='{.spec.clusterIP}'):3000))
+       @$(eval GRAFANA_URL ?= $(shell echo http://`${OC} get svc grafana -n istio-system -o jsonpath='{.spec.clusterIP}'`:3000))
        @echo "Found Grafana at: ${GRAFANA_URL}"
 
 ## openshift-deploy: Deploy docker image in Openshift project.
@@ -278,9 +278,9 @@ openshift-reload-image: .openshift-validate
        @${KUBECTL} get namespace ${NAMESPACE} > /dev/null
 
 .k8s-find-addons: .k8s-validate
-       @$(eval JAEGER_URL ?= $(shell echo http://$$(${KUBECTL} get svc tracing -n istio-system -o jsonpath='{.spec.clusterIP}'):80))
+       @$(eval JAEGER_URL ?= $(shell echo http://`${KUBECTL} get svc tracing -n istio-system -o jsonpath='{.spec.clusterIP}'`:80))
        @echo "Found Jaeger at: ${JAEGER_URL}"
-       @$(eval GRAFANA_URL ?= $(shell echo http://$$(${KUBECTL} get svc grafana -n istio-system -o jsonpath='{.spec.clusterIP}'):3000))
+       @$(eval GRAFANA_URL ?= $(shell echo http://`${KUBECTL} get svc grafana -n istio-system -o jsonpath='{.spec.clusterIP}'`:3000))
        @echo "Found Grafana at: ${GRAFANA_URL}"

@hhovsepy
Copy link
Contributor

hhovsepy commented Jan 7, 2019

Mine is also: GNU Make 3.82

@hhovsepy
Copy link
Contributor

hhovsepy commented Jan 7, 2019

But the workaround did not help. Building on CentOS machines.

@jmazzitelli
Copy link
Collaborator Author

jmazzitelli commented Jan 7, 2019

I just tried on a CentOS container (CentOS 7.6.1810) - and it works for me. I git cloned the kiali repo and ran "make .openshift-find-addons" and "make .k8s-find-addons" and it works. I don't have true "oc" or "kubectl" in my CentOS for this test; I just have a dummy "oc" and "kubectl" scripts that spit out "127.0.0.2" - but this should be OK, we just want to see the .openshift-find-addons and .k8s-find-addons targets run successfully:

[root@8a731268688c kiali]# cat /etc/centos-release
CentOS Linux release 7.6.1810 (Core) 
[root@8a731268688c kiali]# make -version
GNU Make 3.82
Built for x86_64-redhat-linux-gnu
Copyright (C) 2010  Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
[root@8a731268688c kiali]# make .openshift-find-addons
Found Jaeger at: http://127.0.0.2:80
Found Grafana at: http://127.0.0.2:3000
[root@8a731268688c kiali]# make .k8s-find-addons
Found Jaeger at: http://127.0.0.2:80
Found Grafana at: http://127.0.0.2:3000

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.

4 participants