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

Bump dbatools to v2.0.1 #1926

Merged
merged 7 commits into from
May 6, 2023
Merged

Conversation

johlju
Copy link
Member

@johlju johlju commented May 1, 2023

Pull Request (PR) description

  • SqlServerDsc
    • Bumped dbatools to v2.0.1 for the integration tests.

This Pull Request (PR) fixes the following issues

None.

Task list

  • Added an entry to the change log under the Unreleased section of the
    file CHANGELOG.md. Entry should say what was changed and how that
    affects users (if applicable), and reference the issue being resolved
    (if applicable).
  • Resource documentation updated in the resource's README.md.
  • Resource parameter descriptions updated in schema.mof.
  • Comment-based help updated, including parameter descriptions.
  • Localization strings updated.
  • Examples updated.
  • Unit tests updated. See DSC Community Testing Guidelines.
  • Integration tests updated (where possible). See DSC Community Testing Guidelines.
  • Code changes adheres to DSC Community Style Guidelines.

This change is Reviewable

@johlju johlju added the needs review The pull request needs a code review. label May 1, 2023
@codecov
Copy link

codecov bot commented May 1, 2023

Codecov Report

Merging #1926 (0927ee4) into main (d9a84f4) will not change coverage.
The diff coverage is n/a.

❗ Current head 0927ee4 differs from pull request most recent head adf03df. Consider uploading reports for the commit adf03df to get more accurate results

Impacted file tree graph

@@         Coverage Diff          @@
##           main   #1926   +/-   ##
====================================
  Coverage    91%     91%           
====================================
  Files        91      91           
  Lines      7788    7788           
====================================
  Hits       7164    7164           
  Misses      624     624           
Flag Coverage Δ
unit 91% <ø> (ø)

@johlju
Copy link
Member Author

johlju commented May 2, 2023

@potatoqualitee it looks like dbatools does not pin the dbatools.library correctly. I see that the dbatools.library version differ between jobs. PR 1915 uses the preview7 version and we can see that it downloads the same dbatools.library version as 2.0.0 GA. Looking at the build for the full release of SqlServerDsc the other week it passed since it used a different version of dbatools.library.

Passed SqlServerDsc build dbatools using dbatools.library CI Job
✔️ 16.3.0 2.0.0-preview7 2023.4.18 16.3.0
PR 1915 2.0.0-preview7 2023.4.27 16.4.0-PR1915.29
PR 1926 2.0.0 2023.4.27 16.4.0-PR1926.5

The build for this PR was run on both Win 2019 and Win 2022 and both faild with the System.Runtime error.

Downloading the module to a Lab server and running import-module when the environment variable is set fails too:

image

@potatoqualitee
Copy link

What on earth? I can't figure this out. Can you give me $PSVersionTable on that PS Console?

@johlju
Copy link
Member Author

johlju commented May 2, 2023

Here you go:
image

image

PS C:\Users\sqladmin> Get-ComputerInfo | fl OsArchitecture, OSLanguage, WindowsProductName, OSDisplayVersion

OsArchitecture     : 64-bit
OsLanguage         : en-US
WindowsProductName : Windows Server 2022 Standard
OSDisplayVersion   : 21H2

@potatoqualitee
Copy link

okay this give me a good amount of info. ill have to test the other library but this one seems to require a slightly higher version .net than is available. which sounds crazay bc as far as I know, we require a version from 2018. you're on 4.0 and we require 4.x. let me do some digging and at the very least, we'll throw a useful error message.

@potatoqualitee
Copy link

2022! what! okay gimme some time and i'll get this addressed but i think it's incompatible .net versions.

@johlju
Copy link
Member Author

johlju commented May 2, 2023

Thank you. Let me know if I should try to install another .net version in the pipeline. 🙂

@potatoqualitee
Copy link

But it worked the other day! I'll figure this out again 😅

@potatoqualitee
Copy link

sweet, i can repro 👍🏼 ill figure this out, fix and release then let you know

@potatoqualitee
Copy link

Got it. I had to revert to some other version of the DLL 🤷🏼 I also added a test to ensure the DLLs load.

Do you remember that script you shared in a comment that did a quick test? I'd like to add it to our test suite to ensure your tests always pass.

@potatoqualitee
Copy link

With that said, dbatools 2.0.1 should work for you 👍🏼

@johlju
Copy link
Member Author

johlju commented May 6, 2023

Do you remember that script you shared in a comment that did a quick test? I'd like to add it to our test suite to ensure your tests always pass.

You mean the repro here: #1904 (comment)
I think this script will be enough to test this last issue. To check that $analysisServicesObject is correct type:

$env:SMODefaultModuleName = 'dbatools'
import-module -Name 'dbatools'
$analysisServicesObject = New-Object -TypeName 'Microsoft.AnalysisServices.Server'

@johlju
Copy link
Member Author

johlju commented May 6, 2023

With that said, dbatools 2.0.1 should work for you

Thank you! Will bump to that version. 🙇‍♂️

@johlju johlju force-pushed the fix/bump-dbatools-version branch from 32707ae to f85d4ce Compare May 6, 2023 09:56
@johlju johlju changed the title Bump dbatools to 2.0.0 Bump dbatools to 2.0.1 May 6, 2023
@johlju johlju changed the title Bump dbatools to 2.0.1 Bump dbatools to v2.0.1 May 6, 2023
@potatoqualitee
Copy link

