[master] Allow salt-call arguments --file-root, --pillar-root and --states-dir to be specified multiple times
What does this PR do?
What issues does this PR fix or reference?
Fixes: #64486
Previous Behavior
The salt-call arguments --file-root, --pillar-root and --states-dir can each be specified only once.
New Behavior
The salt-call arguments --file-root, --pillar-root and --states-dir can each be specified multiple times.
Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
- [X] Docs
- [X] Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
- [X] Tests written/updated
Commits signed with GPG?
Yes
Please review Salt's Contributing Guide for best practices.
See GitHub's page on GPG signing for more information about signing commits with GPG.
Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey. Please be sure to review our Code of Conduct. Also, check out some of our community resources including:
- Community Wiki
- Salt’s Contributor Guide
- Join our Community Slack
- IRC on LiberaChat
- Salt Project YouTube channel
- Salt Project Twitch channel
There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. If you have additional questions, email us at [email protected]. We’re glad you’ve joined our community and look forward to doing awesome things with you!
Hi @MKLeb, can you have a look at this MR? This looks like a great addition to make salt-call more flexible.
Hi @martijnkruiten , I see you have a test for multiple file roots (I commented on that one). We would really like to see full test coverage for this PR, so are you able to add a similar style of test for multiple pillar roots and state directories?
Hi @MKLeb, I'll have a look. I seem to recall that I didn't add them because these options weren't included in the test suite to begin with.
I've added a test for the pillar roots that merges a dict that's defined in two different directories and then asserts that all the keys are present. What I'm stuck at, however, is writing a test for the states dirs. It seems to be sensitive to the environment that the file exists in (base vs prod), so I have to think of a different way to test this functionality.
I think I'm going to need some help writing a test for --states-dir, as I'm not sure how it should be used exactly and there's not a lot of documentation available (it's not even mentioned in salt-call.rst). I thought that I could just point it to a random directory with sls files in it and that it would be able to run the states defined in those files, but it seems to be a bit more complicated than that.
Alternatively, I'm also fine with reverting the chances to that argument and going forward with the --file-root and --pillar-root options only. I'm still convinced that the change should work for --states-dir the same as it does for the other options, but I lack the knowledge to prove it in a test.
@twangboy I've made all the requested changes. Please review and let me know if further adjustments are needed.
@twangboy I've made sure that the pre-commit checks all pass.
@twangboy is there anything else I can do to make this go through?
Just need to get the tests to pass
Just need to get the tests to pass
Is there anything that someone could do to help? I am interested in this PR.
Once the nightlies for master branch are passing, then we can rebase and see where we're at on the tests.