[BUG] win_pkg weirdness in 3006.1

[darkpixel]

Description
win_pkg is behaving strangely when updating an application.
I have about 380 Windows minions all running 3006.1.

A new version of TeamViewer came out and I updated my custom definition.
I told all the minions to refresh the database.
I told all the minions to install the latest version.
Only ~130 out of the 380 updated the application...but all of them reported some level of success.

Setup

My cait_teamviewer.sls definition:

{%
set versions = [
  '15.42.9.0',
  '15.42.8.0'
]
%}
cait_teamviewer:
  {% for version in versions %}
  '{{ version }}':
    full_name: 'TeamViewer Host'
    installer: 'https://cdn-redacted/TeamViewer_Host.msi'
    uninstaller: 'https://cdn-redacted/TeamViewer_Host.msi'
    arch: x86
    install_flags: '/qn /norestart CUSTOMCONFIGID="--redacted--" ASSIGNMENTOPTIONS="--alias %ComputerName% --grant-easy-access --reassign" APITOKEN="--redacted--"'
    uninstall_flags: '/qn /norestart'
    msiexec: True
    locale: en_US
    reboot: False
{% endfor %}

Please be as specific as possible and give set-up details.

• onedir packaging

Steps to Reproduce the behavior
Here's one of the minions that failed to update:

salt 'USSNISDOFC03*' test.version

USSNISDOFC03.redacted.local:
    3006.1



salt 'USSNISDOFC03*' pkg.refresh_db
USSNISDOFC03.redacted.local:
    ----------
    failed:
        0
    success:
        22
    total:
        22

salt 'USSNISDOFC03*' pkg.latest_version cait_teamviewer

USSNISDOFC03.redacted.local:
    15.42.9.0   <-- this is correct


salt 'USSNISDOFC03*' pkg.list_available cait_teamviewer

USSNISDOFC03.redacted.local:
    - 15.42.8.0
    - 15.42.9.0  <-- it obviously sees the latest version

salt 'USSNISDOFC03*' pkg.upgrade_available cait_teamviewer

USSNISDOFC03.redacted.local:
    True  <-- Yup, there's an upgrade available

salt 'USSNISDOFC03*' pkg.install cait_teamviewer version=latest
USSNISDOFC03.redacted.local:
    ----------
    cait_teamviewer:
        ----------
        install status:
            success

# You'll notice this isn't the "normal" return for a system that upgrades properly.
# Normally you see the old version and the new version listed


salt 'USSNISDOFC03*' pkg.install cait_teamviewer version=15.42.9.0

USSNISDOFC03.redacted.local:
    ----------
    cait_teamviewer:
        ----------
        install status:
            success

After running that, I connect in to the machine and see an older version of TeamViewer installed.

If I run the Windows Installer manually:

salt 'USSNISDOFC03*' cmd.run 'msiexec /i https://cdn-redacted/TeamViewer_Host.msi /qn /norestart CUSTOMCONFIGID="-redacted-" ASSIGNMENTOPTIONS="--alias %ComputerName% --grant-easy-access --reassign" APITOKEN="-redacted-"

USSNISDOFC03.redacted.local:


-------------------------------------------
Summary
-------------------------------------------
# of minions targeted: 1
# of minions returned: 1
# of minions that did not return: 0
# of minions with errors: 0
-------------------------------------------

salt 'USSNISDOFC03*' pkg.upgrade_available cait_teamviewer
USSNISDOFC03.redacted.local:
    False

...and I can sign in to the machine and see it's running the latest version.

Contrast all of that with a machine that worked correctly:

USBVESDOFC01.redacted.local:
    ----------
    cait_teamviewer:
        ----------
        new:
            15.42.9.0
        old:
            15.42.8.0

Expected behavior
I would expect it to either report an error (if there was one) or that the upgrade was successful, but it appears to be reporting a successful upgrade without showing the version numbers when it really didn't do anything.

Minions are all Windows 10 or Windows 11 64-bit machines.
The successful machines are a mix of Windows 10 and 11.
The failing machines are a mix of Windows 10 and 11.

I'm stumped as to why it only works on some machines, but my manual msiexec command (which appears to match what pkg.installed runs on successful machines) works perfectly.

Is there a minion cache of installer files I'm unaware of that isn't updated by pkg.refresh_db?

[darkpixel]

Hmm...actually I just confirmed it.
I connected to one of the machines that wasn't upgrading and giving a bad status.
I deleted: c:\ProgramData\Salt Project\Salt\var\cache\salt\minion\extrn_files\base\*

Then I ran pkg.install cait_teamviewer version=latest and it worked perfectly.

Since some installers have the same name, maybe it would be a good idea to have a versioned cache.
For example, instead of downloading to:
....\cdn-redacted.com\TeamViewer_Host.msi download it to ....\cdn-redacted.com\{{ version_number }}\TeamViewer_Host.msi.

Or just re-download the file every time.
Or provide an arg to win_pkg to clear the cache.

saltutil.clear_cache does NOT clear the file cache.

Created a quick hacky state:

# clear_files.sls
clear_file_cache:
  cmd.run:
    - name: 'del /f /s /q "c:\ProgramData\Salt Project\Salt\var\cache\salt\minion\extrn_files\*"'

Running state.sls clear_files and then pkg.install cait_teamviewer version=latest fixes everything.

[damon-atkins]

https/http caching..... see PR 61391

[darkpixel]

Nice. Looks like PR #61391 will do the job!

[damon-atkins]

