You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/03/05 23:07:07 UTC

[GitHub] [cloudstack] ustcweizhou opened a new pull request #3946: server: add global configuration for default router service offering

ustcweizhou opened a new pull request #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946
 
 
   ## Description
   <!--- Describe your changes in detail -->
   
   This adds a global setting router.service.offering, which can be uuid of a system offering. 
   the service offering will be used by virtual routers; 
   if it is not set, default system offering will be used.
   
   This is a account-scope setting, so it can be set in Account setting as well.
   
   <!-- For new features, provide link to FS, dev ML discussion etc. -->
   <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
   
   <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
   <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
   <!-- Fixes: # -->
   
   ## Types of changes
   <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [X] New feature (non-breaking change which adds functionality)
   - [ ] Bug fix (non-breaking change which fixes an issue)
   - [ ] Enhancement (improves an existing feature and functionality)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   
   ## Screenshots (if appropriate):
   
   ## How Has This Been Tested?
   <!-- Please describe in detail how you tested your changes. -->
   <!-- Include details of your testing environment, and the tests you ran to -->
   <!-- see how your change affects other areas of the code, etc. -->
   
   1. create a system offering
   2. change global setting 
   ![image](https://user-images.githubusercontent.com/3204966/76029444-c4a04f80-5f34-11ea-9cc1-b7840c6e1a4d.png)
   
   3. restart network with cleanup
   4. new VRs will be created with new offering
   ![image](https://user-images.githubusercontent.com/3204966/76029690-485a3c00-5f35-11ea-902b-a439fc4d7961.png)
   
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] DaanHoogland commented on issue #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

DaanHoogland commented on issue #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#issuecomment-595755893
 
 
   @blueorangutan package

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

DaanHoogland commented on a change in pull request #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#discussion_r392351506
 
 

 ##########
 File path: server/src/main/java/org/cloud/network/router/deployment/RouterDeploymentDefinition.java
 ##########
 @@ -389,8 +390,34 @@ protected void findDefaultServiceOfferingId() {
         serviceOfferingId = serviceOffering.getId();
     }
 
+    protected void findAccountServiceOfferingId(long accountId) {
+        String accountRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.valueIn(accountId);
+        String globalRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value();
+        if (accountRouterOffering != null) {
+            verifyServiceOfferingByUuid(accountRouterOffering);
+        }
+        if (serviceOfferingId == null && globalRouterOffering != accountRouterOffering) {
 
 Review comment:
   yes, but if we configured a global router offering wouldn't we prefer that even if serviceOfferingId is not null?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

DaanHoogland commented on a change in pull request #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#discussion_r392342652
 
 

 ##########
 File path: server/src/main/java/org/cloud/network/router/deployment/RouterDeploymentDefinition.java
 ##########
 @@ -389,8 +390,34 @@ protected void findDefaultServiceOfferingId() {
         serviceOfferingId = serviceOffering.getId();
     }
 
+    protected void findAccountServiceOfferingId(long accountId) {
+        String accountRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.valueIn(accountId);
+        String globalRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value();
+        if (accountRouterOffering != null) {
+            verifyServiceOfferingByUuid(accountRouterOffering);
+        }
+        if (serviceOfferingId == null && globalRouterOffering != accountRouterOffering) {
 
 Review comment:
   Wouldn't we want `globalRouterOffering` to take precedence over `serviceOfferingId`, @weizhouapache?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] harikrishna-patnala commented on issue #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

harikrishna-patnala commented on issue #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#issuecomment-598023455
 
 
   > I need a test volunteer, other than
   > @blueorangutan test
   
   I'll test this and post the results

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

blueorangutan commented on issue #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#issuecomment-598023552
 
 
   @harikrishna-patnala unsupported parameters provided. Supported mgmt server os are: `centos6, centos7, ubuntu`. Supported hypervisors are: `kvm-centos6, kvm-centos7, kvm-ubuntu, xenserver-71, xenserver-65sp1, xenserver-62sp1, vmware-67u3, vmware-65u2, vmware-60u2, vmware-55u3, vmware-51u1, vmware-50u1`

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

blueorangutan commented on issue #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#issuecomment-595768718
 
 
   Packaging result: ✖centos6 ✔centos7 ✔debian. JID-1023

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] weizhouapache closed pull request #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

weizhouapache closed pull request #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] weizhouapache commented on a change in pull request #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

weizhouapache commented on a change in pull request #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#discussion_r392347103
 
 

 ##########
 File path: server/src/main/java/org/cloud/network/router/deployment/RouterDeploymentDefinition.java
 ##########
 @@ -389,8 +390,34 @@ protected void findDefaultServiceOfferingId() {
         serviceOfferingId = serviceOffering.getId();
     }
 
+    protected void findAccountServiceOfferingId(long accountId) {
+        String accountRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.valueIn(accountId);
+        String globalRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value();
+        if (accountRouterOffering != null) {
+            verifyServiceOfferingByUuid(accountRouterOffering);
+        }
+        if (serviceOfferingId == null && globalRouterOffering != accountRouterOffering) {
 
 Review comment:
   @DaanHoogland serviceOfferingId is the final offering which will be used on virtual router.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] weizhouapache commented on a change in pull request #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

weizhouapache commented on a change in pull request #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#discussion_r392563747
 
 

 ##########
 File path: server/src/main/java/org/cloud/network/router/deployment/RouterDeploymentDefinition.java
 ##########
 @@ -389,8 +390,34 @@ protected void findDefaultServiceOfferingId() {
         serviceOfferingId = serviceOffering.getId();
     }
 
+    protected void findAccountServiceOfferingId(long accountId) {
+        String accountRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.valueIn(accountId);
+        String globalRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value();
+        if (accountRouterOffering != null) {
+            verifyServiceOfferingByUuid(accountRouterOffering);
+        }
+        if (serviceOfferingId == null && globalRouterOffering != accountRouterOffering) {
 
 Review comment:
   > @weizhouapache `if (serviceOffering != null)`, which should be always, `globalRouterOffering` will not be checked, whether or not `globalRouterOffering` == `accountRouterOffering`.
   > so I don't think your 3 and 4 are correct but this condition will never be met and `verifyServiceOfferingByUuid(globalRouterOffering)` will never be executed.
   > So, why check for `serviceOfferingId` before `globalRouterOffering` when it is the last resort and is only relevant after `globalRouterOffering`?
   
   @DaanHoogland it is serviceOfferingId , not serviceOffering.
   
   https://github.com/apache/cloudstack/blob/3783a092a9431e64f6cf11fc7a1056b338f4f7ab/server/src/main/java/org/cloud/network/router/deployment/RouterDeploymentDefinition.java#L109
   
   
   the check procedure
   
   https://github.com/apache/cloudstack/blob/3783a092a9431e64f6cf11fc7a1056b338f4f7ab/server/src/main/java/org/cloud/network/router/deployment/RouterDeploymentDefinition.java#L416-L424
   
   
   in the account part, considering the uuid might be invalid, so check account setting at first, then global setting if account setting is invalid. (if account setting is not set, accountRouterOffering is actually the global setting, so skip the check if accountRouterOffering equals to globalRouterOffering )
   
   https://github.com/apache/cloudstack/blob/3783a092a9431e64f6cf11fc7a1056b338f4f7ab/server/src/main/java/org/cloud/network/router/deployment/RouterDeploymentDefinition.java#L393-L414
   
   
   hope it helps you understanding all changes in PR. 
   
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] weizhouapache commented on issue #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

weizhouapache commented on issue #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#issuecomment-595719569
 
 
   > We already have a provision to mention the service offering id while creating network offering which serves the same purpose right!
   
   @harikrishna-patnala it is an option, but it is complicated to change the offering of a VR :
   (1)create network offering with router offering id
   (2) update network to new network offering
   Suppose there are many networks with different network offerings, we need to create some new offerings. There is some downtime in step (2).
   
   It is very simple with this PR. (1) change setting, (2) restart network with cleanup.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

harikrishna-patnala commented on a change in pull request #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#discussion_r392017354
 
 

 ##########
 File path: server/src/main/java/org/cloud/network/router/deployment/RouterDeploymentDefinition.java
 ##########
 @@ -389,8 +390,34 @@ protected void findDefaultServiceOfferingId() {
         serviceOfferingId = serviceOffering.getId();
     }
 
+    protected void findAccountServiceOfferingId(long accountId) {
+        String accountRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.valueIn(accountId);
+        String globalRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value();
+        if (accountRouterOffering != null) {
+            verifyServiceOfferingByUuid(accountRouterOffering);
+        }
+        if (serviceOfferingId == null && globalRouterOffering != accountRouterOffering) {
+            verifyServiceOfferingByUuid(globalRouterOffering);
+        }
+    }
+
+    private void verifyServiceOfferingByUuid(String offeringUuid) {
+        logger.debug("Verifying router service offering with uuid : " + offeringUuid);
+        ServiceOfferingVO serviceOffering = serviceOfferingDao.findByUuid(offeringUuid);
+        if (serviceOffering != null && serviceOffering.isSystemUse()) {
+            boolean isLocalStorage = ConfigurationManagerImpl.SystemVMUseLocalStorage.valueIn(dest.getDataCenter().getId());
+            if (isLocalStorage == serviceOffering.isUseLocalStorage()) {
+                logger.debug("service offering " + serviceOffering.getId() + " will be used on virtual router");
 
 Review comment:
   offeringUuid would be better than id in the log message. with that LGTM

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] svenvogel commented on issue #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

svenvogel commented on issue #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#issuecomment-595704580
 
 
   @ustcweizhou nice feature. 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] weizhouapache commented on issue #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

weizhouapache commented on issue #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#issuecomment-598183870
 
 
   > Following are the test cases covered for both isolated and VPC networks.
   > 
   > 1. Created a network with '**router.service.offering**' set to **empty** - VR deployed with '**System Offering For Software Router**' - PASSED
   > 2. Created a network with **global** configuration parameter **'router.service.offering**' set to UUID of '**VRcustomCO1**' compute offering - VR deployed with '**VRcustomCO1**' - PASSED
   > 3. Created a network with **account level** configuration parameter '**router.service.offering**' set to UUID of '**VRcustomCO2**' compute offering - VR deployed with '**VRcustomCO2**' - PASSED
   > 4. Created a network with **account level** configuration parameter '**router.service.offering**' set to an **invalid UUID** - VR deployed with '**System Offering For Software Router**'. Here in this case global configuration parameter is set to '**VRcustomCO1**'.
   > 
   > In test case 4, irrespective of global config parameter VR deployed with default SO. If this behavior is expected a log message would help debugging.
   
   @harikrishna-patnala thanks for testing. I think it make more sense to use '**VRcustomCO1**' instead of '**System Offering For Software Router**' in case 4.  I will update the PR.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] blueorangutan commented on issue #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

blueorangutan commented on issue #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#issuecomment-598042715
 
 
   Packaging result: ✖centos6 ✔centos7 ✔debian. JID-1042

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

DaanHoogland commented on a change in pull request #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#discussion_r392508300
 
 

 ##########
 File path: server/src/main/java/org/cloud/network/router/deployment/RouterDeploymentDefinition.java
 ##########
 @@ -389,8 +390,34 @@ protected void findDefaultServiceOfferingId() {
         serviceOfferingId = serviceOffering.getId();
     }
 
+    protected void findAccountServiceOfferingId(long accountId) {
+        String accountRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.valueIn(accountId);
+        String globalRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value();
+        if (accountRouterOffering != null) {
+            verifyServiceOfferingByUuid(accountRouterOffering);
+        }
+        if (serviceOfferingId == null && globalRouterOffering != accountRouterOffering) {
 
 Review comment:
   @weizhouapache `if (serviceOffering != null)`, which should be always, `globalRouterOffering` will not be checked, whether or not `globalRouterOffering` == `accountRouterOffering`.
   so I don't think your 3 and 4 are correct but this condition will never be met and `verifyServiceOfferingByUuid(globalRouterOffering)` will never be executed.
   So, why check for `serviceOfferingId` before `globalRouterOffering` when it is the last resort and is only relevant after `globalRouterOffering`?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] weizhouapache commented on issue #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

weizhouapache commented on issue #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#issuecomment-598806841
 
 
   updated based on @harikrishna-patnala 's comment

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

DaanHoogland commented on a change in pull request #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#discussion_r392568615
 
 

 ##########
 File path: server/src/main/java/org/cloud/network/router/deployment/RouterDeploymentDefinition.java
 ##########
 @@ -389,8 +390,34 @@ protected void findDefaultServiceOfferingId() {
         serviceOfferingId = serviceOffering.getId();
     }
 
+    protected void findAccountServiceOfferingId(long accountId) {
+        String accountRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.valueIn(accountId);
+        String globalRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value();
+        if (accountRouterOffering != null) {
+            verifyServiceOfferingByUuid(accountRouterOffering);
+        }
+        if (serviceOfferingId == null && globalRouterOffering != accountRouterOffering) {
 
 Review comment:
   ah, i missed the side effect of `verifyServiceOfferingByUuid()`, where the id is set, thanks.
   
   Maybe to make this more obvious for stupid people like me we can make `verifyServiceOfferingByUuid()` return a uuid or null and set it in the `findAccountServiceOfferingId()`?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] weizhouapache commented on a change in pull request #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

weizhouapache commented on a change in pull request #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#discussion_r392476198
 
 

 ##########
 File path: server/src/main/java/org/cloud/network/router/deployment/RouterDeploymentDefinition.java
 ##########
 @@ -389,8 +390,34 @@ protected void findDefaultServiceOfferingId() {
         serviceOfferingId = serviceOffering.getId();
     }
 
+    protected void findAccountServiceOfferingId(long accountId) {
+        String accountRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.valueIn(accountId);
+        String globalRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value();
+        if (accountRouterOffering != null) {
+            verifyServiceOfferingByUuid(accountRouterOffering);
+        }
+        if (serviceOfferingId == null && globalRouterOffering != accountRouterOffering) {
 
 Review comment:
   @daanhoogland correct. it checks following offerings
   
   1. Router service Offering of network/vpc offering
   2. Account route service offering
   3. Global route service offering 
   4. Default route offering 
   
   If 1 or 2 is set and validated, global setting will not be used.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] harikrishna-patnala commented on issue #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

harikrishna-patnala commented on issue #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#issuecomment-595744372
 
 
   I'm not against the new option, but wanted to know the main intention of adding this. 
   Code changes LGTM.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [cloudstack] harikrishna-patnala commented on issue #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

harikrishna-patnala commented on issue #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#issuecomment-598102156

Following are the test cases covered for both isolated and VPC networks.

1. Created a network with '**router.service.offering**' set to **empty** - VR deployed with '**System Offering For Software Router**' - PASSED
2. Created a network with **global** configuration parameter **'router.service.offering**' set to UUID of '**VRcustomCO1**' compute offering - VR deployed with '**VRcustomCO1**' - PASSED
3. Created a network with **account level** configuration parameter '**router.service.offering**' set to UUID of '**VRcustomCO2**' compute offering - VR deployed with '**VRcustomCO2**' - PASSED
4. Created a network with **account level** configuration parameter '**router.service.offering**' set to an **invalid UUID** - VR deployed with '**System Offering For Software Router**'. Here in this case global configuration parameter is set to '**VRcustomCO1**'.

In test case 4, irrespective of global config parameter VR deployed with default SO. If this behavior is expected a log message would help debugging.

----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

[GitHub] [cloudstack] DaanHoogland commented on issue #3946: server: add global configuration for default router service offering

Posted by GitBox <gi...@apache.org>.

DaanHoogland commented on issue #3946: server: add global configuration for default router service offering
URL: https://github.com/apache/cloudstack/pull/3946#issuecomment-596061985
 
 
   I need a test volunteer, other than
   @blueorangutan test

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services