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

PR 44366 #48182

Merged
merged 3 commits into from Jun 19, 2018

Conversation

Projects
None yet
4 participants
@ghost
Copy link

commented Jun 18, 2018

What does this PR do?

Allows Salt Cloud to operate within a separate directory tree without having to define the configuration directive log_file separately (on the command line or in the main cloud configuration file).

What issues does this PR fix or reference?

#44366

Previous Behavior

A user has to specify --log-file separately, even if root_dir is specified in the main cloud configuration file and -c is being used on the command line.

New Behavior

Running SaltCloud with non-default configuration file locations works as expected ie. the log file ends up under the directory hierarchy specified in 'root_dir' directive in the main salt cloud configuration file.

Tests written?

No

Commits signed with GPG?

No

@ghost ghost requested a review from saltstack/team-core as a code owner Jun 18, 2018

@salt-jenkins salt-jenkins requested a review from saltstack/team-suse Jun 18, 2018

@isbm

isbm approved these changes Jun 18, 2018

@@ -2787,7 +2787,7 @@ def cloud_config(path, env_var='SALT_CLOUD_CONFIG', defaults=None,
apply_sdb(opts)

# prepend root_dir
prepend_root_dirs = ['cachedir']
prepend_root_dirs = ['cachedir','log_file']
prepend_root_dir(opts, prepend_root_dirs)

This comment has been minimized.

Copy link
@isbm

isbm Jun 18, 2018

Contributor

@samilaine but can we maybe not to call this list "prepend_root_dirs", since it is path options? I understand that the variable was already there, but let's agree it is greatly misleading name. Maybe I would just do it so:

prepend_root_dir(opts, ['cachedir','log_file'])

So whoever wants to know how this is working, can lookup the function and see that this is just a path_options.

This comment has been minimized.

Copy link
@ghost

ghost Jun 18, 2018

Author

However, it turns out that there might be a reasonable explanation for that funny variable prepend_root_dirs (see line 2358 in salt/config/__init__.py) .

So, instead of getting rid of the funny variable, I opted for abusing it even more.

This comment has been minimized.

Copy link
@gtmanfred

gtmanfred Jun 18, 2018

Contributor

for the record, i am for leaving the variable, as it is how all the other config functions work, master_config, minion_config, etc.

This comment has been minimized.

Copy link
@isbm

isbm Jun 19, 2018

Contributor

@samilaine 😉 Well, OK. I approved this anyway, and that one was up to you. Thanks!

samilaine and others added some commits Jun 18, 2018

@rallytime rallytime merged commit 1d6ab95 into saltstack:develop Jun 19, 2018

6 of 10 checks passed

jenkins/PR/salt-pr-linode-ubuntu16-py3 Pull Requests » Salt PR - Linode Ubuntu16.04 - PY3 #10808 — ABORTED
Details
default Build finished.
Details
jenkins/PR/salt-pr-linode-cent7-py3 Pull Requests » Salt PR - Linode CentOS 7 - PY3 #5837 — FAILURE
Details
jenkins/PR/salt-pr-rs-cent7-n Pull Requests » Salt PR - RS CentOS 7 #19891 — FAILURE
Details
WIP ready for review
Details
codeclimate All good!
Details
jenkins/PR/salt-pr-clone Pull Requests » Salt PR - Clone #26040 — SUCCESS
Details
jenkins/PR/salt-pr-docs-n Pull Requests » Salt PR - Docs #18095 — SUCCESS
Details
jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests » Salt PR - Linode Ubuntu14.04 #23767 — SUCCESS
Details
jenkins/PR/salt-pr-lint-n Pull Requests » Salt PR - Code Lint #22730 — SUCCESS
Details

@ghost ghost deleted the fix-cloud-config-root-dir branch Jun 25, 2018

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.