LGTM On Tue, Sep 29, 2015 at 5:21 PM 'Hrvoje Ribicic' via ganeti-devel < [email protected]> wrote:
> Since the QA RAPI code already uses the horror of global variables to > save the username and password within the qa_rapi module, the code can > be refactored to make the storage of these values outside the module > unnecessary. This encapsulates the RAPI functionality better, and will > allow for easier refactoring in later commits. > > Signed-off-by: Hrvoje Ribicic <[email protected]> > --- > qa/ganeti-qa.py | 40 ++++++--------------------- > qa/qa_cluster.py | 24 ++-------------- > qa/qa_rapi.py | 79 > +++++++++++++++++++++++++++++++++++++++++++++-------- > qa/rapi-workload.py | 4 +-- > 4 files changed, 81 insertions(+), 66 deletions(-) > > diff --git a/qa/ganeti-qa.py b/qa/ganeti-qa.py > index 1d168cf..92ab27f 100755 > --- a/qa/ganeti-qa.py > +++ b/qa/ganeti-qa.py > @@ -186,23 +186,18 @@ def RunEnvTests(): > RunTestIf("env", qa_env.TestGanetiCommands) > > > -def SetupCluster(rapi_user): > +def SetupCluster(): > """Initializes the cluster. > > - @param rapi_user: Login user for RAPI > - @return: Login secret for RAPI > - > """ > - rapi_secret = utils.GenerateSecret() > - RunTestIf("create-cluster", qa_cluster.TestClusterInit, > - rapi_user, rapi_secret) > + > + RunTestIf("create-cluster", qa_cluster.TestClusterInit) > if not qa_config.TestEnabled("create-cluster"): > # If the cluster is already in place, we assume that > exclusive-storage is > # already set according to the configuration > qa_config.SetExclusiveStorage(qa_config.get("exclusive-storage", > False)) > - if qa_rapi.Enabled(): > - # To support RAPI on an existing cluster we have to find out the > secret > - rapi_secret = qa_rapi.LookupRapiSecret(rapi_user) > + > + qa_rapi.SetupRapi() > > qa_group.ConfigureGroups() > > @@ -232,17 +227,10 @@ def SetupCluster(rapi_user): > > RunTestIf("node-info", qa_node.TestNodeInfo) > > - return rapi_secret > > - > -def RunClusterTests(rapi_user=None, rapi_secret=None): > +def RunClusterTests(): > """Runs tests related to gnt-cluster. > > - @type rapi_user: string > - @param rapi_user: name of the rapi user > - @type rapi_secret: string > - @param rapi_secret: the rapi secret > - > """ > for test, fn in [ > ("create-cluster", qa_cluster.TestClusterInitDisk), > @@ -252,11 +240,7 @@ def RunClusterTests(rapi_user=None, rapi_secret=None): > > # Since renew-crypto replaces the RAPI cert, reload it. > if qa_rapi.Enabled(): > - if not rapi_user: > - raise qa_error.Error("No RAPI user given.") > - if not rapi_secret: > - raise qa_error.Error("No RAPI secret given.") > - qa_rapi.Setup(rapi_user, rapi_secret) > + qa_rapi.ReloadCertificates() > > for test, fn in [ > ("cluster-verify", qa_cluster.TestClusterVerify), > @@ -934,16 +918,10 @@ def RunQa(): > """Main QA body. > > """ > - rapi_user = "ganeti-qa" > - > RunTestBlock(RunEnvTests) > - rapi_secret = SetupCluster(rapi_user) > + SetupCluster() > > - if qa_rapi.Enabled(): > - # Load RAPI certificate > - qa_rapi.Setup(rapi_user, rapi_secret) > - > - RunTestBlock(RunClusterTests, rapi_user=rapi_user, > rapi_secret=rapi_secret) > + RunTestBlock(RunClusterTests) > RunTestBlock(RunOsTests) > > RunTestIf("tags", qa_tags.TestClusterTags) > diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py > index 138b5f9..cf4a08e 100644 > --- a/qa/qa_cluster.py > +++ b/qa/qa_cluster.py > @@ -35,7 +35,6 @@ > import re > import tempfile > import time > -import os.path > > from ganeti import _constants > from ganeti import constants > @@ -182,28 +181,8 @@ def TestClusterInitDisk(): > AssertCommand(["gnt-cluster", "init", "-D", param, name], fail=True) > > > -def TestClusterInit(rapi_user, rapi_secret): > +def TestClusterInit(): > """gnt-cluster init""" > - master = qa_config.GetMasterNode() > - > - rapi_users_path = qa_utils.MakeNodePath(master, > pathutils.RAPI_USERS_FILE) > - rapi_dir = os.path.dirname(rapi_users_path) > - > - # First create the RAPI credentials > - fh = tempfile.NamedTemporaryFile() > - try: > - fh.write("%s %s write\n" % (rapi_user, rapi_secret)) > - fh.flush() > - > - tmpru = qa_utils.UploadFile(master.primary, fh.name) > - try: > - AssertCommand(["mkdir", "-p", rapi_dir]) > - AssertCommand(["mv", tmpru, rapi_users_path]) > - finally: > - AssertCommand(["rm", "-f", tmpru]) > - finally: > - fh.close() > - > # Initialize cluster > enabled_disk_templates = qa_config.GetEnabledDiskTemplates() > cmd = [ > @@ -230,6 +209,7 @@ def TestClusterInit(rapi_user, rapi_secret): > if spec_values: > cmd.append("--specs-%s=%s" % (spec_type, ",".join(spec_values))) > > + master = qa_config.GetMasterNode() > if master.secondary: > cmd.append("--secondary-ip=%s" % master.secondary) > > diff --git a/qa/qa_rapi.py b/qa/qa_rapi.py > index 543b37a..9938333 100644 > --- a/qa/qa_rapi.py > +++ b/qa/qa_rapi.py > @@ -34,6 +34,7 @@ > > import functools > import itertools > +import os.path > import random > import re > import tempfile > @@ -64,7 +65,8 @@ from qa_instance import IsDiskReplacingSupported > from qa_instance import IsFailoverSupported > from qa_instance import IsMigrationSupported > from qa_job_utils import RunWithLocks > -from qa_utils import (AssertEqual, AssertIn, AssertMatch, > StartLocalCommand) > +from qa_utils import (AssertEqual, AssertIn, AssertMatch, AssertCommand, > + StartLocalCommand) > from qa_utils import InstanceCheck, INST_DOWN, INST_UP, FIRST_ARG > > > @@ -74,19 +76,18 @@ _rapi_username = None > _rapi_password = None > > > -def Setup(username, password): > - """Configures the RAPI client. > +def ReloadCertificates(): > + """Reloads the client RAPI certificate with the one present on the node. > > """ > + if _rapi_username is None or _rapi_password is None: > + raise qa_error.Error("RAPI username and password have to be set > before" > + " attempting to reload a certificate.") > + > # pylint: disable=W0603 > # due to global usage > global _rapi_ca > global _rapi_client > - global _rapi_username > - global _rapi_password > - > - _rapi_username = username > - _rapi_password = password > > master = qa_config.GetMasterNode() > > @@ -111,16 +112,46 @@ def Setup(username, password): > assert _rapi_client is None > else: > _rapi_client = rapi.client.GanetiRapiClient(master.primary, port=port, > - username=username, > - password=password, > + username=_rapi_username, > + password=_rapi_password, > curl_config_fn=cfg_curl) > > print "RAPI protocol version: %s" % _rapi_client.GetVersion() > > + > +#TODO(riba): Remove in 2.13, used just by rapi-workload which disappears > there > +def GetClient(): > + """Retrieves the RAPI client prepared by this module. > + > + """ > return _rapi_client > > > -def LookupRapiSecret(rapi_user): > +def _CreateRapiUser(rapi_user, rapi_secret): > + """RAPI credentials creation. > + > + """ > + master = qa_config.GetMasterNode() > + > + rapi_users_path = qa_utils.MakeNodePath(master, > pathutils.RAPI_USERS_FILE) > + rapi_dir = os.path.dirname(rapi_users_path) > + > + fh = tempfile.NamedTemporaryFile() > + try: > + fh.write("%s %s write\n" % (rapi_user, rapi_secret)) > + fh.flush() > + > + tmpru = qa_utils.UploadFile(master.primary, fh.name) > + try: > + AssertCommand(["mkdir", "-p", rapi_dir]) > + AssertCommand(["mv", tmpru, rapi_users_path]) > + finally: > + AssertCommand(["rm", "-f", tmpru]) > + finally: > + fh.close() > + > + > +def _LookupRapiSecret(rapi_user): > """Find the RAPI secret for the given user. > > @param rapi_user: Login user > @@ -145,6 +176,32 @@ def LookupRapiSecret(rapi_user): > return secret > > > +def SetupRapi(): > + """Sets up the RAPI certificate and usernames for the client. > + > + """ > + if not Enabled(): > + return (None, None) > + > + # pylint: disable=W0603 > + # due to global usage > + global _rapi_username > + global _rapi_password > + > + _rapi_username = "ganeti-qa" > + if qa_config.TestEnabled("create-cluster"): > + # For a new cluster, we have to invent a secret and a user > + _rapi_password = utils.GenerateSecret() > + _CreateRapiUser(_rapi_username, _rapi_password) > + else: > + # On an existing cluster, just find out the user's secret > + _rapi_password = _LookupRapiSecret(_rapi_username) > + > + # Once a username and password have been set, we can fetch the certs and > + # get all we need for a working RAPI client. > + ReloadCertificates() > + > + > INSTANCE_FIELDS = ("name", "os", "pnode", "snodes", > "admin_state", > "disk_template", "disk.sizes", "disk.spindles", > diff --git a/qa/rapi-workload.py b/qa/rapi-workload.py > index b286336..958af4a 100755 > --- a/qa/rapi-workload.py > +++ b/qa/rapi-workload.py > @@ -90,8 +90,8 @@ class GanetiRapiClientWrapper(object): > > """ > def __init__(self): > - self._client = qa_rapi.Setup(RAPI_USERNAME, > - qa_rapi.LookupRapiSecret(RAPI_USERNAME)) > + qa_rapi.SetupRapi() > + self._client = qa_rapi.GetClient() > > self._method_invocations = {} > > -- > 2.6.0.rc2.230.g3dd15c0 > >
