Add Debian 10 support #674

bastelfreak · 2020-04-13T10:34:12Z

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

bastelfreak · 2020-04-13T10:34:47Z

since the acceptance tests otherwise take ages, I tested the fix from @ekohl here.

.travis.yml

spec/acceptance/agent_spec.rb

ekohl · 2020-04-14T09:28:32Z

#676 may help to make it easier to manage.

ekohl

In general it's fine to treat the major versions as strings and not convert them to integers.

spec/acceptance/agent_spec.rb

ekohl · 2020-04-14T16:22:50Z

manifests/params.pp

+
+

Suggested change

ekohl · 2020-04-14T23:22:02Z

Gemfile

@@ -68,4 +71,6 @@ end
 ENV['PUPPET_VERSION'].nil? ? puppetversion = '~> 6.0' : puppetversion = ENV['PUPPET_VERSION'].to_s
 gem 'puppet', puppetversion, :require => false, :groups => [:test]

+gem 'irb'


ekohl · 2020-04-14T23:22:12Z

Gemfile

@@ -68,4 +71,6 @@ end
 ENV['PUPPET_VERSION'].nil? ? puppetversion = '~> 6.0' : puppetversion = ENV['PUPPET_VERSION'].to_s
 gem 'puppet', puppetversion, :require => false, :groups => [:test]

+gem 'irb'
+gem 'beaker-vagrant'


Should be in voxpupuli-acceptance

ekohl · 2020-04-14T23:23:36Z

Gemfile

@@ -19,6 +19,9 @@ group :test do
  gem 'zabbixapi',                   :require => false
 end

+gem 'rspec-puppet-facts', :git => 'https://github.com/ekohl/rspec-puppet-facts', :branch => 'find-facter-version'
+gem 'facterdb', :git => 'https://github.com/camptocamp/facterdb', :branch => 'master'


I'd prefer not to rely on git branches unless really needed. Perhaps link to the commit/pr that we need to be released?

1.3.0 was released so this can be removed.

ekohl · 2020-04-14T23:28:08Z

spec/acceptance/agent_spec.rb

@@ -11,10 +13,11 @@ def agent_supported(version)
  describe "zabbix::agent class with zabbix_version #{version}", if: agent_supported(version) do
    it 'works idempotently with no errors' do
      pp = <<-EOS
+      include apt


Why is including apt needed? It shouldn't be. There's this:

puppet-zabbix/manifests/repo.pp

Lines 98 to 104 in 87e09e3

if ($manage_apt) {

# We would like to provide the repos with https urls instead of http

# this requires the apt-transport-https package, but we don't want to manage

# that package here. That should be handled in a profile :(

# somebody should implement https here but make it optional

include apt

}

However, there's also https://github.com/puppetlabs/puppetlabs-apt/blob/04b6a898f8e6f733e1b4f62d3c1a1c631b10253c/manifests/source.pp#L71

I don't see why the repo class shouldn't unconditionally include it. It's not a literal class statement so it shouldn't be an issue. However, it defaults to true anyway so this should be redundant.

Suggested change

include apt

(repeated comment)

ekohl · 2020-04-14T23:33:59Z

spec/acceptance/zabbix_application_spec.rb

@@ -1,7 +1,7 @@
 require 'spec_helper_acceptance'
 require 'serverspec_type_zabbixapi'

-describe 'zabbix_application type' do
+describe 'zabbix_application type', unless: default[:platform] =~ %r{debian-10-amd64} do


Rather than skipping, should we just not set the zabbix_version and use the default in this test suite (repeated in multiple files)?

sadly that won't work. the custom types use the zabbixapi rubygem and that currently only supports older zabbix versions

#623 suggests this can work if we update it.

I would like to get this merged and work on #623 afterwards

ekohl · 2020-04-16T19:19:59Z

spec/acceptance/zabbix_application_spec.rb

