Repository: ansible/ansible
Pull request: win_scheduled_task: Added frequency: once and check_mode support
##### SUMMARY
<!--- Describe the change, including rationale and design decisions -->

<!---
If you are fixing an existing issue, please include "Fixes #nnn" in your
commit message and your description; but you should still explain what
the change does.
-->
This patch includes:
- Renamed `execute:` parameter to `executable:`
- Renamed `argument:` parameter to `arguments:`
- Implemented `frequency: once` support
- Implemented check_mode support
- Fix idempotency issue related to empty description
- Added integration tests

##### ISSUE TYPE
<!--- Pick one below and delete the rest: -->
 - Feature Pull Request
 - Bugfix Pull Request

##### COMPONENT NAME
<!--- Name of the module/plugin/module/task -->
win_scheduled_task

##### ANSIBLE VERSION
<!--- Paste verbatim output from “ansible --version” between quotes below -->
v2.3

##### ADDITIONAL INFORMATION
<!---
Include additional information to help people understand the change here.
For bugs that don't have a linked bug report, a step-by-step reproduction
of the problem is helpful.
  -->
This fixes #22485 

Problem statement:
win_scheduled_task
<!---
Verify first that your issue/request is not already reported on GitHub.
Also test if the latest release, and master branch are affected too.
-->

##### ISSUE TYPE
<!--- Pick one below and delete the rest: -->
 - Bug Report
 - Feature Idea
 - Documentation Report

##### COMPONENT NAME
win_scheduled_task:

##### ANSIBLE VERSION

```
ansible 2.3.0 
 config file = /etc/ansible/ansible.cfg
  configured module search path = Default w/o overrides
  python version = 2.7.12 (default, Nov 19 2016, 06:48:10) [GCC 5.4.0 20160609]
```

##### CONFIGURATION
<!---

-->

##### OS / ENVIRONMENT
<!---
Ubuntu 16.04.2 LTS / Windows Server 2012
-->

