[BUG] On Debian, does not work by default

saltstack-formulas / php-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html

Other

57 stars 232 forks source link

[BUG] On Debian, does not work by default #217

Open daks opened 3 years ago

daks commented 3 years ago

Versions reports (master & minion)

Using salt-ssh 3001.1 installed in a virtualenv. Minion is Debian Buster, no master.

Pillar / config used

top.sls contains

base:
  '*':
    - php

No pillar.

Bug details

Describe the bug

This does not work out of the box

----------
          ID: php_install_php
    Function: pkg.installed
        Name: php
      Result: False
     Comment: The following packages failed to install/update: php7.0
     Started: 13:21:35.587509
    Duration: 578.336 ms
     Changes:

Steps to reproduce the bug

Expected behaviour

I expect the formula to work out of the box, with sane defaults so I can use it to simply install available PHP packages on Debian distribution.

Attempts to fix the bug

I still have not looked at how the formula exactly works, but it looks like without external repository I can't use it to use PHP packages from vanilla Debian repositories.

Additional context

daks commented 3 years ago

Solved using following pillar

php:
  version: ''

which lets the formula find all php-* packages available on Debian. Those names are always available and pull available versions (7.1 for oldstable, 7.3 for stable, 7.4 for actual testing)

javierbertoli commented 3 years ago

I think we should make it default to just php, which is Debian's meta-package to latest version available for the distro, right?

Though I checked the map.jinja and php.installed.jinja, and the value for php_version is hardcoded in both.

I think this formula needs some major refactoring :roll_eyes:

myii commented 3 years ago

This shares a common thread with #214, #215 and #216, in that this formula needs to go through the following steps:

Add the map.jinja verifier (after finalising the PR in the openvpn-formula).
Update to the latest map.jinja and YAML files (even if v3 rather than v4 -- although I'd prefer the latter).
Ensure each specific issue is fixed.

daks commented 3 years ago

I think we should make it default to just php, which is Debian's meta-package to latest version available for the distro, right?

Though I checked the map.jinja and php.installed.jinja, and the value for php_version is hardcoded in both.

I think this formula needs some major refactoring roll_eyes

Could work for simple setup (like stated in the description) but once you want php.fpm you need to have the correct version number because the service is named php<version>-fpm.

Not easy to solve...