You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by anshul1886 <gi...@git.apache.org> on 2016/12/22 06:21:46 UTC
[GitHub] cloudstack pull request #1853: CLOUDSTACK-9696: Fixed Virtual Router deploym...
GitHub user anshul1886 opened a pull request:
https://github.com/apache/cloudstack/pull/1853
CLOUDSTACK-9696: Fixed Virtual Router deployment failing if there are\u2026
\u2026 no shared resources available
and has access to resources through parent domain heirarachy
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/anshul1886/cloudstack-1 CLOUDSTACK-9696
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1853.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 #1853
----
commit b866776708ac664a8b71380b0f62009d0a88a3b3
Author: Anshul Gangwar <an...@accelerite.com>
Date: 2016-12-22T06:17:03Z
CLOUDSTACK-9696: Fixed Virtual Router deployment failing if there are no shared resources available
and has access to resources through parent domain heirarachy
----
---
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 #1853: CLOUDSTACK-9696: Fixed Virtual Router deploym...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1853#discussion_r106632802
--- Diff: server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java ---
@@ -584,105 +583,64 @@ private void checkForNonDedicatedResources(VirtualMachineProfile vmProfile, Data
isExplicit = true;
}
- List<Long> allPodsInDc = _podDao.listAllPods(dc.getId());
- List<Long> allDedicatedPods = _dedicatedDao.listAllPods();
- allPodsInDc.retainAll(allDedicatedPods);
-
- List<Long> allClustersInDc = _clusterDao.listAllCusters(dc.getId());
- List<Long> allDedicatedClusters = _dedicatedDao.listAllClusters();
- allClustersInDc.retainAll(allDedicatedClusters);
-
- List<Long> allHostsInDc = _hostDao.listAllHosts(dc.getId());
- List<Long> allDedicatedHosts = _dedicatedDao.listAllHosts();
- allHostsInDc.retainAll(allDedicatedHosts);
-
- //Only when the type is instance VM and not explicitly dedicated.
- if (vm.getType() == VirtualMachine.Type.User && !isExplicit) {
- //add explicitly dedicated resources in avoidList
-
- avoids.addPodList(allPodsInDc);
- avoids.addClusterList(allClustersInDc);
- avoids.addHostList(allHostsInDc);
- }
-
- //Handle the Virtual Router Case
- //No need to check the isExplicit. As both the cases are handled.
- if (vm.getType() == VirtualMachine.Type.DomainRouter) {
- long vmAccountId = vm.getAccountId();
- long vmDomainId = vm.getDomainId();
-
- //Lists all explicitly dedicated resources from vm account ID or domain ID.
- List<Long> allPodsFromDedicatedID = new ArrayList<Long>();
- List<Long> allClustersFromDedicatedID = new ArrayList<Long>();
- List<Long> allHostsFromDedicatedID = new ArrayList<Long>();
-
- //Whether the dedicated resources belong to Domain or not. If not, it may belongs to Account or no dedication.
- List<AffinityGroupDomainMapVO> domainGroupMappings = _affinityGroupDomainMapDao.listByDomain(vmDomainId);
-
- //For temporary storage and indexing.
- List<DedicatedResourceVO> tempStorage;
-
- if (domainGroupMappings == null || domainGroupMappings.isEmpty()) {
- //The dedicated resource belongs to VM Account ID.
-
- tempStorage = _dedicatedDao.searchDedicatedPods(null, vmDomainId, vmAccountId, null).first();
-
- for(DedicatedResourceVO vo : tempStorage) {
- allPodsFromDedicatedID.add(vo.getPodId());
- }
-
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedClusters(null, vmDomainId, vmAccountId, null).first();
+ if ((vm.getType() == VirtualMachine.Type.User && !isExplicit) || vm.getType() == VirtualMachine.Type.DomainRouter) {
+ List<Long> allPodsInDc = _podDao.listAllPods(dc.getId());
+ List<Long> allDedicatedPods = _dedicatedDao.listAllPods();
+ allPodsInDc.retainAll(allDedicatedPods);
- for(DedicatedResourceVO vo : tempStorage) {
- allClustersFromDedicatedID.add(vo.getClusterId());
- }
+ List<Long> allClustersInDc = _clusterDao.listAllCusters(dc.getId());
+ List<Long> allDedicatedClusters = _dedicatedDao.listAllClusters();
+ allClustersInDc.retainAll(allDedicatedClusters);
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedHosts(null, vmDomainId, vmAccountId, null).first();
+ List<Long> allHostsInDc = _hostDao.listAllHosts(dc.getId());
+ List<Long> allDedicatedHosts = _dedicatedDao.listAllHosts();
+ allHostsInDc.retainAll(allDedicatedHosts);
- for(DedicatedResourceVO vo : tempStorage) {
- allHostsFromDedicatedID.add(vo.getHostId());
- }
+ //Only when the type is instance VM and not explicitly dedicated.
+ if (vm.getType() == VirtualMachine.Type.User && !isExplicit) {
+ //add explicitly dedicated resources in avoidList
- //Remove the dedicated ones from main list
- allPodsInDc.removeAll(allPodsFromDedicatedID);
- allClustersInDc.removeAll(allClustersFromDedicatedID);
- allHostsInDc.removeAll(allHostsFromDedicatedID);
+ avoids.addPodList(allPodsInDc);
+ avoids.addClusterList(allClustersInDc);
+ avoids.addHostList(allHostsInDc);
}
- else {
- //The dedicated resource belongs to VM Domain ID or No dedication.
- tempStorage = _dedicatedDao.searchDedicatedPods(null, vmDomainId, null, null).first();
+ //Handle the Virtual Router Case
+ //No need to check the isExplicit. As both the cases are handled.
+ if (vm.getType() == VirtualMachine.Type.DomainRouter) {
+ long vmAccountId = vm.getAccountId();
+ long vmDomainId = vm.getDomainId();
- for(DedicatedResourceVO vo : tempStorage) {
- allPodsFromDedicatedID.add(vo.getPodId());
- }
+ List<Long> allPodsFromDedicatedID = new ArrayList<Long>();
+ List<Long> allClustersFromDedicatedID = new ArrayList<Long>();
+ List<Long> allHostsFromDedicatedID = new ArrayList<Long>();
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedClusters(null, vmDomainId, null, null).first();
-
- for(DedicatedResourceVO vo : tempStorage) {
- allClustersFromDedicatedID.add(vo.getClusterId());
- }
+ for (DedicatedResourceVO vo : _dedicatedDao.listAvailableResources(vmAccountId, vmDomainId)) {
+ if (vo.getPodId() != null) {
+ allPodsFromDedicatedID.add(vo.getPodId());
+ continue;
+ }
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedHosts(null, vmDomainId, null, null).first();
+ if (vo.getClusterId() != null) {
+ allClustersFromDedicatedID.add(vo.getClusterId());
+ continue;
+ }
- for(DedicatedResourceVO vo : tempStorage) {
- allHostsFromDedicatedID.add(vo.getHostId());
+ if (vo.getHostId() != null) {
+ allHostsFromDedicatedID.add(vo.getHostId());
+ }
}
//Remove the dedicated ones from main list
allPodsInDc.removeAll(allPodsFromDedicatedID);
--- End diff --
@koushik-das Renamed variables to make the code more understandable.
---
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 #1853: CLOUDSTACK-9696: Fixed Virtual Router deploym...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1853#discussion_r106138864
--- Diff: server/src/com/cloud/dc/dao/DedicatedResourceDaoImpl.java ---
@@ -312,6 +317,65 @@ public DedicatedResourceVO findByHostId(Long hostId) {
}
@Override
+ public List<DedicatedResourceVO> listAvailableResources(Long accountId, Long... domains) {
--- End diff --
Please rewrite this using the standard query builder (SearchBuilder) that is available and used in the other methods in this class.
---
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 #1853: CLOUDSTACK-9696: Fixed Virtual Router deploym...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1853#discussion_r106590720
--- Diff: server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java ---
@@ -584,105 +583,64 @@ private void checkForNonDedicatedResources(VirtualMachineProfile vmProfile, Data
isExplicit = true;
}
- List<Long> allPodsInDc = _podDao.listAllPods(dc.getId());
- List<Long> allDedicatedPods = _dedicatedDao.listAllPods();
- allPodsInDc.retainAll(allDedicatedPods);
-
- List<Long> allClustersInDc = _clusterDao.listAllCusters(dc.getId());
- List<Long> allDedicatedClusters = _dedicatedDao.listAllClusters();
- allClustersInDc.retainAll(allDedicatedClusters);
-
- List<Long> allHostsInDc = _hostDao.listAllHosts(dc.getId());
- List<Long> allDedicatedHosts = _dedicatedDao.listAllHosts();
- allHostsInDc.retainAll(allDedicatedHosts);
-
- //Only when the type is instance VM and not explicitly dedicated.
- if (vm.getType() == VirtualMachine.Type.User && !isExplicit) {
- //add explicitly dedicated resources in avoidList
-
- avoids.addPodList(allPodsInDc);
- avoids.addClusterList(allClustersInDc);
- avoids.addHostList(allHostsInDc);
- }
-
- //Handle the Virtual Router Case
- //No need to check the isExplicit. As both the cases are handled.
- if (vm.getType() == VirtualMachine.Type.DomainRouter) {
- long vmAccountId = vm.getAccountId();
- long vmDomainId = vm.getDomainId();
-
- //Lists all explicitly dedicated resources from vm account ID or domain ID.
- List<Long> allPodsFromDedicatedID = new ArrayList<Long>();
- List<Long> allClustersFromDedicatedID = new ArrayList<Long>();
- List<Long> allHostsFromDedicatedID = new ArrayList<Long>();
-
- //Whether the dedicated resources belong to Domain or not. If not, it may belongs to Account or no dedication.
- List<AffinityGroupDomainMapVO> domainGroupMappings = _affinityGroupDomainMapDao.listByDomain(vmDomainId);
-
- //For temporary storage and indexing.
- List<DedicatedResourceVO> tempStorage;
-
- if (domainGroupMappings == null || domainGroupMappings.isEmpty()) {
- //The dedicated resource belongs to VM Account ID.
-
- tempStorage = _dedicatedDao.searchDedicatedPods(null, vmDomainId, vmAccountId, null).first();
-
- for(DedicatedResourceVO vo : tempStorage) {
- allPodsFromDedicatedID.add(vo.getPodId());
- }
-
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedClusters(null, vmDomainId, vmAccountId, null).first();
+ if ((vm.getType() == VirtualMachine.Type.User && !isExplicit) || vm.getType() == VirtualMachine.Type.DomainRouter) {
+ List<Long> allPodsInDc = _podDao.listAllPods(dc.getId());
+ List<Long> allDedicatedPods = _dedicatedDao.listAllPods();
+ allPodsInDc.retainAll(allDedicatedPods);
- for(DedicatedResourceVO vo : tempStorage) {
- allClustersFromDedicatedID.add(vo.getClusterId());
- }
+ List<Long> allClustersInDc = _clusterDao.listAllCusters(dc.getId());
+ List<Long> allDedicatedClusters = _dedicatedDao.listAllClusters();
+ allClustersInDc.retainAll(allDedicatedClusters);
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedHosts(null, vmDomainId, vmAccountId, null).first();
+ List<Long> allHostsInDc = _hostDao.listAllHosts(dc.getId());
+ List<Long> allDedicatedHosts = _dedicatedDao.listAllHosts();
+ allHostsInDc.retainAll(allDedicatedHosts);
- for(DedicatedResourceVO vo : tempStorage) {
- allHostsFromDedicatedID.add(vo.getHostId());
- }
+ //Only when the type is instance VM and not explicitly dedicated.
+ if (vm.getType() == VirtualMachine.Type.User && !isExplicit) {
+ //add explicitly dedicated resources in avoidList
- //Remove the dedicated ones from main list
- allPodsInDc.removeAll(allPodsFromDedicatedID);
- allClustersInDc.removeAll(allClustersFromDedicatedID);
- allHostsInDc.removeAll(allHostsFromDedicatedID);
+ avoids.addPodList(allPodsInDc);
+ avoids.addClusterList(allClustersInDc);
+ avoids.addHostList(allHostsInDc);
}
- else {
- //The dedicated resource belongs to VM Domain ID or No dedication.
- tempStorage = _dedicatedDao.searchDedicatedPods(null, vmDomainId, null, null).first();
+ //Handle the Virtual Router Case
+ //No need to check the isExplicit. As both the cases are handled.
+ if (vm.getType() == VirtualMachine.Type.DomainRouter) {
+ long vmAccountId = vm.getAccountId();
+ long vmDomainId = vm.getDomainId();
- for(DedicatedResourceVO vo : tempStorage) {
- allPodsFromDedicatedID.add(vo.getPodId());
- }
+ List<Long> allPodsFromDedicatedID = new ArrayList<Long>();
+ List<Long> allClustersFromDedicatedID = new ArrayList<Long>();
+ List<Long> allHostsFromDedicatedID = new ArrayList<Long>();
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedClusters(null, vmDomainId, null, null).first();
-
- for(DedicatedResourceVO vo : tempStorage) {
- allClustersFromDedicatedID.add(vo.getClusterId());
- }
+ for (DedicatedResourceVO vo : _dedicatedDao.listAvailableResources(vmAccountId, vmDomainId)) {
+ if (vo.getPodId() != null) {
+ allPodsFromDedicatedID.add(vo.getPodId());
+ continue;
+ }
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedHosts(null, vmDomainId, null, null).first();
+ if (vo.getClusterId() != null) {
+ allClustersFromDedicatedID.add(vo.getClusterId());
+ continue;
+ }
- for(DedicatedResourceVO vo : tempStorage) {
- allHostsFromDedicatedID.add(vo.getHostId());
+ if (vo.getHostId() != null) {
+ allHostsFromDedicatedID.add(vo.getHostId());
+ }
}
//Remove the dedicated ones from main list
allPodsInDc.removeAll(allPodsFromDedicatedID);
--- End diff --
@anshul1886 As per the code for user VMs without explicit dedication, all dedicated resources are put in 'avoid' list so deployment happens in non-dedicated resources only. This part is clear. But for VR the opposite is happening as per the code. Can you explain the VR part from 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 #1853: CLOUDSTACK-9696: Fixed Virtual Router deploym...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1853#discussion_r106378310
--- Diff: server/src/com/cloud/dc/dao/DedicatedResourceDaoImpl.java ---
@@ -312,6 +317,65 @@ public DedicatedResourceVO findByHostId(Long hostId) {
}
@Override
+ public List<DedicatedResourceVO> listAvailableResources(Long accountId, Long... domains) {
--- End diff --
@koushik-das Could not find a way to write the conditional statement so went with this way. Here account_id in involved in two places in condition.
---
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 #1853: CLOUDSTACK-9696: Fixed Virtual Router deploym...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1853#discussion_r106145072
--- Diff: server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java ---
@@ -584,105 +583,64 @@ private void checkForNonDedicatedResources(VirtualMachineProfile vmProfile, Data
isExplicit = true;
}
- List<Long> allPodsInDc = _podDao.listAllPods(dc.getId());
- List<Long> allDedicatedPods = _dedicatedDao.listAllPods();
- allPodsInDc.retainAll(allDedicatedPods);
-
- List<Long> allClustersInDc = _clusterDao.listAllCusters(dc.getId());
- List<Long> allDedicatedClusters = _dedicatedDao.listAllClusters();
- allClustersInDc.retainAll(allDedicatedClusters);
-
- List<Long> allHostsInDc = _hostDao.listAllHosts(dc.getId());
- List<Long> allDedicatedHosts = _dedicatedDao.listAllHosts();
- allHostsInDc.retainAll(allDedicatedHosts);
-
- //Only when the type is instance VM and not explicitly dedicated.
- if (vm.getType() == VirtualMachine.Type.User && !isExplicit) {
- //add explicitly dedicated resources in avoidList
-
- avoids.addPodList(allPodsInDc);
- avoids.addClusterList(allClustersInDc);
- avoids.addHostList(allHostsInDc);
- }
-
- //Handle the Virtual Router Case
- //No need to check the isExplicit. As both the cases are handled.
- if (vm.getType() == VirtualMachine.Type.DomainRouter) {
- long vmAccountId = vm.getAccountId();
- long vmDomainId = vm.getDomainId();
-
- //Lists all explicitly dedicated resources from vm account ID or domain ID.
- List<Long> allPodsFromDedicatedID = new ArrayList<Long>();
- List<Long> allClustersFromDedicatedID = new ArrayList<Long>();
- List<Long> allHostsFromDedicatedID = new ArrayList<Long>();
-
- //Whether the dedicated resources belong to Domain or not. If not, it may belongs to Account or no dedication.
- List<AffinityGroupDomainMapVO> domainGroupMappings = _affinityGroupDomainMapDao.listByDomain(vmDomainId);
-
- //For temporary storage and indexing.
- List<DedicatedResourceVO> tempStorage;
-
- if (domainGroupMappings == null || domainGroupMappings.isEmpty()) {
- //The dedicated resource belongs to VM Account ID.
-
- tempStorage = _dedicatedDao.searchDedicatedPods(null, vmDomainId, vmAccountId, null).first();
-
- for(DedicatedResourceVO vo : tempStorage) {
- allPodsFromDedicatedID.add(vo.getPodId());
- }
-
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedClusters(null, vmDomainId, vmAccountId, null).first();
+ if ((vm.getType() == VirtualMachine.Type.User && !isExplicit) || vm.getType() == VirtualMachine.Type.DomainRouter) {
+ List<Long> allPodsInDc = _podDao.listAllPods(dc.getId());
+ List<Long> allDedicatedPods = _dedicatedDao.listAllPods();
+ allPodsInDc.retainAll(allDedicatedPods);
- for(DedicatedResourceVO vo : tempStorage) {
- allClustersFromDedicatedID.add(vo.getClusterId());
- }
+ List<Long> allClustersInDc = _clusterDao.listAllCusters(dc.getId());
+ List<Long> allDedicatedClusters = _dedicatedDao.listAllClusters();
+ allClustersInDc.retainAll(allDedicatedClusters);
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedHosts(null, vmDomainId, vmAccountId, null).first();
+ List<Long> allHostsInDc = _hostDao.listAllHosts(dc.getId());
+ List<Long> allDedicatedHosts = _dedicatedDao.listAllHosts();
+ allHostsInDc.retainAll(allDedicatedHosts);
- for(DedicatedResourceVO vo : tempStorage) {
- allHostsFromDedicatedID.add(vo.getHostId());
- }
+ //Only when the type is instance VM and not explicitly dedicated.
+ if (vm.getType() == VirtualMachine.Type.User && !isExplicit) {
+ //add explicitly dedicated resources in avoidList
- //Remove the dedicated ones from main list
- allPodsInDc.removeAll(allPodsFromDedicatedID);
- allClustersInDc.removeAll(allClustersFromDedicatedID);
- allHostsInDc.removeAll(allHostsFromDedicatedID);
+ avoids.addPodList(allPodsInDc);
+ avoids.addClusterList(allClustersInDc);
+ avoids.addHostList(allHostsInDc);
}
- else {
- //The dedicated resource belongs to VM Domain ID or No dedication.
- tempStorage = _dedicatedDao.searchDedicatedPods(null, vmDomainId, null, null).first();
+ //Handle the Virtual Router Case
+ //No need to check the isExplicit. As both the cases are handled.
+ if (vm.getType() == VirtualMachine.Type.DomainRouter) {
+ long vmAccountId = vm.getAccountId();
+ long vmDomainId = vm.getDomainId();
- for(DedicatedResourceVO vo : tempStorage) {
- allPodsFromDedicatedID.add(vo.getPodId());
- }
+ List<Long> allPodsFromDedicatedID = new ArrayList<Long>();
+ List<Long> allClustersFromDedicatedID = new ArrayList<Long>();
+ List<Long> allHostsFromDedicatedID = new ArrayList<Long>();
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedClusters(null, vmDomainId, null, null).first();
-
- for(DedicatedResourceVO vo : tempStorage) {
- allClustersFromDedicatedID.add(vo.getClusterId());
- }
+ for (DedicatedResourceVO vo : _dedicatedDao.listAvailableResources(vmAccountId, vmDomainId)) {
+ if (vo.getPodId() != null) {
+ allPodsFromDedicatedID.add(vo.getPodId());
+ continue;
+ }
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedHosts(null, vmDomainId, null, null).first();
+ if (vo.getClusterId() != null) {
+ allClustersFromDedicatedID.add(vo.getClusterId());
+ continue;
+ }
- for(DedicatedResourceVO vo : tempStorage) {
- allHostsFromDedicatedID.add(vo.getHostId());
+ if (vo.getHostId() != null) {
+ allHostsFromDedicatedID.add(vo.getHostId());
+ }
}
//Remove the dedicated ones from main list
allPodsInDc.removeAll(allPodsFromDedicatedID);
--- End diff --
Can you put some explanation as to why there is a difference in checking for dedicated resources for user VM and VR? In case of user VMs dedicated resources are put in avoid list, for VR only non-dedicated resources are put in avoid list.
---
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 #1853: CLOUDSTACK-9696: Fixed Virtual Router deploym...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1853#discussion_r106380082
--- Diff: server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java ---
@@ -584,105 +583,64 @@ private void checkForNonDedicatedResources(VirtualMachineProfile vmProfile, Data
isExplicit = true;
}
- List<Long> allPodsInDc = _podDao.listAllPods(dc.getId());
- List<Long> allDedicatedPods = _dedicatedDao.listAllPods();
- allPodsInDc.retainAll(allDedicatedPods);
-
- List<Long> allClustersInDc = _clusterDao.listAllCusters(dc.getId());
- List<Long> allDedicatedClusters = _dedicatedDao.listAllClusters();
- allClustersInDc.retainAll(allDedicatedClusters);
-
- List<Long> allHostsInDc = _hostDao.listAllHosts(dc.getId());
- List<Long> allDedicatedHosts = _dedicatedDao.listAllHosts();
- allHostsInDc.retainAll(allDedicatedHosts);
-
- //Only when the type is instance VM and not explicitly dedicated.
- if (vm.getType() == VirtualMachine.Type.User && !isExplicit) {
- //add explicitly dedicated resources in avoidList
-
- avoids.addPodList(allPodsInDc);
- avoids.addClusterList(allClustersInDc);
- avoids.addHostList(allHostsInDc);
- }
-
- //Handle the Virtual Router Case
- //No need to check the isExplicit. As both the cases are handled.
- if (vm.getType() == VirtualMachine.Type.DomainRouter) {
- long vmAccountId = vm.getAccountId();
- long vmDomainId = vm.getDomainId();
-
- //Lists all explicitly dedicated resources from vm account ID or domain ID.
- List<Long> allPodsFromDedicatedID = new ArrayList<Long>();
- List<Long> allClustersFromDedicatedID = new ArrayList<Long>();
- List<Long> allHostsFromDedicatedID = new ArrayList<Long>();
-
- //Whether the dedicated resources belong to Domain or not. If not, it may belongs to Account or no dedication.
- List<AffinityGroupDomainMapVO> domainGroupMappings = _affinityGroupDomainMapDao.listByDomain(vmDomainId);
-
- //For temporary storage and indexing.
- List<DedicatedResourceVO> tempStorage;
-
- if (domainGroupMappings == null || domainGroupMappings.isEmpty()) {
- //The dedicated resource belongs to VM Account ID.
-
- tempStorage = _dedicatedDao.searchDedicatedPods(null, vmDomainId, vmAccountId, null).first();
-
- for(DedicatedResourceVO vo : tempStorage) {
- allPodsFromDedicatedID.add(vo.getPodId());
- }
-
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedClusters(null, vmDomainId, vmAccountId, null).first();
+ if ((vm.getType() == VirtualMachine.Type.User && !isExplicit) || vm.getType() == VirtualMachine.Type.DomainRouter) {
--- End diff --
@koushik-das Here additional isExplicit check is needed.
---
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 #1853: CLOUDSTACK-9696: Fixed Virtual Router deploym...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1853#discussion_r106142726
--- Diff: server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java ---
@@ -584,105 +583,64 @@ private void checkForNonDedicatedResources(VirtualMachineProfile vmProfile, Data
isExplicit = true;
}
- List<Long> allPodsInDc = _podDao.listAllPods(dc.getId());
- List<Long> allDedicatedPods = _dedicatedDao.listAllPods();
- allPodsInDc.retainAll(allDedicatedPods);
-
- List<Long> allClustersInDc = _clusterDao.listAllCusters(dc.getId());
- List<Long> allDedicatedClusters = _dedicatedDao.listAllClusters();
- allClustersInDc.retainAll(allDedicatedClusters);
-
- List<Long> allHostsInDc = _hostDao.listAllHosts(dc.getId());
- List<Long> allDedicatedHosts = _dedicatedDao.listAllHosts();
- allHostsInDc.retainAll(allDedicatedHosts);
-
- //Only when the type is instance VM and not explicitly dedicated.
- if (vm.getType() == VirtualMachine.Type.User && !isExplicit) {
- //add explicitly dedicated resources in avoidList
-
- avoids.addPodList(allPodsInDc);
- avoids.addClusterList(allClustersInDc);
- avoids.addHostList(allHostsInDc);
- }
-
- //Handle the Virtual Router Case
- //No need to check the isExplicit. As both the cases are handled.
- if (vm.getType() == VirtualMachine.Type.DomainRouter) {
- long vmAccountId = vm.getAccountId();
- long vmDomainId = vm.getDomainId();
-
- //Lists all explicitly dedicated resources from vm account ID or domain ID.
- List<Long> allPodsFromDedicatedID = new ArrayList<Long>();
- List<Long> allClustersFromDedicatedID = new ArrayList<Long>();
- List<Long> allHostsFromDedicatedID = new ArrayList<Long>();
-
- //Whether the dedicated resources belong to Domain or not. If not, it may belongs to Account or no dedication.
- List<AffinityGroupDomainMapVO> domainGroupMappings = _affinityGroupDomainMapDao.listByDomain(vmDomainId);
-
- //For temporary storage and indexing.
- List<DedicatedResourceVO> tempStorage;
-
- if (domainGroupMappings == null || domainGroupMappings.isEmpty()) {
- //The dedicated resource belongs to VM Account ID.
-
- tempStorage = _dedicatedDao.searchDedicatedPods(null, vmDomainId, vmAccountId, null).first();
-
- for(DedicatedResourceVO vo : tempStorage) {
- allPodsFromDedicatedID.add(vo.getPodId());
- }
-
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedClusters(null, vmDomainId, vmAccountId, null).first();
+ if ((vm.getType() == VirtualMachine.Type.User && !isExplicit) || vm.getType() == VirtualMachine.Type.DomainRouter) {
--- End diff --
Any reason for this check? checkForNonDedicatedResources() is anyway called for user and router VMs.
---
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 #1853: CLOUDSTACK-9696: Fixed Virtual Router deploym...
Posted by anshul1886 <gi...@git.apache.org>.
GitHub user anshul1886 reopened a pull request:
https://github.com/apache/cloudstack/pull/1853
CLOUDSTACK-9696: Fixed Virtual Router deployment failing if there are\u2026
\u2026 no shared resources available
and has access to resources through parent domain heirarachy
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/anshul1886/cloudstack-1 CLOUDSTACK-9696
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1853.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 #1853
----
commit 0024dfc62266cc52380b7977860f667414d36e42
Author: Anshul Gangwar <an...@accelerite.com>
Date: 2016-12-22T06:17:03Z
CLOUDSTACK-9696: Fixed Virtual Router deployment failing if there are no shared resources available
and has access to resources through parent domain heirarachy
----
---
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 #1853: CLOUDSTACK-9696: Fixed Virtual Router deploym...
Posted by anshul1886 <gi...@git.apache.org>.
GitHub user anshul1886 reopened a pull request:
https://github.com/apache/cloudstack/pull/1853
CLOUDSTACK-9696: Fixed Virtual Router deployment failing if there are\u2026
\u2026 no shared resources available
and has access to resources through parent domain heirarachy
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/anshul1886/cloudstack-1 CLOUDSTACK-9696
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1853.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 #1853
----
commit 5f6f4ef3c634bbf60f157253253b5720a7177c0d
Author: Anshul Gangwar <an...@accelerite.com>
Date: 2016-12-22T06:17:03Z
CLOUDSTACK-9696: Fixed Virtual Router deployment failing if there are no shared resources available
and has access to resources through parent domain heirarachy
----
---
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 #1853: CLOUDSTACK-9696: Fixed Virtual Router deploym...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 closed the pull request at:
https://github.com/apache/cloudstack/pull/1853
---
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 #1853: CLOUDSTACK-9696: Fixed Virtual Router deploym...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 closed the pull request at:
https://github.com/apache/cloudstack/pull/1853
---
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 #1853: CLOUDSTACK-9696: Fixed Virtual Router deploym...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1853#discussion_r106381321
--- Diff: server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java ---
@@ -584,105 +583,64 @@ private void checkForNonDedicatedResources(VirtualMachineProfile vmProfile, Data
isExplicit = true;
}
- List<Long> allPodsInDc = _podDao.listAllPods(dc.getId());
- List<Long> allDedicatedPods = _dedicatedDao.listAllPods();
- allPodsInDc.retainAll(allDedicatedPods);
-
- List<Long> allClustersInDc = _clusterDao.listAllCusters(dc.getId());
- List<Long> allDedicatedClusters = _dedicatedDao.listAllClusters();
- allClustersInDc.retainAll(allDedicatedClusters);
-
- List<Long> allHostsInDc = _hostDao.listAllHosts(dc.getId());
- List<Long> allDedicatedHosts = _dedicatedDao.listAllHosts();
- allHostsInDc.retainAll(allDedicatedHosts);
-
- //Only when the type is instance VM and not explicitly dedicated.
- if (vm.getType() == VirtualMachine.Type.User && !isExplicit) {
- //add explicitly dedicated resources in avoidList
-
- avoids.addPodList(allPodsInDc);
- avoids.addClusterList(allClustersInDc);
- avoids.addHostList(allHostsInDc);
- }
-
- //Handle the Virtual Router Case
- //No need to check the isExplicit. As both the cases are handled.
- if (vm.getType() == VirtualMachine.Type.DomainRouter) {
- long vmAccountId = vm.getAccountId();
- long vmDomainId = vm.getDomainId();
-
- //Lists all explicitly dedicated resources from vm account ID or domain ID.
- List<Long> allPodsFromDedicatedID = new ArrayList<Long>();
- List<Long> allClustersFromDedicatedID = new ArrayList<Long>();
- List<Long> allHostsFromDedicatedID = new ArrayList<Long>();
-
- //Whether the dedicated resources belong to Domain or not. If not, it may belongs to Account or no dedication.
- List<AffinityGroupDomainMapVO> domainGroupMappings = _affinityGroupDomainMapDao.listByDomain(vmDomainId);
-
- //For temporary storage and indexing.
- List<DedicatedResourceVO> tempStorage;
-
- if (domainGroupMappings == null || domainGroupMappings.isEmpty()) {
- //The dedicated resource belongs to VM Account ID.
-
- tempStorage = _dedicatedDao.searchDedicatedPods(null, vmDomainId, vmAccountId, null).first();
-
- for(DedicatedResourceVO vo : tempStorage) {
- allPodsFromDedicatedID.add(vo.getPodId());
- }
-
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedClusters(null, vmDomainId, vmAccountId, null).first();
+ if ((vm.getType() == VirtualMachine.Type.User && !isExplicit) || vm.getType() == VirtualMachine.Type.DomainRouter) {
+ List<Long> allPodsInDc = _podDao.listAllPods(dc.getId());
+ List<Long> allDedicatedPods = _dedicatedDao.listAllPods();
+ allPodsInDc.retainAll(allDedicatedPods);
- for(DedicatedResourceVO vo : tempStorage) {
- allClustersFromDedicatedID.add(vo.getClusterId());
- }
+ List<Long> allClustersInDc = _clusterDao.listAllCusters(dc.getId());
+ List<Long> allDedicatedClusters = _dedicatedDao.listAllClusters();
+ allClustersInDc.retainAll(allDedicatedClusters);
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedHosts(null, vmDomainId, vmAccountId, null).first();
+ List<Long> allHostsInDc = _hostDao.listAllHosts(dc.getId());
+ List<Long> allDedicatedHosts = _dedicatedDao.listAllHosts();
+ allHostsInDc.retainAll(allDedicatedHosts);
- for(DedicatedResourceVO vo : tempStorage) {
- allHostsFromDedicatedID.add(vo.getHostId());
- }
+ //Only when the type is instance VM and not explicitly dedicated.
+ if (vm.getType() == VirtualMachine.Type.User && !isExplicit) {
+ //add explicitly dedicated resources in avoidList
- //Remove the dedicated ones from main list
- allPodsInDc.removeAll(allPodsFromDedicatedID);
- allClustersInDc.removeAll(allClustersFromDedicatedID);
- allHostsInDc.removeAll(allHostsFromDedicatedID);
+ avoids.addPodList(allPodsInDc);
+ avoids.addClusterList(allClustersInDc);
+ avoids.addHostList(allHostsInDc);
}
- else {
- //The dedicated resource belongs to VM Domain ID or No dedication.
- tempStorage = _dedicatedDao.searchDedicatedPods(null, vmDomainId, null, null).first();
+ //Handle the Virtual Router Case
+ //No need to check the isExplicit. As both the cases are handled.
+ if (vm.getType() == VirtualMachine.Type.DomainRouter) {
+ long vmAccountId = vm.getAccountId();
+ long vmDomainId = vm.getDomainId();
- for(DedicatedResourceVO vo : tempStorage) {
- allPodsFromDedicatedID.add(vo.getPodId());
- }
+ List<Long> allPodsFromDedicatedID = new ArrayList<Long>();
+ List<Long> allClustersFromDedicatedID = new ArrayList<Long>();
+ List<Long> allHostsFromDedicatedID = new ArrayList<Long>();
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedClusters(null, vmDomainId, null, null).first();
-
- for(DedicatedResourceVO vo : tempStorage) {
- allClustersFromDedicatedID.add(vo.getClusterId());
- }
+ for (DedicatedResourceVO vo : _dedicatedDao.listAvailableResources(vmAccountId, vmDomainId)) {
+ if (vo.getPodId() != null) {
+ allPodsFromDedicatedID.add(vo.getPodId());
+ continue;
+ }
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedHosts(null, vmDomainId, null, null).first();
+ if (vo.getClusterId() != null) {
+ allClustersFromDedicatedID.add(vo.getClusterId());
+ continue;
+ }
- for(DedicatedResourceVO vo : tempStorage) {
- allHostsFromDedicatedID.add(vo.getHostId());
+ if (vo.getHostId() != null) {
+ allHostsFromDedicatedID.add(vo.getHostId());
+ }
}
//Remove the dedicated ones from main list
allPodsInDc.removeAll(allPodsFromDedicatedID);
--- End diff --
@koushik-das User can specify the affinity group for VM while deploying VM. So if user has specified the affinity group then only that VM needs to go to dedicated hosts. While in case of router user has not such option and needs to be inferred from account/domain of the user. These whole checks are mainly introduced to avoid virtual router going to dedicated resources of other account/domain.
---
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 #1853: CLOUDSTACK-9696: Fixed Virtual Router deploym...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1853#discussion_r106632584
--- Diff: server/src/com/cloud/dc/dao/DedicatedResourceDaoImpl.java ---
@@ -312,6 +317,65 @@ public DedicatedResourceVO findByHostId(Long hostId) {
}
@Override
+ public List<DedicatedResourceVO> listAvailableResources(Long accountId, Long... domains) {
--- End diff --
@koushik-das Made the changes using SearchBuilder class.
---
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 #1853: CLOUDSTACK-9696: Fixed Virtual Router deployment fai...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the issue:
https://github.com/apache/cloudstack/pull/1853
Code change LGTM.
@anshul1886 Please make sure Travis is green.
---
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 #1853: CLOUDSTACK-9696: Fixed Virtual Router deploym...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1853#discussion_r106589966
--- Diff: server/src/com/cloud/dc/dao/DedicatedResourceDaoImpl.java ---
@@ -312,6 +317,65 @@ public DedicatedResourceVO findByHostId(Long hostId) {
}
@Override
+ public List<DedicatedResourceVO> listAvailableResources(Long accountId, Long... domains) {
--- End diff --
@anshul1886 Check out SearchBuilder class, it has methods like or(), op(), cp() which can be used to construct the query. Also the same field can be used in multiple places, search for reference to or(), op() etc. elsewhere in the code and you will find how to use them. One such place is QueryManagerImpl.java
---
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 #1853: CLOUDSTACK-9696: Fixed Virtual Router deployment fai...
Posted by anshul1886 <gi...@git.apache.org>.
Github user anshul1886 commented on the issue:
https://github.com/apache/cloudstack/pull/1853
@koushik-das Made the changes as per review comments.
---
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 #1853: CLOUDSTACK-9696: Fixed Virtual Router deploym...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1853#discussion_r106591079
--- Diff: server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java ---
@@ -584,105 +583,64 @@ private void checkForNonDedicatedResources(VirtualMachineProfile vmProfile, Data
isExplicit = true;
}
- List<Long> allPodsInDc = _podDao.listAllPods(dc.getId());
- List<Long> allDedicatedPods = _dedicatedDao.listAllPods();
- allPodsInDc.retainAll(allDedicatedPods);
-
- List<Long> allClustersInDc = _clusterDao.listAllCusters(dc.getId());
- List<Long> allDedicatedClusters = _dedicatedDao.listAllClusters();
- allClustersInDc.retainAll(allDedicatedClusters);
-
- List<Long> allHostsInDc = _hostDao.listAllHosts(dc.getId());
- List<Long> allDedicatedHosts = _dedicatedDao.listAllHosts();
- allHostsInDc.retainAll(allDedicatedHosts);
-
- //Only when the type is instance VM and not explicitly dedicated.
- if (vm.getType() == VirtualMachine.Type.User && !isExplicit) {
- //add explicitly dedicated resources in avoidList
-
- avoids.addPodList(allPodsInDc);
- avoids.addClusterList(allClustersInDc);
- avoids.addHostList(allHostsInDc);
- }
-
- //Handle the Virtual Router Case
- //No need to check the isExplicit. As both the cases are handled.
- if (vm.getType() == VirtualMachine.Type.DomainRouter) {
- long vmAccountId = vm.getAccountId();
- long vmDomainId = vm.getDomainId();
-
- //Lists all explicitly dedicated resources from vm account ID or domain ID.
- List<Long> allPodsFromDedicatedID = new ArrayList<Long>();
- List<Long> allClustersFromDedicatedID = new ArrayList<Long>();
- List<Long> allHostsFromDedicatedID = new ArrayList<Long>();
-
- //Whether the dedicated resources belong to Domain or not. If not, it may belongs to Account or no dedication.
- List<AffinityGroupDomainMapVO> domainGroupMappings = _affinityGroupDomainMapDao.listByDomain(vmDomainId);
-
- //For temporary storage and indexing.
- List<DedicatedResourceVO> tempStorage;
-
- if (domainGroupMappings == null || domainGroupMappings.isEmpty()) {
- //The dedicated resource belongs to VM Account ID.
-
- tempStorage = _dedicatedDao.searchDedicatedPods(null, vmDomainId, vmAccountId, null).first();
-
- for(DedicatedResourceVO vo : tempStorage) {
- allPodsFromDedicatedID.add(vo.getPodId());
- }
-
- tempStorage.clear();
- tempStorage = _dedicatedDao.searchDedicatedClusters(null, vmDomainId, vmAccountId, null).first();
+ if ((vm.getType() == VirtualMachine.Type.User && !isExplicit) || vm.getType() == VirtualMachine.Type.DomainRouter) {
--- End diff --
@anshul1886 Got 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.
---