pwsh experimental/fixes #175

briantist · 2023-02-20T14:51:42Z

Description

fixes sanity failures in the experimental plugins (unused imports, see ansible-test unused-import pylint rule to be enabled ansible-collections/news-for-maintainers#34)
fixes the C# compilation problems, see:
- [Regression] Add-Type fails with CS1701 when referencing Newtonsoft.Json on daily build of PS Core PowerShell/PowerShell#18684
- Ansible.Basic.cs - Fix compile error on PS 7.3.x ansible/ansible#79848
pins the devel test container to 7.4.0 in order to have PowerShell 7.2.x instead of 7.3.x
- there's an interaction between dbatools and 7.3 when called from ansible, see other comments

How Has This Been Tested?

CI

Types of changes

Bug fix (non-breaking change which fixes an issue) - Fixes #
New feature (non-breaking change which adds functionality)

Checklist:

I have read/followed the CONTRIBUTING document.
I have read/followed the PR Quick Start Guide
I have added tests to cover my changes or they are N/A.
I have added a changelog fragment describing the changes.
New module options/parameters include a version_added property.

codecov-commenter · 2023-02-20T15:14:05Z

Codecov Report

Base: 93.78% // Head: 93.78% // No change to project coverage 👍

Coverage data is based on head (f48e84c) compared to base (3051f0a).
Patch has no changes to coverable lines.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #175   +/-   ##
=======================================
  Coverage   93.78%   93.78%           
=======================================
  Files          64       64           
  Lines        2188     2188           
=======================================
  Hits         2052     2052           
  Misses        136      136

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

briantist · 2023-02-20T21:20:43Z

After getting through the initial failures (related to the compilation problems) by updating the experimental plugins, we have two new issues.

On 2.12 only:

Exception calling "Create" with "3" argument(s): "EventLog access is not supported on this platform."

This doesn't make sense... it implies that the Ansible.Basic module being used is the one included with ansible (which doesn't have the changes from 2.13 that make it work), but the changes I made to the experimental plugins should ensure that 2.12 uses the vendored modified module utils in the role. I confirmed locally that this works, the conditional for the ansible version is working and using the local copies. I don't have an answer yet for why this is (seemingly) failing in CI.

2.13 and 2.14 work as expected; it's worth noting that the compilation issue only comes up in PowerShell 7.3, and the test container for those doesn't include it, so this mainly shows that the modifications to the experimental plugins are working correctly on these versions (using the module utils included in those core versions).

devel now has a different problem that also stems from PowerShell 7.3.

The 'Get-DbaDbUser' command was found in the module 'dbatools', but the module could not be loaded. For more information, run 'Import-Module dbatools'.
At line:45 char:17
+ $existingUser = Get-DbaDbUser @getUserSplat
+                 ~~~~~~~~~~~~~
    + CategoryInfo          : ObjectNotFound: (Get-DbaDbUser:String) [], ParentContainsErrorRecordException
    + FullyQualifiedErrorId : CouldNotAutoloadMatchingModule

Every invocation of a dbatools command gives an auto-load error like this.

This might to be related to this change (maybe):

Move psxml type definitions from SMA to Security module PowerShell/PowerShell#16355

I can't actually confirm that though, it might be unrelated.

I thought I'd check to see if doing an explicit Import-Module instead of just using #Requires might help it: 97a1904

In CI we can see that this failed spectacularly: https://github.com/lowlydba/lowlydba.sqlserver/actions/runs/4225543845/jobs/7337956510#step:9:197

Basically it just acted as though the module doesn't exist, but it should have been installed in the previous step.
Side note, do you remember why no_log was set here?
https://github.com/lowlydba/lowlydba.sqlserver/blob/main/tests/integration/targets/setup_sqlserver/tasks/main.yml#L6

When I tried this import thing locally, it failed in a different but also incredibly confusing way:

Full traceback

