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

fix(package.install): proper package for CentOS and Rocky Linux 8 #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

emansom
Copy link

@emansom emansom commented May 20, 2022

PR progress checklist (to be filled in by reviewers)

  • Changes to documentation are appropriate (or tick if not required)
  • Changes to tests are appropriate (or tick if not required)
  • Reviews completed

What type of PR is this?

Primary type

  • [build] Changes related to the build system
  • [chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
  • [ci] Changes to the continuous integration configuration
  • [feat] A new feature
  • [fix] A bug fix
  • [perf] A code change that improves performance
  • [refactor] A code change that neither fixes a bug nor adds a feature
  • [revert] A change used to revert a previous commit
  • [style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Secondary type

  • [docs] Documentation changes
  • [test] Adding missing or correcting existing tests

Does this PR introduce a BREAKING CHANGE?

No.

Related issues and/or pull requests

#3

Describe the changes you're proposing

Append CentOS and Rocky Linux 8 package information to the osfingermap.yaml file and implement logic to optionally setup repos as dependencies of packages in package/install.sls.

Pillar / config required to test the proposed changes

/srv/pillar/wireguard.sls

wireguard:
  interfaces:
    vpn0:
      Interface:
        Address: REDACTED
        SaveConfig: true
        ListenPort: REDACTED
        PrivateKey: REDACTED
      Peers:
        REDACTED:
          PublicKey: REDACTED
          AllowedIPs: REDACTED
          Endpoint: REDACTED
        REDACTED:
          PublicKey: REDACTED
          AllowedIPs: REDACTED
          Endpoint: REDACTED

Debug log showing how the proposed changes work

[root@REDACTED ~]# salt-call --local --state-output=terse state.apply wireguard
[WARNING ] The function "module.run" is using its deprecated version and will expire in version "Phosphorus".
local:
  Name: epel - Function: pkgrepo.managed - Result: Clean Started: - 17:43:44.573065 Duration: 793.563 ms
  Name: elrepo - Function: pkgrepo.managed - Result: Clean Started: - 17:43:45.366773 Duration: 24.168 ms
  Name: wireguard-tools - Function: pkg.installed - Result: Clean Started: - 17:43:45.404320 Duration: 635.334 ms
  Name: kmod-wireguard - Function: pkg.installed - Result: Clean Started: - 17:43:46.040167 Duration: 19.086 ms
  Name: /etc/wireguard - Function: file.directory - Result: Clean Started: - 17:43:46.061422 Duration: 1.621 ms
  Name: mine.update - Function: module.run - Result: Changed Started: - 17:43:46.063564 Duration: 1.5 ms
  Name: /etc/wireguard/vpn0.conf - Function: file.managed - Result: Changed Started: - 17:43:46.065432 Duration: 92.476 ms
  Name: wg-quick@vpn0 - Function: service.running - Result: Changed Started: - 17:43:46.190440 Duration: 172.545 ms

Summary for local
------------
Succeeded: 8 (changed=3)
Failed:    0
------------
Total states run:     8
Total run time:   1.740 s
[root@REDACTED ~]# 

Documentation checklist

  • Updated the README (e.g. Available states).
  • Updated pillar.example.

Testing checklist

  • Included in Kitchen (i.e. under state_top).
  • Covered by new/existing tests (e.g. InSpec, Serverspec, etc.).
  • Updated the relevant test pillar.

Additional context

Updated .pre-commit-config.yaml as well from latest template-formula.
Resolved rst-lint "title underline too short" warnings in docs/README.rst.

fixes saltstack-formulas#3

Squashed commit of the following:

commit f07bf84
Author: Ewout van Mansom <[email protected]>
Date:   Fri May 20 19:06:29 2022 +0200

    fix(osfingermap): support CentOS 8

commit 7220f67
Author: Ewout van Mansom <[email protected]>
Date:   Fri May 20 18:56:59 2022 +0200

    fix(osfingermap): set service name to wg-quick for Rocky Linux 8

commit aeb7ac7
Author: Ewout van Mansom <[email protected]>
Date:   Fri May 20 18:48:57 2022 +0200

    fix(package.install): set name to pkgrepo state call

commit 311851a
Author: Ewout van Mansom <[email protected]>
Date:   Fri May 20 18:44:47 2022 +0200

    fix(package.install): indent wireguard-package-install-pkg-{{ pkg_name }} correctly

commit 2793a10
Author: Ewout van Mansom <[email protected]>
Date:   Fri May 20 18:41:38 2022 +0200

    fix(package.install): resolve conflicting state ID

commit 59d9982
Author: Ewout van Mansom <[email protected]>
Date:   Fri May 20 18:40:20 2022 +0200

    fix(package.install): iterate over all pkgs instead

commit 773e53a
Author: Ewout van Mansom <[email protected]>
Date:   Fri May 20 18:37:18 2022 +0200

    fix(package.install): support CentOS and Rocky Linux 8

commit 6e0d328
Author: Ewout van Mansom <[email protected]>
Date:   Fri May 20 18:36:19 2022 +0200

    chore(precommit): update
@emansom emansom force-pushed the pr-centos8-pkgs branch from f13db25 to 4959e4e Compare May 21, 2022 07:33
@myii
Copy link
Member

myii commented May 22, 2022

Thanks for your recent contributions to this formula, @emansom.

I'd like to approach this situation a little differently. The CI has never been activated on this formula until after you sent this PR through -- it prompted me to get it set up. I've also spent some time getting the CI fixed for all Linux distros. I'd like to push through those changes first and then discuss rebasing this PR. The difference is that you're proposing the use of pkgrepo.managed; I'd like to have that as an alternative option (and a different implementation), for various reasons that we can discuss when rebasing.

I'll let you know when the CI is up and running properly.

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

Successfully merging this pull request may close these issues.

2 participants