-
Notifications
You must be signed in to change notification settings - Fork 189
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
Add ability to append to default Vagrantfile #97
Comments
It would be nice to re-add vagrant-cachier or some other things to this through this method. |
+1 |
@fnichol Does this sound like a feature that you would be willing to merge in to master? |
I would appreciate this feature for reasons stated in #100. |
@colindean @nelsonjchen same issue here - also wanted to add vagrant-cachier. Got it successfully added via the global I played around with creating my own I'm 👍 for the possibility to append to the default Vagrantfile rather than copy / pasting the whole file (and all the future problems that this approach brings with) |
👍 |
For those interested, I have started working on a solution which I hope strikes a balance between being simple and extensible. I'll push the branch up soon. |
@byggztryng cool stuff, let us know when there is something we can test |
@fnichol @sethvargo is there any kind of vetted way that I could test my PR? |
#111 is what I'm thinking. More robust symbol detection and replacement logic is needed, though, because Test Kitchen uses SafeYaml to load .kitchen.yml with the option to not deserialize any symbols. |
I did some research on this today. Could we leverage Vagrantfile merging and load order instead of appending? It seems like a more extensible solution. Then, instead of having this in your .kitchen.yml, you would have a real Vagrantfile that is merged with that of Kitchen |
@sethvargo looks like it, so long as people are willing to package their own boxes with customized Vagrantfiles. |
@byggztryng why boxes? You could package it with a cookbook and you only need a partial vagrantfile, which seems to be what is requested here. |
@sethvargo the documentation seems to suggest that the hooks for merging are strictly defined, but I think that packaging a Vagrantfile with a cookbook would require the ability to refer to it in addition to the Vagrantfile generated by kitchen-vagrant, which is sourced at level 3. Is that possible? |
|
Right, but I think that's still just for determining the sole Vagrantfile for level 3. Note that the lookup path logic states that it stops at the first Vagrantfile found, which I think would then block the kitchen-vagrant file. |
@mitchellh what would be a good pattern to follow here? |
@sethvargo The best option would be to somehow |
Okay. So the require_relative '../../Vagrantfile' if File.exist?(File.expand_path('../../Vagrantfile', __FILE__)) |
Yep no problem. You might want to put that at the end if you want that contents to merge AFTER the |
Awesome thanks! |
@sethvargo @byggztryng +1 for using Vagrantfile merging + load order. The name of the Vagrantfile should be configurable though instead of assuming Or default to something more explicit like |
@tknerr I think that entire path could probably be exposed to .kitchen.yml, so that the Vagrantfile could be included in the cookbook files, as suggested by @sethvargo. @sethvargo, I'll take this advice and create a new PR. |
#112 uses the Vagrantfile merging |
@byggztryng do you still need this now that we have user Vagrantfiles? |
@sethvargo personally, no; I've moved on to using docker. |
Cool, thanks! |
I'm willing to implement this feature if there's value seen.
Users may be interested in making simple additions to the default Vagrantfile template, while avoiding having to track and reproduce all of its content or risk breaking functionality. I propose a config option to accept some arbitrary Vagrantfile keys and values that will be appropriately appended to the template, thus providing an additional degree of customization without a significant amount of complexity required.
The text was updated successfully, but these errors were encountered: