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/10/13 06:42:48 UTC

[GitHub] [cloudstack] DK101010 opened a new pull request #4399: PR multi tags in host offering [#4398]

DK101010 opened a new pull request #4399:
URL: https://github.com/apache/cloudstack/pull/4399


   ## Description
   - only changed HostDaoImpl.java to handle with comma separated multi host string
   - old behavior with one tag works like the old implementation 
   
   
   ## 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)
   - [ ] 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)
   
   ## How Has This Been Tested?
   manually tested (run cloudstack in my vmware environment)
   
   
   <!-- 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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in compute offering [#4398]

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


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


-- 
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 #4399: PR multi tags in host offering [#4398]

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


   <b>Trillian test result (tid-1253)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 49143 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4399-t1253-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Smoke tests completed. 87 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 3603.78 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 3619.94 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 41.48 | test_kubernetes_clusters.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] nvazquez merged pull request #4399: PR multi tags in compute offering [#4398]

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


   


-- 
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 #4399: PR multi tags in host offering [#4398]

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


   @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



[GitHub] [cloudstack] DK101010 commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   > found two bugs in testing
   > (1) list hosts response lists only 1 host tag, although hosts have multiple host tags.
   > for example
   > 
   > ```
   > (localcloud) SBCM5> > update host id=d9988a7b-7e02-4e90-9c4a-798c588b1cba hosttags=tag1,tag2
   > {
   >   "host": {
   > ...
   >     "hosttags": "tag2",
   > ...
   >   }
   > }
   > 
   > (localcloud) SBCM5> > list hosts id=d9988a7b-7e02-4e90-9c4a-798c588b1cba filter=hosttags
   > {
   >   "count": 1,
   >   "host": [
   >     {
   >       "hosttags": "tag2"
   >     }
   >   ]
   > }
   > ```
   > 
   > however in db, host has 2 tags.
   > 
   > ```
   > mysql> select * from host_tags;
   > +----+---------+------+
   > | id | host_id | tag  |
   > +----+---------+------+
   > | 29 |       4 | tag1 |
   > | 30 |       4 | tag2 |
   > +----+---------+------+
   > 2 rows in set (0.00 sec)
   > ```
   > 
   > (2) vm can be starteded on a host which does not have all tags.
   > for example, service offering has host tags "tag1,tag2", and host has host tags "tag1,tag1".
   > 
   > @DK101010 could you please fix the bugs above ?
   
   Hi @weizhouapache, many thanks for testing. I'm a little bit confused, the logic doesn't use the list host method to find the right one. I wrote an sql to check this. Also we use this feature already for a long time without problems and I wrote test for this to get sure of this :/. Hmm ... how do you test 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-unsubscribe@cloudstack.apache.org

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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   @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] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


   Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2496


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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


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

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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4399: PR multi tags in host offering [#4398]

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


   @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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4399: PR multi tags in host offering [#4398]

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


   @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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


   Packaging result: ✖centos7 ✖centos8 ✖debian. JID-2758


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



[GitHub] [cloudstack] DaanHoogland edited a comment on pull request #4399: PR multi tags in host offering [#4398]

Posted by GitBox <gi...@apache.org>.
DaanHoogland edited a comment on pull request #4399:
URL: https://github.com/apache/cloudstack/pull/4399#issuecomment-744527004


   not sure yet, but the build failures are about certificates and i don't think related, but as a matter of good accounting:
   ```
   [INFO] Running org.apache.cloudstack.direct.download.DirectDownloadManagerImplTest
   java.security.cert.CertificateException: Could not parse certificate: java.io.IOException: java.lang.IllegalArgumentException: Input byte array has incorrect ending byte at 1132
   	at java.base/sun.security.provider.X509Factory.engineGenerateCertificate(X509Factory.java:115)
   	at java.base/java.security.cert.CertificateFactory.generateCertificate(CertificateFactory.java:355)
   	at com.cloud.utils.security.CertificateHelper.buildCertificate(CertificateHelper.java:130)
   	at org.apache.cloudstack.direct.download.DirectDownloadManagerImpl.getCertificateFromString(DirectDownloadManagerImpl.java:378)
   	at org.apache.cloudstack.direct.download.DirectDownloadManagerImpl.certificateSanity(DirectDownloadManagerImpl.java:389)
   	at org.apache.cloudstack.direct.download.DirectDownloadManagerImpl$MockitoMock$369926905.certificateSanity$accessor$rf85lrMB(Unknown Source)
   	at org.apache.cloudstack.direct.download.DirectDownloadManagerImpl$MockitoMock$369926905$auxiliary$xgeKzGvM.call(Unknown Source)
   	at org.mockito.internal.invocation.RealMethod$FromCallable$1.call(RealMethod.java:40)
   	at org.mockito.internal.invocation.RealMethod$FromBehavior.invoke(RealMethod.java:62)
   	at org.mockito.internal.invocation.InterceptedInvocation.callRealMethod(InterceptedInvocation.java:152)
   	at org.mockito.internal.stubbing.answers.CallsRealMethods.answer(CallsRealMethods.java:44)
   	at org.mockito.Answers.answer(Answers.java:100)
   	at org.mockito.internal.handler.MockHandlerImpl.handle(MockHandlerImpl.java:103)
   	at org.mockito.internal.handler.NullResultGuardian.handle(NullResultGuardian.java:29)
   	at org.mockito.internal.handler.InvocationNotifierHandler.handle(InvocationNotifierHandler.java:35)
   	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor.doIntercept(MockMethodInterceptor.java:61)
   	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor.doIntercept(MockMethodInterceptor.java:49)
   	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor$DispatcherDefaultingToRealMethod.interceptSuperCallable(MockMethodInterceptor.java:108)
   	at org.apache.cloudstack.direct.download.DirectDownloadManagerImpl$MockitoMock$369926905.certificateSanity(Unknown Source)
   	at org.apache.cloudstack.direct.download.DirectDownloadManagerImplTest.testCertificateSanityInvalidCertificate(DirectDownloadManagerImplTest.java:137)
   	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
   	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
   	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
   	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
   	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
   	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
   	at org.junit.internal.runners.statements.ExpectException.evaluate(ExpectException.java:19)
   	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
   	at org.mockito.internal.runners.DefaultInternalRunner$1$1.evaluate(DefaultInternalRunner.java:46)
   	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
   	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
   	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
   	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
   	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
   	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
   	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
   	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
   	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
   	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
   	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
   	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
   	at org.mockito.internal.runners.DefaultInternalRunner$1.run(DefaultInternalRunner.java:77)
   	at org.mockito.internal.runners.DefaultInternalRunner.run(DefaultInternalRunner.java:83)
   	at org.mockito.internal.runners.StrictRunner.run(StrictRunner.java:39)
   	at org.mockito.junit.MockitoJUnitRunner.run(MockitoJUnitRunner.java:163)
   	at org.mockito.runners.MockitoJUnitRunner.run(MockitoJUnitRunner.java:54)
   	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:365)
   	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:273)
   	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:238)
   	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
   	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:384)
   	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:345)
   	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:126)
   	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)
   Caused by: java.io.IOException: java.lang.IllegalArgumentException: Input byte array has incorrect ending byte at 1132
   	at java.base/sun.security.provider.X509Factory.readOneBlock(X509Factory.java:648)
   	at java.base/sun.security.provider.X509Factory.engineGenerateCertificate(X509Factory.java:99)
   	... 55 more
   Caused by: java.lang.IllegalArgumentException: Input byte array has incorrect ending byte at 1132
   	at java.base/java.util.Base64$Decoder.decode0(Base64.java:771)
   	at java.base/java.util.Base64$Decoder.decode(Base64.java:535)
   	at java.base/sun.security.provider.X509Factory.readOneBlock(X509Factory.java:646)
   	... 56 more
   ```
   and in the build summary
   ```
   [ERROR] importUnmanagedInstanceTest(org.apache.cloudstack.vm.UnmanagedVMsManagerImplTest)  Time elapsed: 0.05 s  <<< ERROR!
   java.lang.NullPointerException
   	at org.apache.cloudstack.vm.UnmanagedVMsManagerImplTest.importUnmanagedInstanceTest(UnmanagedVMsManagerImplTest.java:372)
   ```


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



[GitHub] [cloudstack] DaanHoogland removed a comment on pull request #4399: PR multi tags in host offering [#4398]

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


   @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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


   Packaging result: ✖centos7 ✖centos8 ✖debian. JID-2168


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



[GitHub] [cloudstack] DK101010 commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   > > > @DK101010 yes, I agree it should not be allowed. but, since it is allowed in 4.15 and previous versions, it might already exists in some/few user environments that a host have a same tag multiple times (who knows...).
   > > > IMHO options are
   > > > (1) check and throw an exception (as you said) or silently ignoring the duplicated tags, and remove duplications in host_tags in next cloudstack upgrade, or
   > > > (2) change your sql to take the scenario into consideration.
   > > > I would go for (1), but it looks (2) is much easier to you.
   > > 
   > > 
   > > @weizhouapache I'm also for (1), it is cleaner. But we need also checks in the frontend for future actions to prevent doubles. Also I think a DB upgrade is not necessary. In my opinion it is for a user a small thing to fix this mistake. Especially as I can't imagine that there are so many duplicate tags in use.
   > 
   > @DK101010
   > if we choose (1), we should fix it in api/java code as well as gui.
   > 
   > IMHO DB upgrade is also required, as we need to remove the duplications in cloudstack upgrade (otherwise it needs user intervention to remove them from DB manually ). for example, we need to remove record with id=62 from below
   > 
   > ```
   > mysql> select * from host_tags;
   > +----+---------+------+
   > | id | host_id | tag  |
   > +----+---------+------+
   > | 61 |       4 | tag1 |
   > | 62 |       4 | tag1 |
   > +----+---------+------+
   > 2 rows in set (0.00 sec)
   > ```
   > 
   > it is not a small change. I confirm that we can set same storage tag two times on a primary storage as well.
   
   OK, I thought the user can change the tags over the UI.
   
   So we need additional 
       - Checks for UI Input  for Host and SO
       - Checks for API input for Host and SO
       - Exception for findHostByComputeOfferings
       - SQL Upgrade script
   


-- 
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 removed a comment on pull request #4399: PR multi tags in host offering [#4398]

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


   @blueorangutan test centos7 kvm-centos7 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.

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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


   <b>Trillian test result (tid-1193)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 60779 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4399-t1193-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
   Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
   Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
   Intermittent failure detected: /marvin/tests/smoke/test_nested_virtualization.py
   Intermittent failure detected: /marvin/tests/smoke/test_network_acl.py
   Intermittent failure detected: /marvin/tests/smoke/test_network.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Smoke tests completed. 77 look OK, 11 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestLoadBalance>:setup | `Error` | 0.00 | test_loadbalance.py
   test_list_clusters_metrics | `Error` | 1515.52 | test_metrics_api.py
   test_list_vms_metrics | `Error` | 0.13 | test_metrics_api.py
   ContextSuite context=TestNetworkACL>:setup | `Error` | 0.00 | test_network_acl.py
   test_delete_account | `Error` | 1511.03 | test_network.py
   test_delete_network_while_vm_on_it | `Error` | 1.10 | test_network.py
   test_deploy_vm_l2network | `Error` | 1.09 | test_network.py
   test_l2network_restart | `Error` | 2.15 | test_network.py
   ContextSuite context=TestPortForwarding>:setup | `Error` | 3.28 | test_network.py
   ContextSuite context=TestPublicIP>:setup | `Error` | 0.93 | test_network.py
   test_reboot_router | `Failure` | 0.04 | test_network.py
   test_releaseIP | `Error` | 0.40 | test_network.py
   ContextSuite context=TestRouterRules>:setup | `Error` | 0.43 | test_network.py
   test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_02_deploy_and_upgrade_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_04_basic_lifecycle_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_05_delete_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_06_deploy_invalid_kubernetes_ha_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
   test_01_add_delete_kubernetes_supported_version | `Error` | 60.57 | test_kubernetes_supported_versions.py
   ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
   test_nic_secondaryip_add_remove | `Error` | 1511.24 | test_multipleips_per_nic.py
   ContextSuite context=TestNestedVirtualization>:setup | `Error` | 0.00 | test_nested_virtualization.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Failure` | 338.29 | test_routers_network_ops.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 531.28 | test_vpc_redundant.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Error` | 531.29 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 540.46 | 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 #4399: PR multi tags in compute offering [#4398]

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


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


-- 
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 #4399: PR multi tags in compute offering [#4398]

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


   <b>Trillian test result (tid-1471)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 47094 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4399-t1471-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 86 look OK, 3 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` | 144.54 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Failure` | 355.92 | test_routers_network_ops.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 552.49 | test_vpc_redundant.py
   test_01_vpc_site2site_vpn_multiple_options | `Failure` | 589.62 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Failure` | 256.46 | test_vpc_vpn.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 #4399: PR multi tags in compute offering [#4398]

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


   @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] DK101010 commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   > @DK101010 yes, I agree it should not be allowed. but, since it is allowed in 4.15 and previous versions, it might already exists in some/few user environments that a host have a same tag multiple times (who knows...).
   > IMHO options are
   > (1) check and throw an exception (as you said) or silently ignoring the duplicated tags, and remove duplications in host_tags in next cloudstack upgrade, or
   > (2) change your sql to take the scenario into consideration.
   > I would go for (1), but it looks (2) is much easier to you.
   
   @weizhouapache I'm also for (1), it is cleaner. But we need also checks in the frontend for future actions to prevent doubles. Also I think  a DB upgrade is not necessary. In my opinion it is for a user a small thing to fix this mistake. Especially as I can't imagine that there are so many duplicate tags in use.


-- 
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 #4399: PR multi tags in host offering [#4398]

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


   @nvazquez 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 #4399: PR multi tags in host offering [#4398]

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


   Packaging result: ✖centos7 ✖centos8 ✖debian. JID-2484


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



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4399: PR multi tags in host offering [#4398]

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






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



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4399: PR multi tags in host offering [#4398]

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


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

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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


   <b>Trillian test result (tid-2965)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 38819 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4399-t2965-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 83 look OK, 2 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 309.17 | test_vpc_redundant.py
   test_hostha_kvm_host_fencing | `Error` | 172.30 | test_hostha_kvm.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.

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



[GitHub] [cloudstack] DaanHoogland removed a comment on pull request #4399: PR multi tags in host offering [#4398]

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


   @blueorangutan test centos7 kvm-centos7 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.

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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   @nvazquez 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] weizhouapache commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   > > found two bugs in testing
   > > (1) list hosts response lists only 1 host tag, although hosts have multiple host tags.
   > > for example
   > > ```
   > > (localcloud) SBCM5> > update host id=d9988a7b-7e02-4e90-9c4a-798c588b1cba hosttags=tag1,tag2
   > > {
   > >   "host": {
   > > ...
   > >     "hosttags": "tag2",
   > > ...
   > >   }
   > > }
   > > 
   > > (localcloud) SBCM5> > list hosts id=d9988a7b-7e02-4e90-9c4a-798c588b1cba filter=hosttags
   > > {
   > >   "count": 1,
   > >   "host": [
   > >     {
   > >       "hosttags": "tag2"
   > >     }
   > >   ]
   > > }
   > > ```
   > > 
   > > 
   > >     
   > >       
   > >     
   > > 
   > >       
   > >     
   > > 
   > >     
   > >   
   > > however in db, host has 2 tags.
   > > ```
   > > mysql> select * from host_tags;
   > > +----+---------+------+
   > > | id | host_id | tag  |
   > > +----+---------+------+
   > > | 29 |       4 | tag1 |
   > > | 30 |       4 | tag2 |
   > > +----+---------+------+
   > > 2 rows in set (0.00 sec)
   > > ```
   > > 
   > > 
   > >     
   > >       
   > >     
   > > 
   > >       
   > >     
   > > 
   > >     
   > >   
   > > (2) vm can be starteded on a host which does not have all tags.
   > > for example, service offering has host tags "tag1,tag2", and host has host tags "tag1,tag1".
   > > @DK101010 could you please fix the bugs above ?
   > 
   > Hi @weizhouapache, many thanks for testing. I'm a little bit confused, the logic doesn't use the list host method to find the right one. I wrote an sql to check this. Also we use this feature already for a long time without problems and I wrote test for this to get sure of this :/. Hmm ... how do you test this?
   
   @DK101010 
   ignore the first issue, I found the root cause: I upgraded the cloudstack packages but did not make the changes in #4796
   
   for 2nd issue, I use cmk for testing
   (1) update host to tag 'tag1,tag1'
   (2) create service offering, update host tag to 'tag1,tag2'
   (3) create vm with offering in (2)
   (4) start vm on the host in (1).
   
   expected result: vm is not started on host (1)
   actual result: vm is started on host (1) successfully.
   
   root cause: yo do not consider that host might have a tag multiple times. so you need to change the following sql
   ```
       private static final String LIST_HOST_IDS_BY_COMPUTETAGS = "SELECT host_id, COUNT(tag) AS tag_count "
                                                                + "FROM host_tags "
                                                                + "WHERE tag IN(%s) "
                                                                + "GROUP BY host_id "
                                                                + "HAVING tag_count = %s ";
   ```
   https://github.com/apache/cloudstack/pull/4399/files#diff-ced268870eedefd01befd033d2544228bf010c0dccde9c76ac45f80652e503b8R84-R88
   


-- 
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 removed a comment on pull request #4399: PR multi tags in host offering [#4398]

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


   @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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4399: PR multi tags in host offering [#4398]

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


   @blueorangutan test centos7 kvm-centos7 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.

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



[GitHub] [cloudstack] DaanHoogland removed a comment on pull request #4399: PR multi tags in host offering [#4398]

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






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



[GitHub] [cloudstack] DK101010 commented on pull request #4399: PR multi tags in host offering [#4398]

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


   Hi Dahn,
   
   when you talk about automation tests then mean you marvin and similar
   things, correct? I'm a newbie in cloudstack and haven't experience with
   marvin. I can try it but I think it will need some time.
   
   Exists next to this side
   https://cwiki.apache.org/confluence/display/CLOUDSTACK/Marvin+-+Testing+with+Python
   other useful documents for the first steps?
   
   
   Am Do., 12. Nov. 2020 um 13:08 Uhr schrieb dahn <no...@github.com>:
   
   > @DV101010 can you invest some time in an automated test?
   >
   > —
   > You are receiving this because you authored the thread.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/cloudstack/pull/4399#issuecomment-726038488>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/ANW3U4XYLKCT4HIUIO7V7HDSPPF33ANCNFSM4SOGQP3Q>
   > .
   >
   


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



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4399: PR multi tags in host offering [#4398]

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



##########
File path: engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java
##########
@@ -480,6 +482,25 @@ public Integer countAllByTypeInZone(long zoneId, Type type) {
         return listBy(sc);
     }
 
+    @Override
+    public boolean checkHostServiceOfferingTags(HostVO host, ServiceOffering serviceOffering){
+        if (host == null) {
+            return false;
+        }
+        if (serviceOffering == null) {
+            return false;
+        }
+        if (Strings.isNullOrEmpty(serviceOffering.getHostTag())) {
+            return true;
+        }
+
+        List<String> serviceOfferingTags = Arrays.asList(serviceOffering.getHostTag().split(","));
+        if(host.getHostTags() != null && host.getHostTags().containsAll(serviceOfferingTags)){
+            return true;
+        }
+        return false;
+    }

Review comment:
       will you be moving this method @DK101010 ?




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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


   Packaging result: ✖centos7 ✔centos8 ✔debian. JID-2740


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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


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

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



[GitHub] [cloudstack] DK101010 commented on pull request #4399: PR multi tags in host offering [#4398]

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


   > @DK101010 can you explore the previous failures, I'll rekick packaging too
   > @blueorangutan package
   
   Hi @rhtyd, I can do it but i don't see any failure. Which do you mean ? 


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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


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

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



[GitHub] [cloudstack] weizhouapache commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   @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] blueorangutan commented on pull request #4399: PR multi tags in compute offering [#4398]

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


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


-- 
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 #4399: PR multi tags in compute offering [#4398]

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


   <b>Trillian test result (tid-1651)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 34086 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4399-t1651-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] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


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

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



[GitHub] [cloudstack] weizhouapache commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   > > > > @DK101010 yes, I agree it should not be allowed. but, since it is allowed in 4.15 and previous versions, it might already exists in some/few user environments that a host have a same tag multiple times (who knows...).
   > > > > IMHO options are
   > > > > (1) check and throw an exception (as you said) or silently ignoring the duplicated tags, and remove duplications in host_tags in next cloudstack upgrade, or
   > > > > (2) change your sql to take the scenario into consideration.
   > > > > I would go for (1), but it looks (2) is much easier to you.
   > > > 
   > > > 
   > > > @weizhouapache I'm also for (1), it is cleaner. But we need also checks in the frontend for future actions to prevent doubles. Also I think a DB upgrade is not necessary. In my opinion it is for a user a small thing to fix this mistake. Especially as I can't imagine that there are so many duplicate tags in use.
   > > 
   > > 
   > > @DK101010
   > > if we choose (1), we should fix it in api/java code as well as gui.
   > > IMHO DB upgrade is also required, as we need to remove the duplications in cloudstack upgrade (otherwise it needs user intervention to remove them from DB manually ). for example, we need to remove record with id=62 from below
   > > ```
   > > mysql> select * from host_tags;
   > > +----+---------+------+
   > > | id | host_id | tag  |
   > > +----+---------+------+
   > > | 61 |       4 | tag1 |
   > > | 62 |       4 | tag1 |
   > > +----+---------+------+
   > > 2 rows in set (0.00 sec)
   > > ```
   > > 
   > > 
   > >     
   > >       
   > >     
   > > 
   > >       
   > >     
   > > 
   > >     
   > >   
   > > it is not a small change. I confirm that we can set same storage tag two times on a primary storage as well.
   > 
   > OK, I thought the user can change the tags over the UI.
   > 
   > So we need additional
   > - Checks for UI Input for Host and SO
   > - Checks for API input for Host and SO
   > - Exception for findHostByComputeOfferings
   > - SQL Upgrade script
   
   @DK101010 yes, it looks a bit complicated. I suggest you to change SQL in this pr as 1st step.


-- 
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 removed a comment on pull request #4399: PR multi tags in host offering [#4398]

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






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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


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

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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


   @DaanHoogland a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) 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.

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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


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

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



[GitHub] [cloudstack] nvazquez commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   @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 #4399: PR multi tags in compute offering [#4398]

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


   > > @DK101010 yes, I agree it should not be allowed. but, since it is allowed in 4.15 and previous versions, it might already exists in some/few user environments that a host have a same tag multiple times (who knows...).
   > > IMHO options are
   > > (1) check and throw an exception (as you said) or silently ignoring the duplicated tags, and remove duplications in host_tags in next cloudstack upgrade, or
   > > (2) change your sql to take the scenario into consideration.
   > > I would go for (1), but it looks (2) is much easier to you.
   > 
   > @weizhouapache I'm also for (1), it is cleaner. But we need also checks in the frontend for future actions to prevent doubles. Also I think a DB upgrade is not necessary. In my opinion it is for a user a small thing to fix this mistake. Especially as I can't imagine that there are so many duplicate tags in use.
   
   @DK101010 
   if we choose (1), we should fix it in api/java code as well as gui.
   
   IMHO DB upgrade is also required, as we need to remove the duplications in cloudstack upgrade (otherwise it needs user intervention to remove them from DB manually ). for example, we need to remove record with id=62 from below
   ```
   mysql> select * from host_tags;
   +----+---------+------+
   | id | host_id | tag  |
   +----+---------+------+
   | 61 |       4 | tag1 |
   | 62 |       4 | tag1 |
   +----+---------+------+
   2 rows in set (0.00 sec)
   ```
   
   it is not a small change. I confirm that we can set same storage tag two times on a primary storage as well.
   


-- 
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 #4399: PR multi tags in host offering [#4398]

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


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

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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4399: PR multi tags in host offering [#4398]

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


   @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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


   Packaging result: ✖centos7 ✖centos8 ✖debian. JID-2494


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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4399: PR multi tags in host offering [#4398]

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


   regression tests look good, kubernetes is a false positive and travis a timeout on one of the runs.


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



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4399: PR multi tags in host offering [#4398]

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



##########
File path: engine/schema/src/main/java/com/cloud/host/HostVO.java
##########
@@ -740,6 +743,18 @@ public void setUuid(String uuid) {
         this.uuid = uuid;
     }
 
+    public boolean checkHostServiceOfferingTags(ServiceOffering serviceOffering){
+        if (serviceOffering == null) {
+            return false;
+        }
+        if (Strings.isNullOrEmpty(serviceOffering.getHostTag())) {

Review comment:
       sorry to keep nagging you, but we have a string utils proxy class in place. It has an isEmpty() that does the same check. if for some reason you need this one anyway , please add it there if the functioonality doesn't suffice.




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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4399: PR multi tags in host offering [#4398]

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


   @blueorangutan test centos7 kvm-centos7 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.

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



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4399: PR multi tags in host offering [#4398]

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


   Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2173


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



[GitHub] [cloudstack] DK101010 commented on pull request #4399: PR multi tags in host offering [#4398]

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


   Hi @DaanHoogland,
   I have added a marvin test and tried to implement a generic solution, but a couple of things must be adapt like host, network name and hypervisor as well I couldn't found a option to test the ingest feature with marvin. 
   
   Let me know if I can improve the 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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4399: PR multi tags in host offering [#4398]

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


   test failures are not related to the 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



[GitHub] [cloudstack] DK101010 commented on a change in pull request #4399: PR multi tags in host offering [#4398]

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



##########
File path: engine/schema/src/main/java/com/cloud/host/HostVO.java
##########
@@ -740,6 +743,18 @@ public void setUuid(String uuid) {
         this.uuid = uuid;
     }
 
+    public boolean checkHostServiceOfferingTags(ServiceOffering serviceOffering){
+        if (serviceOffering == null) {
+            return false;
+        }
+        if (Strings.isNullOrEmpty(serviceOffering.getHostTag())) {

Review comment:
       @DaanHoogland Is ok, I think I will survive this :D
   This method was not my idea, it was a part of the old implementation and I thought it was nice, so I reused this.
   
   I will change it ;)




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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   @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] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


   Packaging result: ✖centos7 ✖centos8 ✖debian. JID-2733


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



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4399: PR multi tags in host offering [#4398]

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



##########
File path: engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java
##########
@@ -1147,27 +1155,68 @@ public HostVO findByIp(final String ipAddress) {
     }
 
     @Override
-    public List<Long> listClustersByHostTag(String hostTagOnOffering) {
+    public List<Long> listClustersByHostTag(String computeOfferingTags) {
         TransactionLegacy txn = TransactionLegacy.currentTxn();
+        String sql = this.LIST_CLUSTERID_FOR_HOST_TAG;
         PreparedStatement pstmt = null;
-        List<Long> result = new ArrayList<Long>();
-        StringBuilder sql = new StringBuilder(LIST_CLUSTERID_FOR_HOST_TAG);
-        // during listing the clusters that cross the threshold
-        // we need to check with disabled thresholds of each cluster if not defined at cluster consider the global value
+        List<Long> result = new ArrayList();
+        List<String> tags = Arrays.asList(computeOfferingTags.split(this.SEPARATOR));
+        String subselect = getHostIdsByComputeTags(tags);
+        sql = String.format(sql, subselect);
+        
         try {
-            pstmt = txn.prepareAutoCloseStatement(sql.toString());
-            pstmt.setString(1, hostTagOnOffering);
+            pstmt = txn.prepareStatement(sql);
+            
+            for(int i = 0; i < tags.size(); i++){
+                pstmt.setString(i+1, tags.get(i));
+            }
+            //pstmt = txn.prepareAutoCloseStatement();
             ResultSet rs = pstmt.executeQuery();
-            while (rs.next()) {
-                result.add(rs.getLong(1));
+            while (rs.next()) {     
+                result.add(rs.getLong(1));             
+            }        

Review comment:
       trailing white spce on these lines
   ```suggestion
               while (rs.next()) {
                   result.add(rs.getLong(1));
               }
   ```

##########
File path: engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java
##########
@@ -1147,27 +1155,68 @@ public HostVO findByIp(final String ipAddress) {
     }
 
     @Override
-    public List<Long> listClustersByHostTag(String hostTagOnOffering) {
+    public List<Long> listClustersByHostTag(String computeOfferingTags) {
         TransactionLegacy txn = TransactionLegacy.currentTxn();
+        String sql = this.LIST_CLUSTERID_FOR_HOST_TAG;
         PreparedStatement pstmt = null;
-        List<Long> result = new ArrayList<Long>();
-        StringBuilder sql = new StringBuilder(LIST_CLUSTERID_FOR_HOST_TAG);
-        // during listing the clusters that cross the threshold
-        // we need to check with disabled thresholds of each cluster if not defined at cluster consider the global value
+        List<Long> result = new ArrayList();
+        List<String> tags = Arrays.asList(computeOfferingTags.split(this.SEPARATOR));
+        String subselect = getHostIdsByComputeTags(tags);
+        sql = String.format(sql, subselect);
+        
         try {
-            pstmt = txn.prepareAutoCloseStatement(sql.toString());
-            pstmt.setString(1, hostTagOnOffering);
+            pstmt = txn.prepareStatement(sql);
+            
+            for(int i = 0; i < tags.size(); i++){
+                pstmt.setString(i+1, tags.get(i));
+            }
+            //pstmt = txn.prepareAutoCloseStatement();
             ResultSet rs = pstmt.executeQuery();
-            while (rs.next()) {
-                result.add(rs.getLong(1));
+            while (rs.next()) {     
+                result.add(rs.getLong(1));             
+            }        
+            pstmt.close();
+            if(result.isEmpty()){
+                throw new CloudRuntimeException("No suitable host found for follow compute offering tags: " + computeOfferingTags);
             }
             return result;
         } catch (SQLException e) {
             throw new CloudRuntimeException("DB Exception on: " + sql, e);
-        } catch (Throwable e) {
-            throw new CloudRuntimeException("Caught: " + sql, e);
         }
     }
+    
+    private List<Long> findHostByComputeOfferings(String computeOfferingTags){
+        TransactionLegacy txn = TransactionLegacy.currentTxn();
+        PreparedStatement pstmt = null;
+        List<Long> result = new ArrayList();
+        List<String> tags = Arrays.asList(computeOfferingTags.split(this.SEPARATOR));
+        String select = getHostIdsByComputeTags(tags);
+        try {
+            pstmt = txn.prepareStatement(select);
+            
+            for(int i = 0; i < tags.size(); i++){
+                pstmt.setString(i+1, tags.get(i));
+            }
+            
+            ResultSet rs = pstmt.executeQuery();
+            while (rs.next()) {     
+                result.add(rs.getLong(1));             

Review comment:
       ```suggestion
               while (rs.next()) {
                   result.add(rs.getLong(1));
   ```




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



[GitHub] [cloudstack] DK101010 commented on a change in pull request #4399: PR multi tags in host offering [#4398]

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



##########
File path: engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java
##########
@@ -1147,27 +1155,68 @@ public HostVO findByIp(final String ipAddress) {
     }
 
     @Override
-    public List<Long> listClustersByHostTag(String hostTagOnOffering) {
+    public List<Long> listClustersByHostTag(String computeOfferingTags) {
         TransactionLegacy txn = TransactionLegacy.currentTxn();
+        String sql = this.LIST_CLUSTERID_FOR_HOST_TAG;
         PreparedStatement pstmt = null;
-        List<Long> result = new ArrayList<Long>();
-        StringBuilder sql = new StringBuilder(LIST_CLUSTERID_FOR_HOST_TAG);
-        // during listing the clusters that cross the threshold
-        // we need to check with disabled thresholds of each cluster if not defined at cluster consider the global value
+        List<Long> result = new ArrayList();
+        List<String> tags = Arrays.asList(computeOfferingTags.split(this.SEPARATOR));
+        String subselect = getHostIdsByComputeTags(tags);
+        sql = String.format(sql, subselect);
+        
         try {
-            pstmt = txn.prepareAutoCloseStatement(sql.toString());
-            pstmt.setString(1, hostTagOnOffering);
+            pstmt = txn.prepareStatement(sql);
+            
+            for(int i = 0; i < tags.size(); i++){
+                pstmt.setString(i+1, tags.get(i));
+            }
+            //pstmt = txn.prepareAutoCloseStatement();
             ResultSet rs = pstmt.executeQuery();
-            while (rs.next()) {
-                result.add(rs.getLong(1));
+            while (rs.next()) {     
+                result.add(rs.getLong(1));             
+            }        

Review comment:
       ups sorry, i've fixed 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.

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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   @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 pull request #4399: PR multi tags in host offering [#4398]

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


   @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



[GitHub] [cloudstack] DK101010 commented on a change in pull request #4399: PR multi tags in host offering [#4398]

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



##########
File path: engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java
##########
@@ -480,6 +482,25 @@ public Integer countAllByTypeInZone(long zoneId, Type type) {
         return listBy(sc);
     }
 
+    @Override
+    public boolean checkHostServiceOfferingTags(HostVO host, ServiceOffering serviceOffering){
+        if (host == null) {
+            return false;
+        }
+        if (serviceOffering == null) {
+            return false;
+        }
+        if (Strings.isNullOrEmpty(serviceOffering.getHostTag())) {
+            return true;
+        }
+
+        List<String> serviceOfferingTags = Arrays.asList(serviceOffering.getHostTag().split(","));
+        if(host.getHostTags() != null && host.getHostTags().containsAll(serviceOfferingTags)){
+            return true;
+        }
+        return false;
+    }

Review comment:
       is moved ;)




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



[GitHub] [cloudstack] DK101010 commented on pull request #4399: PR multi tags in host offering [#4398]

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


   Hi, 
   yesterday we had a problem during our deployment and I've found a additional tag check in the DeploymentPlanningManager. To prevent code duplicates I've encapsulate tag handling in HostDaoImpl. But I afraid I can not write test for this because we use currently a older version. 


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



[GitHub] [cloudstack] weizhouapache commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   > > > > found two bugs in testing
   > > > > (1) list hosts response lists only 1 host tag, although hosts have multiple host tags.
   > > > > for example
   > > > > ```
   > > > > (localcloud) SBCM5> > update host id=d9988a7b-7e02-4e90-9c4a-798c588b1cba hosttags=tag1,tag2
   > > > > {
   > > > >   "host": {
   > > > > ...
   > > > >     "hosttags": "tag2",
   > > > > ...
   > > > >   }
   > > > > }
   > > > > 
   > > > > (localcloud) SBCM5> > list hosts id=d9988a7b-7e02-4e90-9c4a-798c588b1cba filter=hosttags
   > > > > {
   > > > >   "count": 1,
   > > > >   "host": [
   > > > >     {
   > > > >       "hosttags": "tag2"
   > > > >     }
   > > > >   ]
   > > > > }
   > > > > ```
   > > > > 
   > > > > 
   > > > >     
   > > > >       
   > > > >     
   > > > > 
   > > > >       
   > > > >     
   > > > > 
   > > > >     
   > > > >   
   > > > > however in db, host has 2 tags.
   > > > > ```
   > > > > mysql> select * from host_tags;
   > > > > +----+---------+------+
   > > > > | id | host_id | tag  |
   > > > > +----+---------+------+
   > > > > | 29 |       4 | tag1 |
   > > > > | 30 |       4 | tag2 |
   > > > > +----+---------+------+
   > > > > 2 rows in set (0.00 sec)
   > > > > ```
   > > > > 
   > > > > 
   > > > >     
   > > > >       
   > > > >     
   > > > > 
   > > > >       
   > > > >     
   > > > > 
   > > > >     
   > > > >   
   > > > > (2) vm can be starteded on a host which does not have all tags.
   > > > > for example, service offering has host tags "tag1,tag2", and host has host tags "tag1,tag1".
   > > > > @DK101010 could you please fix the bugs above ?
   > > > 
   > > > 
   > > > Hi @weizhouapache, many thanks for testing. I'm a little bit confused, the logic doesn't use the list host method to find the right one. I wrote an sql to check this. Also we use this feature already for a long time without problems and I wrote test for this to get sure of this :/. Hmm ... how do you test this?
   > > 
   > > 
   > > @DK101010
   > > ignore the first issue, I found the root cause: I upgraded the cloudstack packages but did not make the changes in #4796
   > > for 2nd issue, I use cmk for testing
   > > (1) update host to tag 'tag1,tag1'
   > > (2) create service offering, update host tag to 'tag1,tag2'
   > > (3) create vm with offering in (2)
   > > (4) start vm on the host in (1).
   > > expected result: vm is not started on host (1)
   > > actual result: vm is started on host (1) successfully.
   > > root cause: yo do not consider that host might have a tag multiple times. so you need to change the following sql
   > > ```
   > >     private static final String LIST_HOST_IDS_BY_COMPUTETAGS = "SELECT host_id, COUNT(tag) AS tag_count "
   > >                                                              + "FROM host_tags "
   > >                                                              + "WHERE tag IN(%s) "
   > >                                                              + "GROUP BY host_id "
   > >                                                              + "HAVING tag_count = %s ";
   > > ```
   
   > Hi @weizhouapache, You mean the same tag two times? Hmm .... sounds more like a user input error. What do you think about a exception when user add the same tag in host or service offerings or we add here a check for hosts and service offering ?
   
   @DK101010 yes, I agree it should not be allowed. but, since it is allowed in 4.15 and previous versions, it might already exists in some/few user environments that a host have a same tag multiple times (who knows...). 
   IMHO options are
   (1) check and throw an exception (as you said) or silently ignoring the duplicated tags, and remove duplications in host_tags in next cloudstack upgrade, or
   (2) change your sql to take the scenario into consideration.
   I would go for (1), but it looks (2) is much easier to you.


-- 
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 #4399: PR multi tags in compute offering [#4398]

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


   Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian :heavy_multiplication_x: suse15. SL-JID 740


-- 
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 #4399: PR multi tags in host offering [#4398]

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


   @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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


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

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



[GitHub] [cloudstack] shwstppr commented on pull request #4399: PR multi tags in host offering [#4398]

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


   @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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


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

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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   @nvazquez 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 #4399: PR multi tags in host offering [#4398]

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


   <b>Trillian test result (tid-3316)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 71104 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4399-t3316-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_public_ip_range.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_secondary_storage.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermittent failure detected: /marvin/tests/smoke/test_storage_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_templates.py
   Intermittent failure detected: /marvin/tests/smoke/test_usage.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 63 look OK, 23 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 238.58 | test_kubernetes_clusters.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRAMCPUResourceAccounting>:setup | `Error` | 0.00 | test_resource_accounting.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   ContextSuite context=TestRouterIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestVPCIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestIsolatedNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRedundantIsolateNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   test_01_sys_vm_start | `Failure` | 0.16 | test_secondary_storage.py
   ContextSuite context=TestCpuCapServiceOfferings>:setup | `Error` | 0.00 | test_service_offerings.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 0.19 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   test_01_list_sec_storage_vm | `Failure` | 0.05 | test_ssvm.py
   test_02_list_cpvm_vm | `Failure` | 0.05 | test_ssvm.py
   test_03_ssvm_internals | `Failure` | 0.04 | test_ssvm.py
   test_04_cpvm_internals | `Failure` | 0.04 | test_ssvm.py
   test_05_stop_ssvm | `Failure` | 0.04 | test_ssvm.py
   test_06_stop_cpvm | `Failure` | 0.04 | test_ssvm.py
   test_07_reboot_ssvm | `Failure` | 0.04 | test_ssvm.py
   test_08_reboot_cpvm | `Failure` | 0.04 | test_ssvm.py
   test_09_destroy_ssvm | `Failure` | 0.04 | test_ssvm.py
   test_10_destroy_cpvm | `Failure` | 0.04 | test_ssvm.py
   ContextSuite context=TestVMWareStoragePolicies>:setup | `Error` | 0.00 | test_storage_policy.py
   test_02_create_template_with_checksum_sha1 | `Error` | 65.54 | test_templates.py
   test_03_create_template_with_checksum_sha256 | `Error` | 65.52 | test_templates.py
   test_04_create_template_with_checksum_md5 | `Error` | 65.53 | test_templates.py
   test_05_create_template_with_no_checksum | `Error` | 65.54 | test_templates.py
   test_02_deploy_vm_from_direct_download_template | `Error` | 1.29 | test_templates.py
   test_03_deploy_vm_wrong_checksum | `Error` | 1.33 | test_templates.py
   ContextSuite context=TestTemplates>:setup | `Error` | 17.44 | test_templates.py
   ContextSuite context=TestISOUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestLBRuleUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestNatRuleUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestPublicIPUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestSnapshotUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVmUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVolumeUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVpnUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=Test01DeployVM>:setup | `Error` | 0.00 | test_vm_life_cycle.py
   ContextSuite context=Test02VMLifeCycle>:setup | `Error` | 0.00 | test_vm_life_cycle.py
   test_14_secure_to_secure_vm_migration | `Error` | 11.41 | test_vm_life_cycle.py
   test_15_secured_to_nonsecured_vm_migration | `Error` | 74.06 | test_vm_life_cycle.py
   test_16_nonsecured_to_secured_vm_migration | `Error` | 1.27 | test_vm_life_cycle.py
   ContextSuite context=TestVmSnapshot>:setup | `Error` | 1.95 | test_vm_snapshots.py
   ContextSuite context=TestCreateVolume>:setup | `Error` | 0.00 | test_volumes.py
   ContextSuite context=TestVolumes>:setup | `Error` | 0.00 | test_volumes.py
   ContextSuite context=TestVPCRedundancy>:setup | `Error` | 0.00 | test_vpc_redundant.py
   ContextSuite context=TestVPCNics>:setup | `Error` | 0.00 | test_vpc_router_nics.py
   ContextSuite context=TestRVPCSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVPCSite2SiteVPNMultipleOptions>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVpcRemoteAccessVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVpcSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   test_disable_oobm_ha_state_ineligible | `Error` | 1514.47 | test_hostha_kvm.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.

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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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






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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


   Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2173


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



[GitHub] [cloudstack] weizhouapache closed pull request #4399: PR multi tags in compute offering [#4398]

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


   


-- 
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 #4399: PR multi tags in compute offering [#4398]

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


   tested ok


-- 
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 #4399: PR multi tags in host offering [#4398]

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


   <b>Trillian test result (tid-3307)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 39356 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4399-t3307-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Smoke tests completed. 85 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 254.21 | test_kubernetes_clusters.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.

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



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4399: PR multi tags in host offering [#4398]

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



##########
File path: engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java
##########
@@ -480,6 +482,25 @@ public Integer countAllByTypeInZone(long zoneId, Type type) {
         return listBy(sc);
     }
 
+    @Override
+    public boolean checkHostServiceOfferingTags(HostVO host, ServiceOffering serviceOffering){
+        if (host == null) {
+            return false;
+        }
+        if (serviceOffering == null) {
+            return false;
+        }
+        if (Strings.isNullOrEmpty(serviceOffering.getHostTag())) {
+            return true;
+        }
+
+        List<String> serviceOfferingTags = Arrays.asList(serviceOffering.getHostTag().split(","));
+        if(host.getHostTags() != null && host.getHostTags().containsAll(serviceOfferingTags)){
+            return true;
+        }
+        return false;
+    }

Review comment:
       as said I prefer a default method on the interface but it maight require undesirable imports so HostVO is good for my part.




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



[GitHub] [cloudstack] DaanHoogland removed a comment on pull request #4399: PR multi tags in host offering [#4398]

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






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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4399: PR multi tags in host offering [#4398]

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


   @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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4399: PR multi tags in host offering [#4398]

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


   @blueorangutan test matrix


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



[GitHub] [cloudstack] DK101010 commented on pull request #4399: PR multi tags in host offering [#4398]

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


   > not sure yet, but the build failures are about certificates and i don't think related, but as a matter of good accounting:
   > 
   > ```
   > [INFO] Running org.apache.cloudstack.direct.download.DirectDownloadManagerImplTest
   > java.security.cert.CertificateException: Could not parse certificate: java.io.IOException: java.lang.IllegalArgumentException: Input byte array has incorrect ending byte at 1132
   > 	at java.base/sun.security.provider.X509Factory.engineGenerateCertificate(X509Factory.java:115)
   > 	at java.base/java.security.cert.CertificateFactory.generateCertificate(CertificateFactory.java:355)
   > 	at com.cloud.utils.security.CertificateHelper.buildCertificate(CertificateHelper.java:130)
   > 	at org.apache.cloudstack.direct.download.DirectDownloadManagerImpl.getCertificateFromString(DirectDownloadManagerImpl.java:378)
   > 	at org.apache.cloudstack.direct.download.DirectDownloadManagerImpl.certificateSanity(DirectDownloadManagerImpl.java:389)
   > 	at org.apache.cloudstack.direct.download.DirectDownloadManagerImpl$MockitoMock$369926905.certificateSanity$accessor$rf85lrMB(Unknown Source)
   > 	at org.apache.cloudstack.direct.download.DirectDownloadManagerImpl$MockitoMock$369926905$auxiliary$xgeKzGvM.call(Unknown Source)
   > 	at org.mockito.internal.invocation.RealMethod$FromCallable$1.call(RealMethod.java:40)
   > 	at org.mockito.internal.invocation.RealMethod$FromBehavior.invoke(RealMethod.java:62)
   > 	at org.mockito.internal.invocation.InterceptedInvocation.callRealMethod(InterceptedInvocation.java:152)
   > 	at org.mockito.internal.stubbing.answers.CallsRealMethods.answer(CallsRealMethods.java:44)
   > 	at org.mockito.Answers.answer(Answers.java:100)
   > 	at org.mockito.internal.handler.MockHandlerImpl.handle(MockHandlerImpl.java:103)
   > 	at org.mockito.internal.handler.NullResultGuardian.handle(NullResultGuardian.java:29)
   > 	at org.mockito.internal.handler.InvocationNotifierHandler.handle(InvocationNotifierHandler.java:35)
   > 	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor.doIntercept(MockMethodInterceptor.java:61)
   > 	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor.doIntercept(MockMethodInterceptor.java:49)
   > 	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor$DispatcherDefaultingToRealMethod.interceptSuperCallable(MockMethodInterceptor.java:108)
   > 	at org.apache.cloudstack.direct.download.DirectDownloadManagerImpl$MockitoMock$369926905.certificateSanity(Unknown Source)
   > 	at org.apache.cloudstack.direct.download.DirectDownloadManagerImplTest.testCertificateSanityInvalidCertificate(DirectDownloadManagerImplTest.java:137)
   > 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   > 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
   > 	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   > 	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
   > 	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
   > 	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
   > 	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
   > 	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
   > 	at org.junit.internal.runners.statements.ExpectException.evaluate(ExpectException.java:19)
   > 	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
   > 	at org.mockito.internal.runners.DefaultInternalRunner$1$1.evaluate(DefaultInternalRunner.java:46)
   > 	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
   > 	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
   > 	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
   > 	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
   > 	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
   > 	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
   > 	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
   > 	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
   > 	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
   > 	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
   > 	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
   > 	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
   > 	at org.mockito.internal.runners.DefaultInternalRunner$1.run(DefaultInternalRunner.java:77)
   > 	at org.mockito.internal.runners.DefaultInternalRunner.run(DefaultInternalRunner.java:83)
   > 	at org.mockito.internal.runners.StrictRunner.run(StrictRunner.java:39)
   > 	at org.mockito.junit.MockitoJUnitRunner.run(MockitoJUnitRunner.java:163)
   > 	at org.mockito.runners.MockitoJUnitRunner.run(MockitoJUnitRunner.java:54)
   > 	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:365)
   > 	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:273)
   > 	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:238)
   > 	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
   > 	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:384)
   > 	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:345)
   > 	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:126)
   > 	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)
   > Caused by: java.io.IOException: java.lang.IllegalArgumentException: Input byte array has incorrect ending byte at 1132
   > 	at java.base/sun.security.provider.X509Factory.readOneBlock(X509Factory.java:648)
   > 	at java.base/sun.security.provider.X509Factory.engineGenerateCertificate(X509Factory.java:99)
   > 	... 55 more
   > Caused by: java.lang.IllegalArgumentException: Input byte array has incorrect ending byte at 1132
   > 	at java.base/java.util.Base64$Decoder.decode0(Base64.java:771)
   > 	at java.base/java.util.Base64$Decoder.decode(Base64.java:535)
   > 	at java.base/sun.security.provider.X509Factory.readOneBlock(X509Factory.java:646)
   > 	... 56 more
   > ```
   > 
   > and in the build summary
   > 
   > ```
   > [ERROR] importUnmanagedInstanceTest(org.apache.cloudstack.vm.UnmanagedVMsManagerImplTest)  Time elapsed: 0.05 s  <<< ERROR!
   > java.lang.NullPointerException
   > 	at org.apache.cloudstack.vm.UnmanagedVMsManagerImplTest.importUnmanagedInstanceTest(UnmanagedVMsManagerImplTest.java:372)
   > ```
   
   The first error have I get with the pull from the master. The other error was my mistake, It was a urgent bugfix in our system. I knew that I adapt the mockito conditions because the test runs into in error by this command hostDao.loadHostTags(host). But my knowledge was not so good and therefore I set this test to ignore and have forget to adapt this.... sorry :(


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



[GitHub] [cloudstack] weizhouapache commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   @DK101010 could you please fix the conflicts ?


-- 
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 #4399: PR multi tags in compute offering [#4398]

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


   @nvazquez here is update:
   1. found two bugs in my testing, see my comments above.
   2. fixed another ui bug: https://github.com/apache/cloudstack/pull/5233


-- 
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 #4399: PR multi tags in compute offering [#4398]

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


   tested ok


-- 
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] nvazquez commented on pull request #4399: PR multi tags in host offering [#4398]

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


   @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] DaanHoogland commented on pull request #4399: PR multi tags in host offering [#4398]

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


   @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



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4399: PR multi tags in host offering [#4398]

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


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

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



[GitHub] [cloudstack] DK101010 commented on a change in pull request #4399: PR multi tags in host offering [#4398]

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



##########
File path: engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java
##########
@@ -480,6 +482,25 @@ public Integer countAllByTypeInZone(long zoneId, Type type) {
         return listBy(sc);
     }
 
+    @Override
+    public boolean checkHostServiceOfferingTags(HostVO host, ServiceOffering serviceOffering){
+        if (host == null) {
+            return false;
+        }
+        if (serviceOffering == null) {
+            return false;
+        }
+        if (Strings.isNullOrEmpty(serviceOffering.getHostTag())) {
+            return true;
+        }
+
+        List<String> serviceOfferingTags = Arrays.asList(serviceOffering.getHostTag().split(","));
+        if(host.getHostTags() != null && host.getHostTags().containsAll(serviceOfferingTags)){
+            return true;
+        }
+        return false;
+    }

Review comment:
       Hi,
   In the HostDaoImpl there are a couple of other methods that are check host tags, therefore i thought will be good to add this also here. So will be encapsulate common logic in one class, but my experience is still not very well in CS and when it destroy other guidelines then will be also ok to move this method. For me sounds HostVO good, because it  seems to be only relevant for this object.  




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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


   Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 456


-- 
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 #4399: PR multi tags in host offering [#4398]

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


   Packaging result: ✖centos7 ✖centos8 ✖debian. JID-2721


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



[GitHub] [cloudstack] weizhouapache commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   @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] DaanHoogland removed a comment on pull request #4399: PR multi tags in host offering [#4398]

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


   @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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


   Packaging result: :heavy_multiplication_x: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 446


-- 
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 #4399: PR multi tags in host offering [#4398]

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


   @DV101010 can you invest some time in an automated 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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4399: PR multi tags in host offering [#4398]

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


   @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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


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

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



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4399: PR multi tags in host offering [#4398]

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






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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4399: PR multi tags in host offering [#4398]

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


   @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



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4399: PR multi tags in host offering [#4398]

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



##########
File path: engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java
##########
@@ -480,6 +482,25 @@ public Integer countAllByTypeInZone(long zoneId, Type type) {
         return listBy(sc);
     }
 
+    @Override
+    public boolean checkHostServiceOfferingTags(HostVO host, ServiceOffering serviceOffering){
+        if (host == null) {
+            return false;
+        }
+        if (serviceOffering == null) {
+            return false;
+        }
+        if (Strings.isNullOrEmpty(serviceOffering.getHostTag())) {
+            return true;
+        }
+
+        List<String> serviceOfferingTags = Arrays.asList(serviceOffering.getHostTag().split(","));
+        if(host.getHostTags() != null && host.getHostTags().containsAll(serviceOfferingTags)){
+            return true;
+        }
+        return false;
+    }

Review comment:
       trying to isolate this check is fine but the DAO is for DB interaction, and this is really business logic. personally I would make it a default method for the Host interface, but a member of HostVO could also be.
   Can you (shortly) explain why you put it here, @DK101010 ?




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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4399: PR multi tags in host offering [#4398]

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


   @DK101010 All necesary steps/items are in the cwiki in that article and in others, but if you have specific questions the dev@cloudstack.a.o list can give more feedback.


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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


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


-- 
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 #4399: PR multi tags in compute offering [#4398]

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


   @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 #4399: PR multi tags in host offering [#4398]

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


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

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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


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

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



[GitHub] [cloudstack] weizhouapache commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   @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] DK101010 commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   > > > found two bugs in testing
   > > > (1) list hosts response lists only 1 host tag, although hosts have multiple host tags.
   > > > for example
   > > > ```
   > > > (localcloud) SBCM5> > update host id=d9988a7b-7e02-4e90-9c4a-798c588b1cba hosttags=tag1,tag2
   > > > {
   > > >   "host": {
   > > > ...
   > > >     "hosttags": "tag2",
   > > > ...
   > > >   }
   > > > }
   > > > 
   > > > (localcloud) SBCM5> > list hosts id=d9988a7b-7e02-4e90-9c4a-798c588b1cba filter=hosttags
   > > > {
   > > >   "count": 1,
   > > >   "host": [
   > > >     {
   > > >       "hosttags": "tag2"
   > > >     }
   > > >   ]
   > > > }
   > > > ```
   > > > 
   > > > 
   > > >     
   > > >       
   > > >     
   > > > 
   > > >       
   > > >     
   > > > 
   > > >     
   > > >   
   > > > however in db, host has 2 tags.
   > > > ```
   > > > mysql> select * from host_tags;
   > > > +----+---------+------+
   > > > | id | host_id | tag  |
   > > > +----+---------+------+
   > > > | 29 |       4 | tag1 |
   > > > | 30 |       4 | tag2 |
   > > > +----+---------+------+
   > > > 2 rows in set (0.00 sec)
   > > > ```
   > > > 
   > > > 
   > > >     
   > > >       
   > > >     
   > > > 
   > > >       
   > > >     
   > > > 
   > > >     
   > > >   
   > > > (2) vm can be starteded on a host which does not have all tags.
   > > > for example, service offering has host tags "tag1,tag2", and host has host tags "tag1,tag1".
   > > > @DK101010 could you please fix the bugs above ?
   > > 
   > > 
   > > Hi @weizhouapache, many thanks for testing. I'm a little bit confused, the logic doesn't use the list host method to find the right one. I wrote an sql to check this. Also we use this feature already for a long time without problems and I wrote test for this to get sure of this :/. Hmm ... how do you test this?
   > 
   > @DK101010
   > ignore the first issue, I found the root cause: I upgraded the cloudstack packages but did not make the changes in #4796
   > 
   > for 2nd issue, I use cmk for testing
   > (1) update host to tag 'tag1,tag1'
   > (2) create service offering, update host tag to 'tag1,tag2'
   > (3) create vm with offering in (2)
   > (4) start vm on the host in (1).
   > 
   > expected result: vm is not started on host (1)
   > actual result: vm is started on host (1) successfully.
   > 
   > root cause: yo do not consider that host might have a tag multiple times. so you need to change the following sql
   > 
   > ```
   >     private static final String LIST_HOST_IDS_BY_COMPUTETAGS = "SELECT host_id, COUNT(tag) AS tag_count "
   >                                                              + "FROM host_tags "
   >                                                              + "WHERE tag IN(%s) "
   >                                                              + "GROUP BY host_id "
   >                                                              + "HAVING tag_count = %s ";
   > ```
   > 
   > https://github.com/apache/cloudstack/pull/4399/files#diff-ced268870eedefd01befd033d2544228bf010c0dccde9c76ac45f80652e503b8R84-R88
   
   Hi @weizhouapache, You mean the same tag two times? Hmm .... sounds more like a user input error. What do you think about a exception when user add the same tag in host or service offerings or we add here a check for hosts and service offering ? 


-- 
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] nvazquez commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   Thanks for start incorporating the suggested changes @DK101010, let us know when it is ready for review


-- 
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 closed pull request #4399: PR multi tags in compute offering [#4398]

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


   


-- 
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] nvazquez commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   @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] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


   <b>Trillian test result (tid-3352)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 35317 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4399-t3352-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
   Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Smoke tests completed. 85 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 260.35 | test_kubernetes_clusters.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.

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



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4399: PR multi tags in host offering [#4398]

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


   <b>Trillian test result (tid-3316)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 71104 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4399-t3316-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_public_ip_range.py
   Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers.py
   Intermittent failure detected: /marvin/tests/smoke/test_secondary_storage.py
   Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
   Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermittent failure detected: /marvin/tests/smoke/test_storage_policy.py
   Intermittent failure detected: /marvin/tests/smoke/test_templates.py
   Intermittent failure detected: /marvin/tests/smoke/test_usage.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
   Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
   Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Smoke tests completed. 63 look OK, 23 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 238.58 | test_kubernetes_clusters.py
   ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
   ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
   ContextSuite context=TestRAMCPUResourceAccounting>:setup | `Error` | 0.00 | test_resource_accounting.py
   ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
   ContextSuite context=TestRouterDnsService>:setup | `Error` | 0.00 | test_router_dnsservice.py
   ContextSuite context=TestRouterIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestVPCIpTablesPolicies>:setup | `Error` | 0.00 | test_routers_iptables_default_policy.py
   ContextSuite context=TestIsolatedNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRedundantIsolateNetworks>:setup | `Error` | 0.00 | test_routers_network_ops.py
   ContextSuite context=TestRouterServices>:setup | `Error` | 0.00 | test_routers.py
   test_01_sys_vm_start | `Failure` | 0.16 | test_secondary_storage.py
   ContextSuite context=TestCpuCapServiceOfferings>:setup | `Error` | 0.00 | test_service_offerings.py
   ContextSuite context=TestServiceOfferings>:setup | `Error` | 0.19 | test_service_offerings.py
   ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
   test_01_list_sec_storage_vm | `Failure` | 0.05 | test_ssvm.py
   test_02_list_cpvm_vm | `Failure` | 0.05 | test_ssvm.py
   test_03_ssvm_internals | `Failure` | 0.04 | test_ssvm.py
   test_04_cpvm_internals | `Failure` | 0.04 | test_ssvm.py
   test_05_stop_ssvm | `Failure` | 0.04 | test_ssvm.py
   test_06_stop_cpvm | `Failure` | 0.04 | test_ssvm.py
   test_07_reboot_ssvm | `Failure` | 0.04 | test_ssvm.py
   test_08_reboot_cpvm | `Failure` | 0.04 | test_ssvm.py
   test_09_destroy_ssvm | `Failure` | 0.04 | test_ssvm.py
   test_10_destroy_cpvm | `Failure` | 0.04 | test_ssvm.py
   ContextSuite context=TestVMWareStoragePolicies>:setup | `Error` | 0.00 | test_storage_policy.py
   test_02_create_template_with_checksum_sha1 | `Error` | 65.54 | test_templates.py
   test_03_create_template_with_checksum_sha256 | `Error` | 65.52 | test_templates.py
   test_04_create_template_with_checksum_md5 | `Error` | 65.53 | test_templates.py
   test_05_create_template_with_no_checksum | `Error` | 65.54 | test_templates.py
   test_02_deploy_vm_from_direct_download_template | `Error` | 1.29 | test_templates.py
   test_03_deploy_vm_wrong_checksum | `Error` | 1.33 | test_templates.py
   ContextSuite context=TestTemplates>:setup | `Error` | 17.44 | test_templates.py
   ContextSuite context=TestISOUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestLBRuleUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestNatRuleUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestPublicIPUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestSnapshotUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVmUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVolumeUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=TestVpnUsage>:setup | `Error` | 0.00 | test_usage.py
   ContextSuite context=Test01DeployVM>:setup | `Error` | 0.00 | test_vm_life_cycle.py
   ContextSuite context=Test02VMLifeCycle>:setup | `Error` | 0.00 | test_vm_life_cycle.py
   test_14_secure_to_secure_vm_migration | `Error` | 11.41 | test_vm_life_cycle.py
   test_15_secured_to_nonsecured_vm_migration | `Error` | 74.06 | test_vm_life_cycle.py
   test_16_nonsecured_to_secured_vm_migration | `Error` | 1.27 | test_vm_life_cycle.py
   ContextSuite context=TestVmSnapshot>:setup | `Error` | 1.95 | test_vm_snapshots.py
   ContextSuite context=TestCreateVolume>:setup | `Error` | 0.00 | test_volumes.py
   ContextSuite context=TestVolumes>:setup | `Error` | 0.00 | test_volumes.py
   ContextSuite context=TestVPCRedundancy>:setup | `Error` | 0.00 | test_vpc_redundant.py
   ContextSuite context=TestVPCNics>:setup | `Error` | 0.00 | test_vpc_router_nics.py
   ContextSuite context=TestRVPCSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVPCSite2SiteVPNMultipleOptions>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVpcRemoteAccessVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   ContextSuite context=TestVpcSite2SiteVpn>:setup | `Error` | 0.00 | test_vpc_vpn.py
   test_disable_oobm_ha_state_ineligible | `Error` | 1514.47 | test_hostha_kvm.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.

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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4399: PR multi tags in host offering [#4398]

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


   @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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


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

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



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4399: PR multi tags in host offering [#4398]

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






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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


   Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2461


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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


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

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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


   Packaging result: ✖centos7 ✖centos8 ✖debian. JID-2493


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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4399: PR multi tags in host offering [#4398]

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


   not sure yet, but the build failures are about certificates and i don't think related, but as a matter of good accounting:
   ```
   [INFO] Running org.apache.cloudstack.direct.download.DirectDownloadManagerImplTest
   java.security.cert.CertificateException: Could not parse certificate: java.io.IOException: java.lang.IllegalArgumentException: Input byte array has incorrect ending byte at 1132
   	at java.base/sun.security.provider.X509Factory.engineGenerateCertificate(X509Factory.java:115)
   	at java.base/java.security.cert.CertificateFactory.generateCertificate(CertificateFactory.java:355)
   	at com.cloud.utils.security.CertificateHelper.buildCertificate(CertificateHelper.java:130)
   	at org.apache.cloudstack.direct.download.DirectDownloadManagerImpl.getCertificateFromString(DirectDownloadManagerImpl.java:378)
   	at org.apache.cloudstack.direct.download.DirectDownloadManagerImpl.certificateSanity(DirectDownloadManagerImpl.java:389)
   	at org.apache.cloudstack.direct.download.DirectDownloadManagerImpl$MockitoMock$369926905.certificateSanity$accessor$rf85lrMB(Unknown Source)
   	at org.apache.cloudstack.direct.download.DirectDownloadManagerImpl$MockitoMock$369926905$auxiliary$xgeKzGvM.call(Unknown Source)
   	at org.mockito.internal.invocation.RealMethod$FromCallable$1.call(RealMethod.java:40)
   	at org.mockito.internal.invocation.RealMethod$FromBehavior.invoke(RealMethod.java:62)
   	at org.mockito.internal.invocation.InterceptedInvocation.callRealMethod(InterceptedInvocation.java:152)
   	at org.mockito.internal.stubbing.answers.CallsRealMethods.answer(CallsRealMethods.java:44)
   	at org.mockito.Answers.answer(Answers.java:100)
   	at org.mockito.internal.handler.MockHandlerImpl.handle(MockHandlerImpl.java:103)
   	at org.mockito.internal.handler.NullResultGuardian.handle(NullResultGuardian.java:29)
   	at org.mockito.internal.handler.InvocationNotifierHandler.handle(InvocationNotifierHandler.java:35)
   	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor.doIntercept(MockMethodInterceptor.java:61)
   	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor.doIntercept(MockMethodInterceptor.java:49)
   	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor$DispatcherDefaultingToRealMethod.interceptSuperCallable(MockMethodInterceptor.java:108)
   	at org.apache.cloudstack.direct.download.DirectDownloadManagerImpl$MockitoMock$369926905.certificateSanity(Unknown Source)
   	at org.apache.cloudstack.direct.download.DirectDownloadManagerImplTest.testCertificateSanityInvalidCertificate(DirectDownloadManagerImplTest.java:137)
   	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
   	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
   	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
   	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
   	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
   	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
   	at org.junit.internal.runners.statements.ExpectException.evaluate(ExpectException.java:19)
   	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
   	at org.mockito.internal.runners.DefaultInternalRunner$1$1.evaluate(DefaultInternalRunner.java:46)
   	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
   	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
   	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
   	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
   	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
   	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
   	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
   	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
   	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
   	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
   	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
   	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
   	at org.mockito.internal.runners.DefaultInternalRunner$1.run(DefaultInternalRunner.java:77)
   	at org.mockito.internal.runners.DefaultInternalRunner.run(DefaultInternalRunner.java:83)
   	at org.mockito.internal.runners.StrictRunner.run(StrictRunner.java:39)
   	at org.mockito.junit.MockitoJUnitRunner.run(MockitoJUnitRunner.java:163)
   	at org.mockito.runners.MockitoJUnitRunner.run(MockitoJUnitRunner.java:54)
   	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:365)
   	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:273)
   	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:238)
   	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
   	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:384)
   	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:345)
   	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:126)
   	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)
   Caused by: java.io.IOException: java.lang.IllegalArgumentException: Input byte array has incorrect ending byte at 1132
   	at java.base/sun.security.provider.X509Factory.readOneBlock(X509Factory.java:648)
   	at java.base/sun.security.provider.X509Factory.engineGenerateCertificate(X509Factory.java:99)
   	... 55 more
   Caused by: java.lang.IllegalArgumentException: Input byte array has incorrect ending byte at 1132
   	at java.base/java.util.Base64$Decoder.decode0(Base64.java:771)
   	at java.base/java.util.Base64$Decoder.decode(Base64.java:535)
   	at java.base/sun.security.provider.X509Factory.readOneBlock(X509Factory.java:646)
   	... 56 more
   ```


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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


   <b>Trillian test result (tid-3308)</b>
   Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 46883 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4399-t3308-vmware-65u2.zip
   Intermittent failure detected: /marvin/tests/smoke/test_human_readable_logs.py
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Smoke tests completed. 84 look OK, 2 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_02_enableHumanReadableLogs | `Error` | 0.38 | test_human_readable_logs.py
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 783.88 | test_kubernetes_clusters.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.

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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in host offering [#4398]

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


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

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



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4399: PR multi tags in host offering [#4398]

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


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

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



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4399: PR multi tags in host offering [#4398]

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






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



[GitHub] [cloudstack] DaanHoogland commented on pull request #4399: PR multi tags in host offering [#4398]

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


   @blueorangutan test centos7 kvm-centos7 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.

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



[GitHub] [cloudstack] blueorangutan commented on pull request #4399: PR multi tags in compute offering [#4398]

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


   <b>Trillian test result (tid-1471)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 47094 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4399-t1471-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
   Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
   Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 86 look OK, 3 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` | 144.54 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Failure` | 355.92 | test_routers_network_ops.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 552.49 | test_vpc_redundant.py
   test_01_vpc_site2site_vpn_multiple_options | `Failure` | 589.62 | test_vpc_vpn.py
   test_01_vpc_site2site_vpn | `Failure` | 256.46 | test_vpc_vpn.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] DaanHoogland commented on pull request #4399: PR multi tags in host offering [#4398]

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


   > Hi,
   > yesterday we had a problem during our deployment and I've found a additional tag check in the DeploymentPlanningManager. To prevent code duplicates I've encapsulate tag handling in HostDaoImpl. But I afraid I can not write test for this because we use currently a older version.
   
   very good to avoid duplicates, but I'd put it in the Host object hierarchy somewhere. For this piece of code a unit test would do, @DK101010 . no need for an integration test  as far as I can see.


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