puppetlabs / puppetlabs-puppet_agent

Module for managing Puppet-Agent
Apache License 2.0
41 stars 192 forks source link

(maint) Make spec tests pass on windows #665

Closed tvpartytonight closed 1 year ago

tvpartytonight commented 1 year ago

Windows requires the current puppet collection to be specified in the puppet_agent class, so that is refactored to be globally specified in the spec_helper_acceptance. Additionally, logic was added to specifiy default install paths for the VERSION file for both linux and windows.

kenyon commented 1 year ago

I guess this explains why auto doesn't seem to be working on Windows for us, ending up with various agent versions on Windows.

tvpartytonight commented 1 year ago

@joshcooper before I dig into the spec failures here, do you think this approach is correct? The bundle exec rake beaker passes on windows with these changes.

joshcooper commented 1 year ago

Yeah makes sense. I think the failures are because we have to stub out the VERSION file when testing the windows osfamily class.

tvpartytonight commented 1 year ago

I added a mock similar to the one already existing for /opt/puppetlabs/puppet/VERSION, but it doesn't fix the unit test failure.

joshcooper commented 1 year ago

Maybe one option is to not try to fix auto on windows and puppet apply (and file a ticket to fix it). Instead just modify class_spec to specify the package_version and collection when applying the class, something like this in before(:all):

      version = if %r{windows}i.match?(default['platform'])
                  version_dir = fact_on(default, 'env_windows_installdir')
                  on(default, "cmd /c type '#{version_dir}\\VERSION'").stdout.chomp
                else
                  file_contents_on(default, "/opt/puppetlabs/puppet/VERSION").chomp
                end
      collection = "puppet#{version.split('.').first}"
      pp = "class { 'puppet_agent': package_version => '#{version}', collection => '#{collection}'}"

The beaker helper file_contents_on doesn't seem to like paths with spaces when it calls powershell, so you can shell out to cmd instead and use the builtin type command (similar to cat). But you have to use backslashes in the path otherwise cmd thinks it's an command line switch.

tvpartytonight commented 1 year ago

@joshcooper I was able to keep using the auto value, I realized I just wasn't mocking enough previously.

kenyon commented 11 months ago

I'm testing puppet_agent 4.15.0. I think the change in this PR is causing catalog compilation to fail on Windows agents (PE 2019.8.11, Puppet 6.27.0). It appears to be trying to get the content of "${facts['env_windows_installdir']}\\VERSION" from the primary server, which of course is not running Windows.

joshcooper commented 11 months ago

It appears to be trying to get the content of "${facts['env_windows_installdir']}\VERSION" from the primary server, which of course is not running Windows.

Gah... I forgot the module is using the file function... Right so when running puppet apply it will retrieve the VERSION file from the currently running agent. But when puppetserver is compiling a catalog for a windows node, the file function will fail. I think we need to conditional logic to the module.

We might be able to detect which mode we're in based on server variables https://www.puppet.com/docs/puppet/7/lang_facts_builtin_variables#server-variables I think they're omitted in puppet apply

Rewerson commented 11 months ago

So, any news on it? Also getting errors on Windows hosts like: Error 500 on SERVER: Server Error: Evaluation Error: Error while evaluating a Function Call, Could not find any files from C:\Program Files\Puppet Labs\Puppet\VERSION (file: /etc/puppetlabs/code/environments/production/modules/puppet_agent/manifests/init.pp, line: 170, column: 42)