[GitHub] blueorangutan commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields
blueorangutan commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields URL: https://github.com/apache/cloudstack/pull/2750#issuecomment-406160737 @dhlaluku 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 GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] dhlaluku commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields
dhlaluku commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields URL: https://github.com/apache/cloudstack/pull/2750#issuecomment-406160632 @blueorangutan test This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields
blueorangutan commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields URL: https://github.com/apache/cloudstack/pull/2750#issuecomment-406095441 Trillian test result (tid-2859) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 27365 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2750-t2859-kvm-centos7.zip Intermitten failure detected: /marvin/tests/smoke/test_certauthority_root.py Intermitten failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py Intermitten failure detected: /marvin/tests/smoke/test_vm_life_cycle.py Intermitten failure detected: /marvin/tests/smoke/test_volumes.py Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py Intermitten failure detected: /marvin/tests/smoke/test_hostha_kvm.py Smoke tests completed. 62 look OK, 6 have error(s) Only failed tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_provision_certificate | `Error` | 5.21 | test_certauthority_root.py ContextSuite context=TestDeployVirtioSCSIVM>:setup | `Error` | 0.00 | test_deploy_virtio_scsi_vm.py test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 1131.88 | test_privategw_acl.py test_01_secure_vm_migration | `Error` | 3.15 | test_vm_life_cycle.py test_02_unsecure_vm_migration | `Error` | 2.14 | test_vm_life_cycle.py test_03_secured_to_nonsecured_vm_migration | `Error` | 1.11 | test_vm_life_cycle.py test_04_nonsecured_to_secured_vm_migration | `Error` | 1.11 | test_vm_life_cycle.py test_11_migrate_volume_and_change_offering | `Error` | 128.34 | test_volumes.py test_hostha_enable_ha_when_host_in_maintenance | `Error` | 3.63 | test_hostha_kvm.py This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] GabrielBrascher edited a comment on issue #2732: security_group: Use execute() function instead of non-existing bash()
GabrielBrascher edited a comment on issue #2732: security_group: Use execute()
function instead of non-existing bash()
URL: https://github.com/apache/cloudstack/pull/2732#issuecomment-406037059
The following log messages are from
`/var/log/cloudstack/agent/security_group.log`.
On 4.11.1:
```
2018-07-10 21:27:00,514 - Executing command: network_rules_vmSecondaryIp
2018-07-10 21:27:00,514 - vmName = i-2-3-VM
2018-07-10 21:27:00,514 - action = -A
2018-07-10 21:27:00,516 - vm ip 192.168.100.77
2018-07-10 21:27:00,516 - ipset -A i-2-3-VM 192.168.100.77
2018-07-10 21:27:00,520 - ip = 192.168.100.77
2018-07-10 21:27:00,520 - ebtables -t nat -I i-2-3-VM-in-ips -p ARP
--arp-ip-src 192.168.100.77 -j RETURN
2018-07-10 21:27:00,524 - ebtables -t nat -I i-2-3-VM-out-ips -p ARP
--arp-ip-dst 192.168.100.77 -j RETURN
2018-07-10 21:27:40,175 - Executing command: get_rule_logs_for_vms
2018-07-10 21:27:47,250 - Executing command: destroy_network_rules_for_vm
2018-07-10 21:27:47,250 - iptables-save | awk
'/BF(.*)physdev-is-bridged(.*)i-2-3-def/ { sub(/-A/, "-D", $1) ; print }'
2018-07-10 21:27:47,257 - iptables -D BF-cloudbr1-IN -m physdev --physdev-in
vnet8 --physdev-is-bridged -j i-2-3-def
2018-07-10 21:27:47,261 - iptables -D BF-cloudbr1-OUT -m physdev
--physdev-out vnet8 --physdev-is-bridged -j i-2-3-def
2018-07-10 21:27:47,264 - ip6tables-save | awk
'/BF(.*)physdev-is-bridged(.*)i-2-3-def/ { sub(/-A/, "-D", $1) ; print }'
2018-07-10 21:27:47,269 - ip6tables -D BF-cloudbr1-IN -m physdev
--physdev-in vnet8 --physdev-is-bridged -j i-2-3-def
2018-07-10 21:27:47,274 - ip6tables -D BF-cloudbr1-OUT -m physdev
--physdev-out vnet8 --physdev-is-bridged -j i-2-3-def
2018-07-10 21:27:47,277 - ebtables -t nat -L PREROUTING | grep i-2-3-VM
2018-07-10 21:27:47,283 - ebtables -t nat -L POSTROUTING | grep i-2-3-VM
2018-07-10 21:27:47,288 - ebtables -t nat -D PREROUTING -i vnet8 -j
i-2-3-VM-in
2018-07-10 21:27:47,293 - ebtables -t nat -D POSTROUTING -o vnet8 -j
i-2-3-VM-out
2018-07-10 21:27:47,298 - ebtables -t nat -F i-2-3-VM-in
2018-07-10 21:27:47,304 - ebtables -t nat -X i-2-3-VM-in
2018-07-10 21:27:47,309 - ebtables -t nat -F i-2-3-VM-out
2018-07-10 21:27:47,315 - ebtables -t nat -X i-2-3-VM-out
2018-07-10 21:27:47,320 - ebtables -t nat -F i-2-3-VM-in-ips
2018-07-10 21:27:47,326 - ebtables -t nat -X i-2-3-VM-in-ips
2018-07-10 21:27:47,331 - ebtables -t nat -F i-2-3-VM-out-ips
2018-07-10 21:27:47,337 - ebtables -t nat -X i-2-3-VM-out-ips
```
After updating to 4.12:
Note that there are lines returning non-zero exit status 1 and other lines
returning exit status 255, which does not necessary means an error.
```
2018-07-10 22:08:04,889 - Failed to execute: ebtables -t nat -L PREROUTING |
grep s-5-VM
Traceback (most recent call last):
File "/usr/share/cloudstack-common/scripts/vm/network/security_group.py",
line 62, in execute
return check_output(cmd, shell=True)
File "/usr/lib/python2.7/subprocess.py", line 574, in check_output
raise CalledProcessError(retcode, cmd, output=output)
CalledProcessError: Command 'ebtables -t nat -L PREROUTING | grep s-5-VM'
returned non-zero exit status 1
2018-07-10 22:08:04,891 - ebtables -t nat -L POSTROUTING | grep s-5-VM
2018-07-10 22:08:04,895 - Failed to execute: ebtables -t nat -L POSTROUTING
| grep s-5-VM
Traceback (most recent call last):
File "/usr/share/cloudstack-common/scripts/vm/network/security_group.py",
line 62, in execute
return check_output(cmd, shell=True)
File "/usr/lib/python2.7/subprocess.py", line 574, in check_output
raise CalledProcessError(retcode, cmd, output=output)
CalledProcessError: Command 'ebtables -t nat -L POSTROUTING | grep s-5-VM'
returned non-zero exit status 1
2018-07-10 22:08:04,895 - ebtables -t nat -F s-5-VM-in
2018-07-10 22:08:04,898 - Failed to execute: ebtables -t nat -F s-5-VM-in
Traceback (most recent call last):
File "/usr/share/cloudstack-common/scripts/vm/network/security_group.py",
line 62, in execute
return check_output(cmd, shell=True)
File "/usr/lib/python2.7/subprocess.py", line 574, in check_output
raise CalledProcessError(retcode, cmd, output=output)
CalledProcessError: Command 'ebtables -t nat -F s-5-VM-in' returned non-zero
exit status 255
2018-07-10 22:08:04,898 - ebtables -t nat -X s-5-VM-in
2018-07-10 22:08:04,902 - Failed to execute: ebtables -t nat -X s-5-VM-in
Traceback (most recent call last):
File "/usr/share/cloudstack-common/scripts/vm/network/security_group.py",
line 62, in execute
return check_output(cmd, shell=True)
File "/usr/lib/python2.7/subprocess.py", line 574, in check_output
raise CalledProcessError(retcode, cmd, output=output)
CalledProcessError: Command 'ebtables -t nat -X s-5-VM-in' returned non-zero
exit status 255
```
After adding
[GitHub] GabrielBrascher commented on issue #2732: security_group: Use execute() function instead of non-existing bash()
GabrielBrascher commented on issue #2732: security_group: Use execute()
function instead of non-existing bash()
URL: https://github.com/apache/cloudstack/pull/2732#issuecomment-406037059
On 4.11.1:
```
2018-07-10 21:27:00,514 - Executing command: network_rules_vmSecondaryIp
2018-07-10 21:27:00,514 - vmName = i-2-3-VM
2018-07-10 21:27:00,514 - action = -A
2018-07-10 21:27:00,516 - vm ip 192.168.100.77
2018-07-10 21:27:00,516 - ipset -A i-2-3-VM 192.168.100.77
2018-07-10 21:27:00,520 - ip = 192.168.100.77
2018-07-10 21:27:00,520 - ebtables -t nat -I i-2-3-VM-in-ips -p ARP
--arp-ip-src 192.168.100.77 -j RETURN
2018-07-10 21:27:00,524 - ebtables -t nat -I i-2-3-VM-out-ips -p ARP
--arp-ip-dst 192.168.100.77 -j RETURN
2018-07-10 21:27:40,175 - Executing command: get_rule_logs_for_vms
2018-07-10 21:27:47,250 - Executing command: destroy_network_rules_for_vm
2018-07-10 21:27:47,250 - iptables-save | awk
'/BF(.*)physdev-is-bridged(.*)i-2-3-def/ { sub(/-A/, "-D", $1) ; print }'
2018-07-10 21:27:47,257 - iptables -D BF-cloudbr1-IN -m physdev --physdev-in
vnet8 --physdev-is-bridged -j i-2-3-def
2018-07-10 21:27:47,261 - iptables -D BF-cloudbr1-OUT -m physdev
--physdev-out vnet8 --physdev-is-bridged -j i-2-3-def
2018-07-10 21:27:47,264 - ip6tables-save | awk
'/BF(.*)physdev-is-bridged(.*)i-2-3-def/ { sub(/-A/, "-D", $1) ; print }'
2018-07-10 21:27:47,269 - ip6tables -D BF-cloudbr1-IN -m physdev
--physdev-in vnet8 --physdev-is-bridged -j i-2-3-def
2018-07-10 21:27:47,274 - ip6tables -D BF-cloudbr1-OUT -m physdev
--physdev-out vnet8 --physdev-is-bridged -j i-2-3-def
2018-07-10 21:27:47,277 - ebtables -t nat -L PREROUTING | grep i-2-3-VM
2018-07-10 21:27:47,283 - ebtables -t nat -L POSTROUTING | grep i-2-3-VM
2018-07-10 21:27:47,288 - ebtables -t nat -D PREROUTING -i vnet8 -j
i-2-3-VM-in
2018-07-10 21:27:47,293 - ebtables -t nat -D POSTROUTING -o vnet8 -j
i-2-3-VM-out
2018-07-10 21:27:47,298 - ebtables -t nat -F i-2-3-VM-in
2018-07-10 21:27:47,304 - ebtables -t nat -X i-2-3-VM-in
2018-07-10 21:27:47,309 - ebtables -t nat -F i-2-3-VM-out
2018-07-10 21:27:47,315 - ebtables -t nat -X i-2-3-VM-out
2018-07-10 21:27:47,320 - ebtables -t nat -F i-2-3-VM-in-ips
2018-07-10 21:27:47,326 - ebtables -t nat -X i-2-3-VM-in-ips
2018-07-10 21:27:47,331 - ebtables -t nat -F i-2-3-VM-out-ips
2018-07-10 21:27:47,337 - ebtables -t nat -X i-2-3-VM-out-ips
```
After updating to 4.12:
Note that there are lines returning non-zero exit status 1 and other lines
returning exit status 255, which does not necessary means an error.
```
2018-07-10 22:08:04,889 - Failed to execute: ebtables -t nat -L PREROUTING |
grep s-5-VM
Traceback (most recent call last):
File "/usr/share/cloudstack-common/scripts/vm/network/security_group.py",
line 62, in execute
return check_output(cmd, shell=True)
File "/usr/lib/python2.7/subprocess.py", line 574, in check_output
raise CalledProcessError(retcode, cmd, output=output)
CalledProcessError: Command 'ebtables -t nat -L PREROUTING | grep s-5-VM'
returned non-zero exit status 1
2018-07-10 22:08:04,891 - ebtables -t nat -L POSTROUTING | grep s-5-VM
2018-07-10 22:08:04,895 - Failed to execute: ebtables -t nat -L POSTROUTING
| grep s-5-VM
Traceback (most recent call last):
File "/usr/share/cloudstack-common/scripts/vm/network/security_group.py",
line 62, in execute
return check_output(cmd, shell=True)
File "/usr/lib/python2.7/subprocess.py", line 574, in check_output
raise CalledProcessError(retcode, cmd, output=output)
CalledProcessError: Command 'ebtables -t nat -L POSTROUTING | grep s-5-VM'
returned non-zero exit status 1
2018-07-10 22:08:04,895 - ebtables -t nat -F s-5-VM-in
2018-07-10 22:08:04,898 - Failed to execute: ebtables -t nat -F s-5-VM-in
Traceback (most recent call last):
File "/usr/share/cloudstack-common/scripts/vm/network/security_group.py",
line 62, in execute
return check_output(cmd, shell=True)
File "/usr/lib/python2.7/subprocess.py", line 574, in check_output
raise CalledProcessError(retcode, cmd, output=output)
CalledProcessError: Command 'ebtables -t nat -F s-5-VM-in' returned non-zero
exit status 255
2018-07-10 22:08:04,898 - ebtables -t nat -X s-5-VM-in
2018-07-10 22:08:04,902 - Failed to execute: ebtables -t nat -X s-5-VM-in
Traceback (most recent call last):
File "/usr/share/cloudstack-common/scripts/vm/network/security_group.py",
line 62, in execute
return check_output(cmd, shell=True)
File "/usr/lib/python2.7/subprocess.py", line 574, in check_output
raise CalledProcessError(retcode, cmd, output=output)
CalledProcessError: Command 'ebtables -t nat -X s-5-VM-in' returned non-zero
exit status 255
```
After adding changes from this PR:
```
2018-07-18 19:46:43,797 - vmName = i-2-38-VM
2018-07-18
[GitHub] blueorangutan commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields
blueorangutan commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields URL: https://github.com/apache/cloudstack/pull/2750#issuecomment-406023730 Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2185 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields
blueorangutan commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields URL: https://github.com/apache/cloudstack/pull/2750#issuecomment-406013636 @dhlaluku 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 GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] dhlaluku commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields
dhlaluku commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields URL: https://github.com/apache/cloudstack/pull/2750#issuecomment-406013408 @blueorangutan package This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2636: Fix limitation on tag matching in 'migrateVolume' with disk offering replacement
rafaelweingartner commented on a change in pull request #2636: Fix limitation
on tag matching in 'migrateVolume' with disk offering replacement
URL: https://github.com/apache/cloudstack/pull/2636#discussion_r203463594
##
File path: server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
##
@@ -2189,6 +2189,53 @@ protected void
validateConditionsToReplaceDiskOfferingOfVolume(VolumeVO volume,
s_logger.info(String.format("Changing disk offering to [uuid=%s] while
migrating volume [uuid=%s, name=%s].", newDiskOffering.getUuid(),
volume.getUuid(), volume.getName()));
}
+/**
+ * Checks if the target storage supports the new disk offering.
+ * This validation is consistent with the mechanism used to select a
storage pool to deploy a volume when a virtual machine is deployed or when a
new data disk is allocated.
+ *
+ * The scenarios when this method returns true or false is presented in
the following table.
+ *
+ *
+ *
+ * #Disk offering tagsStorage
tagsDoes the storage support the disk offering?
+ *
+ *
+ *
+ * 1A,BANO
+ *
+ *
+ * 2A,B,CA,B,C,D,XYES
+ *
+ *
+ * 3A,B,CX,Y,ZNO
+ *
+ *
+ * 4nullA,S,DYES
+ *
+ *
+ * 5AnullNO
+ *
+ *
+ * 6nullnullYES
+ *
+ *
+ *
+ */
+protected boolean doesTargetStorageSupportNewDiskOffering(StoragePool
destPool, DiskOfferingVO newDiskOffering) {
Review comment:
That sounds like a plan. I just did not understand what you mean by
`matchOk` . Could you explain it a little bit further?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] blueorangutan commented on issue #2749: [master]Smoketest Health Check
blueorangutan commented on issue #2749: [master]Smoketest Health Check URL: https://github.com/apache/cloudstack/pull/2749#issuecomment-406005245 Trillian test result (tid-2858) Environment: vmware-65 (x2), Advanced Networking with Mgmt server 7 Total time taken: 28061 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2749-t2858-vmware-65.zip Intermitten failure detected: /marvin/tests/smoke/test_certauthority_root.py Intermitten failure detected: /marvin/tests/smoke/test_volumes.py Smoke tests completed. 67 look OK, 1 have error(s) Only failed tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_11_migrate_volume_and_change_offering | `Error` | 10.52 | test_volumes.py This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2749: [master]Smoketest Health Check
blueorangutan commented on issue #2749: [master]Smoketest Health Check URL: https://github.com/apache/cloudstack/pull/2749#issuecomment-406003653 Trillian test result (tid-2857) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 27582 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2749-t2857-kvm-centos7.zip Intermitten failure detected: /marvin/tests/smoke/test_certauthority_root.py Intermitten failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py Intermitten failure detected: /marvin/tests/smoke/test_vm_life_cycle.py Intermitten failure detected: /marvin/tests/smoke/test_volumes.py Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py Intermitten failure detected: /marvin/tests/smoke/test_hostha_kvm.py Smoke tests completed. 62 look OK, 6 have error(s) Only failed tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_provision_certificate | `Error` | 5.19 | test_certauthority_root.py ContextSuite context=TestDeployVirtioSCSIVM>:setup | `Error` | 0.00 | test_deploy_virtio_scsi_vm.py test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 1178.43 | test_privategw_acl.py test_01_secure_vm_migration | `Error` | 3.17 | test_vm_life_cycle.py test_02_unsecure_vm_migration | `Error` | 3.17 | test_vm_life_cycle.py test_03_secured_to_nonsecured_vm_migration | `Error` | 1.11 | test_vm_life_cycle.py test_04_nonsecured_to_secured_vm_migration | `Error` | 0.11 | test_vm_life_cycle.py test_11_migrate_volume_and_change_offering | `Error` | 128.48 | test_volumes.py test_01_redundant_vpc_site2site_vpn | `Failure` | 326.50 | test_vpc_vpn.py This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] DaanHoogland commented on a change in pull request #2636: Fix limitation on tag matching in 'migrateVolume' with disk offering replacement
DaanHoogland commented on a change in pull request #2636: Fix limitation on tag
matching in 'migrateVolume' with disk offering replacement
URL: https://github.com/apache/cloudstack/pull/2636#discussion_r203450480
##
File path: server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
##
@@ -2189,6 +2189,53 @@ protected void
validateConditionsToReplaceDiskOfferingOfVolume(VolumeVO volume,
s_logger.info(String.format("Changing disk offering to [uuid=%s] while
migrating volume [uuid=%s, name=%s].", newDiskOffering.getUuid(),
volume.getUuid(), volume.getName()));
}
+/**
+ * Checks if the target storage supports the new disk offering.
+ * This validation is consistent with the mechanism used to select a
storage pool to deploy a volume when a virtual machine is deployed or when a
new data disk is allocated.
+ *
+ * The scenarios when this method returns true or false is presented in
the following table.
+ *
+ *
+ *
+ * #Disk offering tagsStorage
tagsDoes the storage support the disk offering?
+ *
+ *
+ *
+ * 1A,BANO
+ *
+ *
+ * 2A,B,CA,B,C,D,XYES
+ *
+ *
+ * 3A,B,CX,Y,ZNO
+ *
+ *
+ * 4nullA,S,DYES
+ *
+ *
+ * 5AnullNO
+ *
+ *
+ * 6nullnullYES
+ *
+ *
+ *
+ */
+protected boolean doesTargetStorageSupportNewDiskOffering(StoragePool
destPool, DiskOfferingVO newDiskOffering) {
Review comment:
I think it is a nice future plan to extract the logic to a tagging utility
and then make this method use it and be given a more general name like
```
doesTargetStorageSupportDiskOffering() // not no 'New' in there
```
and have it call the utility
```
matchOk(Set tags, List supported);
```
plan?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] blueorangutan commented on issue #2749: [master]Smoketest Health Check
blueorangutan commented on issue #2749: [master]Smoketest Health Check URL: https://github.com/apache/cloudstack/pull/2749#issuecomment-405986584 Trillian test result (tid-2856) Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7 Total time taken: 24254 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2749-t2856-xenserver-71.zip Intermitten failure detected: /marvin/tests/smoke/test_certauthority_root.py Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py Intermitten failure detected: /marvin/tests/smoke/test_scale_vm.py Intermitten failure detected: /marvin/tests/smoke/test_volumes.py Smoke tests completed. 66 look OK, 2 have error(s) Only failed tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_01_scale_vm | `Error` | 13.41 | test_scale_vm.py test_11_migrate_volume_and_change_offering | `Error` | 6.41 | test_volumes.py This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] dhlaluku commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields
dhlaluku commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields URL: https://github.com/apache/cloudstack/pull/2750#issuecomment-405986425 @rafaelweingartner checked the attributes, it seems like they can be set to private. I will modify them and do some tests before pushing again This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2734: Fix invalid consoleproxy url after upgrade
rafaelweingartner commented on issue #2734: Fix invalid consoleproxy url after upgrade URL: https://github.com/apache/cloudstack/pull/2734#issuecomment-405975344 I think the forward merging is working (normally). Somehow git is managing the new folder structure that was applied between 4.11-4.12 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] resmo commented on issue #2734: Fix invalid consoleproxy url after upgrade
resmo commented on issue #2734: Fix invalid consoleproxy url after upgrade URL: https://github.com/apache/cloudstack/pull/2734#issuecomment-405974841 will work on the unit test but +14 days and I assume forward merging will be tricky (file renames), suggesting to merge into 4.11 and add the test later on master for 4.12. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[cloudstack-docs-install] 01/01: Merge pull request #40 from PaulAngus/4.11
This is an automated email from the ASF dual-hosted git repository. paul_a pushed a commit to branch 4.11 in repository https://gitbox.apache.org/repos/asf/cloudstack-docs-install.git commit 94c543f3bd1437b4ea1b20b84653f63949039d55 Merge: fa75631 a15446a Author: Paul Angus AuthorDate: Wed Jul 18 16:22:55 2018 +0100 Merge pull request #40 from PaulAngus/4.11 4.11.1 updates to installation docs source/building_from_source.rst| 10 +- source/management-server/_pkg_repo.rst | 5 +++-- source/management-server/_systemvm.rst | 12 ++-- 3 files changed, 14 insertions(+), 13 deletions(-)
[cloudstack-docs-install] branch 4.11 updated (fa75631 -> 94c543f)
This is an automated email from the ASF dual-hosted git repository. paul_a pushed a change to branch 4.11 in repository https://gitbox.apache.org/repos/asf/cloudstack-docs-install.git. from fa75631 CLOUDSTACK-10333: update docs to enable libvirtd tls port (#36) add 619e95d Updates for 4.11 release add f7c8d24 update installation instructions for CloudStack 4.11.1 add a15446a update download URL. new 94c543f Merge pull request #40 from PaulAngus/4.11 The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: source/building_from_source.rst| 10 +- source/management-server/_pkg_repo.rst | 5 +++-- source/management-server/_systemvm.rst | 12 ++-- 3 files changed, 14 insertions(+), 13 deletions(-)
[GitHub] rafaelweingartner commented on a change in pull request #2636: Fix limitation on tag matching in 'migrateVolume' with disk offering replacement
rafaelweingartner commented on a change in pull request #2636: Fix limitation
on tag matching in 'migrateVolume' with disk offering replacement
URL: https://github.com/apache/cloudstack/pull/2636#discussion_r203420800
##
File path: server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
##
@@ -2189,6 +2189,53 @@ protected void
validateConditionsToReplaceDiskOfferingOfVolume(VolumeVO volume,
s_logger.info(String.format("Changing disk offering to [uuid=%s] while
migrating volume [uuid=%s, name=%s].", newDiskOffering.getUuid(),
volume.getUuid(), volume.getName()));
}
+/**
+ * Checks if the target storage supports the new disk offering.
+ * This validation is consistent with the mechanism used to select a
storage pool to deploy a volume when a virtual machine is deployed or when a
new data disk is allocated.
+ *
+ * The scenarios when this method returns true or false is presented in
the following table.
+ *
+ *
+ *
+ * #Disk offering tagsStorage
tagsDoes the storage support the disk offering?
+ *
+ *
+ *
+ * 1A,BANO
+ *
+ *
+ * 2A,B,CA,B,C,D,XYES
+ *
+ *
+ * 3A,B,CX,Y,ZNO
+ *
+ *
+ * 4nullA,S,DYES
+ *
+ *
+ * 5AnullNO
+ *
+ *
+ * 6nullnullYES
+ *
+ *
+ *
+ */
+protected boolean doesTargetStorageSupportNewDiskOffering(StoragePool
destPool, DiskOfferingVO newDiskOffering) {
Review comment:
This new method that I implemented is the consolidation of the logic of tag
matching in ACS, which is not extracted in a specific method and spread around
the code. That is one of the reasons why I “made the mistake” in one of the
features I developed where the tag matching was being too strict (requiring all
tags to match) and was not making sense to operation guys.
To answer your question, yes this method can be used in other places. I am
not sure if simply declaring the method in the interface will help much though.
The person that is willing to consolidate these execution flow could do both,
declare this method in the interface, and then use it in all of those places
that you mentioned.
What do you think?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] rafaelweingartner removed a comment on issue #2636: Fix limitation on tag matching in 'migrateVolume' with disk offering replacement
rafaelweingartner removed a comment on issue #2636: Fix limitation on tag matching in 'migrateVolume' with disk offering replacement URL: https://github.com/apache/cloudstack/pull/2636#issuecomment-405968549 This new method that I implemented is the consolidation of the logic of tag matching in ACS, which is not extracted in a specific method and spread around the code. That is one of the reasons why I “made the mistake” in one of the features I developed where the tag matching was being too strict (requiring all tags to match) and was not making sense to operation guys. To answer your question, yes this method can be used in other places. I am not sure if simply declaring the method in the interface will help much though. The person that is willing to consolidate these execution flow could do both, declare this method in the interface, and then use it in all of those places that you mentioned. What do you think? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2636: Fix limitation on tag matching in 'migrateVolume' with disk offering replacement
rafaelweingartner commented on issue #2636: Fix limitation on tag matching in 'migrateVolume' with disk offering replacement URL: https://github.com/apache/cloudstack/pull/2636#issuecomment-405968549 This new method that I implemented is the consolidation of the logic of tag matching in ACS, which is not extracted in a specific method and spread around the code. That is one of the reasons why I “made the mistake” in one of the features I developed where the tag matching was being too strict (requiring all tags to match) and was not making sense to operation guys. To answer your question, yes this method can be used in other places. I am not sure if simply declaring the method in the interface will help much though. The person that is willing to consolidate these execution flow could do both, declare this method in the interface, and then use it in all of those places that you mentioned. What do you think? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] DaanHoogland commented on a change in pull request #2636: Fix limitation on tag matching in 'migrateVolume' with disk offering replacement
DaanHoogland commented on a change in pull request #2636: Fix limitation on tag
matching in 'migrateVolume' with disk offering replacement
URL: https://github.com/apache/cloudstack/pull/2636#discussion_r203406752
##
File path: server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
##
@@ -2189,6 +2189,53 @@ protected void
validateConditionsToReplaceDiskOfferingOfVolume(VolumeVO volume,
s_logger.info(String.format("Changing disk offering to [uuid=%s] while
migrating volume [uuid=%s, name=%s].", newDiskOffering.getUuid(),
volume.getUuid(), volume.getName()));
}
+/**
+ * Checks if the target storage supports the new disk offering.
+ * This validation is consistent with the mechanism used to select a
storage pool to deploy a volume when a virtual machine is deployed or when a
new data disk is allocated.
+ *
+ * The scenarios when this method returns true or false is presented in
the following table.
+ *
+ *
+ *
+ * #Disk offering tagsStorage
tagsDoes the storage support the disk offering?
+ *
+ *
+ *
+ * 1A,BANO
+ *
+ *
+ * 2A,B,CA,B,C,D,XYES
+ *
+ *
+ * 3A,B,CX,Y,ZNO
+ *
+ *
+ * 4nullA,S,DYES
+ *
+ *
+ * 5AnullNO
+ *
+ *
+ * 6nullnullYES
+ *
+ *
+ *
+ */
+protected boolean doesTargetStorageSupportNewDiskOffering(StoragePool
destPool, DiskOfferingVO newDiskOffering) {
Review comment:
ok, nice subject: another valid usage is mutual dependent services. Or
dependency injection.
I this case my concern is that the implementation of tag matching is done in
several places while it should be uniform and thus implemented centrally.
doesTargetStorageASupportNewDiskOffering() is a specialisation of
does_this_entities_tags_match_the_resource_it_is_going_to(entity, resource)
which is applicable to volume/storage in case of new offering but also in case
of migration, and is also applicable for vm-serviceoffering/host-cluster-pod
etc. I cant' think of any others by head but these are surely there as well.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] DaanHoogland commented on a change in pull request #2636: Fix limitation on tag matching in 'migrateVolume' with disk offering replacement
DaanHoogland commented on a change in pull request #2636: Fix limitation on tag
matching in 'migrateVolume' with disk offering replacement
URL: https://github.com/apache/cloudstack/pull/2636#discussion_r203406752
##
File path: server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
##
@@ -2189,6 +2189,53 @@ protected void
validateConditionsToReplaceDiskOfferingOfVolume(VolumeVO volume,
s_logger.info(String.format("Changing disk offering to [uuid=%s] while
migrating volume [uuid=%s, name=%s].", newDiskOffering.getUuid(),
volume.getUuid(), volume.getName()));
}
+/**
+ * Checks if the target storage supports the new disk offering.
+ * This validation is consistent with the mechanism used to select a
storage pool to deploy a volume when a virtual machine is deployed or when a
new data disk is allocated.
+ *
+ * The scenarios when this method returns true or false is presented in
the following table.
+ *
+ *
+ *
+ * #Disk offering tagsStorage
tagsDoes the storage support the disk offering?
+ *
+ *
+ *
+ * 1A,BANO
+ *
+ *
+ * 2A,B,CA,B,C,D,XYES
+ *
+ *
+ * 3A,B,CX,Y,ZNO
+ *
+ *
+ * 4nullA,S,DYES
+ *
+ *
+ * 5AnullNO
+ *
+ *
+ * 6nullnullYES
+ *
+ *
+ *
+ */
+protected boolean doesTargetStorageSupportNewDiskOffering(StoragePool
destPool, DiskOfferingVO newDiskOffering) {
Review comment:
ok, nice subject: another valid usage is mutual dependent services. Or
dependency injection.
I this case my concern is that the implementation of tag matching is doen in
several places while it should be uniform and thus implemented centrally.
doesTargetStorageASupportNewDiskOffering() is a specialisation of
does_this_entities_tags_match_the_resource_it_is_going_to(entity, resource)
which is applicable to volume/storage in case of new offering but also in case
of migration, and is also applicable for vm-serviceoffering/host-cluster-pod
etc. I cant' think of any others by head but these are surely there as well.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2743: CLOUDSTACK-10380: Fix startvm giving another pw after pw reset
rafaelweingartner commented on a change in pull request #2743:
CLOUDSTACK-10380: Fix startvm giving another pw after pw reset
URL: https://github.com/apache/cloudstack/pull/2743#discussion_r203399511
##
File path: server/src/com/cloud/network/element/VirtualRouterElement.java
##
@@ -703,7 +703,14 @@ public boolean savePassword(final Network network, final
NicProfile nic, final V
// save the password in DB
for (final VirtualRouter router : routers) {
if (router.getState() == State.Running) {
-return networkTopology.savePasswordToRouter(network, nic,
uservm, router);
+final boolean result =
networkTopology.savePasswordToRouter(network, nic, uservm, router);
+if (result) {
+// Explicit password reset, while VM hasn't generated a
password yet.
+final UserVmVO userVmVO = _userVmDao.findById(vm.getId());
+userVmVO.setUpdateParameters(false);
Review comment:
What does this `updateParameters` means in the `userVmVo` object?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] blueorangutan commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields
blueorangutan commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields URL: https://github.com/apache/cloudstack/pull/2750#issuecomment-405950842 @dhlaluku 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 GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2743: CLOUDSTACK-10380: Fix startvm giving another pw after pw reset
rafaelweingartner commented on a change in pull request #2743:
CLOUDSTACK-10380: Fix startvm giving another pw after pw reset
URL: https://github.com/apache/cloudstack/pull/2743#discussion_r203400062
##
File path: server/src/com/cloud/vm/UserVmManagerImpl.java
##
@@ -688,10 +689,6 @@ public UserVm resetVMPassword(ResetVMPasswordCmd cmd,
String password) throws Re
if (result) {
userVm.setPassword(password);
-// update the password in vm_details table too
-// Check if an SSH key pair was selected for the instance and if so
-// use it to encrypt & save the vm password
-encryptAndStorePassword(userVm, password);
Review comment:
So, the password will not be encrypted anymore in the DB? Is that it?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] dhlaluku commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields
dhlaluku commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields URL: https://github.com/apache/cloudstack/pull/2750#issuecomment-405950652 @blueorangutan test This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields
blueorangutan commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields URL: https://github.com/apache/cloudstack/pull/2750#issuecomment-405948400 Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2184 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields
blueorangutan commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields URL: https://github.com/apache/cloudstack/pull/2750#issuecomment-405936718 @dhlaluku 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 GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] dhlaluku commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields
dhlaluku commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields URL: https://github.com/apache/cloudstack/pull/2750#issuecomment-405936375 @blueorangutan package This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields
blueorangutan commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields URL: https://github.com/apache/cloudstack/pull/2750#issuecomment-405935137 Packaging result: ✔centos6 ✔centos7 ✖debian. JID-2183 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] dhlaluku commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields
dhlaluku commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields URL: https://github.com/apache/cloudstack/pull/2750#issuecomment-405932506 @DaanHoogland @rafaelweingartner @resmo @marcaurele I am new to this community and I would like to ask for your assistance with reviewing of this PR. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields
blueorangutan commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields URL: https://github.com/apache/cloudstack/pull/2750#issuecomment-405925400 @borisstoyanov 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 GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] borisstoyanov commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields
borisstoyanov commented on issue #2750: Refactor userVmDetailsDao field and remove unusued fields URL: https://github.com/apache/cloudstack/pull/2750#issuecomment-405925149 @blueorangutan package This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] dhlaluku opened a new pull request #2750: Refactor userVmDetailsDao field and remove unusued fields
dhlaluku opened a new pull request #2750: Refactor userVmDetailsDao field and remove unusued fields URL: https://github.com/apache/cloudstack/pull/2750 ## Description This PR refactors a duplicated field vmDetailsDao and _uservmDetailsDao into userVmDetailsDao. vmDetailsDao, uservmDetailsDao --> userVmDetailsDao It also removes several unused injected vars as well i.e.; _domainDao, rulesMgr, _vguTypesDao, _configDepot ## 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) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) ## GitHub Issue/PRs ## Screenshots (if appropriate): ## How Has This Been Tested? I have recompiled CloudStack in my local environment and ran the virtual machine related Marvin smoke tests. ## Checklist: - [x] I have read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document. - [x] My code follows the code style of this project. - [ ] My change requires a change to the documentation. - [ ] I have updated the documentation accordingly. Testing - [ ] I have added tests to cover my changes. - [ ] All relevant new and existing integration tests have passed. - [ ] A full integration testsuite with all test that can run on my environment has passed. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] borisstoyanov commented on issue #2743: CLOUDSTACK-10380: Fix startvm giving another pw after pw reset
borisstoyanov commented on issue #2743: CLOUDSTACK-10380: Fix startvm giving another pw after pw reset URL: https://github.com/apache/cloudstack/pull/2743#issuecomment-405915192 there's seems to be an issue with the vm password I'm able to ssh using default password, looks like the tests are either using some other hard-coded one or trying to set a new and that part does not seem to work. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] borisstoyanov commented on a change in pull request #2743: CLOUDSTACK-10380: Fix startvm giving another pw after pw reset
borisstoyanov commented on a change in pull request #2743: CLOUDSTACK-10380: Fix startvm giving another pw after pw reset URL: https://github.com/apache/cloudstack/pull/2743#discussion_r203357320 ## File path: tools/marvin/setup.py ## @@ -54,10 +54,12 @@ "pyvmomi >= 5.5.0", "netaddr >= 0.7.14", "dnspython", - "ipmisim >= 0.7" + "ipmisim >= 0.7", + "retries", Review comment: (+1) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[cloudstack] branch 4.11 updated: check volumes for state when retrieving pool for configDrive creation (#2709)
This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch 4.11
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.11 by this push:
new 38d0274 check volumes for state when retrieving pool for configDrive
creation (#2709)
38d0274 is described below
commit 38d0274eb488ffbd9d701d1724bdabb528d7f9ba
Author: dahn
AuthorDate: Wed Jul 18 13:13:41 2018 +0200
check volumes for state when retrieving pool for configDrive creation
(#2709)
* only ask for the root volume, removing extensive query
* better name
---
.../network/element/ConfigDriveNetworkElement.java | 79 ++
1 file changed, 66 insertions(+), 13 deletions(-)
diff --git
a/server/src/com/cloud/network/element/ConfigDriveNetworkElement.java
b/server/src/com/cloud/network/element/ConfigDriveNetworkElement.java
index 919d7bd..4d2452d 100644
--- a/server/src/com/cloud/network/element/ConfigDriveNetworkElement.java
+++ b/server/src/com/cloud/network/element/ConfigDriveNetworkElement.java
@@ -23,6 +23,7 @@ import java.util.Set;
import javax.inject.Inject;
+import com.cloud.storage.StoragePool;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
@@ -30,6 +31,8 @@ import
org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector;
import org.apache.cloudstack.storage.configdrive.ConfigDrive;
import org.apache.cloudstack.storage.configdrive.ConfigDriveBuilder;
import org.apache.cloudstack.storage.to.TemplateObjectTO;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.collections.MapUtils;
import org.apache.log4j.Logger;
import com.cloud.agent.AgentManager;
@@ -56,7 +59,6 @@ import com.cloud.offering.NetworkOffering;
import com.cloud.service.dao.ServiceOfferingDao;
import com.cloud.storage.DataStoreRole;
import com.cloud.storage.Storage;
-import com.cloud.storage.StoragePool;
import com.cloud.storage.Volume;
import com.cloud.storage.VolumeVO;
import com.cloud.storage.dao.GuestOSCategoryDao;
@@ -322,20 +324,11 @@ public class ConfigDriveNetworkElement extends
AdapterBase implements NetworkEle
private DataStore findDataStore(VirtualMachineProfile profile,
DeployDestination dest) {
DataStore dataStore = null;
if (VirtualMachineManager.VmConfigDriveOnPrimaryPool.value()) {
-if (dest.getStorageForDisks() != null) {
-for (final Volume volume : dest.getStorageForDisks().keySet())
{
-if (volume.getVolumeType() == Volume.Type.ROOT) {
-final StoragePool primaryPool =
dest.getStorageForDisks().get(volume);
-dataStore =
_dataStoreMgr.getDataStore(primaryPool.getId(), DataStoreRole.Primary);
-break;
-}
-}
+if(MapUtils.isNotEmpty(dest.getStorageForDisks())) {
+dataStore = getPlannedDataStore(dest, dataStore);
}
if (dataStore == null) {
-final List volumes =
_volumeDao.findByInstanceAndType(profile.getVirtualMachine().getId(),
Volume.Type.ROOT);
-if (volumes != null && volumes.size() > 0) {
-dataStore =
_dataStoreMgr.getDataStore(volumes.get(0).getPoolId(), DataStoreRole.Primary);
-}
+dataStore = pickExistingRootVolumeFromDataStore(profile,
dataStore);
}
} else {
dataStore =
_dataStoreMgr.getImageStore(dest.getDataCenter().getId());
@@ -343,6 +336,66 @@ public class ConfigDriveNetworkElement extends AdapterBase
implements NetworkEle
return dataStore;
}
+private DataStore getPlannedDataStore(DeployDestination dest, DataStore
dataStore) {
+for (final Volume volume : dest.getStorageForDisks().keySet()) {
+if (volume.getVolumeType() == Volume.Type.ROOT) {
+final StoragePool primaryPool =
dest.getStorageForDisks().get(volume);
+dataStore = _dataStoreMgr.getDataStore(primaryPool.getId(),
DataStoreRole.Primary);
+break;
+}
+}
+return dataStore;
+}
+
+private DataStore
pickExistingRootVolumeFromDataStore(VirtualMachineProfile profile, DataStore
dataStore) {
+final List volumes =
_volumeDao.findByInstanceAndType(profile.getVirtualMachine().getId(),
Volume.Type.ROOT);
+if (CollectionUtils.isNotEmpty(volumes)) {
+dataStore = pickDataStoreFromVolumes(volumes);
+}
+return dataStore;
+}
+
+private DataStore pickDataStoreFromVolumes(List volumes) {
+DataStore dataStore = null;
+for (Volume vol : volumes) {
+if (doesVolumeStateCheckout(vol)) {
+
[GitHub] rafaelweingartner commented on a change in pull request #2636: Fix limitation on tag matching in 'migrateVolume' with disk offering replacement
rafaelweingartner commented on a change in pull request #2636: Fix limitation
on tag matching in 'migrateVolume' with disk offering replacement
URL: https://github.com/apache/cloudstack/pull/2636#discussion_r203339499
##
File path: server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
##
@@ -2189,6 +2189,53 @@ protected void
validateConditionsToReplaceDiskOfferingOfVolume(VolumeVO volume,
s_logger.info(String.format("Changing disk offering to [uuid=%s] while
migrating volume [uuid=%s, name=%s].", newDiskOffering.getUuid(),
volume.getUuid(), volume.getName()));
}
+/**
+ * Checks if the target storage supports the new disk offering.
+ * This validation is consistent with the mechanism used to select a
storage pool to deploy a volume when a virtual machine is deployed or when a
new data disk is allocated.
+ *
+ * The scenarios when this method returns true or false is presented in
the following table.
+ *
+ *
+ *
+ * #Disk offering tagsStorage
tagsDoes the storage support the disk offering?
+ *
+ *
+ *
+ * 1A,BANO
+ *
+ *
+ * 2A,B,CA,B,C,D,XYES
+ *
+ *
+ * 3A,B,CX,Y,ZNO
+ *
+ *
+ * 4nullA,S,DYES
+ *
+ *
+ * 5AnullNO
+ *
+ *
+ * 6nullnullYES
+ *
+ *
+ *
+ */
+protected boolean doesTargetStorageSupportNewDiskOffering(StoragePool
destPool, DiskOfferingVO newDiskOffering) {
Review comment:
Well, I am not sure if we have some other places that can use this method. I
would normally only set a method to public and declare it in an interface if I
need really need it. I can declare it if you think it is important though.
P.S.: I find our interface usage sometimes quite silly with single
implementations. A good example is using interface for POJOs. It does not
provide benefits, the only thing it does is adding dozens more lines for every
object.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] DaanHoogland closed pull request #2709: check volumes for state when retrieving pool for configDrive creation
DaanHoogland closed pull request #2709: check volumes for state when retrieving
pool for configDrive creation
URL: https://github.com/apache/cloudstack/pull/2709
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/server/src/com/cloud/network/element/ConfigDriveNetworkElement.java
b/server/src/com/cloud/network/element/ConfigDriveNetworkElement.java
index 919d7bd9d18..4d2452d45ce 100644
--- a/server/src/com/cloud/network/element/ConfigDriveNetworkElement.java
+++ b/server/src/com/cloud/network/element/ConfigDriveNetworkElement.java
@@ -23,6 +23,7 @@
import javax.inject.Inject;
+import com.cloud.storage.StoragePool;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
@@ -30,6 +31,8 @@
import org.apache.cloudstack.storage.configdrive.ConfigDrive;
import org.apache.cloudstack.storage.configdrive.ConfigDriveBuilder;
import org.apache.cloudstack.storage.to.TemplateObjectTO;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.collections.MapUtils;
import org.apache.log4j.Logger;
import com.cloud.agent.AgentManager;
@@ -56,7 +59,6 @@
import com.cloud.service.dao.ServiceOfferingDao;
import com.cloud.storage.DataStoreRole;
import com.cloud.storage.Storage;
-import com.cloud.storage.StoragePool;
import com.cloud.storage.Volume;
import com.cloud.storage.VolumeVO;
import com.cloud.storage.dao.GuestOSCategoryDao;
@@ -322,20 +324,11 @@ public void commitMigration(NicProfile nic, Network
network, VirtualMachineProfi
private DataStore findDataStore(VirtualMachineProfile profile,
DeployDestination dest) {
DataStore dataStore = null;
if (VirtualMachineManager.VmConfigDriveOnPrimaryPool.value()) {
-if (dest.getStorageForDisks() != null) {
-for (final Volume volume : dest.getStorageForDisks().keySet())
{
-if (volume.getVolumeType() == Volume.Type.ROOT) {
-final StoragePool primaryPool =
dest.getStorageForDisks().get(volume);
-dataStore =
_dataStoreMgr.getDataStore(primaryPool.getId(), DataStoreRole.Primary);
-break;
-}
-}
+if(MapUtils.isNotEmpty(dest.getStorageForDisks())) {
+dataStore = getPlannedDataStore(dest, dataStore);
}
if (dataStore == null) {
-final List volumes =
_volumeDao.findByInstanceAndType(profile.getVirtualMachine().getId(),
Volume.Type.ROOT);
-if (volumes != null && volumes.size() > 0) {
-dataStore =
_dataStoreMgr.getDataStore(volumes.get(0).getPoolId(), DataStoreRole.Primary);
-}
+dataStore = pickExistingRootVolumeFromDataStore(profile,
dataStore);
}
} else {
dataStore =
_dataStoreMgr.getImageStore(dest.getDataCenter().getId());
@@ -343,6 +336,66 @@ private DataStore findDataStore(VirtualMachineProfile
profile, DeployDestination
return dataStore;
}
+private DataStore getPlannedDataStore(DeployDestination dest, DataStore
dataStore) {
+for (final Volume volume : dest.getStorageForDisks().keySet()) {
+if (volume.getVolumeType() == Volume.Type.ROOT) {
+final StoragePool primaryPool =
dest.getStorageForDisks().get(volume);
+dataStore = _dataStoreMgr.getDataStore(primaryPool.getId(),
DataStoreRole.Primary);
+break;
+}
+}
+return dataStore;
+}
+
+private DataStore
pickExistingRootVolumeFromDataStore(VirtualMachineProfile profile, DataStore
dataStore) {
+final List volumes =
_volumeDao.findByInstanceAndType(profile.getVirtualMachine().getId(),
Volume.Type.ROOT);
+if (CollectionUtils.isNotEmpty(volumes)) {
+dataStore = pickDataStoreFromVolumes(volumes);
+}
+return dataStore;
+}
+
+private DataStore pickDataStoreFromVolumes(List volumes) {
+DataStore dataStore = null;
+for (Volume vol : volumes) {
+if (doesVolumeStateCheckout(vol)) {
+dataStore = _dataStoreMgr.getDataStore(vol.getPoolId(),
DataStoreRole.Primary);
+if (dataStore != null) {
+return dataStore;
+}
+}
+}
+return dataStore;
+}
+
+private boolean doesVolumeStateCheckout(Volume vol) {
+switch (vol.getState()) {
+case Allocated:
+case Creating:
+case Ready:
+case Snapshotting:
+case
[GitHub] DaanHoogland commented on a change in pull request #2636: Fix limitation on tag matching in 'migrateVolume' with disk offering replacement
DaanHoogland commented on a change in pull request #2636: Fix limitation on tag
matching in 'migrateVolume' with disk offering replacement
URL: https://github.com/apache/cloudstack/pull/2636#discussion_r202960678
##
File path: server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
##
@@ -2189,6 +2189,53 @@ protected void
validateConditionsToReplaceDiskOfferingOfVolume(VolumeVO volume,
s_logger.info(String.format("Changing disk offering to [uuid=%s] while
migrating volume [uuid=%s, name=%s].", newDiskOffering.getUuid(),
volume.getUuid(), volume.getName()));
}
+/**
+ * Checks if the target storage supports the new disk offering.
+ * This validation is consistent with the mechanism used to select a
storage pool to deploy a volume when a virtual machine is deployed or when a
new data disk is allocated.
+ *
+ * The scenarios when this method returns true or false is presented in
the following table.
+ *
+ *
+ *
+ * #Disk offering tagsStorage
tagsDoes the storage support the disk offering?
+ *
+ *
+ *
+ * 1A,BANO
+ *
+ *
+ * 2A,B,CA,B,C,D,XYES
+ *
+ *
+ * 3A,B,CX,Y,ZNO
+ *
+ *
+ * 4nullA,S,DYES
+ *
+ *
+ * 5AnullNO
+ *
+ *
+ * 6nullnullYES
+ *
+ *
+ *
+ */
+protected boolean doesTargetStorageSupportNewDiskOffering(StoragePool
destPool, DiskOfferingVO newDiskOffering) {
Review comment:
This is a protected method. When we want to let a hypervisor do the entire
migration it would be convenient to call this method from elsewhere. Does it
make sense to put it on the interface? I am thinking about the
VirtualMachineManager or the HypervisorGurus.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] svenvogel commented on issue #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM and XENSERVER
svenvogel commented on issue #2204: [CLOUDSTACK-10025] Adding Support for NoVNC Console for KVM and XENSERVER URL: https://github.com/apache/cloudstack/pull/2204#issuecomment-405875150 ping @syed some news on theis pull request? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2749: [master]Smoketest Health Check
blueorangutan commented on issue #2749: [master]Smoketest Health Check URL: https://github.com/apache/cloudstack/pull/2749#issuecomment-405859303 @borisstoyanov a Trillian-Jenkins matrix job (centos6 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 GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] borisstoyanov commented on issue #2749: [master]Smoketest Health Check
borisstoyanov commented on issue #2749: [master]Smoketest Health Check URL: https://github.com/apache/cloudstack/pull/2749#issuecomment-405859076 @blueorangutan test matrix This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2749: [master]Smoketest Health Check
blueorangutan commented on issue #2749: [master]Smoketest Health Check URL: https://github.com/apache/cloudstack/pull/2749#issuecomment-405852569 Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2182 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2749: [master]Smoketest Health Check
blueorangutan commented on issue #2749: [master]Smoketest Health Check URL: https://github.com/apache/cloudstack/pull/2749#issuecomment-405843826 @borisstoyanov 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 GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] borisstoyanov opened a new pull request #2749: [master]Smoketest Health Check
borisstoyanov opened a new pull request #2749: [master]Smoketest Health Check URL: https://github.com/apache/cloudstack/pull/2749 --DO NOT MERGE-- Just a dummy PR for checking the state of smoketests This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] borisstoyanov commented on issue #2749: [master]Smoketest Health Check
borisstoyanov commented on issue #2749: [master]Smoketest Health Check URL: https://github.com/apache/cloudstack/pull/2749#issuecomment-405843685 @blueorangutan package This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
