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

MSFT_DFSNamespaceRoot/MSFT_DFSNamespaceFolder: Support for modifying Namespace Root Targets and Folder Target 'State' #96

Closed
thesmall opened this issue Jan 16, 2020 · 5 comments · Fixed by #126
Assignees
Labels
enhancement The issue is an enhancement request. in progress The issue is being actively worked on by someone.

Comments

@thesmall
Copy link

thesmall commented Jan 16, 2020

Details of the scenario you tried and the problem that is occurring

There is currently no way to provision a DFS Namespace Root Target or Folder Target in DFS in an Offline "State" (also defined as a target's "ReferralStatus")

Verbose logs showing the problem

This issue does not inherently manifest itself as an error. Its just currently not possible to specify a Root Target or Folder Target as being Offline. Resources defined with the current version of the DSC Resource are inherently "Online" if they are present. If they are absent, the resource is removed from the system entirely.

Suggested solution to the issue

  1. Change the schema.mof files for MSFT_DFSNamespaceRoot and MSFT_DFSNamespaceFolder to support a new required parameter State which would define the resource as having a ReferralStatus/State of either Online or Offline.
  2. Update the respective DSC Resources to support a new mandatory parameter, State, which would enable consumers of the DSC Resources to define DFS Namespace Root and Folder Targets with a State of "Offline".
  3. Update relevant Unit and Integration tests. Update relevant documentation and examples.

Note: The solution I am proposing would be considered a breaking change. It would define a new mandatory parameter.

The DSC configuration that is used to reproduce the issue (as detailed as possible)

##MSFT_DFSNamespaceRoot
#Current Implementation
DFSNamespaceRoot 'DFSRoot_dc01_Shares' {
	Ensure               = 'Present'
	Path                 = '\\domain.com\shares'
	TargetPath           = 'dc01.domain.com\shares'
	Type                 = 'DomainV2'
	Description          = 'Example Description'
	TimeToLiveSec        = 600
}

#Future Implementation (Online Root Target)
DFSNamespaceRoot 'DFSRoot_dc01_Shares' {
	Ensure               = 'Present'
	State                = 'Online'
	Path                 = '\\domain.com\shares'
	TargetPath           = 'dc01.domain.com\shares'
	Type                 = 'DomainV2'
	Description          = 'Example Description'
	TimeToLiveSec        = 600
}

#Future Implementation (Offline Root Target)
DFSNamespaceRoot 'DFSRoot_dc02_Shares' {
	Ensure               = 'Present'
	State                = 'Offline'
	Path                 = '\\domain.com\shares'
	TargetPath           = 'dc02.domain.com\shares'
	Type                 = 'DomainV2'
	Description          = 'Example Description'
	TimeToLiveSec        = 600
}

##MSFT_DFSNamespaceFolder
#Current Implementation
DFSNamespaceFolder 'DFSFolderTarget_shares_software' {
	Ensure               = 'Present'
	Path                 = \\domain.com\shares\software
	TargetPath           = \\dc01.domain.com\shares\software
	Description          = "Software Repository"
}

#Future Implementation (Online Folder Target)
DFSNamespaceFolder 'DFSFolderTarget_shares_software' {
	Ensure               = 'Present'
	Path                 = \\domain.com\shares\software
	TargetPath           = \\fs01.domain.com\shares\software
	Description          = "Software Repository"
	State                = 'Online'
}

#Future Implementation (Offline Folder Target)
DFSNamespaceFolder 'DFSFolderTarget_shares_software' {
	Ensure               = 'Present'
	Path                 = \\domain.com\shares\software
	TargetPath           = \\fs02.domain.com\shares\software
	Description          = "Software Repository"
	State                = 'Offline'
}

The operating system the target node is running

OsName               : Microsoft Windows Server 2016 Standard
OsOperatingSystemSKU : StandardServerEdition
OsArchitecture       : 64-bit
WindowsBuildLabEx    : 14393.3241.amd64fre.rs1_release_inmarket.190910-1801
OsLanguage           : en-US
OsMuiLanguages       : {en-US}

-->

Version and build of PowerShell the target node is running

Name                           Value
----                           -----
PSVersion                      5.1.14393.3053
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.14393.3053
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

Version of the DSC module that was used ('dev' if using current dev branch)

dev (4.4.0.0)

@thesmall
Copy link
Author

#97 Addresses this issue.

For what it's worth, this issue and associated PR could easily be converted into a non-breaking change by not requiring State to be a mandatory parameter, and instead setting a default value of "Online" for State.

@PlagueHO PlagueHO added enhancement The issue is an enhancement request. in progress The issue is being actively worked on by someone. labels Jan 21, 2020
@PlagueHO
Copy link
Member

Hi @thesmall - this is awesome! Thank you. I'll get onto the review this week. I'll get this PR through before I start the work to move this resource to the new continuous delivery release process.

I'm wondering if we should make the State default to 'Online' as you say. The reason is that I think most people would want to create the Namespace as online and assume it was the default. I could be wrong - so happy for any other comments or thoughts.

We should be OK with a BREAKING CHANGE though because when we move to the new CD process we'll be renaming all the resources from MSFT_ to DSC_.

@thesmall
Copy link
Author

thesmall commented Jan 21, 2020

Hi @PlagueHO, it should be easy enough to convert the State parameter to be non-mandatory, if that's the route we want to take. I guess my thinking for having it be a mandatory parameter is that you should be explicitly thinking about whether Roots Targets and Folder Targets should be "Online" or "Offline". This is in line with the general premise that you always need to specify either 'Present' or 'Absent' for the Ensure for DSC resources. Generally speaking I am of the opinion that if a parameter must have a value, it should be mandatory.

At the end of the day, I don't have a strong opinion either way. Since you are a maintainer of the repo, I will defer to your preference.

@PlagueHO
Copy link
Member

Hi @thesmall - I'm OK with making it mandatory. I don't have strong feelings either way. We haven't had any other community comments on this so I'm assuming no one else has a strong feeling either 😁

Ensure isn't always a mandatory parameter- in some resources it defaults. for example, WindowsFeature defaults to Ensure = 'Present' - as 95% of the time you're installing features rather than removing them.

When I'm figuring out if a parameter should be mandatory, I usually use a few criteria:

  1. Would a consumer of the resource in general expect it to default to a value?
  2. Does the underlying cmdlet that adds/sets the fields default the value? If the engineering team that built the cmdlets have defaulted the value then I'll tend to go with the default value they use. This is not always the case, just a guideline.
  3. Does specifying a default value increase the risk of misuse (e.g. in StorageDsc we have some destructive switches that we definitely don't want to default 😁).

Happy to stick with the parameter being mandatory. The examples and tests will need additional corrections to get them passing.

@thesmall
Copy link
Author

That sounds fine to me. Those criteria also make sense.

Regarding examples and tests: I am not so up to date on how to write/update relevant tests (pester and otherwise), though I have made some attempts in some un-pushed code I've got.

Is that something you might be able to guide me on if I push up some commits?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is an enhancement request. in progress The issue is being actively worked on by someone.
Projects
None yet
2 participants