-
Notifications
You must be signed in to change notification settings - Fork 43
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
Only use sec groups if enabled, don't use networks in Basic #134
Only use sec groups if enabled, don't use networks in Basic #134
Conversation
Ping @miguelaferreira , @j00p34 ; what do you think? This eliminates the need to specify Basic or Advanced zone in the Vagrantfile. It will detect if security groups are enabled, apparently specified separately from Basic/Advanced:
For Basic zone:
Perhaps it would be even more nice if standard objects, like zone, would not only Nice for a refactor, next time 😉 |
Sounds ok to me. It needs an update to the README though |
Good point, will do |
1130066
to
f5d3529
Compare
@nicolasbrechet thanks for you PR, I made something similar only which queries the zone to dynamically see if it's advanced or not. Could you give this one a try to see if it works? |
2462cf1
to
849c83d
Compare
@bheuvel can you please rebase this PR on master (and the last one as well)? |
d0b0cf9
to
f70f529
Compare
This one also looks good:
|
f70f529
to
3ffeeb2
Compare
@j00p34 @miguelaferreira could one of you test with adding
this "proves" the autodetection, and shows that using 'Basic' feature in an 'Advanced' Vagrantfile does not fail (but warns). |
3ffeeb2
to
f6fb216
Compare
Of course!! Will do that the latest tomorrow. |
Done, tests passed:
Vagrantfile:
Should we add this to all vagrant files under functional-tests? |
Only use sec groups if enabled, don't use networks in Basic
I.o.w. replaced domain_config.network_type with dynamically detect for network type and support for security groups.