weizhouapache commented on code in PR #9415:
URL: https://github.com/apache/cloudstack/pull/9415#discussion_r1706550746
##########
test/integration/component/test_advancedsg_networks.py:
##########
@@ -169,6 +171,26 @@ def setUpClass(cls):
# Get Zone, Domain and templates
cls.domain = get_domain(cls.api_client)
cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests())
+ if cls.zone.securitygroupsenabled is False:
Review Comment:
component test won't run in trillian test
can you move to smoke ? if some tests are not really necessary, we can
remove `"advanced"` from `@attr(tags`
if everything is fine, we can move back to component if it takes long time.
##########
ui/src/views/infra/network/ServiceProvidersTab.vue:
##########
@@ -937,10 +937,38 @@ export default {
}
]
},
- // {
- // title: 'SecurityGroupProvider',
- // details: ['name', 'state', 'id', 'servicelist'],
- // },
+ {
+ title: 'SecurityGroupProvider',
Review Comment:
is it disabled by default in advanced zone ?
can you add a doc PR to tell how to support SG ? it seems users only need to
enable the provider ?
can SG be supported per network or per account ?
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -6165,6 +6165,28 @@ public UserVm createVirtualMachine(DeployVMCmd cmd)
throws InsufficientCapacityE
return vm;
}
+ private boolean checkSecurityGroupSupportForNetwork(DataCenter zone,
List<Long> networkIds,
+ List<Long>
securityGroupsIds) {
+ if (zone.isSecurityGroupEnabled()) {
+ return true;
+ }
+ if (CollectionUtils.isNotEmpty(networkIds)) {
+ for (Long networkId : networkIds) {
+ Network network = _networkDao.findById(networkId);
+ if (network == null) {
+ throw new InvalidParameterValueException("Unable to find
network by id " + networkId);
+ }
+ if (network.getGuestType() == Network.GuestType.Shared &&
_networkModel.isSecurityGroupSupportedInNetwork(network)) {
+ return true;
+ }
+ }
+ } else if (CollectionUtils.isNotEmpty(securityGroupsIds)) {
+ Network networkWithSecurityGroup =
_networkModel.getNetworkWithSGWithFreeIPs(zone.getId());
Review Comment:
@vishesh92
can you explain why added this ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
