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 2021/07/11 15:52:31 UTC

[GitHub] [cloudstack] shatoboar opened a new pull request #5195: Persistent Networks Resources created on host during time of connecti…

shatoboar opened a new pull request #5195:
URL: https://github.com/apache/cloudstack/pull/5195


   …on, and cleanup during time of removal
   
   ### Description
   
   
   <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
   This PR adds a feature to L2 persitent networks, where resources of the persistent networks are created at hostConnect phase of DefaultHostListener, and cleaned up at hostAboutToBeRemoved phase. 
   
   <!-- For new features, provide link to FS, dev ML discussion etc. -->
   <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
   This PR also fixes a bug, where rebooting the hosts removed the persistent network's resources, without readding them when the host is readded to the zone.
   
   <!-- 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: # -->
   
   <!--- ********************************************************************************* -->
   <!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
   <!--- PLEASE PUT AN 'X' in only **ONE** box -->
   <!--- ********************************************************************************* -->
   
   ### Types of changes
   
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [ ] Bug fix (non-breaking change which fixes an issue)
   - [x] Enhancement (improves an existing feature and functionality)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [ ] Major
   - [x] Minor
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [x] Minor
   - [ ] Trivial
   
   
   ### Screenshots (if appropriate):
   
   
   ### How Has This Been Tested?
   This has been manually tested with a mbx kvm setup on a local machine. 
   Different test scenarios:
   1. When the persistent network is created and the host is added afterwards, the newly added host has the persistent network's resources.
   2. When a host within a zone with a persistent network is removed from the zone, its resources will be cleaned up.
   3. When a host with a running systemvm is removed from the zone, the running VMs will migrate before the resources are cleaned up.
   4. When a host is rebooted, it still has access to the persistent network's resources.
   <!-- 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. -->
   
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] blueorangutan commented on pull request #5195: Persistent Networks Resources created on host during time of connecti…

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5195:
URL: https://github.com/apache/cloudstack/pull/5195#issuecomment-903202465


   <b>Trillian test result (tid-1742)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 34008 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5195-t1742-kvm-centos7.zip
   Smoke tests completed. 89 look OK, 0 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
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-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] Pearl1594 commented on pull request #5195: Persistent Networks Resources created on host during time of connecti…

Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #5195:
URL: https://github.com/apache/cloudstack/pull/5195#issuecomment-1034562244


   Closing this PR - tracking it with PR #5977 


-- 
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-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] blueorangutan commented on pull request #5195: Persistent Networks Resources created on host during time of connecti…

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5195:
URL: https://github.com/apache/cloudstack/pull/5195#issuecomment-900461133


   <b>Trillian test result (tid-1672)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 43628 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5195-t1672-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Smoke tests completed. 87 look OK, 2 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 347.16 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Failure` | 331.69 | test_routers_network_ops.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 533.36 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 502.04 | test_vpc_redundant.py
   


-- 
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-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] blueorangutan commented on pull request #5195: Persistent Networks Resources created on host during time of connecti…

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5195:
URL: https://github.com/apache/cloudstack/pull/5195#issuecomment-899979856


   @Pearl1594 a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
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-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] blueorangutan commented on pull request #5195: Persistent Networks Resources created on host during time of connecti…

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5195:
URL: https://github.com/apache/cloudstack/pull/5195#issuecomment-899970740


   @Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] blueorangutan commented on pull request #5195: Persistent Networks Resources created on host during time of connecti…

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5195:
URL: https://github.com/apache/cloudstack/pull/5195#issuecomment-900461133


   <b>Trillian test result (tid-1672)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 43628 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5195-t1672-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Smoke tests completed. 87 look OK, 2 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 347.16 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Failure` | 331.69 | test_routers_network_ops.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 533.36 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 502.04 | test_vpc_redundant.py
   


-- 
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-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] Pearl1594 commented on pull request #5195: Persistent Networks Resources created on host during time of connecti…

Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #5195:
URL: https://github.com/apache/cloudstack/pull/5195#issuecomment-899979494


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

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] DaanHoogland commented on pull request #5195: Persistent Networks Resources created on host during time of connecti…

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5195:
URL: https://github.com/apache/cloudstack/pull/5195#issuecomment-903139174


   @blueorangutan test keepEnv


-- 
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-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] blueorangutan commented on pull request #5195: Persistent Networks Resources created on host during time of connecti…

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5195:
URL: https://github.com/apache/cloudstack/pull/5195#issuecomment-899978705


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 889


-- 
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-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] NuxRo commented on pull request #5195: Persistent Networks Resources created on host during time of connecti…

Posted by GitBox <gi...@apache.org>.
NuxRo commented on pull request #5195:
URL: https://github.com/apache/cloudstack/pull/5195#issuecomment-1072513830


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

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] blueorangutan commented on pull request #5195: Persistent Networks Resources created on host during time of connecti…

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5195:
URL: https://github.com/apache/cloudstack/pull/5195#issuecomment-903139218


   @DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
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-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] Pearl1594 commented on pull request #5195: Persistent Networks Resources created on host during time of connecti…

Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #5195:
URL: https://github.com/apache/cloudstack/pull/5195#issuecomment-899970641


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

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #5195: Persistent Networks Resources created on host during time of connecti…

Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #5195:
URL: https://github.com/apache/cloudstack/pull/5195#discussion_r677734833



##########
File path: engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java
##########
@@ -59,12 +70,66 @@
     StoragePoolDetailsDao storagePoolDetailsDao;
     @Inject
     StorageManager storageManager;
+    @Inject
+    NetworkOfferingDao networkOfferingDao;
+    @Inject
+    HostDao hostDao;
+    @Inject
+    NetworkModel networkModel;
+    @Inject
+    ConfigurationManager configManager;
+    @Inject
+    NetworkDao networkDao;
+
 
     @Override
     public boolean hostAdded(long hostId) {
         return true;
     }
 
+    private boolean createPersistentNetworkResourcesOnHost(long hostId) {
+        HostVO host = hostDao.findById(hostId);
+        if (host != null) {

Review comment:
       We could invert this if to reduce indentation and maybe add a log.

##########
File path: engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java
##########
@@ -59,12 +70,66 @@
     StoragePoolDetailsDao storagePoolDetailsDao;
     @Inject
     StorageManager storageManager;
+    @Inject
+    NetworkOfferingDao networkOfferingDao;
+    @Inject
+    HostDao hostDao;
+    @Inject
+    NetworkModel networkModel;
+    @Inject
+    ConfigurationManager configManager;
+    @Inject
+    NetworkDao networkDao;
+
 
     @Override
     public boolean hostAdded(long hostId) {
         return true;
     }
 
+    private boolean createPersistentNetworkResourcesOnHost(long hostId) {
+        HostVO host = hostDao.findById(hostId);
+        if (host != null) {
+            List<NetworkVO> allPersistentNetworks = networkDao.getAllPersistentNetworksFromZone(host.getDataCenterId());
+
+            for (NetworkVO networkVO : allPersistentNetworks) {
+                NetworkOfferingVO networkOfferingVO = networkOfferingDao.findById(networkVO.getNetworkOfferingId());
+
+                SetupPersistentNetworkCommand persistentNetworkCommand =
+                        new SetupPersistentNetworkCommand(createNicTOFromNetworkAndOffering(networkVO, networkOfferingVO, host));
+                Answer answer = agentMgr.easySend(hostId, persistentNetworkCommand);
+                if (answer == null) {
+                    throw new CloudRuntimeException("Unable to get answer to the setup persistent network command " + networkVO.getId());
+                }
+                if (!answer.getResult()) {
+                    String msg = "Unable to create L2 persistent network resources from network " + networkVO.getId() + " on the host" + hostId;

Review comment:
       We could improve this message with more context than only id. Also we could use String.format here. 

##########
File path: engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java
##########
@@ -137,7 +203,26 @@ public boolean hostDisconnected(long hostId, long poolId) {
 
     @Override
     public boolean hostAboutToBeRemoved(long hostId) {
-        return true;
+        // send host the cleanup persistent network resources
+        HostVO host = hostDao.findById(hostId);
+        if (host != null) {

Review comment:
       We could invert this if to reduce indentation and maybe add a log.




-- 
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-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] Pearl1594 closed pull request #5195: Persistent Networks Resources created on host during time of connecti…

Posted by GitBox <gi...@apache.org>.
Pearl1594 closed pull request #5195:
URL: https://github.com/apache/cloudstack/pull/5195


   


-- 
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-unsubscribe@cloudstack.apache.org

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

[GitHub] [cloudstack] NuxRo removed a comment on pull request #5195: Persistent Networks Resources created on host during time of connecti…

Posted by GitBox <gi...@apache.org>.
NuxRo removed a comment on pull request #5195:
URL: https://github.com/apache/cloudstack/pull/5195#issuecomment-1072513830


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

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

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