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

ssh config roster for salt-ssh #42103

Merged
merged 2 commits into from Sep 12, 2017

Conversation

Projects
None yet
6 participants
@davidjoliver86
Copy link
Contributor

commented Jul 3, 2017

Addresses #35727

What does this PR do?

Adds a roster module that leverages an SSH config file, using its Host xxx directives as roster targets, and allowing salt-ssh to leverage the connection parameters.

Use case is primarily targeted toward running shell commands on hosts that match the pattern.

What issues does this PR fix or reference?

#35727

New Behavior

Adds sshconfig roster.

Tests written?

Yes

@salt-jenkins

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2017

@davidjoliver86, thanks for your PR! By analyzing the history of the files in this pull request, we identified @thatch45, @s0undt3ch and @cachedout to be potential reviewers.

@gtmanfred gtmanfred added the Awesome label Jul 4, 2017

@gtmanfred

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2017

This looks awesome!

Would you mind taking a look at the 2 pylint errors?

https://jenkins.saltstack.com/job/PR/job/salt-pr-lint-n/12528/violations/file/tests/unit/test_ssh_config_roster.py/

Thanks,
Daniel

@davidjoliver86 davidjoliver86 force-pushed the davidjoliver86:ssh-config-roster branch 2 times, most recently from 05bd61a to 4233603 Jul 5, 2017

@davidjoliver86

This comment has been minimized.

Copy link
Contributor Author

commented Jul 5, 2017

@gtmanfred Addressed pylint issues. I did a fresh rebase off upstream/develop and I feel like it might've been confusing some of the builds below...

@davidjoliver86 davidjoliver86 force-pushed the davidjoliver86:ssh-config-roster branch from 4233603 to 0937d5f Jul 6, 2017

@vutny

This comment has been minimized.

Copy link
Contributor

commented Jul 13, 2017

This is so cool feature, @davidjoliver86

@@ -136,6 +136,10 @@
if SPM_REACTOR_PATH is None:
SPM_REACTOR_PATH = os.path.join(SRV_ROOT_DIR, 'spm', 'reactor')

HOME_DIR = __generated_syspaths.SPM_REACTOR_PATH

This comment has been minimized.

Copy link
@cachedout

cachedout Jul 25, 2017

Collaborator

I'm unclear on why SPM_REACTOR_PATH was chosen here. Could you please clarify?

This comment has been minimized.

Copy link
@davidjoliver86

davidjoliver86 Aug 12, 2017

Author Contributor

Just a typo. 😯

@rallytime

This comment has been minimized.

Copy link
Contributor

commented Aug 11, 2017

@davidjoliver86 Did you get a chance to look at the comments above?

@davidjoliver86

This comment has been minimized.

Copy link
Contributor Author

commented Aug 12, 2017

@rallytime Gonna look over it now - been a little busy lately, my apologies.

@davidjoliver86 davidjoliver86 force-pushed the davidjoliver86:ssh-config-roster branch from dba2b68 to a4365cd Aug 12, 2017

@davidjoliver86 davidjoliver86 force-pushed the davidjoliver86:ssh-config-roster branch from a4365cd to dd58225 Aug 12, 2017

@cachedout
Copy link
Collaborator

left a comment

The only addition I'd like to request here is to add a description of this new feature to the release notes. They are here: https://github.com/saltstack/salt/blob/develop/doc/topics/releases/oxygen.rst

After that, I'm good with merging this. Thanks!

@cachedout

This comment has been minimized.

Copy link
Collaborator

commented Sep 12, 2017

I'll just go ahead and modify the release notes in a follow-up PR. Thanks, @davidjoliver86 !

@cachedout cachedout merged commit 87ffd3f into saltstack:develop Sep 12, 2017

6 of 8 checks passed

default Build finished.
Details
jenkins/PR/salt-pr-linode-cent7-py3 Pull Requests » Salt PR - Linode CentOS 7 - PY3 #989 — FAILURE
Details
jenkins/PR/salt-pr-clone Pull Requests » Salt PR - Clone #16292 — SUCCESS
Details
jenkins/PR/salt-pr-docs-n Pull Requests » Salt PR - Docs #9096 — SUCCESS
Details
jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests » Salt PR - Linode Ubuntu14.04 #13728 — SUCCESS
Details
jenkins/PR/salt-pr-linode-ubuntu16-py3 Pull Requests » Salt PR - Linode Ubuntu16.04 - PY3 #1050 — SUCCESS
Details
jenkins/PR/salt-pr-lint-n Pull Requests » Salt PR - Code Lint #13632 — SUCCESS
Details
jenkins/PR/salt-pr-rs-cent7-n Pull Requests » Salt PR - RS CentOS 7 #13781 — SUCCESS
Details

garethgreenaway added a commit to garethgreenaway/salt that referenced this pull request Sep 14, 2017

Following the change in saltstack#42103 if Salt is installed via setu…
…p.py then the generated _syspaths.py does not contain the HOME_DIR which results in a failure.

gitebra pushed a commit to gitebra/salt that referenced this pull request Sep 18, 2017

Merge remote-tracking branch 'upstream/develop' into develop
* upstream/develop: (53 commits)
  Adding back in the salt-spm-reactor-dir
  Adding home_dir to SaltDistribution.global_options.
  Following the change in saltstack#42103 if Salt is installed via setup.py then the generated _syspaths.py does not contain the HOME_DIR which results in a failure.
  Test fix: update mock util path
  Lint: Add missing import to utils.args.py
  Lint: Add empty line at end of file
  highstate output pylint fix
  corrected some PEP8 problems reported by jenkins
  highstate output minor cleanup and code duplication removal
  Move salt.utils.argspec_report to salt.utils.args.py
  Move salt.utils.arg_lookup to salt.utils.args.py
  Move salt.utils.xor to salt.utils.value
  Move salt.utils.human_to_bytes to salt.utils.stringutils
  Add /norestart to vcredist installer
  Fixing typo.
  Adding a small check to ensure we do not continue to populate kwargs with __pub_ items from the kwargs item.
  Fix /etc/hosts not being modified when hostname is changed
  Fix `unit.modules.test_hosts` for Windows
  Revert "Reduce fileclient.get_file latency by merging _file_find and _file_hash"
  forgot to mock the proper one
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.