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

Test suite fails when a local HTTP proxy is configured #3585

Closed
mpalmer opened this Issue Dec 14, 2017 · 10 comments

Comments

Projects
None yet
3 participants
@mpalmer
Copy link
Contributor

mpalmer commented Dec 14, 2017

What did you do?

I ran make test in an environment where the http_proxy environment variable was pointing to a Squid 3 proxy.

What did you expect to see?

A successful test suite run.

What did you see instead? Under which circumstances?

--- FAIL: TestRoutePrefix (5.03s)
        web_test.go:259:
                exp: 200
                
                got: 503
--- FAIL: TestReadyAndHealthy (5.04s)
        web_test.go:163:
                exp: 200
                
                got: 503
FAIL
FAIL    github.com/prometheus/prometheus/web    5.305s

The Squid error page included the text:

The following error was encountered while trying to retrieve the URL: :9090
Invalid URL
Some aspect of the requested URL is incorrect.

The last time I ran the test suite was around the time I submitted #3138, and it worked fine then (and I've had http_proxy set for the better part of a decade), so that should narrow down the period in which the problem was introduced.

Environment

  • System information:

    Linux 4.9.0-3-amd64 x86_64

  • Prometheus version:

Git commit b5c30090b

  • Prometheus configuration file:

N/A

@krasi-georgiev

This comment has been minimized.

Copy link
Member

krasi-georgiev commented Dec 14, 2017

I think this is the expected behaviour.
what happens is that http.Get is trying to connect to prometheus through the proxy and it fails.
I tried setting up a global proxy on OSX and the test still passes which means the the http go package ingores the proxy on OSX. I guess this is different under Linux.
There should be an option in linux to set it so that the proxy is ignored for localhost or you can run the tests using http_proxy= make test (which sets http_proxy to empty while running make test)

@gouthamve

This comment has been minimized.

Copy link
Member

gouthamve commented Dec 19, 2017

👍 to what @krasi-georgiev said. You could just run it with http_proxy= make test.

@gouthamve gouthamve closed this Dec 19, 2017

@mpalmer

This comment has been minimized.

Copy link
Contributor Author

mpalmer commented Dec 20, 2017

Sweet merciful heavens... I know I can unset the environment variable, but (a) this used to work, and (b) you're sending invalid HTTP requests.

@krasi-georgiev

This comment has been minimized.

Copy link
Member

krasi-georgiev commented Dec 20, 2017

@mpalmer these tests didn't exist when you submitted your PR on Sep the 5th so it explains why it worked then :)
anyway this is an easy fix and I added it to another PR that is already open for the same test.
#3587
It would be great if you could test it on your system and confirm that fixed your issue before it gets merged.

here is a link to the PR branch
https://github.com/krasi-georgiev/prometheus/tree/web-test-error-check

@mpalmer

This comment has been minimized.

Copy link
Contributor Author

mpalmer commented Dec 20, 2017

Unfortunately, running make test against a4fc8c70 (current HEAD commit of #3587) doesn't seem to change the behaviour of the test suite -- the same two tests still fail, and the error page from the proxy appears to be unchanged. From what little I know of Go, and a quick read through the http.Transport docs, I can't see why it shouldn't be working, though. Very mysterious.

@krasi-georgiev

This comment has been minimized.

Copy link
Member

krasi-georgiev commented Dec 20, 2017

I had a another try, but seems that it is not a trivial fix.
Found a very good workaround to bypass the proxy for local address which is usually the desired behaviour anyway:
export no_proxy="localhost,127.0.0.1"

I tested it on Debian it works as advertised and all tests passed.
Will think how to adjust the tests but in the meanwhile this should do the job.

@krasi-georgiev

This comment has been minimized.

Copy link
Member

krasi-georgiev commented Dec 20, 2017

hm , I tested this again and now the grpc fails so not a good workaround after all. Will try to find a better solution.

@krasi-georgiev

This comment has been minimized.

Copy link
Member

krasi-georgiev commented Dec 20, 2017

found the magic string 🥇 os.Setenv("no_proxy", "localhost,127.0.0.1,0.0.0.0,:")

added the fix to the same PR so download the same branch and confirm if it works on your box.

@mpalmer

This comment has been minimized.

Copy link
Contributor Author

mpalmer commented Dec 20, 2017

Yay! make test against c94fa731 works! Thanks!

@lock

This comment has been minimized.

Copy link

lock bot commented Mar 23, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 23, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.