You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by SudharmaJain <gi...@git.apache.org> on 2015/09/23 08:13:23 UTC
[GitHub] cloudstack pull request: CLOUDSTACK-8865: Adding SR doesn't create...
GitHub user SudharmaJain opened a pull request:
https://github.com/apache/cloudstack/pull/876
CLOUDSTACK-8865: Adding SR doesn't create Storage_pool_host_ref entry…
… for disabled host
This causes VM deployment failure on the host that was disabled while adding the storage repository.
In the attachCluster function of the PrimaryDataStoreLifeCycle, we were only selecting hosts that are up and are in enabled state. Here if we select all up hosts, it will populate the DB properly and will fix this issue.
Also added a unit test for attachCluster function.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/SudharmaJain/cloudstack cs-8865
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/876.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #876
----
commit 00d921e3c81d2e34dce64a9c70c720c76bb8073f
Author: SudharmaJain <su...@citrix.com>
Date: 2015-09-23T05:49:24Z
CLOUDSTACK-8865: Adding SR doesn't create Storage_pool_host_ref entry for disabled host
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8865: Adding SR doesn't create Storage_p...
Posted by bvbharatk <gi...@git.apache.org>.
Github user bvbharatk commented on the pull request:
https://github.com/apache/cloudstack/pull/876
### ACS CI BVT Run
**Sumarry:**
Build Number 104
Hypervisor xenserver
NetworkType Advanced
Passed=70
Failed=0
Skipped=3
_Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
**Failed tests:**
**Skipped tests:**
test_vm_nic_adapter_vmxnet3
test_static_role_account_acls
test_deploy_vgpu_enabled_vm
**Passed test suits:**
test_deploy_vm_with_userdata.py
test_affinity_groups_projects.py
test_portable_publicip.py
test_vpc_vpn.py
test_over_provisioning.py
test_global_settings.py
test_scale_vm.py
test_service_offerings.py
test_routers_iptables_default_policy.py
test_routers.py
test_reset_vm_on_reboot.py
test_snapshots.py
test_deploy_vms_with_varied_deploymentplanners.py
test_login.py
test_list_ids_parameter.py
test_public_ip_range.py
test_multipleips_per_nic.py
test_regions.py
test_affinity_groups.py
test_network_acl.py
test_pvlan.py
test_volumes.py
test_nic.py
test_deploy_vm_root_resize.py
test_resource_detail.py
test_secondary_storage.py
test_disk_offerings.py
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #876: CLOUDSTACK-8865: Adding SR doesn't create Stor...
Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/876#discussion_r109299000
--- Diff: plugins/storage/volume/cloudbyte/src/org/apache/cloudstack/storage/datastore/lifecycle/ElastistorPrimaryDataStoreLifeCycle.java ---
@@ -359,7 +359,7 @@ public boolean attachCluster(DataStore store, ClusterScope scope) {
PrimaryDataStoreInfo primarystore = (PrimaryDataStoreInfo) store;
// Check if there is host up in this cluster
- List<HostVO> allHosts = _resourceMgr.listAllUpAndEnabledHosts(Host.Type.Routing, primarystore.getClusterId(), primarystore.getPodId(), primarystore.getDataCenterId());
+ List<HostVO> allHosts = _resourceMgr.listAllUpHosts(Host.Type.Routing, primarystore.getClusterId(), primarystore.getPodId(), primarystore.getDataCenterId());
--- End diff --
What about hosts which are in maintenance? Do we expect to add an SR to those hosts as well?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8865: Adding SR doesn't create...
Posted by alexandrelimassantana <gi...@git.apache.org>.
Github user alexandrelimassantana commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/876#discussion_r53570532
--- Diff: server/src/com/cloud/resource/ResourceManagerImpl.java ---
@@ -2378,6 +2378,23 @@ public boolean maintenanceFailed(final long hostId) {
}
@Override
+ public List<HostVO> listAllUpHosts(Type type, Long clusterId, Long podId, long dcId) {
--- End diff --
Hi @SudharmaJain
could you also write an unit test for this new method you created since it brings new functionalities to the code?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8865: Adding SR doesn't create...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/876#issuecomment-216194398
@SudharmaJain sorry but can you again please rebase against latest master and push -f, update on status of your PR
I would suggest you can add a marvin test that can be verified against Travis (simulator/mysql) that is more realistic than writing a unit test that mocks tcp stack and/or mysql server
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #876: CLOUDSTACK-8865: Adding SR doesn't create Storage_poo...
Posted by mrunalinikankariya <gi...@git.apache.org>.
Github user mrunalinikankariya commented on the issue:
https://github.com/apache/cloudstack/pull/876
LGTM on the code changes
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8865: Adding SR doesn't create...
Posted by SudharmaJain <gi...@git.apache.org>.
Github user SudharmaJain commented on the pull request:
https://github.com/apache/cloudstack/pull/876#issuecomment-212725838
Rebased with master.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #876: CLOUDSTACK-8865: Adding SR doesn't create Storage_poo...
Posted by PranaliM <gi...@git.apache.org>.
Github user PranaliM commented on the issue:
https://github.com/apache/cloudstack/pull/876
Test LGTM based on manual testing:
Steps used for testing the fix:
1. Added 2 hosts to a cluster
2. Disabled 1 host
3. Added a primary Storage
4. Verified that the mapping for primary Storage was added in the storage_pool_host_ref table for both
enabled and disabled host.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #876: CLOUDSTACK-8865: Adding SR doesn't create Stor...
Posted by SudharmaJain <gi...@git.apache.org>.
Github user SudharmaJain commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/876#discussion_r109859606
--- Diff: plugins/storage/volume/cloudbyte/src/org/apache/cloudstack/storage/datastore/lifecycle/ElastistorPrimaryDataStoreLifeCycle.java ---
@@ -359,7 +359,7 @@ public boolean attachCluster(DataStore store, ClusterScope scope) {
PrimaryDataStoreInfo primarystore = (PrimaryDataStoreInfo) store;
// Check if there is host up in this cluster
- List<HostVO> allHosts = _resourceMgr.listAllUpAndEnabledHosts(Host.Type.Routing, primarystore.getClusterId(), primarystore.getPodId(), primarystore.getDataCenterId());
+ List<HostVO> allHosts = _resourceMgr.listAllUpHosts(Host.Type.Routing, primarystore.getClusterId(), primarystore.getPodId(), primarystore.getDataCenterId());
--- End diff --
@syed We cannot send commands to the host in maintenance mode. So it is not possible to add an SR to those host.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8865: Adding SR doesn't create...
Posted by SudharmaJain <gi...@git.apache.org>.
Github user SudharmaJain commented on the pull request:
https://github.com/apache/cloudstack/pull/876#issuecomment-212794906
@alexandrelimassantana This method is about a query execution on database. Could you suggest how can i add a unit test for it?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #876: CLOUDSTACK-8865: Adding SR doesn't create Storage_poo...
Posted by SudharmaJain <gi...@git.apache.org>.
Github user SudharmaJain commented on the issue:
https://github.com/apache/cloudstack/pull/876
@rhtyd As suggested I have added a marvin test case.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---