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 2022/09/30 10:48:31 UTC

[GitHub] [cloudstack] weizhouapache opened a new pull request, #6792: Support multiple ceph monitors

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

   ### Description
   
   This PR adds the support for multiple ceph monitors on UI and API.
   
   ![image](https://user-images.githubusercontent.com/57355700/193254088-d26f2796-e45c-4a2e-8117-f96782d4cf47.png)
   
   What have been tested and work
   - vm life cycle
   - volume snapshot
   - vm migration to another host
   
   What have been tested but do not work (I believe they are not caused by this PR)
   - vm live migration with volume to another RBD primary storage
   - direct download to RBD primary storage
   - VM snapshots on RBD primary storage
   
   <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
   
   <!-- For new features, provide link to FS, dev ML discussion etc. -->
   <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
   
   <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
   <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
   <!-- Fixes: # -->
   
   <!--- ********************************************************************************* -->
   <!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
   <!--- PLEASE PUT AN 'X' in only **ONE** box -->
   <!--- ********************************************************************************* -->
   
   ### Types of changes
   
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [ ] Bug fix (non-breaking change which fixes an issue)
   - [x] Enhancement (improves an existing feature and functionality)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [x] Major
   - [ ] Minor
   
   
   
   ### Screenshots (if appropriate):
   
   
   ### How Has This Been Tested?
   <!-- Please describe in detail how you tested your changes. -->
   <!-- Include details of your testing environment, and the tests you ran to -->
   <!-- see how your change affects other areas of the code, etc. -->
   
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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

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


[GitHub] [cloudstack] blueorangutan commented on pull request #6792: Support multiple ceph monitors

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

   @weizhouapache a Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. 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 a diff in pull request #6792: Support multiple ceph monitors

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on code in PR #6792:
URL: https://github.com/apache/cloudstack/pull/6792#discussion_r984843284


##########
plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java:
##########
@@ -238,6 +238,39 @@ public void testDiskDefWithEncryption() {
         assertEquals(disk.toString(), expectedXML);
     }
 
+    @Test
+    public void testDiskDefWithMultipleHosts() {
+        String path = "/mnt/primary1";
+        String host = "10.11.12.13,10.11.12.14,10.11.12.15";

Review Comment:
   @wido 
   please ignore my previous comment
   I will use ULA addresses in testing



-- 
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 #6792: Support multiple ceph monitors

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

   <b>Trillian test result (tid-5076)</b>
   Environment: kvm-ubuntu22 (x2), Advanced Networking with Mgmt server u22
   Total time taken: 43956 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6792-t5076-kvm-ubuntu22.zip
   Smoke tests completed. 103 look OK, 1 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_08_upgrade_kubernetes_ha_cluster | `Failure` | 592.41 | 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] blueorangutan commented on pull request #6792: Support multiple ceph monitors

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

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4326


-- 
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 #6792: Support multiple ceph monitors

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

   @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] blueorangutan commented on pull request #6792: Support multiple ceph monitors

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

   @weizhouapache a Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. 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 #6792: Support multiple ceph monitors

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

   @weizhouapache a Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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

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


[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6792: Support multiple ceph monitors

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6792:
URL: https://github.com/apache/cloudstack/pull/6792#discussion_r984777251


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java:
##########
@@ -123,4 +125,13 @@ private static String getAttrValue(String tag, String attr, Element eElement) {
         Element node = (Element)tagNode.item(0);
         return node.getAttribute(attr);
     }
+
+    protected static String getStorageHost(Element parentElement) {

Review Comment:
   ```suggestion
       protected static String getStorageHosts(Element parentElement) {
   ```



-- 
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 merged pull request #6792: Support multiple ceph monitors

Posted by GitBox <gi...@apache.org>.
DaanHoogland merged PR #6792:
URL: https://github.com/apache/cloudstack/pull/6792


-- 
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 #6792: Support multiple ceph monitors

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

   when testing this PR we have found an issue that 
   
   - centos7 kvm host can NOT connect to ceph 17 (single monitor or multiple monitors)
   - centos7 kvm host can connecto to ceph 15
   
   thanks @NuxRo 


-- 
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 #6792: Support multiple ceph monitors

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

   @weizhouapache a Trillian-Jenkins test job (ubuntu22 mgmt + kvm-ubuntu22) 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] wido commented on a diff in pull request #6792: Support multiple ceph monitors

Posted by GitBox <gi...@apache.org>.
wido commented on code in PR #6792:
URL: https://github.com/apache/cloudstack/pull/6792#discussion_r984535550


##########
plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParserTest.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package com.cloud.hypervisor.kvm.resource;
+
+import junit.framework.TestCase;
+import org.junit.Assert;
+
+public class LibvirtStoragePoolXMLParserTest extends TestCase {
+
+    public void testParseNfsStoragePoolXML() {
+        String poolXML = "<pool type='netfs'>\n" +
+                "  <name>feff06b5-84b2-3258-b5f9-1953217295de</name>\n" +
+                "  <uuid>feff06b5-84b2-3258-b5f9-1953217295de</uuid>\n" +
+                "  <capacity unit='bytes'>111111111</capacity>\n" +
+                "  <allocation unit='bytes'>2222222</allocation>\n" +
+                "  <available unit='bytes'>3333333</available>\n" +
+                "  <source>\n" +
+                "    <host name='10.11.12.13'/>\n" +
+                "    <dir path='/mnt/primary1'/>\n" +
+                "    <format type='auto'/>\n" +
+                "  </source>\n" +
+                "  <target>\n" +
+                "    <path>/mnt/feff06b5-84b2-3258-b5f9-1953217295de</path>\n" +
+                "    <permissions>\n" +
+                "      <mode>0755</mode>\n" +
+                "      <owner>0</owner>\n" +
+                "      <group>0</group>\n" +
+                "    </permissions>\n" +
+                "  </target>\n" +
+                "</pool>";
+
+        LibvirtStoragePoolXMLParser parser = new LibvirtStoragePoolXMLParser();
+        LibvirtStoragePoolDef pool = parser.parseStoragePoolXML(poolXML);
+
+        Assert.assertEquals("10.11.12.13", pool.getSourceHost());

Review Comment:
   Another IPv6 test case?



##########
plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParserTest.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package com.cloud.hypervisor.kvm.resource;
+
+import junit.framework.TestCase;
+import org.junit.Assert;
+
+public class LibvirtStoragePoolXMLParserTest extends TestCase {
+
+    public void testParseNfsStoragePoolXML() {
+        String poolXML = "<pool type='netfs'>\n" +
+                "  <name>feff06b5-84b2-3258-b5f9-1953217295de</name>\n" +
+                "  <uuid>feff06b5-84b2-3258-b5f9-1953217295de</uuid>\n" +
+                "  <capacity unit='bytes'>111111111</capacity>\n" +
+                "  <allocation unit='bytes'>2222222</allocation>\n" +
+                "  <available unit='bytes'>3333333</available>\n" +
+                "  <source>\n" +
+                "    <host name='10.11.12.13'/>\n" +
+                "    <dir path='/mnt/primary1'/>\n" +
+                "    <format type='auto'/>\n" +
+                "  </source>\n" +
+                "  <target>\n" +
+                "    <path>/mnt/feff06b5-84b2-3258-b5f9-1953217295de</path>\n" +
+                "    <permissions>\n" +
+                "      <mode>0755</mode>\n" +
+                "      <owner>0</owner>\n" +
+                "      <group>0</group>\n" +
+                "    </permissions>\n" +
+                "  </target>\n" +
+                "</pool>";
+
+        LibvirtStoragePoolXMLParser parser = new LibvirtStoragePoolXMLParser();
+        LibvirtStoragePoolDef pool = parser.parseStoragePoolXML(poolXML);
+
+        Assert.assertEquals("10.11.12.13", pool.getSourceHost());
+    }
+
+    public void testParseRbdStoragePoolXMLWithMultipleHosts() {
+        String poolXML = "<pool type='rbd'>\n" +
+                "  <name>feff06b5-84b2-3258-b5f9-1953217295de</name>\n" +
+                "  <uuid>feff06b5-84b2-3258-b5f9-1953217295de</uuid>\n" +
+                "  <source>\n" +
+                "    <name>rbdpool</name>\n" +
+                "    <host name='10.11.12.13' port='6789'/>\n" +
+                "    <host name='10.11.12.14' port='6789'/>\n" +
+                "    <host name='10.11.12.15' port='6789'/>\n" +
+                "    <format type='auto'/>\n" +
+                "    <auth username='admin' type='ceph'>\n" +
+                "      <secret uuid='262f743a-3726-11ed-aaee-93e90b39d5c4'/>\n" +
+                "    </auth>\n" +
+                "  </source>\n" +
+                "</pool>";
+
+        LibvirtStoragePoolXMLParser parser = new LibvirtStoragePoolXMLParser();
+        LibvirtStoragePoolDef pool = parser.parseStoragePoolXML(poolXML);
+
+        Assert.assertEquals(LibvirtStoragePoolDef.PoolType.RBD, pool.getPoolType());
+        Assert.assertEquals(LibvirtStoragePoolDef.AuthenticationType.CEPH, pool.getAuthType());
+        Assert.assertEquals("10.11.12.13,10.11.12.14,10.11.12.15", pool.getSourceHost());

Review Comment:
   Would be good if we have an IPv6 test case here as well



##########
plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java:
##########
@@ -238,6 +238,39 @@ public void testDiskDefWithEncryption() {
         assertEquals(disk.toString(), expectedXML);
     }
 
+    @Test
+    public void testDiskDefWithMultipleHosts() {
+        String path = "/mnt/primary1";
+        String host = "10.11.12.13,10.11.12.14,10.11.12.15";

Review Comment:
   Could you also make a test-case with IPv6 addresses? Just to make sure we cover that 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] codecov[bot] commented on pull request #6792: Support multiple ceph monitors

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #6792:
URL: https://github.com/apache/cloudstack/pull/6792#issuecomment-1263590144

   # [Codecov](https://codecov.io/gh/apache/cloudstack/pull/6792?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#6792](https://codecov.io/gh/apache/cloudstack/pull/6792?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ac63fe3) into [main](https://codecov.io/gh/apache/cloudstack/commit/784578d46c080612373088c4189a44f861ca6c2e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (784578d) will **increase** coverage by `0.03%`.
   > The diff coverage is `47.91%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##               main    #6792      +/-   ##
   ============================================
   + Coverage     10.52%   10.56%   +0.03%     
   - Complexity     6784     6799      +15     
   ============================================
     Files          2464     2464              
     Lines        243989   243996       +7     
     Branches      38186    38188       +2     
   ============================================
   + Hits          25690    25771      +81     
   + Misses       215065   214985      -80     
   - Partials       3234     3240       +6     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/cloudstack/pull/6792?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ypervisor/kvm/resource/LibvirtDomainXMLParser.java](https://codecov.io/gh/apache/cloudstack/pull/6792/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9oeXBlcnZpc29ycy9rdm0vc3JjL21haW4vamF2YS9jb20vY2xvdWQvaHlwZXJ2aXNvci9rdm0vcmVzb3VyY2UvTGlidmlydERvbWFpblhNTFBhcnNlci5qYXZh) | `48.49% <0.00%> (ø)` | |
   | [...cycle/CloudStackPrimaryDataStoreLifeCycleImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6792/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9zdG9yYWdlL3ZvbHVtZS9kZWZhdWx0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL3N0b3JhZ2UvZGF0YXN0b3JlL2xpZmVjeWNsZS9DbG91ZFN0YWNrUHJpbWFyeURhdGFTdG9yZUxpZmVDeWNsZUltcGwuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [.../cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java](https://codecov.io/gh/apache/cloudstack/pull/6792/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9oeXBlcnZpc29ycy9rdm0vc3JjL21haW4vamF2YS9jb20vY2xvdWQvaHlwZXJ2aXNvci9rdm0vc3RvcmFnZS9LVk1QaHlzaWNhbERpc2suamF2YQ==) | `70.73% <71.42%> (+0.46%)` | :arrow_up: |
   | [...om/cloud/hypervisor/kvm/resource/LibvirtVMDef.java](https://codecov.io/gh/apache/cloudstack/pull/6792/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9oeXBlcnZpc29ycy9rdm0vc3JjL21haW4vamF2YS9jb20vY2xvdWQvaHlwZXJ2aXNvci9rdm0vcmVzb3VyY2UvTGlidmlydFZNRGVmLmphdmE=) | `67.04% <85.71%> (+2.88%)` | :arrow_up: |
   | [...isor/kvm/resource/LibvirtStoragePoolXMLParser.java](https://codecov.io/gh/apache/cloudstack/pull/6792/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9oeXBlcnZpc29ycy9rdm0vc3JjL21haW4vamF2YS9jb20vY2xvdWQvaHlwZXJ2aXNvci9rdm0vcmVzb3VyY2UvTGlidmlydFN0b3JhZ2VQb29sWE1MUGFyc2VyLmphdmE=) | `60.65% <100.00%> (+60.65%)` | :arrow_up: |
   | [...apache/cloudstack/utils/qemu/QemuImageOptions.java](https://codecov.io/gh/apache/cloudstack/pull/6792/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9oeXBlcnZpc29ycy9rdm0vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Nsb3Vkc3RhY2svdXRpbHMvcWVtdS9RZW11SW1hZ2VPcHRpb25zLmphdmE=) | `73.07% <100.00%> (+16.55%)` | :arrow_up: |
   | [...hypervisor/kvm/resource/LibvirtStoragePoolDef.java](https://codecov.io/gh/apache/cloudstack/pull/6792/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9oeXBlcnZpc29ycy9rdm0vc3JjL21haW4vamF2YS9jb20vY2xvdWQvaHlwZXJ2aXNvci9rdm0vcmVzb3VyY2UvTGlidmlydFN0b3JhZ2VQb29sRGVmLmphdmE=) | `76.84% <0.00%> (+1.05%)` | :arrow_up: |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
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 #6792: Support multiple ceph monitors

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

   @wido by the looks of it all your comments are met, right?


-- 
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] zap51 commented on pull request #6792: Support multiple ceph monitors

Posted by "zap51 (via GitHub)" <gi...@apache.org>.
zap51 commented on PR #6792:
URL: https://github.com/apache/cloudstack/pull/6792#issuecomment-1646859932

   @rohityadavcloud @DaanHoogland A [PR](https://github.com/ceph/ceph/pull/52595) has been created on Ceph to include 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] wido commented on a diff in pull request #6792: Support multiple ceph monitors

Posted by GitBox <gi...@apache.org>.
wido commented on code in PR #6792:
URL: https://github.com/apache/cloudstack/pull/6792#discussion_r985040041


##########
plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java:
##########
@@ -238,6 +238,39 @@ public void testDiskDefWithEncryption() {
         assertEquals(disk.toString(), expectedXML);
     }
 
+    @Test
+    public void testDiskDefWithMultipleHosts() {
+        String path = "/mnt/primary1";
+        String host = "10.11.12.13,10.11.12.14,10.11.12.15";

Review Comment:
   > @wido please ignore my previous comment I will use ULA addresses in testing
   
   You can also use the 2001:db8::/32 range for this: https://www.rfc-editor.org/rfc/rfc3849
   
   That would be sufficient to write your test-cases with.



-- 
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 #6792: Support multiple ceph monitors

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

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4363


-- 
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 #6792: Support multiple ceph monitors

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

   @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] sonarcloud[bot] commented on pull request #6792: Support multiple ceph monitors

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6792:
URL: https://github.com/apache/cloudstack/pull/6792#issuecomment-1263466947

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6792)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6792&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6792&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6792&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6792&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6792&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6792&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6792&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6792&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6792&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6792&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6792&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6792&resolved=false&types=CODE_SMELL)
   
   [![58.6%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50-16px.png '58.6%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6792&metric=new_coverage&view=list) [58.6% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6792&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6792&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6792&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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

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


[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6792: Support multiple ceph monitors

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6792:
URL: https://github.com/apache/cloudstack/pull/6792#discussion_r984777251


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java:
##########
@@ -123,4 +125,13 @@ private static String getAttrValue(String tag, String attr, Element eElement) {
         Element node = (Element)tagNode.item(0);
         return node.getAttribute(attr);
     }
+
+    protected static String getStorageHost(Element parentElement) {

Review Comment:
   ```suggestion
       protected static String getStorageHosts(Element parentElement) {
   ```
   and in the callers of course



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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

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


[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #6792: Support multiple ceph monitors

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on code in PR #6792:
URL: https://github.com/apache/cloudstack/pull/6792#discussion_r984591419


##########
plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java:
##########
@@ -238,6 +238,39 @@ public void testDiskDefWithEncryption() {
         assertEquals(disk.toString(), expectedXML);
     }
 
+    @Test
+    public void testDiskDefWithMultipleHosts() {
+        String path = "/mnt/primary1";
+        String host = "10.11.12.13,10.11.12.14,10.11.12.15";

Review Comment:
   @wido 
   It seems monitor with ipv6 address is not suported in this PR.
   
   what is currently supported and any issue in following scenarios ?
   - ipv6 addr without port
   - ipv6 addr with port
   I do not have a ceph env with ipv6 address , it is difficult to verify.



-- 
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 #6792: Support multiple ceph monitors

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

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4361


-- 
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 #6792: Support multiple ceph monitors

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

   <b>Trillian test result (tid-5074)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 39781 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6792-t5074-kvm-centos7.zip
   Smoke tests completed. 99 look OK, 5 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_add_primary_storage_disabled_host | `Error` | 0.62 | test_primary_storage.py
   test_01_primary_storage_nfs | `Error` | 0.12 | test_primary_storage.py
   ContextSuite context=TestStorageTags>:setup | `Error` | 0.19 | test_primary_storage.py
   test_01_secure_vm_migration | `Error` | 158.04 | test_vm_life_cycle.py
   test_02_unsecure_vm_migration | `Error` | 273.58 | test_vm_life_cycle.py
   test_03_secured_to_nonsecured_vm_migration | `Error` | 142.12 | test_vm_life_cycle.py
   test_08_migrate_vm | `Error` | 42.79 | test_vm_life_cycle.py
   test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 32.14 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 62.17 | test_kubernetes_clusters.py
   test_08_upgrade_kubernetes_ha_cluster | `Failure` | 40.35 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 37.09 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 122.45 | test_kubernetes_clusters.py
   test_02_list_snapshots_with_removed_data_store | `Error` | 8.63 | test_snapshots.py
   test_02_list_snapshots_with_removed_data_store | `Error` | 8.63 | test_snapshots.py
   test_hostha_enable_ha_when_host_in_maintenance | `Error` | 305.85 | 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.

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

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


[GitHub] [cloudstack] rohityadavcloud commented on pull request #6792: Support multiple ceph monitors

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6792:
URL: https://github.com/apache/cloudstack/pull/6792#issuecomment-1286630637

   LGTM - let's merge this @DaanHoogland ?


-- 
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 #6792: Support multiple ceph monitors

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

   We are not in "freeze" @rohityadavcloud ;)


-- 
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 #6792: Support multiple ceph monitors

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

   @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 #6792: Support multiple ceph monitors

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

   @blueorangutan test ubuntu22 kvm-ubuntu22


-- 
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 #6792: Support multiple ceph monitors

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

   <b>Trillian test result (tid-5075)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 41746 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6792-t5075-kvm-centos7.zip
   Smoke tests completed. 104 look OK, 0 have errors, 0 did not run
   Only failed and skipped 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] weizhouapache commented on pull request #6792: Support multiple ceph monitors

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

   @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 #6792: Support multiple ceph monitors

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

   @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