Establish `template-formula` as the go-to resource for all formula development

myii commented 5 years ago

This issue is a collection of ideas initially taken from the log of discussions^[1]^[2]^[3]^[4] captured in the wiki.

Visibility

[x] Make this template-formula a sticky formula for the whole SaltStack Formulas project.
- Probably best to wait until most of these changes have been made, to prevent premature use of the template.

Release tagging (for each merged PR)

[x] Apply semantic versioning to this formula -- @aboe76.
[x] Prepare bump:... labels to use when evaluating the extent of change each PR will have on the formula. Helps when bumping version numbers for releases.
- bump:major
- bump:minor
- bump:patch
- Note: If this works well, there are methods available to sync labels across all of the repos, such as this search or using the same keywords in a search engine.
[x] Modify the CHANGELOG to use the format proposed by https://keepachangelog.com/en/1.0.0/:
- If doing this, the relevant section in the main documentation must be updated as well.
- An alternative method is to use https://github.com/vaab/gitchangelog; this automates the process but it does go completely against the advice at https://keepachangelog.com/en/1.0.0/.
- @aboe76 looking into an automation method, with this as an example committed to the django-formula repo.
[x] Add an AUTHORS file, with the primary purpose of identifying the key maintainers of the repo.
- @aboe76 looking into an automation method, with this as an example produced for the salt-formula repo.
[x] Configure semantic-release for this formula, including updating the changelog and updating the FORMULA file -- #35.
[x] Configure commitlint to warn when commit messages aren't appropriate for bumping the version number -- #41.
[x] Update authors file alongside bumping the version number -- #42.
[x] Update the table of contents in Markdown files alongside bumping the version number -- #43.
[x] Convert Markdown files to RST alongside bumping the version number -- #69.
[ ] Collect feedback from the following PRs, which have implemented semantic-release (keeping this list in chronological order for the most part):
- vault-formula
- collectd-formula
- nginx-formula
- cert-formula
- chrony-formula
- rkhunter-formula
- postgres-formula -- specific commit
- keepalived-formula
- systemd-formula
- ufw-formula
- salt-formula
- fail2ban-formula
- bind-formula
- syslog-ng-formula
- apt-formula
- locale-formula
- sudoers-formula
- postfix-formula
- iptables-formula
- golang-formula -- with follow-up
- logrotate-formula
- php-formula -- with follow-up
- deepsea-formula
- mysql-formula
- libvirt-formula
- openvpn-formula
- sysstat-formula
- dhcpd-formula
- ...
- (Numerous other formulas converted after this point but tracked in the documentation instead).
- In progress:
  - packages-formula
- Other (direct, non-PR) implementations include:
  - prometheus-formula
  - grafana-formula
[ ] The script pre-commit_semantic-release.sh could check for the existence of the docs dir.
[x] Investigate how the semantic-release (and/or the TOFS pattern?) can be used to mitigate the use of formula:ng states:
- Most of those can be integrated and removed and only keep one formula.
- Also refer to the discussions at:
  - https://github.com/saltstack-formulas/apache-formula/pull/251
  - https://github.com/saltstack-formulas/nginx-formula/pull/233
- Most complete conversion method (in stages) done for the php-formula:

YAML & Jinja

[x] Update to new map.jinja style -- #20.
[ ] Issue with new map.jinja style due to defaults.merge vs. salt-ssh -- Upstream bug report: https://github.com/saltstack/salt/issues/51605.
[x] Use intermediate map.jinja style for the time being -- #25.
[x] Update YAML to show all possible OSes that can be supported -- #23.
[x] Add other used OS grains-based YAML files; also update map.jinja accordingly.
- oscodename -- e.g. postgres-formula.
- osfinger -- e.g. apache-formula -- #25.
[ ] Add a macros.jinja template with common macros that "Any formulae would find useful":
- Repeatable patterns deserve repeatable code.
[ ] Evaluate whether the 'masterless' extra_yamls approach outlined by @javierbertoli is another approach to solve scalability issues (awaiting sample PR).
[ ] Decide a consistent method to work with jinja_lstrip_blocks and more specfically jinja_trim_blocks until the upstream issue is resolved.
- Example of the problem and the side-effects of the workaround.
[ ] Confirm (and then globally apply) standard of always using ~ for string concatenation instead of +.
- Two sides of the argument starting here: https://github.com/saltstack-formulas/php-formula/pull/172#discussion_r274213741.
[x] Use | json instead of | tojson, at least until 2017.7 support is required -- refer back to this comment.
- Another comment: https://github.com/saltstack-formulas/consul-formula/pull/37#discussion_r230666593
- Commit showing 2018.3.3: https://github.com/saltstack/salt/commit/1499c6abcf8076a2fab43c608cc46ed93c9e6635
- Note that there are issues with both | json and | tojson, so | yaml may be preferable (at least in certain situations), see:
  - https://github.com/saltstack-formulas/postgres-formula/issues/253#issuecomment-470064605
  - https://github.com/saltstack-formulas/postgres-formula/issues/253#issuecomment-515751308
[x] Prefer {%- ... %} as a standard due to linked issues raised in this comment.
[ ] Consider tracking "custom" grain values to the map to cover for situations when the actual grain is unavailable, such as osmajorrelease and osfinger on Arch Linux.
- PR mentioning the issue: https://github.com/saltstack-formulas/firewalld-formula/pull/29
- Potential solution at the end of map.jinja:
  - {%- do template.update({'osmajorrelease': salt['grains.get']('osmajorrelease', 0)|int }) %}
[x] Determine strategy regarding YAML keys with no given value such as comp_lzo: -- insist on explicit values at all times by modifying our yamllint configurations based on https://yamllint.readthedocs.io/en/stable/rules.html#module-yamllint.rules.empty_values?
[ ] Discuss the use of yaml_dump.sls (via. ssf-formula) in all formulas so that changes to the map before and after can easily be picked up in the CI logs.

TOFS pattern vs. pillar

[x] Switch to the TOFS pattern (files_switch) -- #19.
[x] If required, update TOFS pattern to allow working with custom files -- #28.
- See top of discussion on 2019-02-11 log of discussions.
[x] Use source_files instead of files in the pillar (otherwise there are two files, which will cause confusion) -- #76.
[x] Use libtofs.jinja instead of macros.jinja, to work towards the simplest drop-in solution for other formulas -- #79.

Other pillar-avoidance methods to consider

[ ] Consider changing all uses of pillar.get to config.get globally (as in, for all formulas eventually).
[ ] Work through discussions captured from Slack where alternatives are discussed (TODO: capture the discussions in a wiki page here).

Make the formula portable

[x] Find a solution for tpldir not working in template files, see https://github.com/saltstack/salt/issues/41195.
- There is the workaround of sending it through as a context variable.
[ ] Test using tplroot throughout the formula and then changing the root directory to ensure the whole formula is using the new namespace.
[ ] Consider this comment by @noelmcloughlin re: how to support old and new functionality and having a deprecation scheme.

Documentation

[x] Set up Read the Docs for documentation, while maintaining readability from GitHub (i.e. using the docs sub-directory.
[ ] Set up Antora (Asciidoctor-based) documentation, to be deployed on GitHub pages.
- Introducing Antora:
  - https://matthewsetter.com/why-antora-is-the-leading-technical-writing-platform/
  - https://matthewsetter.com/antoras-three-core-concepts/
- List of showcase sites: https://gitlab.com/antora/antora.org/issues/20
- Prepare template module structure for each of deploying new modules.
  - https://docs.antora.org/antora/2.0/modules/#pages-dir
  - Use .keep to push directory structure to Git.
  - Ensure entry in .gitignore to prevent assets being committed (also need the proper solution for these).
- Collect reference links to all of the existing documentation sites out there, for gaining insights in how to perform org-wide configurations for ourselves (inc. Fedora, couchbase, ...)
  - https://github.com/couchbase/docs-site/blob/master/staging-antora-playbook.yml
  - https://github.com/couchbase/docs-site/blob/master/production-antora-playbook.yml
- Later on, consider: https://www.algolia.com/for-open-source/
[ ] Add documentation: guides, proposals, etc.
- Use the template-formula wiki the central location for planning/organisation/reference.
- Idea: @javierbertoli explaining a method that can be used for managing numerous formulas, when formulas are lacking version numbers (near top of 2019-02-12 log of discussions).
[x] Document how to contribute to the RTD-based documentation (e.g. template for new pages).
[ ] Document how to create a formula from this template one, inside this repository or/and in saltstack docs -- @daks looking into this.
[ ] Document how to apply all of the changes made to this formula to existing formulas.
[ ] Document best practices for testing locally during development; current useful examples to build upon:
- tmux-formula
- postgres-formula
[ ] Document the deprecation process for formulas, such as when promoting formula:ng to formula -- see https://github.com/saltstack-formulas/nginx-formula/pull/236.

Tests and CI

[x] Add a test scaffold with kitchen-ci and inspec (or testinfra) examples for the basic tasks (add/remove package, check service, etc) -- want to replace old serverspec tests -- #34.
- Discussions so far seem to be leaning towards inspec.
- Goss and saltcheck also mentioned -- perhaps alongside for the latter?
- Also consider saltscaffold.
[x] ~Compare this with the formula-test-harness introduced to the nginx-formula in this pull.~
- See discussion from here to see why no longer considering this.
[x] Find solution for opensuse not working -- #45.
[x] For instances that requiring restarting (e.g. opensuse usually works the second time its run), evaluate ways of minimising these delays:
- Try travis_retry.
- Try matrix: allow_failures: - os: opensuse-leap.
- Resolved by #45 -- restarts no longer required.
[ ] Add tests for all clean stages -- @javierbertoli looking into this.
[x] Work through this list, closing (and even reverting?) the PRs for the formula-test-harness, since it only has 1 contributor and is no longer maintained.
[ ] Add tests for TOFS, as suggested by @daks in #76.
[x] Use specific versions in Gemfile, to avoid test failures -- #81.
[ ] Consider merging (locale-specific) provision_command for each platform, as used in the systemd-formula -- only if actually useful, not just for the sake of it.
[x] Use pre-salted images for testing for improved performance and reliability -- @javierbertoli has provided the image and @myii has made PR #92.
[x] Update: .kitchen.yml => kitchen.yml -- done in #40.
[x] Update: pillars-from-files => pillars_from_files -- done in #74.

States

[ ] Make the formula "actually functional" so that all parts can be run, to aid understanding and quality assurance.
[ ] Adapt template-formula to not be too focussed on pkg:
- Must also consider archives, tarball releases, GitHub repos, etc.
[x] Add the remove state advocated by @noelmcloughlin.
[x] Ensure that each file does only one thing and do it well (instead of monolithic files):
- E.g. pkg/install.sls, pkg/clean.sls, repo/install.sls, repo/clean.sls, etc.
[ ] Add 'formula components' that can be reused in a formula as suggested by @javierbertoli, such as:
- 'service-template'
- 'pkg-template'
- 'ini-config-template'
- 'yaml-config-template'
- Etc.
[ ] Prevent the formula from running on unsupported minions -- #91.

Repo standards

[x] Complete all guidelines at https://github.com/saltstack-formulas/template-formula/community -- issue #70 opened for this for tracking progress.
[ ] Consider how Security Advisories apply to us.
[x] Consider having a develop branch to allow PRs with the very latest changes, that can be merged back into master at the appropriate times.
- As mentioned in the 4+ comments below starting from: https://github.com/saltstack-formulas/template-formula/issues/21#issuecomment-496053734.
- A real example of where this would be useful is https://github.com/saltstack-formulas/users-formula/issues/198.

Other resources to evaluate for inclusion

[ ] Evaluate https://github.com/claudekenni/stack-formula.
[ ] Evaluate https://github.com/hbokh/awesome-saltstack.
[ ] Evaluate https://github.com/ministryofjustice/salt-shaker.
[ ] Evaluate #18 to see if it can be merged into this issue.
[ ] Evaluate https://github.com/salt-formulas to learn from their ecosystem as discussed in this comment and this comment.
[ ] Need to determine the right processes for ensuring formulas work for all officially supported versions of Salt, while also clearing out redundant content for version that are no longer within the support lifecyle.
- This also includes updating/removing for supported/unsupported platforms as well.
- See this PR discussion as an example of the issues faced.

daks commented 5 years ago

Just an idea: document how to create a formula from this template one, inside this repository or/and in saltstack docs

myii commented 5 years ago

@daks I've added that to the task list above -- thanks for that suggestion.

alxwr commented 5 years ago

@myii I just wanted to say „Thank you!“ :-) This formula enabled me to implement https://github.com/saltstack-formulas/prometheus-formula really fast. I found the structure and especially your turnkey Travis/kitchen configuration very helpful.

myii commented 5 years ago

@alxwr That's really good to hear and it makes all of this effort worthwhile. There were lots of contributions involved, including your own! Good to see that you've got semantic-release running as well.

We've got an upcoming issue due to wanting to implement config.get properly, instead of pillar.get. The only solution I've been able to find so far is using defaults.merge so I'm going to submit a PR in order to open up discussion. Since you're familiar with salt-ssh, I'll request a review from you as well, so that you have the opportunity to argue against my proposal! Actually, the alternative is to go back to pillar.get and that may be the final conclusion anyway...

alxwr commented 5 years ago

@myii AFAIR we could work around the defaults.merge issue, because we can detect whether salt or salt-ssh is used. But this is not pretty. :-) Maybe it's time for a libworkaround.jinja which would be living documentation of several incompatibilities and shortcomings that need to go. :-)

myii commented 5 years ago

@alxwr So there's a reliable way to check for salt-ssh? That's excellent news. So we can have both methods of merging the map available and then no-one loses out. Do you mind sharing it here? Then I can include it in the PR immediately, so that everyone can get on with reviewing it, rather than having a long discussion first. I will ensure that you are credited as the author of that specific commit, identifying whether salt or salt-ssh is being used.

alxwr commented 5 years ago

@myii Sry for the delay.:-) The way I found most straightforward, is inserting grains:salt_ssh: True in the roster file. But I get, that this is not exactly portable.

The second most reliable way is to compare paths:

salt host grains.itms:

    pythonpath:                                                                                                                                                                               
        - /usr/local/bin                                                                                                                                                                      
        - /usr/local/lib/python36.zip                                                                                                                                                         
        - /usr/local/lib/python3.6                                                                                                                                                            
        - /usr/local/lib/python3.6/lib-dynload                                                                                                                                                
        - /usr/local/lib/python3.6/site-packages
    saltpath:
        /usr/local/lib/python3.6/site-packages/salt

salt-ssh host2 grains.itms:

    pythonpath:                                                                                                                                                                               
        - /var/tmp/.root_99b0b6_salt/py3                                                                                                                                                      
        - /var/tmp/.root_99b0b6_salt/pyall                                                                                                                                                    
        - /var/tmp/.root_99b0b6_salt                                                                                                                                                          
        - /usr/lib/python35.zip                                                                                                                                                               
        - /usr/lib/python3.5                                                                                                                                                                  
        - /usr/lib/python3.5/plat-x86_64-linux-gnu                                                                                                                                            
        - /usr/lib/python3.5/lib-dynload                                                                                                                                                      
        - /usr/local/lib/python3.5/dist-packages                                                                                                                                              
        - /usr/lib/python3/dist-packages
    saltpath:                                                                                                                                                                                 
        /var/tmp/.root_99b0b6_salt/pyall/salt

salt-ssh is run from a temporary directory on the minion. This shows in the grain saltpath. (We could also correlate pythonpath[0] with saltpath for higher certainty, but I think focusing on saltpath is sufficient.)

myii commented 5 years ago

@alxwr How about the method in #102? How does that work for you? I got a salt-ssh minion prepared and tested the map both ways. The __cli option appears to be reliable. I did notice the path differences you mentioned above but this appears to be even simpler.

alxwr commented 5 years ago

@myii there is another way to detect salt-ssh, but this involves the py renderer.

__opts__ differs when using salt-ssh:

def config_dir():
    if '__master_opts__' in __opts__:
        # run started via salt-ssh
        return __opts__['__master_opts__']['config_dir']
    else:
        # run started via salt
        return __opts__['config_dir']

Taken from: https://github.com/saltstack-formulas/openssh-formula/blob/master/_pillar/known_hosts_salt_ssh.sls

myii commented 5 years ago

@alxwr Do you mind if we discuss this in #102? It's a bit off-topic for this issue and then it opens up the discussion with the others, so that we can reach a resolution. It's my fault for bringing it up here first!

alxwr commented 5 years ago

@myii I was just going to suggest that. :-)

CosmicToast commented 5 years ago

Does the merge of saltstack/salt#52156 affect map.jinja and similar in any way? Might be nice as a replacement to tplroot and/or things like include: - template.X.

myii commented 5 years ago

@5paceToast Appreciate you bringing that merge to our attention. A cursory look suggests that we will be able to improve our use of include at the very least. The problem is that this will take a while to reach us; we generally have to support the three versions of Salt at any given time. So right now, that means back to 2017.7. It all depends on how far back that merge is backported.

In terms of tplroot, that's in the queue at https://github.com/saltstack/salt/pull/51814.

CosmicToast commented 5 years ago

@myii in that case, it might be nice to also have a template-formula variant that applies only to the latest release, for people interested in that (i.e for development, learning and other purposes). Would that be politically feasible at all?

myii commented 5 years ago

@5paceToast There have been conversations about having various types of template-formula. There are no right or wrong answers as such but a lot comes down to keeping everything maintained and synchronised. The general direction is to focus on this template-formula for the time being, probably until most of the tasks laid out above are completed. Not to mention the amount of time and effort it requires to propagate and review the changes when applying to the rest of the formulas, which has already been taking place thanks to numerous contributors.

In terms of a template-formula for the latest release only, I would surmise that would be fairly low priority. Not because it isn't a good idea; rather, it would be an extremely useful reference as we bring each formula up-to-date over time. However, the issue remains that we have to support all of the current Salt versions across all 300+ formulas in this org. I'm not sure if there is a formula out there that would benefit from a template-formula for the latest release only.

If someone had the desire to go ahead with this idea, I'm sure there are contributors here who would be happy to offer advice along the way, either here in GitHub or in our Slack/IRC/Matrix room. An actual functioning repo in place becomes an excellent stimulus for further discussions about the merits and pitfalls of such an approach and whether it would be something worth supporting at an org level.

Going back to https://github.com/saltstack/salt/pull/52156 for a moment: this is only available in develop (so far). If it does reach 2019.2, what will the latest release template-formula do? Meaning, the earlier versions of the 2019.2 series (e.g. 2019.2.0, 2019.2.1) won't contain that functionality, so it would be no better than where we are right now.

But from another angle, how about the idea of a template-formula that's actually based on the develop branch, rather than the latest release? Now that would definitely generate some interest. In fact, it could be worth having a develop branch in this repo, to track exactly that. I'll add that to the list above -- thanks for the idea!

myii commented 5 years ago

@5paceToast Thanks to @javierbertoli, we've now got testing images for develop. I've already run initial tests with tojson, which shows why we can't use it yet due to having to support 2017.7. So this is an excellent development.

The funny thing is when I looked at the upstream merge again in more detail, I realised that there isn't any way it can be used within this formula, since it is about pillar includes. In any case, at least the discussion led to some useful outcomes.

CosmicToast commented 5 years ago

The funny thing is when I looked at the upstream merge again in more detail, I realized that there isn't any way it can be used within this formula, since it is about pillar includes. In any case, at least the discussion led to some useful outcomes.

I noticed that too, but shortly after also noticed the include/exclude docs which claim that relative includes were added way back in 0.16.0 while parent-based (grandparent etc) includes in 2015.8. (something that might be usable in template.config.file, for example!)

It seems like some new features sometimes get missed, and some don't even make it to the docs (salt.fileserver.s3fs barely even makes sense without salt.pillar.s3 as context, for example). Which is why I think a sort of "latest" version of template-formula really makes sense (a develop branch does indeed sound great to me)!

saltstack-formulas / template-formula