Interdiff with gnt-job wait:
diff --git a/qa/qa_instance.py b/qa/qa_instance.py
index 0514517..eda27ab 100644
--- a/qa/qa_instance.py
+++ b/qa/qa_instance.py
@@ -32,7 +32,6 @@
"""
-import operator
import os
import re
import time
@@ -49,7 +48,7 @@ import qa_utils
import qa_error
from qa_filters import stdout_of
-from qa_utils import AssertCommand, AssertEqual, AssertIn, _GetName,
StartSSH
+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, \
CreateInstanceDrbd8, \
@@ -1524,13 +1523,8 @@ def _TestRedactionOfSecretOsParams(cmd, secret_keys):
"""
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)
+ "--submit", "300"]))
+ cmd_jid = int(stdout_of(cmd))
job_file = "/var/lib/ganeti/queue/job-%s" % cmd_jid
for k in secret_keys:
@@ -1540,7 +1534,7 @@ def _TestRedactionOfSecretOsParams(cmd, secret_keys):
AssertCommand(["gnt-job", "cancel", "--kill", "--yes-do-it",
str(debug_delay_id)])
AssertCommand(["gnt-cluster", "modify", "--max-running-jobs", "20"])
- popen.wait()
+ AssertCommand(["gnt-job", "wait", str(cmd_jid)])
def TestInstanceAddOsParams():
On Tue, Jul 7, 2015 at 6:23 PM, Lisa Velden <[email protected]> wrote:
> 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
>
--
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