@@ -38,7 +38,15 @@ class { 'zabbix':
    # setup zabbix. Apache module isn't idempotent and requires a second run
    it 'works with no error on the first apply' do
      # Cleanup old database
-      shell('/opt/puppetlabs/bin/puppet resource service zabbix-server ensure=stopped; /opt/puppetlabs/bin/puppet resource package zabbix-server-pgsql ensure=purged; rm -f /etc/zabbix/.*done; su - postgres -c "psql -c \'drop database if exists zabbix_server;\'"')
+      cleanup_script = <<-SHELL


Can we extract this somewhere to avoid the repetition?

bastelfreak · 2020-04-21T20:29:05Z

@baurmatt any idea how we can get the tests working on CentOS 7 / any idea why they don't fail on master?
During the tests, agent_spec.rb installs zabbix 4.4, which works. afterwards server_spec.rb gets loaded which installs 3.4: https://travis-ci.org/github/voxpupuli/puppet-zabbix/jobs/677866025#L8857

Somehow the cache is messed up and yum wants to install the 4.4 version of the zabbix server:
https://travis-ci.org/github/voxpupuli/puppet-zabbix/jobs/677866025#L8887

I'm not sure why this is happening now
It even happens with the shell('yum clean all') if fact('os.family') == 'RedHat' snippet that's also executed:

centos7-64-1 20:23:33$ yum clean all
  Loaded plugins: fastestmirror, ovl
  Cleaning repos: Zabbix_7_x86_64 Zabbix_nonsupported_7_x86_64 base extras puppet5
                : updates
  Cleaning up list of fastest mirrors
centos7-64-1 executed in 0.21 seconds

Why does master pass??

bastelfreak · 2020-04-21T21:35:16Z

after adding way more yum cleanup statements, it turned green \o/

bastelfreak added the enhancement New feature or request label Apr 13, 2020

bastelfreak self-assigned this Apr 13, 2020

bastelfreak force-pushed the debian10 branch 9 times, most recently from 8f41e47 to 2c93880 Compare April 13, 2020 16:29

bastelfreak mentioned this pull request Apr 14, 2020

Add acceptance testing for 4.2 and 4.4 #664

Merged

dhoppe requested changes Apr 14, 2020

View reviewed changes

.travis.yml Show resolved Hide resolved

alexjfisher reviewed Apr 14, 2020

View reviewed changes

spec/acceptance/agent_spec.rb Outdated Show resolved Hide resolved

alexjfisher reviewed Apr 14, 2020

View reviewed changes

spec/acceptance/agent_spec.rb Outdated Show resolved Hide resolved

bastelfreak force-pushed the debian10 branch 3 times, most recently from 0765382 to df40e8f Compare April 14, 2020 16:20

ekohl requested changes Apr 14, 2020

View reviewed changes

bastelfreak force-pushed the debian10 branch from df40e8f to ba3e50b Compare April 14, 2020 17:02

ekohl requested changes Apr 14, 2020

View reviewed changes

bastelfreak force-pushed the debian10 branch 4 times, most recently from 2c416ca to 2a506eb Compare April 16, 2020 19:12

ekohl reviewed Apr 16, 2020

View reviewed changes

bastelfreak force-pushed the debian10 branch 2 times, most recently from eece623 to d122a27 Compare April 19, 2020 21:53

bastelfreak added 2 commits April 21, 2020 20:55

Add Debian 10 support

b63f2c6

switch to improved rspec-puppet-facts version

b3c4cad

bastelfreak force-pushed the debian10 branch 3 times, most recently from a90408a to 27d1ec0 Compare April 21, 2020 20:17

bastelfreak added the tests-fail label Apr 21, 2020

bastelfreak force-pushed the debian10 branch 3 times, most recently from 3fb772d to 5323faf Compare April 21, 2020 21:22

bastelfreak removed the tests-fail label Apr 21, 2020

CentOS tests: Purge whole yum cache

7ddc0dc

bastelfreak force-pushed the debian10 branch from 5323faf to 7ddc0dc Compare April 21, 2020 21:36

dhoppe approved these changes Apr 22, 2020

View reviewed changes

baurmatt approved these changes Apr 22, 2020

View reviewed changes

bastelfreak merged commit 8f411bf into voxpupuli:master Apr 22, 2020

bastelfreak deleted the debian10 branch April 22, 2020 09:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Debian 10 support #674

Add Debian 10 support #674

bastelfreak commented Apr 13, 2020

bastelfreak commented Apr 13, 2020

ekohl commented Apr 14, 2020

ekohl left a comment

ekohl Apr 14, 2020

ekohl Apr 14, 2020

ekohl Apr 14, 2020

ekohl Apr 14, 2020

ekohl Apr 16, 2020

ekohl Apr 14, 2020

ekohl Apr 14, 2020

bastelfreak Apr 16, 2020

ekohl Apr 16, 2020

bastelfreak Apr 21, 2020

ekohl Apr 16, 2020

bastelfreak commented Apr 21, 2020 •

edited

Loading

bastelfreak commented Apr 21, 2020

	if ($manage_apt) {
	# We would like to provide the repos with https urls instead of http
	# this requires the apt-transport-https package, but we don't want to manage
	# that package here. That should be handled in a profile :(
	# somebody should implement https here but make it optional
	include apt
	}

Add Debian 10 support #674

Add Debian 10 support #674

Conversation

bastelfreak commented Apr 13, 2020

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

bastelfreak commented Apr 13, 2020

ekohl commented Apr 14, 2020

ekohl left a 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

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

bastelfreak commented Apr 21, 2020 • edited Loading

bastelfreak commented Apr 21, 2020

bastelfreak commented Apr 21, 2020 •

edited

Loading