Repository: ansible/ansible
Pull request: toggle for stripping empty shell/command output
  fixes #45367

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
- Feature Pull Request

##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below -->
shell/command

Problem statement:
Empty lines are stripped from the command module's output.
##### SUMMARY
The command module strips trailing newlines from the output.

##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
command

##### ANSIBLE VERSION
```paste below
ansible 2.6.3
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/merrily/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.5 (default, May 31 2018, 09:41:32) [GCC 4.8.5 20150623 (Red Hat 4.8.5-28)]
```

##### CONFIGURATION
```paste below

```

##### STEPS TO REPRODUCE
Give the command module a program that outputs blank lines.

```yaml
#playbook.yml
---
- hosts: 127.0.0.1
  tasks:
    - shell: echo -e "\n\n\n"
      register: result

    - debug:
        var: result.stdout
```

##### EXPECTED RESULTS
I expect to see the exact program output.

##### ACTUAL RESULTS
I did not get to see the exact program output.

```paste below
 [WARNING]: provided hosts list is empty, only localhost is available. Note
that the implicit localhost does not match 'all'


PLAY [127.0.0.1] ***************************************************************

TASK [Gathering Facts] *********************************************************
ok: [127.0.0.1]

TASK [shell] *******************************************************************
changed: [127.0.0.1]

TASK [debug] *******************************************************************
ok: [127.0.0.1] => {
    "result.stdout": ""
}

PLAY RECAP *********************************************************************
127.0.0.1                  : ok=3    changed=1    unreachable=0    failed=0   


```

Hints from issue discussion:
Files identified in the description:
* [lib/ansible/modules/commands/command.py](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/commands/command.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
Most of the time any trailing blank lines are just useless information as an after effect of some programs, so we automatically drop any trailing ones. 

Can you show an actual use case in which this information is needed?
We have a situation where I think this is happening - I can't confirm yet, but our use case is as follows (this is using `2.6.3`):

We are running some unit tests inside a docker container via the `shell` module using redirection to print output to a file `docker run... &>> /some/log/file` We loop this task until it exits 0 (succeeds). I've been trying to figure out why the logs are all truncated after the first few lines, ex:

```
------------ 2018-10-10 05:36:20 ------------
docker: error during connect: Post https://xx.xxxxx:443/v1.30/containers/create: EOF.
See 'docker run --help'.

> terra@1.3.4 test /srv
> node spec/test_runner.js -- -t unit -testunit

Sanity Tests - Starting
args...
[ '-t', 'unit', '-testunit' ]

> terra@1.3.4 test /srv
> node spec/test_runner.js -- -t unit -testunit


> terra@1.3.4 test /srv
> node spec/test_runner.js -- -t unit -testunit
```

When I run the exact same command manually, everything works and output looks as follows:

```
> terra@1.3.4 test /srv
> node spec/test_runner.js -- -t unit -testunit

Sanity Tests - Starting
args...
[ '-t', 'unit', '-testunit' ]


>> Executing 44 defined specs...




Test Suites & Specs:




1) authentication service
```

I am just a lowly ops guy, so I can't necessarily fix the fact there's so many damn newlines 🤣 but I have a theory that this bug is affecting us here.
@bcoca Is there any workaround for this bug.
@Ksreenivas  I'm still not convinced this is a bug, the original poster never responded to my request and I don't understand how  @chiefy's example applies here. If someone can actually post a case in which the trailing newlines are significant it would help.
@bcoca 

> Most of the time any trailing blank lines are just useless information as an after effect of some programs, so we automatically drop any trailing ones.

I need to come up with a concrete reproducible example but i'm pretty sure it's affecting us as I've explained above. I don't know if this is a bug so much as a misunderstanding? It seems like something we should be able to override w/ module settings?
command has always stripped trailing spaces as they are normally not significant, if you want to add a feature that avoids this, it should be easy enough, this is current code:
```
        stdout=out.rstrip(b"\r\n"),
        stderr=err.rstrip(b"\r\n"),
```

But this is done on purpose as those spaces are not normally significant and a waste of resources to handle in memory and via network connection, so I still don't see it as a bug unless you can give me a concrete case that shows otherwise.
Sorry, I guess there's some confusion about the original issue and what we're seeing. To be clear: we're seeing output from the `shell`module (stdout) stopping after it outputs 2+ blank newlines. It doesn't unbuffer the rest of the output. Not 100% this is Ansible, but we can't reproduce when running the exact same command manually with the same environment etc. 
@bcoca since the OP didn't respond, I may show a valid usecase:

Redis sentinel when changing a master in master-slave cluster, overwrites `redis.conf`, by appending it with new directives.

If we try to manage redis.conf from ansible, runs are not idempotent once one of the slaves was promoted to master. Ansible play wants to change the configs and reload the redis instances.

To avoid that, I've used the following solution:

Play that stores the changes to var:

```- name: fetch config rewrite changes
  become: yes 
  command: grep -A 100 '# Generated by CONFIG REWRITE' {{ redis_config_file }}
  register: redis__reg_config_rewrite_redis
  check_mode: no
  failed_when: no
  changed_when: no

- set_fact:
    redis__config_rewrite: "{{ redis__reg_config_rewrite_redis.stdout | default('') }}" 
```

And jinja template has this as the last line:

```
{{ redis__config_rewrite }}
```

Now, since sentinel can write an empty line at the bottom of the config file, `grep -A 100` gets it, but ansible strips it, and then jinja2 wants to rewrite `redis.conf` and subsequently flush handlers (in this case - restart redis).

So, I would like to see additional option for `command` not to strip empty lines, if possible.
@bcoca Is it a bug that newline characters are stripped from the stdout of `command:` and I think `shell:` too, but not from `script:`? The reason I ask is because I'm used to having clean outputs from `command:` which can be easily parsed, like `when: registered_var.stdout == "banana"`, but I'm bitten when I use `script:` and need to remember to change it to something like `when: registered_var.stdout`__`| trim`__` == "banana"`.

Commit message: toggle for stripping empty shell/command output

  fixes #45367

Patch to review:
diff --git a/changelogs/fragments/no_strippers.yml b/changelogs/fragments/no_strippers.yml
new file mode 100644
index 00000000000000..5160c535be3e1b
--- /dev/null
+++ b/changelogs/fragments/no_strippers.yml
@@ -0,0 +1,2 @@
+minor_changes:
+    - add option to shell/command to control stripping of empty lines at end of outputs
diff --git a/lib/ansible/modules/commands/command.py b/lib/ansible/modules/commands/command.py
index bd22d96a54554e..2b203b42a49457 100644
--- a/lib/ansible/modules/commands/command.py
+++ b/lib/ansible/modules/commands/command.py
@@ -69,6 +69,12 @@
     description:
       - If set to C(yes), append a newline to stdin data.
     version_added: "2.8"
+  strip_empty_ends:
+    description:
+      - Strim empty lines from the end of stdout/stderr in result.
+    version_added: "2.8"
+    type: bool
+    default: yes
 notes:
     -  If you want to run a command through the shell (say you are using C(<), C(>), C(|), etc), you actually want the M(shell) module instead.
        Parsing shell metacharacters can lead to unexpected commands being executed if quoting is not done correctly so it is more secure to
@@ -212,6 +218,7 @@ def main():
             warn=dict(type='bool', default=True),
             stdin=dict(required=False),
             stdin_add_newline=dict(type='bool', default=True),
+            strip_empty_ends=dict(type='bool', default=True),
         ),
         supports_check_mode=True,
     )
@@ -225,6 +232,7 @@ def main():
     warn = module.params['warn']
     stdin = module.params['stdin']
     stdin_add_newline = module.params['stdin_add_newline']
+    strip = module.params['strip_empty_ends']
 
     if not shell and executable:
         module.warn("As of Ansible 2.4, the parameter 'executable' is no longer supported with the 'command' module. Not using '%s'." % executable)
@@ -289,10 +297,14 @@ def main():
     endd = datetime.datetime.now()
     delta = endd - startd
 
+    if strip:
+        out = out.rstrip(b"\r\n")
+        err = err.rstrip(b"\r\n")
+
     result = dict(
         cmd=args,
-        stdout=out.rstrip(b"\r\n"),
-        stderr=err.rstrip(b"\r\n"),
+        stdout=out,
+        stderr=err,
         rc=rc,
         start=str(startd),
         end=str(endd),


Code review comments:
diff_hunk: @@ -69,6 +69,12 @@
     description:
       - If set to C(yes), append a newline to stdin data.
     version_added: "2.8"
+  strip_empty_ends:
+    description:
+      - Strim empty lines from the end of stdout/stderr in result.
path: lib/ansible/modules/commands/command.py
text: @user1:
```suggestion
      - Strip empty lines from the end of stdout/stderr in result.
```