From ad386290b4d701712faafb91f17ad6fe7a361655 Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Wed, 13 Feb 2019 13:54:36 -0500 Subject: [PATCH] Update command/shell docs with note about modules for rebooting(#51499) - Update integration test syntax --- lib/ansible/modules/commands/command.py | 7 +- lib/ansible/modules/commands/shell.py | 1 + .../targets/command_shell/tasks/main.yml | 174 ++++++++++++------ 3 files changed, 121 insertions(+), 61 deletions(-) diff --git a/lib/ansible/modules/commands/command.py b/lib/ansible/modules/commands/command.py index 2c25e2d70a..b8a75e3a7b 100644 --- a/lib/ansible/modules/commands/command.py +++ b/lib/ansible/modules/commands/command.py @@ -85,6 +85,7 @@ notes: check for the existence of the file and report the correct changed status. If these are not supplied, the task will be skipped. - The C(executable) parameter is removed since version 2.4. If you have a need for this parameter, use the M(shell) module instead. - For Windows targets, use the M(win_command) module instead. + - For rebooting systems, use the M(reboot) or M(win_reboot) module. seealso: - module: raw - module: script @@ -182,18 +183,18 @@ def check_command(module, commandline): command = os.path.basename(command) disable_suffix = "If you need to use command because {mod} is insufficient you can add" \ - " warn=False to this command task or set command_warnings=False in" \ + " 'warn: false' to this command task or set 'command_warnings=False' in" \ " ansible.cfg to get rid of this message." substitutions = {'mod': None, 'cmd': command} if command in arguments: - msg = "Consider using the {mod} module with {subcmd} rather than running {cmd}. " + disable_suffix + msg = "Consider using the {mod} module with {subcmd} rather than running '{cmd}'. " + disable_suffix substitutions['mod'] = 'file' substitutions['subcmd'] = arguments[command] module.warn(msg.format(**substitutions)) if command in commands: - msg = "Consider using the {mod} module rather than running {cmd}. " + disable_suffix + msg = "Consider using the {mod} module rather than running '{cmd}'. " + disable_suffix substitutions['mod'] = commands[command] module.warn(msg.format(**substitutions)) diff --git a/lib/ansible/modules/commands/shell.py b/lib/ansible/modules/commands/shell.py index 46f2d26313..9384e0d774 100644 --- a/lib/ansible/modules/commands/shell.py +++ b/lib/ansible/modules/commands/shell.py @@ -87,6 +87,7 @@ notes: do not include evil things like semicolons. - An alternative to using inline shell scripts with this module is to use the M(script) module possibly together with the M(template) module. + - For rebooting systems, use the M(reboot) or M(win_reboot) module. seealso: - module: command - module: raw diff --git a/test/integration/targets/command_shell/tasks/main.yml b/test/integration/targets/command_shell/tasks/main.yml index 390afc79d5..1d614e4945 100644 --- a/test/integration/targets/command_shell/tasks/main.yml +++ b/test/integration/targets/command_shell/tasks/main.yml @@ -29,8 +29,8 @@ - name: assert tar warning was reported assert: that: - - "tar.warnings | length() == 1" - - "'Consider using the unarchive module rather than running tar' in tar.warnings[0]" + - tar.warnings | length() == 1 + - '"Consider using the unarchive module rather than running ''tar''" in tar.warnings[0]' - name: use command to execute chown command: chown -h @@ -40,31 +40,48 @@ - name: assert chown warning was reported assert: that: - - "chown.warnings | length() == 1" - - "'Consider using the file module with owner rather than running chown' in chown.warnings[0]" + - chown.warnings | length() == 1 + - '"Consider using the file module with owner rather than running ''chown''" in chown.warnings[0]' - name: use command with unsupported executable arg - command: ls /dev/null executable=/bogus + command: ls /dev/null + args: + executable: /bogus register: executable - name: assert executable warning was reported assert: that: - - "executable.stdout == '/dev/null'" - - "executable.warnings | length() == 1" + - executable.stdout == '/dev/null' + - executable.warnings | length() == 1 - "'no longer supported' in executable.warnings[0]" +# The warning isn't on the task since it comes from the action plugin. Not sure +# how to test for that. +# +# - name: Use command with reboot +# command: sleep 2 && /not/shutdown -r now +# ignore_errors: yes +# register: reboot + +# - name: Assert that reboot warning was issued +# assert: +# that: +# - '"Consider using the reboot module" in reboot.warnings[0]' + - name: use command with no command - command: chdir=/ + command: + args: + chdir: / register: no_command ignore_errors: true - name: assert executable fails with no command assert: that: - - "no_command.failed == true" - - "no_command.msg == 'no command given'" - - "no_command.rc == 256" + - no_command is failed + - no_command.msg == 'no command given' + - no_command.rc == 256 - name: use argv command: @@ -91,26 +108,40 @@ - name: assert executable fails with both argv and command string assert: that: - - "argv_and_string_command.failed == true" - - "argv_and_string_command.msg == 'only command or argv can be given, not both'" - - "argv_and_string_command.rc == 256" + - argv_and_string_command is failed + - argv_and_string_command.msg == 'only command or argv can be given, not both' + - argv_and_string_command.rc == 256 -- set_fact: output_dir_test={{output_dir}}/test_command_shell +- set_fact: + output_dir_test: "{{ output_dir }}/test_command_shell" - name: make sure our testing sub-directory does not exist - file: path="{{ output_dir_test }}" state=absent + file: + path: "{{ output_dir_test }}" + state: absent - name: create our testing sub-directory - file: path="{{ output_dir_test }}" state=directory + file: + path: "{{ output_dir_test }}" + state: directory - name: prep our test script - copy: src=test.sh dest="{{ output_dir_test }}" mode=0755 + copy: + src: test.sh + dest: "{{ output_dir_test }}" + mode: '0755' - name: prep our test script - copy: src=create_afile.sh dest="{{ output_dir_test }}" mode=0755 + copy: + src: create_afile.sh + dest: "{{ output_dir_test }}" + mode: '0755' - name: prep our test script - copy: src=remove_afile.sh dest="{{ output_dir_test }}" mode=0755 + copy: + src: remove_afile.sh + dest: "{{ output_dir_test }}" + mode: '0755' - name: locate bash shell: which bash @@ -121,22 +152,22 @@ ## - name: execute the test.sh script via command - command: "{{output_dir_test | expanduser}}/test.sh" + command: "{{ output_dir_test }}/test.sh" register: command_result0 - name: assert that the script executed correctly assert: that: - - "command_result0.rc == 0" - - "command_result0.stderr == ''" - - "command_result0.stdout == 'win'" + - command_result0.rc == 0 + - command_result0.stderr == '' + - command_result0.stdout == 'win' # executable # FIXME doesn't have the expected stdout. #- name: execute the test.sh script with executable via command -# command: "{{output_dir_test | expanduser}}/test.sh executable={{ bash.stdout }}" +# command: "{{output_dir_test }}/test.sh executable={{ bash.stdout }}" # register: command_result1 # #- name: assert that the script executed correctly with command @@ -149,55 +180,69 @@ # chdir - name: execute the test.sh script with chdir via command - command: ./test.sh chdir="{{output_dir_test | expanduser}}" + command: ./test.sh + args: + chdir: "{{ output_dir_test }}" register: command_result2 - name: assert that the script executed correctly with chdir assert: that: - - "command_result2.rc == 0" - - "command_result2.stderr == ''" - - "command_result2.stdout == 'win'" + - command_result2.rc == 0 + - command_result2.stderr == '' + - command_result2.stdout == 'win' # creates - name: verify that afile.txt is absent - file: path={{output_dir_test}}/afile.txt state=absent + file: + path: "{{ output_dir_test }}/afile.txt" + state: absent - name: create afile.txt with create_afile.sh via command - command: "{{output_dir_test | expanduser}}/create_afile.sh {{output_dir_test | expanduser}}/afile.txt creates={{output_dir_test | expanduser}}/afile.txt" + command: "{{ output_dir_test }}/create_afile.sh {{output_dir_test }}/afile.txt" + args: + creates: "{{ output_dir_test }}/afile.txt" - name: verify that afile.txt is present - file: path={{output_dir_test}}/afile.txt state=file + file: + path: "{{ output_dir_test }}/afile.txt" + state: file - name: re-run previous command using creates with globbing - command: "{{output_dir_test | expanduser}}/create_afile.sh {{output_dir_test | expanduser}}/afile.txt creates={{output_dir_test | expanduser}}/afile.*" + command: "{{ output_dir_test }}/create_afile.sh {{ output_dir_test }}/afile.txt" + args: + creates: "{{ output_dir_test }}/afile.*" register: command_result3 - name: assert that creates with globbing is working assert: that: - - "command_result3.changed != True" + - command_result3 is not changed # removes - name: remove afile.txt with remote_afile.sh via command - command: "{{output_dir_test | expanduser}}/remove_afile.sh {{output_dir_test | expanduser}}/afile.txt removes={{output_dir_test | expanduser}}/afile.txt" + command: "{{ output_dir_test }}/remove_afile.sh {{ output_dir_test }}/afile.txt" + args: + removes: "{{ output_dir_test }}/afile.txt" - name: verify that afile.txt is absent file: path={{output_dir_test}}/afile.txt state=absent - name: re-run previous command using removes with globbing - command: "{{output_dir_test | expanduser}}/remove_afile.sh {{output_dir_test | expanduser}}/afile.txt removes={{output_dir_test | expanduser}}/afile.*" + command: "{{ output_dir_test }}/remove_afile.sh {{ output_dir_test }}/afile.txt" + args: + removes: "{{ output_dir_test }}/afile.*" register: command_result4 - name: assert that removes with globbing is working assert: that: - - "command_result4.changed != True" + - command_result4.changed != True - name: pass stdin to cat via command - command: "cat" + command: cat args: stdin: 'foobar' register: command_result5 @@ -205,10 +250,10 @@ - name: assert that stdin is passed assert: that: - - "command_result5.stdout == 'foobar'" + - command_result5.stdout == 'foobar' - name: send to stdin literal multiline block - command: "{{ ansible_python_interpreter }} -c 'import hashlib, sys; print(hashlib.sha1((sys.stdin.buffer if hasattr(sys.stdin, \"buffer\") else sys.stdin).read()).hexdigest())'" + command: "{{ ansible_python.executable }} -c 'import hashlib, sys; print(hashlib.sha1((sys.stdin.buffer if hasattr(sys.stdin, \"buffer\") else sys.stdin).read()).hexdigest())'" args: stdin: |- this is the first line @@ -228,14 +273,14 @@ ## - name: Execute the test.sh script - shell: "{{output_dir_test | expanduser}}/test.sh" + shell: "{{ output_dir_test }}/test.sh" register: shell_result0 - name: Assert that the script executed correctly assert: that: - shell_result0 is changed - - shell_result0.cmd == '{{output_dir_test | expanduser}}/test.sh' + - shell_result0.cmd == '{{ output_dir_test }}/test.sh' - shell_result0.rc == 0 - shell_result0.stderr == '' - shell_result0.stdout == 'win' @@ -245,7 +290,7 @@ # FIXME doesn't pass the expected stdout #- name: execute the test.sh script -# shell: "{{output_dir_test | expanduser}}/test.sh executable={{ bash.stdout }}" +# shell: "{{output_dir_test }}/test.sh executable={{ bash.stdout }}" # register: shell_result1 # #- name: assert that the shell executed correctly @@ -258,7 +303,9 @@ # chdir - name: Execute the test.sh script with chdir - shell: ./test.sh chdir="{{output_dir_test | expanduser}}" + shell: ./test.sh + args: + chdir: "{{ output_dir_test }}" register: shell_result2 - name: Assert that the shell executed correctly with chdir @@ -273,18 +320,27 @@ # creates - name: Verify that afile.txt is absent - file: path={{output_dir_test}}/afile.txt state=absent + file: + path: "{{ output_dir_test }}/afile.txt" + state: absent - name: Execute the test.sh script with chdir - shell: "{{output_dir_test | expanduser}}/test.sh > {{output_dir_test | expanduser}}/afile.txt creates={{output_dir_test | expanduser}}/afile.txt" + shell: "{{ output_dir_test }}/test.sh > {{ output_dir_test }}/afile.txt" + args: + chdir: "{{ output_dir_test }}" + creates: "{{ output_dir_test }}/afile.txt" - name: Verify that afile.txt is present - file: path={{output_dir_test}}/afile.txt state=file + file: + path: "{{ output_dir_test }}/afile.txt" + state: file # multiline - name: Remove test file previously created - file: path={{output_dir_test | expanduser}}/afile.txt state=absent + file: + path: "{{ output_dir_test }}/afile.txt" + state: absent - name: Execute a shell command using a literal multiline block args: @@ -294,7 +350,7 @@ "multiline echo" \ "with a new line in quotes" \ - | {{ ansible_python_interpreter }} -c 'import hashlib, sys; print(hashlib.sha1((sys.stdin.buffer if hasattr(sys.stdin, "buffer") else sys.stdin).read()).hexdigest())' + | {{ ansible_python.executable }} -c 'import hashlib, sys; print(hashlib.sha1((sys.stdin.buffer if hasattr(sys.stdin, "buffer") else sys.stdin).read()).hexdigest())' echo "this is a second line" register: shell_result5 @@ -303,13 +359,13 @@ that: - shell_result5 is changed - shell_result5.rc == 0 - - shell_result5.cmd == 'echo this is a "multiline echo" "with a new line\nin quotes" | ' + ansible_python_interpreter + ' -c \'import hashlib, sys; print(hashlib.sha1((sys.stdin.buffer if hasattr(sys.stdin, "buffer") else sys.stdin).read()).hexdigest())\'\necho "this is a second line"\n' + - shell_result5.cmd == 'echo this is a "multiline echo" "with a new line\nin quotes" | ' + ansible_python.executable + ' -c \'import hashlib, sys; print(hashlib.sha1((sys.stdin.buffer if hasattr(sys.stdin, "buffer") else sys.stdin).read()).hexdigest())\'\necho "this is a second line"\n' - shell_result5.stdout == '5575bb6b71c9558db0b6fbbf2f19909eeb4e3b98\nthis is a second line' - name: Execute a shell command using a literal multiline block with arguments in it shell: | executable="{{ bash.stdout }}" - creates={{output_dir_test | expanduser}}/afile.txt + creates={{ output_dir_test }}/afile.txt echo "test" register: shell_result6 @@ -339,14 +395,14 @@ - shell_result7.stdout == 'One\n Two\n Three' - name: execute a shell command with no trailing newline to stdin - shell: cat > {{output_dir_test | expanduser}}/afile.txt + shell: cat > {{output_dir_test }}/afile.txt args: stdin: test stdin_add_newline: no - name: make sure content matches expected copy: - dest: "{{output_dir_test | expanduser}}/afile.txt" + dest: "{{output_dir_test }}/afile.txt" content: test register: shell_result7 failed_when: @@ -354,14 +410,14 @@ shell_result7 is changed - name: execute a shell command with trailing newline to stdin - shell: cat > {{output_dir_test | expanduser}}/afile.txt + shell: cat > {{output_dir_test }}/afile.txt args: stdin: test stdin_add_newline: yes - name: make sure content matches expected copy: - dest: "{{output_dir_test | expanduser}}/afile.txt" + dest: "{{output_dir_test }}/afile.txt" content: | test register: shell_result8 @@ -370,13 +426,13 @@ shell_result8 is changed - name: execute a shell command with trailing newline to stdin, default - shell: cat > {{output_dir_test | expanduser}}/afile.txt + shell: cat > {{output_dir_test }}/afile.txt args: stdin: test - name: make sure content matches expected copy: - dest: "{{output_dir_test | expanduser}}/afile.txt" + dest: "{{output_dir_test }}/afile.txt" content: | test register: shell_result9 @@ -385,4 +441,6 @@ shell_result9 is changed - name: remove the previously created file - file: path={{output_dir_test}}/afile.txt state=absent + file: + path: "{{ output_dir_test }}/afile.txt" + state: absent