[GitHub] cloudstack issue #1935: CLOUDSTACK-9764: Delete domain failure due to Accoun...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1935 @rafaelweingartner thanks for reviewing! I extracted code to new methods and also added unit tests for them --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1935: CLOUDSTACK-9764: Delete domain failure due to Accoun...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1935 @rafaelweingartner I think I got your point, I tried to keep code as similar as it was before, by declaring `rollBackState` as static class variable on line 114. This way inner `finally` block would work the same as before when one of new methods set `rollBackState = true.` On outter `finally` block, `rollBackState` is set to false (line 345), this way each time `deleteDomain` is invoked it would start on false (maybe it would be easier to move it at the beggining of `deleteDomain`). Do you agree with this approach? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1847: CLOUDSTACK-9691: Fixed unhandeled excetion in list s...
Github user nvazquez commented on the issue:
https://github.com/apache/cloudstack/pull/1847
@borisstoyanov @rhtyd I was checking BlueOrangutan logs:
In `test_primary_storage_8NPG5G\runinfo.txt` lines 27-30, there's PS
creation:
2017-02-20 11:03:59,678 - DEBUG - Payload: {'apiKey':
u'LIN6rqXuaJwMPfGYFh13qDwYz5VNNz1J2J6qIOWcd3oLQOq0WtD4CwRundBL6rzXToa3lQOC_vKjI3nkHtiD8Q',
'name': 'Marvin Primary Pool', 'url':
'NFS://10.2.0.16/acs/primary/pr1847-t854-kvm-centos7/marvin_pri1', 'podid':
u'1c6f35e3-a31e-49cc-b0b0-801848dbb9bc', 'clusterid':
u'8ac40777-344d-4f36-93a3-7b685361a523', 'zoneid':
u'9c474c4e-0742-4fbb-b0ae-cb3d1e63cb70', 'command': 'createStoragePool',
'signature': 'zq6ff7P2n9iLmw3Cw9nDgMM1v90=', 'response': 'json'}
2017-02-20 11:03:59,678 - DEBUG - Sending GET Cmd :
createStoragePool===
2017-02-20 11:04:00,038 - DEBUG - Response : {podname : u'Pod1', name :
u'Marvin Primary Pool', disksizeallocated : 0, created :
u'2017-02-20T11:04:00+', clustername : u'p1-c1', ipaddress : u'10.2.0.16',
podid : u'1c6f35e3-a31e-49cc-b0b0-801848dbb9bc', clusterid :
u'8ac40777-344d-4f36-93a3-7b685361a523', zoneid :
u'9c474c4e-0742-4fbb-b0ae-cb3d1e63cb70', state : u'Up', scope : u'CLUSTER',
overprovisionfactor : u'2.0', path :
u'/acs/primary/pr1847-t854-kvm-centos7/marvin_pri1', zonename :
u'pr1847-t854-kvm-centos7', type : u'NetworkFilesystem', id :
u'fa44baa2-267d-3f36-88c9-c5c545e0204b', disksizetotal : 7514055770112}
2017-02-20 11:04:00,038 - DEBUG - Created storage pool in cluster:
8ac40777-344d-4f36-93a3-7b685361a523
Then, it gets removed, lines 46-48:
2017-02-20 11:04:35,193 - DEBUG - Payload: {'apiKey':
u'LIN6rqXuaJwMPfGYFh13qDwYz5VNNz1J2J6qIOWcd3oLQOq0WtD4CwRundBL6rzXToa3lQOC_vKjI3nkHtiD8Q',
'response': 'json', 'command': 'deleteStoragePool', 'signature':
'73NDgRVRZDU4uHR/Vc8pRbVg2p4=', 'id': u'fa44baa2-267d-3f36-88c9-c5c545e0204b'}
2017-02-20 11:04:35,193 - DEBUG - Sending GET Cmd :
deleteStoragePool===
2017-02-20 11:04:35,396 - DEBUG - Response : {success : u'true'}
Then, on `test_volumes_950C4W\runinfo.txt` lines 474-476 `listStoragePools`
command is sent and lists previously deleted PS:
2017-02-20 13:54:49,407 - DEBUG - Payload: {'apiKey':
u'LIN6rqXuaJwMPfGYFh13qDwYz5VNNz1J2J6qIOWcd3oLQOq0WtD4CwRundBL6rzXToa3lQOC_vKjI3nkHtiD8Q',
'command': 'listStoragePools', 'signature': 'l3xL+RNaPCVAYEpCovbu1snTIRI=',
'response': 'json'}
2017-02-20 13:54:49,407 - DEBUG - Sending GET Cmd :
listStoragePools===
2017-02-20 13:54:49,432 - DEBUG - Response : [{podname : u'Pod1',
storagecapabilities : {VOLUME_SNAPSHOT_QUIESCEVM : u'false'}, name : u'Marvin
Primary Pool', disksizeallocated : 8590328832, podid :
u'1c6f35e3-a31e-49cc-b0b0-801848dbb9bc', clustername : u'p1-c1', ipaddress :
u'10.2.0.16', created : u'2017-02-20T12:51:45+', clusterid :
u'8ac40777-344d-4f36-93a3-7b685361a523', zoneid :
u'9c474c4e-0742-4fbb-b0ae-cb3d1e63cb70', state : u'Up', disksizeused :
174845853696, id : u'fa44baa2-267d-3f36-88c9-c5c545e0204b', overprovisionfactor
: u'2.0', path : u'/acs/primary/pr1847-t854-kvm-centos7/marvin_pri1', zonename
: u'pr1847-t854-kvm-centos7', type : u'NetworkFilesystem', scope : u'CLUSTER',
disksizetotal : 7514055770112}, {podname : u'Pod1', storagecapabilities :
{VOLUME_SNAPSHOT_QUIESCEVM : u'false'}, name :
u'pr1847-t854-kvm-centos7-kvm-pri2', disksizeallocated : 977060864, podid :
u'1c6f35e3-a31e-49cc-b0b0-801848dbb9bc', clustername : u'p1-c1', ipaddress :
u'10.2.0.16', create
d : u'2017-02-20T08:52:51+', clusterid :
u'8ac40777-344d-4f36-93a3-7b685361a523', zoneid :
u'9c474c4e-0742-4fbb-b0ae-cb3d1e63cb70', state : u'Up', disksizeused :
174845853696, id : u'7a360219-f4a9-3edb-a1e3-241cbc2dee7f', overprovisionfactor
: u'2.0', path :
u'/acs/primary/pr1847-t854-kvm-centos7/pr1847-t854-kvm-centos7-kvm-pri2',
zonename : u'pr1847-t854-kvm-centos7', type : u'NetworkFilesystem', scope :
u'CLUSTER', disksizetotal : 7514055770112}, {podname : u'Pod1',
storagecapabilities :
[GitHub] cloudstack issue #1935: CLOUDSTACK-9764: Delete domain failure due to Accoun...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1935 @rafaelweingartner no problem, I should have mentioned about changing the variable to static. I'll work on your last 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1935: CLOUDSTACK-9764: Delete domain failure due to...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1935#discussion_r102530895
--- Diff: server/src/com/cloud/user/DomainManagerImpl.java ---
@@ -273,82 +289,133 @@ public boolean deleteDomain(long domainId, Boolean
cleanup) {
@Override
public boolean deleteDomain(DomainVO domain, Boolean cleanup) {
-// mark domain as inactive
-s_logger.debug("Marking domain id=" + domain.getId() + " as " +
Domain.State.Inactive + " before actually deleting it");
-domain.setState(Domain.State.Inactive);
-_domainDao.update(domain.getId(), domain);
-boolean rollBackState = false;
-boolean hasDedicatedResources = false;
+GlobalLock lock = getGlobalLock("AccountCleanup");
+if (lock == null) {
+s_logger.debug("Couldn't get the global lock");
+return false;
+}
+
+if (!lock.lock(30)) {
+s_logger.debug("Couldn't lock the db");
+return false;
+}
try {
-long ownerId = domain.getAccountId();
-if ((cleanup != null) && cleanup.booleanValue()) {
-if (!cleanupDomain(domain.getId(), ownerId)) {
-rollBackState = true;
-CloudRuntimeException e =
-new CloudRuntimeException("Failed to clean up
domain resources and sub domains, delete failed on domain " + domain.getName()
+ " (id: " +
-domain.getId() + ").");
-e.addProxyObject(domain.getUuid(), "domainId");
-throw e;
-}
-} else {
-//don't delete the domain if there are accounts set for
cleanup, or non-removed networks exist, or domain has dedicated resources
-List networkIds =
_networkDomainDao.listNetworkIdsByDomain(domain.getId());
-List accountsForCleanup =
_accountDao.findCleanupsForRemovedAccounts(domain.getId());
-List dedicatedResources =
_dedicatedDao.listByDomainId(domain.getId());
-if (dedicatedResources != null &&
!dedicatedResources.isEmpty()) {
-s_logger.error("There are dedicated resources for the
domain " + domain.getId());
-hasDedicatedResources = true;
-}
-if (accountsForCleanup.isEmpty() && networkIds.isEmpty()
&& !hasDedicatedResources) {
-_messageBus.publish(_name,
MESSAGE_PRE_REMOVE_DOMAIN_EVENT, PublishScope.LOCAL, domain);
-if (!_domainDao.remove(domain.getId())) {
+// mark domain as inactive
+s_logger.debug("Marking domain id=" + domain.getId() + " as "
+ Domain.State.Inactive + " before actually deleting it");
+domain.setState(Domain.State.Inactive);
+_domainDao.update(domain.getId(), domain);
+
+try {
+long ownerId = domain.getAccountId();
+if (BooleanUtils.toBoolean(cleanup)) {
+if (!cleanupDomain(domain.getId(), ownerId)) {
rollBackState = true;
CloudRuntimeException e =
-new CloudRuntimeException("Delete failed on
domain " + domain.getName() + " (id: " + domain.getId() +
-"); Please make sure all users and sub
domains have been removed from the domain before deleting");
+new CloudRuntimeException("Failed to clean up
domain resources and sub domains, delete failed on domain " + domain.getName()
+ " (id: " +
+domain.getId() + ").");
e.addProxyObject(domain.getUuid(), "domainId");
throw e;
}
-_messageBus.publish(_name,
MESSAGE_REMOVE_DOMAIN_EVENT, PublishScope.LOCAL, domain);
} else {
-rollBackState = true;
-String msg = null;
-if (!accountsForCleanup.isEmpty()) {
-msg = accountsForCleanup.size() + " accounts to
cleanup";
-} else if (!networkIds.isEmpty()) {
-msg = networkIds.size() + " non-removed networks";
-} else if (hasDedicatedResources) {
-
[GitHub] cloudstack pull request #1935: CLOUDSTACK-9764: Delete domain failure due to...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1935#discussion_r102531047
--- Diff: server/test/com/cloud/user/DomainManagerImplTest.java ---
@@ -134,4 +164,67 @@ public void testFindDomainByIdOrPathValidId() {
Assert.assertEquals(domain, domainManager.findDomainByIdOrPath(1L,
"/validDomain/"));
}
+@Test(expected=InvalidParameterValueException.class)
+public void testDeleteDomainNullDomain() {
+Mockito.when(_domainDao.findById(DOMAIN_ID)).thenReturn(null);
+domainManager.deleteDomain(DOMAIN_ID, testDomainCleanup);
+}
+
+@Test(expected=PermissionDeniedException.class)
+public void testDeleteDomainRootDomain() {
+
Mockito.when(_domainDao.findById(Domain.ROOT_DOMAIN)).thenReturn(domain);
+domainManager.deleteDomain(Domain.ROOT_DOMAIN, testDomainCleanup);
+}
+
+//TODO testDeleteDomainCleanup
+
+@Test
+public void testDeleteDomainNoCleanup() {
+domainManager.deleteDomain(DOMAIN_ID, testDomainCleanup);
+Mockito.verify(domainManager).deleteDomain(domain,
testDomainCleanup);
+
Mockito.verify(domainManager).checkDomainAccountsNetworksAndResourcesBeforeRemoving(domain);
+Mockito.verify(domainManager).cleanupDomainOfferings(DOMAIN_ID);
+Mockito.verify(lock).unlock();
+Assert.assertEquals(false, domainManager.getRollBackState());
+}
+
+@Test
+public void
testCheckDomainAccountsNetworksAndResourcesBeforeRemovingRemoveDomain() {
+
domainManager.checkDomainAccountsNetworksAndResourcesBeforeRemoving(domain);
+
Mockito.verify(domainManager).publishRemoveEventsAndRemoveDomain(domain);
+}
+
+@Test(expected=CloudRuntimeException.class)
+public void
testCheckDomainAccountsNetworksAndResourcesBeforeRemovingDontRemoveDomain() {
+domainNetworkIds.add(2l);
+
domainManager.checkDomainAccountsNetworksAndResourcesBeforeRemoving(domain);
+Mockito.verify(domainManager).failRemoveOperation(domain,
domainAccountsForCleanup, domainNetworkIds, false);
+}
+
+@Test
+public void testPublishRemoveEventsAndRemoveDomainSuccessfulDelete() {
+domainManager.publishRemoveEventsAndRemoveDomain(domain);
+Mockito.verify(_messageBus).publish(Mockito.anyString(),
Matchers.eq(DomainManager.MESSAGE_PRE_REMOVE_DOMAIN_EVENT),
+Matchers.eq(PublishScope.LOCAL), Matchers.eq(domain));
+Mockito.verify(_messageBus).publish(Mockito.anyString(),
Matchers.eq(DomainManager.MESSAGE_REMOVE_DOMAIN_EVENT),
+Matchers.eq(PublishScope.LOCAL), Matchers.eq(domain));
+Mockito.verify(_domainDao).remove(DOMAIN_ID);
+}
+
+@Test(expected=CloudRuntimeException.class)
+public void testPublishRemoveEventsAndRemoveDomainExceptionDelete() {
+Mockito.when(_domainDao.remove(DOMAIN_ID)).thenReturn(false);
+domainManager.publishRemoveEventsAndRemoveDomain(domain);
+Mockito.verify(_messageBus).publish(Mockito.anyString(),
Matchers.eq(DomainManager.MESSAGE_PRE_REMOVE_DOMAIN_EVENT),
+Matchers.eq(PublishScope.LOCAL), Matchers.eq(domain));
+Mockito.verify(_messageBus,
Mockito.never()).publish(Mockito.anyString(),
Matchers.eq(DomainManager.MESSAGE_REMOVE_DOMAIN_EVENT),
+Matchers.eq(PublishScope.LOCAL), Matchers.eq(domain));
+Mockito.verify(_domainDao).remove(DOMAIN_ID);
+}
+
+@Test(expected=CloudRuntimeException.class)
+public void testFailRemoveOperation() {
--- End diff --
Great, I've added verification, I have missed that out, thanks!
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1935: CLOUDSTACK-9764: Delete domain failure due to...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1935#discussion_r102529801
--- Diff: server/src/com/cloud/user/DomainManagerImpl.java ---
@@ -273,82 +289,133 @@ public boolean deleteDomain(long domainId, Boolean
cleanup) {
@Override
public boolean deleteDomain(DomainVO domain, Boolean cleanup) {
-// mark domain as inactive
-s_logger.debug("Marking domain id=" + domain.getId() + " as " +
Domain.State.Inactive + " before actually deleting it");
-domain.setState(Domain.State.Inactive);
-_domainDao.update(domain.getId(), domain);
-boolean rollBackState = false;
-boolean hasDedicatedResources = false;
+GlobalLock lock = getGlobalLock("AccountCleanup");
+if (lock == null) {
+s_logger.debug("Couldn't get the global lock");
+return false;
+}
+
+if (!lock.lock(30)) {
+s_logger.debug("Couldn't lock the db");
+return false;
+}
try {
-long ownerId = domain.getAccountId();
-if ((cleanup != null) && cleanup.booleanValue()) {
-if (!cleanupDomain(domain.getId(), ownerId)) {
-rollBackState = true;
-CloudRuntimeException e =
-new CloudRuntimeException("Failed to clean up
domain resources and sub domains, delete failed on domain " + domain.getName()
+ " (id: " +
-domain.getId() + ").");
-e.addProxyObject(domain.getUuid(), "domainId");
-throw e;
-}
-} else {
-//don't delete the domain if there are accounts set for
cleanup, or non-removed networks exist, or domain has dedicated resources
-List networkIds =
_networkDomainDao.listNetworkIdsByDomain(domain.getId());
-List accountsForCleanup =
_accountDao.findCleanupsForRemovedAccounts(domain.getId());
-List dedicatedResources =
_dedicatedDao.listByDomainId(domain.getId());
-if (dedicatedResources != null &&
!dedicatedResources.isEmpty()) {
-s_logger.error("There are dedicated resources for the
domain " + domain.getId());
-hasDedicatedResources = true;
-}
-if (accountsForCleanup.isEmpty() && networkIds.isEmpty()
&& !hasDedicatedResources) {
-_messageBus.publish(_name,
MESSAGE_PRE_REMOVE_DOMAIN_EVENT, PublishScope.LOCAL, domain);
-if (!_domainDao.remove(domain.getId())) {
+// mark domain as inactive
+s_logger.debug("Marking domain id=" + domain.getId() + " as "
+ Domain.State.Inactive + " before actually deleting it");
+domain.setState(Domain.State.Inactive);
+_domainDao.update(domain.getId(), domain);
+
+try {
+long ownerId = domain.getAccountId();
+if (BooleanUtils.toBoolean(cleanup)) {
+if (!cleanupDomain(domain.getId(), ownerId)) {
--- End diff --
Done, I liked it how it simplified the code inside those blocks. However, I
still find it difficult to name methods, do you agree with name
`tryCleanupDomain`?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1935: CLOUDSTACK-9764: Delete domain failure due to...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1935#discussion_r102531132
--- Diff: server/src/com/cloud/user/DomainManagerImpl.java ---
@@ -273,82 +289,133 @@ public boolean deleteDomain(long domainId, Boolean
cleanup) {
@Override
public boolean deleteDomain(DomainVO domain, Boolean cleanup) {
-// mark domain as inactive
-s_logger.debug("Marking domain id=" + domain.getId() + " as " +
Domain.State.Inactive + " before actually deleting it");
-domain.setState(Domain.State.Inactive);
-_domainDao.update(domain.getId(), domain);
-boolean rollBackState = false;
-boolean hasDedicatedResources = false;
+GlobalLock lock = getGlobalLock("AccountCleanup");
+if (lock == null) {
+s_logger.debug("Couldn't get the global lock");
+return false;
+}
+
+if (!lock.lock(30)) {
+s_logger.debug("Couldn't lock the db");
+return false;
+}
try {
-long ownerId = domain.getAccountId();
-if ((cleanup != null) && cleanup.booleanValue()) {
-if (!cleanupDomain(domain.getId(), ownerId)) {
-rollBackState = true;
-CloudRuntimeException e =
-new CloudRuntimeException("Failed to clean up
domain resources and sub domains, delete failed on domain " + domain.getName()
+ " (id: " +
-domain.getId() + ").");
-e.addProxyObject(domain.getUuid(), "domainId");
-throw e;
-}
-} else {
-//don't delete the domain if there are accounts set for
cleanup, or non-removed networks exist, or domain has dedicated resources
-List networkIds =
_networkDomainDao.listNetworkIdsByDomain(domain.getId());
-List accountsForCleanup =
_accountDao.findCleanupsForRemovedAccounts(domain.getId());
-List dedicatedResources =
_dedicatedDao.listByDomainId(domain.getId());
-if (dedicatedResources != null &&
!dedicatedResources.isEmpty()) {
-s_logger.error("There are dedicated resources for the
domain " + domain.getId());
-hasDedicatedResources = true;
-}
-if (accountsForCleanup.isEmpty() && networkIds.isEmpty()
&& !hasDedicatedResources) {
-_messageBus.publish(_name,
MESSAGE_PRE_REMOVE_DOMAIN_EVENT, PublishScope.LOCAL, domain);
-if (!_domainDao.remove(domain.getId())) {
+// mark domain as inactive
+s_logger.debug("Marking domain id=" + domain.getId() + " as "
+ Domain.State.Inactive + " before actually deleting it");
+domain.setState(Domain.State.Inactive);
+_domainDao.update(domain.getId(), domain);
+
+try {
+long ownerId = domain.getAccountId();
+if (BooleanUtils.toBoolean(cleanup)) {
+if (!cleanupDomain(domain.getId(), ownerId)) {
rollBackState = true;
CloudRuntimeException e =
-new CloudRuntimeException("Delete failed on
domain " + domain.getName() + " (id: " + domain.getId() +
-"); Please make sure all users and sub
domains have been removed from the domain before deleting");
+new CloudRuntimeException("Failed to clean up
domain resources and sub domains, delete failed on domain " + domain.getName()
+ " (id: " +
+domain.getId() + ").");
e.addProxyObject(domain.getUuid(), "domainId");
throw e;
}
-_messageBus.publish(_name,
MESSAGE_REMOVE_DOMAIN_EVENT, PublishScope.LOCAL, domain);
} else {
-rollBackState = true;
-String msg = null;
-if (!accountsForCleanup.isEmpty()) {
-msg = accountsForCleanup.size() + " accounts to
cleanup";
-} else if (!networkIds.isEmpty()) {
-msg = networkIds.size() + " non-removed networks";
-} else if (hasDedicatedResources) {
-
[GitHub] cloudstack pull request #1961: Fix for test_snapshots.py using nfs2 instead ...
GitHub user nvazquez opened a pull request: https://github.com/apache/cloudstack/pull/1961 Fix for test_snapshots.py using nfs2 instead of nfs template Fix for marvin test failure introduced in #1847 Cc: @borisstoyanov @rhtyd @karuturi You can merge this pull request into a Git repository by running: $ git pull https://github.com/nvazquez/cloudstack addNewNfsTestData Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack/pull/1961.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 #1961 commit e0c2ad887552b090e3a4cfabfa1d9b62e4e22a61 Author: nvazquez Date: 2017-02-22T18:54:02Z Fix for test_snapshots.py using nfs2 instead of nfs template --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1961: Fix for test_snapshots.py using nfs2 instead of nfs ...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1961 @borisstoyanov great, thanks! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1935: CLOUDSTACK-9764: Delete domain failure due to...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1935#discussion_r102556528
--- Diff: server/test/com/cloud/user/DomainManagerImplTest.java ---
@@ -134,4 +164,67 @@ public void testFindDomainByIdOrPathValidId() {
Assert.assertEquals(domain, domainManager.findDomainByIdOrPath(1L,
"/validDomain/"));
}
+@Test(expected=InvalidParameterValueException.class)
+public void testDeleteDomainNullDomain() {
+Mockito.when(_domainDao.findById(DOMAIN_ID)).thenReturn(null);
+domainManager.deleteDomain(DOMAIN_ID, testDomainCleanup);
+}
+
+@Test(expected=PermissionDeniedException.class)
+public void testDeleteDomainRootDomain() {
+
Mockito.when(_domainDao.findById(Domain.ROOT_DOMAIN)).thenReturn(domain);
+domainManager.deleteDomain(Domain.ROOT_DOMAIN, testDomainCleanup);
+}
+
+//TODO testDeleteDomainCleanup
+
+@Test
+public void testDeleteDomainNoCleanup() {
+domainManager.deleteDomain(DOMAIN_ID, testDomainCleanup);
+Mockito.verify(domainManager).deleteDomain(domain,
testDomainCleanup);
+
Mockito.verify(domainManager).checkDomainAccountsNetworksAndResourcesBeforeRemoving(domain);
+Mockito.verify(domainManager).cleanupDomainOfferings(DOMAIN_ID);
+Mockito.verify(lock).unlock();
+Assert.assertEquals(false, domainManager.getRollBackState());
+}
+
+@Test
+public void
testCheckDomainAccountsNetworksAndResourcesBeforeRemovingRemoveDomain() {
+
domainManager.checkDomainAccountsNetworksAndResourcesBeforeRemoving(domain);
+
Mockito.verify(domainManager).publishRemoveEventsAndRemoveDomain(domain);
+}
+
+@Test(expected=CloudRuntimeException.class)
+public void
testCheckDomainAccountsNetworksAndResourcesBeforeRemovingDontRemoveDomain() {
+domainNetworkIds.add(2l);
+
domainManager.checkDomainAccountsNetworksAndResourcesBeforeRemoving(domain);
+Mockito.verify(domainManager).failRemoveOperation(domain,
domainAccountsForCleanup, domainNetworkIds, false);
+}
+
+@Test
+public void testPublishRemoveEventsAndRemoveDomainSuccessfulDelete() {
+domainManager.publishRemoveEventsAndRemoveDomain(domain);
+Mockito.verify(_messageBus).publish(Mockito.anyString(),
Matchers.eq(DomainManager.MESSAGE_PRE_REMOVE_DOMAIN_EVENT),
+Matchers.eq(PublishScope.LOCAL), Matchers.eq(domain));
+Mockito.verify(_messageBus).publish(Mockito.anyString(),
Matchers.eq(DomainManager.MESSAGE_REMOVE_DOMAIN_EVENT),
+Matchers.eq(PublishScope.LOCAL), Matchers.eq(domain));
+Mockito.verify(_domainDao).remove(DOMAIN_ID);
+}
+
+@Test(expected=CloudRuntimeException.class)
+public void testPublishRemoveEventsAndRemoveDomainExceptionDelete() {
+Mockito.when(_domainDao.remove(DOMAIN_ID)).thenReturn(false);
+domainManager.publishRemoveEventsAndRemoveDomain(domain);
+Mockito.verify(_messageBus).publish(Mockito.anyString(),
Matchers.eq(DomainManager.MESSAGE_PRE_REMOVE_DOMAIN_EVENT),
+Matchers.eq(PublishScope.LOCAL), Matchers.eq(domain));
+Mockito.verify(_messageBus,
Mockito.never()).publish(Mockito.anyString(),
Matchers.eq(DomainManager.MESSAGE_REMOVE_DOMAIN_EVENT),
+Matchers.eq(PublishScope.LOCAL), Matchers.eq(domain));
+Mockito.verify(_domainDao).remove(DOMAIN_ID);
+}
+
+@Test(expected=CloudRuntimeException.class)
+public void testFailRemoveOperation() {
--- End diff --
Thanks! I removed `expected` from `@Test` annotation and added a catch
block to assert exception class. It is a nasty method to test indeed :)
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1961: Fix for test_snapshots.py using nfs2 instead of nfs ...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1961 @karuturi @borisstoyanov next step will be updating Marvin's folder `test_data.py` file and configure an url for key "nfs2" similar as it was done for "nfs", this way it can be used to mount nfs in running environment. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1935: CLOUDSTACK-9764: Delete domain failure due to Accoun...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1935 Hi @rafaelweingartner, I've refactored the code instead of using `rollBackState` as static. I think that using static variable could lead to a problem if new methods are invoked from another method different than `deleteDomain` method. Instead of declaring it as static, we reduced the scope again and only set it true when `CloudRuntimeException` is thrown. What do you think about this refactor? I tryied not to introduce major changes in original 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1961: Fix for test_snapshots.py using nfs2 instead of nfs ...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1961 Hi @karuturi @rhtyd @borisstoyanov, I've pushed a new commit for fixing `test_snapshots.py` failure. Can you please run tests against Vmware and Kvm? These were results in our env, using Vmware: [root@ussarlabcsmgt41 cloudstack]# cat /tmp//MarvinLogs//XJVTRH/results.txt Test Snapshot Root Disk ... === TestName: test_01_snapshot_root_disk | Status : SUCCESS === ok Test listing volume snapshots with removed data stores ... === TestName: test_02_list_snapshots_with_removed_data_store | Status : SUCCESS === ok -- Ran 2 tests in 410.231s OK --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1961: Fix for test_snapshots.py using nfs2 instead ...
Github user nvazquez commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1961#discussion_r103932149 --- Diff: test/integration/smoke/test_snapshots.py --- @@ -275,7 +275,7 @@ def test_02_list_snapshots_with_removed_data_store(self): assert isinstance(clusters,list) and len(clusters)>0 storage = StoragePool.create(self.apiclient, --- End diff -- Done, thanks for pointing it out! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1961: Fix for test_snapshots.py using nfs2 instead of nfs ...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1961 @borisstoyanov actually disk is not being dettached from vm before migrating it, it is using vm's ROOT disk, it can be done on Vmware by setting `livemigrate='true'` to migrate command. I'll work on refactoring the test so that migration can be achieved --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1961: Fix for test_snapshots.py using nfs2 instead of nfs ...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1961 @borisstoyanov I refactored marvin test to migrate a detached disk instead of vm's root volume as it was before. Can you please test it again in your env? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1961: Fix for test_snapshots.py using nfs2 instead of nfs ...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1961 That's great! Thanks @borisstoyanov for your help! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1879: CLOUDSTACK-9719: [VMware] VR loses DHCP settings and...
Github user nvazquez commented on the issue:
https://github.com/apache/cloudstack/pull/1879
Sure @serg38, after enabling HA on cluster, stopping VR and the starting VR
we got this exception on ESXi 6:
2017-03-06 12:18:14,654 ERROR [cloud.vm.VmWorkJobDispatcher]
(Work-Job-Executor-4:ctx-d275214a job-8521/job-8522) Unable to complete
AsyncJobVO {id:8522, userId: 2, accountId: 2, instanceType: null, instanceId:
null, cmd: com.cloud.vm.VmWorkStart, cmdInfo:
rO0ABXNyABhjb20uY2xvdWQudm0uVm1Xb3JrU3RhcnR9cMGsvxz73gIAC0oABGRjSWRMAAZhdm9pZHN0ADBMY29tL2Nsb3VkL2RlcGxveS9EZXBsb3ltZW50UGxhbm5lciRFeGNsdWRlTGlzdDtMAAljbHVzdGVySWR0ABBMamF2YS9sYW5nL0xvbmc7TAAGaG9zdElkcQB-AAJMAAtqb3VybmFsTmFtZXQAEkxqYXZhL2xhbmcvU3RyaW5nO0wAEXBoeXNpY2FsTmV0d29ya0lkcQB-AAJMAAdwbGFubmVycQB-AANMAAVwb2RJZHEAfgACTAAGcG9vbElkcQB-AAJMAAlyYXdQYXJhbXN0AA9MamF2YS91dGlsL01hcDtMAA1yZXNlcnZhdGlvbklkcQB-AAN4cgATY29tLmNsb3VkLnZtLlZtV29ya5-ZtlbwJWdrAgAESgAJYWNjb3VudElkSgAGdXNlcklkSgAEdm1JZEwAC2hhbmRsZXJOYW1lcQB-AAN4cAACAAIhUHQAGVZpcnR1YWxNYWNoaW5lTWFuYWdlckltcGwAAHBwcHBwcHBwc3IAEWphdmEudXRpbC5IYXNoTWFwBQfawcMWYNEDAAJGAApsb2FkRmFjdG9ySQAJdGhyZXNob2xkeHA_QAAADHcIEAF0AA5SZXN0YXJ0TmV0d29ya
3QAP3JPMEFCWE55QUJGcVlYWmhMbXhoYm1jdVFtOXZiR1ZoYnMwZ2NvRFZuUHJ1QWdBQldnQUZkbUZzZFdWNGNBRXhw,
cmdVersion: 0, status: IN_PROGRESS, processStatus: 0, resultCode: 0, result:
null, initMsid: 345051565299, completeMsid: null, lastUpdated: null,
lastPolled: null, created: Mon Mar 06 12:18:05 PST 2017}, job origin:8521
com.cloud.exception.InsufficientServerCapacityException: Unable to create a
deployment for VM[DomainRouter|r-8528-VM]Scope=interface
com.cloud.dc.DataCenter; id=1
at
com.cloud.vm.VirtualMachineManagerImpl.orchestrateStart(VirtualMachineManagerImpl.java:961)
at
com.cloud.vm.VirtualMachineManagerImpl.orchestrateStart(VirtualMachineManagerImpl.java:4643)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at
com.cloud.vm.VmWorkJobHandlerProxy.handleVmWorkJob(VmWorkJobHandlerProxy.java:107)
at
com.cloud.vm.VirtualMachineManagerImpl.handleVmWorkJob(VirtualMachineManagerImpl.java:4804)
at
com.cloud.vm.VmWorkJobDispatcher.runJob(VmWorkJobDispatcher.java:102)
at
org.apache.cloudstack.framework.jobs.impl.AsyncJobManagerImpl$5.runInContext(AsyncJobManagerImpl.java:554)
at
org.apache.cloudstack.managed.context.ManagedContextRunnable$1.run(ManagedContextRunnable.java:49)
at
org.apache.cloudstack.managed.context.impl.DefaultManagedContext$1.call(DefaultManagedContext.java:56)
at
org.apache.cloudstack.managed.context.impl.DefaultManagedContext.callWithContext(DefaultManagedContext.java:103)
at
org.apache.cloudstack.managed.context.impl.DefaultManagedContext.runWithContext(DefaultManagedContext.java:53)
at
org.apache.cloudstack.managed.context.ManagedContextRunnable.run(ManagedContextRunnable.java:46)
at
org.apache.cloudstack.framework.jobs.impl.AsyncJobManagerImpl$5.run(AsyncJobManagerImpl.java:502)
Also, on vSphere we got this error: `Configuration vmConfig is not valid
for cluster CLD100`.
@sureshanaparti maybe this link could be useful to solve the issue:
https://communities.vmware.com/thread/467685?start=0&tstart=0
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1879: CLOUDSTACK-9719: [VMware] VR loses DHCP settings and...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1879 @sureshanaparti sure, we use version 6.0.0, build 3634794 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1994: CLOUDSTACK-9827: Storage tags stored in multi...
GitHub user nvazquez opened a pull request: https://github.com/apache/cloudstack/pull/1994 CLOUDSTACK-9827: Storage tags stored in multiple places Issue description: https://issues.apache.org/jira/browse/CLOUDSTACK-9827 ### Fixes - Create Primary Storage: Persist tags into `storage_pool_tags` instead of `storage_pool_details` - List Storage Tags: Queries `storage_pool_tags` table instead of `storage_tag_view` You can merge this pull request into a Git repository by running: $ git pull https://github.com/nvazquez/cloudstack CLOUDSTACK-9827 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack/pull/1994.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 #1994 commit e106919443949f704d77cee3dde7b36100609a73 Author: nvazquez Date: 2017-03-08T18:19:55Z CLOUDSTACK-9827: Storage tags stored in multiple places --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1994: CLOUDSTACK-9827: Storage tags stored in multiple pla...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1994 Hi @rafaelweingartner, you're right, it was basically that fix. I've pushed another commit due to issue reported by @mike-tutkowski in mailing list: I have an NFS SR as primary storage under CloudStack with a storage tag of NFS-A. I have a compute offering with a matching storage tag. I canât seem to get a VM to spin up, however: It says insufficient capacity. The CPU, MHz, and memory are all low (and what I typically use), so I think the problem is with matching the storage tag. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1994: CLOUDSTACK-9827: Storage tags stored in multiple pla...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1994 @serg38 actually is not being used anymore, I'll add removalof the view on last commit --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1994: CLOUDSTACK-9827: Storage tags stored in multiple pla...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1994 @mike-tutkowski awesome, thanks for testing this PR! @rafaelweingartner thanks for reviewing, I'll work on changes proposed @karuturi sure, I'll work on it, thanks! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1994: CLOUDSTACK-9827: Storage tags stored in multiple pla...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1994 @rafaelweingartner I pushed changes and squashed my commits as it could be easier to review. I also added unit tests for new methods --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1994: CLOUDSTACK-9827: Storage tags stored in multi...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1994#discussion_r105796076
--- Diff:
engine/schema/src/com/cloud/storage/dao/StoragePoolTagsDaoImpl.java ---
@@ -77,4 +90,71 @@ public void deleteTags(long poolId) {
txn.commit();
}
+@Override
+public List searchByIds(Long... stIds) {
+String batchCfg = _configDao.getValue("detail.batch.query.size");
--- End diff --
Done, thanks!
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1994: CLOUDSTACK-9827: Storage tags stored in multi...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1994#discussion_r105796786
--- Diff:
engine/schema/src/com/cloud/storage/dao/StoragePoolTagsDaoImpl.java ---
@@ -77,4 +90,71 @@ public void deleteTags(long poolId) {
txn.commit();
}
+@Override
+public List searchByIds(Long... stIds) {
+String batchCfg = _configDao.getValue("detail.batch.query.size");
--- End diff --
About number 2000, I assumed it was a default value for that configuration
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1994: CLOUDSTACK-9827: Storage tags stored in multi...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1994#discussion_r105796872
--- Diff:
engine/schema/src/com/cloud/storage/dao/StoragePoolTagsDaoImpl.java ---
@@ -77,4 +90,71 @@ public void deleteTags(long poolId) {
txn.commit();
}
+@Override
+public List searchByIds(Long... stIds) {
+String batchCfg = _configDao.getValue("detail.batch.query.size");
+
+final int detailsBatchSize = batchCfg != null ?
Integer.parseInt(batchCfg) : 2000;
+
+// query details by batches
+List uvList = new ArrayList();
+int curr_index = 0;
+
+if (stIds.length > detailsBatchSize) {
+while ((curr_index + detailsBatchSize) <= stIds.length) {
+Long[] ids = new Long[detailsBatchSize];
+
+for (int k = 0, j = curr_index; j < curr_index +
detailsBatchSize; j++, k++) {
+ids[k] = stIds[j];
+}
+
+SearchCriteria sc =
StoragePoolIdsSearch.create();
--- End diff --
Done, created method `searchForStoragePoolIdsInternal`
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1994: CLOUDSTACK-9827: Storage tags stored in multi...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1994#discussion_r105797159
--- Diff:
engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java
---
@@ -409,15 +460,13 @@ public StoragePoolVO persist(StoragePoolVO pool,
Map details) {
sc.and(sc.entity().getScope(), Op.EQ, ScopeType.ZONE);
return sc.list();
} else {
-Map details = tagsToDetails(tags);
-
-StringBuilder sql = new
StringBuilder(ZoneWideDetailsSqlPrefix);
+StringBuilder sql = new StringBuilder(ZoneWideTagsSqlPrefix);
--- End diff --
Thanks for pointing this out, I had missed it out. Created methods
`getSqlPreparedStatement` and `searchStoragePoolsPreparedStatement` which are
called from many methods and allow storage pool retrieval
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1918: Management Server UI (VM statistics page) CPU Utiliz...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1918 @jayakarteek @rafaelweingartner what about retrieving CPU performance metric for usage using [PerfomanceManager](https://pubs.vmware.com/vsphere-60/index.jsp#com.vmware.wssdk.apiref.doc/vim.PerformanceManager.html)? It is already used in `VmwareResource.getVmStats` lines 5134-5185 to retrieve vm network metrics. I think that similar as it is done for these metrics, we must query for available metrics and look for [usage CPU counter](https://pubs.vmware.com/vsphere-60/index.jsp?topic=%2Fcom.vmware.wssdk.apiref.doc%2Fcpu_counters.html) which already comes as percentage for a given interval which would have to be low. This way we don't have to calculate depending on limit flag. What do you think about this approach? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1278: CLOUDSTACK-9198: Virtual router gets deployed in dis...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1278 Hi @rafaelweingartner @anshul1886 @GabrielBrascher, I've read this PR's comments several times and I think I could understand @anshul1886's point. Please correct me if I'm wrong. The execution of `getCallingAccount()` is setting the context with the proper account, and I think that's fine, as next methods will use it (e.g. `orchestrateStart` in `VirtualMachineManagerImpl` lines 829-831). I also agree with @rafaelweingartner and @GabrielBrascher that even though the context is being set, variables `user` and `caller` on `start` method (defined on line 266) are not being used. @anshul1886, if no validations are required and the context is already set, don't you think that those unused parameters can be removed? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1879: CLOUDSTACK-9719: [VMware] VR loses DHCP settings and...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1879 @sureshanaparti I pulled the latest code and repeated the first test scenario, got the same failure: `Message: The setting of vmConfig is invalid for cluster CLD100.`. I attach the full [management server logs](https://github.com/apache/cloudstack/files/842198/startVR-HA-on.zip) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1994: CLOUDSTACK-9827: Storage tags stored in multi...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1994#discussion_r105989477
--- Diff:
engine/schema/src/com/cloud/storage/dao/StoragePoolTagsDaoImpl.java ---
@@ -77,4 +90,68 @@ public void deleteTags(long poolId) {
txn.commit();
}
+@Override
+public List searchByIds(Long... stIds) {
+final int detailsBatchSize = getDetailsBatchSize();
+
+// query details by batches
+List uvList = new ArrayList();
+int curr_index = 0;
+
+if (stIds.length > detailsBatchSize) {
+while ((curr_index + detailsBatchSize) <= stIds.length) {
+searchForStoragePoolIdsInternal(curr_index,
detailsBatchSize, stIds, uvList);
+curr_index += detailsBatchSize;
+}
+}
+
+if (curr_index < stIds.length) {
+int batch_size = (stIds.length - curr_index);
+searchForStoragePoolIdsInternal(curr_index, batch_size, stIds,
uvList);
+}
+
+return uvList;
+}
+
+/**
+ * Search storage pools
+ * @param currIndex current index
+ * @param batchSize batch size
+ * @param stIds storage tags array
+ * @param uvList
+ */
+protected void searchForStoragePoolIdsInternal(int currIndex, int
batchSize, Long[] stIds, List uvList) {
--- End diff --
Excellent improvement, I copied almost exactly as you wrote it :) I also
added some description on `uvList` parameter, which also renamed to `pools`
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1994: CLOUDSTACK-9827: Storage tags stored in multi...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1994#discussion_r105990460
--- Diff:
engine/schema/src/com/cloud/storage/dao/StoragePoolTagsDaoImpl.java ---
@@ -77,4 +90,68 @@ public void deleteTags(long poolId) {
txn.commit();
}
+@Override
+public List searchByIds(Long... stIds) {
+final int detailsBatchSize = getDetailsBatchSize();
+
+// query details by batches
+List uvList = new ArrayList();
+int curr_index = 0;
+
+if (stIds.length > detailsBatchSize) {
+while ((curr_index + detailsBatchSize) <= stIds.length) {
+searchForStoragePoolIdsInternal(curr_index,
detailsBatchSize, stIds, uvList);
+curr_index += detailsBatchSize;
+}
+}
+
+if (curr_index < stIds.length) {
+int batch_size = (stIds.length - curr_index);
+searchForStoragePoolIdsInternal(curr_index, batch_size, stIds,
uvList);
+}
+
+return uvList;
+}
+
+/**
+ * Search storage pools
+ * @param currIndex current index
+ * @param batchSize batch size
+ * @param stIds storage tags array
+ * @param uvList
+ */
+protected void searchForStoragePoolIdsInternal(int currIndex, int
batchSize, Long[] stIds, List uvList) {
+Long[] ids = new Long[batchSize];
+for (int k = 0, j = currIndex; j < currIndex + batchSize; j++,
k++) {
+ids[k] = stIds[j];
+}
+SearchCriteria sc =
StoragePoolIdsSearch.create();
+sc.setParameters("idIN", (Object[])ids);
+List vms = searchIncludingRemoved(sc, null,
null, false);
+if (vms != null) {
+uvList.addAll(vms);
+}
+}
+
+/**
+ * Retrieve {@code detail.batch.query.size} configuration value. If
not available, return the default value (2000)
+ * @return detail.batch.query.size configuration value
+ */
+protected int getDetailsBatchSize() {
+String batchCfg = _configDao.getValue("detail.batch.query.size");
+return batchCfg != null ? Integer.parseInt(batchCfg) : 2000;
--- End diff --
Done, thanks!
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1994: CLOUDSTACK-9827: Storage tags stored in multi...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1994#discussion_r105992671
--- Diff:
engine/schema/test/com/cloud/storage/dao/StoragePoolTagsDaoImplTest.java ---
@@ -0,0 +1,105 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.storage.dao;
+
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Matchers;
+import org.mockito.Mock;
+import org.mockito.Spy;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import com.cloud.storage.StoragePoolTagVO;
+import com.cloud.utils.db.Filter;
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
+
+import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.verify;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import junit.framework.TestCase;
+
+@RunWith(PowerMockRunner.class)
+public class StoragePoolTagsDaoImplTest extends TestCase {
+
+@Mock
+ConfigurationDao _configDao;
+@Mock
+SearchBuilder StoragePoolIdsSearch;
+
+@Spy
+@InjectMocks
+private StoragePoolTagsDaoImpl _storagePoolTagsDaoImpl = new
StoragePoolTagsDaoImpl();
+
+@Mock
+static StoragePoolTagVO storagePoolTag1;
--- End diff --
Not really, thinking about it there's no why to make them static, I removed
static declaration from them
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1994: CLOUDSTACK-9827: Storage tags stored in multi...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1994#discussion_r105998903
--- Diff:
engine/schema/src/com/cloud/storage/dao/StoragePoolTagsDaoImpl.java ---
@@ -77,4 +92,71 @@ public void deleteTags(long poolId) {
txn.commit();
}
+@Override
+public List searchByIds(Long... stIds) {
+final int detailsBatchSize = getDetailsBatchSize();
+
+// query details by batches
+List uvList = new ArrayList();
+int curr_index = 0;
+
+if (stIds.length > detailsBatchSize) {
--- End diff --
I think that `if` is needed to control the max query size. I agree with the
example you provided, but let's say for example that lengthOfStIds is greater
that batchSize. If we remove that `if`, we will load pools on lines 100-112 but
with a query size greater that batchSize (defined in line 111)
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1994: CLOUDSTACK-9827: Storage tags stored in multi...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1994#discussion_r106002237
--- Diff:
engine/schema/test/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImplTest.java
---
@@ -0,0 +1,151 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.storage.datastore.db;
+
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.verify;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import
org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDaoImpl.ValueType;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Matchers;
+import org.mockito.Mock;
+import org.mockito.Spy;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import com.cloud.storage.ScopeType;
+import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.storage.dao.StoragePoolTagsDao;
+
+import junit.framework.TestCase;
+
+@RunWith(PowerMockRunner.class)
+public class PrimaryDataStoreDaoImplTest extends TestCase {
+
+@Mock
+StoragePoolDetailsDao _detailsDao;
+@Mock
+StoragePoolHostDao _hostDao;
+@Mock
+StoragePoolTagsDao _tagsDao;
+
+@Spy
+@InjectMocks
+private static PrimaryDataStoreDaoImpl primaryDataStoreDao = new
PrimaryDataStoreDaoImpl();
+
+@Mock
+static StoragePoolVO storagePoolVO;
--- End diff --
Thanks! I removed 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1994: CLOUDSTACK-9827: Storage tags stored in multi...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1994#discussion_r106303439
--- Diff:
engine/schema/src/com/cloud/storage/dao/StoragePoolTagsDaoImpl.java ---
@@ -77,4 +92,71 @@ public void deleteTags(long poolId) {
txn.commit();
}
+@Override
+public List searchByIds(Long... stIds) {
+final int detailsBatchSize = getDetailsBatchSize();
+
+// query details by batches
+List uvList = new ArrayList();
+int curr_index = 0;
+
+if (stIds.length > detailsBatchSize) {
--- End diff --
Hi @rafaelweingartner, I assumed that you meant deleting the whole `if`
block (lines 103-108), is it correct or you mean just deleting line 103 (and
108)?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1994: CLOUDSTACK-9827: Storage tags stored in multi...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1994#discussion_r106311985
--- Diff:
engine/schema/src/com/cloud/storage/dao/StoragePoolTagsDaoImpl.java ---
@@ -77,4 +92,71 @@ public void deleteTags(long poolId) {
txn.commit();
}
+@Override
+public List searchByIds(Long... stIds) {
+final int detailsBatchSize = getDetailsBatchSize();
+
+// query details by batches
+List uvList = new ArrayList();
+int curr_index = 0;
+
+if (stIds.length > detailsBatchSize) {
--- End diff --
Nice, I agree with you that deleting the line can be removed. Sorry for the
confusion, I'll try adding some unit tests for this change. Thanks!
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1994: CLOUDSTACK-9827: Storage tags stored in multiple pla...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1994 Hi @karuturi, I've been working on marvin tests, I hope posting them today --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1994: CLOUDSTACK-9827: Storage tags stored in multi...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1994#discussion_r106724343
--- Diff:
engine/schema/src/com/cloud/storage/dao/StoragePoolTagsDaoImpl.java ---
@@ -77,4 +92,71 @@ public void deleteTags(long poolId) {
txn.commit();
}
+@Override
+public List searchByIds(Long... stIds) {
+final int detailsBatchSize = getDetailsBatchSize();
+
+// query details by batches
+List uvList = new ArrayList();
+int curr_index = 0;
+
+if (stIds.length > detailsBatchSize) {
--- End diff --
Done! Thanks a lot @rafaelweingartner!
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1994: CLOUDSTACK-9827: Storage tags stored in multiple pla...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1994 @karuturi I added marvin tests to simulate tests performed by @mike-tutkowski. This are results in our env: [root@ussarlabcsmgt41 cloudstack]# cat /tmp//MarvinLogs//011CTF/results.txt Test primary storage pools - XEN. Not Supported for kvm,hyperv,vmware ... SKIP: iscsi primary storage not supported on kvm, VMWare, Hyper-V, or LXC Test primary storage pools - XEN, KVM, VMWare. Not Supported for hyperv ... === TestName: test_01_primary_storage_nfs | Status : SUCCESS === ok Test Deploy VMS using different Service Offerings with Storage Tags ... === TestName: test_01_deploy_vms_storage_tags | Status : SUCCESS === ok Test edit Storage Tags ... === TestName: test_02_edit_primary_storage_tags | Status : SUCCESS === ok Test Volume migration options for Storage Pools with different Storage Tags ... SKIP: Skipping test as it is not running on simulator -- Ran 5 tests in 295.996s OK (SKIP=2) @rafaelweingartner @mike-tutkowski @karuturi can you please review marvin tests added? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1994: CLOUDSTACK-9827: Storage tags stored in multiple pla...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1994 Thanks @serg38, we are using `mgtSvr` details provided in .cfg file, should we use these for Marvin too? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1994: CLOUDSTACK-9827: Storage tags stored in multiple pla...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1994 Thanks @mike-tutkowski! I pushed force to kick off Travis again --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1994: CLOUDSTACK-9827: Storage tags stored in multiple pla...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1994 @karuturi I refactored last marvin test which was failing on Travis. These are results in our env: [root@ussarlabcsmgt41 cloudstack]# cat /tmp//MarvinLogs//4GSNSY/results.txt Test primary storage pools - XEN. Not Supported for kvm,hyperv,vmware ... SKIP: iscsi primary storage not supported on kvm, VMWare, Hyper-V, or LXC Test primary storage pools - XEN, KVM, VMWare. Not Supported for hyperv ... === TestName: test_01_primary_storage_nfs | Status : SUCCESS === ok Test Deploy VMS using different Service Offerings with Storage Tags ... === TestName: test_01_deploy_vms_storage_tags | Status : SUCCESS === ok Test Edit Storage Tags ... === TestName: test_02_edit_primary_storage_tags | Status : SUCCESS === ok Test Volume migration options for Storage Pools with different Storage Tags ... === TestName: test_03_migration_options_storage_tags | Status : SUCCESS === ok -- Ran 5 tests in 442.216s OK (SKIP=1) @rhtyd @borisstoyanov @jburwell on `test_primary_storage.py` we are creating 2 primary storages, using urls in `"nfs"` and `"nfs2"` test data templates. As PR #1961 uses `"nfs2"` template on `test_snapshots.py`, should it be a problem for BlueOrangutan? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1994: CLOUDSTACK-9827: Storage tags stored in multiple pla...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1994 @karuturi Travis is now failing as it doesn't find key "nfs2" 2017-03-24 17:33:45,621 - CRITICAL - EXCEPTION: test_03_migration_options_storage_tags: ['Traceback (most recent call last):\n', ' File "/opt/python/2.7.12/lib/python2.7/unittest/case.py", line 329, in run\ntestMethod()\n', ' File "/home/travis/.local/lib/python2.7/site-packages/marvin/lib/decoratorGenerators.py", line 30, in test_wrapper\nreturn test(self, *args, **kwargs)\n', ' File "/home/travis/build/apache/cloudstack/test/integration/smoke/test_primary_storage.py", line 520, in test_03_migration_options_storage_tags\n self.services["nfs2"],\n', "KeyError: 'nfs2'\n"] It is introduced in PR #1961, can these two PRs be merged together? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #2021: CLOUDSTACK-9854: Fix test_primary_storage tes...
GitHub user nvazquez opened a pull request:
https://github.com/apache/cloudstack/pull/2021
CLOUDSTACK-9854: Fix test_primary_storage test failure due to live migration
Fix for test_primary_storage integration tests on simulator.
When finding storage pool migration options for volume on running vm, API
returns None as hypervisor doesn't support live migration.
2017-03-28 06:07:55,958 - DEBUG - Sending GET Cmd :
findStoragePoolsForMigration===
2017-03-28 06:07:55,977 - DEBUG - Response : None
2017-03-28 06:07:55,983 - CRITICAL - EXCEPTION:
test_03_migration_options_storage_tags: ['Traceback (most recent call
last):\n', ' File "/opt/python/2.7.12/lib/python2.7/unittest/case.py", line
329, in run\ntestMethod()\n', ' File
"/home/travis/.local/lib/python2.7/site-packages/marvin/lib/decoratorGenerators.py",
line 30, in test_wrapper\nreturn test(self, *args, **kwargs)\n', ' File
"/home/travis/build/apache/cloudstack/test/integration/smoke/test_primary_storage.py",
line 547, in test_03_migration_options_storage_tags\npools_suitable =
filter(lambda p : p.suitableformigration, pools_response)\n', "TypeError:
'NoneType' object is not iterable\n"]
So we simply stop vm before sending findStoragePoolsForMigration command
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nvazquez/cloudstack CLOUDSTACK-9854
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/2021.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 #2021
commit e313dafea46cf281bf09cc66cfcaf6a38d53ca90
Author: nvazquez
Date: 2017-03-28T14:35:55Z
CLOUDSTACK-9854: Fix test_primary_storage test failure due to live migration
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #2021: CLOUDSTACK-9854: Fix test_primary_storage test failu...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/2021 Thanks @borisstoyanov, I made little refactor due to failure, can you please re kick tests? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #2019: CLOUDSTACK-9851 travis CI build failure after merge ...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/2019 LGTM, travis failure is not related to this PR, it is fixed in #2021 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #2021: CLOUDSTACK-9854: Fix test_primary_storage test failu...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/2021 @karuturi sure, done! I rebased master and pushed, now Travis passes! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1944: CLOUDSTACK-9783: Improve metrics view performance
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1944 Hi @rhtyd, thanks for this great improvement! Along with @serg38 we've been testing in our env and got some failures on `listHostsMetrics` and `listVolumeMetrics`: test_list_clusters_metrics (test_metrics_api.TestMetrics) ... === TestName: test_list_clusters_metrics | Status : SUCCESS === ok test_list_hosts_metrics (test_metrics_api.TestMetrics) ... === TestName: test_list_hosts_metrics | Status : EXCEPTION === ERROR test_list_infrastructure_metrics (test_metrics_api.TestMetrics) ... === TestName: test_list_infrastructure_metrics | Status : SUCCESS === ok test_list_pstorage_metrics (test_metrics_api.TestMetrics) ... === TestName: test_list_pstorage_metrics | Status : SUCCESS === ok test_list_vms_metrics (test_metrics_api.TestMetrics) ... === TestName: test_list_vms_metrics | Status : SUCCESS === ok test_list_volumes_metrics (test_metrics_api.TestMetrics) ... === TestName: test_list_volumes_metrics | Status : EXCEPTION === ERROR test_list_zones_metrics (test_metrics_api.TestMetrics) ... === TestName: test_list_zones_metrics | Status : SUCCESS === ok [root@ussarlabcsmgt41 cloudstack]# cat /tmp//MarvinLogs//1A23HU/failed_plus_exceptions.txt 2017-03-31 10:21:19,514 - CRITICAL - EXCEPTION: test_list_hosts_metrics: ['Traceback (most recent call last):\n', ' File "/usr/local/lib/python2.7/unittest/case.py", line 331, in run\n testMethod()\n', ' File "/home/t_vazqnadmin/cloudstack/test/integration/smoke/test_metrics_api.py", line 77, in test_list_hosts_metrics\nhost_metric = self.apiclient.listHostsMetrics(cmd)[0]\n', ' File "/usr/local/lib/python2.7/site-packages/marvin/cloudstackAPI/cloudstackAPIClient.py", line 3094, in listHostsMetrics\nresponse = self.connection.marvinRequest(command, response_type=response, method=method)\n', ' File "/usr/local/lib/python2.7/site-packages/marvin/cloudstackConnection.py", line 379, in marvinRequest\nraise e\n', "CloudstackAPIException: Execute cmd: listhostsmetrics failed, due to: errorCode: 530, errorText:No value specified for 'Date'\n"] 2017-03-31 10:23:50,276 - CRITICAL - EXCEPTION: test_list_volumes_metrics: ['Traceback (most recent call last):\n', ' File "/usr/local/lib/python2.7/unittest/case.py", line 331, in run\n testMethod()\n', ' File "/home/t_vazqnadmin/cloudstack/test/integration/smoke/test_metrics_api.py", line 172, in test_list_volumes_metrics\nlvm = self.apiclient.listVolumesMetrics(cmd)[0]\n', ' File "/usr/local/lib/python2.7/site-packages/marvin/cloudstackAPI/cloudstackAPIClient.py", line 954, in listVolumesMetrics\nresponse = self.connection.marvinRequest(command, response_type=response, method=method)\n', ' File "/usr/local/lib/python2.7/site-packages/marvin/cloudstackConnection.py", line 379, in marvinRequest\nraise e\n', "CloudstackAPIException: Execute cmd: listvolumesmetrics failed, due to: errorCode: 530, errorText:No value specified for 'Date'\n"] Exception is thrown by `BeanUtils.copyProperties`, log on management server: 2017-03-31 10:28:33,257 ERROR [cloud.api.ApiServer] (catalina-exec-15:ctx-7fb0b31e ctx-6ded5acb) unhandled exception executing api command: [Ljava.lang.String;@63f1496d org.apache.commons.beanutils.ConversionException: No value specified for 'Date' at org.apache.commons.beanutils.converters.AbstractConverter.handleMissing(AbstractConverter.java:310) at org.apache.commons.beanutils.converters.AbstractConverter.convert(AbstractConverter.java:136) at org.apache.commons.beanutils.converters.ConverterFacade.convert(ConverterFacade.java:60) at org.apache.commons.beanutils.BeanUtilsBean.convert(BeanUtilsBean.java:1078) at org.apache.commons.beanutils.BeanUtilsBean.copyProperty(BeanUtilsBean.java:437) at org.apache.commons.beanutils.BeanUtilsBean.copyProperties(BeanUtilsBean.java:286) at org.apache.commons.beanutils.BeanUtils.copyProperties(BeanUtils.java:137) at org.apache.cloudstack.metrics.MetricsServiceImpl.listHostMetrics(MetricsServiceImpl.java:244) at org.apache.cloudstack.api.ListHostsMetricsCmd.execute(ListHostsMetricsCmd.java:48) at com.cloud.api.ApiDispatcher.dispatch(ApiDispatcher.java:150) at com.cloud.api.ApiServer.queueCommand(ApiServer.java:709) at com.cloud.api.ApiServer.handleRequest(ApiServer.java:533) at com.cloud.api.ApiServlet.processReq
[GitHub] cloudstack issue #1935: CLOUDSTACK-9764: Delete domain failure due to Accoun...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1935 @borisstoyanov I've rebased master branch, can we re-run tests on this PR? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1918: Management Server UI (VM statistics page) CPU Utiliz...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1918 LGTM for testing. Tested on Vmware 6.0, I attach some screenshots for test scenarios, in which vm was deployed using service offering, infinite loop was executed to increase CPU utilization and then stopped - Test case 1: Service offering not limiting CPU speed       - Test case 2: Service offering limiting CPU speed     @rafaelweingartner thanks for your effort in this PR @jayakarteek can you please rebase master branch and squash your commits? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1944: CLOUDSTACK-9783: Improve metrics view performance
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1944 Please ommit my last comment, we were using commons-beanutils version 1.8.3 instead of 1.9.2 LGTM [root@ussarlabcsmgt41 cloudstack]# cat /tmp//MarvinLogs//DVIXST/results.txt test_list_clusters_metrics (test_metrics_api.TestMetrics) ... === TestName: test_list_clusters_metrics | Status : SUCCESS === ok test_list_hosts_metrics (test_metrics_api.TestMetrics) ... === TestName: test_list_hosts_metrics | Status : SUCCESS === ok test_list_infrastructure_metrics (test_metrics_api.TestMetrics) ... === TestName: test_list_infrastructure_metrics | Status : SUCCESS === ok test_list_pstorage_metrics (test_metrics_api.TestMetrics) ... === TestName: test_list_pstorage_metrics | Status : SUCCESS === ok test_list_vms_metrics (test_metrics_api.TestMetrics) ... === TestName: test_list_vms_metrics | Status : SUCCESS === ok test_list_volumes_metrics (test_metrics_api.TestMetrics) ... === TestName: test_list_volumes_metrics | Status : SUCCESS === ok test_list_zones_metrics (test_metrics_api.TestMetrics) ... === TestName: test_list_zones_metrics | Status : SUCCESS === ok -- Ran 7 tests in 142.134s OK --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack issue #1879: CLOUDSTACK-9719: [VMware] VR loses DHCP settings and...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1879 Thanks @sureshanaparti! I tested scenarios and work as expected! I attach some screenshots: - Test scenario 1: Enable HA after VR created, stop VR, start VR.  - Test scenario 2: HA enabled before VR created. deployed VM  --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1935: CLOUDSTACK-9764: Delete domain failure due to...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1935#discussion_r110421376
--- Diff: server/src/com/cloud/user/DomainManagerImpl.java ---
@@ -273,82 +284,145 @@ public boolean deleteDomain(long domainId, Boolean
cleanup) {
@Override
public boolean deleteDomain(DomainVO domain, Boolean cleanup) {
-// mark domain as inactive
-s_logger.debug("Marking domain id=" + domain.getId() + " as " +
Domain.State.Inactive + " before actually deleting it");
-domain.setState(Domain.State.Inactive);
-_domainDao.update(domain.getId(), domain);
-boolean rollBackState = false;
-boolean hasDedicatedResources = false;
+GlobalLock lock = getGlobalLock("AccountCleanup");
+if (lock == null) {
+s_logger.debug("Couldn't get the global lock");
+return false;
+}
+
+if (!lock.lock(30)) {
+s_logger.debug("Couldn't lock the db");
+return false;
+}
try {
-long ownerId = domain.getAccountId();
-if ((cleanup != null) && cleanup.booleanValue()) {
-if (!cleanupDomain(domain.getId(), ownerId)) {
-rollBackState = true;
-CloudRuntimeException e =
-new CloudRuntimeException("Failed to clean up
domain resources and sub domains, delete failed on domain " + domain.getName()
+ " (id: " +
-domain.getId() + ").");
-e.addProxyObject(domain.getUuid(), "domainId");
-throw e;
-}
-} else {
-//don't delete the domain if there are accounts set for
cleanup, or non-removed networks exist, or domain has dedicated resources
-List networkIds =
_networkDomainDao.listNetworkIdsByDomain(domain.getId());
-List accountsForCleanup =
_accountDao.findCleanupsForRemovedAccounts(domain.getId());
-List dedicatedResources =
_dedicatedDao.listByDomainId(domain.getId());
-if (dedicatedResources != null &&
!dedicatedResources.isEmpty()) {
-s_logger.error("There are dedicated resources for the
domain " + domain.getId());
-hasDedicatedResources = true;
-}
-if (accountsForCleanup.isEmpty() && networkIds.isEmpty()
&& !hasDedicatedResources) {
-_messageBus.publish(_name,
MESSAGE_PRE_REMOVE_DOMAIN_EVENT, PublishScope.LOCAL, domain);
-if (!_domainDao.remove(domain.getId())) {
-rollBackState = true;
-CloudRuntimeException e =
-new CloudRuntimeException("Delete failed on
domain " + domain.getName() + " (id: " + domain.getId() +
-"); Please make sure all users and sub
domains have been removed from the domain before deleting");
-e.addProxyObject(domain.getUuid(), "domainId");
-throw e;
-}
-_messageBus.publish(_name,
MESSAGE_REMOVE_DOMAIN_EVENT, PublishScope.LOCAL, domain);
+// mark domain as inactive
+s_logger.debug("Marking domain id=" + domain.getId() + " as "
+ Domain.State.Inactive + " before actually deleting it");
+domain.setState(Domain.State.Inactive);
+_domainDao.update(domain.getId(), domain);
+
+boolean rollBackState = false;
+
+try {
+long ownerId = domain.getAccountId();
+if (BooleanUtils.toBoolean(cleanup)) {
+tryCleanupDomain(domain, ownerId);
} else {
-rollBackState = true;
-String msg = null;
-if (!accountsForCleanup.isEmpty()) {
-msg = accountsForCleanup.size() + " accounts to
cleanup";
-} else if (!networkIds.isEmpty()) {
-msg = networkIds.size() + " non-removed networks";
-} else if (hasDedicatedResources) {
-msg = "dedicated resources.";
-}
+
removeDomainWithNoAccountsForCleanupNetworksOrDedicatedResources(domain);
+
[GitHub] cloudstack pull request #1935: CLOUDSTACK-9764: Delete domain failure due to...
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1935#discussion_r110421451
--- Diff: server/test/com/cloud/user/DomainManagerImplTest.java ---
@@ -134,4 +164,69 @@ public void testFindDomainByIdOrPathValidId() {
Assert.assertEquals(domain, domainManager.findDomainByIdOrPath(1L,
"/validDomain/"));
}
+@Test(expected=InvalidParameterValueException.class)
+public void testDeleteDomainNullDomain() {
+Mockito.when(_domainDao.findById(DOMAIN_ID)).thenReturn(null);
+domainManager.deleteDomain(DOMAIN_ID, testDomainCleanup);
+}
+
+@Test(expected=PermissionDeniedException.class)
+public void testDeleteDomainRootDomain() {
+
Mockito.when(_domainDao.findById(Domain.ROOT_DOMAIN)).thenReturn(domain);
+domainManager.deleteDomain(Domain.ROOT_DOMAIN, testDomainCleanup);
+}
+
+@Test
+public void testDeleteDomainNoCleanup() {
+domainManager.deleteDomain(DOMAIN_ID, testDomainCleanup);
+Mockito.verify(domainManager).deleteDomain(domain,
testDomainCleanup);
+
Mockito.verify(domainManager).removeDomainWithNoAccountsForCleanupNetworksOrDedicatedResources(domain);
+Mockito.verify(domainManager).cleanupDomainOfferings(DOMAIN_ID);
+Mockito.verify(lock).unlock();
+}
+
+@Test
+public void
testRemoveDomainWithNoAccountsForCleanupNetworksOrDedicatedResourcesRemoveDomain()
{
+
domainManager.removeDomainWithNoAccountsForCleanupNetworksOrDedicatedResources(domain);
+
Mockito.verify(domainManager).publishRemoveEventsAndRemoveDomain(domain);
+}
+
+@Test(expected=CloudRuntimeException.class)
+public void
testRemoveDomainWithNoAccountsForCleanupNetworksOrDedicatedResourcesDontRemoveDomain()
{
+domainNetworkIds.add(2l);
+
domainManager.removeDomainWithNoAccountsForCleanupNetworksOrDedicatedResources(domain);
+Mockito.verify(domainManager).failRemoveOperation(domain,
domainAccountsForCleanup, domainNetworkIds, false);
+}
+
+@Test
+public void testPublishRemoveEventsAndRemoveDomainSuccessfulDelete() {
+domainManager.publishRemoveEventsAndRemoveDomain(domain);
+Mockito.verify(_messageBus).publish(Mockito.anyString(),
Matchers.eq(DomainManager.MESSAGE_PRE_REMOVE_DOMAIN_EVENT),
+Matchers.eq(PublishScope.LOCAL), Matchers.eq(domain));
+Mockito.verify(_messageBus).publish(Mockito.anyString(),
Matchers.eq(DomainManager.MESSAGE_REMOVE_DOMAIN_EVENT),
+Matchers.eq(PublishScope.LOCAL), Matchers.eq(domain));
+Mockito.verify(_domainDao).remove(DOMAIN_ID);
+}
+
+@Test(expected=CloudRuntimeException.class)
+public void testPublishRemoveEventsAndRemoveDomainExceptionDelete() {
+Mockito.when(_domainDao.remove(DOMAIN_ID)).thenReturn(false);
+domainManager.publishRemoveEventsAndRemoveDomain(domain);
+Mockito.verify(_messageBus).publish(Mockito.anyString(),
Matchers.eq(DomainManager.MESSAGE_PRE_REMOVE_DOMAIN_EVENT),
+Matchers.eq(PublishScope.LOCAL), Matchers.eq(domain));
+Mockito.verify(_messageBus,
Mockito.never()).publish(Mockito.anyString(),
Matchers.eq(DomainManager.MESSAGE_REMOVE_DOMAIN_EVENT),
+Matchers.eq(PublishScope.LOCAL), Matchers.eq(domain));
+Mockito.verify(_domainDao).remove(DOMAIN_ID);
+}
+
+@Test
+public void testFailRemoveOperation() {
+try {
+domainManager.failRemoveOperation(domain,
domainAccountsForCleanup, domainNetworkIds, true);
--- End diff --
Great, thanks!
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1935: CLOUDSTACK-9764: Delete domain failure due to Accoun...
Github user nvazquez commented on the issue: https://github.com/apache/cloudstack/pull/1935 @rafaelweingartner thanks for reviewing again! Minor refactor pushed --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9074: Support shared networkin...
Github user nvazquez commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1094#discussion_r48416674 --- Diff: api/src/com/cloud/network/guru/NetworkGuruAdditionalFunctions.java --- @@ -0,0 +1,12 @@ +package com.cloud.network.guru; --- End diff -- @DaanHoogland Thanks Daan. I added it now --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9074: Support shared networkin...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1094#issuecomment-167150961 @remibergsma Thanks a lot Remi! I rebased with master, moved sql to 4.8.0 schema and pushed it again. @miguelaferreira Thanks a lot for your help and your effort testing this features! Merry Chistmas guys --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9074: Support shared networkin...
Github user nvazquez closed the pull request at: https://github.com/apache/cloudstack/pull/1094 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9074: Support shared networkin...
GitHub user nvazquez reopened a pull request: https://github.com/apache/cloudstack/pull/1094 CLOUDSTACK-9074: Support shared networking in NiciraNVP Plugin JIRA TICKET: https://issues.apache.org/jira/browse/CLOUDSTACK-9074 Design Document: https://cwiki.apache.org/confluence/display/CLOUDSTACK/Support+shared+networking+in+NiciraNVP+Plugin You can merge this pull request into a Git repository by running: $ git pull https://github.com/nvazquez/cloudstack sharedNiciraNVP Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack/pull/1094.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 #1094 commit 46f9fbabdf2fdb3aa7ca92fd8a54a9a0a9443f4d Author: nvazquez Date: 2015-12-01T19:46:22Z CLOUDSTACK-9074: API Changes: Add l2gatewayserviceuuid to NVP devices commit 8149081658738a2d4d9ee088241310460b2d9077 Author: nvazquez Date: 2015-12-01T19:48:27Z CLOUDSTACK-9074: API Changes: Add nsxLogicalSwitch and nsxLogicalSwitchPort to listNics commit 06d5b46e538ba4d1a88995bff4aef5fe2e5275e7 Author: nvazquez Date: 2015-12-01T19:50:52Z CLOUDSTACK-9074: New NiciraNVP classes to support Shared Networks commit c67637180f9d7be627a4fe36fd2709c58c952e94 Author: nvazquez Date: 2015-12-01T19:54:27Z CLOUDSTACK-9074: Support Shared Networks in NiciraNVP Plugin commit 55f460772e5e9872115947b47fa40579c9026dfe Author: nvazquez Date: 2015-12-04T18:20:44Z CLOUDSTACK-9074: New NiciraNVP classes for FindLRouterPort and DeleteLRouterPort API methods commit eb889c0c49313184609d31d4492c58eb0ff19f31 Author: nvazquez Date: 2015-12-09T18:55:30Z CLOUDSTACK-9074: API add Gateway Service Find method commit 07264204f55ccf5ccf724c539ec1f477dc88a190 Author: nvazquez Date: 2015-12-09T18:57:12Z CLOUDSTACK-9074: Drop nicira_nvp_router_map unique index on logicalrouter_uuid commit 4ac2737754a20c0ff8ccb3f1b9622e0651fe1b8b Author: nvazquez Date: 2015-12-09T18:58:02Z CLOUDSTACK-9074: Marvin tests for NSX Shared Networks Support commit 88774a93e88d83bdf2868b382f66ecfae441a69a Author: Miguel Ferreira Date: 2015-12-24T10:08:42Z Only set L2 Gateway in NSX device if defined commit 3dba689f31a89f6b17b3018dfb2dc6f689698812 Author: Miguel Ferreira Date: 2015-12-24T10:08:59Z Add helper method to migrate router vms commit f804c9756d07fcff70aa18a9c50bc30841374f92 Author: Miguel Ferreira Date: 2015-12-24T10:10:18Z Use helper method to migrate router vm commit acdc42cb48f66e76f6337844f3409460da7631b1 Author: Miguel Ferreira Date: 2015-12-24T10:11:00Z Fix method call bugs when accessign non defined variables commit c285d6cfb4f7aaf53bd317cfd80382a5894dfcec Author: Miguel Ferreira Date: 2015-12-24T10:12:41Z Use NSX specific config values instead of zone config values commit 0b20ed4074484113973d8f447b8b8a79c778210d Author: nvazquez Date: 2015-12-24T14:33:06Z CLOUDSTACK-9074: Add NetworkGuruAdditionalFunctions license commit de23c94f33c51cb3c72ceb801e86f3888cfb8604 Author: nvazquez Date: 2015-12-24T19:12:01Z CLOUDSTACK-9074: Move sql to 4.8.0 schema --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9074: Support shared networkin...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1094#issuecomment-167347304 @DaanHoogland Done --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9074: Support shared networkin...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1094#issuecomment-167362000 @DaanHoogland I see that all checks have passed, what is it failing? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9211: Support passing vRAM siz...
GitHub user nvazquez opened a pull request: https://github.com/apache/cloudstack/pull/1310 CLOUDSTACK-9211: Support passing vRAM size to support 3D GPU on Vmware JIRA TICKET: https://issues.apache.org/jira/browse/CLOUDSTACK-9211 CS support passing hypervisor options by creating entries in vm_template_details or user_vm_details To enable software 3D GPU 4 options needs to be added: | name| value | |:-:|:-:| |mks.enable3d|true| |mks.use3dRenderer|automatic| |svga.autodetect|false| |svga.vramSize|(size in KB) e.g. 131072| Currently all options except svga.vramSize works, VMs always get configured with default 64Mb videoRAM instead of the one provided on the option. You can merge this pull request into a Git repository by running: $ git pull https://github.com/nvazquez/cloudstack 3dgpu Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack/pull/1310.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 #1310 commit 656ae109377e7d4400d16f05cbb4096e20a8c05b Author: nvazquez Date: 2016-01-05T18:45:01Z CLOUDSTACK-9211: Support passing vRAM size to support 3D GPU on Vmware --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9211: Support passing vRAM siz...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1310#issuecomment-171752709 Hi @cristofolini Thanks for your comments. I refactored the method to make it more readable into two methods, and added description to both, like javadoc. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9211: Support passing vRAM siz...
Github user nvazquez commented on the pull request:
https://github.com/apache/cloudstack/pull/1310#issuecomment-173350754
Thanks @cristofolini
I've tryied writing a marvin test for testing this feature which would be:
* Select (or create) a vm (id=) which is already running and stop it.
* For vm with id= do the following:
INSERT INTO cloud.user_vm_details (vm_id, name, value) VALUES
('', 'mks.enable3d', 'true');
INSERT INTO cloud.user_vm_details (vm_id, name, value) VALUES
('', 'mks.use3dRenderer', 'automatic');
INSERT INTO cloud.user_vm_details (vm_id, name, value) VALUES
('', 'svga.autodetect', 'false');
INSERT INTO cloud.user_vm_details (vm_id, name, value) VALUES
('', 'svga.vramSize', '');
Where is the ram size in KB
* Start vm again
In previous behaviour, it all worked except vram size, which was always set
to 64MB, ignoring size provided. With this fix, vram size is
set to when provided.
I couldn't be able to write a marvin test which follows these steps,
however I wrote a unit test for this new method, which is only accessed through
execute(StartCommand) method.
Nicolas
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9211: Support passing vRAM siz...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1310#issuecomment-173439752 Thanks for your help @cristofolini! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: Support configurable NFS version for Seco...
GitHub user nvazquez opened a pull request: https://github.com/apache/cloudstack/pull/1361 Support configurable NFS version for Secondary Storage mounts JIRA Ticket: https://issues.apache.org/jira/browse/CLOUDSTACK-9252 ### Description of the problem After starting secondary storage VM, secondary storage tries to be mounted but fails with error: Protocol family not supported It was found out that adding -o vers=X to mount command it would work, where X is the desired NFS version to use. If it is desired to mount a store with a specific NFS version, it has passed in image_store_details table for a store with id Y as a property: | store_id| name| value | |:-:|:-:|:-:| |Y|nfs.version|X (e.g. 3)| Where X stands for NFS version You can merge this pull request into a Git repository by running: $ git pull https://github.com/nvazquez/cloudstack bothgoals Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack/pull/1361.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 #1361 commit 15745be426b474b75ae61a6226a8982029c345ca Author: nvazquez Date: 2016-01-22T18:39:47Z CLOUDSTACK-9252: Add nfs version to commands commit 59631a94b86fbeac9d836943ae4450ea1b407da1 Author: nvazquez Date: 2016-01-22T18:41:23Z CLOUDSTACK-9252: Support configurable nfs version --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-175056442 Hi @rafaelweingartner, Thanks for your review! I agree with @serg38, it was thought just to extend the functionality by adding an entry on image_store_details table with nfs version, but if the entry was not added we wanted it to continue working as it was. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-175103859 Ok, @rafaelweingartner I'll work on refactoring this. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9211: Support passing vRAM siz...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1310#issuecomment-176351414 Hi @bhaisaab , Which test files would you like me to run? Is this ok with this command on each file? nosetests --with-marvin --marvin-config=setup/dev/advanced.cfg test/integration/...file.py Or is it a command to run multiple files? Thanks --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-176752434 @rafaelweingartner I pushed some changes according to your comments. Is it better like this? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-178190394 Hi @rafaelweingartner Thanks for your comments! I've been working following your indications, however I didn't create any test case. I wanted to ask you, as this methods use properties from database, which way could be better to test it. I thought on unit tests, mocking daos to retrieve nfs version, would that be enough? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-178346694 @rafaelweingartner thanks for your reply! I'm familiar to Spring, I've worked with it, I know how it works but I wouldn't say I know it in depth. If you don't mind I would find your detailed explanation really helpful for this feature and future tasks. About bean injection in TemplateServiceImpl, I first tryied @Inject annotation, but ACS couldn't start because it couldn't find a bean of type ImageStoreDetailsUtil. I had the same issue with @Autowired annotation. The only solution I could find was adding bean in xml context file. I agree with you about not extending Manager interface, I saw it was always done like that and just followed the pattern. I will remove that from ImageStoreDetailsUtil I could start ACS but didn't fully test Vmware* classes. I would try to get the context and then the bean from it as you suggested. Thanks a lot for your help! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9211: Support passing vRAM siz...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1310#issuecomment-180363651 Hi @bhaisaab can you help me testing this? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-181041714 Hi @DaanHoogland can you help us with Jenkins build? A test file SecondaryStorageManagerTest.java is failing, but that file is not in the branch. I could compile without problems but when I push my changes that test fails, making Jenkins fail. Do you know what could be happening? Thanks --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-181571630 Thanks @DaanHoogland, I pushed again --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-181644342 This time it failed for timeout --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-181646997 Sure, I'll do 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-181909261 Thanks a lot @rafaelweingartner for your words and your help! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-185194859 Sure @kishankavala, to set NFS version for a store with id STORE_ID you'll need to insert a record in image_store_details_table which has: * store_id = STORE_ID * name = nfs.version * value = X where X is the desired NFS version e.g. 2, 3, 4 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9211: Support passing vRAM siz...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1310#issuecomment-186444960 Hi @bhaisaab sorry I couldn't post this earlier but I've been working on another pull request and couldn't be able to test this one before. As my chages affects vm deployments, I only ran test_deploy_vms_with_varied_deploymentplanners.py, test_reset_vm_on_reboot.py and test_vm_life_cycle.py but I can run all the test if you want ``` # nosetests --with-marvin --marvin-config=setup/dev/advanced.cfg.new test/integration/smoke/test_deploy_vms_with_varied_deploymentplanners.py test/integration/smoke/test_reset_vm_on_reboot.py Marvin Init Started === Marvin Parse Config Successful === === Marvin Setting TestData Successful=== Log Folder Path: /tmp//MarvinLogs//Feb_19_2016_14_55_40_UF5PE3. All logs will be available here === Marvin Init Logging Successful=== Marvin Init Successful ===final results are now copied to: /tmp//MarvinLogs/test_reset_vm_on_reboot_S6H4PW=== ``` ``` # cat /tmp//MarvinLogs/test_reset_vm_on_reboot_S6H4PW/results.txt Test to deploy vm with a first fit offering ... === TestName: test_deployvm_firstfit | Status : SUCCESS === ok Test deploy VMs using user concentrated planner ... === TestName: test_deployvm_userconcentrated | Status : SUCCESS === ok Test deploy VMs using user dispersion planner ... === TestName: test_deployvm_userdispersing | Status : SUCCESS === ok Test reset virtual machine on reboot ... === TestName: test_01_reset_vm_on_reboot | Status : SUCCESS === ok -- Ran 4 tests in 571.499s OK ``` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: From4.5.1: NSX/Nicira Plugin does not sup...
GitHub user nvazquez opened a pull request: https://github.com/apache/cloudstack/pull/935 From4.5.1: NSX/Nicira Plugin does not support NSX v4.2.1 JIRA Ticket: https://issues.apache.org/jira/browse/CLOUDSTACK-8956 You can merge this pull request into a Git repository by running: $ git pull https://github.com/nvazquez/cloudstack from4.5.1 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack/pull/935.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 #935 commit 82e786ff430276390d4b50d5f154ca1493a88e76 Author: nvazquez Date: 2015-09-23T20:23:19Z Add log before portgroup creation commit 68a5bb07f8596779da0f885a08a33ddd7b212d2f Author: nvazquez Date: 2015-09-23T20:36:09Z Logger fix commit 847048b2721b4aeb67983bdadd1d984732577d3a Author: nvazquez Date: 2015-09-23T21:03:23Z Json portgroup commit 9d6a53cc4730c3cf1a597ee7b11b1b710e9948e8 Author: nvazquez Date: 2015-09-24T14:05:03Z Logs before create port commit 9c4645e571a389f15294247142e4db66caf409c2 Author: nvazquez Date: 2015-09-24T16:09:31Z Logging opaqueNetwork commit 6b6431767f684e02f139b00e9987594dad1d2dfb Author: nvazquez Date: 2015-09-24T18:03:12Z Remove retrieve opaquenetwork commit 9a287ac001d65b8b74977767e049bb8dd47872ee Author: nvazquez Date: 2015-09-24T19:44:14Z Only logging networks commit da5eb1a4b17d4e6b245161e5e5c69b927864dede Author: nvazquez Date: 2015-09-24T20:02:21Z Add summary network to properties commit 0dfd291f597152b806a836659cc41ce2c736e27e Author: nvazquez Date: 2015-09-24T21:02:15Z Retrieve opaque networks commit 328c6808e38afddf7126441f328901f119eaa513 Author: nvazquez Date: 2015-09-25T12:26:16Z Extending host network info with opaque networks commit ac71af39b6a0818cfd3eff41267ad37c0ea460bd Author: nvazquez Date: 2015-09-25T12:33:51Z Fixes opaque network commit 6f8b9b683f95ebc3c1cd4c28056622081947544e Author: nvazquez Date: 2015-09-25T12:38:46Z Fix checkstyle commit a313247407195d779a49b38570206ca333914f27 Author: nvazquez Date: 2015-09-25T14:50:01Z vpshere api 5.5 commit 240de4c486418e549c4d13385729af6b23354e66 Author: nvazquez Date: 2015-09-25T15:21:15Z Fix commit 661a80af49f6ccb0a7e304c92e6690f77ea52558 Author: nvazquez Date: 2015-09-25T20:17:43Z Trying opaqueNetwork search commit 93bb3c21af1c0b14f3a492ec02696ffc8e57409b Author: nvazquez Date: 2015-09-28T13:45:58Z Fix iter commit 962eb72339707465046834affb2e3ad9ab3ee80a Author: nvazquez Date: 2015-09-28T15:36:10Z Opaque netowrks commit 85b8ec2880118349395621cdee53f9c7f5d494c9 Author: nvazquez Date: 2015-09-28T16:00:29Z Opaque network class commit a431593119e77ff77e20e03d216ec89540502da3 Author: nvazquez Date: 2015-09-28T18:09:16Z Opaque networks commit 30583786852cfabc245db76d5fa808547478789c Author: nvazquez Date: 2015-09-28T18:41:06Z Opaque networks filter commit 68a10b5976684a1b114bb52db17be62c85fc Author: nvazquez Date: 2015-09-28T19:44:35Z Opaque networks filters commit 03b0aab89b8a3e2cb9b6bc418a184e26a0dc2b2e Author: nvazquez Date: 2015-09-28T20:15:58Z Opaque networks commit 93bad52c43d20942289e101f6bbdac1a77602a58 Author: nvazquez Date: 2015-09-28T21:38:47Z MorNetwork commit 015b84e2e41426a15d64a4afa88c639006a35a68 Author: nvazquez Date: 2015-09-29T15:34:18Z vm92 commit f3efb69073a280bf90fa98e29367044759b6a2b9 Author: nvazquez Date: 2015-09-29T15:43:25Z vm92 commit 92b5149548f733b47cadec72878b05cfcf807a68 Author: nvazquez Date: 2015-09-29T15:58:59Z vm92 commit c3c2db7bfb2fd559c4466aa373b4cb1600d1cf05 Author: nvazquez Date: 2015-09-29T16:02:11Z opaque commit be835c027393ce9bc1fee22a4231b2f850f4edc9 Author: nvazquez Date: 2015-09-29T16:25:47Z opaque commit 8dfca735bc5f85f7c8b9a143a74b6c54618c5034 Author: nvazquez Date: 2015-09-29T19:48:55Z Changes commit 663b17d97e133b5b8c43bd884967dd4c3d09c14a Author: nvazquez Date: 2015-09-29T20:21:41Z GuestNic --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: From4.5.1: NSX/Nicira Plugin does not sup...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/935#issuecomment-148715124 Hi @miguelaferreira @remibergsma Thanks a lot for your help. I will try to make a better description of the problem and work on commits and code as you suggested. Sorry for unorganized commits, this is my first contribution to CS, I will try to make it better. @miguelaferreira I don't have that test file (test/integration/smoke/test_nicira_controller.py) on my branch. I've created this branch from tag 4.5.1, maybe it wasn't available yet. How can I do for testing it? Thanks, Nicolas --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: From4.5.1: NSX/Nicira Plugin does not sup...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/935#issuecomment-149582907 @miguelaferreira @remibergsma Thanks again for your help! As you suggested I rebased master branch. I also added a more detailed description of the patch on this PR description and JIRA ticket. Thanks, Nicolas --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: From4.5.1: NSX/Nicira Plugin does not sup...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/935#issuecomment-150012697 ### VMware vSphere API version from 5.1 to 5.5: Since vSphere API version 5.5, [OpaqueNetworks](https://www.vmware.com/support/developer/converter-sdk/conv55_apireference/vim.OpaqueNetwork.html) are introduced. Its description says: > This interface defines an opaque network, in the sense that the detail and configuration of the network is unknown to vShpere and is managed by a management plane outside of vSphere. However, the identifier and name of these networks is made available to vSphere so that host and virtual machine virtual ethernet device can connect to them. In order to connect a vm's virtual ethernet device to the proper opaque network when deploying a vm into a NSX managed network, we first need to look for a particular opaque network on hosts. This opaque network's id has to be **"br-int"** and its type **"nsx.network"**. Since vSphere API version 5.5 [HostNetworkInfo](https://www.vmware.com/support/developer/converter-sdk/conv55_apireference/vim.host.NetworkInfo.html#opaqueNetwork) introduces a list of available opaque networks for each host. If NSX API version >= 4.2 we look for a [OpaqueNetworkInfo](https://www.vmware.com/support/developer/converter-sdk/conv55_apireference/vim.host.OpaqueNetworkInfo.html) which satisfies: * opaqueNetworkId = "br-int" * opaqueNetworkType = "nsx.netork" If that opaque network is found, then we need to attach vm's NIC to a virtual ethernet device which support this, so we use [VirtualEthernetCardOpaqueNetworkBackingInfo](https://www.vmware.com/support/developer/converter-sdk/conv55_apireference/vim.vm.device.VirtualEthernetCard.OpaqueNetworkBackingInfo.html) setting: * opaqueNetworkId = "br-int" * opaqueNetworkType = "nsx.netork" --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/935#issuecomment-150676496 @miguelaferreira thanks a lot for your help and advices! As suggested I post test_nicira_controller.py results: $ cat /tmp/MarvinLogs/test_nicira_controller_77FOV9/results.txt test_01_nicira_controller (integration.smoke.test_nicira_controller.TestNiciraContoller) ... === TestName: test_01_nicira_controller | Status : SUCCESS === ok Nicira clusters will redirect clients (in this case ACS) to the master node. ... === TestName: test_02_nicira_controller_redirect | Status : SUCCESS === ok -- Ran 2 tests in 398.132s OK --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/935#issuecomment-155418071 @wilderrodrigues Thanks a lot Wilder! @miguelaferreira Thanks for following this PR and testing 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/935#issuecomment-157154339 @miguelaferreira Great, I rebased my branch to master and pushed again --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/935#issuecomment-157718592 @remibergsma It looks like it is not finding 5.5 jar. These are the steps I followed: * Download VMware-vSphere-SDK-5.5.0-1284541.zip file from https://my.vmware.com/group/vmware/get-download?downloadGroup=WEBSDK550 * Extract zip file, cd SDK/vsphere-ws/java/JAXWS/lib/ and rename vim25_51.jar to vim25_55.jar * Copy SDK/vsphere-ws/java/JAXWS/lib/vim25_55.jar to CS deps folder * In CS deps folder: ./install-non-oss.sh --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/935#issuecomment-157727519 @DaanHoogland Hi! No, just renaming the old jar (version 5.1) will fail because it doesn't contain opaque network classes. Those ones are introduced in the 5.5 version jar. I added a line in deps/install-non-oss.sh which includes 5.5 jar to maven repo and also replaced cs.vmware.api.version property in pom.xml file from 5.1 to 5.5 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/935#issuecomment-157731551 @DaanHoogland No, it's not. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/935#issuecomment-157739765 @DaanHoogland Ok, sure --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-8956: NSX/Nicira Plugin does n...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/935#issuecomment-157782057 @DaanHoogland those warnings are in files that don't belong to my pull request. I could see in http://jenkins.buildacloud.org/job/build-master-slowbuild/2660/changes#detail0 that it compiled 5 commits, but the first one is commited by other contributor. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9074: Support shared networkin...
GitHub user nvazquez opened a pull request: https://github.com/apache/cloudstack/pull/1094 CLOUDSTACK-9074: Support shared networking in NiciraNVP Plugin JIRA TICKET: https://issues.apache.org/jira/browse/CLOUDSTACK-9074 Design Document: https://cwiki.apache.org/confluence/display/CLOUDSTACK/Support+shared+networking+in+NiciraNVP+Plugin You can merge this pull request into a Git repository by running: $ git pull https://github.com/nvazquez/cloudstack sharedNiciraNVP Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack/pull/1094.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 #1094 commit e3ebd00831250bdb67419e19fef005316e3b30db Author: nvazquez Date: 2015-11-19T17:34:30Z CLOUDSTACK-9074: API changes add l2gatewayserviceuuid to NVP devices commit 68719b4c81c1cd657b75b44348df7537c8687c32 Author: nvazquez Date: 2015-11-19T17:48:25Z CLOUDSTACK-9074: API changes listNics commit f7bad11e3524e7c415ac779ec8dceea589da3618 Author: nvazquez Date: 2015-11-19T17:50:00Z CLOUDSTACK-9074: New NiciraNVP classes to support Shared Networks commit 1d1a0a392b2a9f7b3948148f2068bfb1a2800296 Author: nvazquez Date: 2015-11-19T17:51:40Z CLOUDSTACK-9074: Support Shared Networks on NiciraNVP Plugin commit 2f7cc2b2fb4051143b25df6eaca371cd8db24313 Author: nvazquez Date: 2015-11-19T17:52:33Z CLOUDSTACK-9074: As shared networks are supported, need to drop logicalrouter_uuid index in nicira_nvp_router_map --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9074: Support shared networkin...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1094#issuecomment-159007416 Thanks @remibergsma ! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9074: Support shared networkin...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1094#issuecomment-159634391 @remibergsma thanks Remi, I rebased to master branch and resolved a few conflicts --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request:
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/commit/e66dd8c1e00abb9a3b4cbe0b8b50c83b251a728c#commitcomment-14620035 Hi @davidamorimfaria , I had the same issue that @pdion891 had. I added missing quotes as you suggested in PR#1114 but still getting an error: ``` [root@XYZ packaging]# ./package.sh -p noredist -d centos63 -p noredist -d centos63 -- Packaging CloudStack... noredist Preparing to package Apache CloudStack 4.7.0-SNAPSHOT . preparing source tarball . executing rpmbuild error: failed to stat /home/t_vazqnadmin/cloudstack/dist/rpmbuild/DEFOSSNOSS: No such file or directory RPM Build Failed ``` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request: CLOUDSTACK-9074: Support shared networkin...
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1094#issuecomment-160636372 @miguelaferreira thanks for reviewing! Sure, would be great to discuss them --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
