From 2740b951b1248cbe9287879156456e13ace3a3d9 Mon Sep 17 00:00:00 2001 From: daniel-caballero Date: Tue, 12 Dec 2017 01:33:51 +0100 Subject: [PATCH 1/4] first version of the discovery module, bringing (integration) tests for eureka compat, and an in progress implementation --- discovery/eureka.go | 44 +++++++++++++++ discovery/eureka_test.go | 119 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 163 insertions(+) create mode 100644 discovery/eureka.go create mode 100644 discovery/eureka_test.go diff --git a/discovery/eureka.go b/discovery/eureka.go new file mode 100644 index 0000000..901cf4d --- /dev/null +++ b/discovery/eureka.go @@ -0,0 +1,44 @@ +package discovery + +import ( + "errors" + "net/http" +) + +//TODO: This should become a discovery interface. And Eureka just the first implementation +type EurekaClient struct { + eurekaUrl string +} + +var errNoEurekaConnection = errors.New("Unable to reach Eureka server") +var errEurekaUnexpectedHttpResponseCode = errors.New("Eureka returned a non 200 http response code") + +func NewEurekaClient(eurekaUrl string) (ec EurekaClient, err error) { + ec.eurekaUrl = eurekaUrl + resp, err := http.Get(eurekaUrl) + if err != nil { + return ec, errNoEurekaConnection + } + defer resp.Body.Close() + if resp.StatusCode != 200 { + return ec,errEurekaUnexpectedHttpResponseCode + } + return ec, nil +} + +var errNoIpsFound = errors.New("No IPs associated to the requested App name") + +func (ec EurekaClient) GetIPs(appName string) ([]string, error) { + eurekaAppUrl := ec.eurekaUrl + "/v2/apps/" + appName + resp, err := http.Get(eurekaAppUrl) + if err != nil { + return []string{}, errNoEurekaConnection + } + defer resp.Body.Close() + if resp.StatusCode == 404 { + return []string{},errNoIpsFound + } else if resp.StatusCode != 200 { + return []string{},errEurekaUnexpectedHttpResponseCode + } + return nil, nil +} diff --git a/discovery/eureka_test.go b/discovery/eureka_test.go new file mode 100644 index 0000000..ac9d880 --- /dev/null +++ b/discovery/eureka_test.go @@ -0,0 +1,119 @@ +package discovery + +import ( + "gopkg.in/ory-am/dockertest.v3" + dc "github.com/fsouza/go-dockerclient" + "github.com/jaume-pinyol/fargo" + "log" + "os" + "testing" + "strconv" + "github.com/op/go-logging" +) + +var eurekaTestPort int = 8080 +var eurekaTestUrl string = "http://127.0.0.1:" + strconv.Itoa(eurekaTestPort) + "/eureka" + +func TestMain(m *testing.M) { + // uses a sensible default on windows (tcp/http) and linux/osx (socket) + pool, err := dockertest.NewPool("") + if err != nil { + log.Fatalf("Could not connect to docker: %s", err) + } + // pulls an image, creates a container based on it and runs it + resource, err := pool.RunWithOptions(&dockertest.RunOptions{ + Repository: "netflixoss/eureka", + Tag: "1.3.1", + PortBindings: map[dc.Port][]dc.PortBinding{ + dc.Port(strconv.Itoa(eurekaTestPort) + "/tcp"): {{HostIP: "", HostPort: strconv.Itoa(eurekaTestPort)}}, + }, + }) + if err != nil { + log.Fatalf("Could not start resource: %s", err) + } + + // exponential backoff-retry, because the application in the container might not be ready to accept connections yet + if err := pool.Retry(func() error { + _, err := NewEurekaClient(eurekaTestUrl) + return err + }); err != nil { + log.Fatalf("Could not connect to the docker resource: %s", err) + } + + code := m.Run() + + // You can't defer this because os.Exit doesn't care for defer + if err := pool.Purge(resource); err != nil { + log.Fatalf("Could not purge resource: %s", err) + } + + os.Exit(code) +} + +func TestEurekaClientNoEureka(t *testing.T) { + _, err := NewEurekaClient("http://localhost:9999/thisshouldntwork") + if err != errNoEurekaConnection { + t.Fatal("We shouldnt reach eureka if Eureka hostname/port is completely wrong") + } +} + +func TestEurekaClientWrongEurekaContext(t *testing.T) { + _, err := NewEurekaClient(eurekaTestUrl + "badsuffix") + if err != errEurekaUnexpectedHttpResponseCode { + t.Fatal("Eureka should be reachable but, when asking a wrong URL, it should return a non 200 response code") + } +} + +func TestEurekaClientUnknownApp(t *testing.T) { + appName := "unknown" + eurekaClient, err := NewEurekaClient(eurekaTestUrl) + if err != nil { + t.Fatal("We cannot connect to the specified eureka server:", err) + } + t.Log("Connection to Eureka established") + _, err = eurekaClient.GetIPs(appName) + if err != errNoIpsFound { + t.Fatal("Eureka did return something different from an No-IPs-error associated to the unknown App") + } +} + +func TestEurekaClientValidApp(t *testing.T) { + appName := "testApp" + ipAddr := "192.0.2.1" + port := 10080 + registerDummyAppInTestEureka(appName, ipAddr, port) + eurekaClient, err := NewEurekaClient(eurekaTestUrl) + if err != nil { + t.Fatal("We cannot connect to the specified eureka server:", err) + } + t.Log("Connection to Eureka established") + ipsFromEureka, err := eurekaClient.GetIPs(appName) + if err != nil { + t.Fatal("Eureka returned an error when requesting the IPs:", err) + } + if len(ipsFromEureka) != 1 || ipsFromEureka[0] != ipAddr { + t.Fatal("Eureka returned a set of IPs we did not expect for our service:", ipsFromEureka ) + } + +} +func registerDummyAppInTestEureka(appName string, ipAddr string, port int) { + logging.SetLevel(logging.ERROR, "fargo") + fargoclient := fargo.NewConn(eurekaTestUrl + "/v2") + appInstance := &fargo.Instance{ + HostName: "dummyhost", + Port: port, + SecurePort: port, + App: appName, + IPAddr: ipAddr, + VipAddress: ipAddr, + SecureVipAddress: ipAddr, + DataCenterInfo: fargo.DataCenterInfo{Name: fargo.MyOwn}, + Status: fargo.UP, + Overriddenstatus: fargo.UNKNOWN, + HealthCheckUrl: "http://" + ipAddr + ":" + "8080" + "/healthcheck", + StatusPageUrl: "http://" + ipAddr + ":" + "8080" + "/healthcheck", + HomePageUrl: "http://" + ipAddr + ":" + "8080" + "/", + AsgName: "dummyAsg", + } + fargoclient.RegisterInstance(appInstance) +} From f28c628b8945434207d0cec2c4f5827d8b03a7d7 Mon Sep 17 00:00:00 2001 From: daniel-caballero Date: Tue, 12 Dec 2017 02:20:27 +0100 Subject: [PATCH 2/4] eureka timeouts management added --- discovery/eureka.go | 25 +++++++++++++++++++++---- discovery/eureka_test.go | 9 ++++++++- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/discovery/eureka.go b/discovery/eureka.go index 901cf4d..e5180e9 100644 --- a/discovery/eureka.go +++ b/discovery/eureka.go @@ -3,6 +3,10 @@ package discovery import ( "errors" "net/http" + "io/ioutil" + "fmt" + "time" + "net" ) //TODO: This should become a discovery interface. And Eureka just the first implementation @@ -10,13 +14,20 @@ type EurekaClient struct { eurekaUrl string } +//TODO: Creating our own error type and wrapping standard net/http errors could be useful to prevent +// the original errors from being lost +var errEurekaTimesOut = errors.New("Eureka server timed out") var errNoEurekaConnection = errors.New("Unable to reach Eureka server") var errEurekaUnexpectedHttpResponseCode = errors.New("Eureka returned a non 200 http response code") +const eurekaClientTimeoutInSeconds = 10 func NewEurekaClient(eurekaUrl string) (ec EurekaClient, err error) { ec.eurekaUrl = eurekaUrl - resp, err := http.Get(eurekaUrl) - if err != nil { + httpclient := http.Client{Timeout: time.Second * eurekaClientTimeoutInSeconds} + resp, err := httpclient.Get(eurekaUrl) + if serr, ok := err.(net.Error); ok && serr.Timeout() { + return ec,errEurekaTimesOut + } else if err != nil { return ec, errNoEurekaConnection } defer resp.Body.Close() @@ -30,8 +41,12 @@ var errNoIpsFound = errors.New("No IPs associated to the requested App name") func (ec EurekaClient) GetIPs(appName string) ([]string, error) { eurekaAppUrl := ec.eurekaUrl + "/v2/apps/" + appName - resp, err := http.Get(eurekaAppUrl) - if err != nil { + //resp, err := http.Get(eurekaAppUrl, "application/json; charset=utf-8") + httpclient := http.Client{Timeout: time.Second * eurekaClientTimeoutInSeconds} + resp, err := httpclient.Get(eurekaAppUrl) + if serr, ok := err.(net.Error); ok && serr.Timeout() { + return []string{},errEurekaTimesOut + } else if err != nil { return []string{}, errNoEurekaConnection } defer resp.Body.Close() @@ -40,5 +55,7 @@ func (ec EurekaClient) GetIPs(appName string) ([]string, error) { } else if resp.StatusCode != 200 { return []string{},errEurekaUnexpectedHttpResponseCode } + body, err := ioutil.ReadAll(resp.Body) + fmt.Println(string(body)) return nil, nil } diff --git a/discovery/eureka_test.go b/discovery/eureka_test.go index ac9d880..68d936e 100644 --- a/discovery/eureka_test.go +++ b/discovery/eureka_test.go @@ -53,7 +53,14 @@ func TestMain(m *testing.M) { func TestEurekaClientNoEureka(t *testing.T) { _, err := NewEurekaClient("http://localhost:9999/thisshouldntwork") if err != errNoEurekaConnection { - t.Fatal("We shouldnt reach eureka if Eureka hostname/port is completely wrong") + t.Fatal("We shouldnt reach eureka if Eureka hostname/port is completely wrong. Actual err:", err) + } +} + +func TestEurekaClientEurekaDoesNotReply(t *testing.T) { + _, err := NewEurekaClient("http://192.0.2.1:9999/thisshouldtimeout") + if err != errEurekaTimesOut { + t.Fatal("Pointing to a destination that drops packages should fail because of timeout. Actual err:", err) } } From 10c795857a19273dbd8bbd71149a5bc0a84e1065 Mon Sep 17 00:00:00 2001 From: daniel-caballero Date: Tue, 12 Dec 2017 03:13:49 +0100 Subject: [PATCH 3/4] not able to get the json version of eureka. Tested against 1.6, and does not fix the issue. To follow up --- discovery/eureka.go | 13 ++++++++++--- discovery/eureka_test.go | 8 +++++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/discovery/eureka.go b/discovery/eureka.go index e5180e9..a5af56a 100644 --- a/discovery/eureka.go +++ b/discovery/eureka.go @@ -39,11 +39,17 @@ func NewEurekaClient(eurekaUrl string) (ec EurekaClient, err error) { var errNoIpsFound = errors.New("No IPs associated to the requested App name") +// TODO: Probably we should break this function func (ec EurekaClient) GetIPs(appName string) ([]string, error) { eurekaAppUrl := ec.eurekaUrl + "/v2/apps/" + appName - //resp, err := http.Get(eurekaAppUrl, "application/json; charset=utf-8") + // can we reuse the client but starting from 0 in terms of timeout? to review httpclient := http.Client{Timeout: time.Second * eurekaClientTimeoutInSeconds} - resp, err := httpclient.Get(eurekaAppUrl) + req, err := http.NewRequest("GET", eurekaAppUrl, nil) + if err != nil { + return nil, err + } + req.Header.Set("Content-Type", "application/json") + resp, err := httpclient.Do(req) if serr, ok := err.(net.Error); ok && serr.Timeout() { return []string{},errEurekaTimesOut } else if err != nil { @@ -56,6 +62,7 @@ func (ec EurekaClient) GetIPs(appName string) ([]string, error) { return []string{},errEurekaUnexpectedHttpResponseCode } body, err := ioutil.ReadAll(resp.Body) - fmt.Println(string(body)) + fmt.Println("Response from eureka:", string(body)) + // parsing to get the right data will be done like this: https://stackoverflow.com/a/35665161 return nil, nil } diff --git a/discovery/eureka_test.go b/discovery/eureka_test.go index 68d936e..41872b2 100644 --- a/discovery/eureka_test.go +++ b/discovery/eureka_test.go @@ -24,8 +24,10 @@ func TestMain(m *testing.M) { resource, err := pool.RunWithOptions(&dockertest.RunOptions{ Repository: "netflixoss/eureka", Tag: "1.3.1", - PortBindings: map[dc.Port][]dc.PortBinding{ - dc.Port(strconv.Itoa(eurekaTestPort) + "/tcp"): {{HostIP: "", HostPort: strconv.Itoa(eurekaTestPort)}}, + //Repository: "containers.schibsted.io/spt-infrastructure/eureka-docker", + //Tag: "latest", + PortBindings: map[dc.Port][]dc.PortBinding{ + dc.Port(strconv.Itoa(eurekaTestPort) + "/tcp"): {{HostIP: "", HostPort: strconv.Itoa(eurekaTestPort)}}, }, }) if err != nil { @@ -99,7 +101,7 @@ func TestEurekaClientValidApp(t *testing.T) { t.Fatal("Eureka returned an error when requesting the IPs:", err) } if len(ipsFromEureka) != 1 || ipsFromEureka[0] != ipAddr { - t.Fatal("Eureka returned a set of IPs we did not expect for our service:", ipsFromEureka ) + t.Fatal("Eureka returned a set of IPs we did not expect for our service:", ipsFromEureka) } } From 48a18320b1d123f5fdcb1fd1991f0dde57a51a30 Mon Sep 17 00:00:00 2001 From: daniel-caballero Date: Fri, 15 Dec 2017 00:12:15 +0100 Subject: [PATCH 4/4] feedback from PR --- discovery/eureka.go | 18 +++++++++--------- discovery/eureka_test.go | 20 ++++++++++---------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/discovery/eureka.go b/discovery/eureka.go index a5af56a..31d64cb 100644 --- a/discovery/eureka.go +++ b/discovery/eureka.go @@ -11,20 +11,20 @@ import ( //TODO: This should become a discovery interface. And Eureka just the first implementation type EurekaClient struct { - eurekaUrl string + eurekaURL string } //TODO: Creating our own error type and wrapping standard net/http errors could be useful to prevent // the original errors from being lost var errEurekaTimesOut = errors.New("Eureka server timed out") var errNoEurekaConnection = errors.New("Unable to reach Eureka server") -var errEurekaUnexpectedHttpResponseCode = errors.New("Eureka returned a non 200 http response code") +var errEurekaUnexpectedHTTPResponseCode = errors.New("Eureka returned a non 200 http response code") const eurekaClientTimeoutInSeconds = 10 -func NewEurekaClient(eurekaUrl string) (ec EurekaClient, err error) { - ec.eurekaUrl = eurekaUrl +func NewEurekaClient(eurekaURL string) (ec EurekaClient, err error) { + ec.eurekaURL = eurekaURL httpclient := http.Client{Timeout: time.Second * eurekaClientTimeoutInSeconds} - resp, err := httpclient.Get(eurekaUrl) + resp, err := httpclient.Get(eurekaURL) if serr, ok := err.(net.Error); ok && serr.Timeout() { return ec,errEurekaTimesOut } else if err != nil { @@ -32,7 +32,7 @@ func NewEurekaClient(eurekaUrl string) (ec EurekaClient, err error) { } defer resp.Body.Close() if resp.StatusCode != 200 { - return ec,errEurekaUnexpectedHttpResponseCode + return ec, errEurekaUnexpectedHTTPResponseCode } return ec, nil } @@ -41,10 +41,10 @@ var errNoIpsFound = errors.New("No IPs associated to the requested App name") // TODO: Probably we should break this function func (ec EurekaClient) GetIPs(appName string) ([]string, error) { - eurekaAppUrl := ec.eurekaUrl + "/v2/apps/" + appName + eurekaAppURL := ec.eurekaURL + "/v2/apps/" + appName // can we reuse the client but starting from 0 in terms of timeout? to review httpclient := http.Client{Timeout: time.Second * eurekaClientTimeoutInSeconds} - req, err := http.NewRequest("GET", eurekaAppUrl, nil) + req, err := http.NewRequest("GET", eurekaAppURL, nil) if err != nil { return nil, err } @@ -59,7 +59,7 @@ func (ec EurekaClient) GetIPs(appName string) ([]string, error) { if resp.StatusCode == 404 { return []string{},errNoIpsFound } else if resp.StatusCode != 200 { - return []string{},errEurekaUnexpectedHttpResponseCode + return []string{}, errEurekaUnexpectedHTTPResponseCode } body, err := ioutil.ReadAll(resp.Body) fmt.Println("Response from eureka:", string(body)) diff --git a/discovery/eureka_test.go b/discovery/eureka_test.go index 41872b2..cf5655b 100644 --- a/discovery/eureka_test.go +++ b/discovery/eureka_test.go @@ -11,8 +11,8 @@ import ( "github.com/op/go-logging" ) -var eurekaTestPort int = 8080 -var eurekaTestUrl string = "http://127.0.0.1:" + strconv.Itoa(eurekaTestPort) + "/eureka" +var eurekaTestPort = 8080 +var eurekaTestURL string = "http://127.0.0.1:" + strconv.Itoa(eurekaTestPort) + "/eureka" func TestMain(m *testing.M) { // uses a sensible default on windows (tcp/http) and linux/osx (socket) @@ -21,7 +21,7 @@ func TestMain(m *testing.M) { log.Fatalf("Could not connect to docker: %s", err) } // pulls an image, creates a container based on it and runs it - resource, err := pool.RunWithOptions(&dockertest.RunOptions{ + eurekaContainer, err := pool.RunWithOptions(&dockertest.RunOptions{ Repository: "netflixoss/eureka", Tag: "1.3.1", //Repository: "containers.schibsted.io/spt-infrastructure/eureka-docker", @@ -36,7 +36,7 @@ func TestMain(m *testing.M) { // exponential backoff-retry, because the application in the container might not be ready to accept connections yet if err := pool.Retry(func() error { - _, err := NewEurekaClient(eurekaTestUrl) + _, err := NewEurekaClient(eurekaTestURL) return err }); err != nil { log.Fatalf("Could not connect to the docker resource: %s", err) @@ -45,7 +45,7 @@ func TestMain(m *testing.M) { code := m.Run() // You can't defer this because os.Exit doesn't care for defer - if err := pool.Purge(resource); err != nil { + if err := pool.Purge(eurekaContainer); err != nil { log.Fatalf("Could not purge resource: %s", err) } @@ -67,15 +67,15 @@ func TestEurekaClientEurekaDoesNotReply(t *testing.T) { } func TestEurekaClientWrongEurekaContext(t *testing.T) { - _, err := NewEurekaClient(eurekaTestUrl + "badsuffix") - if err != errEurekaUnexpectedHttpResponseCode { + _, err := NewEurekaClient(eurekaTestURL + "badsuffix") + if err != errEurekaUnexpectedHTTPResponseCode { t.Fatal("Eureka should be reachable but, when asking a wrong URL, it should return a non 200 response code") } } func TestEurekaClientUnknownApp(t *testing.T) { appName := "unknown" - eurekaClient, err := NewEurekaClient(eurekaTestUrl) + eurekaClient, err := NewEurekaClient(eurekaTestURL) if err != nil { t.Fatal("We cannot connect to the specified eureka server:", err) } @@ -91,7 +91,7 @@ func TestEurekaClientValidApp(t *testing.T) { ipAddr := "192.0.2.1" port := 10080 registerDummyAppInTestEureka(appName, ipAddr, port) - eurekaClient, err := NewEurekaClient(eurekaTestUrl) + eurekaClient, err := NewEurekaClient(eurekaTestURL) if err != nil { t.Fatal("We cannot connect to the specified eureka server:", err) } @@ -107,7 +107,7 @@ func TestEurekaClientValidApp(t *testing.T) { } func registerDummyAppInTestEureka(appName string, ipAddr string, port int) { logging.SetLevel(logging.ERROR, "fargo") - fargoclient := fargo.NewConn(eurekaTestUrl + "/v2") + fargoclient := fargo.NewConn(eurekaTestURL + "/v2") appInstance := &fargo.Instance{ HostName: "dummyhost", Port: port,