Fixed, I'll send the complete patch responding to this email.

On Tue, Aug 12, 2014 at 11:30 AM, Michele Tartara <[email protected]>
wrote:

> Title: usually contains a verb describing what the patch does. Something
> like: "Add tool that..."
>
Fixed.


> Il 11/ago/2014 09:28 "'Aaron Karper' via ganeti-devel" <
> [email protected]> ha scritto:
>
> >
> > This relies on the ReqConfigQuery and takes several RFC6901 paths and
> > will return a table of the keys and values. The data accessible is the
> > same as in config.data (typically found at /var/lib/ganeti/config.data).
> >
> > Signed-off-by: Aaron Karper <[email protected]>
> > ---
> >  Makefile.am                |   3 +-
> >  src/Ganeti/Confd/Server.hs |   3 +-
> >  src/Ganeti/Confd/Utils.hs  |   3 +-
> >  tools/query-config         | 145
> +++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 151 insertions(+), 3 deletions(-)
> >  create mode 100755 tools/query-config
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 3b0fa8f..34d4156 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -1292,7 +1292,8 @@ python_scripts = \
> >         tools/move-instance \
> >         tools/ovfconverter \
> >         tools/post-upgrade \
> > -       tools/sanitize-config
> > +       tools/sanitize-config \
> > +       tools/query-config
>
> Please, double check that this is enough to have the tool built and
> distributed (and commit-check happy)
>
Yes, the tool doesn't require any new modules, so this is actually the only
new file that needs to be deployed.


> >
> >  dist_tools_SCRIPTS = \
> >         tools/kvm-console-wrapper \
> > diff --git a/src/Ganeti/Confd/Server.hs b/src/Ganeti/Confd/Server.hs
> > index 855d1fc..e59d5bc 100644
> > --- a/src/Ganeti/Confd/Server.hs
> > +++ b/src/Ganeti/Confd/Server.hs
> > @@ -233,7 +233,8 @@ buildResponse cdata req@(ConfdRequest { confdRqType
> = ReqInstanceDisks }) = do
> >      Bad e -> fail $ "Could not retrieve disks: " ++ show e
> >
> >  -- | Return arbitrary configuration value given by a path.
> > -buildResponse cdata req@(ConfdRequest { confdRqType = ReqConfigQuery,
> confdRqQuery = pathQ }) = do
> > +buildResponse cdata req@(ConfdRequest { confdRqType = ReqConfigQuery
> > +                                      , confdRqQuery = pathQ }) = do
>
> Include this in the previous patch.
>
> >    let cfg = fst cdata
> >    path <-
> >      case pathQ of
> > diff --git a/src/Ganeti/Confd/Utils.hs b/src/Ganeti/Confd/Utils.hs
> > index b09340b..44151de 100644
> > --- a/src/Ganeti/Confd/Utils.hs
> > +++ b/src/Ganeti/Confd/Utils.hs
> > @@ -127,7 +127,8 @@ pointerFromString s = either J.Error J.Ok $
> P.parseOnly parser $ pack s
> >        tokens <-  token `P.manyTill` P.endOfInput
> >        return $ Pointer tokens
> >      token = do
> > -      P.char '/' *> (P.many' $ P.choice [escaped, P.satisfy $
> P.notInClass "~/"])
> > +      P.char '/' *> (P.many' $ P.choice [ escaped
> > +                                        , P.satisfy $ P.notInClass
> "~/"])
>
> This too.
>

Done.


