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

(maint) Allow installing 32-bit Puppet-Agent on Windows #769

Conversation

Iristyle
Copy link
Contributor

  • Previously, install_utils was modified to be able to install PE
    based on a configuration switch called install_32 in
    6f8deb7

    This allows a 64-bit Windows OS to ask to install a 32-bit Ruby /
    Puppet version. This is important given 64-bit support wasn't
    added to Puppet or PE until 3.7.

    When installing the puppet-agent through install_puppetagent_dev_repo
    helper, allow for selecting the build in a similar fashion.

 - Previously, install_utils was modified to be able to install PE
   based on a configuration switch called `install_32` in
   6f8deb7

   This allows a 64-bit Windows OS to ask to install a 32-bit Ruby /
   Puppet version.  This is important given 64-bit support wasn't
   added to Puppet or PE until 3.7.

   When installing the puppet-agent through install_puppetagent_dev_repo
   helper, allow for selecting the build in a similar fashion.
@@ -693,7 +693,11 @@ def install_puppetagent_dev_repo( host, opts )
when /^windows$/
release_path << 'windows'
onhost_copy_base = '`cygpath -smF 35`/'
arch_suffix = arch =~ /64/ ? '64' : '86'
should_install_64bit = host.is_x86_64? && !host['install_32'] && !opts['install_32']
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would prefer this setting to be :ruby_arch, which is what we've use in Puppet since the introduction of Ruby 2.0.0 / x64 support (which involved build pipeline changes from @rji)

See https://tickets.puppetlabs.com/browse/PUP-2388 for the history around :ruby_arch or the associated PR at puppetlabs/puppet#2903

I've used :install_32 for consistency now with what I see in 6f8deb7 but would prefer to see this changed to :ruby_arch if we know we don't have downstream consumers here.

If you look at https://github.com/puppetlabs/puppet/tree/master/acceptance/config/nodes you'll see all the existing Windows node definitions use :ruby_arch

The changes here could be replaced with the following if we use :ruby_arch instead:

arch_suffix = (host[:ruby_arch] || arch) =~ /64/ ? '64' : '86'

/cc @joshcooper @justinstoller @kevpl @anodelman

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we modify beaker to accept either :ruby_arch or :install_* (in that order), and deprecate the latter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's certainly the correct approach with respect to backward compatibility, but complicates the Beaker code. We could also wait on the next breaking Beaker release? Not sure how far we're going past 2.8 / how close 3.0 is?

@Iristyle Iristyle changed the title (maint) Allow installing 32-bit Puppet Agent (maint) Allow installing 32-bit Puppet Agent on Windows Mar 31, 2015
@Iristyle Iristyle changed the title (maint) Allow installing 32-bit Puppet Agent on Windows (maint) Allow installing 32-bit Puppet-Agent on Windows Mar 31, 2015
@puppetlabs-jenkins
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
http://jenkins-beaker.delivery.puppetlabs.net//job/qe_beaker_btc-intn/708/
🔴 Test failed.

@Iristyle
Copy link
Contributor Author

@joshcooper I added 655569a to allow us to use :ruby_arch.

@Iristyle Iristyle force-pushed the maint/master/allow-installing-32-bit-puppet-agent-MSI branch from 489f008 to 1eecc78 Compare March 31, 2015 01:43
 - Puppet puppetlabs/puppet@92c539f
   added :ruby_arch to support the notion of choosing a Windows
   installer architecture.  A node could set :ruby_arch to either
   x64 or x86 to choose to install a 64-bit or 32-bit Ruby MSI on a
   64-bit version of Windows.

   When similar behavior was introduced to Beaker, it didn't follow
   prior art, and a new setting called :install_32 was introduced to
   manage this functionality in 6f8deb7

   This commit adds in :ruby_arch, with the hope that :install_32 will
   be deprecated on a future major version boundary to simplify the
   logic around settings evaluation here.
@Iristyle Iristyle force-pushed the maint/master/allow-installing-32-bit-puppet-agent-MSI branch from 1eecc78 to 655569a Compare March 31, 2015 01:49
@puppetlabs-jenkins
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
http://jenkins-beaker.delivery.puppetlabs.net//job/qe_beaker_btc-intn/710/
🔴 Test failed.

@puppetlabs-jenkins
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
http://jenkins-beaker.delivery.puppetlabs.net//job/qe_beaker_btc-intn/709/
💚 Test passed.

@puppetlabs-jenkins
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
http://jenkins-beaker.delivery.puppetlabs.net//job/qe_beaker_btc-intn/711/
🔴 Test failed.

@anodelman
Copy link
Contributor

So :ruby_arch is a new addition to the host support?

Please update the yard docs for do_install to reflect this new option.

@anodelman
Copy link
Contributor

Request to update yard docs still current.

@Iristyle
Copy link
Contributor Author

Iristyle commented Apr 8, 2015

@anodelman these settings are on the host, not as part of the opt hash passed into the method. There are no yard docs for install_32 anywhere that I could find FWIW.

@anodelman
Copy link
Contributor

Please add a @note to the yard docs for that method indicating the interaction between ruby_arch and install_32 for windows hosts.

@anodelman
Copy link
Contributor

@Iristyle Just blocked on last review comment.

@anodelman
Copy link
Contributor

@kevpl can you get this over the finish line?

@kevpl
Copy link
Contributor

kevpl commented May 4, 2015

closing in favor of #807

@kevpl kevpl closed this May 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants