gateway: fix the dns discovery method

[bdudelsack]

strip the scheme from the endpoints to have a clean hostname for TCP proxy

Fixes #7452

[fanminshi]

Can you add unit tests for this function? I would like to increase the etcd coverage.

[heyitsanthony]

the discovery stuff should eventually be covered by e2e tests; I don't think a unit test is necessary here

[fanminshi]

@bdudelsack heyitsanthony is right about not having the unit test. The test path should be covered in e2e test where the correct behavior gateway should be verified. Sorry for the wrong suggestion.

[heyitsanthony]

looks OK in general

[heyitsanthony]

use the url library instead?

if u, err := url.Parse(ep); err != nil {
    ep = u.Host
}
endpoints = append(endpoints, ep)

[bdudelsack]

I wanted it to be general and parse the endpoints passed by "--endpoints" param too. I think it could be a common mistake to add "http://" or "https://" to that parameter.

I could use the url library, but then parse only the endpoints which come from the dns discovery, or use this method for both. I don't think url method would work with correct hostnames like "etcd-node:2379" without further checks.

[heyitsanthony]

Oh, sorry. I meant:

if u, err := url.Parse(ep); err == nil {
    ep = u.Host
}
endpoints = append(endpoints, ep)

It'll strip the schema off a valid url and leave anything else (e.g., etcd-node:2379) alone.

[bdudelsack]

url.Parse seems to treat it like a relative path, so my tests are failing:

func stripSchema(eps []string) []string {
	var endpoints []string

	for _, ep := range eps {

		if u, err := url.Parse(ep); err == nil {
			ep = u.Host
		}
		endpoints = append(endpoints, ep)
	}

	return endpoints
}

--- FAIL: TestStripSchema (0.00s)
        gateway_test.go:24: Expected stripSchema to return 'etcd-node:2379' from 'etcd-node:2379' but got ''
        gateway_test.go:24: Expected stripSchema to return 'etcd-node' from 'etcd-node' but got ''

[heyitsanthony]

OK, so if u, err := url.Parse(ep); err == nil && u.Host != ""?

[bdudelsack]

That would work 👌

[heyitsanthony]

standard library packages (e.g., net, os, strings) should be group together at the beginning of the import

[heyitsanthony]

I don't think this test is necessary...

[bdudelsack]

Build failed couse of

$WORK/github.com/coreos/etcd/integration/_test/integration.test: no space left on device

[fanminshi]

lgtm Thanks!