Add namespaced support to objects other than namespaces (#40)

There are several objects/entities that are namespaced by Kubernetes objects. `/api/v1/namespaces/{namespace}/pods/{name}/log` is an obvious one, but there's more. For example: * Proxy objects (e.g., `/api/v1/namespaces/{namespace}/services/{name}/proxy/`) * Deployment related objects (e.g., `/apis/extensions/v1beta1/namespaces/{namespace}/deployments/{name}/rollback`) * ... This PR implements a story for handling these cases and changes the documentation to encourage users to use namespaced functionality by default (e.g., `api.ns.po('foo').get(print)` vs `api.ns.po.get(foo, print)`).
godaddy · Dec 16, 2016 · d083f93 · d083f93
1 parent 017967e
commit d083f93
Show file tree

Hide file tree

Showing 6 changed files with 227 additions and 49 deletions.
diff --git a/README.md b/README.md
@@ -36,7 +36,7 @@ function print(err, result) {
   console.log(JSON.stringify(err || result, null, 2));
 }
 
-k8.namespaces.replicationcontrollers.get('http-rc', print);
+k8.namespaces.replicationcontrollers('http-rc').get(print);
 ```
 
 kubernetes-client supports the Extensions API group. For example, GET
@@ -49,7 +49,7 @@ const k8Ext = new K8Api.Extensions({
   namespace: 'my-project' // Defaults to 'default'
 });
 
-k8Ext.namespaces.deployments.get('http-deployment', print);
+k8Ext.namespaces.deployments('http-deployment').get(print);
 ```
 
 ### Creating and updating
@@ -65,8 +65,7 @@ or update the number of replicas:
 
 ```js
 const patch = { spec: { replicas: 10 } };
-k8.namespaces.replicationcontrollers.patch({
-  name: 'http-rc',
+k8.namespaces.replicationcontrollers('http-rc').patch({
   body: patch
 }, print);
 ```
@@ -107,15 +106,15 @@ resource name (*e.g.*, `namespace` for `namespaces`). We can shorten
 the example above:
 
 ```js
-k8.ns.rc.get('http-rc', print);
+k8.ns.rc('http-rc').get(print);
 ```
 
 ### Switching namespaces
 
 You can call the `namespace` object to specify the namespace:
 
 ```js
-k8.ns('other-project').rc.get('http-rc', print);
+k8.ns('other-project').rc('http-rc').get(print);
 ```
 
 ### Query parameters

diff --git a/lib/base.js b/lib/base.js
@@ -14,18 +14,55 @@ function cb200(cb) {
   };
 }
 
-class BaseObject {
+class CallableObject {
   /**
-   * Create generic Kubernetes API object
+   * Create an object that invokes a function when called.
+   * @param {function} fn - The function to invoke.
+   */
+  constructor(fn) {
+    function wrap() {
+      return fn.apply(this, arguments);
+    }
+
+    if (fn) {
+      return Object.setPrototypeOf(wrap, Object.getPrototypeOf(this));
+    }
+  }
+}
+
+class BaseObject extends CallableObject {
+  /**
+   * Create generic Kubernetes API object. The object is callable (e.g., pod('foo')),
+   * which by default returns a new object of the same type with the parent path
+   * extended by the argument too the function
+   * (e.g., '/api/v1/namespace/default/pods/foo'). Users customize the callable
+   * behavior by passing an optional function to this constructor.
+   *
    * @param {object} options - Options object
    * @param {string} options.api - Kubernetes API URL
+   * @param {string} options.fn - Optional function to invoke when object is
+   * called.
    * @param {string} options.path - Kubernetes resource path
    */
   constructor(options) {
-    this.name = options.name;
+    const api = options.api;
+    const path = `${ options.parentPath }/${ options.name }`;
+
+    let fn = options.fn;
+    if (!fn) {
+      fn = name => {
+        return new this.constructor({
+          api: api,
+          name: name,
+          parentPath: path
+        });
+      }
+    }
+
+    super(fn);
     this.parentPath = options.parentPath;
-    this.api = options.api;
-    this.path = `${ options.parentPath }/${ options.name }`;
+    this.api = api;
+    this.path = path;
     this.qs = options.qs || {};
   }
 
@@ -35,8 +72,13 @@ class BaseObject {
    * @param {callback} cb - The callback that handles the response
    */
   delete(options, cb) {
-    if (typeof options === 'string') options = { name: options };
-    this.api.delete({ path: [this.path, options.name], qs: options.qs },
+    if (typeof options === 'function') {
+      cb = options;
+      options = {};
+    } else if (typeof options === 'string') {
+      options = { name: options };
+    }
+    this.api.delete({ path: this._path(options), qs: options.qs },
                     cb200(cb));
   }
 

diff --git a/lib/pods.js b/lib/pods.js
@@ -1,7 +1,28 @@
 'use strict';
 
+const util = require('util');
+
 const BaseObject = require('./base');
 
+class NamedPods extends BaseObject {
+  /**
+   * Create a named Pod Kubernetes object with a log.
+   * @extends BaseObject
+   * @param {object} options - Options object
+   * @param {Api} options.api - API object
+   * @param {string} options.parentPath - Optional path of parent resource
+   * @param {string} options.path - Optional path of this resource
+   */
+  constructor(options) {
+    super(options);
+    this.log = new BaseObject({
+      api: this.api,
+      name: 'log',
+      parentPath: this.path
+    });
+  }
+}
+
 class Pods extends BaseObject {
   /**
    * Create a Pods Kubernetes API object
@@ -12,11 +33,23 @@ class Pods extends BaseObject {
    * @param {string} options.path - Optional path of this resource
    */
   constructor(options) {
-    super(Object.assign({}, options, { name: options.name || 'pods' }));
+    super(Object.assign({}, options, {
+      fn: name => new NamedPods({
+        api: options.api,
+        name: name,
+        parentPath: this.path
+      }),
+      name: options.name || 'pods'
+    }));
+
+    this.log = util.deprecate(
+      this.log.bind(this),
+      'pods.log is deprecated and will be removed in 4.0.0. ' +
+      'Use pods(name).log.get instead.');
   }
 
   /**
-   * Get a Pod log
+   * @deprecated Will be removed in 4.0.0. Use pods(name).log.get instead.
    * @param {RequestOptions|string} options - GET options, or resource name
    * @param {callback} cb - The callback that handles the response
    * @returns {Stream} If cb is falsy, return a Stream

diff --git a/test/common.js b/test/common.js
@@ -18,10 +18,24 @@ function testing(type) {
   return t.substr(0, 3) === type.substr(0, 3);
 }
 
+/**
+ * Executes mocha's `before` hook if testing `type`.
+ * @param {string} type - Test type (e.g., 'int', or 'unit')
+ * @param {function} fn - Function to execute.
+ */
 function beforeTesting(type, fn) {
   if (testing(type)) { before(fn); }
 }
 
+/**
+ * Executes mocha's `beforeEach` hook if testing `type`.
+ * @param {string} type - Test type (e.g., 'int', or 'unit')
+ * @param {function} fn - Function to execute.
+ */
+function beforeTestingEach(type, fn) {
+  if (testing(type)) { beforeEach(fn); }
+}
+
 function only(types, message, fn) {
   if (typeof (types) === 'string') types = [types];
   for (const type of types) {
@@ -138,4 +152,5 @@ module.exports.apiGroup = apiGroup;
 module.exports.defaultName = defaultName;
 module.exports.testing = testing;
 module.exports.beforeTesting = beforeTesting;
+module.exports.beforeTestingEach = beforeTestingEach;
 module.exports.only = only;
diff --git a/test/objects.test.js b/test/objects.test.js
@@ -3,7 +3,6 @@
 const assume = require('assume');
 const nock = require('nock');
 const ReplicationControllers = require('../lib/replicationcontrollers');
-const Pods = require('../lib/pods');
 const Core = require('../lib/core');
 
 const common = require('./common');
@@ -53,13 +52,6 @@ describe('objects', function () {
     });
   }
 
-  function pods() {
-    return new Pods({
-      api: new Core({ url: _url }),
-      parentPath: _ns
-    });
-  }
-
   describe('.ReplicationControllers.get', function () {
     function nock200() {
       return nock(_url)
@@ -201,30 +193,4 @@ describe('objects', function () {
     });
   });
 
-  describe('.Pods.get', function () {
-    only('unit', 'returns pod', function (done) {
-      const scope = nock(_url).get(`${ _pods }/foo`).reply(200, {
-        kind: 'pod',
-        metadata: { name: 'foo' }
-      });
-      pods().get('foo', (err, pod) => {
-        assume(err).is.falsy();
-        assume(pod.kind).is.equal('pod');
-        assume(scope.isDone()).true();
-        done();
-      });
-    });
-  });
-
-  describe('.Pods.delete', function () {
-    only('unit', 'deletes pod', function (done) {
-      const scope = nock(_url).delete(`${ _pods }/foo`).reply(200, {});
-      pods().delete('foo', err => {
-        assume(err).is.falsy();
-        assume(scope.isDone()).true();
-        done();
-      });
-    });
-  });
-
 });
diff --git a/test/pods.test.js b/test/pods.test.js
@@ -0,0 +1,123 @@
+'use strict';
+
+const assume = require('assume');
+const nock = require('nock');
+
+const common = require('./common');
+const api = common.api;
+const only = common.only;
+const defaultName = common.defaultName;
+const beforeTesting = common.beforeTesting;
+const beforeTestingEach = common.beforeTestingEach;
+
+const testPod = {
+  kind: 'Pod',
+  metadata: {
+    name: 'test-pod'
+  },
+  spec: {
+    containers: [
+      {
+        image: 'does-not-matter:latest',
+        name: 'test'
+      }
+    ]
+  }
+};
+
+describe('lib.pods', () => {
+
+  describe('.post', () => {
+    beforeTesting('int', api.wipe);
+    beforeTestingEach('unit', () => {
+      nock(api.url)
+        .post(`/api/v1/namespaces/${ defaultName }/pods`)
+        .reply(200, {
+          kind: 'Pod',
+          metadata: { name: 'test-pod' }
+        });
+    });
+
+    it('succeeds creating a new pod', done => {
+      api.ns.pods.post({ body: testPod }, (err, pod) => {
+        assume(err).is.falsy();
+        assume(pod.metadata.name).is.equal('test-pod');
+        done();
+      });
+    });
+  });
+
+  describe('.get', () => {
+    beforeTesting('int', done => {
+      api.wipe(err => {
+        assume(err).is.falsy();
+        api.ns.pods.post({ body: testPod }, done);
+      });
+    });
+    beforeTestingEach('unit', () => {
+      nock(api.url)
+        .get(`/api/v1/namespaces/${ defaultName }/pods/test-pod`)
+        .reply(200, {
+          kind: 'Pod',
+          metadata: { name: 'test-pod' }
+        });
+    });
+    it('returns the Pod', done => {
+      api.ns.pods('test-pod').get((err, pod) => {
+        assume(err).is.falsy();
+        assume(pod.kind).is.equal('Pod');
+        done();
+      });
+    });
+    only('unit', 'returns the Pod via the legacy method', done => {
+      api.ns.pods.get('test-pod', (err, pod) => {
+        assume(err).is.falsy();
+        assume(pod.kind).is.equal('Pod');
+        done();
+      });
+    });
+  });
+
+  describe('.delete', () => {
+    beforeTesting('int', done => {
+      api.wipe(err => {
+        assume(err).is.falsy();
+        api.ns.pods.post({ body: testPod }, done);
+      });
+    });
+    beforeTestingEach('unit', () => {
+      nock(api.url)
+        .delete(`/api/v1/namespaces/${ defaultName }/pods/test-pod`)
+        .reply(200, { kind: 'Pod' });
+    });
+    it('deletes the Pod', done => {
+      api.ns.pods('test-pod').delete((err, pod) => {
+        assume(err).is.falsy();
+        assume(pod.kind).is.equal('Pod');
+        done();
+      });
+    });
+  });
+
+  describe('.log', () => {
+    beforeTestingEach('unit', () => {
+      nock(api.url)
+        .get(`/api/v1/namespaces/${ defaultName }/pods/test-pod/log`)
+        .reply(200, 'some log contents');
+    });
+    only('unit', 'returns log contents', done => {
+      api.ns.pods('test-pod').log.get((err, contents) => {
+        assume(err).is.falsy();
+        assume(contents).is.equal('some log contents');
+        done();
+      });
+    });
+    only('unit', 'returns log contents via legacy method', done => {
+      api.ns.pods.log('test-pod', (err, contents) => {
+        assume(err).is.falsy();
+        assume(contents).is.equal('some log contents');
+        done();
+      });
+    });
+  });
+});