The full traceback is:
Exception calling "Invoke" with "3" argument(s): "The running command stopped because the preference variable "ErrorActionPreference" or common parameter is set to Stop: Could not import /home/briantist/.local/share/powershell/Modules/dbatools/1.1.112/bin/smo/coreclr/Microsoft.SqlServer.XEvent.Linq.dll :
MethodInvocationException: /home/briantist/.local/share/powershell/Modules/dbatools/1.1.112/internal/scripts/libraryimport.ps1:150
Line |
 150 |                      [Reflection.Assembly]::LoadFrom($assemblyPath)
     |                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Exception calling "LoadFrom" with "1" argument(s): "Could not load file or assembly '/home/briantist/.local/share/powershell/Modules/dbatools/1.1.112/bin/smo/coreclr/Microsoft.SqlServer.XEvent.Linq.dll'. An attempt was made to load a program with an incorrect format. "

"
At /home/briantist/.local/share/powershell/Modules/dbatools/1.1.112/internal/scripts/libraryimport.ps1:161 char:50
+ … Invoke($script:PSModuleRoot, "$(Join-Path $script:DllRoot smo)", $scr …
+                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], ParentContainsErrorRecordException
    + FullyQualifiedErrorId : ActionPreferenceStopException

ScriptStackTrace:
at <ScriptBlock>, /home/briantist/.local/share/powershell/Modules/dbatools/1.1.112/internal/scripts/libraryimport.ps1: line 161
at <ScriptBlock>, /home/briantist/.local/share/powershell/Modules/dbatools/1.1.112/dbatools.psm1: line 214
at <ScriptBlock>, <No file>: line 10
fatal: [localhost]: FAILED! => {
    "changed": false,
    "msg": "Unhandled exception while executing module: Exception calling \"Invoke\" with \"3\" argument(s): \"The running command stopped because the preference variable \"ErrorActionPreference\" or common parameter is set to Stop: Could not import /home/briantist/.local/share/powershell/Modules/dbatools/1.1.112/bin/smo/coreclr/Microsoft.SqlServer.XEvent.Linq.dll : \nMethodInvocationException: /home/briantist/.local/share/powershell/Modules/dbatools/1.1.112/internal/scripts/libraryimport.ps1:150\nLine |\n 150 |                      [Reflection.Assembly]::LoadFrom($assemblyPath)\n     |                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n     | Exception calling \"LoadFrom\" with \"1\" argument(s): \"Could not load file or assembly '/home/briantist/.local/share/powershell/Modules/dbatools/1.1.112/bin/smo/coreclr/Microsoft.SqlServer.XEvent.Linq.dll'. An attempt was made to load a program with an incorrect format. \"\n\n\""
}

So that's the current state... :(

briantist · 2023-02-21T00:05:23Z

Ok so I've solved some of the above mysteries:

The 2.12 issue was caused by the fact that the hack in the shell plugin was only being executed in non-pipelined mode
- Previously, this worked anyway in pipelined mode, I think because the module_utils folder in the role was still used even when the files were not explicitly read and injected
- But with the conditional change I added in this PR, I had to rename the module_utils files specifically to avoid them being used implicitly, so when pipelining was being used on 2.12, it avoided the conditional, and therefore avoided the injection, and tried to use the insufficient version that comes with core 2.12
- fix was pretty easy, just had to move the hack before the pipelining check
- the reason it WorkedOnMyMachine™ is that for whatever reason, pipelining wasn't being used with my test playbook, but in the ansible-test container, it is used (I was able to repro even locally when running through ansible-test --docker)
The devel issue with the loading remains, but some things are clearer now
- the reason it acted strange in CI (and in local test containers) when I added explicit imports is that I messed up the syntax; I used -RequiredVersion instead of -MinimumVersion
- since the version specified is a minimum, the version installed is actually higher, so when I tried to import exactly that version it was of course missing
- this also worked differently for me locally (outside of the test container) because I had manually installed both the minimum and the latest version, so both were available
- SO we're back to devel being unable to autoload the module, and
- if we try to explicitly import (with the right command now), we're back to the issues of not being able to load some SMO assemblies, see below
- it's possible that the explicit and implicit loading issues are the same, and that the difference is just that the explicit loading gives a better error message
- if we can get explicit working that would be a good sign, but I can't find any good reason why this is failing this way
- also something I forgot to mention before that makes this harder to troubleshoot: importing dbatools within a pwsh session seems to work just fine, so I have no idea why it's failing from within ansible, but it is minimally reproducible, see below

