Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

win_share: tweaks after the caching mode changes #22763

Merged
merged 2 commits into from Mar 23, 2017

Conversation

jborean93
Copy link
Contributor

@jborean93 jborean93 commented Mar 18, 2017

SUMMARY

Some minor tweaks around win_share after some changes before 2.3 release. These changes are

  • Changed the default caching mode to Manual as that is what is set from the original module
  • Removed the Unknown caching mode option as that isn't a valid parameter
  • Added better error responses to return the error info
  • Fixed up the module info file to specify when this parameter was added
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

win_share

ANSIBLE VERSION
ansible 2.3.0.0 (stable-2.3 923c9ef17c) last updated 2017/03/17 09:12:40 (GMT +1000)
  config file = /apps/dev/playbook/jordan/mr-ansible/ansible.cfg
  configured module search path = Default w/o overrides
  python version = 2.7.12 (default, Sep 23 2016, 14:23:49) [GCC 4.4.7 20120313 (Red Hat 4.4.7-17)]
ADDITIONAL INFORMATION

Hoping to get this through before 2.3 as I believe this is a bug as now using this module changes the caching mode of a new share when not setting the mode to None when in fact the previous module before the changes set it to Manual

@jborean93
Copy link
Contributor Author

jborean93 commented Mar 18, 2017

@daBONDi I hope you don't mind but you and @nitzmahone were too quick for me with the issue I raised yesterday :). There are some things I changed around this which is mostly documentation stuff but the major areas is to change the default caching mode to Manual and removing the Unknown option as that doesn't seem to be a valid option.

I've double checked this by creating a new share manually with powershell using New-SmbShare to verify what caching mode it is set to the details are below

PS C:\Windows\system32> New-SmbShare -Name Test -Path C:\Dev

Name ScopeName Path   Description
---- --------- ----   -----------
Test *         C:\Dev

PS C:\Windows\system32> Get-SmbShare -Name Test | Select-Object *


PresetPathAcl         : System.Security.AccessControl.DirectorySecurity
ShareState            : Online
AvailabilityType      : NonClustered
ShareType             : FileSystemDirectory
FolderEnumerationMode : Unrestricted
CachingMode           : Manual
SmbInstance           : Default
CATimeout             : 0
ConcurrentUserLimit   : 0
ContinuouslyAvailable : False
CurrentUsers          : 0
Description           :
EncryptData           : False
Name                  : Test
Path                  : C:\Dev
Scoped                : False
ScopeName             : *
SecurityDescriptor    : O:SYG:SYD:(A;;0x1200a9;;;WD)
ShadowCopy            : False
Special               : False
Temporary             : False
Volume                : \\?\Volume{c2537151-4f13-4e8f-bc29-8a1bf9f990b1}\
PSComputerName        :
CimClass              : ROOT/Microsoft/Windows/SMB:MSFT_SmbShare
CimInstanceProperties : {AvailabilityType, CachingMode, CATimeout, ConcurrentUserLimit...}
CimSystemProperties   : Microsoft.Management.Infrastructure.CimSystemProperties

@ansibot ansibot added affects_2.4 This issue/PR affects Ansible v2.4 bugfix_pull_request core_review In order to be merged, this PR must follow the core review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. windows Windows community labels Mar 18, 2017
@daBONDi
Copy link
Contributor

daBONDi commented Mar 18, 2017

Default Value: Manual
i'm ok with it, and its problay technical wise the better choice but MS Default is "None" according tho this doc:
[https://technet.microsoft.com/en-us/library/jj635722(v=wps.630).aspx]
(https://technet.microsoft.com/en-us/library/jj635722(v=wps.630).aspx)

Option Unkown:
jborean93 Unkown is a Option , i don't know what it do and i could find the option in the current technet docs, don't know what it do, like the word "Unkown", maybe it spawns aliens or something :-)
https://technet.microsoft.com/en-us/itpro/powershell/windows/smb/share/new-smbshare

For me ok if i count something :-)

@jborean93
Copy link
Contributor Author

Thanks for the info, I didn't have a closer look in the technet documents and only looked at what it listed and not under the accepted values list. This got me interested and looking at the WMI class MSFT_SmbShare I can see Unknown as an option here. Still doesn't say what it means and any attempt at setting it to unknown fails, I've tried setting it through the *-SmbShare commands but I get the following errors.

Set-SmbShare -Name Test -CachingMode Unknown -Force

Set-SmbShare : The parameter is incorrect. 
At line:1 char:1
+ Set-SmbShare -Name Test -CachingMode Unknown -Force
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidArgument: (MSFT_SmbShare (...copeName = "*"):ROOT/Microsoft/Windows/SMB/MSFT_SMBShare) [Set-Smb 
   Share], CimException
    + FullyQualifiedErrorId : Windows System Error 87,Set-SmbShare

I then try to manipulate it through WMI itself but I get an even more unhelpful error below

$share = Get-WmiObject -Namespace root\Microsoft\Windows\SMB -Class MSFT_SmbShare -Filter "Name='Test'"
$share.CachingMode = 5
$share.Put()

Exception calling "Put" with "0" argument(s): "Not supported "
At line:3 char:1
+ $share.Put()
+ ~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], ParentContainsErrorRecordException
    + FullyQualifiedErrorId : DotNetMethodException

Ultimately I'll put the Unknown option back in but my curiosity is definitely high as to what the option does.

As for the the default action, while the technet docs specify none as the default I believe this just means when not specifying -CachingMode in the parameter it won't set any of the default listed. This doesn't mean that the None option is the default. I've done more testing in Ansible 2.2 to check what the default option is currently and it is pointing to Manual being the one. Unless there is anything else I am missing I still think this should be changed to Manual

Test Playbook

---
- name: test out share info
  hosts: windows

  tasks:
  - name: remove share
    win_share:
      name: Test
      state: absent

  - name: create share
    win_share:
      name: Test
      path: C:\ansible

  - name: get share info
    win_command: powershell.exe (Get-SmbShare -Name Test).CachingMode
    register: share_info

  - name: debug caching mode
    debug:
      var: share_info

Test Output

TASK [create share] ************************************************************
task path: /mnt/c/dev/test/role/main.yml:22
 [WARNING]: Module invocation had junk after the JSON data:

changed: [server2012R2] => {"changed": true}
 [WARNING]: Module invocation had junk after the JSON data:

changed: [server2016] => {"changed": true}
 [WARNING]: Module invocation had junk after the JSON data:

changed: [server2012] => {"changed": true}

TASK [get share info] **********************************************************
task path: /mnt/c/dev/test/role/main.yml:27
changed: [server2012R2] => {"changed": true, "cmd": "powershell.exe (Get-SmbShare -Name Test).CachingMode", "delta": "0:00:00.719527", "end": "2017-03-19 05:19:55.058263", "rc": 0, "start": "2017-03-19 05:19:54.338735", "stderr": "", "stdout": "Manual\r\n", "stdout_lines": ["Manual"], "warnings": []}
changed: [server2012] => {"changed": true, "cmd": "powershell.exe (Get-SmbShare -Name Test).CachingMode", "delta": "0:00:01.264534", "end": "2017-03-19 05:19:55.667623", "rc": 0, "start": "2017-03-19 05:19:54.403088", "stderr": "", "stdout": "Manual\r\n", "stdout_lines": ["Manual"], "warnings": []}
changed: [server2016] => {"changed": true, "cmd": "powershell.exe (Get-SmbShare -Name Test).CachingMode", "delta": "0:00:00.671876", "end": "2017-03-19 05:19:56.180181", "rc": 0, "start": "2017-03-19 05:19:55.508304", "stderr": "", "stdout": "Manual\r\n", "stdout_lines": ["Manual"], "warnings": []}

TASK [debug caching mode] ******************************************************
task path: /mnt/c/dev/test/role/main.yml:31
ok: [server2012] => {
    "share_info": {
        "changed": true,
        "cmd": "powershell.exe (Get-SmbShare -Name Test).CachingMode",
        "delta": "0:00:01.264534",
        "end": "2017-03-19 05:19:55.667623",
        "rc": 0,
        "start": "2017-03-19 05:19:54.403088",
        "stderr": "",
        "stdout": "Manual\r\n",
        "stdout_lines": [
            "Manual"
        ],
        "warnings": []
    }
}
ok: [server2012R2] => {
    "share_info": {
        "changed": true,
        "cmd": "powershell.exe (Get-SmbShare -Name Test).CachingMode",
        "delta": "0:00:00.719527",
        "end": "2017-03-19 05:19:55.058263",
        "rc": 0,
        "start": "2017-03-19 05:19:54.338735",
        "stderr": "",
        "stdout": "Manual\r\n",
        "stdout_lines": [
            "Manual"
        ],
        "warnings": []
    }
}
ok: [server2016] => {
    "share_info": {
        "changed": true,
        "cmd": "powershell.exe (Get-SmbShare -Name Test).CachingMode",
        "delta": "0:00:00.671876",
        "end": "2017-03-19 05:19:56.180181",
        "rc": 0,
        "start": "2017-03-19 05:19:55.508304",
        "stderr": "",
        "stdout": "Manual\r\n",
        "stdout_lines": [
            "Manual"
        ],
        "warnings": []
    }
}

@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Mar 20, 2017
@jborean93
Copy link
Contributor Author

Hey @nitzmahone any chance with merging this into 2.3 before the next RC is released?

@nitzmahone nitzmahone merged commit 89b78cb into ansible:devel Mar 23, 2017
nitzmahone pushed a commit that referenced this pull request Mar 23, 2017
* win_share: tweaks after the caching mode changes

* Re-added the Unknown option back in

(cherry picked from commit 89b78cb)
@nitzmahone
Copy link
Member

merged to devel and cherry-picked to stable-2.3 for 2.3.0RC2

@jborean93 jborean93 deleted the win_share-tweaks branch March 26, 2017 05:14
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 6, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.4 This issue/PR affects Ansible v2.4 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. module This issue/PR relates to a module. windows Windows community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants