diff --git a/lib/config_reader.js b/lib/config_reader.js index 8b17e2e..6278bf1 100644 --- a/lib/config_reader.js +++ b/lib/config_reader.js @@ -9,25 +9,31 @@ var logger = require('./logging.js'); * Grab the meta config to bootstrap git2consul. */ exports.read = function(params, cb) { - if (!cb) { - cb = params; - params = {'key': 'git2consul/config'}; - } - - if (global.token) { - params = _.extend(params, {'token': global.token}); - } - - consul.kv.get(params, function(err, item) { - if (err) return cb(err); - - try { - var config = JSON.parse(item.Value); - } catch(e) { - return cb('Config value is not valid JSON: ' + require('util').inspect(item)); + if (!cb) { + cb = params; + params = {}; } - cb(null, config, item); - }); + + //If a key is not passed, default to the global configuration value. If that isn't set (which appears not to be + // the case in a test or two), explicitly fall back to default. + if (! params.key ){ + params['key'] = global.config_key || 'git2consul/config'; + } + + if (global.token) { + params = _.extend(params, {'token': global.token}); + } + + consul.kv.get(params, function(err, item) { + if (err) return cb(err); + + try { + var config = JSON.parse(item.Value); + } catch(e) { + return cb('Config value is not valid JSON: ' + require('util').inspect(item)); + } + cb(null, config, item); + }); }; /* istanbul ignore next */ @@ -35,25 +41,31 @@ exports.read = function(params, cb) { * Wait for the consul config KV to change */ exports.wait = function(cb) { - exports.read(function(err, config, item) { - if (err) return cb(err); + exports.read(function(err, config, item) { + if (err) return logger.error("Could not set up configuration watch, encountered error on initial read: %s", err ); - var modify_index = item.ModifyIndex; - logger.info("Current config index is %s, waiting for changes.", modify_index); - var wait_for_change = function() { - exports.read({'key': 'git2consul/config', index: item.ModifyIndex}, function(err, config, item) { - if (err) return cb(err); + var modify_index = item.ModifyIndex; + logger.info("Current config index is %s, waiting for changes.", modify_index); - if (modify_index !== item.ModifyIndex) { - logger.warn("Config changed."); - return cb(null); - } + var wait_for_change = function() { + exports.read({index: item.ModifyIndex}, function(err, config, item) { - process.nextTick(wait_for_change); - }); - }; + if (err) { + //if we run into an error, report then pause for a while before trying again + //so we don't just spin in a tight loop spitting out errors + cb(err); + setTimeout(wait_for_change, global.config_read_retry_wait); + } + else if (modify_index !== item.ModifyIndex) { + logger.warn("Config changed."); + return cb(null); + } else { + process.nextTick(wait_for_change); + } + }); + }; - process.nextTick(wait_for_change); - }); + process.nextTick(wait_for_change); + }); }; diff --git a/lib/index.js b/lib/index.js index 48efd49..7fb14e9 100644 --- a/lib/index.js +++ b/lib/index.js @@ -13,6 +13,7 @@ global.secure = process.env.CONSUL_SECURE || false; global.token = process.env.TOKEN || null; global.config_file = null; global.config_key = "git2consul/config" +global.config_read_retry_wait = 60000; /** * Parse out flags and override defaults if they are set diff --git a/test/git2consul_halt_on_change_test.js b/test/git2consul_halt_on_change_test.js index 7b48290..74c62b4 100644 --- a/test/git2consul_halt_on_change_test.js +++ b/test/git2consul_halt_on_change_test.js @@ -22,55 +22,126 @@ var token = process.env.TOKEN; * Test halt_on_change. We register a config with halt on change functionality and validate * that halt is called on a consul config change. */ -describe('halt_on_change', function() { - - it ('should halt git2consul when config changes', function(done) { - - consul.kv.set({'key': "git2consul/config", value: '{"fake":"config"}', token: token}, function(err) { - if (err) done(err); - - var repo_config = git_utils.createRepoConfig(); - repo_config.hooks = [{ - 'type': 'stash', - 'url': '/stashpoke_bogus_branch', - 'port': 5053 - }]; - repo_config.name = "halt_on_change_repo"; - - git_commands.init(git_utils.TEST_REMOTE_REPO, function(err) { - if (err) return done(err); - - var sample_key = 'readme.md'; - var sample_value = 'stub data'; - git_utils.addFileToGitRepo(sample_key, sample_value, "Stub commit.", function(err) { - if (err) return done(err); - - var config = { - repos: [repo_config], - local_store: git_utils.TEST_WORKING_DIR, - halt_on_change: true - }; - - // Now, create a repo with hooks. The hooks should be active. - git.createRepos(config, function(err) { - (undefined === err).should.equal(true); - - var repo = git.repos['halt_on_change_repo']; - repo.hooks_active.should.equal(true); - - // Now update config and validate that a halt is seen. - consul.kv.set({'key': "git2consul/config", value: '{"fake":"config2electricboogaloo"}', token: token}, function(err) { - if (err) done(err); - - var check_halt = function() { - if (git.halt_seen) return done(); - setTimeout(check_halt, 50); - }; - check_halt(); +describe('halt_on_change', function () { + + it('should halt git2consul when config changes', function (done) { + + consul.kv.set({'key': "git2consul/config", value: '{"fake":"config"}', token: token}, function (err) { + if (err) done(err); + + var repo_config = git_utils.createRepoConfig(); + repo_config.hooks = [{ + 'type': 'stash', + 'url': '/stashpoke_bogus_branch', + 'port': 5053 + }]; + repo_config.name = "halt_on_change_repo"; + + git_commands.init(git_utils.TEST_REMOTE_REPO, function (err) { + if (err) return done(err); + + var sample_key = 'readme.md'; + var sample_value = 'stub data'; + git_utils.addFileToGitRepo(sample_key, sample_value, "Stub commit.", function (err) { + if (err) return done(err); + + var config = { + repos: [repo_config], + local_store: git_utils.TEST_WORKING_DIR, + halt_on_change: true + }; + + // Now, create a repo with hooks. The hooks should be active. + git.createRepos(config, function (err) { + (undefined === err).should.equal(true); + + var repo = git.repos['halt_on_change_repo']; + repo.hooks_active.should.equal(true); + + // Now update config and validate that a halt is seen. + consul.kv.set({ + 'key': "git2consul/config", + value: '{"fake":"config2electricboogaloo"}', + token: token + }, function (err) { + if (err) return done(err); + + var check_halt = function () { + if (git.halt_seen) return done(); + setTimeout(check_halt, 50); + }; + check_halt(); + }); + }); + }); + }); + }); + }); + + it('should halt git2consul even after error in communicating', function (done) { + + consul.kv.set({'key': "git2consul/config2", value: '{"fake":"config"}', token: token}, function (err) { + if (err) done(err); + + global.config_key = "git2consul/config2" + global.config_read_retry_wait = 500; + var repo_config = git_utils.createRepoConfig(); + repo_config.hooks = [{ + 'type': 'stash', + 'url': '/totally_bogus', + 'port': 5053 + }]; + repo_config.name = "halt_on_change_repo"; + + git_commands.init(git_utils.TEST_REMOTE_REPO, function (err) { + if (err) return done(err); + + var sample_key = 'readme.md'; + var sample_value = 'stub data'; + git_utils.addFileToGitRepo(sample_key, sample_value, "Stub commit.", function (err) { + if (err) return done(err); + + var config = { + repos: [repo_config], + local_store: git_utils.TEST_WORKING_DIR, + halt_on_change: true + }; + + // Now, create a repo with hooks. The hooks should be active. + git.createRepos(config, function (err) { + (undefined === err).should.equal(true); + + var repo = git.repos['halt_on_change_repo']; + repo.hooks_active.should.equal(true); + + // Now remove the config data so it doesn't exist + // This will cause an error with the read + consul.kv.del({ + 'key': "git2consul/config2", + token: token + }, function (err) { + if (err) return done(err); + + // Add the configuration back, except modified. This should + // trigger a halt + consul.kv.set({ + 'key': "git2consul/config2", + value: '{"fake":"config2electricboogaloo"}', + token: token + }, function (err) { + if (err) return done(err); + + var check_halt = function () { + if (git.halt_seen) return done(); + setTimeout(check_halt, 50); + }; + check_halt(); + }); + }); + + }); + }); }); - }); }); - }); }); - }); });