Error with explicit import in the ansible module:

The full traceback is:
Exception calling "Invoke" with "3" argument(s): "The running command stopped because the preference variable "ErrorActionPreference" or common parameter is set to Stop: Could not import /root/.local/share/powershell/Modules/dbatools/1.1.145/bin/smo/coreclr/Microsoft.SqlServer.XE.Core.dll :
MethodInvocationException: /root/.local/share/powershell/Modules/dbatools/1.1.145/internal/scripts/libraryimport.ps1:150
Line |
 150 |                      [Reflection.Assembly]::LoadFrom($assemblyPath)
     |                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Exception calling "LoadFrom" with "1" argument(s): "Could not load file or assembly '/root/.local/share/powershell/Modules/dbatools/1.1.145/bin/smo/coreclr/Microsoft.SqlServer.XE.Core.dll'. An attempt was made to load a program with an incorrect format. "

"
At /root/.local/share/powershell/Modules/dbatools/1.1.145/internal/scripts/libraryimport.ps1:161 char:50
+ … Invoke($script:PSModuleRoot, "$(Join-Path $script:DllRoot smo)", $scr …
+                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], ParentContainsErrorRecordException
    + FullyQualifiedErrorId : ActionPreferenceStopException

ScriptStackTrace:
at <ScriptBlock>, /root/.local/share/powershell/Modules/dbatools/1.1.145/internal/scripts/libraryimport.ps1: line 161
at <ScriptBlock>, /root/.local/share/powershell/Modules/dbatools/1.1.145/dbatools.psm1: line 226
at <ScriptBlock>, <No file>: line 10
fatal: [testhost]: FAILED! => {
    "changed": false,
    "msg": "Unhandled exception while executing module: Exception calling \"Invoke\" with \"3\" argument(s): \"The running command stopped because the preference variable \"ErrorActionPreference\" or common parameter is set to Stop: Could not import /root/.local/share/powershell/Modules/dbatools/1.1.145/bin/smo/coreclr/Microsoft.SqlServer.XE.Core.dll : \nMethodInvocationException: /root/.local/share/powershell/Modules/dbatools/1.1.145/internal/scripts/libraryimport.ps1:150\nLine |\n 150 |                      [Reflection.Assembly]::LoadFrom($assemblyPath)\n     |                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n     | Exception calling \"LoadFrom\" with \"1\" argument(s): \"Could not load file or assembly '/root/.local/share/powershell/Modules/dbatools/1.1.145/bin/smo/coreclr/Microsoft.SqlServer.XE.Core.dll'. An attempt was made to load a program with an incorrect format. \"\n\n\""
}

MCVE:

ansible localhost -m command -a 'pwsh -c "ipmo dbatools -verbose"'

the above seems to happen no matter which version of ansible, so I wonder if it's solely related to the version of pwsh

briantist · 2023-02-25T19:39:20Z

thanks @jborean93 for help troubleshooting

It seems that there are assemblies in dbatools that don't load because they are for the wrong platform, and that this has been the case for a while. For some reason, PowerShell versions before 7.3 hid these(?), and for whatever reason these errors are now showing up in some error stream. They seem to still be ignored by PowerShell sort of, you don't normally see them, but things like Ansible look at the error stream and see them.

repros for showing the dbatools error:

Ansible

ansible localhost -m command -a 'pwsh -c "ipmo dbatools -verbose"'

PowerShell

$ErrorActionPreference = "Stop"
Import-Module dbatools

NOTE: ipmo dbatools -ea Stop does not produce the error

$ps = [PowerShell]::Create()
$ps.AddScript('Import-Module dbatools').Invoke()
$ps.Streams.Error

lowlydba · 2023-02-26T14:18:23Z

Thanks so much for all this work and investigation 🙇🏻 !

