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

cChocoPackageInstall doesn't check that the package was actually installed #61

Open
Richiban opened this issue Jan 20, 2017 · 22 comments · May be fixed by #168
Open

cChocoPackageInstall doesn't check that the package was actually installed #61

Richiban opened this issue Jan 20, 2017 · 22 comments · May be fixed by #168

Comments

@Richiban
Copy link

Richiban commented Jan 20, 2017

I've noticed a problem with cChocoPackageInstall -- if the package failed to install (due to the source being misconfigured or the version or package name simply not existing) it's not an error. The failure is logged in the verbose output but the DSC configuration silently continues.

This is a pretty nasty surprise; it appears that the DSC you ran completed successfully but in fact some packages were not installed.

@javydekoning
Copy link
Contributor

Can you provide a scenario to replicate?

@Richiban
Copy link
Author

Sure! This is all it takes:

Configuration FailingDsc
{
    Import-DscResource -ModuleName PSDesiredStateConfiguration
    Import-DscResource -ModuleName cChoco
    
    Node localhost
    {
        cChocoInstaller InstallChocolatey
        {
            InstallDir = "c:\choco"
        }

        cChocoPackageInstaller FailingInstaller
        {
            DependsOn = "[cChocoInstaller]InstallChocolatey"
            Name = "This-package-name-has-a-typo"
        }
    }
}

I'm running this with Start-DscConfiguration -wait FailingDsc, which exits without errors. Running with -verbose yields the following log entry:

VERBOSE: [xxxxxxxx]: [[cChocoPackageInstaller]FailingInstaller] Package output
Chocolatey v0.10.3 Installing the following packages: This-package-name-has-a-typo By installing you accept licenses
for the packages. This-package-name-has-a-typo not installed. The package was not found with the source(s) listed.  If
 you specified a particular version and are receiving this message, it is possible that the package name exists but
the version does not.  Version: ""  Source(s):

@javydekoning
Copy link
Contributor

javydekoning commented Jan 20, 2017

Thanks, once I have some extra time I'll implement something to validate the installer exitcodes and write pester tests :).

@Richiban
Copy link
Author

Cool, thanks. I was thinking that there's already a function defined "IsPackageInstalled" that I can call at the end of the package installation process, which appears to work.

Does this sound like a good idea? If so, I can submit a PR.

@javydekoning
Copy link
Contributor

I think it would be better to throw an error if the package install fails. The IsPackageInstalled is intended to get and test the DSC config.

@Richiban
Copy link
Author

Okay sure. I've created a PR https://github.com/Richiban/cChoco/pull/1 -- I'm not asking you to actually merge it in but just to demonstrate what I meant.

Thanks

@gmatusko
Copy link

Any updates on this, it would be handy for the DSC resource to kick back an error if the package actually failed to install. Right now any depending resources on the package installing successfully will subsequently fail which is fine but would be good to know the root cause of the failure was the install of the package.

@ferventcoder
Copy link
Member

This is set to up for grabs for someone to take a look at.

@elovelan
Copy link

elovelan commented May 11, 2017

I'm looking to pick this up. Does anyone know why it's using Invoke-Expression for all of the choco calls? Error handling is a bit more challenging when using that.

$packageInstallOuput, $exitCode = Invoke-Expression -Command "(choco install $pName $chocoinstallparams), $?"

vs

$packageInstallOuput = choco install $pName @chocoinstallparams
$exitCode = $?

On second thought, this is probably for the best, I've run into a few Chocolatey packages that can't be installed by the System user and this makes it easier to enhance with a Credential parameter for those of us stuck on PS 4.0 (PR likely coming for that as well)

@ferventcoder
Copy link
Member

@elovelan if you have specific packages you can point to that have issues, that would be most helpful.

@elovelan
Copy link

elovelan commented May 24, 2017

@ferventcoder, is your question about packages that will not install under the System account or about packages that fail?

In terms of failure, this is really more about general error handling than just packages that fail to install.

The .NET framework and visual studio isolated shells are examples of packages that will not install under the System account. I can't remember the exact reasons, but the error is from the MSI, it's not a Chocolatey-specific issue. The work-around is to install using the built-in Package resource with the Credential option.

@ferventcoder
Copy link
Member

ferventcoder commented May 24, 2017

The work-around is to install using the built-in Package resource with the Credential option.

@elovelan does cChoco not support the Credential option? I was under the impression that DSC supported Credentials across the board?

@ferventcoder
Copy link
Member

is your question about packages that will not install under the System account or about packages that fail?

@elovelan I was specifically speaking of installers that had issues under the system account (not necessarily packages)

@elovelan
Copy link

does cChoco not support the Credential option? I was under the impression that DSC supported Credentials across the board?

This is supported as of PS 5.0, unfortunately not everyone is able to move to it in a timely fashion 😄

@kewalaka
Copy link

kewalaka commented Jun 4, 2017

Double quotes like that won't work because $? will be evaluated to whatever the variable contains, sticking with Invoke-Expression, you'd need something like this:

$param = '--localonly'
$exp = "choco list $param"
$exp += ';$?'
$e = Invoke-Expression -Command $exp

$p = $e | select -skip 1 -last $e.Length | Out-String
if ($e | select -last 1)
{
    Write-Host "Sweet as!"
    Write-Host "Output was:`n $p"
}
else
{
    Write-Host "Shucks it broke!"
    Write-Host "Output was:`n $p"
}

Note the single quotes in line 3 above.
If you change 'choco list' to 'choco lust' to simulate an error, you get:

Shucks it broke!
Output was:
 Chocolatey v0.10.6
Could not find a command registered that meets 'lust'. 
 Try choco -? for command reference/help.

@esiebes
Copy link

esiebes commented Dec 21, 2017

There is now a pull request for this issue. #103

@githubboffeli
Copy link

Any updates on this?

@Gerthum
Copy link

Gerthum commented May 3, 2018

  • 1 This is quite an important fix for us.

@alphakilo45
Copy link

+1 - Could really use this fixed as well - if there anything in particular it's waiting on? Or just the next release?

@passbt
Copy link

passbt commented Feb 6, 2019

I'm affected by this as well. I have an internal 7zip package that I tried to install. Since I'm new to DSC I wanted to see what would happen if intentionally typed the package named 7zip incorrectly. The Start-DscConfiguration completed without error. However, Test-DscConfiguration returns [cChocoPackageInstaller]Install7zip is not in the desired state. Additionally, I have an error in the chocolatey.log that states that the 72zip package can't be found in my source list - as expected.

@pauby
Copy link
Member

pauby commented Mar 6, 2019

This is waiting on PR changes - #103

@pauby pauby removed this from the 2.5.0 milestone Feb 9, 2021
@pauby pauby added this to the 2.5.1 milestone Feb 9, 2021
@isgxr-a
Copy link

isgxr-a commented May 13, 2021

Any updates on this? This does create some nasty surprises on failing installs

@pauby pauby modified the milestones: 2.5.1, 2.6 Jul 16, 2021
orck-adrouin added a commit to orck-adrouin/cChoco that referenced this issue Jun 8, 2022
Changes cChocoPackagesInstall to validate choco's exit code. Handles the
standard exit codes as documented in Chocolatey's documentation.

Fixes issue chocolatey#61
Supersedes chocolatey#103
@orck-adrouin orck-adrouin linked a pull request Jun 8, 2022 that will close this issue
9 tasks
@vexx32 vexx32 modified the milestones: 2.6.0, 2.x Oct 6, 2023
@vexx32 vexx32 added 3 - Review and removed 4 - Done labels Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.