Interdiff with correct patch:
diff --git a/qa/ganeti-qa.py b/qa/ganeti-qa.py
index e537b70..885aa66 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)
# Test removing instance with offline drbd secondary
if qa_config.TestEnabled(["instance-remove-drbd-offline",
diff --git a/qa/qa_instance.py b/qa/qa_instance.py
index 0321da9..0514517 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,24 +1568,7 @@ def TestInstanceAddOsParams(nodes):
TestInstanceRemove(instance)
instance.Release()
-
-
-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"])
+ pnode.Release()
On Tue, Jul 7, 2015 at 5:33 PM, Lisa Velden <[email protected]> wrote:
> 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
>
--
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