Normally you would have
installer: 'https://cdn-redacted/TeamViewer_Host.{{version}}.msi'
You do not need to set uninstaller.
If you are using the special key word latest instead of a version number in the package definition sls file, then you do not need to have version in the 'installer' path e.g. https://github.com/saltstack/salt-winrepo-ng/blob/master/chrome.sls

[darkpixel]

Yeah--unfortunately TeamViewer doesn't release versioned MSI files.
You have to have a TeamViewer license, then you sign in to their console, and as an authenticated user you can click a link that looks like https://login.teamviewer.com/nav/deploy/modules/edit/<id-number>#.

That hands you a generically named TeamViewer_MSI32.zip that you need to extract to reveal your customized TeamViewer_Host.msi file.

Since it's a stupid process requiring authentication and zip extraction, it's easier to just upload the TeamViewer_Host.msi file to your own CDN and then use salt to update all your clients.

I suppose I could rename it myself and toss a version number into the file...but then again, winrepo should probably just support clearing the cache (or the etags PR) and do the right thing as there are plenty of companies that don't release installers with version numbers (i.e. Chrome, Edge, etc...).

[darkpixel]

Oh...and just using the tag 'latest'....I don't think that does what I want.

If I use latest and pkg.install someapp version=latest, someapp will get installed...let's say version 1.0.0.

But then 2.0.0 gets released...and the system sees the latest tag and doesn't try to install it again...because it has no idea if the latest version is 1.0.0, 2.0.0, or 57.42.21.

[damon-atkins]

@darkpixel most have version numbers even if the file does not have a version number, sometime you need to install it to find the version number. And then use this to include a name in the file. Having the version number in the file also allows you to role it out to pilot users first.

[damon-atkins]

win_pkg uses cp behind the scene.
e.g.

salt abc cp.is_cached https://cdn-redacted/TeamViewer_Host.msi
salt abc cp.cache_file https://cdn-redacted/TeamViewer_Host.msi

[darkpixel]

Maybe I'm just not understanding what you're saying or I was unclear in my previous comment.

From my brief testing, the caching mechanism doesn't seem to care about the version definition in winrepo.
If I have version 1.0.0 of myapp defined in winrepo and it's pointed to https://example.tld/myinstaller.exe, and then I go to MINION001 and run pkg.install myapp version=latest (or version=1.0.0) it will currently cache the file myinstaller.exe.

Then when I update the definition of myapp to 2.0.0 and replace the installer file at https://example.tld/myinstaller.exe with a completely new file (because I'm a bad developer and I don't release installers with version numbers in the name), and then I go to MINION001 and run pkg.install myapp version=latest (or version=2.0.0)....it appears salt will look at the path to the installer and say "I don't need to download that file, it's already cached"....and then attempt to upgrade to version 2.0.0 from that cached installer....which is actually the old 1.0.0 version.

I'm not saying that Salt won't try to upgrade it--just that there's the caching issue with the file. Because the URL to the file doesn't have anything unique in it, it doesn't change, and salt thinks it already has the latest copy downloaded.

The etags PR should fix that for most sites and CDNs, and I checked my specific use-case and it fixes mine.

Maybe for odd edge cases, it might be nice to have a parameter that can be passed to pkg.install similar to refresh for the winrepo database. Maybe something like recache to tell it to delete/re-download.

[damon-atkins]

win_pkg does not use eTag option (its off by default) when it calls cp.cache_file and it also uses cp.is_cache first (which just checks if the file has been download at all). https://docs.saltproject.io/en/latest/ref/modules/all/salt.modules.cp.html

[darkpixel]

Right...hence the PR you mentioned that will bring etags support which will fix things in the case of the CDN I use.

[twangboy]

The latest version was added to handle the scenarios you mentioned, where the developer doesn't provide multiple versions of their software, only a single file of the latest release.

If you're hosting these on your own file server, I would recommend adding the version number to the file name and updating the package definition file.

Etag support was added in 3005, so it should be available in 3006.1. The cp.is_cached function doesn't use the etag stuff. It just looks for a file with the same name. The win_pkg module probably needs to cache the file every time if the version is latest.

[darkpixel]

The latest version was added to handle the scenarios you mentioned, where the developer doesn't provide multiple versions of their software, only a single file of the latest release.

Right. Again....if I have a package definition for something like chrome and all I have is the version latest...pkg.install WILL install the latest version of chrome if chrome hasn't been installed....but if Chrome is already installed it WILL NOT update it because it has no idea it's out of date because there's no version numbering in the definition.

That's fine for Chrome which keeps itself up-to-date automatically...but for terrible applications like TeamViewer it doesn't work. There is no updating to newer versions unless I put version numbers in the winrepo definition and either upload it to my CDN with a version number or delete the old cache files.

[darkpixel]

When dealing with MSI files, msiexec doesn't care what the extension of the filename is, so you can add the version number as a parameter to the URL to invalidate the cache. i.e. ?v={{ version }}:

{%
set versions = [
  '15.42.9.0',
  '15.42.8.0'
]
%}
cait_teamviewer:
  {% for version in versions %}
  '{{ version }}':
    full_name: 'TeamViewer Host'
    installer: 'https://cdn-redacted/TeamViewer_Host.msi?v={{ version }}'
    uninstaller: 'https://cdn-redacted/TeamViewer_Host.msi?v={{ version }}'
    arch: x86
    install_flags: '/qn /norestart CUSTOMCONFIGID="--redacted--" ASSIGNMENTOPTIONS="--alias %ComputerName% --grant-easy-access --reassign" APITOKEN="--redacted--"'
    uninstall_flags: '/qn /norestart'
    msiexec: True
    locale: en_US
    reboot: False
{% endfor %}

[twangboy]

I believe the Above PR addresses the issue