1============
2Contributing
3============
4
5
6So you want to contribute to the Salt project? Excellent! You can help
7in a number of ways:
8
9-  Using Salt and opening well-written bug reports.
10-  Joining a `working group <https://github.com/saltstack/community>`__.
11-  Answering questions on `irc <https://web.libera.chat/#salt>`__,
12   the `community Slack <https://join.slack.com/t/saltstackcommunity/shared_invite/zt-3av8jjyf-oBQ2M0vhXOhJpNpRkPWBvg>`__,
13   the `salt-users mailing
14   list <https://groups.google.com/forum/#!forum/salt-users>`__,
15   `Server Fault <https://serverfault.com/questions/tagged/saltstack>`__,
16   or `r/saltstack on Reddit <https://www.reddit.com/r/saltstack/>`__.
17-  Fixing bugs.
18-  `Enhancing the documentation <https://docs.saltproject.io/en/master/topics/development/conventions/documentation.html#salt-docs>`__.
19- Providing workarounds, patches, or other code without tests.
20- Telling other people about problems you solved using Salt.
21
22If you’d like to update docs or fix an issue, you’re going to need the
23Salt repo. The best way to contribute is using
24`Git <https://git-scm.com/>`__.
25
26Environment Setup
27-----------------
28
29To hack on Salt or the docs you’re going to need to set up your
30development environment. If you already have a workflow that you’re
31comfortable with, you can use that, but otherwise this is an opinionated
32guide for setting up your dev environment. Follow these steps and you’ll
33end out with a functioning dev environment and be able to submit your
34first PR.
35
36This guide assumes at least a passing familiarity with
37`Git <https://git-scm.com/>`__, a common version control tool used
38across many open source projects, and is necessary for contributing to
39Salt. For an introduction to Git, watch `Salt Docs Clinic - Git For the
40True
41Beginner <https://www.youtube.com/watch?v=zJw6KNvmuq4&ab_channel=SaltStack>`__.
42Because of its widespread use, there are many resources for learning
43more about Git. One popular resource is the free online book `Learn Git
44in a Month of
45Lunches <https://www.manning.com/books/learn-git-in-a-month-of-lunches>`__.
46
47pyenv, Virtual Environments, and You
48----------------------------------------
49
50We recommend `pyenv <https://github.com/pyenv/pyenv>`__, since it allows
51installing multiple different Python versions, which is important for
52testing Salt across all the versions of Python that we support.
53
54On Linux
55~~~~~~~~
56
57Install pyenv:
58
59::
60
61   git clone https://github.com/pyenv/pyenv.git ~/.pyenv
62   export PATH="$HOME/.pyenv/bin:$PATH"
63   git clone https://github.com/pyenv/pyenv-virtualenv.git $(pyenv root)/plugins/pyenv-virtualenv
64
65On Mac
66~~~~~~
67
68Install pyenv using brew:
69
70::
71
72   brew update
73   brew install pyenv
74   brew install pyenv-virtualenv
75
76--------------
77
78Now add pyenv to your ``.bashrc``:
79
80::
81
82   echo 'export PATH="$HOME/.pyenv/bin:$PATH"' >> ~/.bashrc
83   pyenv init 2>> ~/.bashrc
84   pyenv virtualenv-init 2>> ~/.bashrc
85
86For other shells, see `the pyenv
87instructions <https://github.com/pyenv/pyenv#basic-github-checkout>`__.
88
89Go ahead and restart your shell. Now you should be able to install a new
90version of Python:
91
92::
93
94   pyenv install 3.7.0
95
96If that fails, don’t panic! You’re probably just missing some build
97dependencies. Check out `pyenv common build
98problems <https://github.com/pyenv/pyenv/wiki/Common-build-problems>`__.
99
100Now that you’ve got your version of Python installed, you can create a
101new virtual environment with this command:
102
103::
104
105   pyenv virtualenv 3.7.0 salt
106
107Then activate it:
108
109::
110
111   pyenv activate salt
112
113Sweet! Now you’re ready to clone Salt so you can start hacking away! If
114you get stuck at any point, check out the resources at the beginning of
115this guide. IRC and Slack are particularly helpful places to go.
116
117Get The Source!
118~~~~~~~~~~~~~~~
119
120Salt uses the fork and clone workflow for Git contributions. See `Using
121the Fork-and-Branch Git
122Workflow <https://blog.scottlowe.org/2015/01/27/using-fork-branch-git-workflow/>`__
123for how to implement it. But if you just want to hurry and get started
124you can go ahead and follow these steps:
125
126Clones are so shallow. Well, this one is anyway:
127
128::
129
130   git clone --depth=1 --origin salt https://github.com/saltstack/salt.git
131
132This creates a shallow clone of Salt, which should be fast. Most of the
133time that’s all you’ll need, and you can start building out other
134commits as you go. If you *really* want all 108,300+ commits you can
135just run ``git fetch --unshallow``. Then go make a sandwich because it’s
136gonna be a while.
137
138You’re also going to want to head over to GitHub and create your own
139`fork of Salt <https://github.com/saltstack/salt/fork>`__. Once you’ve
140got that set up you can add it as a remote:
141
142::
143
144   git remote add yourname <YOUR SALT REMOTE>
145
146If you use your name to refer to your fork, and ``salt`` to refer to the
147official Salt repo you’ll never get ``upstream`` or ``origin`` confused.
148
149.. note::
150
151   Each time you start work on a new issue you should fetch the most recent
152   changes from ``salt/upstream``.
153
154``pre-commit`` and ``nox`` Setup
155~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
156
157Here at Salt we use `pre-commit <https://pre-commit.com/>`__ and
158`nox <https://nox.thea.codes/en/stable/>`__ to make it easier for
159contributors to get quick feedback, for quality control, and to increase
160the chance that your merge request will get reviewed and merged. Nox
161enables us to run multiple different test configurations, as well as
162other common tasks. You can think of it as Make with superpowers.
163Pre-commit does what it sounds like: it configures some Git pre-commit
164hooks to run ``black`` for formatting, ``isort`` for keeping our imports
165sorted, and ``pylint`` to catch issues like unused imports, among
166others. You can easily install them in your virtualenv with:
167
168::
169
170   python -m pip install pre-commit nox
171   pre-commit install
172
173.. warning::
174    Currently there is an issue with the pip-tools-compile pre-commit hook on Windows.
175    The details around this issue are included here:
176    https://github.com/saltstack/salt/issues/56642.
177    Please ensure you export ``SKIP=pip-tools-compile`` to skip pip-tools-compile.
178
179Now before each commit, it will ensure that your code at least *looks*
180right before you open a pull request. And with that step, it’s time to
181start hacking on Salt!
182
183.. _imagemagick-setup:
184
185``imagemagick`` Setup
186~~~~~~~~~~~~~~~~~~~~~
187
188One last prerequisite is to have ``imagemagick`` installed, as it is required
189by Sphinx for generating the HTML documentation.
190
191::
192
193   # On Mac, via homebrew
194   brew install imagemagick
195
196::
197
198   # Example Linux installation: Debian-based
199   sudo apt install imagemagick
200
201
202Salt Issues
203-----------
204
205Create Your Own
206~~~~~~~~~~~~~~~
207
208Perhaps you’ve come to this guide because you found a problem in Salt,
209and you’ve diagnosed the cause. Maybe you need some help figuring out
210the problem. In any case, creating quality bug reports is a great way to
211contribute to Salt even if you lack the skills, time, or inclination to
212fix it yourself. If that’s the case, head on over to `Salt’s issue
213tracker on
214GitHub <https://github.com/saltstack/salt/issues/new/choose>`__.
215
216Creating a **good** report can take a little bit of time - but every
217minute you invest in making it easier for others to reproduce and
218understand your issue is time well spent. The faster someone can
219understand your issue, the faster it will be able to get fixed
220correctly.
221
222The thing that every issue needs goes by many names, but one at least as
223good as any other is MCVE - **M**\ inimum **C**\ omplete
224**V**\ erifiable **E**\ xample.
225
226In a nutshell:
227
228-  **Minimum**: All of the **extra** information has been removed. Will
229   2 or 3 lines of master/minion config still exhibit the behavior?
230-  **Complete**: Minimum also means complete. If your example is missing
231   information, then it’s not complete. Salt, Python, and OS versions
232   are all bits of information that make your example complete. Have you
233   provided the commands that you ran?
234-  **Verifiable**: Can someone take your report and reproduce it?
235
236Slow is smooth, and smooth is fast - it may feel like you’re taking a
237long time to create your issue if you’re creating a proper MCVE, but a
238MCVE eliminates back and forth required to reproduce/verify the issue so
239someone can actually create a fix.
240
241Pick An Issue
242~~~~~~~~~~~~~
243
244If you don’t already have an issue in mind, you can search for `help
245wanted <https://github.com/saltstack/salt/issues?q=is%3Aissue+is%3Aopen+label%3A%22help+wanted%22>`__
246issues. If you also search for `good first
247issue <https://github.com/saltstack/salt/issues?q=is%3Aissue+is%3Aopen+label%3A%22help+wanted%22+label%3A%22good+first+issue%22>`__
248then you should be able to find some issues that are good for getting
249started contributing to Salt. `Documentation
250issues <https://github.com/saltstack/salt/issues?q=is%3Aissue+is%3Aopen+label%3Adocumentation+>`__
251are also good starter issues. When you find an issue that catches your
252eye (or one of your own), it’s a good idea to comment on the issue and
253mention that you’re working on it. Good communication is key to
254collaboration - so if you don’t have time to complete work on the issue,
255just leaving some information about when you expect to pick things up
256again is a great idea!
257
258Hacking Away
259------------
260
261Salt, Tests, Documentation, and You
262~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
263
264Before approving code contributions, Salt requires:
265
266-  documentation
267-  meaningful passing tests
268-  correct code
269
270Documentation fixes just require correct documentation.
271
272What If I Don’t Write Tests or Docs?
273^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
274
275If you aren’t into writing documentation or tests, we still welcome your
276contributions! But your PR will be labeled ``Needs Testcase`` and
277``Help Wanted`` until someone can get to write the tests/documentation.
278Of course, if you have a desire but just lack the skill we are more than
279happy to collaborate and help out! There’s the `documentation working
280group <https://github.com/saltstack/docs-hub>`__ and the `testing
281working
282group <https://github.com/saltstack/community/tree/master/working_groups/wg-Testing>`__.
283We also regularly stream our test clinic `live on
284Twitch <https://www.twitch.tv/saltprojectoss>`__ every Tuesday afternoon
285and Thursday morning, Central Time. If you’d like specific help with
286tests, bring them to the clinic. If no community members need help, you
287can also just watch tests written in real time.
288
289.. _docs-building:
290
291Documentation
292~~~~~~~~~~~~~
293
294Salt uses both docstrings, as well as normal reStructuredText files in
295the ``salt/doc`` folder for documentation. Sphinx is used to generate the
296documentation, and does require :ref:`setting up imagemagick on your OS.<imagemagick-setup>`
297Since we use ``nox``, you can build your docs and view them in your browser
298with this one-liner:
299
300::
301
302   python -m nox -e 'docs-html(compress=False, clean=False)'; cd doc/_build/html; python -m webbrowser http://localhost:8000/contents.html; python -m http.server
303
304The first time this will take a while because there are a *lot* of
305modules. Maybe you should go grab some dessert if you already finished
306that sandwich. But once Sphinx is done building the docs, python should
307launch your default browser with the URL
308http://localhost:8000/contents.html. Now you can navigate to your docs
309and ensure your changes exist. If you make changes, you can simply run
310this:
311
312::
313
314   cd -; python -m nox -e 'docs-html(compress=False, clean=False)'; cd doc/_build/html; python -m http.server
315
316And then refresh your browser to get your updated docs. This one should
317be quite a bit faster since Sphinx won’t need to rebuild everything.
318
319If your change is a docs-only change, you can go ahead and commit/push
320your code and open a PR. You can indicate that it’s a docs-only change by
321adding ``[Documentation]`` to the title of your PR. Otherwise you’ll
322want to write some tests and code.
323
324Running Development Salt
325~~~~~~~~~~~~~~~~~~~~~~~~
326
327Note: If you run into any issues in this section, check the
328Troubleshooting section.
329
330If you’re going to hack on the Salt codebase you’re going to want to be
331able to run Salt locally. The first thing you need to do is install Salt
332as an editable pip install:
333
334::
335
336   python -m pip install -e .
337
338This will let you make changes to Salt without having to re-install it.
339
340After all of the dependencies and Salt are installed, it’s time to set
341up the config for development. Typically Salt runs as ``root``, but you
342can specify which user to run as. To configure that, just copy the
343master and minion configs. We have .gitignore setup to ignore the
344``local/`` directory, so we can put all of our personal files there.
345
346::
347
348   mkdir -p local/etc/salt/
349
350Create a master config file as ``local/etc/salt/master``:
351
352::
353
354   cat <<EOF >local/etc/salt/master
355   user: $(whoami)
356   root_dir: $PWD/local/
357   publish_port: 55505
358   ret_port: 55506
359   EOF
360
361And a minion config as ``local/etc/salt/minion``:
362
363::
364
365   cat <<EOF >local/etc/salt/minion
366   user: $(whoami)
367   root_dir: $PWD/local/
368   master: localhost
369   id: saltdev
370   master_port: 55506
371   EOF
372
373Now you can start your Salt master and minion, specifying the config
374dir.
375
376::
377
378   salt-master --config-dir=local/etc/salt/ --log-level=debug --daemon
379   salt-minion --config-dir=local/etc/salt/ --log-level=debug --daemon
380
381Now you should be able to accept the minion key:
382
383::
384
385   salt-key -c local/etc/salt -Ay
386
387And check that your master/minion are communicating:
388
389::
390
391   salt -c local/etc/salt \* test.version
392
393Rather than running ``test.version`` from your master, you can run it
394from the minion instead:
395
396::
397
398   salt-call -c local/etc/salt test.version
399
400Note that you’re running ``salt-call`` instead of ``salt``, and you’re
401not specifying the minion (``\*``), but if you’re running the dev
402version then you still will need to pass in the config dir. Now that
403you’ve got Salt running, you can hack away on the Salt codebase!
404
405If you need to restart Salt for some reason, if you’ve made changes and
406they don’t appear to be reflected, this is one option:
407
408::
409
410   kill -INT $(pgrep salt-master)
411   kill -INT $(pgrep salt-minion)
412
413If you’d rather not use ``kill``, you can have a couple of terminals
414open with your salt virtualenv activated and omit the ``--daemon``
415argument. Salt will run in the foreground, so you can just use ctrl+c to
416quit.
417
418Test First? Test Last? Test Meaningfully!
419~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
420
421You can write tests first or tests last, as long as your tests are
422meaningful and complete! *Typically* the best tests for Salt are going
423to be unit tests. Testing is `a whole topic on its
424own <https://docs.saltproject.io/en/master/topics/tutorials/writing_tests.html>`__,
425But you may also want to write functional or integration tests. You’ll
426find those in the ``salt/tests`` directory.
427
428When you’re thinking about tests to write, the most important thing to
429keep in mind is, “What, exactly, am I testing?” When a test fails, you
430should know:
431
432-  What, specifically, failed?
433-  Why did it fail?
434-  As much as possible, what do I need to do to fix this failure?
435
436If you can’t answer those questions then you might need to refactor your
437tests.
438
439When you’re running tests locally, you should make sure that if you
440remove your code changes your tests are failing. If your tests *aren’t*
441failing when you haven’t yet made changes, then it’s possible that
442you’re testing the wrong thing.
443
444But whether you adhere to TDD/BDD, or you write your code first and your
445tests last, ensure that your tests are meaningful.
446
447Running Tests
448^^^^^^^^^^^^^
449
450As previously mentioned, we use ``nox``, and that’s how we run our
451tests. You should have it installed by this point but if not you can
452install it with this:
453
454::
455
456   python -m pip install nox
457
458Now you can run your tests:
459
460::
461
462   python -m nox -e "pytest-3.7(coverage=False)" -- tests/unit/cli/test_batch.py
463
464It’s a good idea to install
465`espeak <https://github.com/espeak-ng/espeak-ng>`__ or use ``say`` on
466Mac if you’re running some long-running tests. You can do something like
467this:
468
469::
470
471   python -m nox -e "pytest-3.7(coverage=False)" -- tests/unit/cli/test_batch.py; espeak "Tests done, woohoo!"
472
473That way you don’t have to keep monitoring the actual test run.
474
475Changelog and Commit!
476~~~~~~~~~~~~~~~~~~~~~
477
478When you write your commit message you should use imperative style. Do
479this:
480
481   Add frobnosticate capability
482
483Don’t do this:
484
485   Added frobnosticate capability
486
487But that advice is backwards for the changelog. We follow the
488`keepachangelog <https://keepachangelog.com/en/1.0.0/>`__ approach for
489our changelog, and use towncrier to generate it for each release. As a
490contributor, all that means is that you need to add a file to the
491``salt/changelog`` directory, using the ``<issue #>.<type>`` format. For
492instanch, if you fixed issue 123, you would do:
493
494::
495
496   echo "Made sys.doc inform when no minions return" > changelog/123.fixed
497
498And that’s all that would go into your file. When it comes to your
499commit message, it’s usually a good idea to add other information, such as
500
501- What does a reviewer need to know about the change that you made?
502- If someone isn’t an expert in this area, what will they need to know?
503
504This will also help you out, because when you go to create the PR it
505will automatically insert the body of your commit messages.
506
507PR Time!
508--------
509
510Once you’ve done all your dev work and tested locally, you should check
511out our `PR
512guidelines <https://docs.saltproject.io/en/master/topics/development/pull_requests.html>`__.
513After you read that page, it’s time to `open a new
514PR <https://github.com/saltstack/salt/compare>`__. Fill out the PR
515template - you should have updated or created any necessary docs, and
516written tests if you’re providing a code change. When you submit your
517PR, we have a suite of tests that will run across different platforms to
518help ensure that no known bugs were introduced.
519
520Now What?
521~~~~~~~~~
522
523You’ve made your changes, added documentation, opened your PR, and have
524passing tests… now what? When can you expect your code to be merged?
525
526When you open your PR, a reviewer will get automatically assigned. If
527your PR is submitted during the week you should be able to expect some
528kind of communication within that business day. If your tests are
529passing and we’re not in a code freeze, ideally your code will be merged
530that day. If you haven’t heard from your assigned reviewer, ping them on
531GitHub, `irc <https://web.libera.chat/#salt>`__, or Community Slack.
532
533It’s likely that your reviewer will leave some comments that need
534addressing - it may be a style change, or you forgot a changelog entry,
535or need to update the docs. Maybe it’s something more fundamental -
536perhaps you encountered the rare case where your PR has a much larger
537scope than initially assumed.
538
539Whatever the case, simply make the requested changes (or discuss why the
540requests are incorrect), and push up your new commits. If your PR is
541open for a significant period of time it may be worth rebasing your
542changes on the most recent changes to Salt. If you need help, the
543previously linked Git resources will be valuable.
544
545But if, for whatever reason, you’re not interested in driving your PR to
546completion then just note that in your PR. Something like, “I’m not
547interested in writing docs/tests, I just wanted to provide this fix -
548someone else will need to complete this PR.” If you do that then we’ll
549add a “Help Wanted” label and someone will be able to pick up the PR,
550make the required changes, and it can eventually get merged in.
551
552In any case, now that you have a PR open, congrats! You’re a Salt
553developer! You rock!
554
555Troubleshooting
556---------------
557
558zmq.core.error.ZMQError
559~~~~~~~~~~~~~~~~~~~~~~~
560
561Once the minion starts, you may see an error like the following::
562
563::
564
565   zmq.core.error.ZMQError: ipc path "/path/to/your/virtualenv/var/run/salt/minion/minion_event_7824dcbcfd7a8f6755939af70b96249f_pub.ipc" is longer than 107 characters (sizeof(sockaddr_un.sun_path)).
566
567This means that the path to the socket the minion is using is too long.
568This is a system limitation, so the only workaround is to reduce the
569length of this path. This can be done in a couple different ways:
570
5711. Create your virtualenv in a path that is short enough.
5722. Edit the :conf_minion:``sock_dir`` minion config variable and reduce
573   its length. Remember that this path is relative to the value you set
574   in :conf_minion:``root_dir``.
575
576NOTE: The socket path is limited to 107 characters on Solaris and Linux,
577and 103 characters on BSD-based systems.
578
579No permissions to access …
580~~~~~~~~~~~~~~~~~~~~~~~~~~
581
582If you forget to pass your config path to any of the ``salt*`` commands,
583you might see
584
585::
586
587   No permissions to access "/var/log/salt/master", are you running as the
588   correct user?
589
590Just pass ``-c local/etc/salt`` (or whatever you named it)
591
592File descriptor limit
593~~~~~~~~~~~~~~~~~~~~~
594
595You might need to raise your file descriptor limit. You can check it
596with:
597
598::
599
600   ulimit -n
601
602If the value is less than 3072, you should increase it with:
603
604::
605
606   ulimit -n 3072
607   # For c-shell:
608   limit descriptors 3072
609
610Pygit2 or other dependency install fails
611~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
612
613You may see some failure messages when installing requirements. You can
614directly access your nox environment and possibly install pygit (or
615other dependency) that way. When you run nox, you’ll see a message like
616this:
617
618::
619
620   nox > Re-using existing virtual environment at .nox/pytest-parametrized-3-crypto-none-transport-zeromq-coverage-false.
621
622For this, you would be able to install with:
623
624::
625
626   .nox/pytest-parametrized-3-crypto-none-transport-zeromq-coverage-false/bin/python -m pip install pygit2
627