BREAKING: Ensure python package (adds manage_python option) #623

wyardley · 2017-09-08T18:05:38Z

This is needed for Ubuntu 16.04+ where only Python 3 is installed by default.

This could cause issues in some cases where people have defined the Python package elsewhere, I'm open to suggestions on how to do this more sanely, though really hoping to avoid an additional parameter.

If RabbitMQ distributed it in the package, the dependency could be handled more cleanly.

wyardley · 2017-09-08T19:54:49Z

manifests/install/rabbitmqadmin.pp

+  # Newer versions of Ubuntu (16.04) ship with Python 3 only by default.
+  if $facts['os']['family'] == 'Debian' {
+    ensure_packages('python')
+    $rabbitmqadmin_require = [Archive['rabbitmqadmin'], Package['python']]


Not sure if there's a cleaner way to do this

also, this logic needs to be moved to params, and I think it probably does need to be configurable.

wyardley · 2017-09-08T19:55:47Z

I think this should be a parameter after all, either default undef with the option of setting the package name (as python_ensure), e.g., python_ensure = python. We could do it as a boolean too, but then wouldn't be able to set the package name. I moved the OS specific logic to params.pp.

wyardley · 2017-09-08T20:50:40Z

spec/classes/rabbitmq_spec.rb

-            'command'     => '/usr/bin/systemctl daemon-reload',
-            'notify'      => 'Class[Rabbitmq::Service]',
-            'refreshonly' => true
+            command: '/usr/bin/systemctl daemon-reload',


not sure why rubocop wasn't complaining about these before

bastelfreak · 2017-09-09T11:18:22Z

manifests/init.pp

@@ -213,6 +215,7 @@
  Optional[String] $package_provider             = undef,
  Boolean $repos_ensure                          = $rabbitmq::params::repos_ensure,
  $manage_repos                                  = undef,
+  Boolean $manage_python                         = $rabbitmq::params::manage_python,


datatypes \o/

bastelfreak · 2017-09-09T11:19:28Z

manifests/params.pp

@@ -6,6 +6,7 @@

  case $facts['os']['family'] {
    'Archlinux': {
+      $manage_python    = false


do we need python only on debian like systems?

I think we can require python on all platforms. It's ensure_packages so there's less chance for conflicts and otherwise we provide the option to disable it.

We can if you think that makes sense. My thought was to take a conservative approach, since a) I don't know for sure which systems don't already have it, and b) I don't know if the python 2 package name is consistent across all those platforms, and we don't do acceptance tests on all of them. Once this is enabled, we could switch the param for other platforms. Thoughts?

ekohl · 2017-09-09T13:42:09Z

manifests/params.pp

@@ -6,6 +6,7 @@

  case $facts['os']['family'] {
    'Archlinux': {
+      $manage_python    = false


I think we can require python on all platforms. It's ensure_packages so there's less chance for conflicts and otherwise we provide the option to disable it.

ekohl · 2017-09-09T13:42:58Z

manifests/install/rabbitmqadmin.pp

@@ -3,6 +3,14 @@

  require '::archive'

+  # Newer versions of Ubuntu (16.04) ship with Python 3 only by default.
+  if $rabbitmq::manage_python {
+    ensure_packages('python')


https://github.com/puppetlabs/puppetlabs-stdlib#ensure_packages states it takes a list of packages so I'm surprised this works.

Yeah, I think it does work, but I forced it to list context, which I think is cleaner.

I can switch it to managing it for all platforms, seems kind of silly for RHEL / CentOS, where I think it's a hard dependency for tools like yum / anaconda.

Do we know what the package is called on SUSE?

wyardley · 2017-09-09T19:59:56Z

Ok, how do we feel about this behavior now, @ekohl @bastelfreak? I don't have a way to test on all of these platforms, though verified that the package is python2 on FreeBSD (so presumably on OBSD as well).

I added the package name in params but didn't make it configurable.

…e are set) voxpupuli#618

bastelfreak · 2017-09-09T21:39:55Z

looks good to me. @ekohl and objections here?

Fix test cases for #623 (manage_python)

BREAKING: Ensure python package (adds manage_python option)

Fix test cases for voxpupuli#623 (manage_python)

wyardley force-pushed the ensure_python branch from 3b80852 to 4c35984 Compare September 8, 2017 18:31

wyardley changed the title ~~Ensure python package is installed on Debian when admin_enable and service_manage are set~~ Ensure python package on Debian when admin_enable and service_manage are set Sep 8, 2017

This was referenced Sep 8, 2017

Add official support (and update acceptance tests) for Ubuntu 16.04 #622

Closed

Add official support for Ubuntu 16.04 #624

Merged

wyardley added the bug Something isn't working label Sep 8, 2017

wyardley force-pushed the ensure_python branch 2 times, most recently from 27cbc10 to 440ab00 Compare September 8, 2017 19:53

wyardley commented Sep 8, 2017

View reviewed changes

wyardley requested a review from alexjfisher September 8, 2017 19:55

wyardley removed the request for review from alexjfisher September 8, 2017 20:25

wyardley force-pushed the ensure_python branch from 440ab00 to b5f27b7 Compare September 8, 2017 20:35

wyardley commented Sep 8, 2017

View reviewed changes

wyardley requested a review from alexjfisher September 8, 2017 21:05

wyardley changed the title ~~Ensure python package on Debian when admin_enable and service_manage are set~~ Ensure python package (adds python_ensure option) Sep 8, 2017

wyardley changed the title ~~Ensure python package (adds python_ensure option)~~ Ensure python package (adds manage_python option) Sep 8, 2017

wyardley force-pushed the ensure_python branch 3 times, most recently from 60506de to 43e47e4 Compare September 9, 2017 04:23

bastelfreak reviewed Sep 9, 2017

View reviewed changes

ekohl reviewed Sep 9, 2017

View reviewed changes

wyardley force-pushed the ensure_python branch 3 times, most recently from 4c2ec67 to 58205ea Compare September 9, 2017 19:58

Ensure python package by default (when admin_enable and service_manag…

444fc92

…e are set) voxpupuli#618

wyardley force-pushed the ensure_python branch from 58205ea to 444fc92 Compare September 9, 2017 21:11

bastelfreak approved these changes Sep 9, 2017

View reviewed changes

wyardley removed the request for review from alexjfisher September 9, 2017 21:51

wyardley added the backwards-incompatible label Sep 9, 2017

wyardley changed the title ~~Ensure python package (adds manage_python option)~~ BREAKING: Ensure python package (adds manage_python option) Sep 9, 2017

wyardley merged commit 2562077 into voxpupuli:master Sep 9, 2017

wyardley pushed a commit to wyardley/puppet-rabbitmq that referenced this pull request Sep 9, 2017

Fix test cases for voxpupuli#623 (manage_python)

d494b3e

wyardley mentioned this pull request Sep 9, 2017

Fix test cases for #623 (manage_python) #626

Merged

wyardley deleted the ensure_python branch September 9, 2017 23:29

wyardley pushed a commit to wyardley/puppet-rabbitmq that referenced this pull request Sep 10, 2017

Fix test cases for voxpupuli#623 (manage_python)

78ad1ef

ekohl added a commit that referenced this pull request Sep 11, 2017

Merge pull request #626 from wyardley/fix_python_spec

111532c

Fix test cases for #623 (manage_python)

wyardley pushed a commit to wyardley/puppet-rabbitmq that referenced this pull request Sep 12, 2017

Fix test cases for voxpupuli#623 (manage_python)

66dc58b

Slm0n87 pushed a commit to Slm0n87/puppet-rabbitmq that referenced this pull request Mar 7, 2019

Merge pull request voxpupuli#623 from wyardley/ensure_python

76b282b

BREAKING: Ensure python package (adds manage_python option)

Slm0n87 pushed a commit to Slm0n87/puppet-rabbitmq that referenced this pull request Mar 7, 2019

Fix test cases for voxpupuli#623 (manage_python)

f53a171

Slm0n87 pushed a commit to Slm0n87/puppet-rabbitmq that referenced this pull request Mar 7, 2019

Merge pull request voxpupuli#626 from wyardley/fix_python_spec

fac7eb0

Fix test cases for voxpupuli#623 (manage_python)

cegeka-jenkins pushed a commit to cegeka/puppet-rabbitmq that referenced this pull request Mar 26, 2021

Fix test cases for voxpupuli#623 (manage_python)

f700188

cegeka-jenkins pushed a commit to cegeka/puppet-rabbitmq that referenced this pull request Mar 26, 2021

Merge pull request voxpupuli#626 from wyardley/fix_python_spec

9ec70aa

Fix test cases for voxpupuli#623 (manage_python)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BREAKING: Ensure python package (adds manage_python option) #623

BREAKING: Ensure python package (adds manage_python option) #623

wyardley commented Sep 8, 2017 •

edited

Loading

wyardley Sep 8, 2017

wyardley Sep 8, 2017

wyardley commented Sep 8, 2017 •

edited

Loading

wyardley Sep 8, 2017

bastelfreak Sep 9, 2017

bastelfreak Sep 9, 2017

ekohl Sep 9, 2017

wyardley Sep 9, 2017

ekohl Sep 9, 2017

ekohl Sep 9, 2017

wyardley Sep 9, 2017

wyardley Sep 9, 2017

wyardley commented Sep 9, 2017

bastelfreak commented Sep 9, 2017

BREAKING: Ensure python package (adds manage_python option) #623

BREAKING: Ensure python package (adds manage_python option) #623

Conversation

wyardley commented Sep 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wyardley commented Sep 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wyardley commented Sep 9, 2017

bastelfreak commented Sep 9, 2017

wyardley commented Sep 8, 2017 •

edited

Loading

wyardley commented Sep 8, 2017 •

edited

Loading