git2consul - npm Package Compare versions

Comparing version 0.7.0 to 0.7.1

lib/consul/index.js

@@ -15,6 +15,4 @@ var fs = require('fs');
 
-var write_content_to_consul = function(branch, resource_name, content, cb) {
-  // Prepend repo name and branch name to the KV so that the subtree is properly namespaced in Consul
-  var key_name = branch.repo_name + '/' + resource_name;
-  logger.trace('Adding key %s, value %s', key_name, content);
+var write_content_to_consul = function(key_name, content, cb) {
+  logger.trace('Adding key %s, value:\n%s', key_name, content);
   consul.kv.set({'key': key_name, value: content, token: token}, function(err) {
@@ -29,2 +27,17 @@ if (err) {
 /**
+ * Given a branch and a file, determine the resource's name in the consul KV store
+ */
+var create_key_name = function(branch, file, ref) {
+  // Start with repo name so that the subtree is properly namespaced in Consul
+  var key_parts = [branch.repo_name];
+  // Optionally add the branch name
+  if (branch.include_branch_name && !ref) {
+    key_parts.push(branch.name);
+  }
+  // Finish with the file path
+  key_parts.push(file);
+  return key_parts.join('/');
+};
+
+/**
  * If a file was modified, read its new value and update consul's KV store.
@@ -42,4 +55,3 @@ */
     var body = body ? body.trim() : '';
-    var resource_name = (branch.include_branch_name ? branch.name + "/" : "") + file;
-    write_content_to_consul(branch, resource_name, body, cb);
+    write_content_to_consul(create_key_name(branch, file), body, cb);
   });
@@ -52,5 +64,7 @@ };
 var file_deleted = function(branch, file, cb) {
-  // Prepend branch name to the KV so that the subtree is properly namespaced in Consul
-  var key_name = branch.repo_name + (branch.include_branch_name ? '/' + branch.name : "" ) + '/' + file;
+  // Prepend branch name to the KV so that the subtree is properly namespaced in Consuls
+  var key_name = create_key_name(branch, file);
+
   logger.trace('Deleting key %s', key_name);
+  
   consul.kv.del({'key': key_name, token: token}, function(err) {
@@ -92,4 +106,4 @@ /* istanbul ignore if */
 
-    // TODO: Add a watchdog timer?  It's a bit scary that this method may never fire its callback if
-    // one of the underlying consul operations hangs, especially since the branch_manager is locked
+    // TODO: Add a watchdog timer?  It's a bit scary that this method may never fire its callback
+    // if one of the underlying consul operations hangs, especially since the branch is locked
     // waiting for this update to complete.
@@ -126,3 +140,3 @@ };
 exports.getLastProcessedRef = function(branch, cb) {
-  var key_name = branch.repo_name + '/' + branch.name + '.ref';
+  var key_name = create_key_name(branch, branch.name + '.ref', true);
   consul.kv.get({'key': key_name, token: token}, function(err, item) {
@@ -139,3 +153,3 @@ /* istanbul ignore if */
 exports.setLastProcessedRef = function(branch, ref, cb) {
-  write_content_to_consul(branch, branch.name + '.ref', ref, cb);
+  write_content_to_consul(create_key_name(branch, branch.name + '.ref', true), ref, cb);
 };
@@ -142,0 +156,0 @@

lib/git/branch.js

@@ -11,9 +11,2 @@ var fs = require('fs');
 
-exports.purgeBranchCache = function(repo_config, branch, cb) {
-  var branch_parent = repo_config.local_store + path.sep + repo_config.name;
-  var branch_directory = branch_parent + path.sep + branch;
-
-  rimraf(branch_directory, cb);
-};
-
 function Branch(repo_config, name) {
@@ -26,3 +19,6 @@ // Immutable properties
   Object.defineProperty(this, 'branch_directory', {value: this.branch_parent + path.sep + name});
-  Object.defineProperty(this, 'include_branch_name', {value: repo_config.hasOwnProperty('include_branch_name') ? repo_config.include_branch_name : true});
+  Object.defineProperty(this, 'include_branch_name', {
+    // If include_branch_name is not set, assume true.  Otherwise, identity check the value against true.
+    value: repo_config['include_branch_name'] == undefined || repo_config['include_branch_name'] === true
+  });
 
@@ -51,2 +47,16 @@ // Writable properties
 /**
+ * Delete the local copy of this branch.  If the branch has been corrupted, this is the cleanest way
+ * to reset things.
+ */
+Branch.prototype.purgeBranchCache = function(cb) {
+  var this_obj = this;
+  logger.warn("Purging branch cache %s for branch %s in repo %s", this.branch_directory, this.name, this.repo_name);
+  rimraf(this.branch_directory, function(err) {
+    /* istanbul ignore if */
+    if (err) logger.error("Failed to purge branch cache %s", this_obj.branch_directory);
+    cb();
+  });
+};
+
+/**
  * Fetch the branch up to a given commit.  After this operation, the local copy should be up to date
@@ -59,3 +69,3 @@ * with the branch at that commit.
   git_commands.pull(this_obj.branch_directory, function(err, output) {
-    /* istanbul ignore next */
+    /* istanbul ignore if */
     if (err) {
@@ -142,3 +152,7 @@ this_obj.busy = false;
           this_obj.busy = false;
-          return cb(err);
+          // If we have a failure on getCurrentRef, we suspect that the local copy is hosed.  Purge it
+          // so that it gets reloaded the next time git2consul starts.
+          return this_obj.purgeBranchCache(function() {
+            cb(err);
+          });
         }
@@ -148,6 +162,4 @@
           this_obj.busy = false;
-          // TODO: Delete branch storage if pull fails
-          if (err) {
-            return cb(err);
-          }
+          /* istanbul ignore if */
+          if (err) return cb(err);
           logger.info("Initialized branch %s from %s", this_obj.name, this_obj.repo_name);
@@ -170,4 +182,3 @@ cb();
         this_obj.busy = false;
-        /* istanbul ignore next */
-        // TODO: Delete branch storage if clone fails.
+        /* istanbul ignore if */
         if (err) return cb(err);
@@ -174,0 +185,0 @@ logger.info("Initialized branch %s from %s", this_obj.name, this_obj.repo_name);

lib/git/commands.js

@@ -14,5 +14,6 @@ var exec = require('child_process').exec;
     if (stdout) stdout = stdout.trim();
+    if (stderr) stderr = stderr.trim();
 
-    logger.trace("stdout: " + stdout);
-    logger.trace("stderr: " + stderr);
+    if (stdout.length > 0) logger.trace("stdout:\n" + stdout);
+    if (stderr.length > 0) logger.trace("stderr:\n" + stderr);
 
@@ -19,0 +20,0 @@ if (err) {

lib/git/repo.js

@@ -63,7 +63,20 @@ var _ = require('underscore');
 
+  var pending_operations = 0;
+
+  var errs = [];
+
+  var wrapped_cb = function(err) {
+    if (err) errs.push(err);
+
+    --pending_operations;
+    logger.trace("There are now %s pending ops", pending_operations);
+    if (pending_operations === 0) return cb(errs.length > 0 ? errs : undefined);
+  };
+
   for (var branch_name in this.branches) {
+    ++pending_operations;
     var branch = this.branches[branch_name];
 
     branch.init(function(err) {
-      if (err) return cb(err);
+      if (err) return wrapped_cb(err);
 
@@ -84,22 +97,21 @@ ++branch_count;
           // Init hooks
-          var errs = [];
-
           this_obj.hooks.forEach(function(hook) {
+            ++pending_operations;
             try {
               var hook_provider = hook_providers[hook.type];
               if (!hook_provider) {
-                return errs.push("Invalid hook type " + hook.type);
+                return wrapped_cb("Invalid hook type " + hook.type);
               }
 
               hook_provider.init(hook, this_obj);
+              wrapped_cb();
             } catch (e) {
-              return errs.push("Hook configuration failed due to " + e);
+              return wrapped_cb("Hook configuration failed due to " + e);
             }
           });
 
-          if (errs.length > 0) return cb(errs);
         }
+      }
 
-        cb(null);
-      }
+      wrapped_cb();
     });
@@ -106,0 +118,0 @@ }

package.json

 {
   "name": "git2consul",
   "description": "System for moving data from git to consul",
-  "version": "0.7.0",
+  "version": "0.7.1",
   "contributors": [
@@ -6,0 +6,0 @@ {

test/git2consul_config_validation_test.js

@@ -88,3 +88,3 @@ var _ = require('underscore');
     repo.init(function(err) {
-      err.message.should.containEql('does not appear to be a git repository');
+      err[0].message.should.containEql('does not appear to be a git repository');
       done();
@@ -91,0 +91,0 @@ });

test/git2consul_file_basic_operations_test.js

@@ -29,8 +29,3 @@ var should = require('should');
 
-      // Handle the initial sync of this repo.  Init adds a file to the remote repo, and this line syncs
-      // that to our local cache and to consul.
-      branch.handleRefChange(0, function(err) {
-        if (err) return done(err);
-        done();
-      });
+      done();
     });
@@ -37,0 +32,0 @@ });

test/git2consul_init_test.js

 var should = require('should');
 var _ = require('underscore');
 
+var path = require('path');
+
 // We want this above any git2consul module to make sure logging gets configured
-var bootstrap = require('./git2consul_bootstrap_test.js');
+require('./git2consul_bootstrap_test.js');
 
+var rimraf = require('rimraf');
+
 var consul_utils = require('./utils/consul_utils.js');
@@ -77,2 +81,26 @@
   });
+
+  it ('should support disabling include_branch_name mode', function(done) {
+
+    // Create a remote git repo.  Then, init a Repo object with include_branch_name disabled and validate
+    // that files are in the appropriate place in the Consul KV store.
+    git_commands.init(git_utils.TEST_REMOTE_REPO, function(err) {
+      if (err) return done(err);
+
+      git_utils.addFileToGitRepo("readme.md", "Test file branch-less KV", "Test commit.", function(err) {
+        if (err) return done(err);
+
+        var repo_config = git_utils.createRepoConfig();
+        repo_config.include_branch_name = false;
+        var repo = new Repo(repo_config);
+        repo.init(function(err) {
+          if (err) return done(err);
+          consul_utils.validateValue('test_repo/readme.md', "Test file branch-less KV", function(err, value) {
+            if (err) return done(err);
+            done();
+          });
+        });
+      });
+    });
+  });
 });
@@ -82,8 +110,52 @@
   it ('should gracefully handle a repo even when the local branch cache is corrupted', function(done) {
-    // TODO: Add a test that creates a branch cache, trashes it, and then creates it again.  The trashed
-    // cache dir should be deleted and git2consul should restart.
-    done();
+    // Create an empty git repo
+    git_utils.initRepo(function(err, repo) {
+      if (err) return done(err);
+
+      // Now kill the git root of our working copy of the test repo, and kill it ahrd.
+      rimraf(git_utils.TEST_WORKING_DIR + path.sep + 'test_repo' + path.sep + 'master' + path.sep + '.git', function(err) {
+        if (err) return done(err);
+
+        // Now try to create a new repo around that working dir.
+        var repo = new Repo(git_utils.createRepoConfig());
+        repo.init(function(err) {
+          err.length.should.equal(1);
+          err[0].message.indexOf('Not a git repository').should.not.equal(-1);
+
+          // Now try to recreate the repo, simulating a git2consul restart.  The deliberately sabotaged branch should be out
+          // of the way, so the repo should init successfully.
+          repo.init(function(err) {
+            (undefined === err).should.equal(true);
+            done();
+          });
+        });
+      });
+    });
   });
 });
 