##### SUMMARY
expected behaviour: creates a runnable Task that also runs once (exp. after Installation of Windows updates and a reboot ist required -> schedule reboot to 3am once), there is no way to add Options to a command that should have to be executed.
The command "shutdown -r -t 1" won't be executed by Windows, because the "-r -t 1" Option isn't set as command Option. The System will fail with error code 0x80070002 (can't find file specified).
Also missing is the "once" Option by Script Default

##### STEPS TO REPRODUCE
<!---
just try to schedule a Task to reboot a Windows Server to a specific time
-->

<!--- Paste example playbooks or commands between quotes below -->
```
    - name: Configure | Plan | Reboot
      win_scheduled_task: name="Ansible Reboot System" execute="shutdown -r -t 1" frequency="once" time="4am" description="Scheduled System Reboot by ANSIBLE" enable=yes state=present user=SYSTEM

```

<!--- You can also paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
reboot Server

##### ACTUAL RESULTS
Windows Error Code 0x80070002 during Manual or scheduled execution of that task

Hints from issue discussion:
cc @petemounce
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
I don't use scheduled tasks myself but would you be able to put the shutdown command you want to run inside a .ps1 or .bat file and then schedule running the .bat or .ps1 file instead?

needs_info
I think the `argument:` parameter is meant to be used for adding options to the executable in `execute:`.

IMHO the options of this module are awkward and do not follow the parameter names used by other modules. Maybe we can change the default options and add the old ones as aliases. I would got for `executable:` and `arguments:`.

And for running something *once*, I would go for implementing `frequency: once`. Should not be that hard. I will prepare a PR if you are willing to test.
@littleprivate Please test #22611 (A theoretical implementation :-))

Commit message: Improve the integration test structure

I think this is a great way to test normal mode and check-mode from the same playbook.

Patch to review:
diff --git a/lib/ansible/modules/windows/win_scheduled_task.ps1 b/lib/ansible/modules/windows/win_scheduled_task.ps1
index 8804028345f8df..cfaa579e22930e 100644
--- a/lib/ansible/modules/windows/win_scheduled_task.ps1
+++ b/lib/ansible/modules/windows/win_scheduled_task.ps1
@@ -22,47 +22,34 @@ $ErrorActionPreference = "Stop"
 # WANT_JSON
 # POWERSHELL_COMMON
 
-$params = Parse-Args $args;
+$result = @{
+    changed = $false
+}
 
-$days_of_week = Get-AnsibleParam $params -name "days_of_week"
-$enabled = Get-AnsibleParam $params -name "enabled" -default $true
-$enabled = $enabled | ConvertTo-Bool
-$description = Get-AnsibleParam $params -name "description" -default " "
-$path = Get-AnsibleParam $params -name "path" -type "path"
-$argument = Get-AnsibleParam $params -name "argument"
+$params = Parse-Args $args -supports_check_mode $true
+$check_mode = Get-AnsibleParam -obj $params -name "_ansible_check_mode" -type "bool" -default $false
 
-$result = New-Object PSObject;
-Set-Attr $result "changed" $false;
+$arguments = Get-AnsibleParam -obj $params -name "arguments" -type "str" -aliases "argument"
+$description = Get-AnsibleParam -obj $params -name "description" -type "str" -default "No description."
+$enabled = Get-AnsibleParam -obj $params -name "enabled" -type "bool" -default $true
+$path = Get-AnsibleParam -obj $params -name "path" -type "str" -default '\'
 
-#Required vars
-$name = Get-AnsibleParam -obj $params -name name -failifempty $true -resultobj $result
-$state = Get-AnsibleParam -obj $params -name state -failifempty $true -resultobj $result -validateSet "present","absent"
+# Required vars
+$name = Get-AnsibleParam -obj $params -name "name" -type "str" -failifempty $true
+$state = Get-AnsibleParam -obj $params -name "state" -type "str" -default "present" -validateset "present","absent"
 
-#Vars conditionally required
-$present_args_required = $state -eq "present"
-$execute = Get-AnsibleParam -obj $params -name execute -failifempty $present_args_required  -resultobj $result
-$frequency = Get-AnsibleParam -obj $params -name frequency -failifempty $present_args_required -resultobj $result
-$time = Get-AnsibleParam -obj $params -name time -failifempty $present_args_required -resultobj $result
-$user = Get-AnsibleParam -obj $params -name user -failifempty $present_args_required -resultobj $result
+# Vars conditionally required
+$present = $state -eq "present"
+$executable = Get-AnsibleParam -obj $params -name "executable" -type "str" -aliases "execute" -failifempty $present
+$frequency = Get-AnsibleParam -obj $params -name "frequency" -type "str" -validateset "once","daily","weekly" -failifempty $present
+$time = Get-AnsibleParam -obj $params -name "time" -type "str" -failifempty $present
 
+# TODO: Default user to current user
+$user = Get-AnsibleParam -obj $params -name "user" -type "str" -failifempty $present
 
-# Mandatory Vars
-if ($frequency -eq "weekly")
-{
-    if (!($days_of_week))
-    {
-        Fail-Json $result "missing required argument: days_of_week"
-    }
-}
+$weekly = $frequency -eq "weekly"
+$days_of_week = Get-AnsibleParam -obj $params -name "days_of_week" -type "str" -failifempty $weekly
 
-if ($path)
-{
-  $path = "\{0}\" -f $path
-}
-else
-{
-  $path = "\"  #default
-}
 
 try {
     $task = Get-ScheduledTask -TaskPath "$path" | Where-Object {$_.TaskName -eq "$name"}
@@ -85,7 +72,9 @@ try {
         $exists = $true
     }
     elseif ( ($measure.count -eq 0) -and ($state -eq "absent") ){
-        Set-Attr $result "msg" "Task does not exist"
+        # Nothing to do
+        $result.exists = $false
+        $result.msg = "Task does not exist"
         Exit-Json $result
     }
     elseif ($measure.count -eq 0){
@@ -96,69 +85,77 @@ try {
         Fail-Json $result "$($measure.count) scheduled tasks found"
     }
 
-    Set-Attr $result "exists" "$exists"
+    $result.exists = $exists
 
     if ($frequency){
-        if ($frequency -eq "daily") {
-            $trigger =  New-ScheduledTaskTrigger -Daily -At $time
+        if ($frequency -eq "once") {
+            $trigger = New-ScheduledTaskTrigger -Once -At $time
+        }
+        elseif ($frequency -eq "daily") {
+            $trigger = New-ScheduledTaskTrigger -Daily -At $time
         }
         elseif ($frequency -eq "weekly"){
-            $trigger =  New-ScheduledTaskTrigger -Weekly -At $time -DaysOfWeek $days_of_week
+            $trigger = New-ScheduledTaskTrigger -Weekly -At $time -DaysOfWeek $days_of_week
         }
         else {
             Fail-Json $result "frequency must be daily or weekly"
         }
     }
 
-    if ( ($state -eq "absent") -and ($exists -eq $true) ) {
-        Unregister-ScheduledTask -TaskName $name -Confirm:$false
+    if ( ($state -eq "absent") -and ($exists) ) {
+        Unregister-ScheduledTask -TaskName $name -Confirm:$false -WhatIf:$check_mode
         $result.changed = $true
-        Set-Attr $result "msg" "Deleted task $name"
+        $result.msg = "Deleted task $name"
         Exit-Json $result
     }
-    elseif ( ($state -eq "absent") -and ($exists -eq $false) ) {
-        Set-Attr $result "msg" "Task $name does not exist"
+    elseif ( ($state -eq "absent") -and (-not $exists) ) {
+        $result.msg = "Task $name does not exist"
         Exit-Json $result
     }
 
     $principal = New-ScheduledTaskPrincipal -UserId "$user" -LogonType ServiceAccount
 
-    if ($enabled -eq $false){
-        $settings = New-ScheduledTaskSettingsSet -Disable
+    if ($enabled){
+        $settings = New-ScheduledTaskSettingsSet
     }
     else {
-        $settings = New-ScheduledTaskSettingsSet
+        $settings = New-ScheduledTaskSettingsSet -Disable
     }
 
-    if ($argument) {
-        $action = New-ScheduledTaskAction -Execute $execute -Argument $argument
+    if ($arguments) {
+        $action = New-ScheduledTaskAction -Execute $executable -Argument $arguments
     }
     else {
-        $action = New-ScheduledTaskAction -Execute $execute
+        $action = New-ScheduledTaskAction -Execute $executable
     }
 
-    if ( ($state -eq "present") -and ($exists -eq $false) ){
-        Register-ScheduledTask -Action $action -Trigger $trigger -TaskName $name -Description $description -TaskPath $path -Settings $settings -Principal $principal
-        $task = Get-ScheduledTask -TaskName $name
-        Set-Attr $result "msg" "Added new task $name"
+    if ( ($state -eq "present") -and (-not $exists) ){
+        if (-not $check_mode) {
+            Register-ScheduledTask -Action $action -Trigger $trigger -TaskName $name -Description $description -TaskPath $path -Settings $settings -Principal $principal
+#            $task = Get-ScheduledTask -TaskName $name
+        }
         $result.changed = $true
+        $result.msg = "Added new task $name"
     }
-    elseif( ($state -eq "present") -and ($exists -eq $true) ) {
-        if ($task.Description -eq $description -and $task.TaskName -eq $name -and $task.TaskPath -eq $path -and $task.Actions.Execute -eq $execute -and $taskState -eq $enabled -and $task.Principal.UserId -eq $user) {
+    elseif( ($state -eq "present") -and ($exists) ) {
+        $result.foobar = "($($task.Description) vs $description) -- ($($task.TaskName) vs $name) -- ($($task.TaskPath) vs $path) -- ($($task.Actions.Execute) vs $executable) -- ($taskState vs $enabled) -- ($($task.Principal.UserId) vs $user)"
+        if ($task.Description -eq $description -and $task.TaskName -eq $name -and $task.TaskPath -eq $path -and $task.Actions.Execute -eq $executable -and $taskState -eq $enabled -and $task.Principal.UserId -eq $user) {
             #No change in the task
-            Set-Attr $result "msg" "No change in task $name"
+            $result.msg = "No change in task $name"
         }
         else {
-            Unregister-ScheduledTask -TaskName $name -Confirm:$false
-            Register-ScheduledTask -Action $action -Trigger $trigger -TaskName $name -Description $description -TaskPath $path -Settings $settings -Principal $principal
-            Set-Attr $result "msg" "Updated task $name"
+            Unregister-ScheduledTask -TaskName $name -Confirm:$false -WhatIf:$check_mode
+            if (-not $check_mode) {
+                Register-ScheduledTask -Action $action -Trigger $trigger -TaskName $name -Description $description -TaskPath $path -Settings $settings -Principal $principal
+            }
             $result.changed = $true
+            $result.msg = "Updated task $name"
         }
     }
 
-    Exit-Json $result;
+    Exit-Json $result
 }
 catch
 {
   Fail-Json $result $_.Exception.Message
-}
\ No newline at end of file
+}
diff --git a/lib/ansible/modules/windows/win_scheduled_task.py b/lib/ansible/modules/windows/win_scheduled_task.py
index 807698655a0dc5..07d13f3cc38ed2 100644
--- a/lib/ansible/modules/windows/win_scheduled_task.py
+++ b/lib/ansible/modules/windows/win_scheduled_task.py
@@ -40,7 +40,6 @@
   description:
     description:
       - The description for the scheduled task
-    required: false
   enabled:
     description:
       - Enable/disable the task
@@ -58,30 +57,27 @@
   user:
     description:
       - User to run scheduled task as
-    required: false
-  execute:
+  executable:
     description:
       - Command the scheduled task should execute
-    required: false
-  argument:
+    aliases: [ execute ]
+  arguments:
     description:
       - Arguments to provide scheduled task action
-    required: false
+    aliases: [ argument ]
   frequency:
     description:
       - The frequency of the command, not idempotent
-    required: false
     choices:
+      - once
       - daily
       - weekly
   time:
     description:
       - Time to execute scheduled task, not idempotent
-    required: false
   days_of_week:
     description:
       - Days of the week to run a weekly task, not idempotent
-    required: false
   path:
     description:
       - Task folder in which this task will be stored
@@ -92,12 +88,13 @@
 # Create a scheduled task to open a command prompt
 - win_scheduled_task:
     name: TaskName
-    execute: cmd
-    frequency: daily
-    time: 9am
     description: open command prompt
+    executable: cmd
+    arguments: -opt1 -opt2
     path: example
-    enable: yes
+    time: 9am
+    frequency: daily
     state: present
+    enabled: yes
     user: SYSTEM
 '''
diff --git a/test/integration/targets/win_scheduled_task/aliases b/test/integration/targets/win_scheduled_task/aliases
new file mode 100644
index 00000000000000..c6d619816708c9
--- /dev/null
+++ b/test/integration/targets/win_scheduled_task/aliases
@@ -0,0 +1 @@
+windows/ci/group3
diff --git a/test/integration/targets/win_scheduled_task/tasks/main.yml b/test/integration/targets/win_scheduled_task/tasks/main.yml
new file mode 100644
index 00000000000000..1589a35f1515f8
--- /dev/null
+++ b/test/integration/targets/win_scheduled_task/tasks/main.yml
@@ -0,0 +1,37 @@
+# NOTE: The win_scheduled_task module only works on Win2012+
+
+- name: Test in normal mode (only run on Win2012+)
+  when: "'windows-2008' not in inventory_hostname"
+  block:
+  - include: tests.yml
+
+  - name: Check the various tasks in normal mode
+    assert:
+      that:
+      - add_scheduled_task.changed == true
+      - add_scheduled_task.exists == false
+      - add_scheduled_task_again.changed == false
+      - add_scheduled_task_again.exists == true
+      - remove_scheduled_task.changed == true
+      - remove_scheduled_task.exists == true
+      - remove_scheduled_task_again.changed == false
+      - remove_scheduled_task_again.exists == false
+
+
+- name: Test in check-mode (only run on Win2012+)
+  check_mode: yes
+  when: "'windows-2008' not in inventory_hostname"
+  block:
+  - include: tests.yml
+
+  - name: Check the various tests in check-mode
+    assert:
+      that:
+      - add_scheduled_task.changed == true
+      - add_scheduled_task.exists == false
+      - add_scheduled_task_again.changed == true
+      - add_scheduled_task_again.exists == false
+      - remove_scheduled_task.changed == false
+      - remove_scheduled_task.exists == false
+      - remove_scheduled_task_again.changed == false
+      - remove_scheduled_task_again.exists == false
diff --git a/test/integration/targets/win_scheduled_task/tasks/tests.yml b/test/integration/targets/win_scheduled_task/tasks/tests.yml
new file mode 100644
index 00000000000000..ddd58f1e5461fd
--- /dev/null
+++ b/test/integration/targets/win_scheduled_task/tasks/tests.yml
@@ -0,0 +1,24 @@
+- name: Add scheduled task
+  win_scheduled_task: &wst_present
+    name: Test
+    path: \
+    executable: dir.exe
+    arguments: C:\Windows\Temp\
+    frequency: once
+    time: 5pm
+    user: SYSTEM
+  register: add_scheduled_task
+
+- name: Add scheduled task (again)
+  win_scheduled_task: *wst_present
+  register: add_scheduled_task_again
+
+- name: Remove scheduled task
+  win_scheduled_task: &wst_absent
+    name: Test
+    state: absent
+  register: remove_scheduled_task
+
+- name: Remove scheduled task (again)
+  win_scheduled_task: *wst_absent
+  register: remove_scheduled_task_again


Code review comments:
diff_hunk: @@ -96,69 +85,77 @@ try {
         Fail-Json $result "$($measure.count) scheduled tasks found"
     }
 
-    Set-Attr $result "exists" "$exists"
+    $result.exists = $exists
 
     if ($frequency){
-        if ($frequency -eq "daily") {
-            $trigger =  New-ScheduledTaskTrigger -Daily -At $time
+        if ($frequency -eq "once") {
+            $trigger = New-ScheduledTaskTrigger -Once -At $time
+        }
+        elseif ($frequency -eq "daily") {
+            $trigger = New-ScheduledTaskTrigger -Daily -At $time
         }
         elseif ($frequency -eq "weekly"){
-            $trigger =  New-ScheduledTaskTrigger -Weekly -At $time -DaysOfWeek $days_of_week
+            $trigger = New-ScheduledTaskTrigger -Weekly -At $time -DaysOfWeek $days_of_week
         }
         else {
             Fail-Json $result "frequency must be daily or weekly"
         }
     }
 
-    if ( ($state -eq "absent") -and ($exists -eq $true) ) {
-        Unregister-ScheduledTask -TaskName $name -Confirm:$false
+    if ( ($state -eq "absent") -and ($exists) ) {
+        Unregister-ScheduledTask -TaskName $name -Confirm:$false -WhatIf:$check_mode
         $result.changed = $true
-        Set-Attr $result "msg" "Deleted task $name"
+        $result.msg = "Deleted task $name"
         Exit-Json $result
     }
-    elseif ( ($state -eq "absent") -and ($exists -eq $false) ) {
-        Set-Attr $result "msg" "Task $name does not exist"
+    elseif ( ($state -eq "absent") -and (-not $exists) ) {
+        $result.msg = "Task $name does not exist"
         Exit-Json $result
     }
 
     $principal = New-ScheduledTaskPrincipal -UserId "$user" -LogonType ServiceAccount
 
-    if ($enabled -eq $false){
-        $settings = New-ScheduledTaskSettingsSet -Disable
+    if ($enabled){
+        $settings = New-ScheduledTaskSettingsSet
     }
     else {
-        $settings = New-ScheduledTaskSettingsSet
+        $settings = New-ScheduledTaskSettingsSet -Disable
     }
 
-    if ($argument) {
-        $action = New-ScheduledTaskAction -Execute $execute -Argument $argument
+    if ($arguments) {
+        $action = New-ScheduledTaskAction -Execute $executable -Argument $arguments
     }
     else {
-        $action = New-ScheduledTaskAction -Execute $execute
+        $action = New-ScheduledTaskAction -Execute $executable
     }
 
-    if ( ($state -eq "present") -and ($exists -eq $false) ){
-        Register-ScheduledTask -Action $action -Trigger $trigger -TaskName $name -Description $description -TaskPath $path -Settings $settings -Principal $principal
-        $task = Get-ScheduledTask -TaskName $name
-        Set-Attr $result "msg" "Added new task $name"
+    if ( ($state -eq "present") -and (-not $exists) ){
+        if (-not $check_mode) {
+            Register-ScheduledTask -Action $action -Trigger $trigger -TaskName $name -Description $description -TaskPath $path -Settings $settings -Principal $principal
+#            $task = Get-ScheduledTask -TaskName $name
+        }
         $result.changed = $true
+        $result.msg = "Added new task $name"
     }
-    elseif( ($state -eq "present") -and ($exists -eq $true) ) {
-        if ($task.Description -eq $description -and $task.TaskName -eq $name -and $task.TaskPath -eq $path -and $task.Actions.Execute -eq $execute -and $taskState -eq $enabled -and $task.Principal.UserId -eq $user) {
+    elseif( ($state -eq "present") -and ($exists) ) {
+        $result.foobar = "($($task.Description) vs $description) -- ($($task.TaskName) vs $name) -- ($($task.TaskPath) vs $path) -- ($($task.Actions.Execute) vs $executable) -- ($taskState vs $enabled) -- ($($task.Principal.UserId) vs $user)"
path: lib/ansible/modules/windows/win_scheduled_task.ps1
text: @user1:
Debug cruft?

@author:
Ouch, indeed :) Was there to find out why it was not idempotent.

diff_hunk: @@ -0,0 +1,37 @@
+# NOTE: The win_scheduled_task module only works on Win2012+
+
+- name: Test in normal mode (only run on Win2012+)
path: test/integration/targets/win_scheduled_task/tasks/main.yml
text: @user1:
This only works for current shippable config- should use feature sniff instead, something like `raw: Get-Command New-ScheduledTask -ErrorAction SilentlyContinue; $?` with `failed_when: False` then skip the tests if the rc != 0. I guess we could use the OS version facts too...

@author:
That is clever. I now test for capabilities this way...