Remove input client in http probe, use internal instead. (#76)

Remove input client in http probe, use internal instead. (#76)

Author: humingcheng

Diff for: addresspool/pool.go

@@ -2,6 +2,7 @@ package addresspool
 
 import (
 	"context"
+	"crypto/tls"
 	"fmt"
 	"io"
 	"net"
@@ -35,7 +36,6 @@ const (
 )
 
 type HttpProbeOptions struct {
-	Client   *httpclient.Requests
 	Protocol string
 	Path     string
 }
@@ -53,11 +53,13 @@ type Pool struct {
 	sameAzAddress []string
 	diffAzAddress []string
 
-	status           map[string]string
-	onceMonitor      sync.Once
-	quit             chan struct{}
-	onceQuit         sync.Once
+	status      map[string]string
+	onceMonitor sync.Once
+	quit        chan struct{}
+	onceQuit    sync.Once
+
 	httpProbeOptions *HttpProbeOptions
+	httpProbeClient  *httpclient.Requests
 	statusHistory    []map[string]string
 }
 
@@ -76,11 +78,15 @@ func NewPool(addresses []string, opts ...Options) *Pool {
 	}
 
 	if len(opts) > 0 && opts[0].HttpProbeOptions != nil {
-		p.httpProbeOptions = opts[0].HttpProbeOptions
-		if p.httpProbeOptions.Client == nil {
-			openlog.Info(fmt.Sprintf("http client nil, make one with default options"))
-			p.httpProbeOptions.Client, _ = httpclient.New(nil)
+		optCopy := *(opts[0].HttpProbeOptions)
+		p.httpProbeOptions = &optCopy
+		if len(p.httpProbeOptions.Protocol) == 0 {
+			p.httpProbeOptions.Protocol = "http"
 		}
+		p.httpProbeClient, _ = httpclient.New(&httpclient.Options{
+			TLSConfig:      &tls.Config{InsecureSkipVerify: true},
+			RequestTimeout: 5 * time.Second,
+		})
 	}
 	p.monitor()
 	return p
@@ -268,7 +274,7 @@ func (p *Pool) doCheckConnectivityWithTcp(endpoint string) error {
 
 func (p *Pool) doCheckConnectivityWithHttp(endpoint string) error {
 	u := p.httpProbeOptions.Protocol + "://" + endpoint + p.httpProbeOptions.Path
-	resp, err := p.httpProbeOptions.Client.Get(context.Background(), u, nil)
+	resp, err := p.httpProbeClient.Get(context.Background(), u, nil)
 	if err != nil {
 		return err
 	}

Diff for: addresspool/pool_test.go

@@ -4,6 +4,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"reflect"
 	"sync"
 	"testing"
 	"time"
@@ -33,6 +34,13 @@ func TestNewPool(t *testing.T) {
 	mockHttpServer.Close()
 	time.Sleep(2*time.Second + 100*time.Millisecond)
 	assert.NotEqual(t, statusAvailable, pool.status[defaultAddr]) // the status should be unavailable again
+
+	httpProbeOpt := &HttpProbeOptions{
+		Protocol: "http",
+	}
+	pool = NewPool([]string{defaultAddr}, Options{HttpProbeOptions: httpProbeOpt})
+	assert.False(t, httpProbeOpt == pool.httpProbeOptions) // not equal but deep equal, as copied
+	assert.True(t, reflect.DeepEqual(httpProbeOpt, pool.httpProbeOptions))
 }
 
 func TestAddressPool_GetAvailableAddress_priority(t *testing.T) {
@@ -328,3 +336,62 @@ func TestPool_CheckReadiness(t *testing.T) {
 		})
 	}
 }
+
+func TestPool_doCheckConnectivity(t *testing.T) {
+	server1HttpCalled := false
+	server1 := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
+		server1HttpCalled = true
+		return
+	}))
+	server1Addr := server1.Listener.Addr().String()
+
+	// http probe is empty，use tcp probe
+	p := NewPool([]string{server1Addr})
+	assert.NoError(t, p.doCheckConnectivity(server1Addr))
+	assert.False(t, server1HttpCalled)
+
+	// http probe is not empty
+	p = NewPool([]string{server1Addr}, Options{HttpProbeOptions: &HttpProbeOptions{
+		Protocol: "http",
+		Path:     "/",
+	}})
+	assert.NoError(t, p.doCheckConnectivity(server1Addr))
+	assert.True(t, server1HttpCalled)
+	server1.Close()
+	assert.Error(t, p.doCheckConnectivity(server1Addr))
+
+	// http probe got 404，tcp probe again
+	server1HttpCalled = false
+	mux := http.NewServeMux()
+	mux.HandleFunc("/test", func(writer http.ResponseWriter, request *http.Request) {
+		server1HttpCalled = true
+		return
+	})
+	server1 = httptest.NewServer(mux)
+	server1Addr = server1.Listener.Addr().String()
+	p = NewPool([]string{server1Addr}, Options{HttpProbeOptions: &HttpProbeOptions{
+		Protocol: "http",
+		Path:     "/", // wrong path, got 404
+	}})
+	assert.NoError(t, p.doCheckConnectivity(server1Addr))
+	assert.False(t, server1HttpCalled)
+	p.httpProbeOptions.Path = "/test" // right path
+	assert.NoError(t, p.doCheckConnectivity(server1Addr))
+	assert.True(t, server1HttpCalled)
+	server1.Close()
+
+	// https probe
+	server1HttpCalled = false
+	server1 = httptest.NewTLSServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
+		server1HttpCalled = true
+		return
+	}))
+	server1Addr = server1.Listener.Addr().String()
+	p = NewPool([]string{server1Addr}, Options{HttpProbeOptions: &HttpProbeOptions{
+		Protocol: "https",
+		Path:     "/", // wrong path, got 404
+	}})
+	assert.NoError(t, p.doCheckConnectivity(server1Addr))
+	assert.True(t, server1HttpCalled)
+	server1.Close()
+}