Skip to content

(PA-5826) Only read Windows VERSION file during puppet apply #677

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

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

joshcooper
Copy link
Contributor

Previously, puppetserver attempted to load the wrong VERSION file when managing package_version => auto and compiling catalogs for Windows nodes as can be seen by the new test:

Evaluation Error: Error while evaluating a Function Call, Could not find any files from C:\Program Files\Puppet Labs\Puppet\VERSION (file: path/to/init.pp, line: 170, column: 42)

This adds a puppet_runmode fact. When running as puppet agent, the run mode will be agent. When running as puppet apply, it will be user. If it's the latter and we're a Windows host, then look for the VERSION file locally.

@joshcooper joshcooper requested review from bastelfreak and a team as code owners October 25, 2023 07:16
@kenyon
Copy link
Contributor

kenyon commented Oct 25, 2023

Should fix #665.

@@ -167,6 +167,9 @@
# The AIO package version and Puppet version can, on rare occasion, diverge.
# This logic checks for the AIO version of the server, since that's what the package manager cares about.
if $package_version == 'auto' {
if $facts['os']['family'] == 'windows' and $facts['puppet_runmode'] == 'user' {
$version_file_path = "${facts['env_windows_installdir']}\\VERSION"
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this a reassignment of the $version_file_path variable, since it's already been assigned a value from the class parameter list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I’ll fix that. Weird that all the tests passed. Or would it only show up when running a specific puppet-lint check?

Copy link
Collaborator

Choose a reason for hiding this comment

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

puppet-lint does not catch that. you need an rspec-puppet test that hits this if condition and checks for it should compile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yeah, my test coverage was lacking. I added a test for apply mode on Windows and it triggered the issue:

  1) puppet_agent supported Windows x64 environment with auto when applying is expected to contain File[C:\ProgramData\Puppetlabs]
     Failure/Error: it { is_expected.to contain_file("#{appdata}\\Puppetlabs") }
     
     Puppet::PreformattedError:
       Evaluation Error: Cannot reassign variable '$version_file_path' (file: /home/josh/work/modules/puppetlabs-puppet_agent/spec/fixtures/modules/puppet_agent/manifests/init.pp, line: 171, column: 28) on node xxx
     # /home/josh/.rbenv/versions/3.2.2/lib/ruby/3.2.0/benchmark.rb:311:in `realtime'
     # ./spec/classes/puppet_agent_osfamily_windows_spec.rb:106:in `block (5 levels) in <top (required)>'

Previously, puppetserver attempted to load the wrong VERSION file when managing
`package_version => auto` and compiling catalogs for Windows nodes as can be
seen by the new test:

    Evaluation Error: Error while evaluating a Function Call, Could not find any files from C:\Program Files\Puppet Labs\Puppet\VERSION (file: path/to/init.pp, line: 170, column: 42)

This adds a `puppet_runmode` fact. When running as puppet agent, the run mode will
be `agent`. When running as puppet apply, it will be `user`. If it's the latter
and we're a Windows host, then look for the VERSION file locally.

Since the default class parameter is no longer dependent on the agent
fact, I regenerated the reference using rake strings:generate:reference
@cthorn42 cthorn42 merged commit 5424e8d into puppetlabs:main Oct 30, 2023
@mhashizume mhashizume added the bug Something isn't working label Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants