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

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

Open
daks opened this issue Oct 30, 2020 · 4 comments
Open

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

daks opened this issue Oct 30, 2020 · 4 comments
Labels

Comments

@daks
Copy link
Member

daks commented Oct 30, 2020

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 daks added the bug label Oct 30, 2020
@daks
Copy link
Member Author

daks commented Oct 30, 2020

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
Copy link
Member

javierbertoli commented Oct 30, 2020

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 🙄

@myii
Copy link
Member

myii commented Oct 30, 2020

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

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

@daks
Copy link
Member Author

daks commented Nov 7, 2020

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants