ansible.posix icon indicating copy to clipboard operation
ansible.posix copied to clipboard

Synchronize: suppress synchronize verbose output

Open mandar242 opened this issue 2 years ago • 14 comments

SUMMARY

Adding rsync 'quiet' option to synchronize module. Allows to suppress synchronize verbose output.

Resolves #171

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

Synchronize

mandar242 avatar Jul 12 '21 16:07 mandar242

Could you also add a changelog fragment: https://docs.ansible.com/ansible/latest/community/development_process.html#community-changelogs?

gravesm avatar Jul 12 '21 18:07 gravesm

Could you also add a changelog fragment: https://docs.ansible.com/ansible/latest/community/development_process.html#community-changelogs?

Added.

mandar242 avatar Jul 12 '21 19:07 mandar242

cc @quidame @saito-hideki Could you please review this? Thanks in advance.

Akasurde avatar Jul 14 '21 04:07 Akasurde

This change actually may not work as is. It's breaking the ability to determine changed status. Testing locally for me shows no change when there is, in fact, a change. I think I'm not sure what extraneous output the OP was referring to is. The only case I can find where -v is being used is when link_dest is set, and the note in the code seems to suggest this is required here. @phemmer can you clarify what output you are referring to and possibly provide an example task to reproduce?

@mandar242 pending further information about what's being requested in this feature, we should add a few more tests to this. At least one to check that a task's result has changed and then a following one running the same task to check that the result has not changed.

gravesm avatar Jul 14 '21 14:07 gravesm

as reported by @gravesm , adding --quiet option just breaks check_mode. As said in rsync(1) manpage:

-q, --quiet                 suppress non-error messages

The check_mode and diff are based on --out-format option (with a marker in), which is suppressed by the use of --quiet.

quidame avatar Jul 14 '21 15:07 quidame

@phemmer can you clarify what output you are referring to and possibly provide an example task to reproduce?

$ mkdir /tmp/a

$ touch /tmp/a/{1..100}

$ ansible localhost -m synchronize -a 'src=/tmp/a/ dest=/tmp/b/'
[WARNING]: No inventory was parsed, only implicit localhost is available
localhost | CHANGED => {
    "changed": true,
    "cmd": "/usr/bin/rsync --delay-updates -F --compress --archive --out-format=<<CHANGED>>%i %n%L /tmp/a/ /tmp/b/",
    "msg": "cd+++++++++ ./\n>f+++++++++ 1\n>f+++++++++ 10\n>f+++++++++ 100\n>f+++++++++ 11\n>f+++++++++ 12\n>f+++++++++ 13\n>f+++++++++ 14\n>f+++++++++ 15\n>f+++++++++ 16\n>f+++++++++ 17\n>f+++++++++ 18\n>f+++++++++ 19\n>f+++++++++ 2\n>f+++++++++ 20\n>f+++++++++ 21\n>f+++++++++ 22\n>f+++++++++ 23\n>f+++++++++ 24\n>f+++++++++ 25\n>f+++++++++ 26\n>f+++++++++ 27\n>f+++++++++ 28\n>f+++++++++ 29\n>f+++++++++ 3\n>f+++++++++ 30\n>f+++++++++ 31\n>f+++++++++ 32\n>f+++++++++ 33\n>f+++++++++ 34\n>f+++++++++ 35\n>f+++++++++ 36\n>f+++++++++ 37\n>f+++++++++ 38\n>f+++++++++ 39\n>f+++++++++ 4\n>f+++++++++ 40\n>f+++++++++ 41\n>f+++++++++ 42\n>f+++++++++ 43\n>f+++++++++ 44\n>f+++++++++ 45\n>f+++++++++ 46\n>f+++++++++ 47\n>f+++++++++ 48\n>f+++++++++ 49\n>f+++++++++ 5\n>f+++++++++ 50\n>f+++++++++ 51\n>f+++++++++ 52\n>f+++++++++ 53\n>f+++++++++ 54\n>f+++++++++ 55\n>f+++++++++ 56\n>f+++++++++ 57\n>f+++++++++ 58\n>f+++++++++ 59\n>f+++++++++ 6\n>f+++++++++ 60\n>f+++++++++ 61\n>f+++++++++ 62\n>f+++++++++ 63\n>f+++++++++ 64\n>f+++++++++ 65\n>f+++++++++ 66\n>f+++++++++ 67\n>f+++++++++ 68\n>f+++++++++ 69\n>f+++++++++ 7\n>f+++++++++ 70\n>f+++++++++ 71\n>f+++++++++ 72\n>f+++++++++ 73\n>f+++++++++ 74\n>f+++++++++ 75\n>f+++++++++ 76\n>f+++++++++ 77\n>f+++++++++ 78\n>f+++++++++ 79\n>f+++++++++ 8\n>f+++++++++ 80\n>f+++++++++ 81\n>f+++++++++ 82\n>f+++++++++ 83\n>f+++++++++ 84\n>f+++++++++ 85\n>f+++++++++ 86\n>f+++++++++ 87\n>f+++++++++ 88\n>f+++++++++ 89\n>f+++++++++ 9\n>f+++++++++ 90\n>f+++++++++ 91\n>f+++++++++ 92\n>f+++++++++ 93\n>f+++++++++ 94\n>f+++++++++ 95\n>f+++++++++ 96\n>f+++++++++ 97\n>f+++++++++ 98\n>f+++++++++ 99\n",
    "rc": 0,
    "stdout_lines": [
        "cd+++++++++ ./",
        ">f+++++++++ 1",
        ">f+++++++++ 10",
        ">f+++++++++ 100",
        ">f+++++++++ 11",
        ">f+++++++++ 12",
        ">f+++++++++ 13",
...
        ">f+++++++++ 96",
        ">f+++++++++ 97",
        ">f+++++++++ 98",
        ">f+++++++++ 99"
    ]
}

^ The msg is the problem, as that's what ansible-playbook is going to display.

phemmer avatar Jul 22 '21 15:07 phemmer

note that the --out-format option is given a marker (<<CHANGED>>), which is removed from the output before sending this output in the results (msg, stdout_lines). At this step (the module post-processing rsync results, and being aware of possible changes), I think it is doable to just keep rsync's stderr and suppress rsync's stdout if a user requests that with a quiet=true new module option :)

Maybe keep transfer metadata (number of files, duration...) anyway, even when hidding all the details, could be a good point.

quidame avatar Jul 22 '21 17:07 quidame

@mandar242 , I suggest you try with something like the following, which shouldn't break anything

@@ -619,14 +627,20 @@ def main():
     out_lines = out_clean.split('\n')
     while '' in out_lines:
         out_lines.remove('')
+
+    result = dict(changed=changed, rc=rc, cmd=cmdstr)
+
+    if quiet:
+        result['msg'] = "OUTPUT IS HIDDEN DUE TO 'quiet=true'"
+        result['stdout_lines'] = []
+    else:
+        result['msg'] = out_clean
+        result['stdout_lines'] = out_lines
+
     if module._diff:
-        diff = {'prepared': out_clean}
-        return module.exit_json(changed=changed, msg=out_clean,
-                                rc=rc, cmd=cmdstr, stdout_lines=out_lines,
-                                diff=diff)
+        result['diff'] = {'prepared': out_clean}
 
-    return module.exit_json(changed=changed, msg=out_clean,
-                            rc=rc, cmd=cmdstr, stdout_lines=out_lines)
+    return module.exit_json(**result)
 
 
 if __name__ == '__main__':

quidame avatar Jul 24 '21 00:07 quidame

2 years later, what's needed for this to be merged? I see there's a change requested to add tests, but it looks like tests are present.

phemmer avatar Aug 08 '23 15:08 phemmer

Ping? @Akasurde ?

phemmer avatar Aug 18 '23 15:08 phemmer

@phemmer sorry, but I am not actively looking into ansible.posix collection. @mandar242 Can you please rebase the branch and update version_added?

cc @saito-hideki @maxamillion Could you please take care of this PR? Thanks in advance.

Akasurde avatar Aug 18 '23 17:08 Akasurde

Build failed. https://ansible.softwarefactory-project.io/zuul/buildset/3619c7a8ee3a4bfb9c7a3153163b67da

:heavy_check_mark: ansible-changelog-fragment SUCCESS in 12s :warning: ansible-test-sanity-docker-devel SKIPPED Skipped due to failed job build-ansible-collection (non-voting) :warning: ansible-test-sanity-docker-milestone SKIPPED Skipped due to failed job build-ansible-collection (non-voting) :warning: ansible-test-sanity-docker-stable-2.9 SKIPPED Skipped due to failed job build-ansible-collection :warning: ansible-test-sanity-docker-stable-2.10 SKIPPED Skipped due to failed job build-ansible-collection :warning: ansible-test-sanity-docker-stable-2.11 SKIPPED Skipped due to failed job build-ansible-collection :warning: ansible-test-sanity-docker-stable-2.12 SKIPPED Skipped due to failed job build-ansible-collection :warning: ansible-test-sanity-docker-stable-2.13 SKIPPED Skipped due to failed job build-ansible-collection :warning: ansible-test-sanity-docker-stable-2.14 SKIPPED Skipped due to failed job build-ansible-collection :warning: ansible-test-units-posix-python39 SKIPPED Skipped due to failed job build-ansible-collection :warning: ansible-test-units-posix-python310 SKIPPED Skipped due to failed job build-ansible-collection :warning: ansible-galaxy-importer SKIPPED Skipped due to failed job build-ansible-collection :x: build-ansible-collection NODE_FAILURE Node request 200-0006305028 failed in 0s

@saito-hideki @maxamillion

Looks like this was rebased as requested a few months ago. Is there any way to get this moving? Thanks

phemmer avatar Nov 01 '23 16:11 phemmer

@saito-hideki @maxamillion

Hello???

phemmer avatar Nov 17 '23 12:11 phemmer