SI says [1]:
Often only one *test subject* is passed in. However multiple subjects may
be concatenated together in a shell escaped string. The playbook and inventory script split the string.
It does not say what the separator character is. It might be a space, it might be any whitespace, it might be a comma, it might be a semicolon, etc. This should be clarified.
I'm also confused what "shell escaped" means here. It's not like we'll pipe this to bash, we'll process this with ansible. And even if we did, you still want the raw value, not the escaped one, as the variable, don't you? E.g. for "foo" and "bar" subjects, you want this:
$ export TEST_SUBJECTS='foo bar' $ echo $TEST_SUBJECTS foo bar
and not this:
$ export TEST_SUBJECTS='foo\ bar' $ echo $TEST_SUBJECTS foo\ bar
Correct?
But that brings another question - what if I have the separator character inside the subject name? You might say that's not likely with current fedora artfifacts, but a standard interface definition should be forward-looking. Even now it is technically possible to have a space inside an .iso/.qcow2 image name. It might be even more common for some future test subject type. The spec should describe this as well.
Personally, I think we should get away from string parsing, if we want to design a reliable system. String parsing and especially shell escaping is a never ending story of unexpected failures and corner case issues. It would be better to use a proper structure for this.
As it turns out, ansible can do this already [2]. You can provide a true list to it via --extra-vars, by passing in a json, like this:
$ ansible-playbook tests.yml --extra-vars '{"subjects": ["foo","bar","even a space"]}'
This way you don't need to do string parsing in an ansible playbook, you can immediately use its native structures, like
module: do something for {{item}} with_items: "{{subjects}}"
You also don't need to deal with character escaping (especially shell one), all of that is handled by json. This makes writing tests.yml a lot easier, I think.
There's of course a question what to do with TEST_SUBJECTS envvar if we make "subjects" a native ansible variable. I'd personally use json for it as well, so:
$ export TEST_SUBJECTS='["foo", "bar", "even a space"]'
There's a json parser for every possible language, and there are convenient tools for bash as well [3].
But maybe I'd like to first understand why we require the envvars to be present in the first place. I don't think they're needed. Is it just a convenience measure?
Thanks, Kamil
[1] https://fedoraproject.org/wiki/Changes/InvokingTests#Invocation [2] http://docs.ansible.com/ansible/latest/playbooks_variables.html#passing-vari... [3] https://stedolan.github.io/jq/
On 01.09.2017 15:19, Kamil Paral wrote:
SI says [1]:
Often only one /test subject/ is passed in. However multiple subjects may be concatenated together in a shell escaped string. The playbook and inventory script split the string.
It does not say what the separator character is. It might be a space, it might be any whitespace, it might be a comma, it might be a semicolon, etc. This should be clarified.
I'm also confused what "shell escaped" means here. It's not like we'll pipe this to bash, we'll process this with ansible. And even if we did, you still want the raw value, not the escaped one, as the variable, don't you? E.g. for "foo" and "bar" subjects, you want this:
$ export TEST_SUBJECTS='foo bar' $ echo $TEST_SUBJECTS foo bar
and not this:
$ export TEST_SUBJECTS='foo\ bar' $ echo $TEST_SUBJECTS foo\ bar
Correct?
But that brings another question - what if I have the separator character inside the subject name? You might say that's not likely with current fedora artfifacts, but a standard interface definition should be forward-looking. Even now it is technically possible to have a space inside an .iso/.qcow2 image name. It might be even more common for some future test subject type. The spec should describe this as well.
Personally, I think we should get away from string parsing, if we want to design a reliable system. String parsing and especially shell escaping is a never ending story of unexpected failures and corner case issues. It would be better to use a proper structure for this.
As it turns out, ansible can do this already [2]. You can provide a true list to it via --extra-vars, by passing in a json, like this:
$ ansible-playbook tests.yml --extra-vars '{"subjects": ["foo","bar","even a space"]}'
This way you don't need to do string parsing in an ansible playbook, you can immediately use its native structures, like
module: do something for {{item}} with_items: "{{subjects}}"
You also don't need to deal with character escaping (especially shell one), all of that is handled by json. This makes writing tests.yml a lot easier, I think.
There's of course a question what to do with TEST_SUBJECTS envvar if we make "subjects" a native ansible variable. I'd personally use json for it as well, so:
$ export TEST_SUBJECTS='["foo", "bar", "even a space"]'
There's a json parser for every possible language, and there are convenient tools for bash as well [3].
Another alternative:
* Stop supporting multiple subjects per run in general. Only support multiple subjects where they make sense, such as multiple RPM file names.
What do you think?
But maybe I'd like to first understand why we require the envvars to be present in the first place. I don't think they're needed. Is it just a convenience measure?
I wish that were the case. The sad reality is that inventory scripts do not have access to the ansible environment variables.
If you've found a way that they do, then we could indeed move away from environment variables.
Stef
ci@lists.stg.fedoraproject.org