> >      escaped = P.choice [escapedSlash, escapedTilde]
> >      escapedSlash = P.string (pack "~1") *> return '/'
> >      escapedTilde = P.string (pack "~0") *> return '~'
> > diff --git a/tools/query-config b/tools/query-config
> > new file mode 100755
> > index 0000000..68e2b81
> > --- /dev/null
> > +++ b/tools/query-config
> > @@ -0,0 +1,145 @@
> > +#!/usr/bin/python
> > +#
> > +
> > +# Copyright (C) 2014 Google Inc.
> > +#
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 2 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful, but
> > +# WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +# General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program; if not, write to the Free Software
> > +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > +# 02110-1301, USA.
> > +
> > +
> > +"""Tool to query the cluster configuration over RConfD
> > +
> > +"""
> > +
> > +# functions in this module need to have a given name structure, so:
> > +# pylint: disable=C0103
> What is causing you to do this? Maybe a more informative comment would
> help.
>
> > +
> > +
> > +import optparse
> > +import sys
> > +
> > +from ganeti import constants
> > +from ganeti import cli
> > +from ganeti import utils
> > +from ganeti import pathutils
> > +
> > +from ganeti.confd import client as confd_client
> > +
> > +USAGE = ("\tquery-config [--addr=host] [--hmac=key] QUERY [QUERY...]")
> > +
> > +OPTIONS = [
> > +  cli.cli_option("--hmac", dest="hmac", default=None,
> > +                 help="Specify HMAC key instead of reading"
> > +                 " it from the filesystem",
> > +                 metavar="<KEY>"),
> > +  cli.cli_option("-a", "--address", dest="mc", default="127.0.0.1",
> > +                 help="Server IP to query (default: 127.0.0.1)",
> > +                 metavar="<ADDRESS>"),
> > +  cli.cli_option("-r", "--requests", dest="requests", default=100,
> > +                 help="Number of requests for the timing tests",
> > +                 type="int", metavar="<REQUESTS>"),
> Is this option actually being used somewhere?


Um, no...

-  cli.cli_option("-r", "--requests", dest="requests", default=100,
-                 help="Number of requests for the timing tests",
-                 type="int", metavar="<REQUESTS>"),



>
> > +  ]
> > +
> > +
> > +def Err(msg, exit_code=1):
> > +  """Simple error logging that prints to stderr.
> > +
> > +  """
> > +  sys.stderr.write(msg + "\n")
> > +  sys.stderr.flush()
> > +  sys.exit(exit_code)
> > +
> > +
> > +def Usage():
> > +  """Shows program usage information and exits the program."""
> > +
> > +  print >> sys.stderr, "Usage:"
> > +  print >> sys.stderr, USAGE
> > +  sys.exit(2)
> > +
> > +
> > +class QueryClient(object):
> > +  """Confd client for querying the configuration JSON."""
>
> An empty line at the end of the comment is missing, here and in the other
> docstrings.
>

Done.

>  > +  def __init__(self):
> > +    """Constructor."""
> > +    self.opts = None
> > +    self.cluster_master = None
> > +    self.instance_ips = None
> > +    self.is_timing = False
> > +    self.ParseOptions()
> > +
> > +  def ParseOptions(self):
> > +    """Parses the command line options.
> > +
> > +    In case of command line errors, it will show the usage and exit the
> > +    program.
> > +
> > +    @return: a tuple (options, args), as returned by
> OptionParser.parse_args
> > +
> > +    """
> > +    parser = optparse.OptionParser(usage="\n%s" % USAGE,
> > +                                   version=("%%prog (ganeti) %s" %
> > +                                            constants.RELEASE_VERSION),
> > +                                   option_list=OPTIONS)
> > +
> > +    options, args = parser.parse_args()
> > +    if args == []:
> > +      Usage()
> > +
> > +    self.paths = args
> > +
> > +    if options.hmac is None:
> > +      options.hmac = utils.ReadFile(pathutils.CONFD_HMAC_KEY)
> > +
> > +    self.hmac_key = options.hmac
> > +
> > +    self.mc_list = [options.mc]
> > +
> > +    self.opts = options
> > +
> > +  def Run(self):
> > +    self.store_callback = confd_client.StoreResultCallback()
> > +
> > +    self.confd_client = confd_client.ConfdClient(self.hmac_key,
> > +                                      self.mc_list,
> > +                                      self.store_callback)
> > +
> > +    responses = []
> > +    for path in self.paths:
> > +      req = confd_client.ConfdClientRequest(
> > +        type=constants.CONFD_REQ_CONFIG_QUERY, query=path)
> > +      _, response = self.DoConfdRequestReply(req)
> > +      responses.append(str(response.server_reply.answer))
> > +    table = zip(self.paths, responses)
> > +    longest_path = max(len(p) for p in self.paths)
> > +    for p, a in table:
> > +      print "%s\t%s" % (p.ljust(longest_path), a)
> > +
> > +  def DoConfdRequestReply(self, req):
> > +    """Send request to Confd and await all responses."""
> > +    self.confd_client.SendRequest(req, async=False)
> > +    if not self.confd_client.ReceiveReply():
> > +      Err("Did not receive all expected confd replies")
> > +    return self.store_callback.GetResponse(req.rsalt)
> > +
> > +def main():
> > +  """Application entry point.
> > +
> > +  """
> > +  QueryClient().Run()
> > +
> > +
> > +if __name__ == "__main__":
> > +  main()
> > --
> > 2.0.0.526.g5318336
> >
>
> Thanks,
> Michele
>

Reply via email to