[FAB-3013] Reuse connections in the client

Load tests were failing with EOF error from http.Client.
This is because lib.Client contructed transport and
http.Client for every request. By default transport connection
pools are maintained in the transport and are reused. This
was not happening because lib.Client contructed new transport
for each request. This change set fixes this problem by
associating a http.Client object with lib.Client and reusing
it for all requests. This will use one transport for all the
requests there by reusing the connections pooled by the
transport.

Change-Id: I0a12946002ffc653237e0186dbc452684fc56a8a
Signed-off-by: Anil Ambati <[email protected]>

lib/client.go

@@ -53,6 +53,8 @@ type Client struct {
 	keyFile, certFile, caCertsDir string
 	// The crypto service provider (BCCSP)
 	csp bccsp.BCCSP
+	// HTTP client associated with this Fabric CA client
+	httpClient *http.Client
 }
 
 // Init initializes the client
@@ -93,12 +95,38 @@ func (c *Client) Init() error {
 		if err != nil {
 			return err
 		}
+		// Create http.Client object and associate it with this client
+		err = c.initHTTPClient()
+		if err != nil {
+			return err
+		}
+
 		// Successfully initialized the client
 		c.initialized = true
 	}
 	return nil
 }
 
+func (c *Client) initHTTPClient() error {
+	tr := new(http.Transport)
+	if c.Config.TLS.Enabled {
+		log.Info("TLS Enabled")
+
+		err := tls.AbsTLSClient(&c.Config.TLS, c.HomeDir)
+		if err != nil {
+			return err
+		}
+
+		tlsConfig, err2 := tls.GetClientTLSConfig(&c.Config.TLS, c.csp)
+		if err2 != nil {
+			return fmt.Errorf("Failed to get client TLS config: %s", err2)
+		}
+		tr.TLSClientConfig = tlsConfig
+	}
+	c.httpClient = &http.Client{Transport: tr}
+	return nil
+}
+
 // GetServerInfoResponse is the response from the GetServerInfo call
 type GetServerInfoResponse struct {
 	// CAName is the name of the CA
@@ -391,33 +419,19 @@ func (c *Client) SendReq(req *http.Request, result interface{}) (err error) {
 		return err
 	}
 
-	var tr = new(http.Transport)
-
-	if c.Config.TLS.Enabled {
-		log.Info("TLS Enabled")
-
-		err = tls.AbsTLSClient(&c.Config.TLS, c.HomeDir)
-		if err != nil {
-			return err
-		}
-
-		tlsConfig, err2 := tls.GetClientTLSConfig(&c.Config.TLS, c.csp)
-		if err2 != nil {
-			return errors.WithMessage(err2, "Failed to get client TLS config")
-		}
-
-		tr.TLSClientConfig = tlsConfig
-	}
-
-	httpClient := &http.Client{Transport: tr}
-	resp, err := httpClient.Do(req)
+	resp, err := c.httpClient.Do(req)
 	if err != nil {
 		return errors.Wrapf(err, "POST failure of request: %s", reqStr)
 	}
 	var respBody []byte
 	if resp.Body != nil {
 		respBody, err = ioutil.ReadAll(resp.Body)
-		defer resp.Body.Close()
+		defer func() {
+			err := resp.Body.Close()
+			if err != nil {
+				log.Debugf("Failed to close the response body: %s", err.Error())
+			}
+		}()
 		if err != nil {
 			return errors.Wrapf(err, "Failed to read response of request: %s", reqStr)
 		}

lib/client_whitebox_test.go

@@ -21,6 +21,7 @@ import (
 	"encoding/pem"
 	"fmt"
 	"io/ioutil"
+	"net/http"
 	"os"
 	"path"
 	"testing"
@@ -88,7 +89,9 @@ func TestCWBTLSClientAuth(t *testing.T) {
 
 	// Enroll over HTTP
 	client := &Client{
-		Config:  &ClientConfig{URL: fmt.Sprintf("http://localhost:%d", whitePort)},
+		Config: &ClientConfig{
+			URL: fmt.Sprintf("http://localhost:%d", whitePort),
+		},
 		HomeDir: path.Join(testTLSClientAuthDir, "client"),
 	}
 
@@ -122,6 +125,7 @@ func TestCWBTLSClientAuth(t *testing.T) {
 	testMasqueradeReenroll(t, client, id2)
 
 	// Stop server
+	log.Debug("Stopping the server")
 	err = server.Stop()
 	if err != nil {
 		t.Fatalf("Failed to stop server: %s", err)
@@ -131,22 +135,33 @@ func TestCWBTLSClientAuth(t *testing.T) {
 	// 2) Test over HTTPS with client auth disabled
 	//
 	// Start server
+	log.Debug("Starting the server with TLS")
 	server.Config.TLS.Enabled = true
 	server.Config.TLS.CertFile = "ca-cert.pem"
 	err = server.Start()
 	if err != nil {
 		t.Fatalf("Failed to start server with HTTPS: %s", err)
 	}
+
+	// Close the idle connections that were established to the non-SSL
+	// server. client will create new connection for the next request
+	// There is no need to do this in real scenario where the Fabric CA
+	// server's transport can only be changed from ssl to non-ssl or vice-versa
+	// by restarting the server, in which case connections in the client's
+	// connection pool are invalidated and it is forced to create new connection.
+	client.httpClient.Transport.(*http.Transport).CloseIdleConnections()
+
 	// Try to reenroll over HTTP and it should fail because server is listening on HTTPS
 	_, err = id.Reenroll(&api.ReenrollmentRequest{})
-	t.Logf("id.Reenroll: %v", err)
 	if err == nil {
 		t.Error("Client HTTP should have failed to reenroll with server HTTPS")
 	}
-	// Reenroll over HTTPS
+
 	client.Config.URL = fmt.Sprintf("https://localhost:%d", whitePort)
 	client.Config.TLS.Enabled = true
 	client.Config.TLS.CertFiles = []string{"../server/ca-cert.pem"}
+	// Reinialize the http client with updated config and re-enroll over HTTPS
+	err = client.initHTTPClient()
 	resp, err := id.Reenroll(&api.ReenrollmentRequest{})
 	if err != nil {
 		server.Stop()
@@ -174,14 +189,18 @@ func TestCWBTLSClientAuth(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Failed to start server with HTTPS and client auth: %s", err)
 	}
+	// Close all idle connections
+	client.httpClient.Transport.(*http.Transport).CloseIdleConnections()
+
 	// Try to reenroll and it should fail because client has no client cert
 	_, err = id.Reenroll(&api.ReenrollmentRequest{})
-	t.Logf("id.Reenroll: %v", err)
 	if err == nil {
 		t.Error("Client reenroll without client cert should have failed")
 	}
-	// Reenroll over HTTPS with client auth
+
 	client.Config.TLS.Client.CertFile = path.Join("msp", "signcerts", "cert.pem")
+	// Reinialize the http client with updated config and re-enroll over HTTPS with client auth
+	err = client.initHTTPClient()
 	_, err = id.Reenroll(&api.ReenrollmentRequest{})
 	if err != nil {
 		server.Stop()

lib/identity.go

@@ -129,7 +129,7 @@ func (i *Identity) RegisterAndEnroll(req *api.RegistrationRequest) (*Identity, e
 // Reenroll reenrolls an existing Identity and returns a new Identity
 // @param req The reenrollment request
 func (i *Identity) Reenroll(req *api.ReenrollmentRequest) (*EnrollmentResponse, error) {
-	log.Debugf("Reenrolling %s", req)
+	log.Debugf("Reenrolling %s", util.StructToString(req))
 
 	csrPEM, key, err := i.client.GenCSR(req.CSR, i.GetName())
 	if err != nil {

util/util_test.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"net/http"
 	"os"
 	"path/filepath"
 	"strings"
@@ -688,3 +689,22 @@ func testIsSubsetOf(t *testing.T, small, large string, expectToPass bool) {
 		}
 	}
 }
+
+func TestHostname(t *testing.T) {
+	host := Hostname()
+	assert.NotEqual(t, "", host, "Hostname should not be empty")
+}
+
+func TestHTTPRequestToString(t *testing.T) {
+	url := "http://localhost:7054"
+	reqBody := "Hello"
+	req, err := http.NewRequest("POST", url, strings.NewReader(reqBody))
+	if err != nil {
+		t.Errorf("Failed to create a request: %s", err)
+	} else {
+		reqStr := HTTPRequestToString(req)
+		assert.Contains(t, reqStr, url)
+		assert.Contains(t, reqStr, "POST")
+		assert.Contains(t, reqStr, reqBody)
+	}
+}