-
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
Support multiple network ids and network names #148
Support multiple network ids and network names #148
Conversation
@@ -126,7 +126,8 @@ to update UUIDs in your Vagrantfile. If both are specified, the id parameter tak | |||
* `instance_ready_timeout` - The number of seconds to wait for the instance | |||
to become "ready" in Cloudstack. Defaults to 120 seconds. | |||
* `domain_id` - Domain id to launch the instance into | |||
* `network_id` - Network uuid that the instance should use | |||
* `network_id` - Network uuid(s) that the instance should use | |||
- `network_id` is single value (e.g. `"AAAA"`) or multiple values (e.g. `["AAAA", "BBBB"]`) | |||
* `network_name` - Network name that the instance should use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sue445 Nice one! But if we do multiple networks by network_id
, we should also allow that via network_name
@bheuvel I pushed fixes. Please review again 🙇 |
@@ -34,8 +34,22 @@ def call(env) | |||
|
|||
sanitize_domain_config | |||
|
|||
network_ids = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps you can move this new code block (lines 37-49) to the sanitize_domain_config
?
It's looking good! But I will need to have a closer look later on, so some comments after a quick look: We may need to support either IDs OR Names and not both at the same time. I would suggest to keep it simple and use IDs if present, and only use names if IDs are not present. |
@bheuvel I fix and push. |
@sue445 All tests passing, did some "manual" testing with multiple network names/ids, and is looking good! Thanks! |
😃 |
CloudStack
network_ids
can accept multiple values. But vagrant-cloudstack plugin can not use multiple network_ids (only single network_id).So I added feature for multiple network ids
Usage
Vagrantfile