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.

Sign up for GitHub

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

Duplicate of Abandoned PR #101: Handle FaultException race condition during domain creation & reboots. (Fixes #73) #137

Merged
merged 4 commits into from
Jan 11, 2017
Merged

Duplicate of Abandoned PR #101: Handle FaultException race condition during domain creation & reboots. (Fixes #73) #137

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
456f42b
Fixed the databasePath bug encountered during DC provisioning
Jun 18, 2016
be1bbe6
Fixed tab vs space issue raised in AppVeyor
Jun 18, 2016
ee7551f
Added information to README.md
Jun 18, 2016
3b0a8fa
Add comment for tracking file and remove alias
kwirkykat Jan 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 56 additions & 19 deletions DSCResources/MSFT_xADDomain/MSFT_xADDomain.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,37 @@ data localizedData
{
# culture="en-US"
ConvertFrom-StringData @'
RoleNotFoundError = Please ensure that the PowerShell module for role '{0}' is installed.
InvalidDomainError = Computer is a member of the wrong domain?!
ExistingDomainMemberError = Computer is already a domain member. Cannot create a new '{0}' domain?
InvalidCredentialError = Domain '{0}' is available, but invalid credentials were supplied.

QueryDomainWithLocalCredential = Computer is a domain member; querying domain '{0}' using local credential ...
QueryDomainWithCredential = Computer is a workgroup member; querying for domain '{0}' using supplied credential ...
DomainFound = Active Directory domain '{0}' found.
DomainNotFound = Active Directory domain '{0}' cannot be found.
CreatingChildDomain = Creating domain '{0}' as a child of domain '{1}' ...
CreatedChildDomain = Child domain '{0}' created.
CreatingForest = Creating AD forest '{0}' ...
CreatedForest = AD forest '{0}' created.
ResourcePropertyValueIncorrect = Property '{0}' value is incorrect; expected '{1}', actual '{2}'.
ResourceInDesiredState = Resource '{0}' is in the desired state.
ResourceNotInDesiredState = Resource '{0}' is NOT in the desired state.
RoleNotFoundError = Please ensure that the PowerShell module for role '{0}' is installed.
InvalidDomainError = Computer is a member of the wrong domain?!
ExistingDomainMemberError = Computer is already a domain member. Cannot create a new '{0}' domain?
InvalidCredentialError = Domain '{0}' is available, but invalid credentials were supplied.

QueryDomainWithLocalCredential = Computer is a domain member; querying domain '{0}' using local credential ...
QueryDomainWithCredential = Computer is a workgroup member; querying for domain '{0}' using supplied credential ...
DomainFound = Active Directory domain '{0}' found.
DomainNotFound = Active Directory domain '{0}' cannot be found.
CreatingChildDomain = Creating domain '{0}' as a child of domain '{1}' ...
CreatedChildDomain = Child domain '{0}' created.
CreatingForest = Creating AD forest '{0}' ...
CreatedForest = AD forest '{0}' created.
ResourcePropertyValueIncorrect = Property '{0}' value is incorrect; expected '{1}', actual '{2}'.
ResourceInDesiredState = Resource '{0}' is in the desired state.
ResourceNotInDesiredState = Resource '{0}' is NOT in the desired state.
RetryingGetADDomain = Attempt {0} of {1} to call Get-ADDomain failed, retrying in {2} seconds.
UnhandledError = Unhandled error occured, detail here: {0}
FaultExceptionAndDomainShouldExist = ServiceModel FaultException detected and domain should exist, performing retry...

'@
}

function Get-TrackingFilename {
param(
[Parameter(Mandatory)]
[String] $DomainName
)
Join-Path $Env:TEMP ('{0}.xADDomain.completed' -f $DomainName)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be in temp or the resource cache? Resource cache is automatically cleared when a configuration is updated. Temp of course is not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have left it as temp because if the resource is called again and a domain promotion has already occurred, the resource should not attempt to promote the machine to a domain controller again. The second attempt to promote the domain controller would throw the same error this code is trying to fix.
I have added a comment to the code explaining this and providing the resource cache location if it is needed in the future.

}

function Get-TargetResource
{
[OutputType([System.Collections.Hashtable])]
Expand Down Expand Up @@ -59,6 +71,11 @@ function Get-TargetResource
$domainFQDN = Resolve-DomainFQDN -DomainName $DomainName -ParentDomainName $ParentDomainName;
$isDomainMember = Test-DomainMember;

$retries = 0
$maxRetries = 5
$retryIntervalInSeconds = 30
$domainShouldExist = (Test-Path (Get-TrackingFilename -DomainName $DomainName))
do {
try
{
if ($isDomainMember) {
Expand Down Expand Up @@ -93,6 +110,7 @@ function Get-TargetResource
{
Write-Verbose ($localizedData.DomainNotFound -f $domainFQDN)
$domain = @{ };
# will fall into retry mechanism
}
catch [System.Security.Authentication.AuthenticationException]
{
Expand All @@ -101,10 +119,27 @@ function Get-TargetResource
}
catch
{
## Not sure what's gone on here!
throw $_
$errorMessage = $localizedData.UnhandledError -f ($_.Exception | Format-List -Force | Out-String)
Write-Verbose $errorMessage

if ($domainShouldExist -and ($_.Exception.InnerException -is [System.ServiceModel.FaultException]))
{
Write-Verbose $localizedData.FaultExceptionAndDomainShouldExist
# will fall into retry mechanism
} else {
## Not sure what's gone on here!
throw $_
}
}

if($domainShouldExist) {
$retries++
Write-Verbose ($localizedData.RetryingGetADDomain -f $retries, $maxRetries, $retryIntervalInSeconds)
Sleep -Seconds ($retries * $retryIntervalInSeconds)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use an alias

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

}

} while ($domainShouldExist -and ($retries -le $maxRetries) )

} #end function Get-TargetResource

function Test-TargetResource
Expand Down Expand Up @@ -265,9 +300,11 @@ function Set-TargetResource
$installADDSParams['DomainNetbiosName'] = $DomainNetBIOSName;
}
Install-ADDSForest @installADDSParams;
Write-Verbose -Message ($localizedData.CreatedForest);
Write-Verbose -Message ($localizedData.CreatedForest -f $DomainName);
}

"Finished" | Out-File -FilePath (Get-TrackingFilename -DomainName $DomainName) -Force

# Signal to the LCM to reboot the node to compensate for the one we
# suppressed from Install-ADDSForest/Install-ADDSDomain
$global:DSCMachineStatus = 1
Expand Down
2 changes: 2 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,8 @@ Setting an ODJ Request file path for a configuration that creates a computer acc
### Unreleased

* xAdDomainController: Update to complete fix for SiteName being required field.
* xADDomain: Added retry logic to prevent FaultException to crash in Get-TargetResource on subsequent reboots after a domain is created because the service is not yet running. This error is mostly occur when the resource is used with the DSCExtension on Azure.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either add tests or add an issue to follow up

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since multiple contributors in #73 have confirmed that this fix works, I will skip adding tests for now and open an issue to add tests for this fix later.


### 2.15.0.0
* xAdDomainController: Fixes SiteName being required field.

Expand Down