Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass environment variables through task execution #69

Merged
merged 6 commits into from
Jul 27, 2021
Merged

Pass environment variables through task execution #69

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0815d34
Pass environment variables through task execution
gravesm Jul 14, 2021
82943d8
Add changelog fragment
gravesm Jul 14, 2021
fd02075
Always reset environment
gravesm Jul 22, 2021
272b1c8
Fix sanity
gravesm Jul 22, 2021
ac79eed
Merge remote-tracking branch 'upstream/main' into pass-envvars
gravesm Jul 23, 2021
a30efff
Add lock for environment
gravesm Jul 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelogs/fragments/69-pass-envvar.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---
bugfixes:
- pass current task's environment through to execution (https://github.com/ansible-collections/cloud.common/pull/69).
1 change: 1 addition & 0 deletions plugins/module_utils/turbo/module.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ def run_on_daemon(self):
data = [
ansiblez_path,
json.dumps(args),
dict(os.environ),
]
content = json.dumps(data).encode()
result = turbo_socket.communicate(content)
Expand Down
3 changes: 3 additions & 0 deletions plugins/module_utils/turbo/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ def _terminate(result):
(
ansiblez_path,
params,
env,
) = json.loads(content)
if self.debug_mode:
print( # pylint: disable=ansible-bad-function
Expand All @@ -272,6 +273,8 @@ def _terminate(result):

await embedded_module.load()
try:
os.environ.clear()
os.environ.update(env)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.environ is a global variable. We can get two concurrent async execution of embedded_module. In this case, the second runs would overwrite the environment for the first calll. We should use a lock like in load() to protect to block.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e.g:

#!/usr/bin/env python3

import asyncio
import os

async def my_async_loop(expectation):
    for _ in range(100):
        await asyncio.sleep(0.1)
        assert os.environ["baba"] == expectation

async def main():
    os.environ["baba"] = "loop1"
    l1 = my_async_loop("loop1")
    os.environ["blabla"] = "loop2"
    l2 = my_async_loop("loop2")
    await l2
    await l1

asyncio.run(main())

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a lock. I'll also mention two things. The EmbeddedModule.run() method needed a lock before my changes since it's modifying sys.stdin and then (possibly) awaiting the module. Also, unless I'm missing something, I don't think the lock in the load() or unload() methods is doing anything. It's not hurting anything to be there, but it's also not necessary. There's nowhere in either of those functions that a task switch can occur.

result = await embedded_module.run()
except SystemExit:
backtrace = traceback.format_exc()
Expand Down
1 change: 1 addition & 0 deletions plugins/modules/turbo_demo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ def run_module():
result["changed"] = True
result["message"] = get_message()
result["counter"] = counter.i
result["envvar"] = os.environ.get("TURBO_TEST_VAR")

if module._diff:
result["diff"] = {"before": previous_value, "after": counter.i}
Expand Down
18 changes: 18 additions & 0 deletions tests/integration/targets/turbo_mode/tasks/main.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,21 @@
- assert:
that:
- _result_no_diff.diff is undefined

- name: Test task environment var
cloud.common.turbo_demo:
environment:
TURBO_TEST_VAR: foobar
register: _result

- assert:
that:
- _result.envvar == "foobar"

- name: Test task environment var not set
cloud.common.turbo_demo:
register: _result

- assert:
that:
- not _result.envvar
2 changes: 1 addition & 1 deletion tests/sanity/ignore-2.10.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,4 @@ plugins/module_utils/turbo/common.py future-import-boilerplate!skip
plugins/module_utils/turbo/common.py import-2.6!skip
plugins/module_utils/turbo/common.py import-2.7!skip
plugins/module_utils/turbo/common.py import-3.5!skip
plugins/module_utils/turbo/common.py metaclass-boilerplate!skip
plugins/module_utils/turbo/common.py metaclass-boilerplate!skip
2 changes: 1 addition & 1 deletion tests/sanity/ignore-2.11.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,4 @@ plugins/module_utils/turbo/common.py future-import-boilerplate!skip
plugins/module_utils/turbo/common.py import-2.6!skip
plugins/module_utils/turbo/common.py import-2.7!skip
plugins/module_utils/turbo/common.py import-3.5!skip
plugins/module_utils/turbo/common.py metaclass-boilerplate!skip
plugins/module_utils/turbo/common.py metaclass-boilerplate!skip
2 changes: 1 addition & 1 deletion tests/sanity/ignore-2.9.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,4 @@ plugins/module_utils/turbo/common.py future-import-boilerplate!skip
plugins/module_utils/turbo/common.py import-2.6!skip
plugins/module_utils/turbo/common.py import-2.7!skip
plugins/module_utils/turbo/common.py import-3.5!skip
plugins/module_utils/turbo/common.py metaclass-boilerplate!skip
plugins/module_utils/turbo/common.py metaclass-boilerplate!skip