saltstack-formulas / php-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
57 stars 231 forks source link

Use pkg.latest? #68

Closed estahn closed 8 years ago

estahn commented 8 years ago

I just added backports and PHP didn't update. I had to change pkg.installed to `pkg.latest (https://github.com/saltstack-formulas/php-formula/blob/bd10f4ea92ba2f25d3474286563c91a671b0e289/php/ng/installed.jinja#L40).

gravyboat commented 8 years ago

By default in most formulas we don't use pkg.latest because it can really screw users who need a specific version especially on subsequent runs. I'd prefer to keep it set to pkg.installed for now, and as you have done users can modify to pkg.latest if they need the latest package. Alternatively you can use the version option within pkg.installed if you need a specific release.

puneetk commented 8 years ago

@gravyboat are you opposed to it being conditional and pillar driven ?

gravyboat commented 8 years ago

@puneetk Not really, just seems to complicate the formula. Let's see what @estahn has to say regarding the version value and whether that would solve his issue.

estahn commented 8 years ago

@gravyboat It's really up to you. My opinion is that pkg.installed is not quite predictable in defining the actual state. If you look at infrastructure that spins up and tears down nodes dynamically (e.g. autoscaling) you will eventually end up with nodes in different states. Some will run PHP 5.4.1 then others run PHP 5.4.2-security-patch1. So you will end up to manage this anyways.

I'm usually go with pinning through apt if a specific version is necessary.

Thoughts?

gravyboat commented 8 years ago

The formulas themselves need to have predictable behavior and I don't think using pkg.latest would provide that. If you'd like to add another state or the logic to support pkg.latest I'm fine with that.

estahn commented 8 years ago

Well i guess it comes down to what you define as predictable :)

gravyboat commented 8 years ago

Updating the package unfortunately isn't predictable enough. What happens if you manage multiple distros that both get PHP? Now you have drift between those unless the same versions are available from the repo. There's also the major headache of version drift depending on how often you update your systems, or if you need legacy code. Imagine if you showed off this PHP formula to your boss and it updated the system you were using then broke production because it updated to a version of PHP you don't support in your code? I guarantee if we changed this to use pkg.latest someone would open an issue within 24 hours saying it broke their environment and should just do a pkg.installed, unfortunatey we can't make everyone happy since everyone works in a different environment. In addition to that the default behavior across all the formulas currently is to use pkg.installed, so we'd have to modify it for all of them after coming to a consensus to ensure behavior was the same across every formula.

As @puneetk noted It would be a pretty minor flag to add in the pillar and state:

{% if salt['pillar.get']('php:latest') %}
php_install_{{ state }}:
  pkg.latest:
    - name: {{ state }}
    - pkgs: {{ pkgs|json() }}
{% else %}
php_install_{{ state }}:
  pkg.installed:
    - name: {{ state }}
    - pkgs: {{ pkgs|json() }}
{% endif %}

We just have to ensure we're providing a good starting point for users and pkg.latest doesn't provide the safety someone unfamiliar with the formula might need.

edit: Added a section to the first paragraph with notes regarding current setup across all formulas.

estahn commented 8 years ago

@gravyboat Backwards compatibility is a fair point. In regards to updating the system. Security Patches should be installed regularly anyways or automatically. I don't see how you're able to update your packages with the current formula as it is. I understand your argument regarding multiple distros, but with the current formula i even have multiple nodes with the same distro on different versions of PHP.

Anyways, i'm fine with my current solution or i'm giving the version flag a shot.

Feel free to re-open the issue.

gravyboat commented 8 years ago

@estahn Sounds good, and I totally agree being up to date is important for security patches!