BREAKING: Add acceptance tests + multiple bugfixes #382

bastelfreak · 2017-05-21T13:29:34Z

No description provided.

bastelfreak · 2017-05-21T13:32:52Z

The diff got quite huge. I initially only wanted to add a single acceptance test, to test the new archlinux vagrant image. I found multiple issues during that. Each important commit has a description which describes the changes.

bastelfreak · 2017-05-21T14:10:46Z

Acceptance tests on vagrant ubuntu16.04 and debian-8 fail, the module isn't idempotent there. It works on fedora-25, fedora-24, archlinux, centos-7,

bastelfreak · 2017-05-21T14:16:37Z

spec/acceptance/nodesets/archlinux-2-x64.yml

@@ -0,0 +1,13 @@
+---


I will add this to modulesync_config if this PR gets accepted

bastelfreak · 2017-05-21T14:17:14Z

spec/spec_helper_acceptance.rb

+require 'beaker/puppet_install_helper'
+require 'beaker/module_install_helper'
+
+run_puppet_install_helper unless ENV['BEAKER_provision'] == 'no'


there are so many different ways to install puppet in a module, is this the current best practice?

bastelfreak · 2017-05-21T14:18:36Z

Gemfile

@@ -55,7 +55,7 @@ group :system_tests do
    gem 'beaker-rspec',  :require => false
  end
  gem 'serverspec',                    :require => false
-  gem 'beaker-puppet_install_helper',  :require => false
+  gem 'puppet-module-posix-system-r2.3',  :require => false


Puppetlabs bundled all their gems in a metagem called puppet-module-posix-system. I'm notsure if I really like this or if we should instead include all the gems directly. However, this should be changed in modulesync_config as well.

also: https://tickets.puppetlabs.com/browse/MODULES-4948 😢

ekohl · 2017-05-21T14:49:43Z

spec/spec_helper_acceptance.rb

+  # Configure all nodes in nodeset
+  c.before :suite do
+    hosts.each do |host|
+      copy_module_to(host, :source => proj_root, :module_name => 'ntp')


I believe the install_module_on helper already does this for you.

yep! updated

oranenj · 2017-05-21T17:29:37Z

manifests/agent.pp

+  $tlsservercertissuer                    = $zabbix::params::agent_tlsservercertissuer,
+  $tlsservercertsubject                   = $zabbix::params::agent_tlsservercertsubject,
+  String $agent_config_owner              = $zabbix::params::agent_config_owner,
+  String $agent_config_group              = $zabbix::params::agent_config_group,


It's a bit hard to tell here if you added parameters or if this is just whitespace fixing.
It seems agent_config_* are new?

thats why I throw them in their own commit: 8bdcadd

oranenj · 2017-05-21T17:31:16Z

manifests/agent.pp

@@ -375,4 +380,13 @@
        'ESTABLISHED'],
    }
  }
+  # the agent doesn't work perfectly fine with selinux
+  # https://support.zabbix.com/browse/ZBX-11631
+  if $facts['os']['selinux']['config_mode'] == 'enforcing' {


Maybe make this conditional on $manage_selinux?

yep, added.

apenney · 2017-05-21T17:40:23Z

templates/zabbix_agentd.conf.erb

 ### Option: LogFile
 #	Name of log file.
 #	If not set, syslog is used.
 #
-LogFile=<%= @logfile %>
+LogFile=<%= @logfile %><% end %>
 <% end %>

 ### Option: LogFileSize


Would it not make sense to wrap the if around the comment block so it doesn't appear if you don't set it?

yep, fixed.

bastelfreak · 2017-05-21T21:13:23Z

If we want to do this the beautiful way we first need to merge:

this got successfully tested at voxpupuli/puppet-zabbix#382

3.0 is really old, 3.2 is stable since a long time. We need the 3.2 package for proper acceptance tests.

We hardcoded the default owner/group, this was bad. This is now configureable. Also we use a separate user on archlinux. This prohibits the agent from reading the configs from the proxy or server.

We manage selinux in a few places. There are systems where this is prohibited for puppet. We will manage it when it is on enforcing. This parameter allows people to disable it in each class, even if it is on enforcing.

the user is optional, we don't have to provide it.

We added the --foreground to honor the systemd upstream recommendations. However, this option is only available in zabbix 3.0 and newer. 2.2 still has support. Additional information: voxpupuli#382 http://www.zabbix.com/life_cycle_and_release_policy https://www.zabbix.com/documentation/2.4/manpages/zabbix_agentd https://www.zabbix.com/documentation/3.0/manpages/zabbix_agentd

bastelfreak · 2017-06-03T11:40:36Z

Hi @ianssoftcom, thanks for the notice. I created #396

We added the --foreground to honor the systemd upstream recommendations. However, this option is only available in zabbix 3.0 and newer. 2.2 still has support. Additional information: voxpupuli#382 http://www.zabbix.com/life_cycle_and_release_policy https://www.zabbix.com/documentation/2.4/manpages/zabbix_agentd https://www.zabbix.com/documentation/3.0/manpages/zabbix_agentd

bastelfreak self-assigned this May 21, 2017

bastelfreak force-pushed the bastelfreak_acceptance_tests branch from f03508e to 8f97884 Compare May 21, 2017 13:33

bastelfreak commented May 21, 2017

View reviewed changes

bastelfreak force-pushed the bastelfreak_acceptance_tests branch from 8f97884 to 586cbee Compare May 21, 2017 14:22

ekohl reviewed May 21, 2017

View reviewed changes

oranenj reviewed May 21, 2017

View reviewed changes

apenney reviewed May 21, 2017

View reviewed changes

bastelfreak force-pushed the bastelfreak_acceptance_tests branch 6 times, most recently from 8dadff3 to b23d4a2 Compare May 21, 2017 20:50

bastelfreak mentioned this pull request May 21, 2017

Migrate tests to rspec-puppet-facts #383

Merged

bastelfreak force-pushed the bastelfreak_acceptance_tests branch from 11585a6 to 1659dcc Compare May 21, 2017 21:12

bastelfreak force-pushed the bastelfreak_acceptance_tests branch from 1659dcc to b634c50 Compare May 21, 2017 22:52

bastelfreak added a commit to bastelfreak/modulesync_config that referenced this pull request May 23, 2017

add archlinux nodeset

852f5d5

this got successfully tested at voxpupuli/puppet-zabbix#382

bastelfreak mentioned this pull request May 23, 2017

add archlinux nodeset voxpupuli/modulesync_config#351

Merged

bastelfreak force-pushed the bastelfreak_acceptance_tests branch 2 times, most recently from 6690d22 to 42d8fc3 Compare May 23, 2017 18:25

bastelfreak changed the title ~~[WIP] Add acceptance tests + multiple bugfixes~~ Add acceptance tests + multiple bugfixes May 23, 2017

bastelfreak force-pushed the bastelfreak_acceptance_tests branch 3 times, most recently from dede134 to 9295695 Compare May 24, 2017 16:38

add initial spec tests

7f304a0

bastelfreak added 16 commits May 24, 2017 19:21

use native facts hash in params.pp

8b29d11

bump default zabbix version 3.0->3.2

e4453c4

3.0 is really old, 3.2 is stable since a long time. We need the 3.2 package for proper acceptance tests.

provide sane defaults for agent_pidfile

c65d40a

provide sane defaults for systemd vs non-systemd

6fb4086

configure default owner/group for zabbix agent configs

c22b599

We hardcoded the default owner/group, this was bad. This is now configureable. Also we use a separate user on archlinux. This prohibits the agent from reading the configs from the proxy or server.

add puppet4 datatypes + reformat of agent class

8a0a24c

enable acceptance tests via docker on travis

b2c0915

simplify spec_helper_acceptance.rb

5e60802

Add $manage_selinux parameter

bb0a807

We manage selinux in a few places. There are systems where this is prohibited for puppet. We will manage it when it is on enforcing. This parameter allows people to disable it in each class, even if it is on enforcing.

update system_tests gem group

90a7f6f

fix spec tests for startup class

c5be8a9

fix startup class

66cac0a

the user is optional, we don't have to provide it.

fix systemd templates + spec tests

c970da8

fix selinux handling

3b9e2da

rubocop: autofix

ca1a335

simplify acceptance tests

cc7400e

bastelfreak force-pushed the bastelfreak_acceptance_tests branch from 9295695 to cc7400e Compare May 24, 2017 17:22

dhollinger merged commit 63effd6 into voxpupuli:master May 24, 2017

dhollinger added the backwards-incompatible label May 24, 2017

dhollinger changed the title ~~Add acceptance tests + multiple bugfixes~~ BREAKING: Add acceptance tests + multiple bugfixes May 24, 2017

bastelfreak mentioned this pull request Jun 3, 2017

only add --foreground on 3.0 and newer #396

Merged

bastelfreak deleted the bastelfreak_acceptance_tests branch June 3, 2017 11:40

bastelfreak mentioned this pull request Jun 5, 2017

Fix rpm key handling + changing default values depending on zabbix version #399

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BREAKING: Add acceptance tests + multiple bugfixes #382

BREAKING: Add acceptance tests + multiple bugfixes #382

bastelfreak commented May 21, 2017

bastelfreak commented May 21, 2017

bastelfreak commented May 21, 2017

bastelfreak May 21, 2017

bastelfreak May 21, 2017

bastelfreak May 21, 2017

bastelfreak May 21, 2017

ekohl May 21, 2017

bastelfreak May 21, 2017

oranenj May 21, 2017

bastelfreak May 21, 2017

oranenj May 21, 2017

bastelfreak May 21, 2017

apenney May 21, 2017

bastelfreak May 21, 2017

bastelfreak commented May 21, 2017 •

edited

Loading

bastelfreak commented Jun 3, 2017

BREAKING: Add acceptance tests + multiple bugfixes #382

BREAKING: Add acceptance tests + multiple bugfixes #382

Conversation

bastelfreak commented May 21, 2017

bastelfreak commented May 21, 2017

bastelfreak commented May 21, 2017

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 May 21, 2017 • edited Loading

bastelfreak commented Jun 3, 2017

bastelfreak commented May 21, 2017 •

edited

Loading