diff --git a/pyinfra/api/command.py b/pyinfra/api/command.py index a45e79f6a..1f2f6cbc2 100644 --- a/pyinfra/api/command.py +++ b/pyinfra/api/command.py @@ -45,6 +45,9 @@ def make_formatted_string_command(string: str, *args, **kwargs): return StringCommand(*string_bits) +EvalOperationAtExecution = object() + + class MaskString(str): pass diff --git a/pyinfra/api/operation.py b/pyinfra/api/operation.py index e4ce11882..cc0252292 100644 --- a/pyinfra/api/operation.py +++ b/pyinfra/api/operation.py @@ -14,7 +14,7 @@ from pyinfra.context import ctx_host, ctx_state from .arguments import get_execution_kwarg_keys, pop_global_arguments -from .command import StringCommand +from .command import EvalOperationAtExecution, FunctionCommand, StringCommand from .exceptions import OperationValueError, PyinfraError from .host import Host from .operations import run_host_op @@ -209,16 +209,19 @@ def decorated_func(*args, **kwargs): host.current_op_global_kwargs = global_kwargs # Convert to list as the result may be a generator - commands = func(*args, **kwargs) commands = [ # convert any strings -> StringCommand's StringCommand(command.strip()) if isinstance(command, str) else command - for command in commands + for command in func(*args, **kwargs) ] host.in_op = False host.current_op_hash = None host.current_op_global_kwargs = None + if EvalOperationAtExecution in commands: + logger.warning("Defering operation evaluation until execution: %s", op_meta["names"]) + commands = [FunctionCommand(operation(func), args, kwargs)] + # Add host-specific operation data to state, this mutates state operation_meta = _update_state_meta(state, host, commands, op_hash, op_meta, global_kwargs) diff --git a/pyinfra/api/operations.py b/pyinfra/api/operations.py index ce137cee4..749929c62 100644 --- a/pyinfra/api/operations.py +++ b/pyinfra/api/operations.py @@ -9,7 +9,7 @@ import pyinfra from pyinfra import logger -from pyinfra.context import ctx_host +from pyinfra.context import ctx_host, ctx_state from pyinfra.progress import progress_spinner from .arguments import get_executor_kwarg_keys @@ -115,7 +115,6 @@ def run_condition(condition_name: str) -> bool: all_combined_output_lines = [] for i, command in enumerate(op_data["commands"]): - status = False executor_kwargs = base_executor_kwargs.copy() @@ -217,8 +216,9 @@ def run_condition(condition_name: str) -> bool: def _run_host_op_with_context(state: "State", host: "Host", op_hash: str): - with ctx_host.use(host): - return run_host_op(state, host, op_hash) + with ctx_state.use(state): + with ctx_host.use(host): + return run_host_op(state, host, op_hash) def _run_host_ops(state: "State", host: "Host", progress=None): diff --git a/pyinfra/operations/files.py b/pyinfra/operations/files.py index 041d6e3da..45230821a 100644 --- a/pyinfra/operations/files.py +++ b/pyinfra/operations/files.py @@ -24,7 +24,7 @@ StringCommand, operation, ) -from pyinfra.api.command import make_formatted_string_command +from pyinfra.api.command import EvalOperationAtExecution, make_formatted_string_command from pyinfra.api.util import ( get_call_location, get_file_sha1, @@ -332,16 +332,6 @@ def line( """ match_line = adjust_regex(line, escape_regex_characters) - # - # if escape_regex_characters: - # match_line = re.sub(r"([\.\\\+\*\?\[\^\]\$\(\)\{\}\-])", r"\\\1", match_line) - # - # # Ensure we're matching a whole line, note: match may be a partial line so we - # # put any matches on either side. - # if not match_line.startswith("^"): - # match_line = "^.*{0}".format(match_line) - # if not match_line.endswith("$"): - # match_line = "{0}.*$".format(match_line) # Is there a matching line in this file? if assume_present: @@ -354,6 +344,10 @@ def line( interpolate_variables=interpolate_variables, ) + if present_lines is None and not state.is_executing: + yield EvalOperationAtExecution + return + # If replace present, use that over the matching line if replace: line = replace @@ -397,48 +391,27 @@ def line( # No line and we want it, append it if not present_lines and present: - # If the file does not exist - it *might* be created, so we handle it - # dynamically with a little script. - if present_lines is None: - yield make_formatted_string_command( - """ - if [ -f '{target}' ]; then - ( grep {match_line} '{target}' && \ - {sed_replace_command}) 2> /dev/null || \ - {echo_command} ; - else - {echo_command} ; - fi - """, - target=QuoteString(path), - match_line=QuoteString(match_line), - echo_command=echo_command, - sed_replace_command=sed_replace_command, + # If we're doing replacement, only append if the *replacement* line + # does not exist (as we are appending the replacement). + if replace: + # Ensure replace explicitly matches a whole line + replace_line = replace + if not replace_line.startswith("^"): + replace_line = f"^{replace_line}" + if not replace_line.endswith("$"): + replace_line = f"{replace_line}$" + + present_lines = host.get_fact( + FindInFile, + path=path, + pattern=replace_line, + interpolate_variables=interpolate_variables, ) - # File exists but has no matching lines - append it. + if not present_lines: + yield echo_command else: - # If we're doing replacement, only append if the *replacement* line - # does not exist (as we are appending the replacement). - if replace: - # Ensure replace explicitly matches a whole line - replace_line = replace - if not replace_line.startswith("^"): - replace_line = f"^{replace_line}" - if not replace_line.endswith("$"): - replace_line = f"{replace_line}$" - - present_lines = host.get_fact( - FindInFile, - path=path, - pattern=replace_line, - interpolate_variables=interpolate_variables, - ) - - if not present_lines: - yield echo_command - else: - host.noop('line "{0}" exists in {1}'.format(replace or line, path)) + host.noop('line "{0}" exists in {1}'.format(replace or line, path)) host.create_fact( FindInFile, @@ -1213,8 +1186,13 @@ def link( info = host.get_fact(Link, path=path) - # Not a link? - if info is False: + if info is None and not state.is_executing: # eval at execute because we can't tell state now + yield EvalOperationAtExecution + return + if assume_present: + logger.warning("The `assume_present` argument is no longer needed in `files.link`") + + if info is False: # not a link yield from _raise_or_remove_invalid_path( "link", path, @@ -1231,8 +1209,15 @@ def link( add_cmd = StringCommand(" ".join(add_args), QuoteString(target), QuoteString(path)) remove_cmd = StringCommand("rm", "-f", QuoteString(path)) - # No link and we want it - if not assume_present and info is None and present: + if not present: + if info: + yield remove_cmd + host.delete_fact(Link, kwargs={"path": path}) + else: + host.noop("link {link} does not exist") + return + + if info is None: # create if create_remote_dir: yield from _create_remote_dir(state, host, path, user, group) @@ -1246,18 +1231,7 @@ def link( kwargs={"path": path}, data={"link_target": target, "group": group, "user": user}, ) - - # It exists and we don't want it - elif (assume_present or info) and not present: - yield remove_cmd - host.delete_fact(Link, kwargs={"path": path}) - - # Exists and want to ensure it's state - elif (assume_present or info) and present: - if assume_present and not info: - info = {"link_target": None, "group": None, "user": None} - host.create_fact(Link, kwargs={"path": path}, data=info) - + else: # edit # If we have an absolute path - prepend to any non-absolute values from the fact # and/or the source. if os.path.isabs(path): @@ -1274,18 +1248,14 @@ def link( changed = False # If the target is wrong, remove & recreate the link - if not info or info["link_target"] != target: + if info["link_target"] != target: changed = True yield remove_cmd yield add_cmd info["link_target"] = target # Check user/group - if ( - (not info and (user or group)) - or (user and info["user"] != user) - or (group and info["group"] != group) - ): + if (user and info["user"] != user) or (group and info["group"] != group): yield file_utils.chown(path, user, group, dereference=False) changed = True if user: @@ -1318,7 +1288,6 @@ def file( + path: name/path of the remote file + present: whether the file should exist - + assume_present: whether to assume the file exists + user: user to own the files + group: group to own the files + mode: permissions of the files as an integer, eg: 755 @@ -1354,8 +1323,13 @@ def file( mode = ensure_mode_int(mode) info = host.get_fact(File, path=path) - # Not a file?! - if info is False: + if info is None and not state.is_executing: # eval at execute because we can't tell state now + yield EvalOperationAtExecution + return + if assume_present: + logger.warning("The `assume_present` argument is no longer needed in `files.file`") + + if info is False: # not a file yield from _raise_or_remove_invalid_path( "file", path, @@ -1365,8 +1339,15 @@ def file( ) info = None - # Doesn't exist & we want it - if not assume_present and info is None and present: + if not present: + if info: + yield StringCommand("rm", "-f", QuoteString(path)) + host.delete_fact(File, kwargs={"path": path}) + else: + host.noop("file {0} does not exist") + return + + if info is None: # create if create_remote_dir: yield from _create_remote_dir(state, host, path, user, group) @@ -1382,14 +1363,7 @@ def file( kwargs={"path": path}, data={"mode": mode, "group": group, "user": user}, ) - - # It exists and we don't want it - elif (assume_present or info) and not present: - yield StringCommand("rm", "-f", QuoteString(path)) - host.delete_fact(File, kwargs={"path": path}) - - # It exists & we want to ensure its state - elif (assume_present or info) and present: + else: # update if assume_present and not info: info = {"mode": None, "group": None, "user": None} host.create_fact(File, kwargs={"path": path}, data=info) @@ -1400,18 +1374,12 @@ def file( changed = True yield StringCommand("touch", QuoteString(path)) - # Check mode - if mode and (not info or info["mode"] != mode): + if mode and info["mode"] != mode: yield file_utils.chmod(path, mode) info["mode"] = mode changed = True - # Check user/group - if ( - (not info and (user or group)) - or (user and info["user"] != user) - or (group and info["group"] != group) - ): + if (user and info["user"] != user) or (group and info["group"] != group): yield file_utils.chown(path, user, group) changed = True if user: @@ -1445,7 +1413,6 @@ def directory( + path: path of the remote folder + present: whether the folder should exist - + assume_present: whether to assume the directory exists + user: user to own the folder + group: group to own the folder + mode: permissions of the folder @@ -1484,8 +1451,13 @@ def directory( mode = ensure_mode_int(mode) info = host.get_fact(Directory, path=path) - # Not a directory?! - if info is False: + if info is None and not state.is_executing: # eval at execute because we can't tell state now + yield EvalOperationAtExecution + return + if assume_present: + logger.warning("The `assume_present` argument is no longer needed in `files.directory`") + + if info is False: # not a directory if _no_fail_on_link and host.get_fact(Link, path=path): host.noop("directory {0} already exists (as a link)".format(path)) return @@ -1498,8 +1470,15 @@ def directory( ) info = None - # Doesn't exist & we want it - if not assume_present and info is None and present: + if not present: + if info: + yield StringCommand("rm", "-rf", QuoteString(path)) + host.delete_fact(Directory, kwargs={"path": path}) + else: + host.noop("directory {0} does not exist") + return + + if info is None: # create yield StringCommand("mkdir", "-p", QuoteString(path)) if mode: yield file_utils.chmod(path, mode, recursive=recursive) @@ -1511,14 +1490,7 @@ def directory( kwargs={"path": path}, data={"mode": mode, "group": group, "user": user}, ) - - # It exists and we don't want it - elif (assume_present or info) and not present: - yield StringCommand("rm", "-rf", QuoteString(path)) - host.delete_fact(Directory, kwargs={"path": path}) - - # It exists & we want to ensure its state - elif (assume_present or info) and present: + else: # update if assume_present and not info: info = {"mode": None, "group": None, "user": None} host.create_fact(Directory, kwargs={"path": path}, data=info) @@ -1528,16 +1500,12 @@ def directory( changed = False - if mode and (not info or info["mode"] != mode): + if mode and info["mode"] != mode: yield file_utils.chmod(path, mode, recursive=recursive) info["mode"] = mode changed = True - if ( - (not info and (user or group)) - or (user and info["user"] != user) - or (group and info["group"] != group) - ): + if (user and info["user"] != user) or (group and info["group"] != group): yield file_utils.chown(path, user, group, recursive=recursive) changed = True if user: diff --git a/pyinfra/operations/server.py b/pyinfra/operations/server.py index 4f0459286..e92f5240e 100644 --- a/pyinfra/operations/server.py +++ b/pyinfra/operations/server.py @@ -11,6 +11,7 @@ from pyinfra import host, logger, state from pyinfra.api import FunctionCommand, OperationError, StringCommand, operation +from pyinfra.api.command import EvalOperationAtExecution from pyinfra.api.util import try_int from pyinfra.connectors.util import remove_any_sudo_askpass_file from pyinfra.facts.files import Directory, FindInFile, Link @@ -789,6 +790,10 @@ def group(group, present=True, system=False, gid=None): groups = host.get_fact(Groups) is_present = group in groups + if not is_present and not state.is_executing: + yield EvalOperationAtExecution # eval at execute because we can't tell state now + return + # Group exists but we don't want them? if not present and is_present: yield "groupdel {0}".format(group) @@ -809,10 +814,7 @@ def group(group, present=True, system=False, gid=None): # Groups are often added by other operations (package installs), so check # for the group at runtime before adding. - yield "grep '^{0}:' /etc/group || groupadd {1}".format( - group, - " ".join(args), - ) + yield "groupadd {0}".format(" ".join(args)) groups.append(group) @@ -986,6 +988,10 @@ def user( existing_groups = host.get_fact(Groups) existing_user = users.get(user) + if existing_user is None and not state.is_executing: + yield EvalOperationAtExecution # eval at execute because we can't tell state now + return + if groups is None: groups = [] @@ -1040,10 +1046,7 @@ def user( # Users are often added by other operations (package installs), so check # for the user at runtime before adding. - yield "grep '^{1}:' /etc/passwd || useradd {0} {1}".format( - " ".join(args), - user, - ) + yield "useradd {0} {1}".format(" ".join(args), user) users[user] = { "comment": comment, "home": home, diff --git a/tests/operations/files.directory/delete_assume_present.json b/tests/operations/files.directory/delete_assume_present.json deleted file mode 100644 index 4c8608aa6..000000000 --- a/tests/operations/files.directory/delete_assume_present.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "args": ["testdir"], - "kwargs": { - "present": false, - "assume_present": true - }, - "facts": { - "files.Directory": { - "path=testdir": null - } - }, - "commands": [ - "rm -rf testdir" - ], - "idempotent": false, - "disable_idempotent_warning_reason": "remove is always executed when assume_present" -} diff --git a/tests/operations/files.directory/edit_assume_present.json b/tests/operations/files.directory/edit_assume_present.json deleted file mode 100644 index f35d5b9fb..000000000 --- a/tests/operations/files.directory/edit_assume_present.json +++ /dev/null @@ -1,18 +0,0 @@ -{ - "args": ["testdir"], - "kwargs": { - "user": "testuser", - "group": "testgroup", - "mode": 777, - "assume_present": true - }, - "facts": { - "files.Directory": { - "path=testdir": null - } - }, - "commands": [ - "chmod 777 testdir", - "chown testuser:testgroup testdir" - ] -} diff --git a/tests/operations/files.file/delete_assume_present.json b/tests/operations/files.file/delete_assume_present.json deleted file mode 100644 index 0c4edfdc9..000000000 --- a/tests/operations/files.file/delete_assume_present.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "args": ["testfile"], - "kwargs": { - "present": false, - "assume_present": true - }, - "facts": { - "files.File": { - "path=testfile": null - } - }, - "commands": [ - "rm -f testfile" - ], - "idempotent": false, - "disable_idempotent_warning_reason": "delete operation always executed when assume_present" -} diff --git a/tests/operations/files.file/edit_assume_present.json b/tests/operations/files.file/edit_assume_present.json deleted file mode 100644 index 922cd0cfa..000000000 --- a/tests/operations/files.file/edit_assume_present.json +++ /dev/null @@ -1,18 +0,0 @@ -{ - "args": ["testfile"], - "kwargs": { - "assume_present": true, - "user": "testuser", - "group": "testgroup", - "mode": "777" - }, - "facts": { - "files.File": { - "path=testfile": null - } - }, - "commands": [ - "chmod 777 testfile", - "chown testuser:testgroup testfile" - ] -} diff --git a/tests/operations/files.line/add_no_exist.json b/tests/operations/files.line/add_no_exist.json deleted file mode 100644 index d7d4c017c..000000000 --- a/tests/operations/files.line/add_no_exist.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "args": ["somefile", "match_line"], - "kwargs": { - "replace": "replace_line", - "flags": "abc" - }, - "facts": { - "files.FindInFile": { - "path=somefile, pattern=^.*match_line.*$, interpolate_variables=False": null - } - }, - "commands": [ - "if [ -f somefile ]; then ( grep '^.*match_line.*$' somefile && sed -i.a-timestamp 's/^.*match_line.*$/replace_line/abc' somefile && rm -f somefile.a-timestamp ) 2> /dev/null || echo replace_line >> somefile ; else echo replace_line >> somefile ; fi" - ] -} diff --git a/tests/operations/files.line/add_no_exist_with_quote.json b/tests/operations/files.line/add_no_exist_with_quote.json deleted file mode 100644 index 2779d44b4..000000000 --- a/tests/operations/files.line/add_no_exist_with_quote.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "args": ["some file", "match line"], - "kwargs": { - "replace": "replace line", - "flags": "abc" - }, - "facts": { - "files.FindInFile": { - "path=somefile, pattern=^.*match_line.*$, interpolate_variables=False": null - } - }, - "commands": [ - "if [ -f 'some file' ]; then ( grep '^.*match line.*$' 'some file' && sed -i.a-timestamp 's/^.*match line.*$/replace line/abc' 'some file' && rm -f 'some file.a-timestamp' ) 2> /dev/null || echo 'replace line' >> 'some file' ; else echo 'replace line' >> 'some file' ; fi" - ] -} diff --git a/tests/operations/files.link/delete_assume_present.json b/tests/operations/files.link/delete_assume_present.json deleted file mode 100644 index c54904455..000000000 --- a/tests/operations/files.link/delete_assume_present.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "args": ["testlink"], - "kwargs": { - "present": false, - "assume_present": true - }, - "facts": { - "files.Link": { - "path=testlink": null - } - }, - "commands": [ - "rm -f testlink" - ], - "idempotent": false, - "disable_idempotent_warning_reason": "delete operation always executed when assume_present" -} diff --git a/tests/operations/files.link/edit_assume_present.json b/tests/operations/files.link/edit_assume_present.json deleted file mode 100644 index 15802de41..000000000 --- a/tests/operations/files.link/edit_assume_present.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "args": ["testlink"], - "kwargs": { - "target": "/etc/init.d/nginx", - "assume_present": true - }, - "facts": { - "files.Link": { - "path=testlink": null - } - }, - "commands": [ - "rm -f testlink", - "ln -s /etc/init.d/nginx testlink" - ] -} diff --git a/tests/operations/server.group/add.json b/tests/operations/server.group/add.json index 4ff5f5ece..be9e11e54 100644 --- a/tests/operations/server.group/add.json +++ b/tests/operations/server.group/add.json @@ -9,6 +9,6 @@ "server.Os": "Linux" }, "commands": [ - "grep '^somegroup:' /etc/group || groupadd -r somegroup --gid 1000" + "groupadd -r somegroup --gid 1000" ] } diff --git a/tests/operations/server.user/add.json b/tests/operations/server.user/add.json index 854231213..3b91aa8f3 100644 --- a/tests/operations/server.user/add.json +++ b/tests/operations/server.user/add.json @@ -19,7 +19,7 @@ "server.Os": "Linux" }, "commands": [ - "grep '^someuser:' /etc/passwd || useradd -d homedir -s shellbin -g mygroup -G secondary_group,another -r --uid 1000 -c 'Full Name' -m someuser", + "useradd -d homedir -s shellbin -g mygroup -G secondary_group,another -r --uid 1000 -c 'Full Name' -m someuser", "mkdir -p homedir", "chown someuser:mygroup homedir" ] diff --git a/tests/operations/server.user/add_group_exists.json b/tests/operations/server.user/add_group_exists.json index 2d9ebde06..6d339d639 100644 --- a/tests/operations/server.user/add_group_exists.json +++ b/tests/operations/server.user/add_group_exists.json @@ -9,7 +9,7 @@ "server.Os": "Linux" }, "commands": [ - "grep '^someuser:' /etc/passwd || useradd -d /home/someuser -g someuser someuser", + "useradd -d /home/someuser -g someuser someuser", "mkdir -p /home/someuser", "chown someuser:someuser /home/someuser" ] diff --git a/tests/operations/server.user/add_home_is_link.json b/tests/operations/server.user/add_home_is_link.json index f4bd76a46..f24e8f577 100644 --- a/tests/operations/server.user/add_home_is_link.json +++ b/tests/operations/server.user/add_home_is_link.json @@ -14,6 +14,6 @@ } }, "commands": [ - "grep '^someuser:' /etc/passwd || useradd -d homedir someuser" + "useradd -d homedir someuser" ] } diff --git a/tests/operations/server.user/add_non_unique.json b/tests/operations/server.user/add_non_unique.json index a0a033783..ee4cf87bf 100644 --- a/tests/operations/server.user/add_non_unique.json +++ b/tests/operations/server.user/add_non_unique.json @@ -9,6 +9,6 @@ "server.Groups": {} }, "commands": [ - "grep '^someuser:' /etc/passwd || useradd -d /home/someuser -o someuser" + "useradd -d /home/someuser -o someuser" ] } diff --git a/tests/util.py b/tests/util.py index 98611dac3..00047f0fa 100644 --- a/tests/util.py +++ b/tests/util.py @@ -47,7 +47,7 @@ class FakeState: in_op = True in_deploy = True pipelining = False - is_executing = False + is_executing = True deploy_name = None deploy_kwargs = None