client: enable HTTP(S) keep-alive

Enable HTTP(S) keep-alive to improve network performance and reduce
latency.  We need several HTTP(S) requests before we get to request the
blob and each of them requires a new HTTP(S) connection and that slows
down significantly pulling images, especially on networks with a
higher latency (e.g. wifi).
This will allow multiple requests to be sent over a single connection,
reducing the overhead of establishing new connections for each
request.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

docker/daemon/client_test.go

@@ -19,6 +19,8 @@ func TestDockerClientFromNilSystemContext(t *testing.T) {
 
 	assert.Equal(t, dockerclient.DefaultDockerHost, client.DaemonHost(), "The default docker host should have been used")
 	assert.Equal(t, defaultAPIVersion, client.ClientVersion(), "The default api version should have been used")
+
+	assert.NoError(t, client.Close())
 }
 
 func TestDockerClientFromCertContext(t *testing.T) {
@@ -38,6 +40,8 @@ func TestDockerClientFromCertContext(t *testing.T) {
 
 	assert.Equal(t, host, client.DaemonHost())
 	assert.Equal(t, "1.22", client.ClientVersion())
+
+	assert.NoError(t, client.Close())
 }
 
 func TestTlsConfigFromInvalidCertPath(t *testing.T) {
@@ -94,6 +98,7 @@ func TestSpecifyPlainHTTPViaHostScheme(t *testing.T) {
 	assert.NotNil(t, client, "A Docker client reference should have been returned")
 
 	assert.Equal(t, host, client.DaemonHost())
+	assert.NoError(t, client.Close())
 }
 
 func testDir(t *testing.T) string {

docker/daemon/daemon_dest.go

@@ -69,6 +69,7 @@ func newImageDestination(ctx context.Context, sys *types.SystemContext, ref daem
 
 // imageLoadGoroutine accepts tar stream on reader, sends it to c, and reports error or success by writing to statusChannel
 func imageLoadGoroutine(ctx context.Context, c *client.Client, reader *io.PipeReader, statusChannel chan<- error) {
+	defer c.Close()
 	err := errors.New("Internal error: unexpected panic in imageLoadGoroutine")
 	defer func() {
 		logrus.Debugf("docker-daemon: sending done, status %v", err)

docker/daemon/daemon_src.go

@@ -28,6 +28,8 @@ func newImageSource(ctx context.Context, sys *types.SystemContext, ref daemonRef
 	if err != nil {
 		return nil, fmt.Errorf("initializing docker engine client: %w", err)
 	}
+	defer c.Close()
+
 	// Per NewReference(), ref.StringWithinTransport() is either an image ID (config digest), or a !reference.NameOnly() reference.
 	// Either way ImageSave should create a tarball with exactly one image.
 	inputStream, err := c.ImageSave(ctx, []string{ref.StringWithinTransport()})

docker/docker_client.go

@@ -213,6 +213,7 @@ func dockerCertDir(sys *types.SystemContext, hostPort string) (string, error) {
 // newDockerClientFromRef returns a new dockerClient instance for refHostname (a host a specified in the Docker image reference, not canonicalized to dockerRegistry)
 // “write” specifies whether the client will be used for "write" access (in particular passed to lookaside.go:toplevelFromSection)
 // signatureBase is always set in the return value
+// The caller must call .Close() on the returned client when done.
 func newDockerClientFromRef(sys *types.SystemContext, ref dockerReference, registryConfig *registryConfiguration, write bool, actions string) (*dockerClient, error) {
 	auth, err := config.GetCredentialsForRef(sys, ref.ref)
 	if err != nil {
@@ -247,6 +248,7 @@ func newDockerClientFromRef(sys *types.SystemContext, ref dockerReference, regis
 // (e.g., "registry.com[:5000][/some/namespace]/repo").
 // Please note that newDockerClient does not set all members of dockerClient
 // (e.g., username and password); those must be set by callers if necessary.
+// The caller must call .Close() on the returned client when done.
 func newDockerClient(sys *types.SystemContext, registry, reference string) (*dockerClient, error) {
 	hostName := registry
 	if registry == dockerHostname {
@@ -302,6 +304,7 @@ func CheckAuth(ctx context.Context, sys *types.SystemContext, username, password
 	if err != nil {
 		return fmt.Errorf("creating new docker client: %w", err)
 	}
+	defer client.Close()
 	client.auth = types.DockerAuthConfig{
 		Username: username,
 		Password: password,
@@ -371,6 +374,7 @@ func SearchRegistry(ctx context.Context, sys *types.SystemContext, registry, ima
 	if err != nil {
 		return nil, fmt.Errorf("creating new docker client: %w", err)
 	}
+	defer client.Close()
 	client.auth = auth
 	if sys != nil {
 		client.registryToken = sys.DockerBearerRegistryToken
@@ -1084,3 +1088,9 @@ func (c *dockerClient) getExtensionsSignatures(ctx context.Context, ref dockerRe
 func sigstoreAttachmentTag(d digest.Digest) string {
 	return strings.Replace(d.String(), ":", "-", 1) + ".sig"
 }
+
+// Close removes resources associated with an initialized dockerClient, if any.
+func (c *dockerClient) Close() error {
+	c.client.CloseIdleConnections()
+	return nil
+}

docker/docker_image.go

@@ -68,6 +68,7 @@ func GetRepositoryTags(ctx context.Context, sys *types.SystemContext, ref types.
 	if err != nil {
 		return nil, fmt.Errorf("failed to create client: %w", err)
 	}
+	defer client.Close()
 
 	tags := make([]string, 0)
 
@@ -136,6 +137,7 @@ func GetDigest(ctx context.Context, sys *types.SystemContext, ref types.ImageRef
 	if err != nil {
 		return "", fmt.Errorf("failed to create client: %w", err)
 	}
+	defer client.Close()
 
 	path := fmt.Sprintf(manifestPath, reference.Path(dr.ref), tagOrDigest)
 	headers := map[string][]string{

docker/docker_image_dest.go

@@ -93,7 +93,7 @@ func (d *dockerImageDestination) Reference() types.ImageReference {
 
 // Close removes resources associated with an initialized ImageDestination, if any.
 func (d *dockerImageDestination) Close() error {
-	return nil
+	return d.c.Close()
 }
 
 // SupportsSignatures returns an error (to be displayed to the user) if the destination certainly can't store signatures.

docker/docker_image_src.go

@@ -153,6 +153,7 @@ func newImageSourceAttempt(ctx context.Context, sys *types.SystemContext, logica
 	s.Compat = impl.AddCompat(s)
 
 	if err := s.ensureManifestIsLoaded(ctx); err != nil {
+		client.Close()
 		return nil, err
 	}
 	return s, nil
@@ -166,7 +167,7 @@ func (s *dockerImageSource) Reference() types.ImageReference {
 
 // Close removes resources associated with an initialized ImageSource, if any.
 func (s *dockerImageSource) Close() error {
-	return nil
+	return s.c.Close()
 }
 
 // simplifyContentType drops parameters from a HTTP media type (see https://tools.ietf.org/html/rfc7231#section-3.1.1.1)
@@ -605,6 +606,7 @@ func deleteImage(ctx context.Context, sys *types.SystemContext, ref dockerRefere
 	if err != nil {
 		return err
 	}
+	defer c.Close()
 
 	headers := map[string][]string{
 		"Accept": manifest.DefaultRequestedManifestMIMETypes,

oci/layout/oci_src.go

@@ -94,6 +94,7 @@ func (s *ociImageSource) Reference() types.ImageReference {
 
 // Close removes resources associated with an initialized ImageSource, if any.
 func (s *ociImageSource) Close() error {
+	s.client.CloseIdleConnections()
 	return nil
 }
 

pkg/tlsclientconfig/tlsclientconfig.go

@@ -96,8 +96,8 @@ func NewTransport() *http.Transport {
 		Proxy:               http.ProxyFromEnvironment,
 		DialContext:         direct.DialContext,
 		TLSHandshakeTimeout: 10 * time.Second,
-		// TODO(dmcgowan): Call close idle connections when complete and use keep alive
-		DisableKeepAlives: true,
+		IdleConnTimeout:     90 * time.Second,
+		MaxIdleConns:        100,
 	}
 	return tr
 }