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/08/20 08:12:22 UTC

[GitHub] [cloudstack] weizhouapache opened a new pull request #5348: security group: fix component test test_multiple_nic_support.py failures 

weizhouapache opened a new pull request #5348:
URL: https://github.com/apache/cloudstack/pull/5348


   ### Description
   
   This PR fixes component test failure with test_multiple_nic_support.py, in advanced zone with security groups.
   
   <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
   
   <!-- 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: # -->
   
   <!--- ********************************************************************************* -->
   <!--- 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)
   - [x] 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)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [ ] Major
   - [ ] Minor
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [x] Minor
   - [ ] Trivial
   
   
   ### 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. -->
   
   manually run the component test, all passed.
   
   ```
   Create Vm with multiple NIC's ... === TestName: test_01_create_vm_with_multiple_nics | Status : SUCCESS ===
   ok
   Create VM with single NIC and then add additional NIC ... === TestName: test_02_add_nic_to_vm | Status : SUCCESS ===
   ok
   Add secondary IP's to the VM ... === TestName: test_03_add_ip_to_default_nic | Status : SUCCESS ===
   ok
   Add secondary IP's to remaining NIC's ... === TestName: test_04_add_ip_to_remaining_nics | Status : SUCCESS ===
   ok
   Stop and Start a VM with Multple NIC ... === TestName: test_05_stop_start_vm_with_multiple_nic | Status : SUCCESS ===
   ok
   Migrate a VM with Multple NIC ... === TestName: test_06_migrate_vm_with_multiple_nic | Status : SUCCESS ===
   ok
   Remove secondary IP from any NIC ... === TestName: test_07_remove_secondary_ip_from_nic | Status : SUCCESS ===
   ok
   Remove NIC from VM ... === TestName: test_08_remove_nic_from_vm | Status : SUCCESS ===
   ok
   Reboot a VM with Multple NIC ... === TestName: test_09_reboot_vm_with_multiple_nic | Status : SUCCESS ===
   ok
   
   ----------------------------------------------------------------------
   Ran 9 tests in 359.228s
   
   OK
   
   ```
   
   <!-- 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] rhtyd commented on a change in pull request #5348: security group: fix component test test_multiple_nic_support.py failures

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



##########
File path: test/integration/component/test_multiple_nic_support.py
##########
@@ -224,7 +224,7 @@ def setUpClass(cls):
                     cls.virtual_machine1.default_network_id = nic.networkid
                     break
         except Exception as e:
-            cls.fail("Exception while deploying virtual machine: %s" % e)
+            cls.fail("Exception while deploying virtual machine: %s" % (e))

Review comment:
       @DaanHoogland there's an `f"Ex...` is that a typo or something new in py3?




-- 
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 #5348: security group: fix component test test_multiple_nic_support.py failures

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


   wait, this was merged with these suggestions? Do we need to revert?


-- 
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] weizhouapache commented on a change in pull request #5348: security group: fix component test test_multiple_nic_support.py failures

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



##########
File path: test/integration/component/test_multiple_nic_support.py
##########
@@ -224,7 +224,7 @@ def setUpClass(cls):
                     cls.virtual_machine1.default_network_id = nic.networkid
                     break
         except Exception as e:
-            cls.fail("Exception while deploying virtual machine: %s" % e)
+            cls.fail(f"Exception while deploying virtual machine: {e}")

Review comment:
       @DaanHoogland 
   a bit strange that this does not work in centos7/python2
   
   ```
   ======================================================================
   ERROR: Failure: SyntaxError (invalid syntax (test_multiple_nic_support.py, line 227))
   ----------------------------------------------------------------------
   Traceback (most recent call last):
     File "/usr/lib/python2.7/site-packages/nose/case.py", line 132, in run
       self.beforeTest(result)
     File "/usr/lib/python2.7/site-packages/nose/case.py", line 74, in beforeTest
       beforeTest(self.test)
     File "/usr/lib/python2.7/site-packages/nose/proxy.py", line 117, in beforeTest
       self.plugins.beforeTest(self.test)
     File "/usr/lib/python2.7/site-packages/nose/plugins/manager.py", line 99, in __call__
       return self.call(*arg, **kw)
     File "/usr/lib/python2.7/site-packages/nose/plugins/manager.py", line 167, in simple
       result = meth(*arg, **kw)
     File "/usr/lib/python2.7/site-packages/marvin/marvinPlugin.py", line 174, in beforeTest
       join([self.__identifier, self.__testName])
   TypeError: sequence item 0: expected string, NoneType found
   -------------------- >> begin captured stdout << ---------------------
   === TestName: Failure: | Status : EXCEPTION ===
   ```




-- 
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 #5348: security group: fix component test test_multiple_nic_support.py failures

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


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


-- 
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 a change in pull request #5348: security group: fix component test test_multiple_nic_support.py failures

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



##########
File path: test/integration/component/test_multiple_nic_support.py
##########
@@ -224,7 +224,7 @@ def setUpClass(cls):
                     cls.virtual_machine1.default_network_id = nic.networkid
                     break
         except Exception as e:
-            cls.fail("Exception while deploying virtual machine: %s" % e)
+            cls.fail(f"Exception while deploying virtual machine: {e}")

Review comment:
       not sure why, but the old way works (and still does in py3) no worries. I'll give it a look one of these days.




-- 
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] weizhouapache commented on pull request #5348: security group: fix component test test_multiple_nic_support.py failures

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


   > wait, this was merged with these suggestions? Do we need to revert?
   
   @DaanHoogland if it works with python3, no need to revert it. it is just component test which is rarely run.
   


-- 
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] rhtyd merged pull request #5348: security group: fix component test test_multiple_nic_support.py failures

Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #5348:
URL: https://github.com/apache/cloudstack/pull/5348


   


-- 
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 #5348: security group: fix component test test_multiple_nic_support.py failures

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


   @weizhouapache 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] DaanHoogland commented on a change in pull request #5348: security group: fix component test test_multiple_nic_support.py failures

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



##########
File path: test/integration/component/test_multiple_nic_support.py
##########
@@ -224,7 +224,7 @@ def setUpClass(cls):
                     cls.virtual_machine1.default_network_id = nic.networkid
                     break
         except Exception as e:
-            cls.fail("Exception while deploying virtual machine: %s" % e)
+            cls.fail("Exception while deploying virtual machine: %s" % (e))

Review comment:
       ```suggestion
               cls.fail(f"Exception while deploying virtual machine: {e}")
   ```
   not necessary but nice ;)

##########
File path: test/integration/component/test_multiple_nic_support.py
##########
@@ -243,7 +243,7 @@ def setUpClass(cls):
                     cls.virtual_machine2.default_network_id = nic.networkid
                     break
         except Exception as e:
-            cls.fail("Exception while deploying virtual machine: %s" % e)
+            cls.fail("Exception while deploying virtual machine: %s" % (e))

Review comment:
       ```suggestion
               cls.fail(f"Exception while deploying virtual machine: {e}")
   ```




-- 
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 #5348: security group: fix component test test_multiple_nic_support.py failures

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


   <b>Trillian Build Failed (tid-1734)<b/>


-- 
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] rhtyd commented on pull request #5348: security group: fix component test test_multiple_nic_support.py failures

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


   Trillian does not run SG tests, we can merge based on Wei's test 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.

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

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

[GitHub] [cloudstack] rhtyd commented on a change in pull request #5348: security group: fix component test test_multiple_nic_support.py failures

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



##########
File path: test/integration/component/test_multiple_nic_support.py
##########
@@ -224,7 +224,7 @@ def setUpClass(cls):
                     cls.virtual_machine1.default_network_id = nic.networkid
                     break
         except Exception as e:
-            cls.fail("Exception while deploying virtual machine: %s" % e)
+            cls.fail("Exception while deploying virtual machine: %s" % (e))

Review comment:
       Nevermind, I googled - I need a bit a re-learning py3 https://realpython.com/python-f-strings/




-- 
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 #5348: security group: fix component test test_multiple_nic_support.py failures

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


   @weizhouapache 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] DaanHoogland commented on a change in pull request #5348: security group: fix component test test_multiple_nic_support.py failures

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



##########
File path: test/integration/component/test_multiple_nic_support.py
##########
@@ -224,7 +224,7 @@ def setUpClass(cls):
                     cls.virtual_machine1.default_network_id = nic.networkid
                     break
         except Exception as e:
-            cls.fail("Exception while deploying virtual machine: %s" % e)
+            cls.fail(f"Exception while deploying virtual machine: {e}")

Review comment:
       ok, double checked:
   ```
   >>> e = Exception("root cause")
   >>> raise Exception(f"nested exception {e}")
   Traceback (most recent call last):
     File "<stdin>", line 1, in <module>
   Exception: nested exception root cause
   ```
   I'll fix if it turns out to be a problem for anyone ;)




-- 
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] weizhouapache commented on pull request #5348: security group: fix component test test_multiple_nic_support.py failures

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


   @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] weizhouapache commented on pull request #5348: security group: fix component test test_multiple_nic_support.py failures

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


   test with advanced zone is not needed actually.
   
   @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 #5348: security group: fix component test test_multiple_nic_support.py failures

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


   just to clarify @weizhouapache , I suppose you created a advanced zone with security groups and ran the test-file against that?
   
   kind of asking also to let you consider my non-vital suggestion. I'm fine to merge without.


-- 
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 #5348: security group: fix component test test_multiple_nic_support.py failures

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


   ok, i didn't test it yet, but i will and amend if needed. 


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