Interdiff:
diff --git a/qa/ganeti-qa.py b/qa/ganeti-qa.py
index bc6dd52..77e2cce 100755
--- a/qa/ganeti-qa.py
+++ b/qa/ganeti-qa.py
@@ -1064,10 +1064,7 @@ def RunQa():
"instance-add-restricted-by-disktemplates",
qa_instance.TestInstanceCreationRestrictedByDiskTemplates)
- pnode = qa_config.AcquireNode()
- RunTestIf("instance-add-osparams", qa_instance.TestInstanceAddOsParams,
- [pnode])
- pnode.Release()
+ RunTestIf("instance-add-osparams", qa_instance.TestInstanceAddOsParams)
RunTestIf("instance-add-osparams", qa_instance.TestOsParams)
# Test removing instance with offline drbd secondary
diff --git a/qa/qa_instance.py b/qa/qa_instance.py
index 91d4c78..e45fe76 100644
--- a/qa/qa_instance.py
+++ b/qa/qa_instance.py
@@ -32,6 +32,7 @@
"""
+import operator
import os
import re
import time
@@ -48,7 +49,7 @@ import qa_utils
import qa_error
from qa_filters import stdout_of
-from qa_utils import AssertCommand, AssertEqual, AssertIn
+from qa_utils import AssertCommand, AssertEqual, AssertIn, _GetName,
StartSSH
from qa_utils import InstanceCheck, INST_DOWN, INST_UP, FIRST_ARG,
RETURN_VALUE
from qa_instance_utils import CheckSsconfInstanceList, \
CreateInstanceDrbd8, \
@@ -1517,17 +1518,47 @@ def TestInstanceCommunication(instance, master):
print result_output
-def TestInstanceAddOsParams(nodes):
+def _TestRedactionOfSecretOsParams(cmd, secret_keys):
+ """Tests redaction of secret os parameters
+
+ """
+ AssertCommand(["gnt-cluster", "modify", "--max-running-jobs", "1"])
+ debug_delay_id = int(stdout_of(["gnt-debug", "delay", "--print-jobid",
+ "--submit", "300"]))
+
+ nodename = _GetName(qa_config.GetMasterNode(),
operator.attrgetter("primary"))
+ popen = StartSSH(nodename, utils.ShellQuoteArgs(cmd), log_cmd=False)
+
+ stdout, _ = popen.communicate()
+ cmd_jid = int(stdout)
+ 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"])
+ popen.wait()
+
+
+def TestInstanceAddOsParams():
"""Tests instance add with secret os parameters"""
+ if not qa_config.IsTemplateSupported(constants.DT_PLAIN):
+ return
+
+ pnode = qa_config.AcquireNode()
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] +
+ "--node=%s" % pnode.primary] +
GetGenericAddParameters(instance, constants.DT_PLAIN))
cmd.append("--submit")
cmd.append("--print-jobid")
@@ -1537,6 +1568,7 @@ def TestInstanceAddOsParams(nodes):
TestInstanceRemove(instance)
instance.Release()
+ pnode.Release()
def TestOsParams():
@@ -1552,24 +1584,6 @@ def TestOsParams():
AssertIn("\'param2\': \'secret2\'", cmd_output)
-def _TestRedactionOfSecretOsParams(cmd, secret_keys):
- """Tests redaction of secret os parameters"""
-
- AssertCommand(["gnt-cluster", "modify", "--max-running-jobs", "1"])
- debug_delay_id = int(stdout_of(["gnt-debug", "delay", "--interruptible",
- "--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"])
-
-
available_instance_tests = [
("instance-add-plain-disk", constants.DT_PLAIN,
TestInstanceAddWithPlainDisk, 1),
On Tue, Jul 7, 2015 at 12:23 PM, Hrvoje Ribicic <[email protected]> wrote:
>
>
> 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
>
--
Lisa Velden
Software Engineer
[email protected]
Google Germany GmbH
Dienerstraße 12
80331 München
Geschäftsführer: Graham Law, Christine Elizabeth Flores
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
