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

Fix for php db package name on Ubuntu 16.04 #284

Merged
merged 12 commits into from
Oct 12, 2016

Conversation

frozenfoxx
Copy link
Contributor

@frozenfoxx frozenfoxx commented Sep 30, 2016

As explained in #282 the code for installing the php5 database package does not currently work on Ubuntu 16.04. This is because the name has changed from "php5-${db}" to "php-${db}" in the official repos. This pull will update it only in those instances and still use the legacy facts for determining version for backward compatibility.

@frozenfoxx
Copy link
Contributor Author

It looks like the puppet-zabbix/spec/classes/web_spec.rb lines 59 and 70 are upset with this change since they're looking explicitly for the nonexistent php5-pgsql package instead of the new php-pgsql package. I would correct this but I'm not really sure how to rewrite the spec to classify a different package name depending on facts[:os][:release][:major] version being >= 16.04. I'm looking for an example but if someone else can help me rewrite that I think it will pass the tests just fine.

@bastelfreak
Copy link
Member

Hi @frozenfoxx, thanks for the PR! I guess you have to do something disgusting like I did here:
https://github.com/voxpupuli/puppet-zabbix/blob/master/spec/classes/agent_spec.rb#L17-L22

@@ -56,7 +56,20 @@ def package_provider_for_gems
let :params do
super().merge(database_type: 'postgresql')
end
packages = facts[:osfamily] == 'RedHat' ? ['zabbix-web-pgsql', 'zabbix-web'] : ['zabbix-frontend-php', 'php5-pgsql']

pgsqlpackage = ""
Copy link
Member

Choose a reason for hiding this comment

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

this is not needed

@@ -67,7 +80,20 @@ def package_provider_for_gems
let :params do
super().merge(database_type: 'mysql')
end
packages = facts[:osfamily] == 'RedHat' ? ['zabbix-web-mysql', 'zabbix-web'] : ['zabbix-frontend-php', 'php5-mysql']

mysqlpackage = ""
Copy link
Member

Choose a reason for hiding this comment

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

not needed

case facts[:osfamily]
when 'Debian'
if facts[:operatingsystemmajrelease] >= '16.04'
pgsqlpackage = "php-pgsql"
Copy link
Member

Choose a reason for hiding this comment

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

migrate all of the double quotes to single quotes unless they contain a variable


pgsqlpackage = ""
case facts[:osfamily]
when 'Debian'
Copy link
Member

Choose a reason for hiding this comment

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

please migrate to ConditionalAssignment

mysqlpackage = ""
case facts[:osfamily]
when 'Debian'
if facts[:operatingsystemmajrelease] >= '16.04'
Copy link
Member

Choose a reason for hiding this comment

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

same here, please migrate to ConditionalAssignment

@@ -56,7 +56,25 @@ def package_provider_for_gems
let :params do
super().merge(database_type: 'postgresql')
end
packages = facts[:osfamily] == 'RedHat' ? ['zabbix-web-pgsql', 'zabbix-web'] : ['zabbix-frontend-php', 'php5-pgsql']

pgsqlpackage = case facts[:operatingsystem]
Copy link
Member

Choose a reason for hiding this comment

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

almost feels like package maintainer hate us

@bastelfreak bastelfreak merged commit f245992 into voxpupuli:master Oct 12, 2016
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.

2 participants