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

Fix custom minion_config upload #6473

Conversation

aneeshusa
Copy link
Contributor

If a custom minion_config is specified, upload it during
the upload_configs step, then use sudo to copy into the config_dir
instead of uploading it again.

If a custom minion_config is specified, upload it during
the upload_configs step, then use sudo to copy into the config_dir
instead of uploading it again.
@aneeshusa
Copy link
Contributor Author

Fixes #5973. Unlike #6073, uses sudo to always get around permissions errors, while retaining the ability to use a different config_dir. IMO, it is better for the default to be to respect the default configuration location - /etc/salt - and use sudo to put the configuration there, instead of to be to upload the configuration to a temporary directory and rely on salt-bootstrap to copy the minion configuration into /etc/salt. I use a custom bootstrap script to handle installation for Ubuntu and OS X for builds on Travis, and I would rather not have to have my bootstrap script copy this salt-bootstrap script behavior when it's easier to just put the file in the needed place..

However, I would not be opposed to removing the ability to use a different config_dir as in #6073.

@mitchellh
Copy link
Contributor

Fixed using #6073 Thanks!

@mitchellh mitchellh closed this Nov 18, 2015
aneeshusa added a commit to aneeshusa/servo-saltfs that referenced this pull request Dec 22, 2015
The original Vagrantfile was meant to be used with Vagrant 1.7.4, but
required 2 patches to Vagrant for proper functionality, which I had
submitted as pull requests. Vagrant has since released a new version,
1.8.0, which has updates the Salt provisioner. PR status:
 - [One PR](hashicorp/vagrant#6474) was merged
 - [Another PR](hashicorp/vagrant#6473) was not

The second PR was rejected "in favor" of [a third
PR](hashicorp/vagrant#6073) which simply removed
the config_dir option entirely instead of using sudo to be able to write
in a priveleged directory. [An unrelated
PR](https://github.com/mitchellh/vagrant/pull/6382/files) removed a
second option we had been using: install_command.

These changes in Vagrant were due to incompabilities with the official
Salt bootstrap script, so this commit brings the install_salt script's
behavior closer to that of the official Salt bootstrap script as well.
Namely, it can now copy over configuration files as part of
installation, using the same flags as the official script.

The configure_salt script is now used just to setup the salt and pillar
roots on Travis (this is accomplished via shared folders in Vagrant),
hence the renaming.

With this commit and the new Vagrant version, the included Vagrantfile
now works with a vanilla Vagrant 1.8.0 install, no patching needed.
bors-servo pushed a commit to servo/saltfs that referenced this pull request Feb 9, 2016
…r=larsbergstrom

Make Salt install/configure Vagrant 1.8 compatible

The original Vagrantfile was meant to be used with Vagrant 1.7.4, but
required 2 patches to Vagrant for proper functionality, which I had
submitted as pull requests. Vagrant has since released a new version,
1.8.0, which has updates the Salt provisioner. PR status:
 - [One PR](hashicorp/vagrant#6474) was merged
 - [Another PR](hashicorp/vagrant#6473) was not

The second PR was rejected "in favor" of [a third PR](hashicorp/vagrant#6073) which simply
removed the config_dir option entirely instead of using sudo to be able
to write in a privileged directory. [An unrelated PR](https://github.com/mitchellh/vagrant/pull/6382/files) removed a second
option we had been using: install_command.

These changes in Vagrant were due to incompatibilities with the official
Salt bootstrap script, so this commit brings the install_salt script's
behavior closer to that of the official Salt bootstrap script as well.
Namely, it can now copy over configuration files as part of
installation, using the same flags as the official script.

The configure_salt script is now used just to setup the salt and pillar
roots on Travis (this is accomplished via shared folders in Vagrant),
hence the renaming.

With this commit and the new Vagrant version, the included Vagrantfile
now works with a vanilla Vagrant 1.8.0 install, no patching needed.

Note to self: update the wiki when this is merged!

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/180)
<!-- Reviewable:end -->
@ghost ghost locked and limited conversation to collaborators Apr 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants