Fixes #9568: Add id autodetection in vagrantfile #84
Conversation
|
ping @amousset |
| @@ -90,15 +120,15 @@ def configure(config, os, pf_name, pf_id, host_name, host_id, | |||
|
|
|||
| # provisioning script | |||
| if os == $windows7 or os == $windows2008 then | |||
| command = "c:/vagrant/scripts/network.cmd #{net} #{host_list}\n" | |||
| command = "c:/vagrant/scripts/network.cmd #{net} @host_list@\n" | |||
There was a problem hiding this comment.
what does the @ mean ? I did'nt find anything relatd in ruby ...
There was a problem hiding this comment.
nothing, it is a marker
The goal is to avoid interpreting this variable now because it is not yes full.
It is interpreted at the bottom of the script with a sub function
There was a problem hiding this comment.
ok ! saw it! command.sub("@host_list@", host_list)
It's confusing with the host_list variable passed as parameter
There was a problem hiding this comment.
yes it could be called host_list_to_replace
| } | ||
| $last_pf_id = 0 | ||
|
|
||
| def configure_box(config, os, pf_name, host_name, |
There was a problem hiding this comment.
Is this function called elsewhere ?
There was a problem hiding this comment.
no, it is meant to be called from the varantfile
| windows_plugin:false, advanced_reporting:false, | ||
| ncf_version:nil, cfengine_version:nil, ram:nil | ||
| ) | ||
| pf = $platforms.fetch(pf_name) { |key| |
There was a problem hiding this comment.
As i understand the goal is to replace all calls to configure to configure_box in vagrantfile in another pull request ?
But, unless you start all platforms, the platforms variable will always start from scratch and so have id 0 for all platform ?
There was a problem hiding this comment.
Not, the vagrantfile is considered to be a configuration file and not just a script. So it is fully evaluated before launching ant VM
There was a problem hiding this comment.
It means you need to have only configure_box call in your vagrantfile, if you have a mixed state it won't work ?
There was a problem hiding this comment.
Yes, we need something to migrate
|
OK, merging this PR |
https://www.rudder-project.org/redmine/issues/9568