DOCKER-983 'docker --tls pull nope.example.com/nope' is way too slow

Reviewed by: Todd Whiteman <todd.whiteman@joyent.com>
Approved by: Todd Whiteman <todd.whiteman@joyent.com>

CHANGES.md

@@ -2,6 +2,13 @@
 
 ## not yet released
 
+(nothing yet)
+
+## 3.2.5
+
+- DOCKER-983 Change v1 registry session setup to *not* do retries. This allows
+  failing fast when, for example, the registry endpoint URL is bogus.
+
 ## 3.2.4
 
 - DOCKER-959 Unable to pull from registry when registry response sends multiple

lib/registry-client-v1.js

@@ -471,7 +471,7 @@ RegistryClientV1.prototype._ensureSession = function _ensureSession(cb) {
     }
 
     this.log.trace('get session token/cookie');
-    this.listRepoImgs(function (err, repoImgs, res) {
+    this.listRepoImgs({retry: false}, function (err, repoImgs, res) {
         if (err) {
             return cb(err);
         }
@@ -754,20 +754,35 @@ RegistryClientV1.prototype.search = function search(opts, cb) {
  * Note: This same endpoint is typically used to get a index.docker.io
  * registry auth token or cookie, and an endpoint URL. See `_ensureToken` for
  * details.
+ *
+ * @param {Object} opts - Optional.
+ *      - retry - Optional. Pass through the restify-client 'retry' option.
+ *        For example, pass `retry: false` to disable retries on request
+ *        failure. By default there are 4 retries.
+ * @param {Function} cb - Required. `function (err, repoImgs, res)`
  */
-RegistryClientV1.prototype.listRepoImgs = function listRepoImgs(cb) {
+RegistryClientV1.prototype.listRepoImgs = function listRepoImgs(opts, cb) {
     var self = this;
+    if (typeof (opts) === 'function') {
+        cb = opts;
+        opts = {};
+    }
+    assert.object(opts, 'opts');
     assert.func(cb, 'cb');
 
-    self._request({
+    var reqOpts = {
         client: this._indexApi,
         path: fmt('/v1/repositories/%s/images',
             encodeURI(this.repo.remoteName)),
         headers: {
             'X-Docker-Token': 'true'
         },
         followRedirect: true
-    }, function _afterListRepoImgs(err, req, res, repoImgs) {
+    };
+    if (opts.hasOwnProperty('retry')) {
+        reqOpts.retry = opts.retry;
+    }
+    self._request(reqOpts, function _after(err, req, res, repoImgs) {
         if (err) {
             cb(_sanitizeErr(err, res, res,
                 self.repo.localName + ' v1 repo not found'));
@@ -812,6 +827,10 @@ RegistryClientV1.prototype.listRepoTags = function listRepoTags(cb) {
 /**
  * // JSSTYLED
  * <https://docs.docker.com/reference/api/registry_api/#get-image-id-for-a-particular-tag>
+ *
+ * @param {Object} opts - Required.
+ *      - {String} tag - Required. The image tag for which to get the ID.
+ * @param {Function} cb - `function (err, imgId, res)`
  */
 RegistryClientV1.prototype.getImgId = function getImgId(opts, cb) {
     var self = this;

package.json

@@ -1,6 +1,6 @@
 {
     "name": "docker-registry-client",
-    "version": "3.2.4",
+    "version": "3.2.5",
     "description": "node.js client for the Docker Registry API",
     "author": "Joyent (joyent.com)",
     "main": "./lib/index.js",

test/v1.dockerio.test.js

@@ -59,8 +59,9 @@ test('v1 docker.io', function (tt) {
         client.listRepoImgs(function (err, imgs) {
             t.ifErr(err);
             t.ok(Array.isArray(imgs));
-            t.ok(imgs.length > 0);
-            t.ok(/[0-9a-f]{64}/.test(imgs[0].id));
+            if (imgs.length > 0) {
+                t.ok(/[0-9a-f]{64}/.test(imgs[0].id));
+            }
             t.end();
         });
     });

test/v2.gcrio.test.js

@@ -11,12 +11,14 @@
 var crypto = require('crypto');
 var strsplit = require('strsplit');
 var test = require('tape');
+var util = require('util');
 
 var drc = require('..');
 
 
 // --- globals
 
+var format = util.format;
 var log = require('./lib/log');
 
 var REPO = 'gcr.io/google_containers/pause-amd64';
@@ -113,7 +115,7 @@ test('v2 gcr.io', function (tt) {
     tt.test('  getManifest (by digest)', function (t) {
         client.getManifest({ref: manifestDigest}, function (err, manifest_) {
             t.ifErr(err);
-            t.ok(manifest);
+            t.ok(manifest_);
             ['schemaVersion',
              'name',
              'tag',
@@ -137,15 +139,15 @@ test('v2 gcr.io', function (tt) {
         var digest = manifest.fsLayers[0].blobSum;
         client.headBlob({digest: digest}, function (err, ress) {
             t.ifErr(err);
-            t.ok(ress);
-            t.ok(Array.isArray(ress));
+            t.ok(ress, 'got responses');
+            t.ok(Array.isArray(ress), 'responses is an array');
             var first = ress[0];
 
             // First request statusCode on a redirect:
             // - gcr.io gives 302 (Found)
             // - docker.io gives 307
             t.ok([200, 302, 303, 307].indexOf(first.statusCode) !== -1,
-                'first request status code 200, 302 or 307: statusCode=' +
+                'first response status code 200, 302 or 307: statusCode=' +
                 first.statusCode);
 
             // No digest head is returned (it's using an earlier version of the
@@ -159,13 +161,19 @@ test('v2 gcr.io', function (tt) {
 
             var last = ress[ress.length - 1];
             t.ok(last);
-            t.equal(last.statusCode, 200);
+            t.equal(last.statusCode, 200,
+                'last response status code should be 200');
 
             // Content-Type:
             // - docker.io gives 'application/octet-stream', which is what
             //   I'd expect for the GET response at least.
+            // - However gcr.io, at least for the iamge being tested, now
+            //   returns text/html.
             t.equal(last.headers['content-type'],
-                'application/octet-stream');
+                'text/html',
+                format('expect specific Content-Type on last response; '
+                    + 'statusCode=%s headers=%j',
+                    last.statusCode, last.headers));
 
             t.ok(last.headers['content-length']);
             t.end();