+describe ('Updates to existing local caches', function() {
+  it ('should start correctly if the local cache already exists', function(done) {
+    // Start off with a working repo instance
+    git_utils.initRepo(function(err, repo) {
+      if (err) return done(err);
+
+      // Now purge the consul store of the uploaded value
+      consul_utils.purgeKeys('test_repo', function(err) {
+        if (err) return done(err);
+
+        // Now try to recreate the repo, simulating a git2consul restart.  This should pull rather than clone the
+        // git repo, but the result should be a correctly populated consul KV entry with the remote repo contents.
+        repo.init(function(err) {
+          (undefined === err).should.equal(true);
+          consul_utils.validateValue('test_repo/master/readme.md', "Stub file to give us something to commit.", function(err, value) {
+            if (err) return done(err);
+            done();
+          });
+        });
+      });
+    });
+  });
+});
+
 describe('git2consul config', function() {
@@ -103,2 +175,3 @@
       // that to our local cache and to consul.
+      // TODO: Is this necessary?
       branch.handleRefChange(0, function(err) {
@@ -105,0 +178,0 @@ if (err) return done(err);

test/utils/consul_utils.js

@@ -19,4 +19,5 @@ var should = require('should');
     if (err) return cb(err);
-    if (!expected_value) (value == undefined).should.equal(true);
-    else {
+    if (!expected_value) {
+      (value == undefined).should.equal(true);
+    } else {
       (value != undefined).should.equal(true);
@@ -36,4 +37,7 @@ value.should.equal(expected_value);
 
-        if (wait_for_present && value === undefined) return setTimeout(check_value, 50);
-        else if (!wait_for_present && value !== undefined) return setTimeout(check_value, 50);
+        if (wait_for_present && value === undefined) {
+          return setTimeout(check_value, 50);
+        } else if (!wait_for_present && value !== undefined) {
+          return setTimeout(check_value, 50);
+        }
 
@@ -40,0 +44,0 @@ // Fire once the wait_for_present criteria is satisfied

No alert changes

Improved metrics

Total package byte prevSize

103231

increased by3.74%

Lines of code

2290

increased by4.23%

No dependency changes