Side note, do you remember why no_log was set here? https://github.com/lowlydba/lowlydba.sqlserver/blob/main/tests/integration/targets/setup_sqlserver/tasks/main.yml#L6

I think I had tried a few methods to make the install less verbose since it was writing way too many lines out in noninteractive mode, but had trouble and just used no_log as a workaround.

When I tried this import thing locally, it failed in a different but also incredibly confusing way:
....
So that's the current state... :(

That behavior looks to be captured by dataplat/dbatools#8657. As wsmelton mentions there, apparently pwsh 7.3 included the move to .NET 7 so that probably explains the abrupt change in behavior. I'm going to look into it from that angle a bit, but have low expectations.

lowlydba · 2023-02-26T14:48:29Z

The preview of 2.0 works!

Quick repro:

Install-Module dbatools -AllowPrerelease -Force
ansible localhost -m command -a 'pwsh -c "ipmo dbatools -verbose"'

I don't get the impression that < 2.0 will get any of the needed fixes backported, it probably makes sense to do an enhancement that checks for 2.0+ when the pwsh version is >= 7.3 in the CI. It might also be worth it to set the new min version to 2.0 once it is finally released to avoid any confusing compatibility issues as well.

briantist · 2023-02-26T16:48:24Z

imo, let's merge this, and use #177 and a new PR to deal with 2.0 if <2 is never fixed, wdyt?

lowlydba · 2023-02-26T18:37:54Z

imo, let's merge this, and use #177 and a new PR to deal with 2.0 if <2 is never fixed, wdyt?

You read my mind.

briantist added 2 commits February 20, 2023 09:37

remove unused imports

5c7e2a6

try to use hack only in required ansible versions

f64fcbf

briantist added the bug Something isn't working label Feb 20, 2023

briantist marked this pull request as draft February 20, 2023 14:54

briantist added 3 commits February 20, 2023 10:33

change modified utils names

c803091

update Ansible.Basic from upstream

2d3d52f

try something

97a1904

briantist added 2 commits February 20, 2023 16:22

revert imports

a8a709d

move hack to the right place for pipelining

080b740

briantist added 3 commits February 25, 2023 14:21

pin devel container

6965d07

matrix

d059f66

revert testing change

f48e84c

briantist marked this pull request as ready for review February 25, 2023 19:40

briantist requested a review from lowlydba February 25, 2023 19:40

briantist added the ci/cd label Feb 25, 2023

lowlydba approved these changes Feb 26, 2023

View reviewed changes

briantist mentioned this pull request Feb 26, 2023

dbatools errors on SSH with pwsh 7.3 dataplat/dbatools#8657

Closed

lowlydba merged commit f927c1f into lowlydba:main Feb 26, 2023

briantist deleted the pwsh-experimental/fixes branch February 26, 2023 19:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pwsh experimental/fixes #175

pwsh experimental/fixes #175

briantist commented Feb 20, 2023 •

edited

Loading

codecov-commenter commented Feb 20, 2023 •

edited

Loading

briantist commented Feb 20, 2023 •

edited

Loading

briantist commented Feb 21, 2023

briantist commented Feb 25, 2023

lowlydba commented Feb 26, 2023

lowlydba commented Feb 26, 2023

briantist commented Feb 26, 2023

lowlydba commented Feb 26, 2023

pwsh experimental/fixes #175

pwsh experimental/fixes #175

Conversation

briantist commented Feb 20, 2023 • edited Loading

Description

How Has This Been Tested?

Types of changes

Checklist:

codecov-commenter commented Feb 20, 2023 • edited Loading

Codecov Report

briantist commented Feb 20, 2023 • edited Loading

briantist commented Feb 21, 2023

briantist commented Feb 25, 2023

Ansible

PowerShell

lowlydba commented Feb 26, 2023

lowlydba commented Feb 26, 2023

briantist commented Feb 26, 2023

lowlydba commented Feb 26, 2023

briantist commented Feb 20, 2023 •

edited

Loading

codecov-commenter commented Feb 20, 2023 •

edited

Loading

briantist commented Feb 20, 2023 •

edited

Loading