From 5aeeac2c8fef5a126d9a058b56e4a7b70511e2c5 Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Thu, 10 Aug 2017 11:21:44 -0700 Subject: [PATCH 1/3] logging: use correct case for cluster-name attribtue Metadata servies attributes seem to be in kebab-case. --- packages/logging/src/metadata.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/logging/src/metadata.js b/packages/logging/src/metadata.js index 18aacffccb1..0bbe1dffc63 100644 --- a/packages/logging/src/metadata.js +++ b/packages/logging/src/metadata.js @@ -103,7 +103,7 @@ Metadata.getGCEDescriptor = function(callback) { * @return {object} */ Metadata.getGKEDescriptor = function(callback) { - gcpMetadata.instance('attributes/clusterName', function(err, _, clusterName) { + gcpMetadata.instance('attributes/cluster-name', function(err, _, clusterName) { if (err) { callback(err); return; From 1549c751fbf20d7605902b4e0aaaa4423e0b09b4 Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Thu, 10 Aug 2017 12:03:14 -0700 Subject: [PATCH 2/3] expand testing of metadata access and fix lint errors --- packages/logging/src/metadata.js | 26 ++++++++++++----------- packages/logging/test/metadata.js | 34 +++++++++++++++++++++++-------- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/packages/logging/src/metadata.js b/packages/logging/src/metadata.js index 0bbe1dffc63..6ca246717ba 100644 --- a/packages/logging/src/metadata.js +++ b/packages/logging/src/metadata.js @@ -103,20 +103,22 @@ Metadata.getGCEDescriptor = function(callback) { * @return {object} */ Metadata.getGKEDescriptor = function(callback) { - gcpMetadata.instance('attributes/cluster-name', function(err, _, clusterName) { - if (err) { - callback(err); - return; - } - - callback(null, { - type: 'container', - labels: { - // TODO(ofrobots): it would be good to include the namespace_id as well. - cluster_name: clusterName + gcpMetadata.instance('attributes/cluster-name', + function(err, _, clusterName) { + if (err) { + callback(err); + return; } + + callback(null, { + type: 'container', + labels: { + // TODO(ofrobots): it would be good to include the namespace_id as + // well. + cluster_name: clusterName + } + }); }); - }); }; /** diff --git a/packages/logging/test/metadata.js b/packages/logging/test/metadata.js index e39bf42777c..7297b63743e 100644 --- a/packages/logging/test/metadata.js +++ b/packages/logging/test/metadata.js @@ -20,11 +20,15 @@ var assert = require('assert'); var extend = require('extend'); var proxyquire = require('proxyquire'); -var instanceArgsOverride; +var instanceOverride; var fakeGcpMetadata = { instance: function(path, cb) { setImmediate(function() { - var args = instanceArgsOverride || [null, null, 'fake-instance-value']; + if (instanceOverride && instanceOverride.path) { + assert.strictEqual(path, instanceOverride.path); + } + var args = (instanceOverride && instanceOverride.args) || + [null, null, 'fake-instance-value']; cb.apply(fakeGcpMetadata, args); }); } @@ -53,7 +57,7 @@ describe('metadata', function() { }; extend(Metadata, MetadataCached); metadata = new Metadata(LOGGING); - instanceArgsOverride = null; + instanceOverride = null; }); afterEach(function() { @@ -119,7 +123,10 @@ describe('metadata', function() { var CLUSTER_NAME = 'gke-cluster-name'; it('should return the correct descriptor', function(done) { - instanceArgsOverride = [null, null, CLUSTER_NAME]; + instanceOverride = { + path: 'attributes/cluster-name', + args: [null, null, CLUSTER_NAME] + }; Metadata.getGKEDescriptor(function(err, descriptor) { assert.ifError(err); @@ -135,7 +142,7 @@ describe('metadata', function() { it('should return error on failure to acquire metadata', function(done) { var FAKE_ERROR = new Error(); - instanceArgsOverride = [ FAKE_ERROR ]; + instanceOverride = { args: [ FAKE_ERROR ] }; Metadata.getGKEDescriptor(function(err) { assert.strictEqual(err, FAKE_ERROR); @@ -148,7 +155,10 @@ describe('metadata', function() { var INSTANCE_ID = 'fake-instance-id'; it('should return the correct descriptor', function(done) { - instanceArgsOverride = [null, null, INSTANCE_ID]; + instanceOverride = { + path: 'id', + args: [null, null, INSTANCE_ID] + }; Metadata.getGCEDescriptor(function(err, descriptor) { assert.ifError(err); @@ -164,7 +174,7 @@ describe('metadata', function() { it('should return error on failure to acquire metadata', function(done) { var FAKE_ERROR = new Error(); - instanceArgsOverride = [ FAKE_ERROR ]; + instanceOverride = { args: [ FAKE_ERROR ] }; Metadata.getGCEDescriptor(function(err) { assert.strictEqual(err, FAKE_ERROR); @@ -240,7 +250,10 @@ describe('metadata', function() { describe('compute engine', function() { it('should return correct descriptor', function(done) { var INSTANCE_ID = 'overridden-value'; - instanceArgsOverride = [null, null, INSTANCE_ID]; + instanceOverride = { + path: 'id', + args: [null, null, INSTANCE_ID] + }; metadata.logging.auth.getEnvironment = function(callback) { callback(null, { @@ -264,7 +277,10 @@ describe('metadata', function() { describe('container engine', function() { it('should return correct descriptor', function(done) { var CLUSTER_NAME = 'overridden-value'; - instanceArgsOverride = [null, null, CLUSTER_NAME]; + instanceOverride = { + path: 'attributes/cluster-name', + args: [null, null, CLUSTER_NAME] + }; metadata.logging.auth.getEnvironment = function(callback) { callback(null, { From d8c14cb8ee3e404075a9eae16ef14f910c2419a3 Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Fri, 11 Aug 2017 15:35:36 -0400 Subject: [PATCH 3/3] err -> e to save new line --- packages/logging/src/metadata.js | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/packages/logging/src/metadata.js b/packages/logging/src/metadata.js index 6ca246717ba..73e02ac22d3 100644 --- a/packages/logging/src/metadata.js +++ b/packages/logging/src/metadata.js @@ -103,22 +103,21 @@ Metadata.getGCEDescriptor = function(callback) { * @return {object} */ Metadata.getGKEDescriptor = function(callback) { - gcpMetadata.instance('attributes/cluster-name', - function(err, _, clusterName) { - if (err) { - callback(err); - return; - } + gcpMetadata.instance('attributes/cluster-name', function(e, _, clusterName) { + if (e) { + callback(e); + return; + } - callback(null, { - type: 'container', - labels: { - // TODO(ofrobots): it would be good to include the namespace_id as - // well. - cluster_name: clusterName - } - }); + callback(null, { + type: 'container', + labels: { + // TODO(ofrobots): it would be good to include the namespace_id as + // well. + cluster_name: clusterName + } }); + }); }; /**