it was the "Script"command that helped ensure SqlServerDsc could uhh make a mof? Quite a few lines.

@johlju
Copy link
Member Author

johlju commented May 6, 2023

The first issue was that when importing the module inside the LCM’s PowerShell process it failed. If you want to run a DSC resource with LCM we could make an easier integration test using Invoke-DscResource. I can look into it tomorrow and see I can simplify it a bit.

@johlju johlju force-pushed the fix/bump-dbatools-version branch from 0927ee4 to adf03df Compare May 6, 2023 15:04
Copy link
Member Author

@johlju johlju left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: 2 of 4 files reviewed, all discussions resolved

@johlju johlju merged commit 7d58bf5 into dsccommunity:main May 6, 2023
@johlju johlju deleted the fix/bump-dbatools-version branch May 6, 2023 15:07
@johlju johlju removed the needs review The pull request needs a code review. label May 6, 2023
@potatoqualitee
Copy link

🥳

@johlju
Copy link
Member Author

johlju commented May 6, 2023

@potatoqualitee There are two options to run integration tests. All require Windows PowerShell and a SQL Server instance available to connect to to create a SQL login. Option 2 should be safest and will definitely work. Let me know if you get in any problem and I can help. 🙂

For both options this should be run (change paths as required):

# Remove any installed SqlServer och SQLPS modules from the build worker (host that runs the integration tests).
# See here for a function that could be inspiration (or re-use): https://github.com/dsccommunity/SqlServerDsc/blob/7d58bf5dfc4d951778ed5f3fe19821ddfb309ca8/tests/TestHelpers/CommonTestHelper.psm1#L442-L517

Set-ExecutionPolicy RemoteSigned -Force
winrm quickconfig -quiet
Install-Module SqlServerDsc -RequiredVersion 16.3.1 -Scope AllUsers -Force
Install-Module PSDscResources -RequiredVersion '2.12.0.0' -Scope AllUsers -Force
Copy-Item -Path './dbatools' -Destination 'C:\Program Files\WindowsPowerShell\Modules' -Recurse -Force
Copy-Item -Path './dbatools.library' -Destination 'C:\Program Files\WindowsPowerShell\Modules' -Recurse -Force

Option #1

If you make sure the machine environment variable is set prior to LCM is run this might work, so the LCM's PowerShell process sees the environment variable, this might be sufficient (but if it requires a restart then this option is not gonna work). If it works it should NOT output a verbose statement saying No preferred PowerShell module was found. Change values as required.

$loginPassword = [PSCredential]::new(
    'NotUsed',
    ('MyPassword' | ConvertTo-SecureString -AsPlainText -Force)
)

Invoke-DscResource -ModuleName 'SqlServerDsc' -Name 'SqlLogin' -Method 'Get' -Property @{
    Ensure                         = 'Present'
    Name                           = 'TestDSCUser'
    LoginType                      = 'SqlLogin'
    ServerName                     = 'TestServer.company.local'
    InstanceName                   = 'DSC'
    LoginCredential                = $loginPassword
    LoginMustChangePassword        = $false
    LoginPasswordExpirationEnabled = $true
    LoginPasswordPolicyEnforced    = $true

    # If SYSTEM is not allowed to connect to the SQl instance, set this to credentials allowed to connect to SQL instance.
    #PsDscRunAsCredential           = $SqlAdministratorCredential 
} -Verbose

Option #2

If above Invoke-DscResource does not work (if it can't see the environment variable) then this is required. The below compiles a MOF configuration and pushes it to the local node, then make the node forget it had a configuration pushed. Change values as required.

$loginPassword = [PSCredential]::new(
    'NotUsed',
    ('MyPassword' | ConvertTo-SecureString -AsPlainText -Force)
)


Configuration Example
{
    Import-DscResource -ModuleName PSDscResources
    Import-DscResource -ModuleName SqlServerDsc

    node localhost
    {
        Environment 'SetSMODefaultModuleName'
        {
            Name = 'SMODefaultModuleName'
            Value = 'dbatools'
            Ensure = 'Present'
            Path = $false
            Target = @('Process', 'Machine')
        }

        SqlLogin 'CreateSqlLogin'
        {
            Ensure                         = 'Present'
            Name                           = 'TestDSCUser'
            LoginType                      = 'SqlLogin'
            ServerName                     = 'TestServer.company.local'
            InstanceName                   = 'DSC'
            LoginCredential                = $loginPassword
            LoginMustChangePassword        = $false
            LoginPasswordExpirationEnabled = $true
            LoginPasswordPolicyEnforced    = $true

            # If SYSTEM is not allowed to connect to the SQl instance, set this to credentials allowed to connect to SQL instance.
            #PsDscRunAsCredential           = $SqlAdministratorCredential
        }
    }
}

$configurationData = @{
    AllNodes    = , @{
        NodeName        = 'localhost'
        PSDSCAllowPlainTextPassword = $true
        PSDscAllowDomainUser = $true
    }
}

Example -OutputPath c:\DSC -ConfigurationData $configurationData

Start-DscConfiguration -Wait -Force -Path C:\DSC -ComputerName localhost -Verbose

Remove-DscConfigurationDocument -Stage Current,Pending,Previous -Force -Verbose

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants