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

modulesync 2.12.0 / zabbix::web: Allow httpd to speak to the database #680

Merged
merged 6 commits into from
Apr 26, 2020

Conversation

bastelfreak
Copy link
Member

modulesync 2.12.0

Copy link
Member

@dhoppe dhoppe left a comment

Choose a reason for hiding this comment

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

@bastelfreak
Copy link
Member Author

bastelfreak commented Apr 19, 2020

so:

  • spec/acceptance/server_spec.rb passes locally for me
  • I can reduce the 5 errors in spec/acceptance/zabbix_application_spec.rb to 3 with this spatch:
diff --git a/spec/acceptance/zabbix_application_spec.rb b/spec/acceptance/zabbix_application_spec.rb
index 61129f8..b6d6d65 100644
--- a/spec/acceptance/zabbix_application_spec.rb
+++ b/spec/acceptance/zabbix_application_spec.rb
@@ -38,7 +38,15 @@ describe 'zabbix_application type' do
     # 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
+      /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
+      if id postgres > /dev/null 2>&1; then
+        su - postgres -c "psql -c 'drop database if exists zabbix_server;'"
+      fi
+      SHELL
+      shell(cleanup_script)
 
       apply_manifest(pp1, catch_failures: true)
     end

(which I would like to refactor into a method so all acceptance test files can use it)

@bastelfreak
Copy link
Member Author

I restarted the last successful merge to master build: https://travis-ci.org/github/voxpupuli/puppet-zabbix/jobs/675009991

@bastelfreak
Copy link
Member Author

whyyyyyyyyyyyyyyyyyyyyyyyyyyy does it pass

@bastelfreak
Copy link
Member Author

I created a simple PR against master that only touches our README.md: #681

@bastelfreak
Copy link
Member Author

which passed. Now I cherry-picked the cleanup commit into another PR:
#682

@bastelfreak
Copy link
Member Author

@baurmatt I'm still debugging this, but if you've any ideas why this works on master but fails here, please let me know :)

Without this change it's not possible to execute tests in random orders
/ the agent_spec.rb after any other acceptance test.
Turns out our acceptance tests fail in vagrant because the default
centos image has selinux enabled. The docker image on travis not. The is
an selinux boolean to allow the httpd service to speak to a database. By
default this is of. This PR fixes this.
@bastelfreak
Copy link
Member Author

I've no clue why the code in docker fails to create a database for one zabbix version:
https://travis-ci.org/github/voxpupuli/puppet-zabbix/jobs/679220993#L9204

I'm unable to reproduce this locally with vagrant

@bastelfreak bastelfreak force-pushed the modulesync branch 4 times, most recently from 8445187 to 47caa55 Compare April 25, 2020 11:47
@baurmatt
Copy link
Contributor

@bastelfreak Ha! I've seen something similar with Ubuntu 18.04. Turned out that it ships a /etc/dpkg/dpkg.cfg.d/excludes which includes the following directories by default:

# Drop all man pages
path-exclude=/usr/share/man/*

# Drop all documentation ...
path-exclude=/usr/share/doc/*

# ... except copyright files ...
path-include=/usr/share/doc/*/copyright

# ... and Debian changelogs
path-include=/usr/share/doc/*/changelog.Debian.*

This led to /usr/share/doc/zabbix--pgsql-3.4/*.sql not being installed and hence failing tests.

I think the problem here is, that nodocs isn't removed anymore from /etc/yum.conf. That's probably the equivalent in CentOS.

c.formatter = :documentation
hosts.each do |host|
if host[:platform] =~ %r{el-7-x86_64} && host[:hypervisor] =~ %r{docker}
on(host, "sed -i '/nodocs/d' /etc/yum.conf")
Copy link
Contributor

Choose a reason for hiding this comment

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

@bastelfreak This shitty thing! :D

Copy link
Member

Choose a reason for hiding this comment

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

So we need this? What breaks?

Copy link
Contributor

Choose a reason for hiding this comment

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

I leads to files in /usr/share/doc/* not being installed. As Zabbix places their database initialization script there, this breaks our acceptance CI.

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes, I now see the IRC conversation. Pointing @bastelfreak in the right direction.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @baurmatt . We figured the same out in parallel yesterday on IRC. The current code should now work again on docker images. Are you already working on Ubuntu 18/20 support for the module?

@bastelfreak bastelfreak changed the title modulesync 2.12.0 modulesync 2.12.0 / zabbix::web: Allow httpd to speak to the database Apr 26, 2020
@bastelfreak bastelfreak added bug Something isn't working and removed modulesync labels Apr 26, 2020
@bastelfreak bastelfreak merged commit 13a4eb5 into master Apr 26, 2020
@bastelfreak bastelfreak deleted the modulesync branch April 26, 2020 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants