refactor(naming): reserve 'pkg' as name of packaging dict

[noelmcloughlin]

prometheus on centos needs a repo setup. I've studied how best to incorporate this into this formula and the result is here. My method involved introducing a pkg dictionary to collect packaging related details. That provided the model on which to propose handling package repos. The problem is that "pkg" variable is already in use.

pkg: template

This PR is to reclaim "pkg" variable as dict. Let me know if you like the approach.

    pkg:
      name: template
      use_upstream_repo: True
      use_upstream_archive: False
      archive: {}
      repo:
        managed: {}

The followup PR will be to to add repo support to both template and then prometheus formulas.

[baby-gnu]

So if I understand, imagine I have a main program and plugins, I could have something like:

program:
  pkg:
    name: the-program
    use_upstream_repo: true
    repo:
      name: deb {{ repo.official }}/debian-{{ repo.release }}/ {{ grains.oscodename }} main
      humanname: Something Debian Repository
      
  plugins:
    an-external-plugin:
      pkg:
        name: a-cool-plugin
        use_upstream_repo: true
        repo:
          name: deb https://example.net/repo/a-cool-plugin/ {{ grains.oscodename }} stable
          humanname: Debian reposiory for a cool plugin
          key_url: https://example.net/repo/a-cool-plugin/project/gpg_key.pub
          gpgcheck: 1

Actually, I found several formulas using pkg_repo, pkgrepo or simply repo to avoid an extra level in the hash, for example:

program:
  pkg: something
  use_upstream_repo: true
  repo:
    name: deb {{ repo.official }}/debian-{{ repo.release }}/ {{ grains.oscodename }} main
    humanname: Something Debian Repository
    
  plugins:
    an-external-plugin:
      pkg: a-cool-plugin
      use_upstream_repo: true
      repo:
        name: deb https://example.net/repo/a-cool-plugin/ {{ grains.oscodename }} stable
        humanname: Debian reposiory for a cool plugin
        key_url: https://example.net/repo/a-cool-plugin/project/gpg_key.pub
        gpgcheck: 1

I like your proposal because it produce meanful program.pkg.name, program.pkg.use_upstream_repo and program.pkg.repo, we see use_upstream_repo and repo as something related to the package. The package related stuffs are located under program.pkg.

The cons would be the additional level, let see if the SaltStack community prefer the flatter one.

[baby-gnu]

If this is to be merged, this is a breaking changes and should be announce in the commit message.

[daks]

@noelmcloughlin not sure I completely understand the problem, but if I do, my comment is: if a formula manages package installation, it should have default values to indicate for example repo, package name, etc. Then those values are populated in the jinja files (osmap...), then they can be override with the lookup pillar dict.
Am I understanding it correctly?

[noelmcloughlin]

Hi @baby-gnu
Exactly - I'm really glad you understood the intention.

My 1st implementation in cloudfoundry-formula looks like your examples. I added an extra (perhaps superfluous) level.

  pkg:
    repo:
       managed:
         humanname: cloudfoundry-cli
         name: cloudfoundry-cli
         comments:
           - installed by salt
          gpgcheck: 1

My related "pkg:archive" implementation (the extra level was to allow archive.extracted detect format_type correctly) further illustrates that making pkg a dict feels important.

  pkg:
      archive:
        source: https://packages.cloudfoundry.org/stable?release=linux64-binary&version=6.44.1&source=github-rel
        extracted:
          name: /usr/local/bin
          source: /tmp/cloudfoundry/cf-cli.tgz
          source_hash: b986a55fef4bd6f580d7a070f4592362309fe009ffdd9489b9d3b871aed07aa3
          trim_output: True
          enforce_toplevel: False

Good point about breaking change - I'll update the commit message.

[noelmcloughlin]

Then those values are populated in the jinja files (osmap...), then they can be override with the lookup pillar dict.

Hi @daks, yes that correct. The intention of this PR was small breaking change to convert pkg to dict since its useful implementation detail to support upcoming archive/repo support features.

[baby-gnu]

Hello @noelmcloughlinx

The extra sublevel maybe superfluous, but we should make sure to be able to use format_kwargs as proposed in #134, which means only store under template.pkg.repo things understandable by pkgrepo.managed.

For me, use_upstream_repo & co. are better placed under template.pkg:

• I prefer to read template.pkg.use_upstream_repo than template.pkg.repo.use_upstream_repo
• I prefer to have the repository information under template.pkg.repo than template.pkg.repo.managed because I don't understand why managed is related to repository data. It's only a mirror of SaltStack state pkgrepo.managed

My related "pkg:archive" implementation (the extra level was to allow archive.extracted detect format_type correctly) further illustrates that making pkg a dict feels important

I don't get the point about archive.extracted and format_type.

Why

  pkg:
      archive:
        source: https://packages.cloudfoundry.org/stable?release=linux64-binary&version=6.44.1&source=github-rel
        extracted:
          name: /usr/local/bin
          source: /tmp/cloudfoundry/cf-cli.tgz
          source_hash: b986a55fef4bd6f580d7a070f4592362309fe009ffdd9489b9d3b871aed07aa3
          trim_output: True
          enforce_toplevel: False

is better than

  pkg:
      archive:
        name: /usr/local/bin
        source: https://packages.cloudfoundry.org/stable?release=linux64-binary&version=6.44.1&source=github-rel
        source_hash: b986a55fef4bd6f580d7a070f4592362309fe009ffdd9489b9d3b871aed07aa3
        archive_format: tar
        trim_output: True
        enforce_toplevel: False

For me, it avoid managing /tmp/cloudfoundry/cf-cli.tgz.

Regards.

[daks]

Then those values are populated in the jinja files (osmap...), then they can be override with the lookup pillar dict.

Hi @daks, yes that correct. The intention of this PR was small breaking change to convert pkg to dict since its useful implementation detail to support upcoming archive/repo support features.

OK. Does it needs to be done in template-formula or simply adapted in needed formulas? I mean template-formula has a few useful pillar key/values but those are only 'indicative' and could/should be adapted in every formula.

[noelmcloughlin]

Then those values are populated in the jinja files (osmap...), then they can be override with the lookup pillar dict.

Hi @daks, yes that correct. The intention of this PR was small breaking change to convert pkg to dict since its useful implementation detail to support upcoming archive/repo support features.

OK. Does it needs to be done in template-formula or simply adapted in needed formulas? I mean template-formula has a few useful pillar key/values but those are only 'indicative' and could/should be adapted in every formula.

It should be done here template formula in my opinion - i.e. change pkg to dict.

[baby-gnu]

@daks OK. Does it needs to be done in template-formula or simply adapted in needed formulas?

We want to make template-formula the “go-to resource for all formula development” (#21), so this should be done here.

Regards.

[noelmcloughlin]

We want to make template-formula the “go-to resource for all formula development” (#21), so this should be done here.

Thanks @baby-gnu @daks this was the item related to this PR.

Adapt template-formula to not be too focussed on pkg:
Must also consider archives, tarball releases, GitHub repos, etc.

This PR is an enabler only.

[noelmcloughlin]

Here are derived implementations based on this pkg dict ...
saltstack-formulas/grafana-formula#3
saltstack-formulas/prometheus-formula#4

[noelmcloughlin]

Hi @baby-gnu
I agree with your analysis here and will include in upcoming PR.

Thanks for reviewing!!

[myii]

Related issue: #135.

[baby-gnu]

I'm OK with this pull request.

The failed build is not related to the changes here.

[noelmcloughlin]

Thanks @baby-gnu @daks for the review - appreciated.

[saltstack-formulas-travis]

🎉 This PR is included in version 3.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