Azure - add AZURE_NETWORK_RESOURCE_GROUP to pick vnet from another ResourceGroup #321
Conversation
… from another ResourceGroup
|
@patchkez Thanks for this PR. The maintainers will take it up for discussion on Tuesday and will be able to provide some feedback then. An immediate comment which sprang to mind is whether the partitioning of resource groups is somewhat subjective, or whether having a separate network resource group is standard practice. In the former case, we should perhaps look into making this more generic. Please also look into the linting failures in the PR. |
|
Lint error has been fixed. I think such scenario is common for enterprise/bigger companies, where every team want's to have it's resources in own resource group. Such segregation allows automatically apply/inherit tags to resources and use tags to manage and allocate costs across the different groups. Then when such Azure cloud account is connected to internal company network, the configuration (peer links, virtual network, net security group, address space) can be deployed in separate resource group and access can be given to other resource groups to obtain IPs from pool of internal network. |
|
Hey @patchkez ! First of all, thank you for the PR! We love to see people using and contributing back to the repo!! First, a small mostly aesthetic change: could you rename Beyond that, we discussed this change today, and given that it is Azure-specific and backwards compatible, we believe it is okay to be done for networking only, but have a suggestion for a possible way to make it better long-term. The idea is to allow for specifying various resource groups per service type, rather than a specific value only for networking service. A similar implementation was done for Zones in Openstack, where the need arose for one to be able to specify different zone name per service (eg: different compute vs networking zone name). You can find this implementation here: https://github.com/CloudVE/cloudbridge/blob/main/cloudbridge/providers/openstack/provider.py#L308 Concretely, the idea would be to add an optional variable for each type of service (networking, security, compute, storage), so you'd then be able to provide the configuration option or environment variable If that makes sense and you're willing to add the more general extension for other services, that'd be awesome, and will likely become a useful feature that other users might benefit from more generally! We however understand time can be hard to find, so while we would encourage that to be added, we don't want to block this PR until that can be done, since this one can exist in the meantime as it's backwards compatible. We do want the name change to happen, unless you can argue why it makes more sense to remain |
|
@almahmoud Thanks for the feedback. I have no problem with renaming NETWORK->NETWORKING. Just pushed new commit with this change. Not sure when I will have more time for this, but I could try to implement similar pattern as you showed in Openstack/provider.py for other Azure services. I also noticed the GH tests are failing, also for other providers, not sure what is wrong there... |
|
No worries, the integration tests are likely failing because we don't make tokens available to PR runs. |
This PR adds new option for defining extra ResourceGroup from which vnet can be picked.
Closes #320