On Mon, Jul 6, 2015 at 10:20 PM, 'Lisa Velden' via ganeti-devel <
[email protected]> wrote:

> Test if secret parameter values for instance create jobs are
> redacted in job files.
>
> Signed-off-by: Lisa Velden <[email protected]>
> ---
>  qa/ganeti-qa.py   |  5 +++++
>  qa/qa_instance.py | 41 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+)
>
> diff --git a/qa/ganeti-qa.py b/qa/ganeti-qa.py
> index 8572cd3..e537b70 100755
> --- a/qa/ganeti-qa.py
> +++ b/qa/ganeti-qa.py
> @@ -1064,6 +1064,11 @@ def RunQa():
>      "instance-add-restricted-by-disktemplates",
>      qa_instance.TestInstanceCreationRestrictedByDiskTemplates)
>
> +  pnode = qa_config.AcquireNode()
> +  RunTestIf("instance-add-osparams", qa_instance.TestInstanceAddOsParams,
> +            [pnode])
> +  pnode.Release()
>

While some tests acquire a node in this file, you can move this acquisition
of the node to the function - it will make the code more readable.


> +
>    # Test removing instance with offline drbd secondary
>    if qa_config.TestEnabled(["instance-remove-drbd-offline",
>                              "instance-add-drbd-disk"]):
> diff --git a/qa/qa_instance.py b/qa/qa_instance.py
> index 3355f77..0321da9 100644
> --- a/qa/qa_instance.py
> +++ b/qa/qa_instance.py
> @@ -47,6 +47,7 @@ import qa_daemon
>  import qa_utils
>  import qa_error
>
> +from qa_filters import stdout_of
>  from qa_utils import AssertCommand, AssertEqual, AssertIn
>  from qa_utils import InstanceCheck, INST_DOWN, INST_UP, FIRST_ARG,
> RETURN_VALUE
>  from qa_instance_utils import CheckSsconfInstanceList, \
> @@ -1516,6 +1517,46 @@ def TestInstanceCommunication(instance, master):
>    print result_output
>
>
> +def TestInstanceAddOsParams(nodes):
> +  """Tests instance add with secret os parameters"""
> +
>

Please test if the plain template is supported with IsTemplateSupported,
and exit if not.


> +  instance = qa_config.AcquireInstance()
> +  secret_keys = ["param1", "param2"]
> +  cmd = (["gnt-instance", "add",
> +          "--os-type=%s" % qa_config.get("os"),
> +          "--disk-template=%s" % constants.DT_PLAIN,
> +          "--os-parameters-secret",
> +          "param1=secret1,param2=secret2",
> +          "--node=%s" % nodes[0].primary] +
> +          GetGenericAddParameters(instance, constants.DT_PLAIN))
> +  cmd.append("--submit")
> +  cmd.append("--print-jobid")
> +  cmd.append(instance.name)
> +
> +  _TestRedactionOfSecretOsParams(cmd, secret_keys)
> +
> +  TestInstanceRemove(instance)
> +  instance.Release()
> +
> +
> +def _TestRedactionOfSecretOsParams(cmd, secret_keys):
>

We usually put helper functions before the functions which use them.


> +  """Tests redaction of secret os parameters"""
>

This docstring style is only used for functions invoked with "RunTestIf",
because it allows us to fetch the description via introspection.

For helper functions, use the usual docstring style.


> +
> +  AssertCommand(["gnt-cluster", "modify", "--max-running-jobs", "1"])
> +  debug_delay_id = int(stdout_of(["gnt-debug", "delay", "--interruptible",
>

If you're using the --kill option, you can omit the --interruptible flag -
it was a hack for a time before we could kill jobs.


> +                       "--print-jobid", "--submit", "300"]))
> +  cmd_jid = int(stdout_of(cmd))
> +  job_file = "/var/lib/ganeti/queue/job-%s" % cmd_jid
> +
> +  for k in secret_keys:
> +    grep_cmd = ["grep", "\"%s\":\"<redacted>\"" % k, job_file]
> +    AssertCommand(grep_cmd)
> +
> +  AssertCommand(["gnt-job", "cancel", "--kill", "--yes-do-it",
> +                str(debug_delay_id)])
> +  AssertCommand(["gnt-cluster", "modify", "--max-running-jobs", "20"])
>

After you have done this, the original job is going to start running, and
it would be wise to wait for it to complete before exiting the function.


> +
> +
>  available_instance_tests = [
>    ("instance-add-plain-disk", constants.DT_PLAIN,
>     TestInstanceAddWithPlainDisk, 1),
> --
> 2.4.3.573.g4eafbef
>
>
Hrvoje Ribicic
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München

Geschäftsführer: Graham Law, Christine Elizabeth Flores
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Reply via email to