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

MacOS compatibility #531

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

lkubb
Copy link

@lkubb lkubb commented Mar 23, 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

I did not open issues for both fixes since the cause is evident.

  1. remove-macpackage-salt uses file.absent, but is referenced by salt-minion onchanges_in requisite as cmd: remove-macpackage-salt
  2. There is no group root on MacOS:
----------
          ID: permissions-minion-config
    Function: file.managed
        Name: /private/etc/salt/minion
      Result: False
     Comment: Group root is not available
     Started: 09:04:11.330316
    Duration: 2.895 ms
  1. The unless requisites on two MacOS-specific states always evaluate to false, causing a reinstall on every run on MacOS. This is caused by an incorrect shell command.

Describe the changes you're proposing

  • Fix faulty onchanges_in requisite on salt-minion for remove-macpackage-salt.
  • Include MacOS in the list of OS where the root user's primary group is wheel (correction: staff, but wheel exists at least. correction^2: It is wheel, staff is the admin user's primary group).
  • Fix incorrect unless shell command on salt-minion and download-salt-minion on MacOS.

Pillar / config required to test the proposed changes

Debug log showing how the proposed changes work

Documentation checklist

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

Testing checklist

There is no test suite for MacOS.

  • 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

@lkubb lkubb requested a review from a team as a code owner March 23, 2022 12:26
@myii
Copy link
Member

myii commented Apr 14, 2022

Thanks for the PR, @lkubb. It would be great if we had MacOS testing running in the CI but we haven't got that far yet.

@noelmcloughlin Do you still have access to MacOS minions that you can test this on?

@lkubb
Copy link
Author

lkubb commented Oct 5, 2022

Any updates on this PR? :) Disregarding the missing root group, the issues described here can be retraced by checking the code, which cannot work as intended in its current form.

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.

None yet

2 participants