From a7c474a09893b9aa3d6c20249c32b3150e202d19 Mon Sep 17 00:00:00 2001 From: david-sarah Date: Tue, 3 Aug 2010 01:54:16 -0700 Subject: [PATCH] CLI: further improve consistency of basedir options and add tests. addresses #118 --- src/allmydata/scripts/cli.py | 6 +- src/allmydata/scripts/create_node.py | 5 +- src/allmydata/scripts/debug.py | 4 +- src/allmydata/scripts/keygen.py | 10 +- src/allmydata/scripts/runner.py | 13 +- src/allmydata/scripts/startstop_node.py | 57 ++++--- src/allmydata/scripts/stats_gatherer.py | 34 ++--- src/allmydata/test/test_runner.py | 189 ++++++++---------------- 8 files changed, 133 insertions(+), 185 deletions(-) diff --git a/src/allmydata/scripts/cli.py b/src/allmydata/scripts/cli.py index 1c01fe4d..bb0f4db2 100644 --- a/src/allmydata/scripts/cli.py +++ b/src/allmydata/scripts/cli.py @@ -13,9 +13,9 @@ class VDriveOptions(BaseOptions): "Specify which Tahoe node directory should be used. The directory " "should either contain a full Tahoe node, or a file named node.url " "that points to some other Tahoe node. It should also contain a file " - "named private/aliases which contains the mapping from alias name " - "to root dirnode URI." + ( - _default_nodedir and (" [default for most commands: " + quote_output(_default_nodedir) + "]") or "")], + "named '" + os.path.join('private', 'aliases') + "' which contains the " + "mapping from alias name to root dirnode URI." + ( + _default_nodedir and (" [default: " + quote_output(_default_nodedir) + "]") or "")], ["node-url", "u", None, "Specify the URL of the Tahoe gateway node, such as 'http://127.0.0.1:3456'. " "This overrides the URL found in the --node-directory ."], diff --git a/src/allmydata/scripts/create_node.py b/src/allmydata/scripts/create_node.py index 5a7f2527..d12c0285 100644 --- a/src/allmydata/scripts/create_node.py +++ b/src/allmydata/scripts/create_node.py @@ -7,7 +7,6 @@ import allmydata class CreateClientOptions(BasedirMixin, BaseOptions): optParameters = [ - ("basedir", "C", None, "which directory to create the node in"), # we provide 'create-node'-time options for the most common # configuration knobs. The rest can be controlled by editing # tahoe.cfg before node startup. @@ -26,8 +25,8 @@ class CreateIntroducerOptions(BasedirMixin, BaseOptions): default_nodedir = None optParameters = [ - ["basedir", "C", None, "which directory to create the introducer in"], - ] + ["node-directory", "d", None, "Specify which directory the introducer should be created in. [no default]"], + ] client_tac = """ # -*- python -*- diff --git a/src/allmydata/scripts/debug.py b/src/allmydata/scripts/debug.py index 793ee1c3..bd828ee0 100644 --- a/src/allmydata/scripts/debug.py +++ b/src/allmydata/scripts/debug.py @@ -770,7 +770,8 @@ def corrupt_share(options): class ReplOptions(usage.Options): - pass + def getSynopsis(self): + return "Usage: tahoe debug repl" def repl(options): import code @@ -801,6 +802,7 @@ Subcommands: tahoe debug find-shares Locate sharefiles in node directories. tahoe debug catalog-shares Describe all shares in node dirs. tahoe debug corrupt-share Corrupt a share by flipping a bit. + tahoe debug repl Open a Python interpreter. Please run e.g. 'tahoe debug dump-share --help' for more details on each subcommand. diff --git a/src/allmydata/scripts/keygen.py b/src/allmydata/scripts/keygen.py index a8fd7fc1..f96b3a76 100644 --- a/src/allmydata/scripts/keygen.py +++ b/src/allmydata/scripts/keygen.py @@ -1,14 +1,14 @@ import os, sys from allmydata.scripts.common import BasedirMixin, BaseOptions +from allmydata.util.assertutil import precondition from allmydata.util.encodingutil import listdir_unicode, quote_output class CreateKeyGeneratorOptions(BasedirMixin, BaseOptions): default_nodedir = None - allow_multiple = False optParameters = [ - ["basedir", "C", None, "which directory to create the key-generator in"], + ["node-directory", "d", None, "Specify which directory the key-generator should be created in. [no default]"], ] keygen_tac = """ @@ -26,8 +26,10 @@ application = service.Application("allmydata_key_generator") k.setServiceParent(application) """ -def create_key_generator(config, out=sys.stdout, err=sys.stderr): - basedir = config['basedirs'][0] +def create_key_generator(basedir, config, out=sys.stdout, err=sys.stderr): + # This should always be called with an absolute Unicode basedir. + precondition(isinstance(basedir, unicode), basedir) + if os.path.exists(basedir): if listdir_unicode(basedir): print >>err, "The base directory %s is not empty." % quote_output(basedir) diff --git a/src/allmydata/scripts/runner.py b/src/allmydata/scripts/runner.py index 1da7059a..4a81b06d 100644 --- a/src/allmydata/scripts/runner.py +++ b/src/allmydata/scripts/runner.py @@ -41,6 +41,11 @@ class Options(BaseOptions, usage.Options): if not hasattr(self, 'subOptions'): raise usage.UsageError("must specify a command") + +create_dispatch = {} +for module in (create_node, keygen, stats_gatherer): + create_dispatch.update(module.dispatch) + def runner(argv, run_by_human=True, stdin=None, stdout=None, stderr=None, @@ -87,9 +92,9 @@ def runner(argv, so.stdin = stdin rc = 0 - if command in create_node.dispatch: + if command in create_dispatch: + f = create_dispatch[command] for basedir in so.basedirs: - f = create_node.dispatch[command] rc = f(basedir, so, stdout, stderr) or rc elif command in startstop_node.dispatch: rc = startstop_node.dispatch[command](so, stdout, stderr) @@ -97,10 +102,6 @@ def runner(argv, rc = debug.dispatch[command](so) elif command in cli.dispatch: rc = cli.dispatch[command](so) - elif command in keygen.dispatch: - rc = keygen.dispatch[command](so, stdout, stderr) - elif command in stats_gatherer.dispatch: - rc = stats_gatherer.dispatch[command](so) elif command in ac_dispatch: rc = ac_dispatch[command](so, stdout, stderr) else: diff --git a/src/allmydata/scripts/startstop_node.py b/src/allmydata/scripts/startstop_node.py index 551b92fd..1fe996e2 100644 --- a/src/allmydata/scripts/startstop_node.py +++ b/src/allmydata/scripts/startstop_node.py @@ -2,25 +2,19 @@ import os, sys, signal, time from allmydata.scripts.common import BasedirMixin, BaseOptions from allmydata.util import fileutil, find_exe +from allmydata.util.assertutil import precondition +from allmydata.util.encodingutil import listdir_unicode, quote_output class StartOptions(BasedirMixin, BaseOptions): - optParameters = [ - ["basedir", "C", None, "which directory to start the node in"], - ] optFlags = [ ["profile", "p", "Run under the Python profiler, putting results in 'profiling_results.prof'."], ["syslog", None, "Tell the node to log to syslog, not a file."], ] class StopOptions(BasedirMixin, BaseOptions): - optParameters = [ - ["basedir", "C", None, "which directory to stop the node in"], - ] + pass class RestartOptions(BasedirMixin, BaseOptions): - optParameters = [ - ["basedir", "C", None, "which directory to restart the node in"], - ] optFlags = [ ["profile", "p", "Run under the Python profiler, putting results in 'profiling_results.prof'."], ["syslog", None, "Tell the node to log to syslog, not a file."], @@ -28,22 +22,24 @@ class RestartOptions(BasedirMixin, BaseOptions): class RunOptions(BasedirMixin, BaseOptions): default_nodedir = u"." + allow_multiple = False optParameters = [ - ["basedir", "C", None, "which directory to run the node in, CWD by default"], - ] + ["node-directory", "d", None, "Specify the directory of the node to be run. [default, for 'tahoe run' only: current directory]"], + ["multiple", "m", None, "['tahoe run' cannot accept multiple node directories]"], + ] def do_start(basedir, opts, out=sys.stdout, err=sys.stderr): - print >>out, "STARTING", basedir + print >>out, "STARTING", quote_output(basedir) if not os.path.isdir(basedir): - print >>err, "%s does not look like a directory at all" % basedir + print >>err, "%s does not look like a directory at all" % quote_output(basedir) return 1 - for fn in os.listdir(basedir): - if fn.endswith(".tac"): - tac = fn + for fn in listdir_unicode(basedir): + if fn.endswith(u".tac"): + tac = str(fn) break else: - print >>err, "%s does not look like a node directory (no .tac file)" % basedir + print >>err, "%s does not look like a node directory (no .tac file)" % quote_output(basedir) return 1 if "client" in tac: nodetype = "client" @@ -108,10 +104,10 @@ def do_start(basedir, opts, out=sys.stdout, err=sys.stderr): return 1 def do_stop(basedir, out=sys.stdout, err=sys.stderr): - print >>out, "STOPPING", basedir + print >>out, "STOPPING", quote_output(basedir) pidfile = os.path.join(basedir, "twistd.pid") if not os.path.exists(pidfile): - print >>err, "%s does not look like a running node directory (no twistd.pid)" % basedir + print >>err, "%s does not look like a running node directory (no twistd.pid)" % quote_output(basedir) # we define rc=2 to mean "nothing is running, but it wasn't me who # stopped it" return 2 @@ -194,11 +190,26 @@ def run(config, stdout, stderr): from twisted.python import log, logfile from allmydata import client - basedir = config['basedir'] - if basedir is None: - basedir = '.' + basedir = config['basedirs'][0] + precondition(isinstance(basedir, unicode), basedir) + + if not os.path.isdir(basedir): + print >>stderr, "%s does not look like a directory at all" % quote_output(basedir) + return 1 + for fn in listdir_unicode(basedir): + if fn.endswith(u".tac"): + tac = str(fn) + break else: - os.chdir(basedir) + print >>stderr, "%s does not look like a node directory (no .tac file)" % quote_output(basedir) + return 1 + if "client" not in tac: + print >>stderr, ("%s looks like it contains a non-client node (%s).\n" + "Use 'tahoe start' instead of 'tahoe run'." + % (quote_output(basedir), tac)) + return 1 + + os.chdir(basedir) # set up twisted logging. this will become part of the node rsn. logdir = os.path.join(basedir, 'logs') diff --git a/src/allmydata/scripts/stats_gatherer.py b/src/allmydata/scripts/stats_gatherer.py index e799d4e8..0764bfc5 100644 --- a/src/allmydata/scripts/stats_gatherer.py +++ b/src/allmydata/scripts/stats_gatherer.py @@ -1,16 +1,15 @@ -import os -from twisted.python import usage +import os, sys +from allmydata.scripts.common import BasedirMixin, BaseOptions +from allmydata.util.assertutil import precondition +from allmydata.util.encodingutil import listdir_unicode, quote_output -class CreateStatsGathererOptions(usage.Options): - optParameters = [ - ["basedir", "C", None, "which directory to create the stats-gatherer in"], - ] +class CreateStatsGathererOptions(BasedirMixin, BaseOptions): + default_nodedir = None - def parseArgs(self, basedir=None): - if basedir is not None: - assert self["basedir"] is None - self["basedir"] = basedir + optParameters = [ + ["node-directory", "d", None, "Specify which directory the stats-gatherer should be created in. [no default]"], + ] stats_gatherer_tac = """ @@ -26,15 +25,14 @@ application = service.Application('allmydata_stats_gatherer') g.setServiceParent(application) """ -def create_stats_gatherer(config): - err = config.stderr - basedir = config['basedir'] - if not basedir: - print >>err, "a basedir was not provided, please use --basedir or -C" - return -1 + +def create_stats_gatherer(basedir, config, out=sys.stdout, err=sys.stderr): + # This should always be called with an absolute Unicode basedir. + precondition(isinstance(basedir, unicode), basedir) + if os.path.exists(basedir): - if os.listdir(basedir): - print >>err, "The base directory \"%s\", which is \"%s\" is not empty." % (basedir, os.path.abspath(basedir)) + if listdir_unicode(basedir): + print >>err, "The base directory %s is not empty." % quote_output(basedir) print >>err, "To avoid clobbering anything, I am going to quit now." print >>err, "Please use a different directory, or empty this one." return -1 diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index 62c328ae..42e73a75 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -147,27 +147,33 @@ class CreateNode(unittest.TestCase): rc = runner.runner(argv, stdout=out, stderr=err) return rc, out.getvalue(), err.getvalue() - def do_create(self, command, basedir): - c1 = os.path.join(basedir, command + "-c1") - argv = ["--quiet", command, "--basedir", c1] + def do_create(self, kind): + basedir = self.workdir("test_" + kind) + command = "create-" + kind + is_client = kind in ("node", "client") + tac = is_client and "tahoe-client.tac" or ("tahoe-" + kind + ".tac") + + n1 = os.path.join(basedir, command + "-n1") + argv = ["--quiet", command, "--basedir", n1] rc, out, err = self.run_tahoe(argv) self.failUnlessEqual(err, "") self.failUnlessEqual(out, "") self.failUnlessEqual(rc, 0) - self.failUnless(os.path.exists(c1)) - self.failUnless(os.path.exists(os.path.join(c1, "tahoe-client.tac"))) - - # tahoe.cfg should exist, and should have storage enabled for - # 'create-node', and disabled for 'create-client'. - tahoe_cfg = os.path.join(c1, "tahoe.cfg") - self.failUnless(os.path.exists(tahoe_cfg)) - content = open(tahoe_cfg).read() - if command == "create-client": - self.failUnless("\n[storage]\nenabled = false\n" in content) - else: - self.failUnless("\n[storage]\nenabled = true\n" in content) - - # creating the client a second time should be rejected + self.failUnless(os.path.exists(n1)) + self.failUnless(os.path.exists(os.path.join(n1, tac))) + + if is_client: + # tahoe.cfg should exist, and should have storage enabled for + # 'create-node', and disabled for 'create-client'. + tahoe_cfg = os.path.join(n1, "tahoe.cfg") + self.failUnless(os.path.exists(tahoe_cfg)) + content = open(tahoe_cfg).read() + if kind == "client": + self.failUnless("\n[storage]\nenabled = false\n" in content) + else: + self.failUnless("\n[storage]\nenabled = true\n" in content) + + # creating the node a second time should be rejected rc, out, err = self.run_tahoe(argv) self.failIfEqual(rc, 0, str((out, err, rc))) self.failUnlessEqual(out, "") @@ -179,138 +185,67 @@ class CreateNode(unittest.TestCase): self.failIf(re.search("[\S][^\.!?]$", line), (line,)) # test that the non --basedir form works too - c2 = os.path.join(basedir, command + "c2") - argv = ["--quiet", command, c2] - rc, out, err = self.run_tahoe(argv) - self.failUnless(os.path.exists(c2)) - self.failUnless(os.path.exists(os.path.join(c2, "tahoe-client.tac"))) - - # make sure it rejects too many arguments - argv = [command, "basedir", "extraarg"] - self.failUnlessRaises(usage.UsageError, - runner.runner, argv, - run_by_human=False) - - def test_node(self): - basedir = self.workdir("test_node") - self.do_create("create-node", basedir) - - def test_client(self): - # create-client should behave like create-node --no-storage. - basedir = self.workdir("test_client") - self.do_create("create-client", basedir) - - def test_introducer(self): - basedir = self.workdir("test_introducer") - c1 = os.path.join(basedir, "c1") - argv = ["--quiet", "create-introducer", "--basedir", c1] + n2 = os.path.join(basedir, command + "-n2") + argv = ["--quiet", command, n2] rc, out, err = self.run_tahoe(argv) - self.failUnlessEqual(err, "", err) + self.failUnlessEqual(err, "") self.failUnlessEqual(out, "") self.failUnlessEqual(rc, 0) - self.failUnless(os.path.exists(c1)) - self.failUnless(os.path.exists(os.path.join(c1,"tahoe-introducer.tac"))) - - # creating the introducer a second time should be rejected - rc, out, err = self.run_tahoe(argv) - self.failIfEqual(rc, 0) - self.failUnlessEqual(out, "") - self.failUnless("is not empty" in err) - - # Fail if there is a non-empty line that doesn't end with a - # punctuation mark. - for line in err.splitlines(): - self.failIf(re.search("[\S][^\.!?]$", line), (line,)) + self.failUnless(os.path.exists(n2)) + self.failUnless(os.path.exists(os.path.join(n2, tac))) - # test the non --basedir form - c2 = os.path.join(basedir, "c2") - argv = ["--quiet", "create-introducer", c2] + # test the --node-directory form + n3 = os.path.join(basedir, command + "-n3") + argv = ["--quiet", command, "--node-directory", n3] rc, out, err = self.run_tahoe(argv) - self.failUnlessEqual(err, "", err) + self.failUnlessEqual(err, "") self.failUnlessEqual(out, "") self.failUnlessEqual(rc, 0) - self.failUnless(os.path.exists(c2)) - self.failUnless(os.path.exists(os.path.join(c2,"tahoe-introducer.tac"))) - - # reject extra arguments - argv = ["create-introducer", "basedir", "extraarg"] - self.failUnlessRaises(usage.UsageError, - runner.runner, argv, - run_by_human=False) - # and require basedir to be provided in some form - argv = ["create-introducer"] - self.failUnlessRaises(usage.UsageError, - runner.runner, argv, - run_by_human=False) + self.failUnless(os.path.exists(n3)) + self.failUnless(os.path.exists(os.path.join(n3, tac))) - def test_key_generator(self): - basedir = self.workdir("test_key_generator") - kg1 = os.path.join(basedir, "kg1") - argv = ["--quiet", "create-key-generator", "--basedir", kg1] + # test the --multiple form + n4 = os.path.join(basedir, command + "-n4") + n5 = os.path.join(basedir, command + "-n5") + argv = ["--quiet", command, "--multiple", n4, n5] rc, out, err = self.run_tahoe(argv) self.failUnlessEqual(err, "") self.failUnlessEqual(out, "") self.failUnlessEqual(rc, 0) - self.failUnless(os.path.exists(kg1)) - self.failUnless(os.path.exists(os.path.join(kg1, "tahoe-key-generator.tac"))) - - # creating it a second time should be rejected - rc, out, err = self.run_tahoe(argv) - self.failIfEqual(rc, 0, str((out, err, rc))) - self.failUnlessEqual(out, "") - self.failUnlessIn("is not empty.", err) + self.failUnless(os.path.exists(n4)) + self.failUnless(os.path.exists(os.path.join(n4, tac))) + self.failUnless(os.path.exists(n5)) + self.failUnless(os.path.exists(os.path.join(n5, tac))) - # make sure it rejects too many arguments - argv = ["create-key-generator", "basedir", "extraarg"] + # make sure it rejects too many arguments without --multiple + argv = [command, "basedir", "extraarg"] self.failUnlessRaises(usage.UsageError, runner.runner, argv, run_by_human=False) - # make sure it rejects a missing basedir specification - argv = ["create-key-generator"] - self.failUnlessRaises(usage.UsageError, - runner.runner, argv, - run_by_human=False) + # when creating a non-client, there is no default for the basedir + if not is_client: + argv = [command] + self.failUnlessRaises(usage.UsageError, + runner.runner, argv, + run_by_human=False) - def test_stats_gatherer(self): - basedir = self.workdir("test_stats_gatherer") - sg1 = os.path.join(basedir, "sg1") - argv = ["--quiet", "create-stats-gatherer", "--basedir", sg1] - rc, out, err = self.run_tahoe(argv) - self.failUnlessEqual(err, "") - self.failUnlessEqual(out, "") - self.failUnlessEqual(rc, 0) - self.failUnless(os.path.exists(sg1)) - self.failUnless(os.path.exists(os.path.join(sg1, "tahoe-stats-gatherer.tac"))) - # creating it a second time should be rejected - rc, out, err = self.run_tahoe(argv) - self.failIfEqual(rc, 0, str((out, err, rc))) - self.failUnlessEqual(out, "") - self.failUnless("is not empty." in err) + def test_node(self): + self.do_create("node") - # test the non --basedir form - kg2 = os.path.join(basedir, "kg2") - argv = ["--quiet", "create-stats-gatherer", kg2] - rc, out, err = self.run_tahoe(argv) - self.failUnlessEqual(err, "", err) - self.failUnlessEqual(out, "") - self.failUnlessEqual(rc, 0) - self.failUnless(os.path.exists(kg2)) - self.failUnless(os.path.exists(os.path.join(kg2,"tahoe-stats-gatherer.tac"))) + def test_client(self): + # create-client should behave like create-node --no-storage. + self.do_create("client") - # make sure it rejects too many arguments - argv = ["create-stats-gatherer", "basedir", "extraarg"] - self.failUnlessRaises(usage.UsageError, - runner.runner, argv, - run_by_human=False) + def test_introducer(self): + self.do_create("introducer") - # make sure it rejects a missing basedir specification - argv = ["create-stats-gatherer"] - rc, out, err = self.run_tahoe(argv) - self.failIfEqual(rc, 0, str((out, err, rc))) - self.failUnlessEqual(out, "") - self.failUnless("a basedir was not provided" in err) + def test_key_generator(self): + self.do_create("key-generator") + + def test_stats_gatherer(self): + self.do_create("stats-gatherer") def test_subcommands(self): # no arguments should trigger a command listing, via UsageError -- 2.45.2